Skip to content

Commit

Permalink
Add aware node/relationship test
Browse files Browse the repository at this point in the history
  • Loading branch information
LucasG0 committed Feb 26, 2025
1 parent a8bb971 commit 84aa6e2
Show file tree
Hide file tree
Showing 7 changed files with 298 additions and 153 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,10 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: dict[str, Any]) -> No
// Note that if an AWARE node has been deleted on a branch and relationship is AGNOSTIC, we do not "delete" this relationship
// right now as this aware node might exist on another branch.
// Set to time if there is an active edge on deleted edge branch
// Set to time if there is an active edge:
// - on deleted edge branch
// - or on any branch and deleted node is agnostic
// - or deleted node is aware and rel is agnostic
CALL {
WITH rel, deleted_edge
OPTIONAL MATCH (rel)-[peer_active_edge {status: "active"}]-(peer_1)
Expand All @@ -111,7 +114,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: dict[str, Any]) -> No
deleted_node.branch_support as deleted_node_branch_support
// No need to check deleted edge branch because
// No need to check deleted edge branch because
// If deleted_node has different branch support type (agnostic/aware) than rel type,
// there might already be a deleted edge that we would not match if we filter on deleted_edge_branch.
// If both are aware, it still works, as we would have one Relationship node for each branch on which this relationship exists.
Expand Down
14 changes: 12 additions & 2 deletions backend/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -969,14 +969,24 @@ def car_person_branch_agnostic_schema() -> dict[str, Any]:
],
"relationships": [
{
"name": "owner",
"name": "agnostic_owner",
"label": "Commander of Car",
"peer": "TestPerson",
"optional": False,
"kind": "Parent",
"cardinality": "one",
"direction": "outbound",
"branch": BranchSupportType.AGNOSTIC.value,
"identifier": "agnostic_owner",
},
{
"name": "aware_owner",
"label": "Commander of Car",
"peer": "TestPerson",
"optional": True,
"cardinality": "one",
"direction": "outbound",
"branch": BranchSupportType.AWARE.value,
"identifier": "aware_owner",
},
],
},
Expand Down
10 changes: 5 additions & 5 deletions backend/tests/functional/branch/test_delete_agnostic_rel.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ async def owner_2(self, client: InfrahubClient, load_schema) -> InfrahubNode:

@pytest.fixture(scope="class")
async def car(self, client: InfrahubClient, load_schema, owner_1: InfrahubNode) -> InfrahubNode:
car = await client.create(kind="TestCar", name="car_name", owner=owner_1)
car = await client.create(kind="TestCar", name="car_name", agnostic_owner=owner_1)
await car.save()
return car

@pytest.fixture(scope="class")
async def car_2(self, client: InfrahubClient, load_schema, owner_2: InfrahubNode) -> InfrahubNode:
car = await client.create(kind="TestCar", name="car_name_2", owner=owner_2)
car = await client.create(kind="TestCar", name="car_name_2", agnostic_owner=owner_2)
await car.save()
return car

