ENG-3324: System pre-delete hook for cross-repo cascade cleanup

CHANGELOG.md

@@ -21,6 +21,9 @@ Changes can also be flagged with a GitHub label for tracking purposes. The URL o
 
 ## [Unreleased](https://github.com/ethyca/fides/compare/2.86.0..main)
 
+### Added
+- Added `system_pre_delete_hooks` registry so cross-repo consumers can snapshot dependent state before a system delete cascades. Fired from the single and bulk system-delete routes.
+
 ## [2.86.0](https://github.com/ethyca/fides/compare/2.85.1..2.86.0)
 
 ### Added

src/fides/api/system_connection_config_link_change_hooks.py

@@ -9,7 +9,7 @@
 recomputed when that linkage changes.
 
 Like ``system_steward_change_hooks``, this module exists ONLY because the
-event framework in fides PR #8096 is not yet merged. When it lands:
+event framework is not yet merged. When it lands:
 
   - Call sites of ``notify_system_connection_config_link_changed`` (in the
     ``SystemIntegrationLinkService`` mutators) are replaced with

src/fides/api/system_pre_delete_hooks.py

@@ -0,0 +1,70 @@
+"""
+System pre-delete hook — TEMPORARY EVENT-FRAMEWORK SHIM.
+
+Fires synchronously immediately BEFORE a ``System`` is deleted, while the
+system row and its dependent rows (``system_manager``,
+``system_connection_config_link``, ``monitorsteward.source_system_id`` refs)
+still exist. Registered callbacks may snapshot whatever state they need
+before FK cascade wipes the joins they depend on.
+
+Distinct from ``system_steward_change_hooks``: that fires on add/remove
+against the ``system_manager`` join, where the BG-task model works because
+the post-commit state is still queryable. Deletion is different — by the
+time a BG task would run, FK cascades have erased the rows consumers need.
+Hooks here therefore execute their snapshot synchronously and defer only
+follow-up work via ``background_tasks``.
+
+Like the adjacent change-hook shims, this module exists ONLY because the
+event framework is not yet merged. When it lands:
+
+  - Call sites of ``notify_system_about_to_be_deleted`` (in the two v1
+    system-delete routes) are replaced with
+    ``publish_before_commit(session, SystemDeleted(...))``.
+  - Fidesplus's registered callback is replaced with a
+    ``@subscribes_to(SystemDeleted)`` handler (with the snapshot moved
+    into a pre-commit listener).
+  - This file is deleted.
+
+DO NOT use this as a precedent for adding more cross-repo hooks. If you need
+a similar shim before the framework lands, create a separate single-purpose
+module — do not generalize this into a ``hooks`` package.
+"""
+
+from typing import Callable, List
+
+from fastapi import BackgroundTasks
+from loguru import logger
+
+SystemPreDeleteHook = Callable[[BackgroundTasks, str], None]
+
+_HOOKS: List[SystemPreDeleteHook] = []
+
+
+def register_system_pre_delete_hook(hook: SystemPreDeleteHook) -> None:
+    """Register a callback invoked synchronously before a system is deleted.
+
+    Idempotent: registering the same hook twice is a no-op.
+    """
+    if hook not in _HOOKS:
+        _HOOKS.append(hook)
+
+
+def notify_system_about_to_be_deleted(
+    background_tasks: BackgroundTasks, system_id: str
+) -> None:
+    """Invoke every registered hook BEFORE the system row is deleted.
+
+    Each hook runs synchronously in the request thread and must complete
+    its snapshot quickly; heavy work should be deferred via
+    ``background_tasks``. Failures are logged and swallowed — a misbehaving
+    consumer must not block the delete.
+    """
+    for hook in _HOOKS:
+        try:
+            hook(background_tasks, system_id)
+        except Exception:
+            logger.exception(
+                "System pre-delete hook {} raised for system_id={}",
+                hook,
+                system_id,
+            )

src/fides/api/system_steward_change_hooks.py

@@ -5,8 +5,8 @@
 added/removed as a data steward (system manager) of a system. The only known
 consumer today is fidesplus's monitor-stewardship-inheritance propagation.
 
-This module exists ONLY because the event framework in fides PR #8096 is
-not yet merged. When it lands:
+This module exists ONLY because the event framework is not yet merged. When
+it lands:
 
   - Call sites of ``notify_system_stewards_changed`` (in the three v1
     system-manager routes) are replaced with

src/fides/api/v1/endpoints/system.py

@@ -57,6 +57,7 @@
     BasicSystemResponse,
     SystemResponse,
 )
