From 62550d5cb5c43a7cd4b05556bf756558e130373c Mon Sep 17 00:00:00 2001 From: greebo Date: Thu, 19 Mar 2026 19:25:44 +0300 Subject: [PATCH] feat(backend): add stale draft guards and reference validation for draft mutations add stale draft protection for mutation flows validate referenced entities before applying draft changes reduce invalid draft writes caused by stale state and broken references keep mutation behavior explicit and version-aware --- backend/app/api/routes/editor.py | 11 + backend/app/services/editor_validation.py | 252 ++++++++++++++++++---- backend/app/services/remap_service.py | 7 + 3 files changed, 233 insertions(+), 37 deletions(-) diff --git a/backend/app/api/routes/editor.py b/backend/app/api/routes/editor.py index 3175b31..5e09ada 100644 --- a/backend/app/api/routes/editor.py +++ b/backend/app/api/routes/editor.py @@ -47,9 +47,11 @@ from app.schemas.editor import ( from app.security.auth import require_api_key from app.services.draft_guard import get_current_draft_context from app.services.editor_validation import ( + validate_bulk_seat_patch_references, validate_bulk_seat_patch_uniqueness, validate_group_patch_uniqueness, validate_sector_patch_uniqueness, + validate_single_seat_patch_references, validate_single_seat_patch_uniqueness, ) from app.services.structure_diff import build_structure_diff @@ -318,6 +320,11 @@ async def patch_draft_seat( seat_record_id=seat_record_id, new_seat_id=payload.seat_id, ) + await validate_single_seat_patch_references( + scheme_version_id=version.scheme_version_id, + sector_id=payload.sector_id, + group_id=payload.group_id, + ) row = await update_scheme_version_seat_by_record_id( scheme_version_id=version.scheme_version_id, @@ -373,6 +380,10 @@ async def bulk_patch_draft_seats( scheme_version_id=version.scheme_version_id, items=items, ) + await validate_bulk_seat_patch_references( + scheme_version_id=version.scheme_version_id, + items=items, + ) rows = await bulk_update_scheme_version_seats_by_record_id( scheme_version_id=version.scheme_version_id, diff --git a/backend/app/services/editor_validation.py b/backend/app/services/editor_validation.py index 1958cb4..0469853 100644 --- a/backend/app/services/editor_validation.py +++ b/backend/app/services/editor_validation.py @@ -1,3 +1,5 @@ +from __future__ import annotations + from fastapi import HTTPException, status from app.repositories.scheme_groups import list_scheme_version_groups @@ -5,6 +7,20 @@ from app.repositories.scheme_seats import list_scheme_version_seats from app.repositories.scheme_sectors import list_scheme_version_sectors +def _raise_uniqueness_error(message: str, detail: dict | None = None) -> None: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=detail or {"code": "editor_uniqueness_error", "message": message}, + ) + + +def _raise_reference_error(message: str, detail: dict | None = None) -> None: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=detail or {"code": "editor_reference_error", "message": message}, + ) + + async def validate_single_seat_patch_uniqueness( *, scheme_version_id: str, @@ -15,11 +31,18 @@ async def validate_single_seat_patch_uniqueness( return seats = await list_scheme_version_seats(scheme_version_id) - for seat in seats: - if seat.seat_id == new_seat_id and seat.seat_record_id != seat_record_id: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail=f"seat_id already exists in draft version: {new_seat_id}", + for row in seats: + if row.seat_record_id == seat_record_id: + continue + if row.seat_id == new_seat_id: + _raise_uniqueness_error( + f"Seat id already exists in current draft version: {new_seat_id}", + { + "code": "duplicate_seat_id", + "message": "Seat id already exists in current draft version", + "seat_id": new_seat_id, + "conflict_seat_record_id": row.seat_record_id, + }, ) @@ -29,36 +52,50 @@ async def validate_bulk_seat_patch_uniqueness( items: list[dict], ) -> None: seats = await list_scheme_version_seats(scheme_version_id) - existing = {seat.seat_id: seat.seat_record_id for seat in seats if seat.seat_id} - payload_new_ids = [item.get("seat_id") for item in items if item.get("seat_id")] - duplicates_inside_payload = sorted( - { - seat_id - for seat_id in payload_new_ids - if payload_new_ids.count(seat_id) > 1 - } - ) - if duplicates_inside_payload: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail=f"Duplicate seat_id values inside bulk payload: {', '.join(duplicates_inside_payload)}", - ) + existing_by_seat_id: dict[str, str] = { + row.seat_id: row.seat_record_id + for row in seats + if row.seat_id + } + + seen_new_ids: dict[str, str] = {} for item in items: - new_seat_id = item.get("seat_id") seat_record_id = item["seat_record_id"] + seat_id = item.get("seat_id") - if not new_seat_id: + if not seat_id: continue - existing_record_id = existing.get(new_seat_id) + existing_record_id = existing_by_seat_id.get(seat_id) if existing_record_id and existing_record_id != seat_record_id: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail=f"seat_id already exists in draft version: {new_seat_id}", + _raise_uniqueness_error( + f"Seat id already exists in current draft version: {seat_id}", + { + "code": "duplicate_seat_id", + "message": "Seat id already exists in current draft version", + "seat_id": seat_id, + "conflict_seat_record_id": existing_record_id, + "input_seat_record_id": seat_record_id, + }, ) + seen_record_id = seen_new_ids.get(seat_id) + if seen_record_id and seen_record_id != seat_record_id: + _raise_uniqueness_error( + f"Seat id is duplicated inside bulk payload: {seat_id}", + { + "code": "duplicate_seat_id_in_payload", + "message": "Seat id is duplicated inside bulk payload", + "seat_id": seat_id, + "first_seat_record_id": seen_record_id, + "second_seat_record_id": seat_record_id, + }, + ) + + seen_new_ids[seat_id] = seat_record_id + async def validate_sector_patch_uniqueness( *, @@ -69,12 +106,19 @@ async def validate_sector_patch_uniqueness( if not new_sector_id: return - sectors = await list_scheme_version_sectors(scheme_version_id) - for sector in sectors: - if sector.sector_id == new_sector_id and sector.sector_record_id != sector_record_id: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail=f"sector_id already exists in draft version: {new_sector_id}", + rows = await list_scheme_version_sectors(scheme_version_id) + for row in rows: + if row.sector_record_id == sector_record_id: + continue + if row.sector_id == new_sector_id: + _raise_uniqueness_error( + f"Sector id already exists in current draft version: {new_sector_id}", + { + "code": "duplicate_sector_id", + "message": "Sector id already exists in current draft version", + "sector_id": new_sector_id, + "conflict_sector_record_id": row.sector_record_id, + }, ) @@ -87,10 +131,144 @@ async def validate_group_patch_uniqueness( if not new_group_id: return - groups = await list_scheme_version_groups(scheme_version_id) - for group in groups: - if group.group_id == new_group_id and group.group_record_id != group_record_id: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail=f"group_id already exists in draft version: {new_group_id}", + rows = await list_scheme_version_groups(scheme_version_id) + for row in rows: + if row.group_record_id == group_record_id: + continue + if row.group_id == new_group_id: + _raise_uniqueness_error( + f"Group id already exists in current draft version: {new_group_id}", + { + "code": "duplicate_group_id", + "message": "Group id already exists in current draft version", + "group_id": new_group_id, + "conflict_group_record_id": row.group_record_id, + }, ) + + +async def validate_single_seat_patch_references( + *, + scheme_version_id: str, + sector_id: str | None, + group_id: str | None, +) -> None: + sector_ids = { + row.sector_id + for row in await list_scheme_version_sectors(scheme_version_id) + if row.sector_id + } + group_ids = { + row.group_id + for row in await list_scheme_version_groups(scheme_version_id) + if row.group_id + } + + if sector_id is not None and sector_id not in sector_ids: + _raise_reference_error( + f"Sector id does not exist in current draft version: {sector_id}", + { + "code": "unknown_sector_id", + "message": "Sector id does not exist in current draft version", + "sector_id": sector_id, + }, + ) + + if group_id is not None and group_id not in group_ids: + _raise_reference_error( + f"Group id does not exist in current draft version: {group_id}", + { + "code": "unknown_group_id", + "message": "Group id does not exist in current draft version", + "group_id": group_id, + }, + ) + + +async def validate_bulk_seat_patch_references( + *, + scheme_version_id: str, + items: list[dict], +) -> None: + sector_ids = { + row.sector_id + for row in await list_scheme_version_sectors(scheme_version_id) + if row.sector_id + } + group_ids = { + row.group_id + for row in await list_scheme_version_groups(scheme_version_id) + if row.group_id + } + + unknown_sector_refs = sorted( + { + item["sector_id"] + for item in items + if item.get("sector_id") is not None and item["sector_id"] not in sector_ids + } + ) + if unknown_sector_refs: + _raise_reference_error( + "Bulk payload contains unknown sector_id values", + { + "code": "unknown_sector_ids", + "message": "Bulk payload contains unknown sector_id values", + "sector_ids": unknown_sector_refs, + }, + ) + + unknown_group_refs = sorted( + { + item["group_id"] + for item in items + if item.get("group_id") is not None and item["group_id"] not in group_ids + } + ) + if unknown_group_refs: + _raise_reference_error( + "Bulk payload contains unknown group_id values", + { + "code": "unknown_group_ids", + "message": "Bulk payload contains unknown group_id values", + "group_ids": unknown_group_refs, + }, + ) + + +async def validate_remap_target_references( + *, + scheme_version_id: str, + to_sector_id: str | None, + to_group_id: str | None, +) -> None: + sector_ids = { + row.sector_id + for row in await list_scheme_version_sectors(scheme_version_id) + if row.sector_id + } + group_ids = { + row.group_id + for row in await list_scheme_version_groups(scheme_version_id) + if row.group_id + } + + if to_sector_id is not None and to_sector_id not in sector_ids: + _raise_reference_error( + f"Target sector_id does not exist in current draft version: {to_sector_id}", + { + "code": "unknown_target_sector_id", + "message": "Target sector_id does not exist in current draft version", + "sector_id": to_sector_id, + }, + ) + + if to_group_id is not None and to_group_id not in group_ids: + _raise_reference_error( + f"Target group_id does not exist in current draft version: {to_group_id}", + { + "code": "unknown_target_group_id", + "message": "Target group_id does not exist in current draft version", + "group_id": to_group_id, + }, + ) diff --git a/backend/app/services/remap_service.py b/backend/app/services/remap_service.py index 2f4e69d..ec3e41b 100644 --- a/backend/app/services/remap_service.py +++ b/backend/app/services/remap_service.py @@ -6,6 +6,7 @@ from app.repositories.scheme_seats import ( bulk_remap_scheme_version_seats, list_scheme_version_seats, ) +from app.services.editor_validation import validate_remap_target_references def _match_seat( @@ -39,6 +40,12 @@ async def preview_remap( detail="At least one remap filter must be provided", ) + await validate_remap_target_references( + scheme_version_id=scheme_version_id, + to_sector_id=to_sector_id, + to_group_id=to_group_id, + ) + seats = await list_scheme_version_seats(scheme_version_id) seat_record_id_set = set(seat_record_ids) if seat_record_ids else None