[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,42 @@
             data = contract_state.encode("utf-8")
             self.contract.checksum = hashlib.new("sha256", data).hexdigest()
 
+    def same_contract_removed(self) -> bool:
+        """
+        self: the removed (deleted, renamed, or disabled) model node
+        """
+        # If the contract wasn't previously enforced, no contract change has occurred
+        if self.contract.enforced is False:
+            return True
+
+        # Removed node is past its deprecation_date, so deletion does not constitute a contract change
+        if self.is_past_deprecation_date:
+            return True
+
+        # Disabled, deleted, or renamed node with previously enforced contract.
+        if not self.config.enabled:
+            breaking_change = f"Contracted model '{self.unique_id}' was disabled."
+        else:
+            breaking_change = f"Contracted model '{self.unique_id}' was deleted or renamed."
+
+        if self.version is None:
+            warn_or_error(
+                UnversionedBreakingChange(
+                    breaking_changes=[breaking_change],
+                    model_name=self.name,
+                    model_file_path=self.original_file_path,
+                ),
+                node=self,
+            )
+            return False
+        else:
+            raise (
+                ContractBreakingChangeError(
+                    breaking_changes=[breaking_change],
+                    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 +646,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 + "_removed"):
+                return getattr(old, compare_method + "_removed")()
+            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,22 @@ 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 removed nodes
+        if checker.__name__ in ["check_modified_contract"]:
+            # ignore included_nodes, since those cannot contain removed nodes
+            for previous_unique_id, previous_node in manifest.nodes.items():
+                # detect removed (deleted, renamed, or disabled) nodes
+                removed_node = None
+                if previous_unique_id in self.manifest.disabled.keys():
+                    removed_node = self.manifest.disabled[previous_unique_id][0]
+                elif previous_unique_id not in self.manifest.nodes.keys():
+                    removed_node = previous_node
+
+                if removed_node:
+                    # do not yield -- removed nodes should never be selected for downstream execution
+                    # as they are not part of the current project's manifest.nodes
+                    checker(removed_node, None, **keyword_args)  # type: ignore
+
 
 class ResultSelectorMethod(SelectorMethod):
     def search(self, included_nodes: Set[UniqueId], selector: str) -> Iterator[UniqueId]:

core/dbt/parser/manifest.py

@@ -570,25 +570,21 @@
 
     def check_for_model_deprecations(self):
         for node in self.manifest.nodes.values():
-            if isinstance(node, ModelNode):
-                if (
-                    node.deprecation_date
-                    and node.deprecation_date < datetime.datetime.now().astimezone()
-                ):
-                    warn_or_error(
-                        DeprecatedModel(
-                            model_name=node.name,
-                            model_version=version_to_str(node.version),
-                            deprecation_date=node.deprecation_date.isoformat(),
-                        )
+            if isinstance(node, ModelNode) and node.is_past_deprecation_date:
+                warn_or_error(
+                    DeprecatedModel(
+                        model_name=node.name,
+                        model_version=version_to_str(node.version),
+                        deprecation_date=node.deprecation_date.isoformat(),
                     )
+                )
 
                 resolved_refs = self.manifest.resolve_refs(node, self.root_project.project_name)
                 resolved_model_refs = [r for r in resolved_refs if isinstance(r, ModelNode)]
                 node.depends_on
                 for resolved_ref in resolved_model_refs:
                     if resolved_ref.deprecation_date:
-                        if resolved_ref.deprecation_date < datetime.datetime.now().astimezone():
+                        if resolved_ref.is_past_deprecation_date:
                             event_cls = DeprecatedReference
                         else:
                             event_cls = UpcomingReferenceDeprecation

tests/functional/defer_state/fixtures.py

@@ -108,6 +108,25 @@
         data_type: text
 """
 
+disabled_contract_schema_yml = """
+version: 2
+models:
+  - name: table_model
+    config:
+      contract:
+        enforced: True
+      enabled: False
+    columns:
+      - name: id
+        data_type: integer
+        data_tests:
+          - unique:
+              severity: error
+          - not_null
+      - name: name
+        data_type: text
+"""
+
 modified_contract_schema_yml = """
 version: 2
 models:
@@ -126,7 +145,7 @@
         data_type: text
 """
 
-disabled_contract_schema_yml = """
+unenforced_contract_schema_yml = """
 version: 2
 models:
   - name: table_model
@@ -144,6 +163,25 @@
         data_type: text
 """
 
+disabled_unenforced_contract_schema_yml = """
+version: 2
+models:
+  - name: table_model
+    config:
+      contract:
+        enforced: False
+      enabled: False
+    columns:
+      - name: id
+        data_type: integer
+        data_tests:
+          - unique:
+              severity: error
+          - not_null
+      - name: name
+        data_type: text
+"""
+
 versioned_no_contract_schema_yml = """
 version: 2
 models:
@@ -182,6 +220,27 @@
         data_type: text
 """
 
+disabled_versioned_contract_schema_yml = """
+version: 2
+models:
+  - name: table_model
+    config:
+      contract:
+        enforced: True
+      enabled: False
+    versions:
+      - v: 1
+    columns:
+      - name: id
+        data_type: integer
+        data_tests:
+          - unique:
+              severity: error
+          - not_null
+      - name: name
+        data_type: text
+"""
+
 versioned_modified_contract_schema_yml = """
 version: 2
 models:
@@ -202,7 +261,28 @@
         data_type: text
 """
 
-versioned_disabled_contract_schema_yml = """
+disabled_versioned_unenforced_contract_schema_yml = """
+version: 2
+models:
+  - name: table_model
+    config:
+      contract:
+        enforced: False
+      enabled: False
+    versions:
+      - v: 1
+    columns:
+      - name: id
+        data_type: integer
+        data_tests:
+          - unique:
+              severity: error
+          - not_null
+      - name: name
+        data_type: text
+"""
+
+versioned_unenforced_contract_schema_yml = """
 version: 2
 models:
   - name: table_model