HJ-131 Delete monitors before deleting integration

CHANGELOG.md

@@ -20,6 +20,9 @@ The types of changes are:
 ### Added
 - Added support for GPP national string to be used alongside state-by-state using a new approach option [#5480](https://github.com/ethyca/fides/pull/5480)
 
+### Fixed
+- Fixed deletion of ConnectionConfigs that have related MonitorConfigs [#5478](https://github.com/ethyca/fides/pull/5478)
+
 
 ## [2.49.0](https://github.com/ethyca/fidesplus/compare/2.48.2...2.49.0)
 

src/fides/api/db/base.py

@@ -12,7 +12,7 @@
 from fides.api.models.custom_connector_template import CustomConnectorTemplate
 from fides.api.models.custom_report import CustomReport
 from fides.api.models.datasetconfig import DatasetConfig
-from fides.api.models.detection_discovery import StagedResource
+from fides.api.models.detection_discovery import MonitorConfig, StagedResource
 from fides.api.models.experience_notices import ExperienceNotices
 from fides.api.models.fides_cloud import FidesCloud
 from fides.api.models.fides_user import FidesUser

src/fides/api/models/connectionconfig.py

@@ -2,8 +2,9 @@
 
 import enum
 from datetime import datetime
-from typing import Any, Dict, Optional, Type
+from typing import TYPE_CHECKING, Any, Dict, List, Optional, Type
 
+from loguru import logger
 from sqlalchemy import Boolean, Column, DateTime, Enum, ForeignKey, String, event
 from sqlalchemy.dialects.postgresql import ARRAY, JSONB
 from sqlalchemy.ext.mutable import MutableDict
@@ -21,6 +22,9 @@
 from fides.api.schemas.saas.saas_config import SaaSConfig
 from fides.config import CONFIG
 
+if TYPE_CHECKING:
+    from fides.api.models.detection_discovery import MonitorConfig
+
 
 class ConnectionTestStatus(enum.Enum):
     """Enum for supplying statuses of validating credentials for a Connection Config to the user"""
@@ -162,6 +166,14 @@
         cascade="all, delete",
     )
 
+    # Monitor configs related to this connection config.
+    # If the connection config is deleted, the monitor configs will be deleted as well.
+    monitors: RelationshipProperty[List["MonitorConfig"]] = relationship(
+        "MonitorConfig",
+        back_populates="connection_config",
+        cascade="all, delete",
+    )
+
     access_manual_webhook = relationship(  # type: ignore[misc]
         "AccessManualWebhook",
         back_populates="connection_config",
@@ -276,9 +288,21 @@
 
     def delete(self, db: Session) -> Optional[FidesBase]:
         """Hard deletes datastores that map this ConnectionConfig."""
+        logger.info(
+            "Deleting connection config {}...",
+            self.key,
+        )
         for dataset in self.datasets:
             dataset.delete(db=db)
 
+        for monitor in self.monitors:
+            logger.info(
+                "Deleting monitor config {} associated with connection config {}...",
+                monitor.key,
+                self.key,
+            )
+            monitor.delete(db=db)
+
         return super().delete(db=db)
 
 

tests/conftest.py

@@ -48,6 +48,7 @@
 from tests.fixtures.application_fixtures import *
 from tests.fixtures.bigquery_fixtures import *
 from tests.fixtures.datahub_fixtures import *
+from tests.fixtures.detection_discovery_fixtures import *
 from tests.fixtures.dynamodb_fixtures import *
 from tests.fixtures.email_fixtures import *
 from tests.fixtures.fides_connector_example_fixtures import *

tests/fixtures/detection_discovery_fixtures.py

@@ -0,0 +1,62 @@
+from typing import Generator
+
+import pytest
+from sqlalchemy.orm import Session
+from sqlalchemy.orm.exc import ObjectDeletedError
+
+from fides.api.models.connectionconfig import ConnectionConfig
+from fides.api.models.detection_discovery import MonitorConfig
+
+
+@pytest.fixture(scope="function")
+def monitor_config(
+    db: Session, connection_config: ConnectionConfig
+) -> Generator[MonitorConfig, None, None]:
+    mc = MonitorConfig.create(
+        db=db,
+        data={
+            "name": "test monitor config 1",
+            "key": "test_monitor_config_1",
+            "connection_config_id": connection_config.id,
+            "classify_params": {
+                "num_samples": 25,
+                "num_threads": 2,
+            },
+            "databases": ["db1", "db2"],
+            "execution_frequency": None,
+            "execution_start_date": None,
+        },
+    )
+    yield mc
+    try:
+        db.delete(mc)
+    except ObjectDeletedError:
+        # Object was already deleted, do nothing
+        return
+
+
+@pytest.fixture(scope="function")
+def monitor_config_2(
+    db: Session, connection_config: ConnectionConfig
+) -> Generator[MonitorConfig, None, None]:
+    mc = MonitorConfig.create(
+        db=db,
+        data={
+            "name": "test monitor config 2",
+            "key": "test_monitor_config_2",
+            "connection_config_id": connection_config.id,
+            "classify_params": {
+                "num_samples": 20,
+                "num_threads": 2,
+            },
+            "databases": ["db1", "db3"],
+            "execution_frequency": None,
+            "execution_start_date": None,
+        },
+    )
+    yield mc
+    try:
+        db.delete(mc)
+    except ObjectDeletedError:
+        # Object was already deleted, do nothing
+        return

tests/ops/api/v1/endpoints/test_connection_config_endpoints.py

@@ -17,6 +17,7 @@
     ConnectionType,
 )
 from fides.api.models.datasetconfig import DatasetConfig
+from fides.api.models.detection_discovery import MonitorConfig
 from fides.api.models.manual_webhook import AccessManualWebhook
 from fides.api.models.privacy_request import PrivacyRequestStatus
 from fides.api.models.sql_models import Dataset
@@ -1365,6 +1366,33 @@ def test_delete_saas_connection_config(
         assert db.query(DatasetConfig).filter_by(id=dataset_config.id).first() is None
         assert db.query(Dataset).filter_by(id=dataset.id).first() is None
 
+    def test_delete_connection_config_with_related_monitors(
+        self,
+        api_client: TestClient,
+        db: Session,
+        generate_role_header,
+        connection_config: ConnectionConfig,
+        monitor_config: MonitorConfig,
+        monitor_config_2: MonitorConfig,
+    ):
+        auth_header = generate_role_header(roles=[OWNER])
+
+        # Save ids of monitor configs before deletion to use in query
+        id_monitor_config_1 = monitor_config.id
+        id_monitor_config_2 = monitor_config_2.id
+
+        response = api_client.delete(
+            f"{V1_URL_PREFIX}{CONNECTIONS}/{connection_config.key}", headers=auth_header
+        )
+
+        assert response.status_code == 204
+        assert (
+            db.query(ConnectionConfig).filter_by(key=connection_config.key).first()
+        ) is None
+
+        assert db.query(MonitorConfig).filter_by(id=id_monitor_config_1).first() is None
+        assert db.query(MonitorConfig).filter_by(id=id_monitor_config_2).first() is None
+
 
 class TestPutConnectionConfigSecrets:
     @pytest.fixture(scope="function")

tests/ops/models/test_detection_discovery.py

@@ -190,34 +190,14 @@ def test_staged_resource_helpers(self, db: Session, create_staged_resource):
 
 
 class TestMonitorConfigModel:
-    @pytest.fixture
-    def create_monitor_config(self, db: Session, connection_config: ConnectionConfig):
-        mc = MonitorConfig.create(
-            db=db,
-            data={
-                "name": "test monitor config 1",
-                "key": "test_monitor_config_1",
-                "connection_config_id": connection_config.id,
-                "classify_params": {
-                    "num_samples": 25,
-                    "num_threads": 2,
-                },
-                "databases": ["db1", "db2"],
-                "execution_frequency": None,
-                "execution_start_date": None,
-            },
-        )
-        yield mc
-        db.delete(mc)
-
     def test_create_monitor_config(
-        self, db: Session, create_monitor_config, connection_config: ConnectionConfig
+        self, db: Session, monitor_config, connection_config: ConnectionConfig
     ) -> None:
         """
         Creation fixture creates the config, this tests that it was created successfully
         and that we can access its attributes as expected.
         """
-        mc: MonitorConfig = MonitorConfig.get(db=db, object_id=create_monitor_config.id)
+        mc: MonitorConfig = MonitorConfig.get(db=db, object_id=monitor_config.id)
         assert mc.name == "test monitor config 1"
         assert mc.key == "test_monitor_config_1"
         assert mc.classify_params == {
@@ -237,11 +217,11 @@ def test_create_monitor_config(
         assert mc.excluded_databases == []
 
     def test_update_monitor_config_fails_with_conflicting_dbs(
-        self, db: Session, create_monitor_config, connection_config: ConnectionConfig
+        self, db: Session, monitor_config, connection_config: ConnectionConfig
     ) -> None:
         """ """
         with pytest.raises(ValueError):
-            create_monitor_config.update(
+            monitor_config.update(
                 db=db,
                 data={
                     "name": "updated test monitor config 1",
@@ -388,12 +368,12 @@ def test_update_monitor_config_execution_trigger_logic(
         self,
         db: Session,
         connection_config: ConnectionConfig,
-        create_monitor_config: MonitorConfig,
+        monitor_config: MonitorConfig,
         monitor_frequency,
         expected_dict,
     ):
         """Tests that execution_trigger logic works as expected during within `update`"""
-        create_monitor_config.update(
+        monitor_config.update(
             db=db,
             data={
                 "name": "updated test monitor config 1",
@@ -407,7 +387,7 @@ def test_update_monitor_config_execution_trigger_logic(
                 "execution_frequency": monitor_frequency,
             },
         )
-        mc: MonitorConfig = MonitorConfig.get(db=db, object_id=create_monitor_config.id)
+        mc: MonitorConfig = MonitorConfig.get(db=db, object_id=monitor_config.id)
 
         # first ensure update works as expected on a "normal" field
         assert mc.name == "updated test monitor config 1"