diff --git a/backend/infrahub/core/graph/__init__.py b/backend/infrahub/core/graph/__init__.py index d549910d22..5a991cd92e 100644 --- a/backend/infrahub/core/graph/__init__.py +++ b/backend/infrahub/core/graph/__init__.py @@ -1 +1 @@ -GRAPH_VERSION = 18 +GRAPH_VERSION = 19 diff --git a/backend/infrahub/core/migrations/graph/__init__.py b/backend/infrahub/core/migrations/graph/__init__.py index f8b314ab25..99fc4a3b5e 100644 --- a/backend/infrahub/core/migrations/graph/__init__.py +++ b/backend/infrahub/core/migrations/graph/__init__.py @@ -20,6 +20,7 @@ from .m016_diff_delete_bug_fix import Migration016 from .m017_add_core_profile import Migration017 from .m018_uniqueness_nulls import Migration018 +from .m019_restore_rels_to_time import Migration019 if TYPE_CHECKING: from infrahub.core.root import Root @@ -45,6 +46,7 @@ Migration016, Migration017, Migration018, + Migration019, ] diff --git a/backend/infrahub/core/migrations/graph/m019_restore_rels_to_time.py b/backend/infrahub/core/migrations/graph/m019_restore_rels_to_time.py new file mode 100644 index 0000000000..cca0e5d1f1 --- /dev/null +++ b/backend/infrahub/core/migrations/graph/m019_restore_rels_to_time.py @@ -0,0 +1,256 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING, Any, Sequence + +from infrahub.core.migrations.shared import GraphMigration, MigrationResult +from infrahub.log import get_logger + +from ...constants import GLOBAL_BRANCH_NAME, BranchSupportType +from ...query import Query, QueryType + +if TYPE_CHECKING: + from infrahub.database import InfrahubDatabase + +log = get_logger() + + +class FixBranchAwareEdgesQuery(Query): + name = "replace_global_edges" + type = QueryType.WRITE + insert_return = False + + async def query_init(self, db: InfrahubDatabase, **kwargs: dict[str, Any]) -> None: + """ + Between a Node and a Relationship, if Relationship.branch_support=aware, replace any global edge + to the branch of a non-global edge leaving out of the Relationship node. Note that there can't + be multiple non-global branches on these edges, as a dedicated Relationship node would exist for that. + """ + + query = """ + MATCH (node:Node)-[global_edge:IS_RELATED {branch: $global_branch}]-(rel:Relationship) + WHERE rel.branch_support=$branch_aware + MATCH (rel)-[non_global_edge:IS_RELATED]-(node_2: Node) + WHERE non_global_edge.branch <> $global_branch + SET global_edge.branch = non_global_edge.branch + """ + + params = { + "global_branch": GLOBAL_BRANCH_NAME, + "branch_aware": BranchSupportType.AWARE.value, + "branch_agnostic": BranchSupportType.AGNOSTIC.value, + } + + self.params.update(params) + self.add_to_query(query) + + +class SetMissingToTimeQuery(Query): + name = "set_missing_to_time" + type = QueryType.WRITE + insert_return = False + + async def query_init(self, db: InfrahubDatabase, **kwargs: dict[str, Any]) -> None: + """ + If both a deleted edge and an active edge with no time exist between 2 nodes on the same branch, + set `to` time of active edge using `from` time of the deleted one. This would typically happen after having + replaced a deleted edge on global branch by correct branch with above query. + """ + + query = """ + MATCH (node:Node)-[deleted_edge:IS_RELATED {status: "deleted"}]-(rel:Relationship) + MATCH (rel)-[active_edge:IS_RELATED {status: "active"}]-(node) + WHERE active_edge.to IS NULL AND deleted_edge.branch = active_edge.branch + SET active_edge.to = deleted_edge.from + """ + + self.add_to_query(query) + + +class DeleteNodesRelsQuery(Query): + name = "delete_relationships_of_deleted_nodes" + type = QueryType.WRITE + insert_return = False + + async def query_init(self, db: InfrahubDatabase, **kwargs: dict[str, Any]) -> None: + """ + Some nodes may have been incorrectly deleted, typically, while these nodes edges connected to Root + are correctly deleted, edges connected to other `Node` through a `Relationship` node may still be active. + Following query correctly deletes these edges by both setting correct to time and creating corresponding deleted edge. + """ + + query = """ + MATCH (deleted_node: Node)-[deleted_edge:IS_PART_OF {status: "deleted"}]->(:Root) + MATCH (deleted_node)-[:IS_RELATED]-(rel:Relationship) + + // exclude nodes having been deleted through migration. find those with same uuid and exclude the one with earlier + // timestamp on active branch + WHERE NOT EXISTS { + MATCH (deleted_node)-[e1:IS_RELATED]-(rel)-[e2:IS_RELATED]-(other_node) + WITH deleted_node, other_node, MIN(e1.from) AS min_e1_from, MIN(e2.from) AS min_e2_from + WHERE deleted_node <> other_node AND deleted_node.uuid = other_node.uuid AND min_e1_from < min_e2_from + } + + // 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 + // - 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) + WHERE (peer_active_edge.branch = deleted_edge.branch OR (rel.branch_support <> $branch_agnostic AND deleted_edge.branch = $global_branch)) + AND peer_active_edge.to IS NULL + SET peer_active_edge.to = deleted_edge.from + } + + // Get distinct rel nodes linked to a deleted node, with the time at which we should delete rel edges. + // Take the MAX time so if it does not take the deleted time of a node deleted through a duplication migration. + WITH DISTINCT rel, + deleted_edge.branch AS deleted_edge_branch, + deleted_edge.branch_level AS branch_level, + MAX(deleted_edge.from) as deleted_time, + deleted_node.branch_support as deleted_node_branch_support + + + // 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. + MATCH (rel)-[]-(peer_2) + WHERE NOT exists((rel)-[{status: "deleted"}]-(peer_2)) + + + // If res is agnostic and delete node is agnostic, we should delete on global branch + // If rel is aware and deleted node is aware, we should use deleted edge branch + // If rel is aware and delete node is agnostic, we need to create deleted edges for every distinct branch on which this relationship exists. + WITH DISTINCT + CASE + // Branch on which `deleted` edge should be created depends on rel.branch_support. + WHEN rel.branch_support = $branch_agnostic + THEN CASE + WHEN deleted_node_branch_support = $branch_agnostic THEN [$global_branch] + ELSE [] + END + ELSE + CASE + WHEN deleted_node_branch_support = $branch_agnostic + THEN COLLECT { + WITH rel + MATCH (rel)-[active_edge {status: "active"}]-(peer_2) + RETURN DISTINCT active_edge.branch + } + ELSE + CASE + // if no active edge on this branch exists it means this relationship node is dedicated for another branch + WHEN exists((rel)-[{status: "active", branch: deleted_edge_branch}]-(peer_2)) THEN [deleted_edge_branch] + ELSE [] + END + END + END AS branches, + branch_level, + deleted_time, + peer_2, + rel + + UNWIND branches as branch + + // Then creates `deleted` edge. + // Below CALL subqueries are called once for each rel-peer_2 pair for which we want to create a deleted edge. + // Note that with current infrahub relationships edges design, only one of this CALL should be matched per pair. + + CALL { + WITH rel, peer_2, branch, branch_level, deleted_time + MATCH (rel)-[:IS_RELATED]->(peer_2) + MERGE (rel)-[:IS_RELATED {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]->(peer_2) + } + + CALL { + WITH rel, peer_2, branch, branch_level, deleted_time + MATCH (rel)-[:IS_PROTECTED]->(peer_2) + MERGE (rel)-[:IS_PROTECTED {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]->(peer_2) + } + + CALL { + WITH rel, peer_2, branch, branch_level, deleted_time + MATCH (rel)-[:IS_VISIBLE]->(peer_2) + MERGE (rel)-[:IS_VISIBLE {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]->(peer_2) + } + + CALL { + WITH rel, peer_2, branch, branch_level, deleted_time + MATCH (rel)-[:HAS_OWNER]->(peer_2) + MERGE (rel)-[:HAS_OWNER {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]->(peer_2) + } + + CALL { + WITH rel, peer_2, branch, branch_level, deleted_time + MATCH (rel)-[:HAS_SOURCE]->(peer_2) + MERGE (rel)-[:HAS_SOURCE {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]->(peer_2) + } + + CALL { + WITH rel, peer_2, branch, branch_level, deleted_time + MATCH (rel)<-[:IS_RELATED]-(peer_2) + MERGE (rel)<-[:IS_RELATED {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]-(peer_2) + } + + CALL { + WITH rel, peer_2, branch, branch_level, deleted_time + MATCH (rel)<-[:IS_PROTECTED]-(peer_2) + MERGE (rel)<-[:IS_PROTECTED {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]-(peer_2) + } + + CALL { + WITH rel, peer_2, branch, branch_level, deleted_time + MATCH (rel)<-[:IS_VISIBLE]-(peer_2) + MERGE (rel)<-[:IS_VISIBLE {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]-(peer_2) + } + + CALL { + WITH rel, peer_2, branch, branch_level, deleted_time + MATCH (rel)<-[:HAS_OWNER]-(peer_2) + MERGE (rel)<-[:HAS_OWNER {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]-(peer_2) + } + + CALL { + WITH rel, peer_2, branch, branch_level, deleted_time + MATCH (rel)<-[:HAS_SOURCE]-(peer_2) + MERGE (rel)<-[:HAS_SOURCE {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]-(peer_2) + } + """ + + params = { + "global_branch": GLOBAL_BRANCH_NAME, + "branch_aware": BranchSupportType.AWARE.value, + "branch_agnostic": BranchSupportType.AGNOSTIC.value, + } + + self.params.update(params) + self.add_to_query(query) + + +class Migration019(GraphMigration): + """ + Fix corrupted state introduced by Migration012 when duplicating a CoreAccount (branch Aware) + being part of a CoreStandardGroup (branch Agnostic). Database is corrupted at multiple points: + - Old CoreAccount node <> group_member node `active` edge has no `to` time (possibly because of #5590). + - Old CoreAccount node <> group_member node `deleted` edge is on `$global_branch` branch instead of `main`. + - New CoreAccount node <> group_member node `active` edge is on `$global_branch` branch instead of `main`. + + Also, users having deleted corresponding CoreStandardGroup will also have the following data corruption, + as deletion did not happen correctly due to above issues: + - Both CoreAccount <> group_member and CoreStandardGroup <> group_member edges + have not been deleted (ie status is `active` without `to` time and no additional `deleted` edge). + + This migration fixes all above issues to have consistent edges, and fixes IFC-1204. + """ + + name: str = "019_fix_edges_state" + minimum_version: int = 18 + queries: Sequence[type[Query]] = [FixBranchAwareEdgesQuery, SetMissingToTimeQuery, DeleteNodesRelsQuery] + + async def validate_migration(self, db: InfrahubDatabase) -> MigrationResult: + result = MigrationResult() + return result diff --git a/backend/infrahub/core/relationship/model.py b/backend/infrahub/core/relationship/model.py index ff9fe9b17f..82a928cfda 100644 --- a/backend/infrahub/core/relationship/model.py +++ b/backend/infrahub/core/relationship/model.py @@ -884,6 +884,8 @@ def get_branch_based_on_support_type(self) -> Branch: """If the attribute is branch aware, return the Branch object associated with this attribute If the attribute is branch agnostic return the Global Branch + Note that if this relationship is Aware and source node is Agnostic, it will return -global- branch. + Returns: Branch: """ @@ -959,7 +961,7 @@ async def _fetch_relationships( self.has_fetched_relationships = True for peer_id in details.peer_ids_present_local_only: - await self.remove(peer_id=peer_id, db=db) + await self.remove_locally(peer_id=peer_id, db=db) async def get(self, db: InfrahubDatabase) -> Relationship | list[Relationship] | None: rels = await self.get_relationships(db=db) @@ -1077,22 +1079,17 @@ async def resolve(self, db: InfrahubDatabase) -> None: for rel in self._relationships: await rel.resolve(db=db) - async def remove( + async def remove_locally( self, peer_id: Union[str, UUID], db: InfrahubDatabase, - update_db: bool = False, ) -> bool: - """Remove a peer id from the local relationships list, - need to investigate if and when we should update the relationship in the database.""" + """Remove a peer id from the local relationships list""" for idx, rel in enumerate(await self.get_relationships(db=db)): if str(rel.peer_id) != str(peer_id): continue - if update_db: - await rel.delete(db=db) - self._relationships.pop(idx) return True @@ -1109,14 +1106,13 @@ async def remove_in_db( # - Update the existing relationship if we are on the same branch rel_ids_per_branch = peer_data.rel_ids_per_branch() + + # In which cases do we end up here and do not want to set `to` time? if branch.name in rel_ids_per_branch: await update_relationships_to([str(ri) for ri in rel_ids_per_branch[branch.name]], to=remove_at, db=db) # - Create a new rel of type DELETED if the existing relationship is on a different branch - rel_branches: set[str] = set() - if peer_data.rels: - rel_branches = {r.branch for r in peer_data.rels} - if rel_branches == {peer_data.branch}: + if peer_data.rels and {r.branch for r in peer_data.rels} == {peer_data.branch}: return query = await RelationshipDataDeleteQuery.init( diff --git a/backend/infrahub/core/schema/definitions/core.py b/backend/infrahub/core/schema/definitions/core.py index 1acbe0e36f..9e855f015b 100644 --- a/backend/infrahub/core/schema/definitions/core.py +++ b/backend/infrahub/core/schema/definitions/core.py @@ -223,6 +223,7 @@ "optional": True, "identifier": "group_member", "cardinality": "many", + "branch": BranchSupportType.AWARE, }, { "name": "subscribers", diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index e46e9eecbf..ef72412f25 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -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", }, ], }, diff --git a/backend/tests/functional/branch/test_delete_agnostic_rel.py b/backend/tests/functional/branch/test_delete_agnostic_rel.py index 6e2625889b..dade8f7bdb 100644 --- a/backend/tests/functional/branch/test_delete_agnostic_rel.py +++ b/backend/tests/functional/branch/test_delete_agnostic_rel.py @@ -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 @@ -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 @@ -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 ) diff --git a/backend/tests/helpers/db_validation.py b/backend/tests/helpers/db_validation.py index aa11638b4e..c9a62398c5 100644 --- a/backend/tests/helpers/db_validation.py +++ b/backend/tests/helpers/db_validation.py @@ -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 @@ -17,16 +31,20 @@ 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 @@ -34,7 +52,8 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: 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 @@ -47,7 +66,8 @@ 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 @@ -55,14 +75,16 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: // 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 """ @@ -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) diff --git a/backend/tests/unit/core/migrations/graph/test_019.py b/backend/tests/unit/core/migrations/graph/test_019.py new file mode 100644 index 0000000000..813f7fa8d2 --- /dev/null +++ b/backend/tests/unit/core/migrations/graph/test_019.py @@ -0,0 +1,245 @@ +from infrahub.core import registry +from infrahub.core.constants import GLOBAL_BRANCH_NAME +from infrahub.core.migrations.graph import Migration019 +from infrahub.core.node import Node +from infrahub.core.schema import SchemaRoot, core_models +from infrahub.core.timestamp import current_timestamp +from infrahub.database import InfrahubDatabase +from tests.helpers.db_validation import validate_node_relationships + + +async def test_migration_019( + db: InfrahubDatabase, + default_branch, +): + """ + Reproduce corrupted state introduced by migration 12, and apply the migration fixing it. + """ + + schema = SchemaRoot(**core_models) + registry.schema.register_schema(schema=schema, branch=default_branch.name) + + test_group = await Node.init(db=db, schema="CoreStandardGroup") + await test_group.new(db=db, name="test_group") + await test_group.save(db=db) + + core_acc = await Node.init(db=db, schema="CoreAccount") + await core_acc.new(db=db, name="core_acc", account_type="User", password="def", member_of_groups=[test_group]) + await core_acc.save(db=db) + + # Delete CoreStandardGroup. This should also (correctly) update rels to CoreGenericAccount and CoreAccount + # but we will override them afterward to reproduce corrupted state. + await test_group.delete(db=db) + + # Make relationship between CoreAccount <> group_member <> CoreStandardGroup active while it should have been deleted. + # and make the group_member <> CoreAccount edge part on global branch + + query = """ + MATCH (new_core_acc: CoreAccount)-[:HAS_ATTRIBUTE]->(:Attribute {name: "name"})-[:HAS_VALUE]->(:AttributeValue {value: "core_acc"}) + MATCH (new_core_acc)-[r1:IS_RELATED]-(group_member: Relationship)-[r2:IS_RELATED]-(test_group: CoreStandardGroup) + MATCH (new_core_acc)-[active_r1]-(group_member) + WHERE active_r1.status = 'active' + MATCH (new_core_acc)-[deleted_r1]-(group_member) + WHERE deleted_r1.status = 'deleted' + MATCH (test_group)-[active_r2]-(group_member) + WHERE active_r2.status = 'active' + MATCH (test_group)-[deleted_r2]-(group_member) + WHERE deleted_r2.status = 'deleted' + + DELETE deleted_r1 + REMOVE active_r1.to + SET active_r1.branch = '-global-' + + DELETE deleted_r2 + REMOVE active_r2.to + + return new_core_acc, group_member, test_group + """ + + await db.execute_query(query=query, name="query_1") + + # Create the old CoreAccount object - not inheriting from GenericAccount - + # sharing same attributes / relationships than above CoreAccount + + query_2 = """ + // Match the existing CoreAccount node with the specified attributes + MATCH (new_core_acc:CoreAccount)-[:HAS_ATTRIBUTE]->(:Attribute {name: "name"})-[:HAS_VALUE]->(:AttributeValue {value: "core_acc"}) + + // Create the new CoreAccount node with the same uuid and additional properties + CREATE (new_node:CoreAccount:LineageOwner:LineageSource:Node {uuid: new_core_acc.uuid, + branch_support: new_core_acc.branch_support, namespace: new_core_acc.namespace, kind: "CoreAccount"}) + + WITH new_node, new_core_acc + + // Match the relationships of the existing CoreAccount node + MATCH (new_core_acc)-[r:IS_RELATED]->(group_member:Relationship {name: "group_member"}) + + // Create active branch with no to time on main branch + CREATE (new_node)-[:IS_RELATED {branch: "main", from: "2024-02-05T15:37:07.228145Z", status: "active"}]->(group_member) + + // Create deleted branch with no to time on global branch + CREATE (new_node)-[:IS_RELATED {branch: $global_branch, from: r.from, status: "deleted"}]->(group_member) + + // Return the new_node + RETURN new_node; + """ + + await db.execute_query(query=query_2, name="query_2", params={"global_branch": GLOBAL_BRANCH_NAME}) + + # Make sure migration executes without error, and that we can query accounts afterwards. + # Note generated corrupted state does not trigger IFC-1204 bug, + # but a manual test confirmed migration solves this issue. + + migration = Migration019() + await migration.execute(db=db) + await migration.validate_migration(db=db) + + # Verify edges are correct, ie: + # - 2 edges on main branch between old CoreAccount and group_member, 1 active with from/to set, 1 deleted + # - 2 edges on main branch between new CoreAccount and group_member, 1 active with from/to set, 1 deleted + # - 2 edges on main branch between CoreStandardGroup and group_member, 1 active with from/to set, 1 deleted + + check_deleted_edges_query = """ + MATCH (n {uuid: $uuid})-[r1:IS_RELATED]-(rel:Relationship {name: 'group_member'}) + WHERE r1.branch = 'main' + WITH n, rel, COLLECT(r1) AS edges + WITH + edges, + SIZE(edges) AS edgeCount, + [e IN edges WHERE e.status = 'active' AND e.from IS NOT NULL AND e.to IS NOT NULL] AS activeEdges, + [e IN edges WHERE e.status = 'deleted'] AS deletedEdges + WHERE edgeCount = 2 AND SIZE(activeEdges) = 1 AND SIZE(deletedEdges) = 1 AND activeEdges[0].to = deletedEdges[0].from + RETURN 'Old CoreAccount edges are correct' AS result + """ + + core_accounts_results = await db.execute_query( + query=check_deleted_edges_query, name="check_deleted_edges_query", params={"uuid": core_acc.id} + ) + assert len(core_accounts_results) == 2 + + group_results = await db.execute_query( + query=check_deleted_edges_query, name="check_deleted_edges_query", params={"uuid": test_group.id} + ) + assert len(group_results) == 1 + + # Additional sanity checks + await validate_node_relationships(node=test_group, branch=default_branch, db=db) + await validate_node_relationships(node=test_group, branch=registry.get_global_branch(), db=db) + + +async def test_incorrectly_deleted_aware_nodes_and_relationship( + db: InfrahubDatabase, branch, car_person_schema_unregistered +): + """ + Reproduce a state where a branch aware node would have been incorrectly deleted, this node being + connected to another node through a branch aware relationship. + """ + + registry.schema.register_schema(schema=car_person_schema_unregistered, branch=branch.name) + + john = await Node.init(schema="TestPerson", db=db, branch=branch) + await john.new(db=db, name="John") + await john.save(db=db) + + car = await Node.init(schema="TestCar", db=db, branch=branch) + await car.new(db=db, name="test-car", owner=john) + await car.save(db=db) + + # Reproduce corrupted state by only deleting is_part_of edge + + delete_only_is_part_of_query = """ + MATCH (car:TestCar {uuid: $uuid})-[r:IS_PART_OF {status: "active"}]-(root:Root) + SET r.to = $at + CREATE (car)-[:IS_PART_OF {status: "deleted", from: $at, branch: $branch, branch_level: 2}]->(root) + + """ + + await db.execute_query( + query=delete_only_is_part_of_query, + name="delete_only_is_part_of_query", + params={"uuid": car.id, "at": current_timestamp(), "branch": branch.name}, + ) + + migration = Migration019() + await migration.execute(db=db) + await migration.validate_migration(db=db) + + await validate_node_relationships(node=car, branch=branch, db=db) + + +async def test_incorrectly_deleted_agnostic_node(db: InfrahubDatabase, branch, car_person_branch_agnostic_schema): + """ + Reproduce a state where a branch agnostic node would have been incorrectly deleted, this node being + connected to another node through 2 relationships, both aware and agnostic. + """ + + # await load_schema(db, schema=CAR_SCHEMA) + registry.schema.register_schema(schema=SchemaRoot(**car_person_branch_agnostic_schema), branch=branch.name) + + aware_person = await Node.init(schema="TestPerson", db=db, branch=branch) + await aware_person.new(db=db, name="John") + await aware_person.save(db=db) + + agnostic_car = await Node.init(schema="TestCar", db=db, branch=branch) + await agnostic_car.new(db=db, name="test-car", agnostic_owner=aware_person, aware_owner=aware_person) + await agnostic_car.save(db=db) + + # Reproduce corrupted state by only deleting is_part_of edge + + delete_only_is_part_of_query = """ + MATCH (car:TestCar {uuid: $uuid})-[r:IS_PART_OF {status: "active"}]-(root:Root) + SET r.to = $at + CREATE (car)-[:IS_PART_OF {status: "deleted", from: $at, branch: $global_branch, branch_level: 1}]->(root) + """ + + await db.execute_query( + query=delete_only_is_part_of_query, + name="delete_only_is_part_of_query", + params={"uuid": agnostic_car.id, "at": current_timestamp(), "global_branch": GLOBAL_BRANCH_NAME}, + ) + + migration = Migration019() + await migration.execute(db=db) + await migration.validate_migration(db=db) + + await validate_node_relationships(node=agnostic_car, branch=branch, db=db) + await validate_node_relationships(node=agnostic_car, branch=registry.get_global_branch(), db=db) + + +async def test_incorrectly_deleted_aware_node(db: InfrahubDatabase, branch, car_person_branch_agnostic_schema): + """ + Reproduce a state where a branch agnostic node would have been incorrectly deleted, this node being + connected to another node through 2 relationships, both aware and agnostic. + Note that, after deleting an aware node, agnostic edges of this node will not be deleted. + """ + + registry.schema.register_schema(schema=SchemaRoot(**car_person_branch_agnostic_schema), branch=branch.name) + + aware_person = await Node.init(schema="TestPerson", db=db, branch=branch) + await aware_person.new(db=db, name="John") + await aware_person.save(db=db) + + agnostic_car = await Node.init(schema="TestCar", db=db, branch=branch) + await agnostic_car.new(db=db, name="test-car", agnostic_owner=aware_person, aware_owner=aware_person) + await agnostic_car.save(db=db) + + # Reproduce corrupted state by only deleting is_part_of edge + + delete_only_is_part_of_query = """ + MATCH (person:TestPerson {uuid: $uuid})-[r:IS_PART_OF {status: "active"}]-(root:Root) + SET r.to = $at + CREATE (person)-[:IS_PART_OF {status: "deleted", from: $at, branch: $branch, branch_level: 1}]->(root) + """ + + await db.execute_query( + query=delete_only_is_part_of_query, + name="delete_only_is_part_of_query", + params={"uuid": aware_person.id, "at": current_timestamp(), "branch": branch.name}, + ) + + migration = Migration019() + await migration.execute(db=db) + await migration.validate_migration(db=db) + + await validate_node_relationships(node=aware_person, branch=branch, db=db) + await validate_node_relationships(node=aware_person, branch=registry.get_global_branch(), db=db) diff --git a/backend/tests/unit/core/migrations/schema/test_node_kind_update.py b/backend/tests/unit/core/migrations/schema/test_node_kind_update.py index 67e81d6867..218f5d98ca 100644 --- a/backend/tests/unit/core/migrations/schema/test_node_kind_update.py +++ b/backend/tests/unit/core/migrations/schema/test_node_kind_update.py @@ -99,7 +99,7 @@ async def test_migration_agnostic_relationship( await person_john.save(db=db) car = await Node.init(db=db, schema="TestCar") - await car.new(db=db, name="yaris", owner=person_john.id) + await car.new(db=db, name="yaris", agnostic_owner=person_john.id) await car.save(db=db) schema = registry.schema.get_schema_branch(name=default_branch.name) diff --git a/backend/tests/unit/core/migrations/schema/test_node_remove.py b/backend/tests/unit/core/migrations/schema/test_node_remove.py index a63b21c2b0..1af390fa2c 100644 --- a/backend/tests/unit/core/migrations/schema/test_node_remove.py +++ b/backend/tests/unit/core/migrations/schema/test_node_remove.py @@ -115,7 +115,7 @@ async def test_migration_agnostic_relationship( await person_john.save(db=db) car = await Node.init(db=db, schema="TestCar") - await car.new(db=db, name="yaris", owner=person_john.id) + await car.new(db=db, name="yaris", agnostic_owner=person_john.id) await car.save(db=db) schema = registry.schema.get_schema_branch(name=default_branch.name)