Skip to content

Commit 6958ecb

Browse files
LucasG0LucasG0
authored
Merge pull request #5891 from opsmill/lgu-merge-stable-release-1.2
Merge stable into release-1.2
2 parents 4cf8cfe + 2b76b81 commit 6958ecb

File tree

16 files changed

+752
-190
lines changed

16 files changed

+752
-190
lines changed

‎backend/infrahub/core/graph/__init__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
GRAPH_VERSION = 18
1+
GRAPH_VERSION = 19

‎backend/infrahub/core/migrations/graph/__init__.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
from .m016_diff_delete_bug_fix import Migration016
2121
from .m017_add_core_profile import Migration017
2222
from .m018_uniqueness_nulls import Migration018
23+
from .m019_restore_rels_to_time import Migration019
2324

2425
if TYPE_CHECKING:
2526
from infrahub.core.root import Root
@@ -45,6 +46,7 @@
4546
Migration016,
4647
Migration017,
4748
Migration018,
49+
Migration019,
4850
]
4951

5052

‎backend/infrahub/core/migrations/graph/m019_restore_rels_to_time.py‎

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
from __future__ import annotations
2+
3+
from typing import TYPE_CHECKING, Any, Sequence
4+
5+
from infrahub.core.migrations.shared import GraphMigration, MigrationResult
6+
from infrahub.log import get_logger
7+
8+
from ...constants import GLOBAL_BRANCH_NAME, BranchSupportType
9+
from ...query import Query, QueryType
10+
11+
if TYPE_CHECKING:
12+
from infrahub.database import InfrahubDatabase
13+
14+
log = get_logger()
15+
16+
17+
class FixBranchAwareEdgesQuery(Query):
18+
name = "replace_global_edges"
19+
type = QueryType.WRITE
20+
insert_return = False
21+
22+
async def query_init(self, db: InfrahubDatabase, **kwargs: dict[str, Any]) -> None: # noqa: ARG002
23+
"""
24+
Between a Node and a Relationship, if Relationship.branch_support=aware, replace any global edge
25+
to the branch of a non-global edge leaving out of the Relationship node. Note that there can't
26+
be multiple non-global branches on these edges, as a dedicated Relationship node would exist for that.
27+
"""
28+
29+
query = """
30+
MATCH (node:Node)-[global_edge:IS_RELATED {branch: $global_branch}]-(rel:Relationship)
31+
WHERE rel.branch_support=$branch_aware
32+
MATCH (rel)-[non_global_edge:IS_RELATED]-(node_2: Node)
33+
WHERE non_global_edge.branch <> $global_branch
34+
SET global_edge.branch = non_global_edge.branch
35+
"""
36+
37+
params = {
38+
"global_branch": GLOBAL_BRANCH_NAME,
39+
"branch_aware": BranchSupportType.AWARE.value,
40+
"branch_agnostic": BranchSupportType.AGNOSTIC.value,
41+
}
42+
43+
self.params.update(params)
44+
self.add_to_query(query)
45+
46+
47+
class SetMissingToTimeQuery(Query):
48+
name = "set_missing_to_time"
49+
type = QueryType.WRITE
50+
insert_return = False
51+
52+
async def query_init(self, db: InfrahubDatabase, **kwargs: dict[str, Any]) -> None: # noqa: ARG002
53+
"""
54+
If both a deleted edge and an active edge with no time exist between 2 nodes on the same branch,
55+
set `to` time of active edge using `from` time of the deleted one. This would typically happen after having
56+
replaced a deleted edge on global branch by correct branch with above query.
57+
"""
58+
59+
query = """
60+
MATCH (node:Node)-[deleted_edge:IS_RELATED {status: "deleted"}]-(rel:Relationship)
61+
MATCH (rel)-[active_edge:IS_RELATED {status: "active"}]-(node)
62+
WHERE active_edge.to IS NULL AND deleted_edge.branch = active_edge.branch
63+
SET active_edge.to = deleted_edge.from
64+
"""
65+
66+
self.add_to_query(query)
67+
68+
69+
class DeleteNodesRelsQuery(Query):
70+
name = "delete_relationships_of_deleted_nodes"
71+
type = QueryType.WRITE
72+
insert_return = False
73+
74+
async def query_init(self, db: InfrahubDatabase, **kwargs: dict[str, Any]) -> None: # noqa: ARG002
75+
"""
76+
Some nodes may have been incorrectly deleted, typically, while these nodes edges connected to Root
77+
are correctly deleted, edges connected to other `Node` through a `Relationship` node may still be active.
78+
Following query correctly deletes these edges by both setting correct to time and creating corresponding deleted edge.
79+
"""
80+
81+
query = """
82+
MATCH (deleted_node: Node)-[deleted_edge:IS_PART_OF {status: "deleted"}]->(:Root)
83+
MATCH (deleted_node)-[:IS_RELATED]-(rel:Relationship)
84+
85+
// exclude nodes having been deleted through migration. find those with same uuid and exclude the one with earlier
86+
// timestamp on active branch
87+
WHERE NOT EXISTS {
88+
MATCH (deleted_node)-[e1:IS_RELATED]-(rel)-[e2:IS_RELATED]-(other_node)
89+
WITH deleted_node, other_node, MIN(e1.from) AS min_e1_from, MIN(e2.from) AS min_e2_from
90+
WHERE deleted_node <> other_node AND deleted_node.uuid = other_node.uuid AND min_e1_from < min_e2_from
91+
}
92+
93+
// Note that if an AWARE node has been deleted on a branch and relationship is AGNOSTIC, we do not "delete" this relationship
94+
// right now as this aware node might exist on another branch.
95+
96+
// Set to time if there is an active edge:
97+
// - on deleted edge branch
98+
// - or on any branch and deleted node is agnostic
99+
// - or deleted node is aware and rel is agnostic
100+
CALL {
101+
WITH rel, deleted_edge
102+
OPTIONAL MATCH (rel)-[peer_active_edge {status: "active"}]-(peer_1)
103+
WHERE (peer_active_edge.branch = deleted_edge.branch OR (rel.branch_support <> $branch_agnostic AND deleted_edge.branch = $global_branch))
104+
AND peer_active_edge.to IS NULL
105+
SET peer_active_edge.to = deleted_edge.from
106+
}
107+
108+
// Get distinct rel nodes linked to a deleted node, with the time at which we should delete rel edges.
109+
// Take the MAX time so if it does not take the deleted time of a node deleted through a duplication migration.
110+
WITH DISTINCT rel,
111+
deleted_edge.branch AS deleted_edge_branch,
112+
deleted_edge.branch_level AS branch_level,
113+
MAX(deleted_edge.from) as deleted_time,
114+
deleted_node.branch_support as deleted_node_branch_support
115+
116+
117+
// No need to check deleted edge branch because
118+
// If deleted_node has different branch support type (agnostic/aware) than rel type,
119+
// there might already be a deleted edge that we would not match if we filter on deleted_edge_branch.
120+
// If both are aware, it still works, as we would have one Relationship node for each branch on which this relationship exists.
121+
MATCH (rel)-[]-(peer_2)
122+
WHERE NOT exists((rel)-[{status: "deleted"}]-(peer_2))
123+
124+
125+
// If res is agnostic and delete node is agnostic, we should delete on global branch
126+
// If rel is aware and deleted node is aware, we should use deleted edge branch
127+
// If rel is aware and delete node is agnostic, we need to create deleted edges for every distinct branch on which this relationship exists.
128+
WITH DISTINCT
129+
CASE
130+
// Branch on which `deleted` edge should be created depends on rel.branch_support.
131+
WHEN rel.branch_support = $branch_agnostic
132+
THEN CASE
133+
WHEN deleted_node_branch_support = $branch_agnostic THEN [$global_branch]
134+
ELSE []
135+
END
136+
ELSE
137+
CASE
138+
WHEN deleted_node_branch_support = $branch_agnostic
139+
THEN COLLECT {
140+
WITH rel
141+
MATCH (rel)-[active_edge {status: "active"}]-(peer_2)
142+
RETURN DISTINCT active_edge.branch
143+
}
144+
ELSE
145+
CASE
146+
// if no active edge on this branch exists it means this relationship node is dedicated for another branch
147+
WHEN exists((rel)-[{status: "active", branch: deleted_edge_branch}]-(peer_2)) THEN [deleted_edge_branch]
148+
ELSE []
149+
END
150+
END
151+
END AS branches,
152+
branch_level,
153+
deleted_time,
154+
peer_2,
155+
rel
156+
157+
UNWIND branches as branch
158+
159+
// Then creates `deleted` edge.
160+
// Below CALL subqueries are called once for each rel-peer_2 pair for which we want to create a deleted edge.
161+
// Note that with current infrahub relationships edges design, only one of this CALL should be matched per pair.
162+
163+
CALL {
164+
WITH rel, peer_2, branch, branch_level, deleted_time
165+
MATCH (rel)-[:IS_RELATED]->(peer_2)
166+
MERGE (rel)-[:IS_RELATED {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]->(peer_2)
167+
}
168+
169+
CALL {
170+
WITH rel, peer_2, branch, branch_level, deleted_time
171+
MATCH (rel)-[:IS_PROTECTED]->(peer_2)
172+
MERGE (rel)-[:IS_PROTECTED {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]->(peer_2)
173+
}
174+
175+
CALL {
176+
WITH rel, peer_2, branch, branch_level, deleted_time
177+
MATCH (rel)-[:IS_VISIBLE]->(peer_2)
178+
MERGE (rel)-[:IS_VISIBLE {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]->(peer_2)
179+
}
180+
181+
CALL {
182+
WITH rel, peer_2, branch, branch_level, deleted_time
183+
MATCH (rel)-[:HAS_OWNER]->(peer_2)
184+
MERGE (rel)-[:HAS_OWNER {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]->(peer_2)
185+
}
186+
187+
CALL {
188+
WITH rel, peer_2, branch, branch_level, deleted_time
189+
MATCH (rel)-[:HAS_SOURCE]->(peer_2)
190+
MERGE (rel)-[:HAS_SOURCE {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]->(peer_2)
191+
}
192+
193+
CALL {
194+
WITH rel, peer_2, branch, branch_level, deleted_time
195+
MATCH (rel)<-[:IS_RELATED]-(peer_2)
196+
MERGE (rel)<-[:IS_RELATED {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]-(peer_2)
197+
}
198+
199+
CALL {
200+
WITH rel, peer_2, branch, branch_level, deleted_time
201+
MATCH (rel)<-[:IS_PROTECTED]-(peer_2)
202+
MERGE (rel)<-[:IS_PROTECTED {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]-(peer_2)
203+
}
204+
205+
CALL {
206+
WITH rel, peer_2, branch, branch_level, deleted_time
207+
MATCH (rel)<-[:IS_VISIBLE]-(peer_2)
208+
MERGE (rel)<-[:IS_VISIBLE {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]-(peer_2)
209+
}
210+
211+
CALL {
212+
WITH rel, peer_2, branch, branch_level, deleted_time
213+
MATCH (rel)<-[:HAS_OWNER]-(peer_2)
214+
MERGE (rel)<-[:HAS_OWNER {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]-(peer_2)
215+
}
216+
217+
CALL {
218+
WITH rel, peer_2, branch, branch_level, deleted_time
219+
MATCH (rel)<-[:HAS_SOURCE]-(peer_2)
220+
MERGE (rel)<-[:HAS_SOURCE {status: "deleted", branch: branch, branch_level: branch_level, from: deleted_time}]-(peer_2)
221+
}
222+
"""
223+
224+
params = {
225+
"global_branch": GLOBAL_BRANCH_NAME,
226+
"branch_aware": BranchSupportType.AWARE.value,
227+
"branch_agnostic": BranchSupportType.AGNOSTIC.value,
228+
}
229+
230+
self.params.update(params)
231+
self.add_to_query(query)
232+
233+
234+
class Migration019(GraphMigration):
235+
"""
236+
Fix corrupted state introduced by Migration012 when duplicating a CoreAccount (branch Aware)
237+
being part of a CoreStandardGroup (branch Agnostic). Database is corrupted at multiple points:
238+
- Old CoreAccount node <> group_member node `active` edge has no `to` time (possibly because of #5590).
239+
- Old CoreAccount node <> group_member node `deleted` edge is on `$global_branch` branch instead of `main`.
240+
- New CoreAccount node <> group_member node `active` edge is on `$global_branch` branch instead of `main`.
241+
242+
Also, users having deleted corresponding CoreStandardGroup will also have the following data corruption,
243+
as deletion did not happen correctly due to above issues:
244+
- Both CoreAccount <> group_member and CoreStandardGroup <> group_member edges
245+
have not been deleted (ie status is `active` without `to` time and no additional `deleted` edge).
246+
247+
This migration fixes all above issues to have consistent edges, and fixes IFC-1204.
248+
"""
249+
250+
name: str = "019_fix_edges_state"
251+
minimum_version: int = 18
252+
queries: Sequence[type[Query]] = [FixBranchAwareEdgesQuery, SetMissingToTimeQuery, DeleteNodesRelsQuery]
253+
254+
async def validate_migration(self, db: InfrahubDatabase) -> MigrationResult: # noqa: ARG002
255+
result = MigrationResult()
256+
return result

