Fix relationship changelogs with new RelationshipDeleteAllQuery

Author: LucasG0

backend/infrahub/core/changelog/diff.py

@@ -177,7 +177,7 @@ def _process_node_cardinality_one_relationship(
                             value_previous=rel_prop.previous_value,
                         )
 
-        node.add_relationship(relationship=changelog_rel)
+        node.add_relationship(relationship_changelog=changelog_rel)
 
     def _convert_string_boolean_value(self, value: str | None) -> bool | None:
         """Convert string based boolean for is_protected and is_visible."""
@@ -227,7 +227,7 @@ def _process_node_cardinality_many_relationship(
 
             changelog_rel.peers.append(peer_log)
 
-        node.add_relationship(relationship=changelog_rel)
+        node.add_relationship(relationship_changelog=changelog_rel)
 
     def collect_changelogs(self) -> Sequence[tuple[DiffAction, NodeChangelog]]:
         self._populate_diff_nodes()

backend/infrahub/core/changelog/models.py

@@ -129,8 +129,8 @@ def add_property(self, name: str, value_current: bool | str | None, value_previo
     def set_parent(self, parent_id: str, parent_kind: str) -> None:
         self._parent = ChangelogRelatedNode(node_id=parent_id, node_kind=parent_kind)
 
-    def set_parent_from_relationship(self, relationship: Relationship) -> None:
-        if relationship.schema.kind == RelationshipKind.PARENT:
+    def set_parent_from_relationship(self, rel_kind: RelationshipKind) -> None:
+        if rel_kind == RelationshipKind.PARENT:
             if (
                 self.peer_status in [DiffAction.ADDED, DiffAction.UNCHANGED, DiffAction.UPDATED]
                 and self.peer_id
@@ -308,14 +308,14 @@ def add_attribute(self, attribute: AttributeChangelog) -> None:
             self.attributes[attribute.name] = attribute
 
     def add_relationship(
-        self, relationship: RelationshipCardinalityOneChangelog | RelationshipCardinalityManyChangelog
+        self, relationship_changelog: RelationshipCardinalityOneChangelog | RelationshipCardinalityManyChangelog
     ) -> None:
-        if isinstance(relationship, RelationshipCardinalityOneChangelog) and relationship.parent:
-            self.add_parent(parent=relationship.parent)
-        if relationship.is_empty:
+        if isinstance(relationship_changelog, RelationshipCardinalityOneChangelog) and relationship_changelog.parent:
+            self.add_parent(parent=relationship_changelog.parent)
+        if relationship_changelog.is_empty:
             return
 
-        self.relationships[relationship.name] = relationship
+        self.relationships[relationship_changelog.name] = relationship_changelog
 
     def create_attribute(self, attribute: BaseAttribute) -> None:
         changelog_attribute = AttributeChangelog(
@@ -383,7 +383,7 @@ def remove_peer(self, peer_data: RelationshipPeerData) -> None:
     def _set_cardinality_one_peer(self, relationship: Relationship) -> None:
         self.cardinality_one_relationship.peer_id = relationship.peer_id
         self.cardinality_one_relationship.peer_kind = relationship.get_peer_kind()
-        self.cardinality_one_relationship.set_parent_from_relationship(relationship=relationship)
+        self.cardinality_one_relationship.set_parent_from_relationship(rel_kind=relationship.schema.kind)
 
     def add_parent_from_relationship(self, relationship: Relationship) -> None:
         if self.schema.cardinality == RelationshipCardinality.ONE:
@@ -420,18 +420,16 @@ def add_updated_relationship(
                     value_current=getattr(relationship, property_name),
                     value_previous=previous_value,
                 )
-            self.cardinality_one_relationship.set_parent_from_relationship(relationship=relationship)
+            self.cardinality_one_relationship.set_parent_from_relationship(rel_kind=relationship.schema.kind)
 
-    def delete_relationship(self, relationship: Relationship) -> None:
+    def delete_relationship(self, peer_id: str, peer_kind: str, rel_schema: RelationshipSchema) -> None:
         if self.schema.cardinality == RelationshipCardinality.ONE:
-            self.cardinality_one_relationship.peer_id_previous = relationship.get_peer_id()
-            self.cardinality_one_relationship.peer_kind_previous = relationship.get_peer_kind()
-            self.cardinality_one_relationship.set_parent_from_relationship(relationship=relationship)
+            self.cardinality_one_relationship.peer_id_previous = peer_id
+            self.cardinality_one_relationship.peer_kind_previous = peer_kind
+            self.cardinality_one_relationship.set_parent_from_relationship(rel_kind=rel_schema.kind)
 
         elif self.schema.cardinality == RelationshipCardinality.MANY:
-            self.cardinality_many_relationship.remove_peer(
-                peer_id=relationship.get_peer_id(), peer_kind=relationship.get_peer_kind()
-            )
+            self.cardinality_many_relationship.remove_peer(peer_id=peer_id, peer_kind=peer_kind)
 
     @property
     def changelog(self) -> RelationshipCardinalityOneChangelog | RelationshipCardinalityManyChangelog:

backend/infrahub/core/constants/schema.py

@@ -1,5 +1,7 @@
 from enum import Enum, Flag, auto
 
+PARENT_CHILD_IDENTIFIER = "parent__child"
+
 
 class FlagProperty(Enum):
     IS_VISIBLE = "is_visible"

backend/infrahub/core/node/__init__.py

@@ -7,23 +7,16 @@
 from infrahub_sdk.uuidt import UUIDT
 
 from infrahub.core import registry
-<<<<<<< HEAD
 from infrahub.core.changelog.models import NodeChangelog
 from infrahub.core.constants import (
+    GLOBAL_BRANCH_NAME,
     OBJECT_TEMPLATE_NAME_ATTR,
     OBJECT_TEMPLATE_RELATIONSHIP_NAME,
-=======
-from infrahub.core.constants import (
-    GLOBAL_BRANCH_NAME,
->>>>>>> stable
     BranchSupportType,
     ComputedAttributeKind,
     InfrahubKind,
     RelationshipCardinality,
-<<<<<<< HEAD
     RelationshipKind,
-=======
->>>>>>> stable
 )
 from infrahub.core.constants.schema import SchemaElementPathType
 from infrahub.core.protocols import CoreNumberPool, CoreObjectTemplate
@@ -659,7 +652,7 @@ async def _update(
                 processed_relationships.append(name)
                 rel: RelationshipManager = getattr(self, name)
                 updated_relationship = await rel.save(at=update_at, db=db)
-                node_changelog.add_relationship(relationship=updated_relationship)
+                node_changelog.add_relationship(relationship_changelog=updated_relationship)
 
         if len(processed_relationships) != len(self._relationships):
             # Analyze if the node has a parent and add it to the changelog if missing
@@ -699,24 +692,17 @@ async def delete(self, db: InfrahubDatabase, at: Optional[Timestamp] = None) ->
             if deleted_attribute:
                 node_changelog.add_attribute(attribute=deleted_attribute)
 
-<<<<<<< HEAD
-        # Go over the list of relationships and update them one by one
-        for name in self._relationships:
-            rel: RelationshipManager = getattr(self, name)
-            updated_relationship = await rel.delete(at=delete_at, db=db)
-            node_changelog.add_relationship(relationship=updated_relationship)
-
-        # Need to check if there are some unidirectional relationship as well
-        # For example, if we delete a tag, we must check the permissions and update all the relationships pointing at it
-=======
->>>>>>> stable
         branch = self.get_branch_based_on_support_type()
 
         delete_query = await RelationshipDeleteAllQuery.init(
             db=db, node_id=self.get_id(), branch=branch, at=delete_at, branch_agnostic=branch.name == GLOBAL_BRANCH_NAME
         )
         await delete_query.execute(db=db)
 
+        deleted_relationships_changelogs = delete_query.get_deleted_relationships_changelog(self._schema)
+        for relationship_changelog in deleted_relationships_changelogs:
+            node_changelog.add_relationship(relationship_changelog=relationship_changelog)
+
         # Update the relationship to the branch itself
         query = await NodeGetListQuery.init(
             db=db,

backend/infrahub/core/query/relationship.py

@@ -7,12 +7,18 @@
 
 from infrahub_sdk.uuidt import UUIDT
 
+from infrahub.core.changelog.models import (
+    ChangelogRelationshipMapper,
+    RelationshipCardinalityManyChangelog,
+    RelationshipCardinalityOneChangelog,
+)
 from infrahub.core.constants import RelationshipDirection, RelationshipStatus
 from infrahub.core.constants.database import DatabaseEdgeType
 from infrahub.core.query import Query, QueryType
 from infrahub.core.query.subquery import build_subquery_filter, build_subquery_order
 from infrahub.core.timestamp import Timestamp
 from infrahub.core.utils import extract_field_filters
+from infrahub.log import get_logger
 
 if TYPE_CHECKING:
     from uuid import UUID
@@ -22,11 +28,13 @@
     from infrahub.core.branch import Branch
     from infrahub.core.node import Node
     from infrahub.core.relationship import Relationship
-    from infrahub.core.schema import RelationshipSchema
+    from infrahub.core.schema import NodeSchema, RelationshipSchema
     from infrahub.database import InfrahubDatabase
 
 # pylint: disable=redefined-builtin,too-many-lines
 
+log = get_logger()
+
 
 @dataclass
 class RelData:
@@ -959,6 +967,8 @@ class RelationshipDeleteAllQuery(Query):
     - Set `to` time if an active edge exist on the same branch.
     - Create `deleted` edge.
     - Apply above to every edges linked to any connected Relationship node.
+    This query returns node uuids/kinds and corresponding relationship identifiers of deleted nodes,
+    that are later used to update node changelog.
     """
 
     name = "node_delete_all_relationships"
@@ -969,7 +979,7 @@ def __init__(self, node_id: str, **kwargs):
         self.node_id = node_id
         super().__init__(**kwargs)
 
-    async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
+    async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:  # noqa: ARG002
         self.params["source_id"] = kwargs["node_id"]
         self.params["branch"] = self.branch.name
 
@@ -987,30 +997,29 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
         )
         self.params.update(rel_params)
 
-        query_match_relationships = """
+        query = """
         MATCH (s:Node { uuid: $source_id })-[active_edge:IS_RELATED]-(rl:Relationship)
         WHERE %(active_rel_filter)s AND active_edge.status = "active"
         WITH DISTINCT rl
         """ % {"active_rel_filter": active_rel_filter}
 
-        self.add_to_query(query_match_relationships)
+        edge_types = [
+            DatabaseEdgeType.IS_VISIBLE.value,
+            DatabaseEdgeType.IS_PROTECTED.value,
+            DatabaseEdgeType.HAS_OWNER.value,
+            DatabaseEdgeType.HAS_SOURCE.value,
+        ]
 
         for arrow_left, arrow_right in (("<-", "-"), ("-", "->")):
-            for edge_type in [
-                DatabaseEdgeType.IS_RELATED.value,
-                DatabaseEdgeType.IS_VISIBLE.value,
-                DatabaseEdgeType.IS_PROTECTED.value,
-                DatabaseEdgeType.HAS_OWNER.value,
-                DatabaseEdgeType.HAS_SOURCE.value,
-            ]:
-                query = """
+            for edge_type in edge_types:
+                sub_query = """
                     CALL {
                         WITH rl
                         MATCH (rl)%(arrow_left)s[active_edge:%(edge_type)s]%(arrow_right)s(n)
                         WHERE %(active_rel_filter)s AND active_edge.status ="active"
                         CREATE (rl)%(arrow_left)s[deleted_edge:%(edge_type)s $rel_prop]%(arrow_right)s(n)
                         SET deleted_edge.hierarchy = active_edge.hierarchy
-                        WITH active_edge
+                        WITH active_edge, n
                         WHERE active_edge.branch = $branch AND active_edge.to IS NULL
                         SET active_edge.to = $at
                     }
@@ -1020,4 +1029,92 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
                     "active_rel_filter": active_rel_filter,
                     "edge_type": edge_type,
                 }
-                self.add_to_query(query)
+
+                query += sub_query
+
+        # We only want to return uuid/kind of `Node` connected through `IS_RELATED` edges.
+        query += """
+        CALL {
+            WITH rl
+            MATCH (rl)-[active_edge:IS_RELATED]->(n)
+            WHERE %(active_rel_filter)s AND active_edge.status ="active"
+            CREATE (rl)-[deleted_edge:IS_RELATED $rel_prop]->(n)
+            SET deleted_edge.hierarchy = active_edge.hierarchy
+            WITH rl, active_edge, n
+            WHERE active_edge.branch = $branch AND active_edge.to IS NULL
+            SET active_edge.to = $at
+            RETURN
+                n.uuid as uuid,
+                n.kind as kind,
+                rl.name as rel_identifier,
+                "outbound" as rel_direction
+
+            UNION
+
+            WITH rl
+            MATCH (rl)<-[active_edge:IS_RELATED]-(n)
+            WHERE %(active_rel_filter)s AND active_edge.status ="active"
+            CREATE (rl)<-[deleted_edge:IS_RELATED $rel_prop]-(n)
+            SET deleted_edge.hierarchy = active_edge.hierarchy
+            WITH rl, active_edge, n
+            WHERE active_edge.branch = $branch AND active_edge.to IS NULL
+            SET active_edge.to = $at
+            RETURN
+                n.uuid as uuid,
+                n.kind as kind,
+                rl.name as rel_identifier,
+                "inbound" as rel_direction
+        }
+        RETURN DISTINCT uuid, kind, rel_identifier, rel_direction
+        """ % {
+            "active_rel_filter": active_rel_filter,
+        }
+
+        self.add_to_query(query)
+
+    def get_deleted_relationships_changelog(
+        self, node_schema: NodeSchema
+    ) -> list[RelationshipCardinalityOneChangelog | RelationshipCardinalityManyChangelog]:
+        rel_identifier_to_changelog_mapper = {}
+
+        for result in self.get_results():
+            peer_uuid = result.data["uuid"]
+            if peer_uuid == self.node_id:
+                continue
+
+            rel_identifier = result.data["rel_identifier"]
+            kind = result.data["kind"]
+            deleted_rel_schemas = [
+                rel_schema for rel_schema in node_schema.relationships if rel_schema.identifier == rel_identifier
+            ]
+
+            if len(deleted_rel_schemas) == 0:
+                continue  # TODO Unidirectional relationship changelog should be handled, cf IFC-1319.
+
+            if len(deleted_rel_schemas) > 2:
+                log.error(f"Duplicated relationship schema with identifier {rel_identifier}")
+                continue
+
+            if len(deleted_rel_schemas) == 2:
+                # Hierarchical schema nodes have 2 relationships with `parent_child` identifiers,
+                # which are differentiated by their direction within the database.
+                # assert rel_identifier != PARENT_CHILD_IDENTIFIER
+
+                rel_direction = result.data["rel_direction"]
+                deleted_rel_schema = (
+                    deleted_rel_schemas[0]
+                    if deleted_rel_schemas[0].direction.value == rel_direction
+                    else deleted_rel_schemas[1]
+                )
+            else:
+                deleted_rel_schema = deleted_rel_schemas[0]
+
+            try:
+                changelog_mapper = rel_identifier_to_changelog_mapper[rel_identifier]
+            except KeyError:
+                changelog_mapper = ChangelogRelationshipMapper(schema=deleted_rel_schema)
+                rel_identifier_to_changelog_mapper[rel_identifier] = changelog_mapper
+
+            changelog_mapper.delete_relationship(peer_id=peer_uuid, peer_kind=kind, rel_schema=deleted_rel_schema)
+
+        return [changelog_mapper.changelog for changelog_mapper in rel_identifier_to_changelog_mapper.values()]

backend/infrahub/core/relationship/model.py

@@ -1208,7 +1208,9 @@ async def delete(
         await self._fetch_relationships(at=delete_at, db=db, force_refresh=True)
 
         for rel in await self.get_relationships(db=db):
-            relationship_mapper.delete_relationship(relationship=rel)
+            relationship_mapper.delete_relationship(
+                peer_kind=rel.get_peer_kind(), peer_id=rel.get_peer_id(), rel_schema=rel.schema
+            )
             await rel.delete(at=delete_at, db=db)
 
         return relationship_mapper.changelog

backend/infrahub/core/schema/schema_branch.py

@@ -55,6 +55,7 @@
 from infrahub.utils import format_label
 from infrahub.visuals import select_color
 
+from ..constants.schema import PARENT_CHILD_IDENTIFIER
 from .constants import INTERNAL_SCHEMA_NODE_KINDS, SchemaNamespace
 from .schema_branch_computed import ComputedAttributes
 
@@ -1535,7 +1536,7 @@ def add_groups(self) -> None:
     def _get_hierarchy_child_rel(self, peer: str, hierarchical: str | None, read_only: bool) -> RelationshipSchema:
         return RelationshipSchema(
             name="children",
-            identifier="parent__child",
+            identifier=PARENT_CHILD_IDENTIFIER,
             peer=peer,
             kind=RelationshipKind.HIERARCHY,
             cardinality=RelationshipCardinality.MANY,
@@ -1550,7 +1551,7 @@ def _get_hierarchy_parent_rel(
     ) -> RelationshipSchema:
         return RelationshipSchema(
             name="parent",
-            identifier="parent__child",
+            identifier=PARENT_CHILD_IDENTIFIER,
             peer=peer,
             kind=RelationshipKind.HIERARCHY,
             cardinality=RelationshipCardinality.ONE,

backend/tests/unit/core/changelog/test_models.py

@@ -299,8 +299,8 @@ async def test_node_changelog_delete_with_cardinality_many_relationship(
     await person1_update.delete(db=db)
 
     animals = person1_update.node_changelog.relationships["animals"].peers
-    assert RelationshipPeerChangelog(peer_id=dog1.id, peer_kind="TestAnimal", peer_status=DiffAction.REMOVED) in animals
-    assert RelationshipPeerChangelog(peer_id=dog2.id, peer_kind="TestAnimal", peer_status=DiffAction.REMOVED) in animals
+    assert RelationshipPeerChangelog(peer_id=dog1.id, peer_kind="TestDog", peer_status=DiffAction.REMOVED) in animals
+    assert RelationshipPeerChangelog(peer_id=dog2.id, peer_kind="TestDog", peer_status=DiffAction.REMOVED) in animals
 
 
 async def test_node_changelog_parent(db: InfrahubDatabase, default_branch, car_person_schema: None) -> None: