diff --git a/backend/infrahub/core/diff/conflicts_enricher.py b/backend/infrahub/core/diff/conflicts_enricher.py index e00d98ad58..1502e79433 100644 --- a/backend/infrahub/core/diff/conflicts_enricher.py +++ b/backend/infrahub/core/diff/conflicts_enricher.py @@ -97,7 +97,11 @@ def _add_attribute_conflicts( for property_type in common_property_types: base_property = base_property_map[property_type] branch_property = branch_property_map[property_type] - if base_property.new_value != branch_property.new_value: + same_value = base_property.new_value == branch_property.new_value or ( + base_property.action is DiffAction.UNCHANGED + and base_property.previous_value == branch_property.previous_value + ) + if not same_value: self._add_property_conflict( base_property=base_property, branch_property=branch_property, diff --git a/backend/infrahub/core/query/diff.py b/backend/infrahub/core/query/diff.py index 51d2e532c5..041559b7cd 100644 --- a/backend/infrahub/core/query/diff.py +++ b/backend/infrahub/core/query/diff.py @@ -634,7 +634,22 @@ async def query_init(self, db: InfrahubDatabase, **kwargs): AND (p.branch_support IN $branch_support OR q.branch_support IN $branch_support) RETURN root, r_root, p, diff_rel, q } - WITH root, r_root, p, diff_rel, q + WITH root, r_root, p, diff_rel, q, from_time + // ------------------------------------- + // Exclude attributes/relationship under nodes deleted on this branch in the timeframe + // because those were all handled above at the node level + // ------------------------------------- + CALL { + WITH root, p, from_time + OPTIONAL MATCH (root)<-[r_root_deleted:IS_PART_OF {branch: $branch_name}]-(p) + WHERE from_time <= r_root_deleted.from <= $to_time + WITH r_root_deleted + ORDER BY r_root_deleted.status DESC + LIMIT 1 + RETURN COALESCE(r_root_deleted.status = "deleted", FALSE) AS node_deleted + } + WITH root, r_root, p, diff_rel, q, node_deleted + WHERE node_deleted = FALSE // ------------------------------------- // Get every path on this branch under each attribute/relationship // ------------------------------------- @@ -717,6 +732,37 @@ async def query_init(self, db: InfrahubDatabase, **kwargs): // Add base branch paths, if they exist, to capture previous values // Add peer-side of any relationships to get the peer's ID // ------------------------------------- + WITH n, p, from_time, diff_rel, diff_rel_path + CALL { + // ------------------------------------- + // Exclude properties under nodes and attributes/relationship deleted + // on this branch in the timeframe because those were all handled above + // ------------------------------------- + WITH n, p, from_time + CALL { + WITH n, from_time + OPTIONAL MATCH (root:Root)<-[r_root_deleted:IS_PART_OF {branch: $branch_name}]-(n) + WHERE from_time <= r_root_deleted.from <= $to_time + WITH r_root_deleted + ORDER BY r_root_deleted.status DESC + LIMIT 1 + RETURN COALESCE(r_root_deleted.status = "deleted", FALSE) AS node_deleted + } + WITH n, p, from_time, node_deleted + CALL { + WITH n, p, from_time + OPTIONAL MATCH (n)-[r_node_deleted {branch: $branch_name}]-(p) + WHERE from_time <= r_node_deleted.from <= $to_time + AND type(r_node_deleted) IN ["HAS_ATTRIBUTE", "IS_RELATED"] + WITH r_node_deleted + ORDER BY r_node_deleted.status DESC + LIMIT 1 + RETURN COALESCE(r_node_deleted.status = "deleted", FALSE) AS field_deleted + } + RETURN node_deleted OR field_deleted AS node_or_field_deleted + } + WITH n, p, diff_rel, diff_rel_path, node_or_field_deleted + WHERE node_or_field_deleted = FALSE WITH n, p, type(diff_rel) AS drt, head(collect(diff_rel_path)) AS deepest_diff_path CALL { WITH n, p, deepest_diff_path diff --git a/backend/tests/unit/core/diff/test_diff_calculator.py b/backend/tests/unit/core/diff/test_diff_calculator.py index 9202e3c7a9..1d27056150 100644 --- a/backend/tests/unit/core/diff/test_diff_calculator.py +++ b/backend/tests/unit/core/diff/test_diff_calculator.py @@ -1,13 +1,14 @@ import pytest from infrahub.core.branch import Branch -from infrahub.core.constants import DiffAction +from infrahub.core.constants import DiffAction, RelationshipCardinality from infrahub.core.constants.database import DatabaseEdgeType from infrahub.core.diff.calculator import DiffCalculator from infrahub.core.diff.model.path import NodeFieldSpecifier from infrahub.core.initialization import create_branch from infrahub.core.manager import NodeManager from infrahub.core.node import Node +from infrahub.core.schema_manager import SchemaBranch from infrahub.core.timestamp import Timestamp from infrahub.database import InfrahubDatabase @@ -1578,3 +1579,121 @@ async def test_diff_attribute_branch_update_with_separate_previous_base_update_c assert property_diff.new_value == "Little Alfred" assert property_diff.action is DiffAction.UPDATED assert branch_before_change < property_diff.changed_at < branch_after_change + + +async def test_branch_relationship_delete_with_property_update( + db: InfrahubDatabase, default_branch: Branch, animal_person_schema: SchemaBranch +): + person_schema = animal_person_schema.get(name="TestPerson") + dog_schema = animal_person_schema.get(name="TestDog") + persons = [] + for i in range(3): + person = await Node.init(db=db, schema=person_schema, branch=default_branch) + await person.new(db=db, name=f"Person{i}") + await person.save(db=db) + persons.append(person) + dogs = [] + for i in range(3): + dog = await Node.init(db=db, schema=dog_schema, branch=default_branch) + await dog.new(db=db, name=f"Dog{i}", breed=f"Breed{i}", owner=persons[i], best_friend=persons[i]) + await dog.save(db=db) + dogs.append(dog) + branch = await create_branch(db=db, branch_name="branch") + from_time = Timestamp() + dog_branch = await NodeManager.get_one(db=db, branch=branch, id=dogs[0].id) + before_branch_change = Timestamp() + await dog_branch.best_friend.update(db=db, data=[None]) + await dog_branch.save(db=db) + after_branch_change = Timestamp() + + dog_main = await NodeManager.get_one(db=db, id=dogs[0].id) + before_main_change = Timestamp() + await dog_main.best_friend.update(db=db, data={"id": persons[0].id, "_relation__is_visible": False}) + await dog_main.save(db=db) + after_main_change = Timestamp() + + 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() + ) + + base_diff = calculated_diffs.base_branch_diff + assert base_diff.branch == default_branch.name + node_diffs_by_id = {n.uuid: n for n in base_diff.nodes} + node_diff = node_diffs_by_id[dog_main.id] + assert node_diff.uuid == dog_main.id + assert node_diff.kind == "TestDog" + assert node_diff.action is DiffAction.UPDATED + assert len(node_diff.attributes) == 0 + assert len(node_diff.relationships) == 1 + rel_diffs_by_name = {r.name: r for r in node_diff.relationships} + rel_diff = rel_diffs_by_name["best_friend"] + assert rel_diff.cardinality is RelationshipCardinality.ONE + assert rel_diff.action is DiffAction.UPDATED + assert len(rel_diff.relationships) == 1 + rel_elements_by_peer_id = {e.peer_id: e for e in rel_diff.relationships} + rel_element_diff = rel_elements_by_peer_id[persons[0].id] + assert rel_element_diff.action is DiffAction.UPDATED + prop_diff_by_type = {p.property_type: p for p in rel_element_diff.properties} + prop_diff = prop_diff_by_type[DatabaseEdgeType.IS_VISIBLE] + assert prop_diff.action is DiffAction.UPDATED + assert prop_diff.new_value is False + assert prop_diff.previous_value is True + assert before_main_change < prop_diff.changed_at < after_main_change + for prop_diff in prop_diff_by_type.values(): + if prop_diff.property_type is DatabaseEdgeType.IS_VISIBLE: + continue + assert prop_diff.action is DiffAction.UNCHANGED + + branch_diff = calculated_diffs.diff_branch_diff + assert branch_diff.branch == branch.name + node_diffs_by_id = {n.uuid: n for n in branch_diff.nodes} + assert set(node_diffs_by_id.keys()) == {dog_branch.id, persons[0].id} + dog_node = node_diffs_by_id[dog_branch.id] + assert dog_node.action is DiffAction.UPDATED + assert len(dog_node.attributes) == 0 + assert len(dog_node.relationships) == 1 + rel_diffs_by_name = {r.name: r for r in dog_node.relationships} + rel_diff = rel_diffs_by_name["best_friend"] + assert rel_diff.cardinality is RelationshipCardinality.ONE + assert rel_diff.action is DiffAction.REMOVED + assert len(rel_diff.relationships) == 1 + rel_elements_by_peer_id = {e.peer_id: e for e in rel_diff.relationships} + rel_element_diff = rel_elements_by_peer_id[persons[0].id] + assert rel_element_diff.action is DiffAction.REMOVED + prop_diff_by_type = {p.property_type: p for p in rel_element_diff.properties} + assert len(prop_diff_by_type) == 3 + for property_type, previous_value in [ + (DatabaseEdgeType.IS_RELATED, persons[0].id), + (DatabaseEdgeType.IS_VISIBLE, True), + (DatabaseEdgeType.IS_PROTECTED, False), + ]: + prop_diff = prop_diff_by_type[property_type] + assert prop_diff.action is DiffAction.REMOVED + assert prop_diff.new_value is None + assert prop_diff.previous_value == previous_value + assert before_branch_change < prop_diff.changed_at < after_branch_change + person_node = node_diffs_by_id[persons[0].id] + assert person_node.action is DiffAction.UPDATED + assert len(person_node.attributes) == 0 + assert len(person_node.relationships) == 1 + rel_diffs_by_name = {r.name: r for r in person_node.relationships} + rel_diff = rel_diffs_by_name["best_friends"] + assert rel_diff.cardinality is RelationshipCardinality.MANY + assert rel_diff.action is DiffAction.UPDATED + assert len(rel_diff.relationships) == 1 + rel_elements_by_peer_id = {e.peer_id: e for e in rel_diff.relationships} + rel_element_diff = rel_elements_by_peer_id[dog_branch.id] + assert rel_element_diff.action is DiffAction.REMOVED + prop_diff_by_type = {p.property_type: p for p in rel_element_diff.properties} + assert len(prop_diff_by_type) == 3 + for property_type, previous_value in [ + (DatabaseEdgeType.IS_RELATED, dog_branch.id), + (DatabaseEdgeType.IS_VISIBLE, True), + (DatabaseEdgeType.IS_PROTECTED, False), + ]: + prop_diff = prop_diff_by_type[property_type] + assert prop_diff.action is DiffAction.REMOVED + assert prop_diff.new_value is None + assert prop_diff.previous_value == previous_value + assert before_branch_change < prop_diff.changed_at < after_branch_change