Expand All @@ -64,11 +64,11 @@ async def test_delete_agnostic_rel(
See https://github.com/opsmill/infrahub/issues/5559.
"""
car = await client.get(kind="TestCar", name__value="car_name", prefetch_relationships=True)
car.owner = owner_2
car.agnostic_owner = owner_2
await car.save()

car = await client.get(kind="TestCar", name__value="car_name", prefetch_relationships=True)
assert car.owner.peer.name.value == "owner_2"
assert car.agnostic_owner.peer.name.value == "owner_2"

async def test_delete_aware_mandatory_node_blocked(
self, client: InfrahubClient, owner_2: InfrahubNode, car: InfrahubNode
Expand All @@ -79,7 +79,7 @@ async def test_delete_aware_mandatory_node_blocked(
await owner_2.delete()

assert (
f"Cannot delete TestPerson '{owner_2.id}'. It is linked to mandatory relationship owner on node TestCar '{car.id}'"
f"Cannot delete TestPerson '{owner_2.id}'. It is linked to mandatory relationship agnostic_owner on node TestCar '{car.id}'"
in exc.value.message
)

Expand Down
57 changes: 35 additions & 22 deletions backend/tests/helpers/db_validation.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,26 @@
from typing import Any

from infrahub.core.branch import Branch
from infrahub.core.constants import GLOBAL_BRANCH_NAME, BranchSupportType
from infrahub.core.node import Node
from infrahub.core.query import Query, QueryType
from infrahub.database import InfrahubDatabase


class ValidateNodeRelationshipQuery(Query):
"""
This query will return error message if for any couple (input_node, relationship):
- If relationship type is agnostic, all edges branches should be -global-
- Else, there should not be any edge on global branch
- Considering edges on the input branch:
- Either 1 active edge without `to`
- Either 1 deleted edge, and potentially 1 active edge having `active.to` = `deleted.from`
NOTE: This query currently validates a subset of all possible valid edge states as edges states are mainly
validated on input branch. Having a validation on any branch would require more logic
(typically, a "groupby edge.branch" like behavior) and is TODO.
"""

name: str = "validate_node_rels"
type: QueryType = QueryType.READ

Expand All @@ -17,24 +31,29 @@ def __init__(self, node_id: str, **kwargs: Any) -> None:
async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None:
self.params["node_id"] = self.node_id
self.params["branch"] = self.branch.name
self.params["global_branch_name"] = GLOBAL_BRANCH_NAME
self.params["branch_agnostic"] = BranchSupportType.AGNOSTIC.value

query = """
// Match the pattern with specific branch conditions
MATCH (node {uuid: $node_id})-[r:IS_RELATED]-(rel:Relationship)
MATCH (input_node {uuid: $node_id})-[r:IS_RELATED]-(rel:Relationship)
WITH DISTINCT rel
MATCH (rel)-[is_related:IS_RELATED]-(node: Node)
// Collect and process edges
WITH
node,
rel,
COLLECT(r) AS edges
COLLECT(is_related) AS edges
// Count and categorize edges
WITH
node,
rel,
edges,
SIZE(edges) as nb_edges,
[e IN edges WHERE e.branch = $branch] AS edges_on_correct_branch,
[e IN edges WHERE e.branch = $branch] AS edges_on_branch,
[e IN edges WHERE e.branch = $global_branch_name] AS edges_on_global_branch,
[e IN edges WHERE e.status = 'active' AND e.to IS NOT NULL] AS active_with_to_edges,
[e IN edges WHERE e.status = 'active' AND e.to IS NULL] AS active_no_to_edges,
[e IN edges WHERE e.status = 'deleted'] AS deleted_edges
Expand All @@ -47,22 +66,25 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None:
active_no_to_edges,
deleted_edges,
nb_edges,
SIZE(edges_on_correct_branch) as nb_edges_on_correct_branch,
SIZE(edges_on_branch) as nb_edges_on_branch,
SIZE(edges_on_global_branch) as nb_edges_on_global_branch,
SIZE(active_with_to_edges) AS nb_active_with_to_edges,
SIZE(active_no_to_edges) AS nb_active_no_to_edges,
SIZE(deleted_edges) AS nb_deleted_edges
// Return the result based on conditions
WITH
CASE
WHEN nb_edges_on_correct_branch <> nb_edges
THEN "nb_edges: " + nb_edges + " VS nb_edges_on_correct_branch: " + nb_edges_on_correct_branch
WHEN nb_edges = 1 AND nb_active_no_to_edges <> 1
THEN "1 edge but nb_active_no_to_edges: " + nb_active_no_to_edges
WHEN nb_edges = 2 AND NOT (nb_active_with_to_edges = 1 AND nb_deleted_edges = 1
AND active_with_to_edges[0].to = deleted_edges[0].from)
THEN "2 edges but they are invalid"
ELSE "Edges state is correct"
WHEN rel.branch_support = $branch_agnostic AND nb_edges_on_global_branch <> nb_edges
THEN "Relationship is agnostic but found: " + (nb_edges - nb_edges_on_global_branch) + " aware edge(s)"
WHEN rel.branch_support <> $branch_agnostic AND nb_edges_on_global_branch > 0
THEN "Relationship is aware but found " + nb_edges_on_global_branch + " agnostic edge(s)"
WHEN nb_edges_on_branch > 2
THEN "More than 2 edges on a given branch between a node and a relationship"
WHEN nb_edges_on_branch = 2 AND NOT (nb_active_with_to_edges = 1 AND nb_deleted_edges = 1
AND active_with_to_edges[0].to = deleted_edges[0].from)
THEN "Found 2 inconsistent edges between a node and a relationship"
ELSE "Edges state is correct" // It currently allows having one edge as we might not always create a `deleted` edge?
END AS res
"""

Expand All @@ -72,16 +94,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None:

async def validate_node_relationships(node: Node, branch: Branch, db: InfrahubDatabase) -> None:
"""
This function will raise an error if following conditions are not met:
- All IS_RELATED edges between this node and Relationship nodes should be on input branch
- Between this node and any Relationship node, is expected:
- Either 1 active edge without `to`
- Or 1 active edge with `to` and 1 deleted edge with `active.to` = `deleted.from`
IMPORTANT NOTE: This function currently validates a subset of all possible valid edge states.
Typically, if between two nodes there are 1 active edge on main and 1 deleted on branch2,
this function will raise an error while this state may happen while migrating an existing object node kind
on branch2, but not on main.
Raises an error if validation conditions of the query are not met.
"""

query = await ValidateNodeRelationshipQuery.init(db=db, branch=branch, node_id=node.id)
Expand Down
Loading

0 comments on commit 84aa6e2

Please sign in to comment.