fix(suppliers): do not cascade facility rows that arrive during removal

services/admin/app/supplier_facilities_routes.py

@@ -7,11 +7,11 @@
 from __future__ import annotations
 
 import uuid as uuid_module
-from typing import Any
+from typing import Any, NoReturn
 
 from fastapi import APIRouter, Depends, HTTPException, Query
 from shared.pagination import PaginationParams
-from sqlalchemy import delete, func, select
+from sqlalchemy import delete, exists, func, select
 from sqlalchemy.orm import Session
 
 from .database import get_session
@@ -134,6 +134,73 @@ async def create_supplier_facility(
     )
 
 
+_LINKED_RECORD_MODELS = (
+    SupplierTraceabilityLotModel,
+    SupplierCTEEventModel,
+    SupplierFunnelEventModel,
+)
+
+
+def _linked_record_counts(
+    db: Session,
+    tenant_id: uuid_module.UUID,
+    facility_id: uuid_module.UUID,
+) -> tuple[int, int, int]:
+    counts: list[int] = []
+    for model in _LINKED_RECORD_MODELS:
+        counts.append(
+            db.execute(
+                select(func.count())
+                .select_from(model)
+                .where(model.tenant_id == tenant_id, model.facility_id == facility_id)
+            ).scalar()
+            or 0
+        )
+    return counts[0], counts[1], counts[2]
+
+
+def _no_linked_records(tenant_id: uuid_module.UUID, facility_id: uuid_module.UUID) -> tuple[Any, ...]:
+    """WHERE clauses that match only a facility with no linked rows."""
+    return tuple(
+        ~exists(
+            select(model.id).where(model.tenant_id == tenant_id, model.facility_id == facility_id)
+        )
+        for model in _LINKED_RECORD_MODELS
+    )
+
+
+def _load_facility_for_removal(
+    db: Session,
+    facility_id: uuid_module.UUID,
+    tenant_id: uuid_module.UUID,
+) -> SupplierFacilityModel | None:
+    stmt = select(SupplierFacilityModel).where(
+        SupplierFacilityModel.id == facility_id,
+        SupplierFacilityModel.tenant_id == tenant_id,
+    )
+    bind = db.get_bind()
+    if bind is not None and bind.dialect.name == "postgresql":
+        stmt = stmt.with_for_update()
+    return db.execute(stmt).scalar_one_or_none()
+
+
+def _reject_linked_facility(
+    facility_name: str,
+    lots_count: int,
+    cte_events_count: int,
+    funnel_events_count: int,
+) -> NoReturn:
+    raise HTTPException(
+        status_code=409,
+        detail=(
+            f"Cannot remove facility '{facility_name}' with linked records: "
+            f"{lots_count} lots/TLCs, {cte_events_count} CTE events, "
+            f"{funnel_events_count} other records. "
+            f"Delete only when zero linked rows."
+        ),
+    )
+
+
 @router.get("/facilities/{facility_id}/removal-check", response_model=FacilityRemovalCheckResponse)
 async def check_supplier_facility_removal(
     facility_id: str,
@@ -154,26 +221,9 @@ async def check_supplier_facility_removal(
     if facility is None or facility.tenant_id != tenant_id:
         raise HTTPException(status_code=404, detail="Facility not found")
 
-    lots_count = db.execute(
-        select(func.count()).select_from(SupplierTraceabilityLotModel).where(
-            SupplierTraceabilityLotModel.tenant_id == tenant_id,
-            SupplierTraceabilityLotModel.facility_id == facility_uuid,
-        )
-    ).scalar() or 0
-
-    cte_events_count = db.execute(
-        select(func.count()).select_from(SupplierCTEEventModel).where(
-            SupplierCTEEventModel.tenant_id == tenant_id,
-            SupplierCTEEventModel.facility_id == facility_uuid,
-        )
-    ).scalar() or 0
-
-    funnel_events_count = db.execute(
-        select(func.count()).select_from(SupplierFunnelEventModel).where(
-            SupplierFunnelEventModel.tenant_id == tenant_id,
-            SupplierFunnelEventModel.facility_id == facility_uuid,
-        )
-    ).scalar() or 0
+    lots_count, cte_events_count, funnel_events_count = _linked_record_counts(
+        db, tenant_id, facility_uuid
+    )
 
     scoping_config_count = db.execute(
         select(func.count()).select_from(SupplierFacilityFTLCategoryModel).where(
@@ -225,56 +275,57 @@ async def delete_supplier_facility(
     except ValueError as exc:
         raise HTTPException(status_code=400, detail="Invalid facility id") from exc
 
-    facility = db.get(SupplierFacilityModel, facility_uuid)
-    if facility is None or facility.tenant_id != tenant_id:
+    # Lock the facility on PostgreSQL before counting. A lot or CTE insert
+    # takes a key share lock on this row, so it cannot commit until this
+    # transaction ends. Lots and CTE events are ON DELETE CASCADE: a count
+    # in an earlier statement would miss a row that commits before the
+    # DELETE, and the cascade would destroy it.
+    facility = _load_facility_for_removal(db, facility_uuid, tenant_id)
+    if facility is None:
         raise HTTPException(status_code=404, detail="Facility not found")
 
-    lots_count = db.execute(
-        select(func.count()).select_from(SupplierTraceabilityLotModel).where(
-            SupplierTraceabilityLotModel.tenant_id == tenant_id,
-            SupplierTraceabilityLotModel.facility_id == facility_uuid,
-        )
-    ).scalar() or 0
-
-    cte_events_count = db.execute(
-        select(func.count()).select_from(SupplierCTEEventModel).where(
-            SupplierCTEEventModel.tenant_id == tenant_id,
-            SupplierCTEEventModel.facility_id == facility_uuid,
-        )
-    ).scalar() or 0
-
-    funnel_events_count = db.execute(
-        select(func.count()).select_from(SupplierFunnelEventModel).where(
-            SupplierFunnelEventModel.tenant_id == tenant_id,
-            SupplierFunnelEventModel.facility_id == facility_uuid,
-        )
-    ).scalar() or 0
-
-    if lots_count > 0 or cte_events_count > 0 or funnel_events_count > 0:
-        raise HTTPException(
-            status_code=409,
-            detail=(
-                f"Cannot remove facility '{facility.name}' with linked records: "
-                f"{lots_count} lots/TLCs, {cte_events_count} CTE events, "
-                f"{funnel_events_count} other records. "
-                f"Delete only when zero linked rows."
-            ),
+    lots_count, cte_events_count, funnel_events_count = _linked_record_counts(
+        db, tenant_id, facility_uuid
+    )
+    if lots_count or cte_events_count or funnel_events_count:
+        _reject_linked_facility(facility.name, lots_count, cte_events_count, funnel_events_count)
+
+    facility_name = facility.name
+    # The DELETE re-checks linked rows in the same statement. A row that
+    # arrived after the count above makes this match nothing, so the
+    # cascade never runs.
+    result = db.execute(
+        delete(SupplierFacilityModel).where(
+            SupplierFacilityModel.id == facility_uuid,
+            SupplierFacilityModel.tenant_id == tenant_id,
+            *_no_linked_records(tenant_id, facility_uuid),
+        ),
+        execution_options={"synchronize_session": "fetch"},
+    )
+    if result.rowcount != 1:
+        db.rollback()
+        if _load_facility_for_removal(db, facility_uuid, tenant_id) is None:
+            raise HTTPException(status_code=404, detail="Facility not found")
+        lots_count, cte_events_count, funnel_events_count = _linked_record_counts(
+            db, tenant_id, facility_uuid
         )
+        _reject_linked_facility(facility_name, lots_count, cte_events_count, funnel_events_count)
 
+    # FTL scoping is configuration, not traceability evidence. Postgres
+    # already cascades it with the facility; this covers databases that
+    # are not enforcing that foreign key.
     db.execute(
         delete(SupplierFacilityFTLCategoryModel).where(
             SupplierFacilityFTLCategoryModel.tenant_id == tenant_id,
             SupplierFacilityFTLCategoryModel.facility_id == facility_uuid,
         )
     )
-
-    db.delete(facility)
     db.commit()
 
     return {
         "status": "deleted",
         "id": str(facility_uuid),
-        "facility_name": facility.name,
+        "facility_name": facility_name,
     }
 
 

services/admin/tests/test_supplier_facilities_routes_1342.py

@@ -26,6 +26,7 @@
 
 from __future__ import annotations
 
+from datetime import datetime, timezone
 from unittest.mock import MagicMock, patch
 from uuid import UUID
 
@@ -519,7 +520,7 @@ def test_lots_with_facility_filter(self, client: TestClient, db_session: Session
             supplier_user_id=TEST_USER_ID,
             facility_id=f.id,
             tlc_code="TLC-FILTER-001",
-            product_description="Spinach",
+            product_description="",
             status="active",
         )
         db_session.add(lot)
@@ -596,3 +597,98 @@ def _deny_owner():
             resp = member_client.delete("/v1/supplier/facilities/00000000-0000-0000-0000-000000000000")
             assert resp.status_code == 403
 
+    def test_delete_empty_facility(self, client: TestClient, db_session: Session):
+        facility = _make_facility(db_session)
+        resp = client.delete(f"/v1/supplier/facilities/{facility.id}")
+        assert resp.status_code == 200
+        assert resp.json()["status"] == "deleted"
+        db_session.expire_all()
+        assert db_session.get(SupplierFacilityModel, facility.id) is None
+
+    def test_linked_lot_returns_409(self, client: TestClient, db_session: Session):
+        facility = _make_facility(db_session)
+        db_session.add(
+            SupplierTraceabilityLotModel(
+                tenant_id=TEST_TENANT_ID,
+                supplier_user_id=TEST_USER_ID,
+                facility_id=facility.id,
+                tlc_code="",
+                product_description="",
+                status="active",
+            )
+        )
+        db_session.commit()
+
+        resp = client.delete(f"/v1/supplier/facilities/{facility.id}")
+        assert resp.status_code == 409
+        db_session.expire_all()
+        assert db_session.get(SupplierFacilityModel, facility.id) is not None
+
+    def test_stale_empty_count_does_not_cascade_lot(self, client: TestClient, db_session: Session, monkeypatch: pytest.MonkeyPatch):
+        """A lot committed after the pre-check must not be deleted with the facility.
+
+        Lots use ON DELETE CASCADE. The pre-check and the DELETE used to be
+        separate statements, so a lot that landed in between was cascaded away.
+        """
+        import app.supplier_facilities_routes as fac_routes
+
+        facility = _make_facility(db_session)
+        lot = SupplierTraceabilityLotModel(
+            tenant_id=TEST_TENANT_ID,
+            supplier_user_id=TEST_USER_ID,
+            facility_id=facility.id,
+            tlc_code="",
+            product_description="",
+            status="active",
+        )
+        db_session.add(lot)
+        db_session.commit()
+        monkeypatch.setattr(fac_routes, "_linked_record_counts", lambda *_args, **_kwargs: (0, 0, 0))
+
+        resp = client.delete(f"/v1/supplier/facilities/{facility.id}")
+        assert resp.status_code == 409
+        db_session.expire_all()
+        assert db_session.get(SupplierFacilityModel, facility.id) is not None
+        assert db_session.get(SupplierTraceabilityLotModel, lot.id) is not None
+
+    def test_stale_empty_count_does_not_cascade_cte_event(
+        self, client: TestClient, db_session: Session, monkeypatch: pytest.MonkeyPatch
+    ):
+        """An event filed on this facility must survive even when its lot lives elsewhere."""
+        import app.supplier_facilities_routes as fac_routes
+
+        target = _make_facility(db_session, "Target")
+        other = _make_facility(db_session, "Other")
+        lot = SupplierTraceabilityLotModel(
+            tenant_id=TEST_TENANT_ID,
+            supplier_user_id=TEST_USER_ID,
+            facility_id=other.id,
+            tlc_code="",
+            product_description="",
+            status="active",
+        )
+        db_session.add(lot)
+        db_session.commit()
+        event = SupplierCTEEventModel(
+            tenant_id=TEST_TENANT_ID,
+            supplier_user_id=TEST_USER_ID,
+            facility_id=target.id,
+            lot_id=lot.id,
+            cte_type="shipping",
+            event_time=datetime.now(timezone.utc),
+            kde_data={},
+            payload_sha256="",
+            merkle_hash="",
+            sequence_number=1,
+            obligation_ids=[],
+        )
+        db_session.add(event)
+        db_session.commit()
+        monkeypatch.setattr(fac_routes, "_linked_record_counts", lambda *_args, **_kwargs: (0, 0, 0))
+
+        resp = client.delete(f"/v1/supplier/facilities/{target.id}")
+        assert resp.status_code == 409
+        db_session.expire_all()
+        assert db_session.get(SupplierFacilityModel, target.id) is not None
+        assert db_session.get(SupplierCTEEventModel, event.id) is not None
+