full and correct support for relationships with unit tests

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

@@ -66,7 +66,7 @@ class DiffRoot:
 
 
 @dataclass
-class DatabasePath:
+class DatabasePath:  # pylint: disable=too-many-public-methods
     root_node: Neo4jNode
     path_to_node: Neo4jRelationship
     node_node: Neo4jNode
@@ -83,7 +83,11 @@ def __str__(self) -> str:
         property_branch = self.path_to_property.get("branch")
         property_status = self.path_to_property.get("status")
         property_value = self.property_value if self.property_value is not None else self.peer_id
-        return f"branch={self.deepest_branch} (:Root)-[{node_branch=},{node_status=}]-({self.node_kind} '{self.node_id}')-[{attribute_branch=},{attribute_status=}]-({self.attribute_name})-[{property_branch=},{property_status=}]-({self.property_type=},{property_value=})"
+        return (
+            f"branch={self.deepest_branch} (:Root)-[{node_branch=},{node_status=}]-({self.node_kind}"
+            f" '{self.node_id}')-[{attribute_branch=},{attribute_status=}]-({self.attribute_name})-"
+            f"[{property_branch=},{property_status=}]-({self.property_type=},{property_value=})"
+        )
 
     @classmethod
     def from_cypher_path(cls, cypher_path: Neo4jPath) -> DatabasePath:
@@ -187,3 +191,9 @@ def peer_id(self) -> Optional[str]:
         if "Node" not in self.property_node.labels:
             return None
         return str(self.property_node.get("uuid"))
+
+    @property
+    def peer_kind(self) -> Optional[str]:
+        if "Node" not in self.property_node.labels:
+            return None
+        return str(self.property_node.get("kind"))

backend/infrahub/core/diff/query_parser.py

