handle generics and generics peers correctly when deleting nodes (#5133)

* partial fix and tests for generic delete constraints

* fix unit test

* add changelog

backend/infrahub/core/node/delete_validator.py

@@ -11,7 +11,7 @@
     RelationshipGetByIdentifierQuery,
     RelationshipPeersData,
 )
-from infrahub.core.schema import MainSchemaTypes
+from infrahub.core.schema import MainSchemaTypes, NodeSchema, ProfileSchema
 from infrahub.core.timestamp import Timestamp
 from infrahub.database import InfrahubDatabase
 from infrahub.exceptions import ValidationError
@@ -28,20 +28,29 @@ def __init__(self, all_schemas_map: dict[str, MainSchemaTypes]) -> None:
         # {node_schema: {DeleteRelationshipType: {relationship_identifier: peer_node_schema}}}
         self._dependency_graph: dict[str, dict[DeleteRelationshipType, dict[str, set[str]]]] = {}
 
-    def index(self, start_schemas: Iterable[MainSchemaTypes]) -> None:
+    def index(self, start_schemas: Iterable[NodeSchema | ProfileSchema]) -> None:
         self._index_cascading_deletes(start_schemas=start_schemas)
         self._index_dependent_schema(start_schemas=start_schemas)
 
+    def _get_schema_kinds(self, schema_kind: str) -> set[str]:
+        schema = self._all_schemas_map[schema_kind]
+        if schema.is_node_schema:
+            return {schema_kind}
+        used_by = getattr(schema, "used_by", None)
+        if schema.is_generic_schema and used_by:
+            return set(used_by)
+        return set()
+
     def _add_to_dependency_graph(
-        self, kind: str, relationship_type: DeleteRelationshipType, relationship_identifier: str, peer_kind: str
+        self, kind: str, relationship_type: DeleteRelationshipType, relationship_identifier: str, peer_kinds: set[str]
     ) -> None:
         if kind not in self._dependency_graph:
             self._dependency_graph[kind] = {}
         if relationship_type not in self._dependency_graph[kind]:
             self._dependency_graph[kind][relationship_type] = defaultdict(set)
-        self._dependency_graph[kind][relationship_type][relationship_identifier].add(peer_kind)
+        self._dependency_graph[kind][relationship_type][relationship_identifier].update(peer_kinds)
 
-    def _index_cascading_deletes(self, start_schemas: Iterable[MainSchemaTypes]) -> None:
+    def _index_cascading_deletes(self, start_schemas: Iterable[NodeSchema | ProfileSchema]) -> None:
         kinds_to_check: set[str] = {schema.kind for schema in start_schemas}
         while True:
             try:
@@ -52,27 +61,35 @@ def _index_cascading_deletes(self, start_schemas: Iterable[MainSchemaTypes]) ->
             for relationship_schema in node_schema.relationships:
                 if relationship_schema.on_delete != RelationshipDeleteBehavior.CASCADE:
                     continue
+                peer_kinds = self._get_schema_kinds(schema_kind=relationship_schema.peer)
                 self._add_to_dependency_graph(
                     kind=kind_to_check,
                     relationship_type=DeleteRelationshipType.CASCADE_DELETE,
                     relationship_identifier=relationship_schema.get_identifier(),
-                    peer_kind=relationship_schema.peer,
+                    peer_kinds=peer_kinds,
                 )
-                if relationship_schema.peer not in self._dependency_graph:
-                    kinds_to_check.add(relationship_schema.peer)
-
-    def _index_dependent_schema(self, start_schemas: Iterable[MainSchemaTypes]) -> None:
-        start_schema_kinds = {schema.kind for schema in start_schemas}
+                for peer_kind in peer_kinds:
+                    if peer_kind not in self._dependency_graph:
+                        kinds_to_check.add(peer_kind)
+
+    def _index_dependent_schema(self, start_schemas: Iterable[NodeSchema | ProfileSchema]) -> None:
+        start_schema_kinds: set[str] = set()
+        for start_schema in start_schemas:
+            start_schema_kinds.add(start_schema.kind)
+            if start_schema.inherit_from:
+                start_schema_kinds.update(set(start_schema.inherit_from))
         for node_schema in self._all_schemas_map.values():
             for relationship_schema in node_schema.relationships:
                 if relationship_schema.optional is True or relationship_schema.peer not in start_schema_kinds:
                     continue
