diff --git a/backend/infrahub/core/diff/merger/serializer.py b/backend/infrahub/core/diff/merger/serializer.py index 13b83aedbb..8da971b7f2 100644 --- a/backend/infrahub/core/diff/merger/serializer.py +++ b/backend/infrahub/core/diff/merger/serializer.py @@ -161,6 +161,16 @@ async def serialize_diff( if node.conflict and node.conflict.selected_branch is ConflictSelection.BASE_BRANCH: continue node_action = self._get_action(action=node.action, conflict=node.conflict) + if node_action is DiffAction.REMOVED: + serialized_node_diffs.append( + NodeMergeDict( + uuid=node.uuid, + action=self._to_action_str(action=node_action), + attributes=[], + relationships=[], + ) + ) + continue serial_attr_diffs = [] for attr_diff in node.attributes: serial_attr_diff, attribute_property_diff = self._serialize_attribute( @@ -169,7 +179,7 @@ async def serialize_diff( if serial_attr_diff: serial_attr_diffs.append(serial_attr_diff) serialized_property_diffs.append(attribute_property_diff) - relationship_diffs = [] + relationship_diffs: list[RelationshipMergeDict] = [] for rel_diff in node.relationships: relationship_identifier = self._get_relationship_identifier( schema_kind=node.kind, relationship_name=rel_diff.name diff --git a/backend/infrahub/core/diff/query/merge.py b/backend/infrahub/core/diff/query/merge.py index b78b1d8434..dc5b614a2c 100644 --- a/backend/infrahub/core/diff/query/merge.py +++ b/backend/infrahub/core/diff/query/merge.py @@ -86,6 +86,64 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: <-[:IS_PART_OF { branch: $target_branch, branch_level: $branch_level, from: $at, status: node_rel_status }] -(n) } + // ------------------------------ + // shortcut to delete all attributes and relationships for this node if the node is deleted + // ------------------------------ + CALL { + WITH n, node_rel_status + WITH n, node_rel_status + WHERE node_rel_status = "deleted" + CALL { + WITH n + OPTIONAL MATCH (n)-[rel1:IS_RELATED]-(:Relationship)-[rel2]-(p) + WHERE (p.uuid IS NULL OR n.uuid <> p.uuid) + AND rel1.branch = $target_branch + AND rel2.branch = $target_branch + AND rel1.status = "active" + AND rel2.status = "active" + RETURN rel1, rel2 + UNION + WITH n + OPTIONAL MATCH (n)-[rel1:HAS_ATTRIBUTE]->(:Attribute)-[rel2]->() + WHERE type(rel2) <> "HAS_ATTRIBUTE" + AND rel1.branch = $target_branch + AND rel2.branch = $target_branch + AND rel1.status = "active" + AND rel2.status = "active" + RETURN rel1, rel2 + } + WITH n, rel1, rel2 + WHERE rel1.to IS NULL + AND rel2.to IS NULL + AND rel1.from <= $at + AND rel2.from <= $at + SET rel1.to = $at + SET rel2.to = $at + // ------------------------------ + // and delete HAS_OWNER and HAS_SOURCE edges to this node if the node is deleted + // ------------------------------ + WITH n + CALL { + WITH n + CALL { + WITH n + MATCH (n)<-[rel:HAS_OWNER]-() + WHERE rel.branch = $target_branch + AND rel.status = "active" + AND rel.from <= $at + AND rel.to IS NULL + RETURN rel + UNION + MATCH (n)<-[rel:HAS_SOURCE]-() + WHERE rel.branch = $target_branch + AND rel.status = "active" + AND rel.from <= $at + AND rel.to IS NULL + RETURN rel + } + SET rel.to = $at + } + } } WITH n, node_diff_map CALL { diff --git a/backend/tests/unit/core/diff/test_diff_and_merge.py b/backend/tests/unit/core/diff/test_diff_and_merge.py index 92aad03df1..ec22fb7ca7 100644 --- a/backend/tests/unit/core/diff/test_diff_and_merge.py +++ b/backend/tests/unit/core/diff/test_diff_and_merge.py @@ -14,12 +14,14 @@ from infrahub.core.initialization import create_branch from infrahub.core.manager import NodeManager from infrahub.core.node import Node +from infrahub.core.schema import SchemaRoot from infrahub.core.schema.attribute_schema import AttributeSchema from infrahub.core.schema.node_schema import NodeSchema from infrahub.core.schema.schema_branch import SchemaBranch from infrahub.core.timestamp import Timestamp from infrahub.database import InfrahubDatabase from infrahub.dependencies.registry import get_component_registry +from tests.unit.core.test_utils import verify_all_linked_edges_deleted class TestDiffAndMerge: @@ -643,3 +645,94 @@ async def test_base_delete_with_added_branch_relationship( # validate that car remains deleted after rollback rolled_back_car = await NodeManager.get_one(db=db, id=car_accord_main.id) assert rolled_back_car is None + + async def test_delete_with_many_relationship_added( + self, db: InfrahubDatabase, default_branch: Branch, car_person_schema_unregistered: SchemaRoot + ): + # remove TestCar relationship to TestPerson + car_schema = car_person_schema_unregistered.get(name="TestCar") + car_schema.relationships = [] + registry.schema.register_schema(schema=car_person_schema_unregistered, branch=default_branch.name) + # initial data + person_1 = await Node.init(db=db, schema="TestPerson", branch=default_branch) + await person_1.new(db=db, name="Alice", height=160) + await person_1.save(db=db) + person_2 = await Node.init(db=db, schema="TestPerson", branch=default_branch) + await person_2.new(db=db, name="Bob", height=161) + await person_2.save(db=db) + car_1 = await Node.init(db=db, schema="TestCar", branch=default_branch) + await car_1.new(db=db, name="smart", nbr_seats=2, is_electric=True) + await car_1.save(db=db) + car_2 = await Node.init(db=db, schema="TestCar", branch=default_branch) + await car_2.new(db=db, name="big", nbr_seats=12, is_electric=False) + await car_2.save(db=db) + # make the branch + branch2 = await create_branch(db=db, branch_name="branch2") + + # add relationship on main + person_1_main = await NodeManager.get_one(db=db, id=person_1.id) + await person_1_main.cars.update(db=db, data=[car_1, car_2]) + await person_1_main.save(db=db) + # delete node on branch + person_1_branch = await NodeManager.get_one(db=db, branch=branch2, id=person_1.id) + await person_1_branch.delete(db=db) + + # check that there are no conflicts + diff_coordinator = await self._get_diff_coordinator(db=db, branch=branch2) + enriched_diff = await diff_coordinator.update_branch_diff(base_branch=default_branch, diff_branch=branch2) + conflicts_map = enriched_diff.get_all_conflicts() + assert len(conflicts_map) == 0 + + # merge the branch + at = Timestamp() + diff_merger = await self._get_diff_merger(db=db, branch=branch2) + await diff_merger.merge_graph(at=at) + + # validate that person_1 is deleted + deleted_person = await NodeManager.get_one(db=db, id=person_1.id) + assert deleted_person is None + # validate that all attributes and relationships connected to person_1, + # including the relationship connecting car_1 and person_1 is deleted, + # requires a special query b/c TestCar has no relationship to TestPerson in the schema + await verify_all_linked_edges_deleted(db=db, node_uuid=person_1.id, branch_name=default_branch.name) + + @pytest.mark.parametrize("selection", [ConflictSelection.BASE_BRANCH, ConflictSelection.DIFF_BRANCH]) + async def test_attribute_update_with_conflict( + self, + db: InfrahubDatabase, + default_branch: Branch, + diff_repository: DiffRepository, + person_john_main: Node, + selection: ConflictSelection, + ): + main_value = 200 + branch_value = 150 + branch2 = await create_branch(db=db, branch_name="branch2") + person_main = await NodeManager.get_one(db=db, branch=default_branch, id=person_john_main.id) + person_main.height.value = main_value + await person_main.save(db=db) + person_branch = await NodeManager.get_one(db=db, branch=branch2, id=person_john_main.id) + person_branch.height.value = branch_value + await person_branch.save(db=db) + + # set the conflict resolution + diff_coordinator = await self._get_diff_coordinator(db=db, branch=branch2) + enriched_diff = await diff_coordinator.update_branch_diff(base_branch=default_branch, diff_branch=branch2) + conflicts_map = enriched_diff.get_all_conflicts() + assert len(conflicts_map) == 1 + expected_path = f"data/{person_john_main.id}/height/value" + assert expected_path in conflicts_map + conflict = conflicts_map[expected_path] + await diff_repository.update_conflict_by_id(conflict_id=conflict.uuid, selection=selection) + + # merge the branch + at = Timestamp() + diff_merger = await self._get_diff_merger(db=db, branch=branch2) + await diff_merger.merge_graph(at=at) + + # validate that person has correct age + updated_person = await NodeManager.get_one(db=db, branch=default_branch, id=person_john_main.id) + if selection is ConflictSelection.DIFF_BRANCH: + assert updated_person.height.value == branch_value + else: + assert updated_person.height.value == main_value diff --git a/backend/tests/unit/core/test_utils.py b/backend/tests/unit/core/test_utils.py index 64b9069c18..fe6b7bbc6a 100644 --- a/backend/tests/unit/core/test_utils.py +++ b/backend/tests/unit/core/test_utils.py @@ -3,6 +3,7 @@ import pytest from infrahub.core.utils import convert_ip_to_binary_str +from infrahub.database import InfrahubDatabase @pytest.mark.parametrize( @@ -15,3 +16,68 @@ ) def test_convert_ip_to_binary_str(input, response): assert convert_ip_to_binary_str(obj=input) == response + + +async def verify_all_linked_edges_deleted(db: InfrahubDatabase, node_uuid: str, branch_name: str) -> None: + """ + Verify that a node is completely deleted at the database level + + check that all edges linked to a given node on a given branch are deleted or inactive + """ + query = """ + MATCH (n:Node {uuid: $node_uuid})-[r1]-(attr_rel)-[r2]-(p) + WHERE p <> n + AND r1.branch = $target_branch + AND r2.branch = $target_branch + AND ( + "Attribute" IN labels(attr_rel) OR "Relationship" IN labels(attr_rel) + ) + WITH n, attr_rel, p, r1, r2 + ORDER by r1.from DESC, r2.from DESC + WITH n, type(r1) AS r1_type, attr_rel, type(r2) AS r2_type, p, head(collect(r1)) AS latest_r1, head(collect(r2)) AS latest_r2 + RETURN n, attr_rel, p, latest_r1, latest_r2, + ( + (latest_r1.status = "deleted" AND latest_r1.to IS NULL) + OR (latest_r1.status = "active" AND latest_r1.to IS NOT NULL) + ) AS latest_r1_is_deleted, + ( + (latest_r2.status = "deleted" AND latest_r2.to IS NULL) + OR (latest_r2.status = "active" AND latest_r2.to IS NOT NULL) + ) AS latest_r2_is_deleted + """ + records = await db.execute_query(query=query, params={"node_uuid": node_uuid, "target_branch": branch_name}) + for record in records: + if record.get("latest_r1_is_deleted") is False or record.get("latest_r2_is_deleted") is False: + node_uuid = record.get("n", {}).get("uuid") + r1 = record.get("latest_r1") + r1_type = r1.type if r1 else None + attr_rel_name = record.get("attr_rel", {}).get("name") + r2 = record.get("latest_r2") + r2_type = r2.type if r2 else None + p = record.get("p", {}) + p_label = p.get("uuid") or p.get("value") + raise ValueError( + f"Latest path '{node_uuid}'-[{r1_type}]-'{attr_rel_name}'-[{r2_type}]-'{p_label}' is not deleted" + ) + + query = """ + MATCH (n:Node {uuid: $node_uuid})<-[r1]-(attr_rel) + WHERE r1.branch = $target_branch + AND type(r1) IN ["HAS_OWNER", "HAS_SOURCE"] + WITH n, attr_rel, r1 + ORDER by r1.from DESC + WITH n, attr_rel, head(collect(r1)) AS latest_r1 + RETURN n, attr_rel, latest_r1, + ( + (latest_r1.status = "deleted" AND latest_r1.to IS NULL) + OR (latest_r1.status = "active" AND latest_r1.to IS NOT NULL) + ) AS latest_r1_is_deleted + """ + records = await db.execute_query(query=query, params={"node_uuid": node_uuid, "target_branch": branch_name}) + for record in records: + if record.get("latest_r1_is_deleted") is False: + node_uuid = record.get("n", {}).get("uuid") + r1 = record.get("latest_r1") + r1_type = r1.type if r1 else None + attr_rel_name = record.get("attr_rel", {}).get("name") + raise ValueError(f"Latest path '{node_uuid}'<-[{r1_type}]-'{attr_rel_name}' is not deleted")