@@ -111,16 +111,17 @@ def get_property_details(self, from_time: Timestamp) -> tuple[DiffAction, Timest
                 action = DiffAction.REMOVED
                 previous = lone_value.value
             return (action, lone_value.changed_at, previous, new)
-        previous_diff_value = ordered_values[0]
+        previous_value = ordered_values[0].value
         new_diff_value = ordered_values[-1]
+        new_value = new_diff_value.value
         action = DiffAction.UPDATED
-        if previous_diff_value.value in (None, "NULL") and new_diff_value.value not in (None, "NULL"):
+        if previous_value in (None, "NULL") and new_value not in (None, "NULL"):
             action = DiffAction.ADDED
-        if previous_diff_value.value not in (None, "NULL") and new_diff_value.value in (None, "NULL"):
+        if previous_value not in (None, "NULL") and new_value in (None, "NULL"):
             action = DiffAction.REMOVED
-        if previous_diff_value.value == new_diff_value.value:
+        if previous_value == new_value or {previous_value, new_value} <= {None, "NULL"}:
             action = DiffAction.UNCHANGED
-        return (action, new_diff_value.changed_at, previous_diff_value.value, new_diff_value.value)
+        return (action, new_diff_value.changed_at, previous_value, new_value)
 
     def to_diff_property(self, from_time: Timestamp) -> DiffProperty:
         action, changed_at, previous_value, new_value = self.get_property_details(from_time=from_time)
@@ -174,9 +175,9 @@ def from_properties(
         if not properties:
             raise DiffNoChildPathError()
         peer_id = None
-        for property in properties:
-            if property.property_type == "IS_RELATED":
-                peer_id = property.value
+        for diff_property in properties:
+            if diff_property.property_type == "IS_RELATED":
+                peer_id = diff_property.value
                 break
         if not peer_id:
             raise DiffNoPeerIdError(f"Cannot identify peer ID for relationship property {(properties[0]).db_id}")
@@ -207,7 +208,10 @@ def _get_single_relationship_final_property(
         last_diff_prop = chronological_properties[-1]
         changed_at = last_diff_prop.changed_at
         if last_diff_prop is first_diff_prop:
-            new_value = None if last_diff_prop.status is RelationshipStatus.DELETED else last_diff_prop.value
+            if last_diff_prop.status is RelationshipStatus.DELETED:
+                previous_value = last_diff_prop.value
+            else:
+                new_value = last_diff_prop.value
         elif last_diff_prop.status != RelationshipStatus.DELETED:
             new_value = last_diff_prop.value
         action = DiffAction.UPDATED
@@ -217,7 +221,7 @@ def _get_single_relationship_final_property(
             action = DiffAction.ADDED
         elif previous_value not in (None, "NULL") and new_value in (None, "NULL"):
             action = DiffAction.REMOVED
-        elif previous_value == new_value:
+        elif previous_value == new_value or {previous_value, new_value} <= {None, "NULL"}:
             action = DiffAction.UNCHANGED
         return DiffProperty(
             property_type=property_type,
@@ -409,9 +413,7 @@ def _update_attribute_level(self, database_path: DatabasePath, diff_node: DiffNo
         )
         if not relationship_schema:
             return
-        diff_relationship = self._get_diff_relationship(
-            database_path=database_path, diff_node=diff_node, relationship_schema=relationship_schema
-        )
+        diff_relationship = self._get_diff_relationship(diff_node=diff_node, relationship_schema=relationship_schema)
         diff_relationship.add_path(database_path=database_path)
 
     def _get_diff_attribute(
@@ -443,7 +445,7 @@ def _update_attribute_property(
         )
 
     def _get_diff_relationship(
-        self, database_path: DatabasePath, diff_node: DiffNodeIntermediate, relationship_schema: RelationshipSchema
+        self, diff_node: DiffNodeIntermediate, relationship_schema: RelationshipSchema
     ) -> DiffRelationshipIntermediate:
         diff_relationship = diff_node.relationships_by_name.get(relationship_schema.name)
         if not diff_relationship:
@@ -495,7 +497,9 @@ def _apply_relationship_previous_values(
                 base_property_set = base_diff_relationship.properties_by_db_id.get(db_id)
                 if not base_property_set:
                     continue
-                base_diff_property_by_type: dict[str, Optional[DiffRelationshipPropertyIntermediate]] = {p.property_type: None for p in property_set}
+                base_diff_property_by_type: dict[str, Optional[DiffRelationshipPropertyIntermediate]] = {
+                    p.property_type: None for p in property_set
+                }
                 base_diff_property_by_type["IS_RELATED"] = None
                 for base_diff_property in base_property_set:
                     prop_type = base_diff_property.property_type

backend/infrahub/core/query/diff.py

@@ -523,7 +523,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs):
 
         query = """
             // all updated edges
-            MATCH (p:Node|Attribute|Relationship)-[diff_rel]->(q)
+            MATCH (p:Node|Attribute|Relationship)-[diff_rel]-(q)
             WHERE %(diff_rel_filter)s
             AND p.branch_support IN $branch_support
             AND %(p_node_where)s
@@ -561,6 +561,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs):
                 WITH diff_rel_path, latest_base_path, diff_rel, r_root, n, r_node, p
                 ORDER BY base_diff_rel.from DESC, r_node.from DESC, r_root.from DESC
                 LIMIT 1
+                // get peer node for updated relationship properties
                 WITH diff_rel_path, latest_base_path, diff_rel, r_root, n, r_node, p
                 OPTIONAL MATCH base_peer_path = (
                    (:Root)<-[r_root]-(n)-[r_node]-(p:Relationship)-[base_r_peer:IS_RELATED]-(base_peer:Node)
@@ -590,18 +591,16 @@ async def query_init(self, db: InfrahubDatabase, **kwargs):
                     AND r.branch IN $branch_names
                 )
                 AND p <> prop
-                WITH path, q, prop, r_prop, r_root
+                WITH path, prop, r_prop, r_root
                 ORDER BY
-                    q,
                     prop,
                     r_prop.branch = diff_rel.branch DESC,
                     r_root.branch = diff_rel.branch DESC,
                     r_prop.from DESC,
                     r_root.from DESC
-                WITH q, prop, head(collect(path)) AS latest_path
-                RETURN latest_path
+                WITH prop, head(collect(path)) AS latest_prop_path
+                RETURN collect(latest_prop_path) AS latest_paths
             }
-            WITH p, q, diff_rel, full_diff_paths, collect(latest_path) AS latest_paths
             WITH p, q, diff_rel, full_diff_paths + latest_paths AS full_diff_paths
             // whole node-paths - IS_PART_OF
             CALL {

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

@@ -168,7 +168,7 @@ async def test_attribute_branch_set_null(db: InfrahubDatabase, default_branch: B
     assert before_change < property_diff.changed_at < after_change
 
 
-async def test_node_branch_delete(db: InfrahubDatabase, default_branch: Branch, car_accord_main):
+async def test_node_branch_delete(db: InfrahubDatabase, default_branch: Branch, car_accord_main, person_john_main):
     branch = await create_branch(db=db, branch_name="branch")
     from_time = Timestamp(branch.created_at)
     car_branch = await NodeManager.get_one(db=db, branch=branch, id=car_accord_main.id)
@@ -188,12 +188,15 @@ async def test_node_branch_delete(db: InfrahubDatabase, default_branch: Branch,
     assert diff_parser.get_branches() == {branch.name}
     branch_root_path = diff_parser.get_diff_root_for_branch(branch=branch.name)
     assert branch_root_path.branch == branch.name
-    assert len(branch_root_path.nodes) == 1
-    node_diff = branch_root_path.nodes[0]
+    assert len(branch_root_path.nodes) == 2
+    node_diffs_by_id = {n.uuid: n for n in branch_root_path.nodes}
+    node_diff = node_diffs_by_id[car_accord_main.id]
     assert node_diff.uuid == car_accord_main.id
     assert node_diff.kind == "TestCar"
     assert node_diff.action is DiffAction.REMOVED
     assert len(node_diff.attributes) == 5
+    assert len(node_diff.relationships) == 1
+    relationship_diff = node_diff.relationships[0]
     attributes_by_name = {attr.name: attr for attr in node_diff.attributes}
     assert set(attributes_by_name.keys()) == {"name", "nbr_seats", "color", "is_electric", "transmission"}
     for attribute_name in attributes_by_name:
@@ -203,6 +206,30 @@ async def test_node_branch_delete(db: InfrahubDatabase, default_branch: Branch,
         diff_property = properties_by_type["HAS_VALUE"]
         assert diff_property.action is DiffAction.REMOVED
         assert diff_property.new_value is None
+    assert len(node_diff.relationships) == 1
+    relationship_diff = node_diff.relationships[0]
+    assert relationship_diff.name == "owner"
+    assert relationship_diff.action is DiffAction.REMOVED
+    assert len(relationship_diff.relationships) == 1
+    single_relationship_diff = relationship_diff.relationships[0]
+    assert single_relationship_diff.peer_id == person_john_main.id
+    assert single_relationship_diff.action is DiffAction.REMOVED
+    node_diff = node_diffs_by_id[person_john_main.id]
+    assert node_diff.uuid == person_john_main.id
+    assert node_diff.kind == "TestPerson"
+    assert node_diff.action is DiffAction.UPDATED
+    assert len(node_diff.attributes) == 0
+    assert len(node_diff.relationships) == 1
+    relationship_diff = node_diff.relationships[0]
+    assert relationship_diff.name == "cars"
+    assert relationship_diff.action is DiffAction.UPDATED
+    assert len(relationship_diff.relationships) == 1
+    single_relationship_diff = relationship_diff.relationships[0]
+    assert single_relationship_diff.peer_id == car_branch.id
+    assert single_relationship_diff.action is DiffAction.REMOVED
+    assert len(single_relationship_diff.properties) == 3
+    for diff_property in single_relationship_diff.properties:
+        assert diff_property.action is DiffAction.REMOVED
 
 
 async def test_node_branch_add(db: InfrahubDatabase, default_branch: Branch, car_accord_main):
@@ -355,8 +382,35 @@ async def test_relationship_one_property_branch_update(
     assert property_diff.changed_at < before_branch_change
     root_main_path = diff_parser.get_diff_root_for_branch(branch=default_branch.name)
     assert root_main_path.branch == default_branch.name
-    assert len(root_main_path.nodes) == 1
-    node_diff = root_main_path.nodes[0]
+    assert len(root_main_path.nodes) == 3
+    diff_nodes_by_id = {n.uuid: n for n in root_main_path.nodes}
+    node_diff = diff_nodes_by_id[person_jane_main.id]
+    assert node_diff.uuid == person_jane_main.id
+    assert node_diff.kind == "TestPerson"
+    assert node_diff.action is DiffAction.UPDATED
+    assert len(node_diff.attributes) == 0
+    assert len(node_diff.relationships) == 1
+    relationship_diff = node_diff.relationships[0]
+    assert relationship_diff.name == "cars"
+    assert relationship_diff.action is DiffAction.UPDATED
+    assert len(relationship_diff.relationships) == 1
+    single_relationship_diff = relationship_diff.relationships[0]
+    assert single_relationship_diff.peer_id == car_accord_main.id
+    assert single_relationship_diff.action is DiffAction.ADDED
+    node_diff = diff_nodes_by_id[person_john_main.id]
+    assert node_diff.uuid == person_john_main.id
+    assert node_diff.kind == "TestPerson"
+    assert node_diff.action is DiffAction.UPDATED
+    assert len(node_diff.attributes) == 0
+    assert len(node_diff.relationships) == 1
+    relationship_diff = node_diff.relationships[0]
+    assert relationship_diff.name == "cars"
+    assert relationship_diff.action is DiffAction.UPDATED
+    assert len(relationship_diff.relationships) == 1
+    single_relationship_diff = relationship_diff.relationships[0]
+    assert single_relationship_diff.peer_id == car_accord_main.id
+    assert single_relationship_diff.action is DiffAction.REMOVED
+    node_diff = diff_nodes_by_id[car_accord_main.id]
     assert node_diff.uuid == car_accord_main.id
     assert node_diff.kind == "TestCar"
     assert node_diff.action is DiffAction.UPDATED
@@ -415,3 +469,87 @@ async def test_relationship_one_property_branch_update(
     assert property_diff.new_value is None
     assert property_diff.action is DiffAction.REMOVED
     assert before_main_change < property_diff.changed_at < after_main_change
+
+
+async def test_add_node_branch(
+    db: InfrahubDatabase,
+    default_branch: Branch,
+    person_alfred_main,
+    person_jane_main,
+    person_john_main,
+    car_accord_main,
+):
+    branch = await create_branch(db=db, branch_name="branch")
+    from_time = Timestamp(branch.created_at)
+    new_car = await Node.init(db=db, branch=branch, schema="TestCar")
+    await new_car.new(db=db, name="Batmobile", color="#000000", owner=person_jane_main)
+    await new_car.save(db=db)
+
+    diff_query = await DiffAllPathsQuery.init(
+        db=db,
+        branch=branch,
+        base_branch=default_branch,
+    )
+    await diff_query.execute(db=db)
+    diff_parser = DiffQueryParser(
+        diff_query=diff_query, base_branch_name=default_branch.name, schema_manager=registry.schema, from_time=from_time
+    )
+    diff_parser.parse()
+
+    assert diff_parser.get_branches() == {branch.name}
+    root_path = diff_parser.get_diff_root_for_branch(branch=branch.name)
+    assert root_path.branch == branch.name
+    assert len(root_path.nodes) == 2
+    diff_nodes_by_id = {n.uuid: n for n in root_path.nodes}
+    node_diff = diff_nodes_by_id[person_jane_main.id]
+    assert node_diff.uuid == person_jane_main.id
+    assert node_diff.kind == "TestPerson"
+    assert node_diff.action is DiffAction.UPDATED
+    assert len(node_diff.attributes) == 0
+    assert len(node_diff.relationships) == 1
+    relationship_diff = node_diff.relationships[0]
+    assert relationship_diff.name == "cars"
+    assert relationship_diff.action is DiffAction.UPDATED
+    assert len(relationship_diff.relationships) == 1
+    single_relationship = relationship_diff.relationships[0]
+    assert single_relationship.peer_id == new_car.id
+    assert single_relationship.action is DiffAction.ADDED
+    assert len(single_relationship.properties) == 3
+    assert {p.property_type for p in single_relationship.properties} == {"IS_RELATED", "IS_VISIBLE", "IS_PROTECTED"}
+    assert all(p.action is DiffAction.ADDED for p in single_relationship.properties)
+    node_diff = diff_nodes_by_id[new_car.id]
+    assert node_diff.uuid == new_car.id
+    assert node_diff.kind == "TestCar"
+    assert node_diff.action is DiffAction.ADDED
+    assert len(node_diff.attributes) == 5
+    attributes_by_name = {a.name: a for a in node_diff.attributes}
+    assert set(attributes_by_name.keys()) == {"name", "color", "transmission", "nbr_seats", "is_electric"}
+    assert all(a.action is DiffAction.ADDED for a in node_diff.attributes)
+    attribute_diff = attributes_by_name["name"]
+    assert len(attribute_diff.properties) == 3
+    assert {(p.property_type, p.action, p.new_value, p.previous_value) for p in attribute_diff.properties} == {
+        ("IS_VISIBLE", DiffAction.ADDED, True, None),
+        ("IS_PROTECTED", DiffAction.ADDED, False, None),
+        ("HAS_VALUE", DiffAction.ADDED, "Batmobile", None),
+    }
+    attribute_diff = attributes_by_name["color"]
+    assert len(attribute_diff.properties) == 3
+    assert {(p.property_type, p.action, p.new_value, p.previous_value) for p in attribute_diff.properties} == {
+        ("IS_VISIBLE", DiffAction.ADDED, True, None),
+        ("IS_PROTECTED", DiffAction.ADDED, False, None),
+        ("HAS_VALUE", DiffAction.ADDED, "#000000", None),
+    }
+    assert len(node_diff.relationships) == 1
+    relationship_diff = node_diff.relationships[0]
+    assert relationship_diff.name == "owner"
+    assert relationship_diff.action is DiffAction.ADDED
+    assert len(relationship_diff.relationships) == 1
+    single_relationship = relationship_diff.relationships[0]
+    assert single_relationship.peer_id == person_jane_main.id
+    assert single_relationship.action is DiffAction.ADDED
+    assert len(single_relationship.properties) == 3
+    assert {(p.property_type, p.action, p.new_value, p.previous_value) for p in single_relationship.properties} == {
+        ("IS_VISIBLE", DiffAction.ADDED, True, None),
+        ("IS_PROTECTED", DiffAction.ADDED, False, None),
+        ("IS_RELATED", DiffAction.ADDED, person_jane_main.id, None),
+    }