-                self._add_to_dependency_graph(
-                    kind=relationship_schema.peer,
-                    relationship_type=DeleteRelationshipType.DEPENDENT_NODE,
-                    relationship_identifier=relationship_schema.get_identifier(),
-                    peer_kind=node_schema.kind,
-                )
+
+                for peer_kind in self._get_schema_kinds(schema_kind=relationship_schema.peer):
+                    self._add_to_dependency_graph(
+                        kind=peer_kind,
+                        relationship_type=DeleteRelationshipType.DEPENDENT_NODE,
+                        relationship_identifier=relationship_schema.get_identifier(),
+                        peer_kinds={node_schema.kind},
+                    )
 
     def get_relationship_identifiers(self) -> list[FullRelationshipIdentifier]:
         full_relationship_identifiers = []

backend/tests/conftest.py

@@ -670,6 +670,109 @@ async def animal_person_schema(
     return registry.schema.register_schema(schema=animal_person_schema_unregistered, branch=default_branch.name)
 
 
+@pytest.fixture
+async def dependent_generics_unregistered(db: InfrahubDatabase, node_group_schema, data_schema) -> SchemaRoot:
+    schema: dict[str, Any] = {
+        "generics": [
+            {
+                "name": "Animal",
+                "namespace": "Test",
+                "human_friendly_id": ["owner__name__value", "name__value"],
+                "uniqueness_constraints": [
+                    ["owner", "name__value"],
+                ],
+                "branch": BranchSupportType.AWARE.value,
+                "attributes": [
+                    {"name": "name", "kind": "Text"},
+                    {"name": "weight", "kind": "Number", "optional": True},
+                ],
+                "relationships": [
+                    {
+                        "name": "owner",
+                        "peer": "TestPerson",
+                        "optional": False,
+                        "identifier": "person__animal",
+                        "cardinality": "one",
+                        "direction": "outbound",
+                    },
+                ],
+            },
+            {
+                "name": "Person",
+                "namespace": "Test",
+                "human_friendly_id": ["name__value"],
+                "uniqueness_constraints": [
+                    ["name__value"],
+                ],
+                "branch": BranchSupportType.AWARE.value,
+                "attributes": [
+                    {"name": "name", "kind": "Text"},
+                    {"name": "height", "kind": "Number", "optional": True},
+                ],
+                "relationships": [
+                    {
+                        "name": "animals",
+                        "peer": "TestAnimal",
+                        "identifier": "person__animal",
+                        "cardinality": "many",
+                        "direction": "inbound",
+                    },
+                ],
+            },
+        ],
+        "nodes": [
+            {
+                "name": "Dog",
+                "namespace": "Test",
+                "inherit_from": ["TestAnimal"],
+                "display_labels": ["name__value", "breed__value"],
+                "attributes": [
+                    {"name": "breed", "kind": "Text", "optional": False},
+                    {"name": "color", "kind": "Color", "default_value": "#444444", "optional": True},
+                ],
+            },
+            {
+                "name": "Cat",
+                "namespace": "Test",
+                "inherit_from": ["TestAnimal"],
+                "display_labels": ["name__value", "breed__value", "color__value"],
+                "attributes": [
+                    {"name": "breed", "kind": "Text", "optional": False},
+                    {"name": "color", "kind": "Color", "default_value": "#444444", "optional": True},
+                ],
+            },
+            {
+                "name": "Human",
+                "namespace": "Test",
+                "display_labels": ["name__value"],
+                "inherit_from": ["TestPerson"],
+                "default_filter": "name__value",
+                "human_friendly_id": ["name__value"],
+            },
+            {
+                "name": "Cylon",
+                "namespace": "Test",
+                "display_labels": ["name__value"],
+                "inherit_from": ["TestPerson"],
+                "default_filter": "name__value",
+                "human_friendly_id": ["name__value"],
+                "attributes": [
+                    {"name": "model_number", "kind": "Number", "optional": False},
+                ],
+            },
+        ],
+    }
+
+    return SchemaRoot(**schema)
+
+
+@pytest.fixture
+async def dependent_generics_schema(
+    db: InfrahubDatabase, default_branch: Branch, dependent_generics_unregistered
+) -> SchemaBranch:
+    return registry.schema.register_schema(schema=dependent_generics_unregistered, branch=default_branch.name)
+
+
 @pytest.fixture
 async def node_group_schema(db: InfrahubDatabase, default_branch: Branch, data_schema) -> None:
     SCHEMA: dict[str, Any] = {

backend/tests/unit/core/test_node_manager_delete.py

@@ -8,6 +8,7 @@
 from infrahub.core.manager import NodeManager
 from infrahub.core.node import Node
 from infrahub.core.schema.relationship_schema import RelationshipSchema
+from infrahub.core.schema.schema_branch import SchemaBranch
 from infrahub.database import InfrahubDatabase
 from infrahub.exceptions import ValidationError
 
@@ -162,3 +163,42 @@ async def test_delete_with_cascade_both_directions_succeeds(
     assert {d.id for d in deleted} == {person_john_main.id, car_accord_main.id, car_prius_main.id}
     node_map = await NodeManager.get_many(db=db, ids=[person_john_main.id, car_accord_main.id, car_prius_main.id])
     assert node_map == {}
+
+
+async def test_delete_with_required_on_generic_prevented(db, default_branch, dependent_generics_schema: SchemaBranch):
+    human = await Node.init(db=db, schema="TestHuman", branch=default_branch)
+    await human.new(db=db, name="Jane", height=180)
+    await human.save(db=db)
+    dog = await Node.init(db=db, schema="TestDog", branch=default_branch)
+    await dog.new(db=db, name="Roofus", breed="whocares", weight=50, owner=human)
+    await dog.save(db=db)
+
+    with pytest.raises(ValidationError) as exc:
+        await NodeManager.delete(db=db, branch=default_branch, nodes=[human])
+
+    assert f"Cannot delete TestHuman '{human.id}'" in str(exc.value)
+    assert f"It is linked to mandatory relationship owner on node TestDog '{dog.id}'" in str(exc.value)
+
+    retrieved_human = await NodeManager.get_one(db=db, id=human.id)
+    assert retrieved_human.id == human.id
+
+
+async def test_delete_with_cascade_on_generic_allowed(db, default_branch, dependent_generics_schema: SchemaBranch):
+    # set TestPerson.animals to be cascade delete
+    schema_branch = registry.schema.get_schema_branch(name=default_branch.name)
+    for schema_kind in ("TestPerson", "TestHuman", "TestCylon"):
+        schema = schema_branch.get(name=schema_kind, duplicate=False)
+        schema.get_relationship("animals").on_delete = RelationshipDeleteBehavior.CASCADE
+
+    human = await Node.init(db=db, schema="TestHuman", branch=default_branch)
+    await human.new(db=db, name="Jane", height=180)
+    await human.save(db=db)
+    dog = await Node.init(db=db, schema="TestDog", branch=default_branch)
+    await dog.new(db=db, name="Roofus", breed="whocares", weight=50, owner=human)
+    await dog.save(db=db)
+
+    deleted = await NodeManager.delete(db=db, branch=default_branch, nodes=[human])
+
+    assert {d.id for d in deleted} == {human.id, dog.id}
+    node_map = await NodeManager.get_many(db=db, ids=[human.id, dog.id])
+    assert node_map == {}

changelog/4332.fixed.md

@@ -0,0 +1 @@
+Update delete constraints to correctly account for relationships on generics and relationships for which the peer kind is a generic.