Skip to content

Commit

Permalink
diff lifecycle tests and bugs (#4168)
Browse files Browse the repository at this point in the history
* test and fix for bug when removed on main and updated on branch

* diff merge integration test

* more tests

* confirm nodes added on branch are added

* update path identifiers to match values expected by merge logic

* more tests and bug fixes

* fix query bug

* formatting

* add delete to integration test

* final integration tests and bug fixes
  • Loading branch information
ajtmccarty authored Aug 27, 2024
1 parent 2ad0441 commit e884e09
Show file tree
Hide file tree
Showing 9 changed files with 1,626 additions and 72 deletions.
18 changes: 12 additions & 6 deletions backend/infrahub/core/diff/conflicts_enricher.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,14 +148,14 @@ def _add_relationship_conflicts_for_one_peer(
base_property = base_properties_by_type[property_type]
branch_property = branch_properties_by_type[property_type]
if base_property.new_value != branch_property.new_value:
if branch_property.conflict:
conflict_uuid = branch_property.conflict.uuid
selected_branch = branch_property.conflict.selected_branch
else:
conflict_uuid = str(uuid4())
selected_branch = None
# special handling for cardinality-one peer ID conflict
if branch_property.property_type is DatabaseEdgeType.IS_RELATED and is_cardinality_one:
if branch_element.conflict:
conflict_uuid = branch_element.conflict.uuid
selected_branch = branch_element.conflict.selected_branch
else:
conflict_uuid = str(uuid4())
selected_branch = None
branch_element.conflict = EnrichedDiffConflict(
uuid=conflict_uuid,
base_branch_action=base_element.action,
Expand All @@ -167,6 +167,12 @@ def _add_relationship_conflicts_for_one_peer(
selected_branch=selected_branch,
)
else:
if branch_property.conflict:
conflict_uuid = branch_property.conflict.uuid
selected_branch = branch_property.conflict.selected_branch
else:
conflict_uuid = str(uuid4())
selected_branch = None
branch_property.conflict = EnrichedDiffConflict(
uuid=conflict_uuid,
base_branch_action=base_property.action,
Expand Down
18 changes: 10 additions & 8 deletions backend/infrahub/core/diff/coordinator.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ async def _update_diffs(
diff_branch_names=[branch.name],
from_time=from_time,
to_time=to_time,
tracking_id=tracking_id,
)
if tracking_id:
diff_uuids_to_delete += [
Expand Down Expand Up @@ -199,16 +200,17 @@ def _get_missing_time_ranges(
) -> list[TimeRange]:
if not time_ranges:
return [TimeRange(from_time=from_time, to_time=to_time)]
sorted_time_ranges = sorted(time_ranges, key=lambda tr: tr.from_time)
missing_time_ranges = []
if time_ranges[0].from_time > from_time:
missing_time_ranges.append(TimeRange(from_time=from_time, to_time=time_ranges[0].from_time))
if sorted_time_ranges[0].from_time > from_time:
missing_time_ranges.append(TimeRange(from_time=from_time, to_time=sorted_time_ranges[0].from_time))
index = 0
while index < len(time_ranges) - 1:
this_diff = time_ranges[index]
next_diff = time_ranges[index + 1]
if this_diff.to_time != next_diff.from_time:
while index < len(sorted_time_ranges) - 1:
this_diff = sorted_time_ranges[index]
next_diff = sorted_time_ranges[index + 1]
if this_diff.to_time < next_diff.from_time:
missing_time_ranges.append(TimeRange(from_time=this_diff.to_time, to_time=next_diff.from_time))
index += 1
if time_ranges[-1].to_time < to_time:
missing_time_ranges.append(TimeRange(from_time=time_ranges[-1].to_time, to_time=to_time))
if sorted_time_ranges[-1].to_time < to_time:
missing_time_ranges.append(TimeRange(from_time=sorted_time_ranges[-1].to_time, to_time=to_time))
return missing_time_ranges
4 changes: 2 additions & 2 deletions backend/infrahub/core/diff/enricher/path_identifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ async def enrich(self, enriched_diff_root: EnrichedDiffRoot, calculated_diffs: C
for relationship_element in relationship.relationships:
relationship_element_path = relationship_path.model_copy()
relationship_element_path.peer_id = relationship_element.peer_id
relationship_element.path_identifier = relationship_element_path.get_path()
relationship_element.path_identifier = relationship_element_path.get_path(with_peer=False)
for relationship_property in relationship_element.properties:
relationship_property_path = relationship_element_path.model_copy()
relationship_property_path.property_name = relationship_property.property_type.value
relationship_property.path_identifier = relationship_property_path.get_path()
relationship_property.path_identifier = relationship_property_path.get_path(with_peer=False)
6 changes: 6 additions & 0 deletions backend/infrahub/core/diff/model/path.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ def __eq__(self, other: object) -> bool:
def __hash__(self) -> int:
return hash(self.serialize())

def __str__(self) -> str:
return self.serialize()

def __repr__(self) -> str:
return f"{self.__class__.__name__} ({self.serialize()})"


class BranchTrackingId(TrackingId):
prefix = "branch"
Expand Down
22 changes: 17 additions & 5 deletions backend/infrahub/core/diff/query_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ def _get_single_relationship_final_property(
changed_at = chronological_properties[-1].changed_at
previous_value = None
first_diff_prop = chronological_properties[0]
if first_diff_prop.changed_at < from_time and first_diff_prop.status is not RelationshipStatus.DELETED:
if first_diff_prop.status is RelationshipStatus.DELETED or first_diff_prop.changed_at < from_time:
previous_value = first_diff_prop.value
new_value = None
last_diff_prop = chronological_properties[-1]
Expand Down Expand Up @@ -268,6 +268,14 @@ def get_final_single_relationship(self, from_time: Timestamp) -> DiffSingleRelat
final_properties = [peer_final_property] + other_final_properties
last_changed_property = max(final_properties, key=lambda fp: fp.changed_at)
last_changed_at = last_changed_property.changed_at
# handle case when peer has been deleted on another branch, but the properties of the relationship are updated
if (
last_changed_property.property_type is not DatabaseEdgeType.IS_RELATED
and any(p.action is not DiffAction.REMOVED for p in other_final_properties)
and peer_final_property.action is DiffAction.REMOVED
):
peer_final_property.action = DiffAction.UNCHANGED
peer_final_property.new_value = peer_id
if last_changed_at < from_time:
action = DiffAction.UNCHANGED
elif peer_final_property.action in (DiffAction.ADDED, DiffAction.REMOVED):
Expand Down Expand Up @@ -543,12 +551,16 @@ def _apply_relationship_previous_values(
prop_type = DatabaseEdgeType(base_diff_property.property_type)
if prop_type not in base_diff_property_by_type:
continue
if base_diff_property.changed_at >= self.from_time:
continue
if (
base_diff_property_by_type[prop_type] is None
or base_diff_property.changed_at < base_diff_property_by_type[prop_type]
base_diff_property.status is RelationshipStatus.ACTIVE
and base_diff_property.changed_at >= self.from_time
):
continue
if base_diff_property_by_type[prop_type] is None:
base_diff_property_by_type[prop_type] = base_diff_property
continue
current_property = base_diff_property_by_type.get(prop_type)
if current_property and base_diff_property.changed_at < current_property.changed_at:
base_diff_property_by_type[prop_type] = base_diff_property
for diff_property in base_diff_property_by_type.values():
if diff_property:
Expand Down
56 changes: 42 additions & 14 deletions backend/infrahub/core/query/diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs):
self.params.update(br_params)
self.params["branch_support"] = [item.value for item in self.branch_support]

# ruff: noqa: E501
query = """
// all updated edges for our branches and time frame
MATCH (p)-[diff_rel]-(q)
Expand All @@ -525,8 +526,8 @@ async def query_init(self, db: InfrahubDatabase, **kwargs):
WITH p, diff_rel, q
// -- DEEPEST EDGE SUBQUERY --
// get full path for every HAS_VALUE, HAS_SOURCE/OWNER, IS_VISIBLE/PROTECTED
// explicitly add in base branch path, if it exists to capture previous value
// explicitly add in far-side of any relationship to get peer_id for rel properties
// can be multiple paths in the case of HAS_SOURCE/OWNER, IS_VISIBLE/PROTECTED to include
// both peers in the relationship
CALL {
WITH p, q, diff_rel
OPTIONAL MATCH path = (
Expand All @@ -538,19 +539,32 @@ async def query_init(self, db: InfrahubDatabase, **kwargs):
AND any(l in labels(inner_q) WHERE l in ["Boolean", "Node", "AttributeValue"])
AND type(r_node) IN ["HAS_ATTRIBUTE", "IS_RELATED"]
AND %(n_node_where)s
AND ID(n) <> ID(inner_q)
AND [ID(n), type(r_node)] <> [ID(inner_q), type(inner_diff_rel)]
AND ALL(
r in [r_root, r_node]
WHERE r.from <= $to_time AND (r.to IS NULL or r.to >= $to_time)
AND r.branch IN $branch_names
WHERE r.from <= $to_time AND r.branch IN $branch_names
)
// exclude paths where an active edge is below a deleted edge
AND (inner_diff_rel.status = "deleted" OR r_node.status = "active")
AND (r_node.status = "deleted" OR r_root.status = "active")
WITH path AS diff_rel_path, diff_rel, r_root, n, r_node, p
ORDER BY
ID(n) DESC,
ID(p) DESC,
r_node.branch = diff_rel.branch DESC,
r_root.branch = diff_rel.branch DESC,
r_node.from DESC,
r_root.from DESC
LIMIT 1
WITH p, n, head(collect(diff_rel_path)) AS deepest_diff_path
RETURN deepest_diff_path
}
WITH p, diff_rel, q, deepest_diff_path
// explicitly add in base branch path, if it exists to capture previous value
// explicitly add in far-side of any relationship to get peer_id for rel properties
CALL {
WITH p, diff_rel, deepest_diff_path
WITH p, diff_rel, deepest_diff_path AS diff_rel_path, nodes(deepest_diff_path) AS drp_nodes, relationships(deepest_diff_path) AS drp_relationships
WITH p, diff_rel, diff_rel_path, drp_relationships[0] AS r_root, drp_nodes[1] AS n, drp_relationships[1] AS r_node
// get base branch version of the diff path, if it exists
WITH diff_rel_path, diff_rel, r_root, n, r_node, p
OPTIONAL MATCH latest_base_path = (:Root)<-[r_root2]-(n2)-[r_node2]-(inner_p2)-[base_diff_rel]->(base_prop)
Expand All @@ -560,9 +574,11 @@ async def query_init(self, db: InfrahubDatabase, **kwargs):
AND type(base_diff_rel) = type(diff_rel)
AND all(
r in relationships(latest_base_path)
WHERE r.branch = $base_branch_name
AND r.from <= $from_time AND (r.to IS NULL or r.to <= $from_time)
WHERE r.branch = $base_branch_name AND r.from <= $from_time
)
// exclude paths where an active edge is below a deleted edge
AND (base_diff_rel.status = "deleted" OR r_node2.status = "active")
AND (r_node2.status = "deleted" OR r_root2.status = "active")
WITH diff_rel_path, latest_base_path, diff_rel, r_root, n, r_node, p
ORDER BY base_diff_rel.from DESC, r_node.from DESC, r_root.from DESC
LIMIT 1
Expand All @@ -573,9 +589,12 @@ async def query_init(self, db: InfrahubDatabase, **kwargs):
)
WHERE ID(r_root3) = ID(r_root) AND ID(n3) = ID(n) AND ID(r_node3) = ID(r_node) AND ID(inner_p3) = ID(p)
AND type(diff_rel) <> "IS_RELATED"
AND ID(n3) <> ID(base_peer)
AND base_r_peer.from <= $from_time
AND [ID(n3), type(r_node3)] <> [ID(base_peer), type(base_r_peer)]
AND base_r_peer.from <= $to_time
AND base_r_peer.branch IN $branch_names
// exclude paths where an active edge is below a deleted edge
AND (base_r_peer.status = "deleted" OR r_node3.status = "active")
AND (r_node3.status = "deleted" OR r_root3.status = "active")
WITH diff_rel_path, latest_base_path, base_peer_path, base_r_peer, diff_rel
ORDER BY base_r_peer.branch = diff_rel.branch DESC, base_r_peer.from DESC
LIMIT 1
Expand All @@ -601,10 +620,12 @@ async def query_init(self, db: InfrahubDatabase, **kwargs):
AND any(l in labels(prop) WHERE l in ["Boolean", "Node", "AttributeValue"])
AND ALL(
r in [r_root, r_prop]
WHERE r.from <= $to_time AND (r.to IS NULL or r.to >= $to_time)
AND r.branch IN $branch_names
WHERE r.from <= $to_time AND r.branch IN $branch_names
)
AND [ID(inner_p), type(inner_diff_rel)] <> [ID(prop), type(r_prop)]
// exclude paths where an active edge is below a deleted edge
AND (inner_diff_rel.status = "active" OR (r_prop.status = "deleted" AND inner_diff_rel.branch = r_prop.branch))
AND (inner_diff_rel.status = "deleted" OR r_root.status = "active")
WITH path, prop, r_prop, r_root
ORDER BY
ID(prop),
Expand Down Expand Up @@ -632,10 +653,17 @@ async def query_init(self, db: InfrahubDatabase, **kwargs):
AND any(l in labels(prop) WHERE l in ["Boolean", "Node", "AttributeValue"])
AND ALL(
r in [r_node, r_prop]
WHERE r.from <= $to_time AND (r.to IS NULL or r.to >= $to_time)
AND r.branch IN $branch_names
WHERE r.from <= $to_time AND r.branch IN $branch_names
)
AND [ID(inner_p), type(r_node)] <> [ID(prop), type(r_prop)]
// exclude paths where an active edge is below a deleted edge
AND (inner_diff_rel.status = "active" OR
(
r_node.status = "deleted" AND inner_diff_rel.branch = r_node.branch
AND r_prop.status = "deleted" AND inner_diff_rel.branch = r_prop.branch
)
)
AND (r_prop.status = "deleted" OR r_node.status = "active")
WITH path, node, prop, r_prop, r_node
ORDER BY
ID(node),
Expand Down
Loading

0 comments on commit e884e09

Please sign in to comment.