+from fides.api.system_pre_delete_hooks import notify_system_about_to_be_deleted
 from fides.api.system_steward_change_hooks import notify_system_stewards_changed
 from fides.api.util.api_router import APIRouter
 from fides.api.util.connection_util import (
@@ -338,6 +339,7 @@ async def upsert(
 )
 async def delete(
     fides_key: str,
+    background_tasks: BackgroundTasks,
     # to retrieve the System and also return a value
     db: AsyncSession = Depends(get_async_db),
 ) -> Dict:
@@ -346,6 +348,9 @@ async def delete(
     to add additional "system manager" permission checks.
     """
     system_to_delete = await get_resource(System, fides_key, db)
+    # Pre-delete hook runs while dependent rows still exist so consumers can
+    # snapshot state before FK cascade wipes joins they depend on.
+    notify_system_about_to_be_deleted(background_tasks, system_to_delete.id)
     async with db.begin():
         await db.delete(system_to_delete)
     # Convert the resource to a dict explicitly for the response
@@ -370,6 +375,7 @@ async def delete(
 )
 async def system_bulk_delete(
     fides_keys: List[str],
+    background_tasks: BackgroundTasks,
     db: AsyncSession = Depends(get_async_db),
 ) -> Dict:
     """Delete multiple systems by their fides_keys."""
@@ -383,6 +389,7 @@ async def system_bulk_delete(
         systems_to_delete = result.scalars().all()
 
         for system in systems_to_delete:
+            notify_system_about_to_be_deleted(background_tasks, system.id)
             await db.delete(system)
             deleted.append(SystemSchema.model_validate(system).model_dump(mode="json"))
 

tests/fides/api/test_system_pre_delete_hooks.py

@@ -0,0 +1,55 @@
+"""Tests for the system pre-delete hook registry."""
+
+from unittest.mock import MagicMock
+
+import pytest
+from fastapi import BackgroundTasks
+
+from fides.api import system_pre_delete_hooks as pre_delete
+
+
+@pytest.fixture(autouse=True)
+def isolated_registry(monkeypatch):
+    """Each test starts with an empty registry."""
+    monkeypatch.setattr(pre_delete, "_HOOKS", [])
+    yield
+
+
+def test_register_is_idempotent():
+    hook = MagicMock()
+    pre_delete.register_system_pre_delete_hook(hook)
+    pre_delete.register_system_pre_delete_hook(hook)
+    assert pre_delete._HOOKS == [hook]
+
+
+def test_notify_dispatches_to_all_hooks():
+    first = MagicMock()
+    second = MagicMock()
+    pre_delete.register_system_pre_delete_hook(first)
+    pre_delete.register_system_pre_delete_hook(second)
+
+    bg = BackgroundTasks()
+    pre_delete.notify_system_about_to_be_deleted(bg, "sys-1")
+
+    first.assert_called_once_with(bg, "sys-1")
+    second.assert_called_once_with(bg, "sys-1")
+
+
+def test_notify_isolates_hook_failures():
+    """A raising hook must not prevent later hooks from firing."""
+    raising = MagicMock(side_effect=RuntimeError("boom"))
+    survivor = MagicMock()
+    pre_delete.register_system_pre_delete_hook(raising)
+    pre_delete.register_system_pre_delete_hook(survivor)
+
+    bg = BackgroundTasks()
+    pre_delete.notify_system_about_to_be_deleted(bg, "sys-1")
+
+    raising.assert_called_once_with(bg, "sys-1")
+    survivor.assert_called_once_with(bg, "sys-1")
+
+
+def test_notify_empty_registry_is_noop():
+    """No hooks registered → no error, no side effects."""
+    bg = BackgroundTasks()
+    pre_delete.notify_system_about_to_be_deleted(bg, "sys-1")

tests/fides/ops/api/v1/endpoints/test_system.py

@@ -13,6 +13,7 @@
 )
 from starlette.testclient import TestClient
 
+from fides.api import system_pre_delete_hooks
 from fides.api.models.connectionconfig import (
     AccessLevel,
     ConnectionConfig,
@@ -1298,3 +1299,49 @@ def test_delete_system_without_staged_resources(
 
         assert resp.status_code == HTTP_200_OK
         assert db.query(System).filter_by(id=system_id).first() is None
+
+
+class TestDeleteSystemFiresPreDeleteHook:
+    """Verify both delete routes invoke the pre-delete hook before the row is removed."""
+
+    @pytest.fixture(scope="function")
+    def registered_hook(self):
+        hook = mock.MagicMock()
+        system_pre_delete_hooks.register_system_pre_delete_hook(hook)
+        yield hook
+        system_pre_delete_hooks._HOOKS.remove(hook)
+
+    def test_single_delete_fires_pre_delete_hook(
+        self,
+        api_client: TestClient,
+        generate_auth_header,
+        db: Session,
+        system,
+        registered_hook,
+    ) -> None:
+        system_id = system.id
+        url = V1_URL_PREFIX + f"/system/{system.fides_key}"
+        auth_header = generate_auth_header(scopes=[SYSTEM_DELETE])
+        resp = api_client.delete(url, headers=auth_header)
+
+        assert resp.status_code == HTTP_200_OK
+        registered_hook.assert_called_once()
+        # Second positional arg is system_id.
+        assert registered_hook.call_args.args[1] == system_id
+
+    def test_bulk_delete_fires_pre_delete_hook_per_system(
+        self,
+        api_client: TestClient,
+        generate_auth_header,
+        db: Session,
+        system,
+        registered_hook,
+    ) -> None:
+        system_id = system.id
+        bulk_url = V1_URL_PREFIX + "/system/bulk-delete"
+        auth_header = generate_auth_header(scopes=[SYSTEM_DELETE])
+        resp = api_client.post(bulk_url, headers=auth_header, json=[system.fides_key])
+
+        assert resp.status_code == HTTP_200_OK
+        registered_hook.assert_called_once()
+        assert registered_hook.call_args.args[1] == system_id