‎backend/infrahub/core/relationship/model.py‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,8 @@ def get_branch_based_on_support_type(self) -> Branch:
892892
"""If the attribute is branch aware, return the Branch object associated with this attribute
893893
If the attribute is branch agnostic return the Global Branch
894894
895+
Note that if this relationship is Aware and source node is Agnostic, it will return -global- branch.
896+
895897
Returns:
896898
Branch:
897899
"""
@@ -967,7 +969,7 @@ async def _fetch_relationships(
967969
self.has_fetched_relationships = True
968970

969971
for peer_id in details.peer_ids_present_local_only:
970-
await self.remove(peer_id=peer_id, db=db)
972+
await self.remove_locally(peer_id=peer_id, db=db)
971973

972974
async def get(self, db: InfrahubDatabase) -> Relationship | list[Relationship] | None:
973975
rels = await self.get_relationships(db=db)
@@ -1098,22 +1100,17 @@ async def resolve(self, db: InfrahubDatabase) -> None:
10981100
for rel in self._relationships:
10991101
await rel.resolve(db=db)
11001102

1101-
async def remove(
1103+
async def remove_locally(
11021104
self,
11031105
peer_id: Union[str, UUID],
11041106
db: InfrahubDatabase,
1105-
update_db: bool = False,
11061107
) -> bool:
1107-
"""Remove a peer id from the local relationships list,
1108-
need to investigate if and when we should update the relationship in the database."""
1108+
"""Remove a peer id from the local relationships list"""
11091109

11101110
for idx, rel in enumerate(await self.get_relationships(db=db)):
11111111
if str(rel.peer_id) != str(peer_id):
11121112
continue
11131113

1114-
if update_db:
1115-
await rel.delete(db=db)
1116-
11171114
self._relationships.pop(idx)
11181115
return True
11191116

@@ -1130,14 +1127,13 @@ async def remove_in_db(
11301127

11311128
# - Update the existing relationship if we are on the same branch
11321129
rel_ids_per_branch = peer_data.rel_ids_per_branch()
1130+
1131+
# In which cases do we end up here and do not want to set `to` time?
11331132
if branch.name in rel_ids_per_branch:
11341133
await update_relationships_to([str(ri) for ri in rel_ids_per_branch[branch.name]], to=remove_at, db=db)
11351134

11361135
# - Create a new rel of type DELETED if the existing relationship is on a different branch
1137-
rel_branches: set[str] = set()
1138-
if peer_data.rels:
1139-
rel_branches = {r.branch for r in peer_data.rels}
1140-
if rel_branches == {peer_data.branch}:
1136+
if peer_data.rels and {r.branch for r in peer_data.rels} == {peer_data.branch}:
11411137
return
11421138

11431139
query = await RelationshipDataDeleteQuery.init(

‎backend/infrahub/core/schema/definitions/core/group.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
"optional": True,
3636
"identifier": "group_member",
3737
"cardinality": "many",
38+
"branch": BranchSupportType.AWARE,
3839
},
3940
{
4041
"name": "subscribers",

‎backend/tests/conftest.py‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -979,14 +979,24 @@ def car_person_branch_agnostic_schema() -> dict[str, Any]:
979979
],
980980
"relationships": [
981981
{
982-
"name": "owner",
982+
"name": "agnostic_owner",
983983
"label": "Commander of Car",
984984
"peer": "TestPerson",
985985
"optional": False,
986-
"kind": "Parent",
987986
"cardinality": "one",
988987
"direction": "outbound",
989988
"branch": BranchSupportType.AGNOSTIC.value,
989+
"identifier": "agnostic_owner",
990+
},
991+
{
992+
"name": "aware_owner",
993+
"label": "Commander of Car",
994+
"peer": "TestPerson",
995+
"optional": True,
996+
"cardinality": "one",
997+
"direction": "outbound",
998+
"branch": BranchSupportType.AWARE.value,
999+
"identifier": "aware_owner",
9901000
},
9911001
],
9921002
},

0 commit comments

Comments
 (0)