fix hierarchical relationship handling in diff (#4889)

Author: ajtmccarty

backend/infrahub/core/diff/model/path.py

@@ -706,6 +706,34 @@ def peer_kind(self) -> Optional[str]:
             return None
         return str(self.property_node.get("kind"))
 
+    @property
+    def possible_relationship_directions(self) -> list[RelationshipDirection]:
+        path_to_node = "Node" in self.property_node.labels
+        attr_start_node = self.path_to_attribute.start_node
+        attr_end_node = self.path_to_attribute.end_node
+        prop_start_node = self.path_to_property.start_node
+        prop_end_node = self.path_to_property.end_node
+        if path_to_node and (
+            attr_start_node
+            and attr_start_node.element_id == self.node_node.element_id
+            and prop_start_node
+            and prop_start_node.element_id == self.attribute_node.element_id
+        ):
+            return [RelationshipDirection.OUTBOUND]
+        if path_to_node and (
+            attr_end_node
+            and attr_end_node.element_id == self.node_node.element_id
+            and prop_end_node
+            and prop_end_node.element_id == self.attribute_node.element_id
+        ):
+            return [RelationshipDirection.INBOUND]
+        # if we only have one Node->Relationship path, we cannot fully determine the relationship direction
+        if attr_start_node and attr_start_node.element_id == self.node_node.element_id:
+            return [RelationshipDirection.OUTBOUND, RelationshipDirection.BIDIR]
+        if attr_end_node and attr_end_node.element_id == self.node_node.element_id:
+            return [RelationshipDirection.INBOUND, RelationshipDirection.BIDIR]
+        return [RelationshipDirection.BIDIR, RelationshipDirection.INBOUND, RelationshipDirection.OUTBOUND]
+
 
 @dataclass
 class EnrichedNodeCreateRequest:

backend/infrahub/core/diff/query_parser.py

@@ -22,6 +22,7 @@
 if TYPE_CHECKING:
     from infrahub.core.branch import Branch
     from infrahub.core.query import QueryResult
+    from infrahub.core.schema import MainSchemaTypes
     from infrahub.core.schema.manager import SchemaManager
     from infrahub.core.schema.relationship_schema import RelationshipSchema
 
@@ -517,6 +518,18 @@ def _get_diff_node(self, database_path: DatabasePath, diff_root: DiffRootInterme
         diff_node.track_database_path(database_path=database_path)
         return diff_node
 
+    def _get_relationship_schema(
+        self, database_path: DatabasePath, node_schema: MainSchemaTypes
+    ) -> RelationshipSchema | None:
+        relationship_schemas = node_schema.get_relationships_by_identifier(id=database_path.attribute_name)
+        if len(relationship_schemas) == 1:
+            return relationship_schemas[0]
+        possible_path_directions = database_path.possible_relationship_directions
+        for rel_schema in relationship_schemas:
+            if rel_schema.direction in possible_path_directions:
+                return rel_schema
+        return None
+
     def _update_attribute_level(self, database_path: DatabasePath, diff_node: DiffNodeIntermediate) -> None:
         node_schema = self.schema_manager.get(
             name=database_path.node_kind, branch=database_path.deepest_branch, duplicate=False
@@ -525,9 +538,7 @@ def _update_attribute_level(self, database_path: DatabasePath, diff_node: DiffNo
             diff_attribute = self._get_diff_attribute(database_path=database_path, diff_node=diff_node)
             self._update_attribute_property(database_path=database_path, diff_attribute=diff_attribute)
             return
-        relationship_schema = node_schema.get_relationship_by_identifier(
-            id=database_path.attribute_name, raise_on_error=False
-        )
+        relationship_schema = self._get_relationship_schema(database_path=database_path, node_schema=node_schema)
         if not relationship_schema:
             return
         diff_relationship = self._get_diff_relationship(diff_node=diff_node, relationship_schema=relationship_schema)

backend/tests/unit/core/diff/test_diff_calculator.py

@@ -1,7 +1,8 @@
 import pytest
 
+from infrahub.core import registry
 from infrahub.core.branch import Branch
-from infrahub.core.constants import DiffAction, RelationshipCardinality
+from infrahub.core.constants import DiffAction, InfrahubKind, RelationshipCardinality
 from infrahub.core.constants.database import DatabaseEdgeType
 from infrahub.core.diff.calculator import DiffCalculator
 from infrahub.core.diff.model.path import NodeFieldSpecifier
@@ -2283,3 +2284,146 @@ async def test_property_update_then_relationship_deleted(
 
     base_diff_root = calculated_diffs.base_branch_diff
     assert base_diff_root.nodes == []
+
+
+async def test_hierarchy_with_same_kind_parent_and_child(
+    db: InfrahubDatabase,
+    default_branch: Branch,
+    register_core_models_schema: SchemaBranch,
+    register_ipam_schema: SchemaBranch,
+):
+    prefix_schema = registry.schema.get_node_schema(name="IpamIPPrefix")
+    ip_namespace = await Node.init(db=db, schema=InfrahubKind.NAMESPACE)
+    await ip_namespace.new(db=db, name="ns1")
+    await ip_namespace.save(db=db)
+    top_node = await Node.init(db=db, schema=prefix_schema)
+    await top_node.new(db=db, prefix="10.0.0.0/7", ip_namespace=ip_namespace)
+    await top_node.save(db=db)
+    mid_node = await Node.init(db=db, schema=prefix_schema)
+    await mid_node.new(db=db, prefix="10.0.0.0/8", ip_namespace=ip_namespace)
+    await mid_node.save(db=db)
+    bottom_node = await Node.init(db=db, schema=prefix_schema)
+    await bottom_node.new(db=db, prefix="10.0.0.0/9", ip_namespace=ip_namespace)
+    await bottom_node.save(db=db)
+    branch = await create_branch(db=db, branch_name="branch2")
+    from_time = Timestamp()
+    mid_branch = await NodeManager.get_one(db=db, branch=branch, id=mid_node.id)
+    await mid_branch.parent.update(db=db, data=top_node.id)
+    await mid_branch.save(db=db)
+    bottom_branch = await NodeManager.get_one(db=db, branch=branch, id=bottom_node.id)
+    await bottom_branch.parent.update(db=db, data=mid_node.id)
+    await bottom_branch.save(db=db)
+
+    diff_calculator = DiffCalculator(db=db)
+    calculated_diffs = await diff_calculator.calculate_diff(
+        base_branch=default_branch, diff_branch=branch, from_time=from_time, to_time=Timestamp()
+    )
+
+    branch_root_path = calculated_diffs.diff_branch_diff
+    assert branch_root_path.branch == branch.name
+    nodes_by_id = {n.uuid: n for n in branch_root_path.nodes}
+    assert set(nodes_by_id.keys()) == {top_node.id, mid_node.id, bottom_node.id}
+    # top node
+    node_diff = nodes_by_id[top_node.id]
+    assert node_diff.action is DiffAction.UPDATED
+    assert len(node_diff.attributes) == 0
+    rels_by_name = {r.name: r for r in node_diff.relationships}
+    assert set(rels_by_name.keys()) == {"children"}
+    children_rel = rels_by_name["children"]
+    assert children_rel.action is DiffAction.UPDATED
+    assert len(children_rel.relationships) == 1
+    child_element = children_rel.relationships[0]
+    assert child_element.action is DiffAction.ADDED
+    assert child_element.peer_id == mid_node.id
+    properties_by_type = {p.property_type: p for p in child_element.properties}
+    assert set(properties_by_type.keys()) == {
+        DatabaseEdgeType.IS_RELATED,
+        DatabaseEdgeType.IS_PROTECTED,
+        DatabaseEdgeType.IS_VISIBLE,
+    }
+    for prop_type, new_value in (
+        (DatabaseEdgeType.IS_RELATED, mid_node.id),
+        (DatabaseEdgeType.IS_PROTECTED, False),
+        (DatabaseEdgeType.IS_VISIBLE, True),
+    ):
+        diff_prop = properties_by_type[prop_type]
+        assert diff_prop.action is DiffAction.ADDED
+        assert diff_prop.previous_value is None
+        assert diff_prop.new_value == new_value
+
+    # middle node
+    node_diff = nodes_by_id[mid_node.id]
+    assert node_diff.action is DiffAction.UPDATED
+    assert len(node_diff.attributes) == 0
+    rels_by_name = {r.name: r for r in node_diff.relationships}
+    assert set(rels_by_name.keys()) == {"parent", "children"}
+    parent_rel = rels_by_name["parent"]
+    assert parent_rel.action is DiffAction.ADDED
+    assert len(parent_rel.relationships) == 1
+    parent_element = parent_rel.relationships[0]
+    assert parent_element.action is DiffAction.ADDED
+    assert parent_element.peer_id == top_node.id
+    properties_by_type = {p.property_type: p for p in parent_element.properties}
+    assert set(properties_by_type.keys()) == {
+        DatabaseEdgeType.IS_RELATED,
+        DatabaseEdgeType.IS_PROTECTED,
+        DatabaseEdgeType.IS_VISIBLE,
+    }
+    for prop_type, new_value in (
+        (DatabaseEdgeType.IS_RELATED, top_node.id),
+        (DatabaseEdgeType.IS_PROTECTED, False),
+        (DatabaseEdgeType.IS_VISIBLE, True),
+    ):
+        diff_prop = properties_by_type[prop_type]
+        assert diff_prop.action is DiffAction.ADDED
+        assert diff_prop.previous_value is None
+        assert diff_prop.new_value == new_value
+    children_rel = rels_by_name["children"]
+    assert children_rel.action is DiffAction.UPDATED
+    assert len(children_rel.relationships) == 1
+    child_element = children_rel.relationships[0]
+    assert child_element.action is DiffAction.ADDED
+    assert child_element.peer_id == bottom_node.id
+    properties_by_type = {p.property_type: p for p in child_element.properties}
+    assert set(properties_by_type.keys()) == {
+        DatabaseEdgeType.IS_RELATED,
+        DatabaseEdgeType.IS_PROTECTED,
+        DatabaseEdgeType.IS_VISIBLE,
+    }
+    for prop_type, new_value in (
+        (DatabaseEdgeType.IS_RELATED, bottom_node.id),
+        (DatabaseEdgeType.IS_PROTECTED, False),
+        (DatabaseEdgeType.IS_VISIBLE, True),
+    ):
+        diff_prop = properties_by_type[prop_type]
+        assert diff_prop.action is DiffAction.ADDED
+        assert diff_prop.previous_value is None
+        assert diff_prop.new_value == new_value
+
+    # bottom node
+    node_diff = nodes_by_id[bottom_node.id]
+    assert node_diff.action is DiffAction.UPDATED
+    assert len(node_diff.attributes) == 0
+    rels_by_name = {r.name: r for r in node_diff.relationships}
+    assert set(rels_by_name.keys()) == {"parent"}
+    parent_rel = rels_by_name["parent"]
+    assert parent_rel.action is DiffAction.ADDED
+    assert len(parent_rel.relationships) == 1
+    parent_element = parent_rel.relationships[0]
+    assert parent_element.action is DiffAction.ADDED
+    assert parent_element.peer_id == mid_node.id
+    properties_by_type = {p.property_type: p for p in parent_element.properties}
+    assert set(properties_by_type.keys()) == {
+        DatabaseEdgeType.IS_RELATED,
+        DatabaseEdgeType.IS_PROTECTED,
+        DatabaseEdgeType.IS_VISIBLE,
+    }
+    for prop_type, new_value in (
+        (DatabaseEdgeType.IS_RELATED, mid_node.id),
+        (DatabaseEdgeType.IS_PROTECTED, False),
+        (DatabaseEdgeType.IS_VISIBLE, True),
+    ):
+        diff_prop = properties_by_type[prop_type]
+        assert diff_prop.action is DiffAction.ADDED
+        assert diff_prop.previous_value is None
+        assert diff_prop.new_value == new_value