[Fix] Renaming or removing a contracted model raises a BreakingChange warning/error

.changes/unreleased/Fixes-20240523-204251.yaml

@@ -0,0 +1,6 @@
+kind: Fixes
+body: Renaming or removing a contracted model should raise a BreakingChange warning/error
+time: 2024-05-23T20:42:51.033946-04:00
+custom:
+  Author: michelleark
+  Issue: "10116"

core/dbt/contracts/graph/nodes.py

@@ -19,6 +19,8 @@
 from mashumaro.types import SerializableType
 
 from dbt import deprecations
+from dbt.adapters.base import ConstraintSupport
+from dbt.adapters.factory import get_adapter_constraint_support
 from dbt.artifacts.resources import Analysis as AnalysisResource
 from dbt.artifacts.resources import (
     BaseResource,
@@ -469,6 +471,13 @@
     def is_latest_version(self) -> bool:
         return self.version is not None and self.version == self.latest_version
 
+    @property
+    def is_past_deprecation_date(self) -> bool:
+        return (
+            self.deprecation_date is not None
+            and self.deprecation_date < datetime.now().astimezone()
+        )
+
     @property
     def search_name(self):
         if self.version is None:
@@ -570,6 +579,37 @@
             data = contract_state.encode("utf-8")
             self.contract.checksum = hashlib.new("sha256", data).hexdigest()
 
+    def same_contract_deleted(self) -> bool:
+        """
+        self: the deleted model node
+        """
+        # If the contract wasn't previously enforced, so contract has not changed
+        if self.contract.enforced is False:
+            return True
+
+        # Deleted node is passed its deprecation_date, so deletion does not constitute a contract change
+        if self.is_past_deprecation_date:
+            return True
+
+        breaking_changes = [f"Contracted model '{self.unique_id}' was deleted or renamed."]
+        if self.version is None:
+            warn_or_error(
+                UnversionedBreakingChange(
+                    breaking_changes=breaking_changes,
+                    model_name=self.name,
+                    model_file_path=self.original_file_path,
+                ),
+                node=self,
+            )
+            return False
+        else:
+            raise (
+                ContractBreakingChangeError(
+                    breaking_changes=breaking_changes,
+                    node=self,
+                )
+            )
+
     def same_contract(self, old, adapter_type=None) -> bool:
         # If the contract wasn't previously enforced:
         if old.contract.enforced is False and self.contract.enforced is False:
@@ -601,10 +641,6 @@
             # Breaking change: the contract was previously enforced, and it no longer is
             contract_enforced_disabled = True
 
-        # TODO: this avoid the circular imports but isn't ideal
-        from dbt.adapters.base import ConstraintSupport
-        from dbt.adapters.factory import get_adapter_constraint_support
-
         constraint_support = get_adapter_constraint_support(adapter_type)
         column_constraints_exist = False
 

core/dbt/graph/selector_methods.py

@@ -719,7 +719,9 @@ def check_modified_contract(
     ) -> Callable[[Optional[SelectorTarget], SelectorTarget], bool]:
         # get a function that compares two selector target based on compare method provided
         def check_modified_contract(old: Optional[SelectorTarget], new: SelectorTarget) -> bool:
-            if hasattr(new, compare_method):
+            if new is None and hasattr(old, compare_method + "_deleted"):
+                return getattr(old, compare_method + "_deleted")()
+            elif hasattr(new, compare_method):
                 # when old body does not exist or old and new are not the same
                 return not old or not getattr(new, compare_method)(old, adapter_type)  # type: ignore
             else:
@@ -785,6 +787,16 @@ def search(self, included_nodes: Set[UniqueId], selector: str) -> Iterator[Uniqu
             if checker(previous_node, node, **keyword_args):  # type: ignore
                 yield unique_id
 
+        # checkers that can handle deleted nodes
+        if checker.__name__ in ["check_modified_contract"]:
+            # ignore included_nodes, since those cannot contain deleted nodes
+            for previous_unique_id, previous_node in manifest.nodes.items():
+                # detect deleted or renamed node
+                if previous_unique_id not in self.manifest.nodes.keys():
+                    # do not yield -- deleted nodes should never be selected for downstream execution
+                    # as they are not part of the current project's manifest
+                    checker(previous_node, None, **keyword_args)  # type: ignore
+
 
 class ResultSelectorMethod(SelectorMethod):
     def search(self, included_nodes: Set[UniqueId], selector: str) -> Iterator[UniqueId]:

tests/functional/defer_state/test_modified_state.py

@@ -8,6 +8,7 @@
 from dbt.exceptions import CompilationError, ContractBreakingChangeError
 from dbt.tests.util import (
     get_manifest,
+    rm_file,
     run_dbt,
     run_dbt_and_capture,
     update_config_file,
@@ -649,6 +650,50 @@ def test_changed_contract_versioned(self, project):
             results = run_dbt(["run", "--models", "state:modified.contract", "--state", "./state"])
 
 
+class TestDeleteUnversionedContractedModel(BaseModifiedState):
+    MODEL_UNIQUE_ID = "model.test.table_model"
+    CONTRACT_SCHEMA_YML = contract_schema_yml
+
+    def test_delete_unversioned_contracted_model(self, project):
+        # ensure table_model is contracted
+        write_file(self.CONTRACT_SCHEMA_YML, "models", "schema.yml")
+        self.run_and_save_state()
+
+        # delete versioned contracted model
+        rm_file(project.project_root, "models", "table_model.sql")
+
+        # since the models are unversioned, they raise a warning but not an error
+        _, logs = run_dbt_and_capture(
+            ["run", "--models", "state:modified.contract", "--state", "./state"]
+        )
+
+        expected_warning = "While comparing to previous project state, dbt detected a breaking change to an unversioned model"
+        expected_change = "Contracted model 'model.test.table_model' was deleted or renamed"
+        assert expected_warning in logs
+        assert expected_change in logs
+
+
+class TestDeleteVersionedContractedModel(BaseModifiedState):
+    MODEL_UNIQUE_ID = "model.test.table_model.v1"
+    CONTRACT_SCHEMA_YML = versioned_contract_schema_yml
+
+    def test_delete_unversioned_contracted_model(self, project):
+        # ensure table_model is versioned + contracted
+        write_file(self.CONTRACT_SCHEMA_YML, "models", "schema.yml")
+        self.run_and_save_state()
+
+        # delete versioned contracted model
+        rm_file(project.project_root, "models", "table_model.sql")
+
+        # since the models are unversioned, they raise a warning but not an error
+        with pytest.raises(ContractBreakingChangeError) as e:
+            run_dbt(["run", "--models", "state:modified.contract", "--state", "./state"])
+
+        assert "Contracted model 'model.test.table_model.v1' was deleted or renamed." in str(
+            e.value
+        )
+
+
 class TestChangedConstraintUnversioned(BaseModifiedState):
     def test_changed_constraint(self, project):
         self.run_and_save_state()