Skip to content

Commit

Permalink
handle node deleted merge edge case
Browse files Browse the repository at this point in the history
  • Loading branch information
ajtmccarty committed Nov 25, 2024
1 parent eb9c540 commit fa55c0a
Show file tree
Hide file tree
Showing 4 changed files with 185 additions and 1 deletion.
12 changes: 11 additions & 1 deletion backend/infrahub/core/diff/merger/serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
Expand Down
56 changes: 56 additions & 0 deletions backend/infrahub/core/diff/query/merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,62 @@ 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
// ------------------------------
WITH n, node_rel_status WHERE node_rel_status = "deleted"
CALL {
WITH n
CALL {
WITH n
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
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 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
}
WITH n
// ------------------------------
// and delete HAS_OWNER and HAS_SOURCE edges to this node if the node is deleted
// ------------------------------
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 {
Expand Down
52 changes: 52 additions & 0 deletions backend/tests/unit/core/diff/test_diff_and_merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -643,3 +645,53 @@ 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)
66 changes: 66 additions & 0 deletions backend/tests/unit/core/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import pytest

from infrahub.core.utils import convert_ip_to_binary_str
from infrahub.database import InfrahubDatabase


@pytest.mark.parametrize(
Expand All @@ -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")

0 comments on commit fa55c0a

Please sign in to comment.