From 7cd4a9e80638bb8d6e60b496294771721007c295 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Mon, 20 Jul 2026 17:17:20 -0700 Subject: [PATCH 01/31] move NodeDiffIndex component --- .../core/validators/node_diff_index.py | 56 ++++++++++++++ .../tests/unit/core/validators/__init__.py | 0 .../core/validators/test_node_diff_index.py | 75 +++++++++++++++++++ 3 files changed, 131 insertions(+) create mode 100644 backend/infrahub/core/validators/node_diff_index.py create mode 100644 backend/tests/unit/core/validators/__init__.py create mode 100644 backend/tests/unit/core/validators/test_node_diff_index.py diff --git a/backend/infrahub/core/validators/node_diff_index.py b/backend/infrahub/core/validators/node_diff_index.py new file mode 100644 index 00000000000..340632c40c0 --- /dev/null +++ b/backend/infrahub/core/validators/node_diff_index.py @@ -0,0 +1,56 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from infrahub.core.diff.model.path import NodeDiffFieldSummary + + +class NodeDiffIndex: + """Which fields and nodes changed, per kind, in a data diff. + + `initialize` must be called for a given diff before any query method is used. + """ + + def __init__(self) -> None: + self._initialized: bool = False + self._kinds: set[str] = set() + self._attribute_names_by_kind: dict[str, set[str]] = {} + self._relationship_names_by_kind: dict[str, set[str]] = {} + self._node_uuids_by_kind: dict[str, set[str]] = {} + + def initialize(self, node_diffs: list[NodeDiffFieldSummary]) -> None: + self._kinds = set() + self._attribute_names_by_kind = {} + self._relationship_names_by_kind = {} + self._node_uuids_by_kind = {} + for node_diff in node_diffs: + self._kinds.add(node_diff.kind) + self._attribute_names_by_kind.setdefault(node_diff.kind, set()).update(node_diff.attribute_names) + self._relationship_names_by_kind.setdefault(node_diff.kind, set()).update(node_diff.relationship_names) + self._node_uuids_by_kind.setdefault(node_diff.kind, set()).update(node_diff.node_uuids) + self._initialized = True + + def _ensure_initialized(self) -> None: + if not self._initialized: + raise RuntimeError("NodeDiffIndex must be initialized with initialize() before its query methods are used") + + @property + def kinds(self) -> set[str]: + self._ensure_initialized() + return self._kinds + + def has_attribute_diff(self, kind: str, name: str) -> bool: + self._ensure_initialized() + return name in self._attribute_names_by_kind.get(kind, set()) + + def has_relationship_diff(self, kind: str, name: str) -> bool: + self._ensure_initialized() + return name in self._relationship_names_by_kind.get(kind, set()) + + def uuids_for_kinds(self, kinds: set[str]) -> set[str]: + self._ensure_initialized() + uuids: set[str] = set() + for kind in kinds: + uuids |= self._node_uuids_by_kind.get(kind, set()) + return uuids diff --git a/backend/tests/unit/core/validators/__init__.py b/backend/tests/unit/core/validators/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/backend/tests/unit/core/validators/test_node_diff_index.py b/backend/tests/unit/core/validators/test_node_diff_index.py new file mode 100644 index 00000000000..24e7f8464e5 --- /dev/null +++ b/backend/tests/unit/core/validators/test_node_diff_index.py @@ -0,0 +1,75 @@ +import pytest + +from infrahub.core.diff.model.path import NodeDiffFieldSummary +from infrahub.core.validators.node_diff_index import NodeDiffIndex + + +def test_query_before_initialize_raises() -> None: + index = NodeDiffIndex() + + match = r"^NodeDiffIndex must be initialized with initialize\(\) before its query methods are used$" + with pytest.raises(RuntimeError, match=match): + _ = index.kinds + with pytest.raises(RuntimeError, match=match): + index.has_attribute_diff(kind="TestCar", name="name") + with pytest.raises(RuntimeError, match=match): + index.has_relationship_diff(kind="TestCar", name="owner") + with pytest.raises(RuntimeError, match=match): + index.uuids_for_kinds({"TestCar"}) + + +def test_indexes_fields_and_uuids_per_kind() -> None: + index = NodeDiffIndex() + index.initialize( + [ + NodeDiffFieldSummary( + kind="TestCar", + attribute_names={"name", "color"}, + relationship_names={"owner"}, + node_uuids={"c1", "c2"}, + ), + NodeDiffFieldSummary(kind="TestPerson", attribute_names={"height"}, node_uuids={"p1"}), + ] + ) + + assert index.kinds == {"TestCar", "TestPerson"} + assert index.has_attribute_diff(kind="TestCar", name="name") + assert not index.has_attribute_diff(kind="TestCar", name="missing") + assert not index.has_attribute_diff(kind="Unknown", name="name") + assert index.has_relationship_diff(kind="TestCar", name="owner") + assert not index.has_relationship_diff(kind="TestPerson", name="owner") + assert index.uuids_for_kinds({"TestCar", "TestPerson"}) == {"c1", "c2", "p1"} + assert index.uuids_for_kinds({"Unknown"}) == set() + + +def test_summaries_for_same_kind_are_merged() -> None: + index = NodeDiffIndex() + index.initialize( + [ + NodeDiffFieldSummary(kind="TestCar", attribute_names={"name"}, node_uuids={"c1"}), + NodeDiffFieldSummary(kind="TestCar", attribute_names={"color"}, node_uuids={"c2"}), + ] + ) + + assert index.has_attribute_diff(kind="TestCar", name="name") + assert index.has_attribute_diff(kind="TestCar", name="color") + assert index.uuids_for_kinds({"TestCar"}) == {"c1", "c2"} + + +def test_initialize_resets_prior_state() -> None: + index = NodeDiffIndex() + index.initialize([NodeDiffFieldSummary(kind="A", attribute_names={"x"}, node_uuids={"a1"})]) + index.initialize([NodeDiffFieldSummary(kind="B", attribute_names={"y"}, node_uuids={"b1"})]) + + assert index.kinds == {"B"} + assert not index.has_attribute_diff(kind="A", name="x") + assert index.uuids_for_kinds({"A"}) == set() + + +def test_empty_diff() -> None: + index = NodeDiffIndex() + index.initialize([]) + + assert index.kinds == set() + assert not index.has_attribute_diff(kind="A", name="x") + assert index.uuids_for_kinds({"A"}) == set() From cd8f003a5b77e2ccdaa1af8f08dca9b1e3298148 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Tue, 21 Jul 2026 14:07:17 -0700 Subject: [PATCH 02/31] move existing uniqueness queries into dir --- .../core/validators/uniqueness/query/__init__.py | 14 ++++++++++++++ .../uniqueness/{query.py => query/validation.py} | 6 +++--- 2 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 backend/infrahub/core/validators/uniqueness/query/__init__.py rename backend/infrahub/core/validators/uniqueness/{query.py => query/validation.py} (98%) diff --git a/backend/infrahub/core/validators/uniqueness/query/__init__.py b/backend/infrahub/core/validators/uniqueness/query/__init__.py new file mode 100644 index 00000000000..b5309bba360 --- /dev/null +++ b/backend/infrahub/core/validators/uniqueness/query/__init__.py @@ -0,0 +1,14 @@ +from ..model import QueryAttributePathValued, QueryRelationshipPathValued +from .affected_dependents import AffectedUniquenessDependentsQuery +from .targeted_validation import TargetedUniquenessValidationQuery, TargetedUniquenessViolation +from .validation import NodeUniqueAttributeConstraintQuery, UniquenessValidationQuery + +__all__ = [ + "AffectedUniquenessDependentsQuery", + "NodeUniqueAttributeConstraintQuery", + "QueryAttributePathValued", + "QueryRelationshipPathValued", + "TargetedUniquenessValidationQuery", + "TargetedUniquenessViolation", + "UniquenessValidationQuery", +] diff --git a/backend/infrahub/core/validators/uniqueness/query.py b/backend/infrahub/core/validators/uniqueness/query/validation.py similarity index 98% rename from backend/infrahub/core/validators/uniqueness/query.py rename to backend/infrahub/core/validators/uniqueness/query/validation.py index c739803644a..50df006c8c8 100644 --- a/backend/infrahub/core/validators/uniqueness/query.py +++ b/backend/infrahub/core/validators/uniqueness/query/validation.py @@ -7,12 +7,12 @@ from infrahub.core.query import Query, QueryType from infrahub.types import is_large_attribute_type -from .model import QueryAttributePathValued, QueryRelationshipPathValued +from ..model import QueryAttributePathValued, QueryRelationshipPathValued if TYPE_CHECKING: from infrahub.database import InfrahubDatabase - from .model import NodeUniquenessQueryRequest, NodeUniquenessQueryRequestValued + from ..model import NodeUniquenessQueryRequest, NodeUniquenessQueryRequestValued class NodeUniqueAttributeConstraintQuery(Query): @@ -350,7 +350,7 @@ def _build_rel_subquery( params: dict[str, str | int | float | bool] = {} rel_attr_query = "" rel_attr_match = "" - if rel_path.attribute_name and rel_path.attribute_value: + if rel_path.attribute_name is not None and rel_path.attribute_value is not None: attr_name_var = f"attr_name_{index}" attr_value_var = f"attr_value_{index}" From f5da58d229da0a3e8f201e158f3d67452fc4b7d0 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Tue, 21 Jul 2026 17:48:16 -0700 Subject: [PATCH 03/31] add targeted uniqueness validation query --- .../uniqueness/query/targeted_validation.py | 379 +++++++++++++++ .../test_targeted_uniqueness_query.py | 438 ++++++++++++++++++ 2 files changed, 817 insertions(+) create mode 100644 backend/infrahub/core/validators/uniqueness/query/targeted_validation.py create mode 100644 backend/tests/component/core/constraint_validators/test_targeted_uniqueness_query.py diff --git a/backend/infrahub/core/validators/uniqueness/query/targeted_validation.py b/backend/infrahub/core/validators/uniqueness/query/targeted_validation.py new file mode 100644 index 00000000000..c85b5a10eab --- /dev/null +++ b/backend/infrahub/core/validators/uniqueness/query/targeted_validation.py @@ -0,0 +1,379 @@ +from __future__ import annotations + +from dataclasses import dataclass +from typing import TYPE_CHECKING, Any + +from infrahub.core.graph.schema import GraphAttributeValueIndexedNode, GraphAttributeValueNode +from infrahub.core.query import Query, QueryType +from infrahub.types import is_large_attribute_type + +if TYPE_CHECKING: + from collections.abc import Generator + + from infrahub.core.schema.basenode_schema import SchemaAttributePath + from infrahub.database import InfrahubDatabase + + +@dataclass(frozen=True) +class TargetedUniquenessViolation: + """One changed node sharing its full constraint-value tuple with at least one other node.""" + + changed_uuid: str + element_values: tuple[Any, ...] + partner_uuids: tuple[str, ...] + + +class TargetedUniquenessValidationQuery(Query): + """Find uniqueness violations for a set of changed nodes. + + For one uniqueness constraint group, resolve the changed nodes' current value for a first + element only, probe the whole population for other nodes sharing that value, and drop every + changed node with no match. Each subsequent element is resolved only for the changed nodes + that still have live matches, and each candidate set shrinks by comparing the candidates' + current value for that element. A changed node is reported only if at least one other node + still shares its full value tuple after the last element. + + Values are compared as stored in the graph: enum values in their raw form and null attribute + values as the null sentinel, so two nulls collide. A node without a live value for an element + (e.g. a relationship with no peer) contributes no tuple and cannot collide on that group. + + Changed nodes and candidate partners must themselves be live: their latest IS_PART_OF edge on + a visible branch must be active. This excludes nodes deleted on the branch and stale same-uuid + duplicates left behind by kind or inheritance migrations. + + Supported constraint elements are node attributes (compared by value) and cardinality-one + relationships (compared by peer uuid). Attributes of related peers are not supported. + """ + + name = "uniqueness_constraint_validation_targeted" + type = QueryType.READ + insert_return = False + + def __init__( + self, + kind: str, + constraint_elements: list[SchemaAttributePath], + node_uuids: list[str], + **kwargs: Any, + ) -> None: + if not constraint_elements: + raise ValueError("At least one constraint element is required") + for element in constraint_elements: + if element.relationship_schema is not None and element.attribute_schema is not None: + raise ValueError( + f"{element.to_string()} is not supported for a targeted uniqueness check: " + "attributes of related peers cannot be part of a uniqueness constraint" + ) + if element.relationship_schema is None and element.attribute_schema is None: + raise ValueError("A constraint element requires an attribute or a relationship") + if element.attribute_schema is not None and (element.attribute_property_name or "value") != "value": + raise ValueError( + f"{element.attribute_property_name} is not a valid property for a uniqueness constraint" + ) + self.kind = kind + self.constraint_elements = constraint_elements + self.node_uuids = node_uuids + super().__init__(**kwargs) + + def get_context(self) -> dict[str, str]: + return {"kind": self.kind} + + def _render_liveness_check(self, source_var: str, alias: str, branch_filter: str) -> str: + """Render a subquery keeping only rows whose node is live. + + The latest IS_PART_OF edge on a visible branch decides: a node deleted on the branch, or a + stale same-uuid duplicate left by a kind or inheritance migration, resolves to a non-active + winner and the row is eliminated. + """ + return """ +CALL (%(source_var)s) { + MATCH (%(source_var)s)-[r:IS_PART_OF]->(:Root) + WHERE %(branch_filter)s + WITH r.status = "active" AS is_active + ORDER BY r.branch_level DESC, r.from DESC, r.status ASC + WITH is_active + LIMIT 1 + WITH is_active + WHERE is_active = TRUE + RETURN TRUE AS %(alias)s +} + """ % { + "source_var": source_var, + "alias": alias, + "branch_filter": branch_filter, + } + + def _is_large_type(self, element: SchemaAttributePath) -> bool: + return element.attribute_schema is not None and is_large_attribute_type(element.attribute_schema.kind) + + def _anchor_element_index(self) -> int: + """Pick the element whose population probe anchors the query. + + The anchor is the only population-wide MATCH, so prefer an element whose value can be + found through an index: any relationship element (peer uuid) or any attribute whose kind + has an indexed value label. Fall back to the first element when none qualifies. + """ + for index, element in enumerate(self.constraint_elements): + if not self._is_large_type(element): + return index + return 0 + + def _render_value_resolution( + self, source_var: str, element: SchemaAttributePath, index: int, value_var: str, branch_filter: str + ) -> tuple[str, dict[str, Any]]: + """Render a subquery resolving the current value of one constraint element for one node. + + The winning edge per step is the latest one on the deepest branch; the row is eliminated + when the winner is not active, so a node without a live value yields no row. + """ + if element.attribute_schema is not None: + attr_name_var = f"attr_name_{index}" + query = """ +CALL (%(source_var)s) { + MATCH (%(source_var)s)-[r:HAS_ATTRIBUTE]->(attr:Attribute {name: $%(attr_name_var)s}) + WHERE %(branch_filter)s + WITH attr, r.status = "active" AS is_active + ORDER BY r.branch_level DESC, r.from DESC, r.status ASC + WITH attr, is_active + LIMIT 1 + WITH attr, is_active + WHERE is_active = TRUE + MATCH (attr)-[r:HAS_VALUE]->(av:AttributeValue) + WHERE %(branch_filter)s + WITH av, r.status = "active" AS is_active + ORDER BY r.branch_level DESC, r.from DESC, r.status ASC + WITH av, is_active + LIMIT 1 + WITH av, is_active + WHERE is_active = TRUE + RETURN av.value AS %(value_var)s +} + """ % { + "source_var": source_var, + "attr_name_var": attr_name_var, + "branch_filter": branch_filter, + "value_var": value_var, + } + return query, {attr_name_var: element.attribute_schema.name} + + relationship_schema = element.active_relationship_schema + rel_identifier_var = f"rel_identifier_{index}" + query_arrows = relationship_schema.get_query_arrows() + query = """ +CALL (%(source_var)s) { + MATCH (%(source_var)s)%(lstart)s[r:IS_RELATED]%(lend)s(rel:Relationship {name: $%(rel_identifier_var)s}) + WHERE %(branch_filter)s + WITH rel, r.status = "active" AS is_active + ORDER BY r.branch_level DESC, r.from DESC, r.status ASC + WITH rel, is_active + LIMIT 1 + WITH rel, is_active + WHERE is_active = TRUE + MATCH (rel)%(rstart)s[r:IS_RELATED]%(rend)s(peer:Node) + WHERE %(branch_filter)s AND peer.uuid <> %(source_var)s.uuid + WITH peer, r.status = "active" AS is_active + ORDER BY r.branch_level DESC, r.from DESC, r.status ASC + WITH peer, is_active + LIMIT 1 + WITH peer, is_active + WHERE is_active = TRUE + RETURN peer.uuid AS %(value_var)s +} + """ % { + "source_var": source_var, + "rel_identifier_var": rel_identifier_var, + "lstart": query_arrows.left.start, + "lend": query_arrows.left.end, + "rstart": query_arrows.right.start, + "rend": query_arrows.right.end, + "branch_filter": branch_filter, + "value_var": value_var, + } + return query, {rel_identifier_var: relationship_schema.get_identifier()} + + def _render_anchor_probe( + self, element: SchemaAttributePath, index: int, value_var: str, matches_var: str, branch_filter: str + ) -> str: + """Render the population-wide probe for the anchor element. + + The MATCH is historical (any edge ever created), so each candidate's current value is + re-resolved and compared before it counts as a match. + """ + if element.attribute_schema is not None: + attr_value_label = ( + GraphAttributeValueNode.get_default_label() + if self._is_large_type(element) + else GraphAttributeValueIndexedNode.get_default_label() + ) + anchor_match = ( + "MATCH (candidate:%(kind)s)-[:HAS_ATTRIBUTE]->(:Attribute {name: $attr_name_%(index)s})" + "-[:HAS_VALUE]->(av:%(attr_value_label)s)\n" + " WHERE av.value = %(value_var)s AND candidate.uuid <> changed.uuid" + ) % { + "kind": self.kind, + "index": index, + "attr_value_label": attr_value_label, + "value_var": value_var, + } + else: + query_arrows = element.active_relationship_schema.get_query_arrows() + anchor_match = ( + "MATCH (candidate:%(kind)s)%(lstart)s[:IS_RELATED]%(lend)s" + "(:Relationship {name: $rel_identifier_%(index)s})%(rstart)s[:IS_RELATED]%(rend)s(anchor_peer:Node)\n" + " WHERE anchor_peer.uuid = %(value_var)s AND candidate.uuid <> changed.uuid" + ) % { + "kind": self.kind, + "index": index, + "lstart": query_arrows.left.start, + "lend": query_arrows.left.end, + "rstart": query_arrows.right.start, + "rend": query_arrows.right.end, + "value_var": value_var, + } + + candidate_liveness = self._render_liveness_check( + source_var="candidate", alias="candidate_is_live", branch_filter=branch_filter + ) + candidate_resolution, _ = self._render_value_resolution( + source_var="candidate", + element=element, + index=index, + value_var=f"cand_value_{index}", + branch_filter=branch_filter, + ) + return """ +CALL (changed, %(value_var)s) { + %(anchor_match)s + WITH DISTINCT candidate, %(value_var)s + %(candidate_liveness)s + %(candidate_resolution)s + WITH candidate, cand_value_%(index)s, %(value_var)s + WHERE cand_value_%(index)s = %(value_var)s + RETURN collect(DISTINCT candidate) AS %(matches_var)s +} + """ % { + "value_var": value_var, + "anchor_match": anchor_match, + "candidate_liveness": candidate_liveness, + "candidate_resolution": candidate_resolution, + "index": index, + "matches_var": matches_var, + } + + def _render_reduction_probe( + self, + element: SchemaAttributePath, + index: int, + value_var: str, + previous_matches_var: str, + matches_var: str, + branch_filter: str, + carried_vars: list[str], + ) -> str: + """Render the filter keeping only surviving candidates whose current value still matches. + + The candidate list is unwound in the outer scope, so only the per-candidate value + resolution needs a subquery. A changed node whose candidates are all eliminated produces + no row for the final aggregation, which is what removes it. + """ + candidate_resolution, _ = self._render_value_resolution( + source_var="candidate", + element=element, + index=index, + value_var=f"cand_value_{index}", + branch_filter=branch_filter, + ) + carried = ", ".join(carried_vars) + return """ +UNWIND %(previous_matches_var)s AS candidate +%(candidate_resolution)s +WITH %(carried)s, candidate, cand_value_%(index)s +WHERE cand_value_%(index)s = %(value_var)s +WITH %(carried)s, collect(DISTINCT candidate) AS %(matches_var)s + """ % { + "previous_matches_var": previous_matches_var, + "value_var": value_var, + "candidate_resolution": candidate_resolution, + "index": index, + "matches_var": matches_var, + "carried": carried, + } + + async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # noqa: ARG002 + branch_filter, branch_params = self.branch.get_query_filter_path(at=self.at.to_string(), is_isolated=False) + self.params.update(branch_params) + self.params["node_uuids"] = self.node_uuids + + anchor_index = self._anchor_element_index() + probe_order = [anchor_index] + [ + index for index in range(len(self.constraint_elements)) if index != anchor_index + ] + + query_parts = [ + "MATCH (changed:Node)\nWHERE changed.uuid IN $node_uuids", + self._render_liveness_check(source_var="changed", alias="changed_is_live", branch_filter=branch_filter), + ] + resolved_value_vars: list[str] = [] + matches_var = "" + for step, element_index in enumerate(probe_order): + element = self.constraint_elements[element_index] + value_var = f"value_{element_index}" + resolution, element_params = self._render_value_resolution( + source_var="changed", + element=element, + index=element_index, + value_var=value_var, + branch_filter=branch_filter, + ) + self.params.update(element_params) + query_parts.append(resolution) + resolved_value_vars.append(value_var) + + previous_matches_var = matches_var + matches_var = f"matches_{step}" + if step == 0: + query_parts.append( + self._render_anchor_probe( + element=element, + index=element_index, + value_var=value_var, + matches_var=matches_var, + branch_filter=branch_filter, + ) + ) + else: + query_parts.append( + self._render_reduction_probe( + element=element, + index=element_index, + value_var=value_var, + previous_matches_var=previous_matches_var, + matches_var=matches_var, + branch_filter=branch_filter, + carried_vars=["changed", *resolved_value_vars], + ) + ) + carried_vars = ["changed", *resolved_value_vars, matches_var] + query_parts.append( + "WITH %(carried_vars)s\nWHERE size(%(matches_var)s) > 0" + % {"carried_vars": ", ".join(carried_vars), "matches_var": matches_var} + ) + + element_values = ", ".join(f"value_{index}" for index in range(len(self.constraint_elements))) + query_parts.append( + "RETURN changed.uuid AS changed_uuid,\n" + " [%(element_values)s] AS element_values,\n" + " [candidate IN %(matches_var)s | candidate.uuid] AS partner_uuids" + % {"element_values": element_values, "matches_var": matches_var} + ) + + self.add_to_query("\n".join(query_parts)) + self.return_labels = ["changed_uuid", "element_values", "partner_uuids"] + + def get_data(self) -> Generator[TargetedUniquenessViolation, None, None]: + for result in self.results: + yield TargetedUniquenessViolation( + changed_uuid=result.get_as_type("changed_uuid", return_type=str), + element_values=result.get_as_type("element_values", return_type=tuple), + partner_uuids=tuple(result.get_as_list_of_type("partner_uuids", return_type=str)), + ) diff --git a/backend/tests/component/core/constraint_validators/test_targeted_uniqueness_query.py b/backend/tests/component/core/constraint_validators/test_targeted_uniqueness_query.py new file mode 100644 index 00000000000..51780eb309c --- /dev/null +++ b/backend/tests/component/core/constraint_validators/test_targeted_uniqueness_query.py @@ -0,0 +1,438 @@ +import pytest + +from infrahub.core import registry +from infrahub.core.branch import Branch +from infrahub.core.manager import NodeManager +from infrahub.core.node import Node +from infrahub.core.schema import AttributeSchema, MainSchemaTypes, NodeSchema, SchemaRoot +from infrahub.core.schema.basenode_schema import SchemaAttributePath +from infrahub.core.validators.uniqueness.query import ( + TargetedUniquenessValidationQuery, + TargetedUniquenessViolation, +) +from infrahub.database import InfrahubDatabase + + +def _attr_element(schema: MainSchemaTypes, name: str) -> SchemaAttributePath: + return SchemaAttributePath(attribute_schema=schema.get_attribute(name)) + + +def _rel_element(schema: MainSchemaTypes, name: str) -> SchemaAttributePath: + return SchemaAttributePath(relationship_schema=schema.get_relationship(name)) + + +async def _run_query( + db: InfrahubDatabase, + branch: Branch, + kind: str, + constraint_elements: list[SchemaAttributePath], + node_uuids: list[str], +) -> list[TargetedUniquenessViolation]: + query = await TargetedUniquenessValidationQuery.init( + db=db, branch=branch, kind=kind, constraint_elements=constraint_elements, node_uuids=node_uuids + ) + await query.execute(db=db) + return list(query.get_data()) + + +async def _update_car(db: InfrahubDatabase, branch: Branch, car_id: str, **attribute_values: object) -> None: + car = await NodeManager.get_one(id=car_id, db=db, branch=branch) + for name, value in attribute_values.items(): + car.get_attribute(name).value = value + await car.save(db=db) + + +class TestTargetedUniquenessQuery: + async def test_batched_mixed_colliding_and_unique( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_prius_main: Node, + car_camry_main: Node, + car_volt_main: Node, + car_yaris_main: Node, + branch: Branch, + ) -> None: + # accord shares nbr_seats=5 with the untouched prius and camry; volt is made unique + await _update_car(db, branch, car_volt_main.id, nbr_seats=2) + schema = registry.schema.get("TestCar", branch=branch) + + violations = await _run_query( + db, branch, "TestCar", [_attr_element(schema, "nbr_seats")], [car_accord_main.id, car_volt_main.id] + ) + + assert len(violations) == 1 + violation = violations[0] + assert violation.changed_uuid == car_accord_main.id + assert violation.element_values == (5,) + assert set(violation.partner_uuids) == {car_prius_main.id, car_camry_main.id} + + async def test_multi_field_matches_full_tuple_only( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_prius_main: Node, + car_camry_main: Node, + car_volt_main: Node, + car_yaris_main: Node, + branch: Branch, + ) -> None: + # pairwise overlap on single elements but no shared full tuple: accord(5, red), + # prius(5, blue), volt(4, blue); camry and yaris are moved out of the way + await _update_car(db, branch, car_accord_main.id, nbr_seats=5, color="#ff0000") + await _update_car(db, branch, car_prius_main.id, nbr_seats=5, color="#0000ff") + await _update_car(db, branch, car_volt_main.id, nbr_seats=4, color="#0000ff") + await _update_car(db, branch, car_camry_main.id, nbr_seats=9) + await _update_car(db, branch, car_yaris_main.id, nbr_seats=8) + schema = registry.schema.get("TestCar", branch=branch) + elements = [_attr_element(schema, "nbr_seats"), _attr_element(schema, "color")] + changed = [car_accord_main.id, car_prius_main.id, car_volt_main.id] + + violations = await _run_query(db, branch, "TestCar", elements, changed) + + assert violations == [] + + # completing the tuple match flags exactly the two matching cars + await _update_car(db, branch, car_prius_main.id, color="#ff0000") + + violations = await _run_query(db, branch, "TestCar", elements, changed) + + assert {v.changed_uuid for v in violations} == {car_accord_main.id, car_prius_main.id} + by_changed = {v.changed_uuid: v for v in violations} + assert by_changed[car_accord_main.id].partner_uuids == (car_prius_main.id,) + assert by_changed[car_prius_main.id].partner_uuids == (car_accord_main.id,) + assert by_changed[car_accord_main.id].element_values == (5, "#ff0000") + + async def test_multi_field_group_with_relationship_element( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_prius_main: Node, + car_camry_main: Node, + person_john_main: Node, + person_jane_main: Node, + branch: Branch, + ) -> None: + # accord and prius share owner john AND nbr_seats=5; camry has the same seats but a + # different owner, so it must not match the tuple + schema = registry.schema.get("TestCar", branch=branch) + elements = [_rel_element(schema, "owner"), _attr_element(schema, "nbr_seats")] + + violations = await _run_query(db, branch, "TestCar", elements, [car_accord_main.id]) + + assert len(violations) == 1 + assert violations[0].changed_uuid == car_accord_main.id + assert violations[0].partner_uuids == (car_prius_main.id,) + assert violations[0].element_values == (person_john_main.id, 5) + + # same owner but different seats no longer matches + await _update_car(db, branch, car_prius_main.id, nbr_seats=7) + + violations = await _run_query(db, branch, "TestCar", elements, [car_accord_main.id]) + + assert violations == [] + + async def test_relationship_only_collision_and_missing_peer( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_camry_main: Node, + car_volt_main: Node, + person_john_main: Node, + branch: Branch, + ) -> None: + # driver is an optional cardinality-one relationship: accord and camry are given the same + # driver, volt has none + for car_id in (car_accord_main.id, car_camry_main.id): + car = await NodeManager.get_one(id=car_id, db=db, branch=branch) + await car.get_relationship("driver").update(data=person_john_main, db=db) + await car.save(db=db) + schema = registry.schema.get("TestCar", branch=branch) + elements = [_rel_element(schema, "driver")] + + violations = await _run_query(db, branch, "TestCar", elements, [car_accord_main.id]) + + assert len(violations) == 1 + assert violations[0].changed_uuid == car_accord_main.id + assert violations[0].partner_uuids == (car_camry_main.id,) + assert violations[0].element_values == (person_john_main.id,) + + # a changed node without a live peer contributes no value and cannot collide + violations = await _run_query(db, branch, "TestCar", elements, [car_volt_main.id]) + + assert violations == [] + + async def test_null_attribute_values_collide( + self, + db: InfrahubDatabase, + car_accord_main: Node, + person_john_main: Node, + branch: Branch, + ) -> None: + # two cars without nbr_seats store the null sentinel and must still collide + car_a = await Node.init(db=db, schema="TestCar", branch=branch) + await car_a.new(db=db, name="nullseats-a", owner=person_john_main.id) + await car_a.save(db=db) + car_b = await Node.init(db=db, schema="TestCar", branch=branch) + await car_b.new(db=db, name="nullseats-b", owner=person_john_main.id) + await car_b.save(db=db) + schema = registry.schema.get("TestCar", branch=branch) + + violations = await _run_query(db, branch, "TestCar", [_attr_element(schema, "nbr_seats")], [car_a.id]) + + assert len(violations) == 1 + assert violations[0].changed_uuid == car_a.id + assert violations[0].partner_uuids == (car_b.id,) + assert violations[0].element_values == ("NULL",) + + async def test_enum_attribute_collision_uses_stored_value( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_camry_main: Node, + car_prius_main: Node, + branch: Branch, + ) -> None: + await _update_car(db, branch, car_accord_main.id, transmission="manual") + await _update_car(db, branch, car_camry_main.id, transmission="manual") + schema = registry.schema.get("TestCar", branch=branch) + + violations = await _run_query( + db, branch, "TestCar", [_attr_element(schema, "transmission")], [car_accord_main.id] + ) + + assert len(violations) == 1 + assert violations[0].changed_uuid == car_accord_main.id + assert violations[0].partner_uuids == (car_camry_main.id,) + assert violations[0].element_values == ("manual",) + + async def test_large_attribute_type_validated( + self, + db: InfrahubDatabase, + default_branch: Branch, + car_person_schema: object, + ) -> None: + schema_root = SchemaRoot( + nodes=[ + NodeSchema( + name="Document", + namespace="Test", + attributes=[ + AttributeSchema(name="name", kind="Text"), + AttributeSchema(name="content", kind="TextArea", optional=True), + ], + ) + ] + ) + registry.schema.register_schema(schema=schema_root, branch=default_branch.name) + shared_content = "lorem ipsum " * 50 + doc_a = await Node.init(db=db, schema="TestDocument", branch=default_branch) + await doc_a.new(db=db, name="doc-a", content=shared_content) + await doc_a.save(db=db) + doc_b = await Node.init(db=db, schema="TestDocument", branch=default_branch) + await doc_b.new(db=db, name="doc-b", content=shared_content) + await doc_b.save(db=db) + schema = registry.schema.get("TestDocument", branch=default_branch) + + # a large-type attribute has no value index but is still validated + violations = await _run_query( + db, default_branch, "TestDocument", [_attr_element(schema, "content")], [doc_a.id] + ) + + assert len(violations) == 1 + assert violations[0].partner_uuids == (doc_b.id,) + + # in a multi-element group the full tuple must match: shared content but distinct names + elements = [_attr_element(schema, "content"), _attr_element(schema, "name")] + + violations = await _run_query(db, default_branch, "TestDocument", elements, [doc_a.id]) + + assert violations == [] + + # a third document sharing both content and name completes the tuple + doc_c = await Node.init(db=db, schema="TestDocument", branch=default_branch) + await doc_c.new(db=db, name="doc-a", content=shared_content) + await doc_c.save(db=db) + + violations = await _run_query(db, default_branch, "TestDocument", elements, [doc_a.id]) + + assert len(violations) == 1 + assert violations[0].partner_uuids == (doc_c.id,) + assert violations[0].element_values == (shared_content, "doc-a") + + async def test_post_fork_updates_on_main_and_branch( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_prius_main: Node, + car_camry_main: Node, + default_branch: Branch, + branch: Branch, + ) -> None: + # accord, prius and camry all share nbr_seats=5 when the branch forks; both partners then + # get distinct values on main, which the branch must see: the collision is gone + await _update_car(db, default_branch, car_prius_main.id, nbr_seats=8) + await _update_car(db, default_branch, car_camry_main.id, nbr_seats=9) + schema = registry.schema.get("TestCar", branch=branch) + elements = [_attr_element(schema, "nbr_seats")] + + violations = await _run_query(db, branch, "TestCar", elements, [car_accord_main.id]) + + assert violations == [] + + # camry then gets a new value on main and accord the same new value on the branch; the + # two non-conflicting updates must collide across branches + await _update_car(db, default_branch, car_camry_main.id, nbr_seats=7) + await _update_car(db, branch, car_accord_main.id, nbr_seats=7) + + violations = await _run_query(db, branch, "TestCar", elements, [car_accord_main.id]) + + assert len(violations) == 1 + assert violations[0].partner_uuids == (car_camry_main.id,) + assert violations[0].element_values == (7,) + + # the same collision is found when the main-updated node is the changed one + violations = await _run_query(db, branch, "TestCar", elements, [car_camry_main.id]) + + assert len(violations) == 1 + assert violations[0].partner_uuids == (car_accord_main.id,) + + async def test_branch_visibility( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_prius_main: Node, + car_camry_main: Node, + branch: Branch, + ) -> None: + schema = registry.schema.get("TestCar", branch=branch) + elements = [_attr_element(schema, "nbr_seats")] + + # data created on main only is visible from the branch + violations = await _run_query(db, branch, "TestCar", elements, [car_accord_main.id]) + + assert len(violations) == 1 + assert set(violations[0].partner_uuids) == {car_prius_main.id, car_camry_main.id} + + # the branch-local value overrides main and no longer collides + await _update_car(db, branch, car_accord_main.id, nbr_seats=6) + + violations = await _run_query(db, branch, "TestCar", elements, [car_accord_main.id]) + + assert violations == [] + + # two branch-local values collide with each other + await _update_car(db, branch, car_camry_main.id, nbr_seats=6) + + violations = await _run_query(db, branch, "TestCar", elements, [car_accord_main.id]) + + assert len(violations) == 1 + assert violations[0].partner_uuids == (car_camry_main.id,) + assert violations[0].element_values == (6,) + + async def test_partner_deleted_on_branch_is_ignored( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_prius_main: Node, + car_camry_main: Node, + branch: Branch, + ) -> None: + # accord, prius and camry share nbr_seats=5 on main, but both partners are deleted on the + # branch, so the branch-level deletion overrides the value they still hold on main + for car_id in (car_prius_main.id, car_camry_main.id): + car = await NodeManager.get_one(id=car_id, db=db, branch=branch) + await car.delete(db=db) + schema = registry.schema.get("TestCar", branch=branch) + + violations = await _run_query(db, branch, "TestCar", [_attr_element(schema, "nbr_seats")], [car_accord_main.id]) + + assert violations == [] + + async def test_changed_node_deleted_on_branch_is_ignored( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_prius_main: Node, + car_camry_main: Node, + branch: Branch, + ) -> None: + # a node in the changed set was deleted on the branch: the value it still holds on main + # must not be used, even though prius and camry still share it + car = await NodeManager.get_one(id=car_accord_main.id, db=db, branch=branch) + await car.delete(db=db) + schema = registry.schema.get("TestCar", branch=branch) + + violations = await _run_query(db, branch, "TestCar", [_attr_element(schema, "nbr_seats")], [car_accord_main.id]) + + assert violations == [] + + async def test_changed_nodes_collide_with_each_other( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_prius_main: Node, + car_camry_main: Node, + branch: Branch, + ) -> None: + # only accord and prius keep nbr_seats=5; both are changed and must report each other + await _update_car(db, branch, car_camry_main.id, nbr_seats=9) + schema = registry.schema.get("TestCar", branch=branch) + + violations = await _run_query( + db, branch, "TestCar", [_attr_element(schema, "nbr_seats")], [car_accord_main.id, car_prius_main.id] + ) + + by_changed = {v.changed_uuid: v for v in violations} + assert set(by_changed) == {car_accord_main.id, car_prius_main.id} + assert by_changed[car_accord_main.id].partner_uuids == (car_prius_main.id,) + assert by_changed[car_prius_main.id].partner_uuids == (car_accord_main.id,) + + async def test_peer_attribute_element_rejected( + self, + db: InfrahubDatabase, + car_accord_main: Node, + branch: Branch, + ) -> None: + car_schema = registry.schema.get("TestCar", branch=branch) + person_schema = registry.schema.get_node_schema("TestPerson", branch=branch) + peer_attribute_element = SchemaAttributePath( + relationship_schema=car_schema.get_relationship("owner"), + related_schema=person_schema, + attribute_schema=person_schema.get_attribute("height"), + ) + + with pytest.raises( + ValueError, + match=( + r"^owner__height is not supported for a targeted uniqueness check: " + r"attributes of related peers cannot be part of a uniqueness constraint$" + ), + ): + await TargetedUniquenessValidationQuery.init( + db=db, + branch=branch, + kind="TestCar", + constraint_elements=[peer_attribute_element], + node_uuids=[car_accord_main.id], + ) + + async def test_generic_kind_finds_collisions_across_implementations( + self, + db: InfrahubDatabase, + default_branch: Branch, + car_person_generics_data_simple: dict[str, Node], + ) -> None: + # all three cars (two TestElectricCar, one TestGazCar) share nbr_seats=4; the query is + # anchored on the generic kind and one changed implementing node + data = car_person_generics_data_simple + schema = registry.schema.get("TestCar", branch=default_branch) + + violations = await _run_query( + db, default_branch, "TestCar", [_attr_element(schema, "nbr_seats")], [data["c1"].id] + ) + + assert len(violations) == 1 + assert violations[0].changed_uuid == data["c1"].id + assert set(violations[0].partner_uuids) == {data["c2"].id, data["c3"].id} + assert violations[0].element_values == (4,) From c32e235c0bac77341a211bb59154a0f0bbc55e36 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Tue, 21 Jul 2026 18:33:11 -0700 Subject: [PATCH 04/31] add uniqueness dependent resolver --- .../uniqueness/dependent_resolver.py | 53 ++++++++ .../uniqueness/query/affected_dependents.py | 101 ++++++++++++++ .../test_uniqueness_dependent_resolver.py | 127 ++++++++++++++++++ 3 files changed, 281 insertions(+) create mode 100644 backend/infrahub/core/validators/uniqueness/dependent_resolver.py create mode 100644 backend/infrahub/core/validators/uniqueness/query/affected_dependents.py create mode 100644 backend/tests/component/core/constraint_validators/test_uniqueness_dependent_resolver.py diff --git a/backend/infrahub/core/validators/uniqueness/dependent_resolver.py b/backend/infrahub/core/validators/uniqueness/dependent_resolver.py new file mode 100644 index 00000000000..fe84de6ded2 --- /dev/null +++ b/backend/infrahub/core/validators/uniqueness/dependent_resolver.py @@ -0,0 +1,53 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING, Protocol + +from infrahub.core import registry + +from .query import AffectedUniquenessDependentsQuery + +if TYPE_CHECKING: + from infrahub.core.branch import Branch + from infrahub.core.timestamp import Timestamp + from infrahub.database import InfrahubDatabase + + +class UniquenessDependentResolverInterface(Protocol): + """Resolve which nodes of a kind reference a set of changed peer nodes. + + A uniqueness path such as "owner__name" makes a kind's constraint depend on a peer kind's + attribute; when the peer changes, the constrained nodes themselves are absent from the diff and + must be resolved by traversal before they can be node-scoped. + """ + + async def resolve(self, node_kind: str, relationship_identifier: str, peer_uuids: list[str]) -> set[str]: ... + + +class UniquenessDependentResolver: + """Resolve which nodes of a kind reference changed peer nodes, for cross-kind uniqueness scoping.""" + + def __init__(self, db: InfrahubDatabase, branch: Branch, at: Timestamp | str | None = None) -> None: + self.db = db + self.branch = branch + self.at = at + + async def resolve(self, node_kind: str, relationship_identifier: str, peer_uuids: list[str]) -> set[str]: + """Return the uuids of `node_kind` nodes related via `relationship_identifier` to any peer in `peer_uuids`. + + Only relationships visible from this branch (its own, its base, and the global branch) are + considered. The result is a superset of the truly-related nodes and is empty when no peer + uuids are given or none are referenced. + """ + if not peer_uuids: + return set() + query = await AffectedUniquenessDependentsQuery.init( + db=self.db, + branch=self.branch, + at=self.at, + node_kind=node_kind, + relationship_identifier=relationship_identifier, + peer_uuids=peer_uuids, + default_branch_name=registry.default_branch, + ) + await query.execute(db=self.db) + return query.get_dependent_uuids() diff --git a/backend/infrahub/core/validators/uniqueness/query/affected_dependents.py b/backend/infrahub/core/validators/uniqueness/query/affected_dependents.py new file mode 100644 index 00000000000..54ce40f9ce7 --- /dev/null +++ b/backend/infrahub/core/validators/uniqueness/query/affected_dependents.py @@ -0,0 +1,101 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +from infrahub.core.constants import GLOBAL_BRANCH_NAME +from infrahub.core.query import Query, QueryType + +if TYPE_CHECKING: + from infrahub.database import InfrahubDatabase + + +class AffectedUniquenessDependentsQuery(Query): + """Return the nodes of a kind related, through a named relationship, to any of a set of peer nodes. + + Considers edges at the timestamp on the input branch, default branch, and global branch, regardless + of when the input branch forked from the default branch. That is, changes made on the default branch + after the input branch was created WILL be included in the results. + + Each hop of the path (peer→relationship and relationship→node) is resolved to a single winner + across the visible branches: the latest edge on the deepest branch decides, so a change on the + input branch overrides the default branch. The user branch will always override the default + branch if changes conflict. + """ + + name = "affected_uniqueness_dependents" + type = QueryType.READ + + def __init__( + self, + node_kind: str, + relationship_identifier: str, + peer_uuids: list[str], + default_branch_name: str, + **kwargs: Any, + ) -> None: + self.node_kind = node_kind + self.relationship_identifier = relationship_identifier + self.peer_uuids = peer_uuids + self.default_branch_name = default_branch_name + super().__init__(**kwargs) + + async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # noqa: ARG002 + self.params = { + "rel_identifier": self.relationship_identifier, + "peer_uuids": self.peer_uuids, + "at": self.at.to_string(), + "branch": self.branch.name, + "default_branch": self.default_branch_name, + "global_branch": GLOBAL_BRANCH_NAME, + } + query = """ +// -------------------- +// start with all possible active Relationship paths on a branch we care about +// -------------------- +MATCH (peer)-[r1:IS_RELATED]-(rel:Relationship {name: $rel_identifier})-[r2:IS_RELATED]-(node:%(node_kind)s) +WHERE peer.uuid IN $peer_uuids +AND peer <> node +AND r1.branch IN [$branch, $default_branch, $global_branch] +AND r1.status = "active" +AND r1.from <= $at +AND (r1.to IS NULL OR r1.to >= $at) +AND r2.branch IN [$branch, $default_branch, $global_branch] +AND r2.status = "active" +AND r2.from <= $at +AND (r2.to IS NULL OR r2.to >= $at) +WITH DISTINCT peer, rel, node +// -------------------- +// keep only active edges. the latest edge on the deepest branch wins. +// -------------------- +CALL (peer, rel) { + MATCH (peer)-[r:IS_RELATED]-(rel) + WHERE r.branch IN [$branch, $default_branch, $global_branch] + AND r.from <= $at + AND (r.to IS NULL OR r.to >= $at) + WITH r.status = "active" AS is_active + ORDER BY r.branch_level DESC, r.from DESC, r.status ASC + LIMIT 1 + WITH is_active + WHERE is_active = TRUE + RETURN TRUE AS peer_rel_is_live +} +CALL (rel, node) { + MATCH (rel)-[r:IS_RELATED]-(node) + WHERE r.branch IN [$branch, $default_branch, $global_branch] + AND r.from <= $at + AND (r.to IS NULL OR r.to >= $at) + WITH r.status = "active" AS is_active + ORDER BY r.branch_level DESC, r.from DESC, r.status ASC + LIMIT 1 + WITH is_active + WHERE is_active = TRUE + RETURN TRUE AS rel_node_is_live +} + """ % { + "node_kind": self.node_kind, + } + self.add_to_query(query=query) + self.return_labels = ["DISTINCT node.uuid AS node_uuid"] + + def get_dependent_uuids(self) -> set[str]: + return {result.get_as_type("node_uuid", return_type=str) for result in self.results} diff --git a/backend/tests/component/core/constraint_validators/test_uniqueness_dependent_resolver.py b/backend/tests/component/core/constraint_validators/test_uniqueness_dependent_resolver.py new file mode 100644 index 00000000000..88559c3dc75 --- /dev/null +++ b/backend/tests/component/core/constraint_validators/test_uniqueness_dependent_resolver.py @@ -0,0 +1,127 @@ +from infrahub.core import registry +from infrahub.core.branch import Branch +from infrahub.core.initialization import create_branch +from infrahub.core.manager import NodeManager +from infrahub.core.node import Node +from infrahub.core.validators.uniqueness.dependent_resolver import UniquenessDependentResolver +from infrahub.database import InfrahubDatabase + + +async def _owner_identifier(default_branch: Branch) -> str: + car_schema = registry.schema.get("TestCar", branch=default_branch) + return car_schema.get_relationship("owner").get_identifier() + + +class TestUniquenessDependentResolver: + async def test_resolves_nodes_referencing_changed_peers( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_volt_main: Node, + car_prius_main: Node, + car_camry_main: Node, + person_john_main: Node, + default_branch: Branch, + ) -> None: + resolver = UniquenessDependentResolver(db=db, branch=default_branch) + + dependents = await resolver.resolve( + node_kind="TestCar", + relationship_identifier=await _owner_identifier(default_branch), + peer_uuids=[person_john_main.id], + ) + + # the cars owned by john, and not those owned by anyone else + assert dependents == {car_accord_main.id, car_volt_main.id, car_prius_main.id} + assert car_camry_main.id not in dependents + + async def test_empty_peer_uuids_returns_empty( + self, db: InfrahubDatabase, car_accord_main: Node, default_branch: Branch + ) -> None: + resolver = UniquenessDependentResolver(db=db, branch=default_branch) + + dependents = await resolver.resolve( + node_kind="TestCar", + relationship_identifier=await _owner_identifier(default_branch), + peer_uuids=[], + ) + + assert dependents == set() + + async def test_covers_default_branch_changes_made_after_the_branch_forked( + self, + db: InfrahubDatabase, + car_accord_main: Node, + person_john_main: Node, + default_branch: Branch, + ) -> None: + # fork a branch, THEN add a car on the default branch after the fork + feature_branch = await create_branch(branch_name="feature-branch", db=db) + car_schema = registry.schema.get_node_schema("TestCar", branch=default_branch, duplicate=False) + latecomer_car = await Node.init(db=db, schema=car_schema, branch=default_branch) + await latecomer_car.new(db=db, name="latecomer", nbr_seats=2, is_electric=False, owner=person_john_main.id) + await latecomer_car.save(db=db) + + resolver = UniquenessDependentResolver(db=db, branch=feature_branch) + + dependents = await resolver.resolve( + node_kind="TestCar", + relationship_identifier=await _owner_identifier(default_branch), + peer_uuids=[person_john_main.id], + ) + + # validation runs against the current default branch, so a relationship added to default after + # the fork is included even though the branch's own view predates it + assert latecomer_car.id in dependents + assert car_accord_main.id in dependents + + async def test_post_fork_default_branch_deletion_excludes_when_branch_is_silent( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_prius_main: Node, + person_john_main: Node, + default_branch: Branch, + ) -> None: + # after the fork, accord is deleted on the DEFAULT branch; the input branch never touches + # it, so the default branch's latest state decides and accord is excluded + input_branch = await create_branch(branch_name="silent-branch", db=db) + accord_on_main = await NodeManager.get_one(db=db, id=car_accord_main.id, branch=default_branch) + await accord_on_main.delete(db=db) + + resolver = UniquenessDependentResolver(db=db, branch=input_branch) + + dependents = await resolver.resolve( + node_kind="TestCar", + relationship_identifier=await _owner_identifier(default_branch), + peer_uuids=[person_john_main.id], + ) + + assert car_accord_main.id not in dependents + assert car_prius_main.id in dependents + + async def test_excludes_node_whose_relationship_is_deleted_on_input_branch( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_prius_main: Node, + person_john_main: Node, + default_branch: Branch, + ) -> None: + # accord's owner relationship is active on the default branch; delete it only on the input branch + input_branch = await create_branch(branch_name="delete-branch", db=db) + accord_on_branch = await NodeManager.get_one(db=db, id=car_accord_main.id, branch=input_branch) + await accord_on_branch.delete(db=db) + + resolver = UniquenessDependentResolver(db=db, branch=input_branch) + + dependents = await resolver.resolve( + node_kind="TestCar", + relationship_identifier=await _owner_identifier(default_branch), + peer_uuids=[person_john_main.id], + ) + + # the input-branch deletion overrides the default branch (it is what the merge will + # produce), so accord is excluded; prius keeps its untouched relationship and stays + assert car_accord_main.id not in dependents + assert car_prius_main.id in dependents From d31eb521cbe83905a2c4147452e2d4fa00ec54d0 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Tue, 21 Jul 2026 18:56:03 -0700 Subject: [PATCH 05/31] add node_uuids to NodeDiffFieldSummary --- backend/infrahub/core/diff/model/path.py | 1 + 1 file changed, 1 insertion(+) diff --git a/backend/infrahub/core/diff/model/path.py b/backend/infrahub/core/diff/model/path.py index 0787154a5a8..9d0d5eb50a1 100644 --- a/backend/infrahub/core/diff/model/path.py +++ b/backend/infrahub/core/diff/model/path.py @@ -105,6 +105,7 @@ class NodeDiffFieldSummary: kind: str attribute_names: set[str] = field(default_factory=set) relationship_names: set[str] = field(default_factory=set) + node_uuids: set[str] = field(default_factory=set) @dataclass From 38e6b1c194ca92d1762903f4967690133f53a789 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 12:44:33 -0700 Subject: [PATCH 06/31] add test for uniqueness on migrated-kind schema on branch --- .../test_targeted_uniqueness_query.py | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/backend/tests/component/core/constraint_validators/test_targeted_uniqueness_query.py b/backend/tests/component/core/constraint_validators/test_targeted_uniqueness_query.py index 51780eb309c..4f4c9298a6d 100644 --- a/backend/tests/component/core/constraint_validators/test_targeted_uniqueness_query.py +++ b/backend/tests/component/core/constraint_validators/test_targeted_uniqueness_query.py @@ -2,8 +2,13 @@ from infrahub.core import registry from infrahub.core.branch import Branch +from infrahub.core.constants import SchemaPathType +from infrahub.core.initialization import create_branch from infrahub.core.manager import NodeManager +from infrahub.core.migrations.schema.node_kind_update import NodeKindUpdateMigration +from infrahub.core.migrations.shared import MigrationInput from infrahub.core.node import Node +from infrahub.core.path import SchemaPath from infrahub.core.schema import AttributeSchema, MainSchemaTypes, NodeSchema, SchemaRoot from infrahub.core.schema.basenode_schema import SchemaAttributePath from infrahub.core.validators.uniqueness.query import ( @@ -42,6 +47,27 @@ async def _update_car(db: InfrahubDatabase, branch: Branch, car_id: str, **attri await car.save(db=db) +async def _migrate_car_kind_on_branch(db: InfrahubDatabase, default_branch: Branch, branch: Branch) -> MainSchemaTypes: + """Migrate the whole TestCar kind to Test2NewCar on the branch and return the new schema. + + The migration results in multiple Node vertices with the same UUID, a case that can be difficult + to handle correctly. + """ + schema_branch = registry.schema.get_schema_branch(name=default_branch.name) + new_car_schema = schema_branch.get_node(name="TestCar") + new_car_schema.name = "NewCar" + new_car_schema.namespace = "Test2" + registry.schema.set(name="Test2NewCar", schema=new_car_schema, branch=branch.name) + migration = NodeKindUpdateMigration( + previous_node_schema=schema_branch.get(name="TestCar"), + new_node_schema=new_car_schema, + schema_path=SchemaPath(path_type=SchemaPathType.ATTRIBUTE, schema_kind="Test2NewCar", field_name="namespace"), + ) + execution_result = await migration.execute(migration_input=MigrationInput(db=db), branch=branch) + assert not execution_result.errors + return new_car_schema + + class TestTargetedUniquenessQuery: async def test_batched_mixed_colliding_and_unique( self, @@ -388,6 +414,58 @@ async def test_changed_nodes_collide_with_each_other( assert by_changed[car_accord_main.id].partner_uuids == (car_prius_main.id,) assert by_changed[car_prius_main.id].partner_uuids == (car_accord_main.id,) + async def test_same_uuid_duplicate_from_kind_migration( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_prius_main: Node, + car_camry_main: Node, + default_branch: Branch, + ) -> None: + # accord, prius, and camry all share nbr_seats=5 + migration_branch = await create_branch(db=db, branch_name="kind-migration-branch") + new_car_schema = await _migrate_car_kind_on_branch( + db=db, default_branch=default_branch, branch=migration_branch + ) + + # the branch accord moves to 9 after the migration; still 5 on default branch + migrated_accord = await NodeManager.get_one(db=db, branch=migration_branch, id=car_accord_main.id) + migrated_accord.get_attribute("nbr_seats").value = 9 + await migrated_accord.save(db=db) + + elements = [_attr_element(new_car_schema, "nbr_seats")] + + # updated value on branch is accepted as unique following the migration + violations = await _run_query(db, migration_branch, "Test2NewCar", elements, [car_accord_main.id]) + + assert violations == [] + + # camry and prius (both nbr_seats=5) collide. accord does not. + violations = await _run_query(db, migration_branch, "Test2NewCar", elements, [car_camry_main.id]) + + assert len(violations) == 1 + assert violations[0].changed_uuid == car_camry_main.id + assert violations[0].partner_uuids == (car_prius_main.id,) + assert violations[0].element_values == (5,) + + # the stale prius object is then updated on the DEFAULT branch after the migration; the + # attribute vertices are shared, so the new value must be visible through prius's live + # vertex on the migration branch and now collide with the branch-updated accord (9) + await _update_car(db, default_branch, car_prius_main.id, nbr_seats=9) + + violations = await _run_query(db, migration_branch, "Test2NewCar", elements, [car_accord_main.id]) + + assert len(violations) == 1 + assert violations[0].changed_uuid == car_accord_main.id + assert violations[0].partner_uuids == (car_prius_main.id,) + assert violations[0].element_values == (9,) + + # and camry (still 5) has lost its only partner: prius's superseded value-5 edge on the + # default branch must not still count for it + violations = await _run_query(db, migration_branch, "Test2NewCar", elements, [car_camry_main.id]) + + assert violations == [] + async def test_peer_attribute_element_rejected( self, db: InfrahubDatabase, From a14b418e3c0d2fe995a9d218d71670ea10438d00 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 16:28:59 -0700 Subject: [PATCH 07/31] refactor(backend): batch node-scoped uniqueness checks and simplify UniquenessChecker Replace the per-node targeted uniqueness path with a single batched TargetedUniquenessValidationQuery per constraint group, paged over the changed nodes. Take branch and schema_branch from the validator request rather than the constructor, and open one read-only session per change instead of per query. Update the dependency builder and the m018 migration for the new signatures, and switch the node-scoped tests to exact violation-set assertions. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../migrations/graph/m018_uniqueness_nulls.py | 4 +- .../core/validators/uniqueness/checker.py | 181 ++++++++++++++-- .../builder/constraint/schema/uniqueness.py | 2 +- .../test_uniqueness_checker.py | 5 +- .../test_uniqueness_checker_node_scoped.py | 196 ++++++++++++++++++ 5 files changed, 360 insertions(+), 28 deletions(-) create mode 100644 backend/tests/component/core/constraint_validators/test_uniqueness_checker_node_scoped.py diff --git a/backend/infrahub/core/migrations/graph/m018_uniqueness_nulls.py b/backend/infrahub/core/migrations/graph/m018_uniqueness_nulls.py index fd6261455e5..0928223c751 100644 --- a/backend/infrahub/core/migrations/graph/m018_uniqueness_nulls.py +++ b/backend/infrahub/core/migrations/graph/m018_uniqueness_nulls.py @@ -56,7 +56,9 @@ async def validate_nulls_in_uniqueness_constraints(db: InfrahubDatabase) -> Migr if not includes_optional_attr: continue - non_unique_nodes = await uniqueness_checker.check_one_schema(schema=schema) + non_unique_nodes = await uniqueness_checker.check_one_schema( + schema=schema, branch=default_branch, schema_branch=schema_branch + ) if non_unique_nodes: non_unique_nodes_by_kind[schema_kind] = non_unique_nodes diff --git a/backend/infrahub/core/validators/uniqueness/checker.py b/backend/infrahub/core/validators/uniqueness/checker.py index 70a365df045..7f51c1ac8cf 100644 --- a/backend/infrahub/core/validators/uniqueness/checker.py +++ b/backend/infrahub/core/validators/uniqueness/checker.py @@ -1,11 +1,9 @@ from __future__ import annotations import asyncio -from itertools import chain from typing import TYPE_CHECKING -from infrahub.core import registry -from infrahub.core.branch import Branch +from infrahub.core.constants import PathType from infrahub.core.path import DataPath, GroupedDataPaths from infrahub.core.schema import AttributeSchema, MainSchemaTypes, RelationshipSchema from infrahub.core.validators.uniqueness.index import UniquenessQueryResultsIndex @@ -19,13 +17,24 @@ QueryAttributePath, QueryRelationshipAttributePath, ) -from .query import NodeUniqueAttributeConstraintQuery +from .query import NodeUniqueAttributeConstraintQuery, TargetedUniquenessValidationQuery if TYPE_CHECKING: + from collections.abc import Iterator + + from infrahub.core.branch import Branch from infrahub.core.query import QueryResult + from infrahub.core.schema.basenode_schema import SchemaAttributePath + from infrahub.core.schema.schema_branch import SchemaBranch from infrahub.database import InfrahubDatabase from ..model import SchemaConstraintValidatorRequest + from .query import TargetedUniquenessViolation + + +def _chunked(items: list[str], size: int) -> Iterator[list[str]]: + for start in range(0, len(items), size): + yield items[start : start + size] def get_attribute_path_from_string( @@ -46,11 +55,14 @@ def get_attribute_path_from_string( class UniquenessChecker(ConstraintCheckerInterface): def __init__( - self, db: InfrahubDatabase, branch: Branch | str | None = None, max_concurrent_execution: int = 5 + self, + db: InfrahubDatabase, + max_concurrent_execution: int = 5, + query_batch_size: int = 500, ) -> None: self.db = db - self.branch = branch self.semaphore = asyncio.Semaphore(max_concurrent_execution) + self.query_batch_size = query_batch_size @property def name(self) -> str: @@ -59,19 +71,140 @@ def name(self) -> str: def supports(self, request: SchemaConstraintValidatorRequest) -> bool: return request.constraint_name == self.name - async def get_branch(self) -> Branch: - if not isinstance(self.branch, Branch): - self.branch = await registry.get_branch(db=self.db, branch=self.branch) - return self.branch - async def check(self, request: SchemaConstraintValidatorRequest) -> list[GroupedDataPaths]: - schema_objects = [request.node_schema] - non_unique_nodes_lists = await asyncio.gather(*[self.check_one_schema(schema) for schema in schema_objects]) + if request.node_uuids is None: + non_unique_nodes = await self.check_one_schema( + schema=request.node_schema, branch=request.branch, schema_branch=request.schema_branch + ) + grouped_data_paths = GroupedDataPaths() + for non_unique_node in non_unique_nodes: + self.generate_data_paths(non_unique_node, grouped_data_paths) + return [grouped_data_paths] + + return [ + await self._check_targeted( + schema=request.node_schema, + node_uuids=request.node_uuids, + branch=request.branch, + schema_branch=request.schema_branch, + ) + ] + + async def _check_targeted( + self, + schema: MainSchemaTypes, + node_uuids: list[str], + branch: Branch, + schema_branch: SchemaBranch, + ) -> GroupedDataPaths: + """Validate uniqueness for only the changed nodes, one batched query per constraint group. + + Each query resolves the changed nodes' current constraint values and probes the whole + population for other nodes sharing the full value tuple, so a collision with an untouched + peer still surfaces. Only the changed nodes are queried, so the work is bounded by the size + of the change rather than the kind's population, and the changed set is paged so a very + large change does not travel in a single query. + + All queries run in one read-only session so reads route to a replica and the session is + opened once for the whole change rather than per query. + """ + constraint_paths = schema.get_unique_constraint_schema_attribute_paths(schema_branch=schema_branch) grouped_data_paths = GroupedDataPaths() - for non_unique_node in chain(*non_unique_nodes_lists): - self.generate_data_paths(non_unique_node, grouped_data_paths) - return [grouped_data_paths] + if not constraint_paths: + return grouped_data_paths + + seen_data_paths: set[DataPath] = set() + async with self.db.start_session(read_only=True) as session_db: + for constraint_path in constraint_paths: + constraint_elements = constraint_path.attributes_paths + for window in _chunked(node_uuids, self.query_batch_size): + data_paths = await self._query_group_violations( + session_db=session_db, + schema=schema, + branch=branch, + constraint_elements=constraint_elements, + node_uuids=window, + ) + for data_path in data_paths: + if data_path in seen_data_paths: + continue + seen_data_paths.add(data_path) + grouped_data_paths.add_data_path( + data_path, grouping_key=f"{schema.kind}/{data_path.field_name}/{data_path.value}" + ) + return grouped_data_paths + + async def _query_group_violations( + self, + session_db: InfrahubDatabase, + schema: MainSchemaTypes, + branch: Branch, + constraint_elements: list[SchemaAttributePath], + node_uuids: list[str], + ) -> list[DataPath]: + """Run one constraint group's targeted query for a window of changed nodes and expand it.""" + query = await TargetedUniquenessValidationQuery.init( + db=session_db, + branch=branch, + kind=schema.kind, + constraint_elements=constraint_elements, + node_uuids=node_uuids, + ) + await query.execute(db=session_db) + + data_paths: list[DataPath] = [] + for violation in query.get_data(): + data_paths.extend( + self._violation_to_data_paths( + schema=schema, + constraint_elements=constraint_elements, + violation=violation, + branch_name=branch.name, + ) + ) + return data_paths + + def _violation_to_data_paths( + self, + schema: MainSchemaTypes, + constraint_elements: list[SchemaAttributePath], + violation: TargetedUniquenessViolation, + branch_name: str, + ) -> list[DataPath]: + """Expand one violation into a data path per involved node and constraint element. + + The changed node and every partner share the full value tuple, so each element's value is + emitted for the changed node and all its partners. A relationship element carries the shared + peer's id; an attribute element carries the shared attribute value. + """ + involved_node_ids = [violation.changed_uuid, *violation.partner_uuids] + data_paths: list[DataPath] = [] + for element, value in zip(constraint_elements, violation.element_values, strict=True): + if element.relationship_schema is not None: + field_name: str | None = element.relationship_schema.name + property_name = "id" + path_type = PathType.RELATIONSHIP_ONE + peer_id: str | None = value + else: + field_name = element.active_attribute_schema.name + property_name = "value" + path_type = PathType.ATTRIBUTE + peer_id = None + for node_id in involved_node_ids: + data_paths.append( + DataPath( + branch=branch_name, + path_type=path_type, + node_id=node_id, + kind=schema.kind, + field_name=field_name, + property_name=property_name, + value=value, + peer_id=peer_id, + ) + ) + return data_paths async def build_query_request(self, schema: MainSchemaTypes) -> NodeUniquenessQueryRequest: unique_attr_paths = { @@ -112,28 +245,30 @@ async def build_query_request(self, schema: MainSchemaTypes) -> NodeUniquenessQu async def check_one_schema( self, schema: MainSchemaTypes, + branch: Branch, + schema_branch: SchemaBranch, ) -> list[NonUniqueNode]: query_request = await self.build_query_request(schema) if not query_request: return [] - query = await NodeUniqueAttributeConstraintQuery.init( - db=self.db, branch=await self.get_branch(), query_request=query_request - ) + query = await NodeUniqueAttributeConstraintQuery.init(db=self.db, branch=branch, query_request=query_request) async with self.semaphore: async with self.db.start_session(read_only=True) as db: query_results = await query.execute(db=db) - return await self._parse_results(schema=schema, query_results=query_results.results) + return await self._parse_results( + schema=schema, query_results=query_results.results, schema_branch=schema_branch + ) - async def _parse_results(self, schema: MainSchemaTypes, query_results: list[QueryResult]) -> list[NonUniqueNode]: + async def _parse_results( + self, schema: MainSchemaTypes, query_results: list[QueryResult], schema_branch: SchemaBranch + ) -> list[NonUniqueNode]: relationship_schema_by_identifier = {rel.identifier: rel for rel in schema.relationships} all_non_unique_nodes: list[NonUniqueNode] = [] results_index = UniquenessQueryResultsIndex(query_results=query_results) - branch = await self.get_branch() - schema_branch = self.db.schema.get_schema_branch(name=branch.name) uniqueness_constraint_paths = schema.get_unique_constraint_schema_attribute_paths(schema_branch=schema_branch) for uniqueness_constraint_path in uniqueness_constraint_paths: non_unique_nodes_by_id: dict[str, NonUniqueNode] = {} diff --git a/backend/infrahub/dependencies/builder/constraint/schema/uniqueness.py b/backend/infrahub/dependencies/builder/constraint/schema/uniqueness.py index 26cd0c3cecb..bb89d7f0333 100644 --- a/backend/infrahub/dependencies/builder/constraint/schema/uniqueness.py +++ b/backend/infrahub/dependencies/builder/constraint/schema/uniqueness.py @@ -5,4 +5,4 @@ class SchemaUniquenessConstraintDependency(DependencyBuilder[UniquenessChecker]): @classmethod def build(cls, context: DependencyBuilderContext) -> UniquenessChecker: - return UniquenessChecker(db=context.db, branch=context.branch) + return UniquenessChecker(db=context.db) diff --git a/backend/tests/component/core/constraint_validators/test_uniqueness_checker.py b/backend/tests/component/core/constraint_validators/test_uniqueness_checker.py index 104e83b76e9..d7a55c34920 100644 --- a/backend/tests/component/core/constraint_validators/test_uniqueness_checker.py +++ b/backend/tests/component/core/constraint_validators/test_uniqueness_checker.py @@ -8,7 +8,6 @@ from infrahub.core.path import DataPath, SchemaPath from infrahub.core.schema import MainSchemaTypes, SchemaRoot from infrahub.core.schema.relationship_schema import RelationshipSchema -from infrahub.core.schema.schema_branch import SchemaBranch from infrahub.core.validators.model import SchemaConstraintValidatorRequest from infrahub.core.validators.uniqueness.checker import UniquenessChecker from infrahub.database import InfrahubDatabase @@ -16,14 +15,14 @@ class TestUniquenessChecker: async def __call_system_under_test(self, db: InfrahubDatabase, branch: Branch, schema: MainSchemaTypes): - checker = UniquenessChecker(db, branch) + checker = UniquenessChecker(db) schema_path = SchemaPath(path_type=SchemaPathType.NODE, schema_kind=schema.kind) request = SchemaConstraintValidatorRequest( branch=branch, constraint_name="node.uniqueness_constraints.update", node_schema=schema, schema_path=schema_path, - schema_branch=SchemaBranch(cache={}, name="test"), + schema_branch=db.schema.get_schema_branch(name=branch.name), ) return await checker.check(request) diff --git a/backend/tests/component/core/constraint_validators/test_uniqueness_checker_node_scoped.py b/backend/tests/component/core/constraint_validators/test_uniqueness_checker_node_scoped.py new file mode 100644 index 00000000000..96cd095740f --- /dev/null +++ b/backend/tests/component/core/constraint_validators/test_uniqueness_checker_node_scoped.py @@ -0,0 +1,196 @@ +import pytest + +from infrahub.core import registry +from infrahub.core.branch import Branch +from infrahub.core.constants import SchemaPathType +from infrahub.core.diff.model.path import NodeDiffFieldSummary +from infrahub.core.manager import NodeManager +from infrahub.core.node import Node +from infrahub.core.path import SchemaPath +from infrahub.core.schema import SchemaRoot +from infrahub.core.schema.node_schema import NodeSchema +from infrahub.core.validators.determiner import build_constraint_validator_determiner +from infrahub.core.validators.model import SchemaConstraintValidatorRequest +from infrahub.core.validators.uniqueness.checker import UniquenessChecker +from infrahub.database import InfrahubDatabase + + +async def _run_checker( + db: InfrahubDatabase, branch: Branch, schema: NodeSchema, node_uuids: list[str] | None +) -> set[tuple[str, str | None, str | None]]: + checker = UniquenessChecker(db) + request = SchemaConstraintValidatorRequest( + branch=branch, + constraint_name="node.uniqueness_constraints.update", + node_schema=schema, + schema_path=SchemaPath(path_type=SchemaPathType.NODE, schema_kind=schema.kind), + schema_branch=db.schema.get_schema_branch(name=branch.name), + node_uuids=node_uuids, + ) + grouped_data_paths = await checker.check(request) + assert len(grouped_data_paths) == 1 + return {(path.node_id, path.field_name, str(path.value)) for path in grouped_data_paths[0].get_all_data_paths()} + + +def _make_nbr_seats_unique(branch: Branch) -> NodeSchema: + schema = registry.schema.get_node_schema("TestCar", branch=branch) + schema.get_attribute("nbr_seats").unique = True + registry.schema.register_schema(schema=SchemaRoot(nodes=[schema]), branch=branch.name) + return registry.schema.get_node_schema(name="TestCar", branch=branch, duplicate=False) + + +class TestUniquenessCheckerNodeScoped: + async def test_targeted_change_finds_collision_with_untouched_peer( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_prius_main: Node, + car_volt_main: Node, + branch: Branch, + ) -> None: + # accord and prius share nbr_seats=5; only accord is "changed" (passed via node_uuids) + schema = _make_nbr_seats_unique(branch) + + violations = await _run_checker(db, branch, schema, node_uuids=[car_accord_main.id]) + + # volt is not included (nbr_seats=4) + assert violations == { + (car_accord_main.id, "nbr_seats", "5"), + (car_prius_main.id, "nbr_seats", "5"), + } + + async def test_targeted_change_without_collision_reports_nothing( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_prius_main: Node, + car_volt_main: Node, + branch: Branch, + ) -> None: + # volt's nbr_seats is unique among the fixtures, so validating only volt finds no collision + schema = _make_nbr_seats_unique(branch) + + violations = await _run_checker(db, branch, schema, node_uuids=[car_volt_main.id]) + + assert violations == set() + + async def test_targeted_mutual_collision_between_changed_nodes( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_prius_main: Node, + car_volt_main: Node, + car_yaris_main: Node, + branch: Branch, + ) -> None: + # make two previously-distinct cars collide, then validate both as changed nodes + yaris_main = await NodeManager.get_one(db=db, id=car_yaris_main.id) + yaris_main.get_attribute("nbr_seats").value = 9 + await yaris_main.save(db=db) + volt_branch = await NodeManager.get_one(db=db, branch=branch, id=car_volt_main.id) + volt_branch.get_attribute("nbr_seats").value = 9 + await volt_branch.save(db=db) + + schema = _make_nbr_seats_unique(branch) + + violations = await _run_checker(db, branch, schema, node_uuids=[car_volt_main.id, car_yaris_main.id]) + + assert violations == { + (car_volt_main.id, "nbr_seats", "9"), + (car_yaris_main.id, "nbr_seats", "9"), + } + + async def test_full_scan_when_node_uuids_is_none( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_prius_main: Node, + branch: Branch, + ) -> None: + # node_uuids=None preserves the full-population scan used for a newly added/broadened constraint + schema = _make_nbr_seats_unique(branch) + + violations = await _run_checker(db, branch, schema, node_uuids=None) + + assert violations == { + (car_accord_main.id, "nbr_seats", "5"), + (car_prius_main.id, "nbr_seats", "5"), + } + + @pytest.mark.xfail( + reason="peer-attribute uniqueness (owner__height) is not supported by the batched targeted " + "query; such constraints are rejected at schema load, so this path is unreachable in a " + "valid schema. Full-population validation still covers it.", + raises=ValueError, + strict=True, + ) + async def test_targeted_cross_kind_peer_attribute_collision( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_camry_main: Node, + person_john_main: Node, + person_jane_main: Node, + branch: Branch, + ) -> None: + # accord is owned by John and camry by Jane, but both owners have height 180. A + # uniqueness on the peer's attribute value (not the peer's identity) must flag them as + # colliding even though the changed node (accord) points at a different peer than camry. + schema = registry.schema.get_node_schema("TestCar", branch=branch) + schema.uniqueness_constraints = [["owner__height"]] + registry.schema.register_schema(schema=SchemaRoot(nodes=[schema]), branch=branch.name) + synced_schema = registry.schema.get_node_schema(name="TestCar", branch=branch, duplicate=False) + + violations = await _run_checker(db, branch, synced_schema, node_uuids=[car_accord_main.id]) + + # the violation is reported against the relationship path, carrying the peer's value + assert (car_accord_main.id, "owner", "180") in violations + assert (car_camry_main.id, "owner", "180") in violations + + @pytest.mark.xfail( + reason="peer-attribute uniqueness (owner__height) is not supported by the batched targeted " + "query; the determiner still resolves the cross-kind change, but the checker rejects the " + "peer-attribute constraint. Such constraints cannot exist in a valid schema.", + raises=ValueError, + strict=True, + ) + async def test_cross_kind_peer_change_resolves_and_detects_end_to_end( + self, + db: InfrahubDatabase, + car_accord_main: Node, + car_camry_main: Node, + person_john_main: Node, + person_jane_main: Node, + branch: Branch, + ) -> None: + # accord->john and camry->jane, both owners height 180, uniqueness on the peer's height. + # A change to john's height (the peer) leaves accord itself absent from the diff; the whole + # chain (determiner cross-kind descriptor -> real resolver -> checker) must still flag it. + car_schema = registry.schema.get_node_schema("TestCar", branch=branch) + car_schema.uniqueness_constraints = [["owner__height"]] + registry.schema.register_schema(schema=SchemaRoot(nodes=[car_schema]), branch=branch.name) + synced_schema = registry.schema.get_node_schema(name="TestCar", branch=branch, duplicate=False) + + determiner = build_constraint_validator_determiner( + db=db, branch=branch, schema_branch=registry.schema.get_schema_branch(name=branch.name) + ) + person_change = NodeDiffFieldSummary( + kind="TestPerson", attribute_names={"height"}, node_uuids={person_john_main.id} + ) + + constraints = await determiner.get_constraints(node_diffs=[person_change]) + + car_constraint = next( + c + for c in constraints + if c.constraint_name == "node.uniqueness_constraints.update" and c.path.schema_kind == "TestCar" + ) + # the resolver mapped the changed person to the car that owns it (accord), not camry + assert car_constraint.node_uuids == [car_accord_main.id] + + violations = await _run_checker(db, branch, synced_schema, node_uuids=car_constraint.node_uuids) + + violating_ids = {node_id for node_id, _, _ in violations} + assert car_accord_main.id in violating_ids + # camry is untouched and points at a different owner, but shares the owner height value + assert car_camry_main.id in violating_ids From 81908cbdeefce094ee2d5f91ba715fe07c6d3708 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 16:49:58 -0700 Subject: [PATCH 08/31] retrieve UUIds in NodeDiffFieldSummary --- backend/infrahub/core/diff/query/field_summary.py | 14 ++++++++++---- .../core/diff/repository/test_diff_repository.py | 1 + 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/backend/infrahub/core/diff/query/field_summary.py b/backend/infrahub/core/diff/query/field_summary.py index 8a19c793557..d2e31a616ff 100644 --- a/backend/infrahub/core/diff/query/field_summary.py +++ b/backend/infrahub/core/diff/query/field_summary.py @@ -42,7 +42,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # noqa AND (diff_root.uuid = $diff_id OR $diff_id IS NULL) OPTIONAL MATCH (diff_root)-[:DIFF_HAS_NODE]->(n:DiffNode) WHERE n.action <> $unchanged_str - WITH DISTINCT n.kind AS kind + WITH n.kind AS kind, collect(DISTINCT n.uuid) AS node_uuids CALL (kind) { OPTIONAL MATCH (n:DiffNode {kind: kind})-[:DIFF_HAS_ATTRIBUTE]->(a:DiffAttribute) WHERE n.action <> $unchanged_str @@ -50,7 +50,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # noqa WITH DISTINCT a.name AS attr_name RETURN collect(attr_name) AS attr_names } - WITH kind, attr_names + WITH kind, node_uuids, attr_names CALL (kind) { OPTIONAL MATCH (n:DiffNode {kind: kind})-[:DIFF_HAS_RELATIONSHIP]->(r:DiffRelationship) WHERE n.action <> $unchanged_str @@ -61,16 +61,22 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # noqa """ self.add_to_query(query=query) self.order_by = ["kind"] - self.return_labels = ["kind", "attr_names", "rel_names"] + self.return_labels = ["kind", "node_uuids", "attr_names", "rel_names"] async def get_field_summaries(self) -> list[NodeDiffFieldSummary]: field_summaries = [] for result in self.get_results(): kind = result.get_as_type(label="kind", return_type=str) + node_uuids = result.get_as_type(label="node_uuids", return_type=list[str]) attr_names = result.get_as_type(label="attr_names", return_type=list[str]) rel_names = result.get_as_type(label="rel_names", return_type=list[str]) if attr_names or rel_names: field_summaries.append( - NodeDiffFieldSummary(kind=kind, attribute_names=set(attr_names), relationship_names=set(rel_names)) + NodeDiffFieldSummary( + kind=kind, + attribute_names=set(attr_names), + relationship_names=set(rel_names), + node_uuids=set(node_uuids), + ) ) return field_summaries diff --git a/backend/tests/component/core/diff/repository/test_diff_repository.py b/backend/tests/component/core/diff/repository/test_diff_repository.py index 67b6d8ea664..35b12c719bc 100644 --- a/backend/tests/component/core/diff/repository/test_diff_repository.py +++ b/backend/tests/component/core/diff/repository/test_diff_repository.py @@ -630,6 +630,7 @@ async def test_get_node_field_summaries(self, diff_repository: DiffRepository) - if node.kind not in expected_map: expected_map[node.kind] = NodeDiffFieldSummary(kind=node.kind) field_summary = expected_map[node.kind] + field_summary.node_uuids.add(node.uuid) attr_names = {a.name for a in node.attributes if a.action is not DiffAction.UNCHANGED} field_summary.attribute_names.update(attr_names) rel_names = {r.name for r in node.relationships if r.action is not DiffAction.UNCHANGED} From f096473275661137eb4f4fa156107c2a79a49ad6 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 17:08:24 -0700 Subject: [PATCH 09/31] add UniquenessConstraintScoper --- .../core/validators/uniqueness/scope.py | 162 ++++++++++++++++++ .../test_uniqueness_scope.py | 118 +++++++++++++ 2 files changed, 280 insertions(+) create mode 100644 backend/infrahub/core/validators/uniqueness/scope.py create mode 100644 backend/tests/component/core/constraint_validators/test_uniqueness_scope.py diff --git a/backend/infrahub/core/validators/uniqueness/scope.py b/backend/infrahub/core/validators/uniqueness/scope.py new file mode 100644 index 00000000000..7e5f1e14d4d --- /dev/null +++ b/backend/infrahub/core/validators/uniqueness/scope.py @@ -0,0 +1,162 @@ +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import TYPE_CHECKING + +from infrahub.core.schema import AttributePathParsingError, GenericSchema +from infrahub.log import get_logger + +LOG = get_logger(__name__) + +if TYPE_CHECKING: + from infrahub.core.schema import MainSchemaTypes + from infrahub.core.schema.schema_branch import SchemaBranch + from infrahub.core.validators.node_diff_index import NodeDiffIndex + + from .dependent_resolver import UniquenessDependentResolverInterface + + +@dataclass +class CrossKindPeerChange: + """A change to a peer kind that can break the uniqueness of the kind pointing at it. + + Reached from a constraint path such as "owner__name": the peer kind's attribute changed, so the + nodes of the constrained kind related through ``relationship_identifier`` to those peers must be + resolved and re-validated. ``changed_peer_uuids`` are uuids of the PEER kind, not the constrained + kind. + """ + + relationship_identifier: str + changed_peer_uuids: set[str] + + +@dataclass +class UniquenessScopeForKind: + """How a data diff implicates a single kind's uniqueness.""" + + # whether any diffed field participates in the kind's uniqueness (if False, no need to validate) + requires_validation: bool = False + # uuids of directly-changed nodes of the kind whose changed field participates in its uniqueness + object_uuids: set[str] = field(default_factory=set) + # peer-kind changes reached across a relationship, still to be resolved into this kind's nodes + cross_kind_peer_changes: list[CrossKindPeerChange] = field(default_factory=list) + + +class UniquenessConstraintScoper: + """Identify which nodes a data change can make violate a kind's uniqueness. + + Directly changed nodes of the constrained kind are collected from the diff; when a constraint + reads a peer's attribute (e.g. "owner__name") the changed peers are handed to the dependent + resolver to find the constrained nodes pointing at them. The affected set is returned, or None + when validation must fall back to the full population because the affected nodes cannot be + identified. + """ + + def __init__( + self, + schema_branch: SchemaBranch, + dependent_resolver: UniquenessDependentResolverInterface, + node_diff_index: NodeDiffIndex, + ) -> None: + self.schema_branch = schema_branch + self.dependent_resolver = dependent_resolver + self.node_diff_index = node_diff_index + # scopes are recomputed for the same kind across the trigger check and the uuid resolution; + # the schema branch and node-diff index are fixed for this scoper's lifetime, so cache them + self._scope_cache: dict[str, UniquenessScopeForKind] = {} + + def requires_validation(self, schema: MainSchemaTypes) -> bool: + return self._scope(schema=schema).requires_validation + + async def affected_node_uuids(self, schema: MainSchemaTypes) -> list[str] | None: + scope = self._scope(schema=schema) + node_uuids = set(scope.object_uuids) + for peer_change in scope.cross_kind_peer_changes: + if not peer_change.changed_peer_uuids: + # the changed peers are not identified, so the constrained nodes cannot be either + return None + node_uuids |= await self.dependent_resolver.resolve( + node_kind=schema.kind, + relationship_identifier=peer_change.relationship_identifier, + peer_uuids=sorted(peer_change.changed_peer_uuids), + ) + if not node_uuids: + return None + return sorted(node_uuids) + + def _diffed_kinds_with_field(self, schema: MainSchemaTypes, field_name: str, is_relationship: bool) -> set[str]: + """Return the diffed kinds where `field_name` changed, on `schema` or a kind that inherits it. + + An inherited field keeps its name on the implementing kind, so a generic-level constraint + is implicated when an implementation's copy of the field is what changed in the diff. A + generic knows its implementing kinds through `used_by`; the diff check then keeps only the + kinds actually present in the diff. + """ + kinds = {schema.kind} + if isinstance(schema, GenericSchema): + kinds.update(schema.used_by) + check = ( + self.node_diff_index.has_relationship_diff if is_relationship else self.node_diff_index.has_attribute_diff + ) + return {kind for kind in kinds if check(kind=kind, name=field_name)} + + def _scope(self, schema: MainSchemaTypes) -> UniquenessScopeForKind: + cached = self._scope_cache.get(schema.kind) + if cached is None: + cached = self._compute_scope(schema=schema) + self._scope_cache[schema.kind] = cached + return cached + + def _compute_scope(self, schema: MainSchemaTypes) -> UniquenessScopeForKind: + """Compute why and how a data change implicates `schema`'s uniqueness. + + Uniqueness spans single unique attributes and multi-field constraint groups. A group + element such as "owner__name" reads an attribute of a related peer, so a data change on + the peer kind can create a violation without any change to the constrained kind itself: + that case yields a cross-kind peer change. Directly changed nodes of the constrained kind + are collected as object uuids. + """ + scope = UniquenessScopeForKind() + for attribute_schema in schema.unique_attributes: + kinds = self._diffed_kinds_with_field( + schema=schema, field_name=attribute_schema.name, is_relationship=False + ) + if kinds: + scope.requires_validation = True + scope.object_uuids |= self.node_diff_index.uuids_for_kinds(kinds) + for constraint_group in schema.uniqueness_constraints or []: + for constraint_path in constraint_group: + try: + schema_path = schema.parse_schema_path(path=constraint_path, schema=self.schema_branch) + except AttributePathParsingError: + LOG.warning(f"Cannot parse {schema.kind}.uniqueness_constraints element '{constraint_path}'") + continue + if schema_path.relationship_schema is not None: + rel_kinds = self._diffed_kinds_with_field( + schema=schema, field_name=schema_path.relationship_schema.name, is_relationship=True + ) + if rel_kinds: + scope.requires_validation = True + scope.object_uuids |= self.node_diff_index.uuids_for_kinds(rel_kinds) + if schema_path.attribute_schema is not None and schema_path.related_schema is not None: + peer_kinds = self._diffed_kinds_with_field( + schema=schema_path.related_schema, + field_name=schema_path.attribute_schema.name, + is_relationship=False, + ) + if peer_kinds: + scope.requires_validation = True + scope.cross_kind_peer_changes.append( + CrossKindPeerChange( + relationship_identifier=schema_path.relationship_schema.get_identifier(), + changed_peer_uuids=self.node_diff_index.uuids_for_kinds(peer_kinds), + ) + ) + elif schema_path.attribute_schema is not None: + attr_kinds = self._diffed_kinds_with_field( + schema=schema, field_name=schema_path.attribute_schema.name, is_relationship=False + ) + if attr_kinds: + scope.requires_validation = True + scope.object_uuids |= self.node_diff_index.uuids_for_kinds(attr_kinds) + return scope diff --git a/backend/tests/component/core/constraint_validators/test_uniqueness_scope.py b/backend/tests/component/core/constraint_validators/test_uniqueness_scope.py new file mode 100644 index 00000000000..cb1570bbfa4 --- /dev/null +++ b/backend/tests/component/core/constraint_validators/test_uniqueness_scope.py @@ -0,0 +1,118 @@ +from infrahub.core import registry +from infrahub.core.branch import Branch +from infrahub.core.diff.model.path import NodeDiffFieldSummary +from infrahub.core.schema import SchemaRoot +from infrahub.core.schema.schema_branch import SchemaBranch +from infrahub.core.validators.node_diff_index import NodeDiffIndex +from infrahub.core.validators.uniqueness.scope import UniquenessConstraintScoper + + +class _RecordingResolver: + """Test double returning a fixed dependent set and capturing what it was asked to resolve.""" + + def __init__(self, dependents: set[str]) -> None: + self.dependents = dependents + self.calls: list[tuple[str, str, list[str]]] = [] + + async def resolve(self, node_kind: str, relationship_identifier: str, peer_uuids: list[str]) -> set[str]: + self.calls.append((node_kind, relationship_identifier, peer_uuids)) + return set(self.dependents) + + +def _scoper(schema_branch: SchemaBranch, resolver: _RecordingResolver, node_diffs: list[NodeDiffFieldSummary]): + node_diff_index = NodeDiffIndex() + node_diff_index.initialize(node_diffs) + return UniquenessConstraintScoper( + schema_branch=schema_branch, dependent_resolver=resolver, node_diff_index=node_diff_index + ) + + +class TestUniquenessConstraintScoper: + async def test_same_kind_change_scopes_to_changed_nodes( + self, car_person_schema: SchemaBranch, default_branch: Branch + ) -> None: + schema_branch = registry.schema.get_schema_branch(name=default_branch.name) + scoper = _scoper( + schema_branch, + _RecordingResolver(dependents=set()), + [NodeDiffFieldSummary(kind="TestPerson", attribute_names={"name"}, node_uuids={"person-1", "person-2"})], + ) + person_schema = schema_branch.get(name="TestPerson") + + assert scoper.requires_validation(schema=person_schema) is True + assert await scoper.affected_node_uuids(schema=person_schema) == ["person-1", "person-2"] + + async def test_triggered_without_node_uuids_falls_back_to_full_scan( + self, car_person_schema: SchemaBranch, default_branch: Branch + ) -> None: + schema_branch = registry.schema.get_schema_branch(name=default_branch.name) + scoper = _scoper( + schema_branch, + _RecordingResolver(dependents=set()), + [NodeDiffFieldSummary(kind="TestPerson", attribute_names={"name"})], + ) + person_schema = schema_branch.get(name="TestPerson") + + assert scoper.requires_validation(schema=person_schema) is True + assert await scoper.affected_node_uuids(schema=person_schema) is None + + async def test_unrelated_field_change_does_not_trigger( + self, car_person_schema: SchemaBranch, default_branch: Branch + ) -> None: + schema_branch = registry.schema.get_schema_branch(name=default_branch.name) + # height is not part of any TestPerson uniqueness constraint + scoper = _scoper( + schema_branch, + _RecordingResolver(dependents=set()), + [NodeDiffFieldSummary(kind="TestPerson", attribute_names={"height"}, node_uuids={"person-1"})], + ) + person_schema = schema_branch.get(name="TestPerson") + + assert scoper.requires_validation(schema=person_schema) is False + assert await scoper.affected_node_uuids(schema=person_schema) is None + + async def test_cross_kind_peer_change_resolves_dependents( + self, car_person_schema: SchemaBranch, default_branch: Branch + ) -> None: + schema_branch = registry.schema.get_schema_branch(name=default_branch.name) + car_schema = schema_branch.get_node(name="TestCar") + car_schema.uniqueness_constraints = [["owner__name"]] + registry.schema.register_schema(schema=SchemaRoot(nodes=[car_schema]), branch=default_branch.name) + schema_branch = registry.schema.get_schema_branch(name=default_branch.name) + + resolver = _RecordingResolver(dependents={"car-1", "car-2"}) + # a change to the peer kind's name, with no change to TestCar itself + scoper = _scoper( + schema_branch, + resolver, + [NodeDiffFieldSummary(kind="TestPerson", attribute_names={"name"}, node_uuids={"person-1"})], + ) + car_schema = schema_branch.get(name="TestCar") + + assert scoper.requires_validation(schema=car_schema) is True + assert await scoper.affected_node_uuids(schema=car_schema) == ["car-1", "car-2"] + # the peer change is routed to the resolver as a single call carrying the changed peer uuids + owner_identifier = car_schema.get_relationship(name="owner").get_identifier() + assert resolver.calls == [("TestCar", owner_identifier, ["person-1"])] + + async def test_cross_kind_without_known_peer_uuids_falls_back_to_full_scan( + self, car_person_schema: SchemaBranch, default_branch: Branch + ) -> None: + schema_branch = registry.schema.get_schema_branch(name=default_branch.name) + car_schema = schema_branch.get_node(name="TestCar") + car_schema.uniqueness_constraints = [["owner__name"]] + registry.schema.register_schema(schema=SchemaRoot(nodes=[car_schema]), branch=default_branch.name) + schema_branch = registry.schema.get_schema_branch(name=default_branch.name) + + resolver = _RecordingResolver(dependents={"car-1"}) + # the peer changed but its node uuids are unknown, so the dependents cannot be resolved + scoper = _scoper( + schema_branch, + resolver, + [NodeDiffFieldSummary(kind="TestPerson", attribute_names={"name"})], + ) + car_schema = schema_branch.get(name="TestCar") + + assert scoper.requires_validation(schema=car_schema) is True + assert await scoper.affected_node_uuids(schema=car_schema) is None + assert not resolver.calls # resolver is never called when the peer uuids are unknown From 0b49394835c330599e4c91291a574c30244b42c6 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 17:15:48 -0700 Subject: [PATCH 10/31] update ConstraintValidatorDeterminer with new components --- .../infrahub/core/validators/determiner.py | 143 +++++++----------- .../constraint_validators/test_determiner.py | 47 ++++-- 2 files changed, 85 insertions(+), 105 deletions(-) diff --git a/backend/infrahub/core/validators/determiner.py b/backend/infrahub/core/validators/determiner.py index eb154c3dc78..5706dc6e723 100644 --- a/backend/infrahub/core/validators/determiner.py +++ b/backend/infrahub/core/validators/determiner.py @@ -1,60 +1,55 @@ +from __future__ import annotations + from typing import TYPE_CHECKING, Any from infrahub.core.constants import RelationshipKind, SchemaPathType from infrahub.core.constants.schema import UpdateSupport -from infrahub.core.diff.model.path import NodeDiffFieldSummary from infrahub.core.models import SchemaUpdateConstraintInfo from infrahub.core.path import SchemaPath -from infrahub.core.schema import AttributePathParsingError, AttributeSchema, MainSchemaTypes from infrahub.core.schema.attribute_parameters import AttributeParameters from infrahub.core.schema.relationship_schema import RelationshipSchema -from infrahub.core.schema.schema_branch import SchemaBranch from infrahub.core.validators import CONSTRAINT_VALIDATOR_MAP +from infrahub.core.validators.node_diff_index import NodeDiffIndex +from infrahub.core.validators.uniqueness.dependent_resolver import UniquenessDependentResolver +from infrahub.core.validators.uniqueness.scope import UniquenessConstraintScoper from infrahub.exceptions import SchemaNotFoundError from infrahub.log import get_logger if TYPE_CHECKING: from pydantic.fields import FieldInfo + from infrahub.core.branch import Branch + from infrahub.core.diff.model.path import NodeDiffFieldSummary + from infrahub.core.schema import AttributeSchema, MainSchemaTypes + from infrahub.core.schema.schema_branch import SchemaBranch + from infrahub.core.timestamp import Timestamp + from infrahub.database import InfrahubDatabase + LOG = get_logger(__name__) class ConstraintValidatorDeterminer: - def __init__(self, schema_branch: SchemaBranch) -> None: + def __init__( + self, + schema_branch: SchemaBranch, + node_diff_index: NodeDiffIndex, + uniqueness_scoper: UniquenessConstraintScoper, + ) -> None: self.schema_branch = schema_branch - self._node_kinds: set[str] = set() - self._attribute_element_map: dict[str, set[str]] = {} - self._relationship_element_map: dict[str, set[str]] = {} - - def _index_node_diffs(self, node_diffs: list[NodeDiffFieldSummary]) -> None: - for node_diff in node_diffs: - self._node_kinds.add(node_diff.kind) - if node_diff.kind not in self._attribute_element_map: - self._attribute_element_map[node_diff.kind] = set() - for attribute_name in node_diff.attribute_names: - self._attribute_element_map[node_diff.kind].add(attribute_name) - if node_diff.kind not in self._relationship_element_map: - self._relationship_element_map[node_diff.kind] = set() - for relationship_name in node_diff.relationship_names: - self._relationship_element_map[node_diff.kind].add(relationship_name) - - def _has_attribute_diff(self, kind: str, name: str) -> bool: - return name in self._attribute_element_map.get(kind, set()) - - def _has_relationship_diff(self, kind: str, name: str) -> bool: - return name in self._relationship_element_map.get(kind, set()) + self.node_diff_index = node_diff_index + self.uniqueness_scoper = uniqueness_scoper async def get_constraints( self, node_diffs: list[NodeDiffFieldSummary], filter_invalid: bool = True ) -> list[SchemaUpdateConstraintInfo]: - self._index_node_diffs(node_diffs) + self.node_diff_index.initialize(node_diffs) constraints: list[SchemaUpdateConstraintInfo] = [] if not node_diffs: return constraints constraints.extend(await self._get_property_constraints_for_impacted_kinds()) - for kind in self._node_kinds: + for kind in self.node_diff_index.kinds: schema = self._get_schema_or_none(kind=kind) if schema is None: # a branch can hold data changes for a kind whose schema it also deletes @@ -97,7 +92,7 @@ def _get_impacted_kinds(self) -> set[str]: generic-level uniqueness check spans every implementing node. """ kinds: set[str] = set() - for kind in self._node_kinds: + for kind in self.node_diff_index.kinds: schema = self._get_schema_or_none(kind=kind) if schema is None: continue @@ -105,62 +100,6 @@ def _get_impacted_kinds(self) -> set[str]: kinds.update(getattr(schema, "inherit_from", None) or []) return kinds - def _field_in_diff(self, schema: MainSchemaTypes, field_name: str, is_relationship: bool) -> bool: - """Return True if `field_name` changed on `schema` or on a diffed kind that inherits it. - - An inherited field keeps its name on the implementing kind, so a generic-level constraint - is implicated when an implementation's copy of the field is what changed in the diff. - """ - kinds = {schema.kind} - for kind in self._node_kinds: - implementing_schema = self._get_schema_or_none(kind=kind) - if implementing_schema is not None and schema.kind in ( - getattr(implementing_schema, "inherit_from", None) or [] - ): - kinds.add(kind) - check = self._has_relationship_diff if is_relationship else self._has_attribute_diff - return any(check(kind=kind, name=field_name) for kind in kinds) - - def _diff_triggers_uniqueness(self, schema: MainSchemaTypes) -> bool: - """Return True when a diffed field participates in `schema`'s uniqueness. - - Uniqueness spans single unique attributes and multi-field constraint groups. A group - element such as "owner__name" reads an attribute of a related peer, so a data change on - the peer kind can create a violation without any change to the constrained kind itself. - """ - for attribute_schema in schema.unique_attributes: - if self._field_in_diff(schema=schema, field_name=attribute_schema.name, is_relationship=False): - return True - for constraint_group in schema.uniqueness_constraints or []: - for constraint_path in constraint_group: - try: - schema_path = schema.parse_schema_path(path=constraint_path, schema=self.schema_branch) - except AttributePathParsingError: - LOG.warning(f"Cannot parse {schema.kind}.uniqueness_constraints element '{constraint_path}'") - continue - if schema_path.relationship_schema is not None: - # check if the relationship changed - if self._field_in_diff( - schema=schema, field_name=schema_path.relationship_schema.name, is_relationship=True - ): - return True - # check if an attribute on the peer changed - if ( - schema_path.attribute_schema is not None - and schema_path.related_schema is not None - and self._field_in_diff( - schema=schema_path.related_schema, - field_name=schema_path.attribute_schema.name, - is_relationship=False, - ) - ): - return True - elif schema_path.attribute_schema is not None and self._field_in_diff( - schema=schema, field_name=schema_path.attribute_schema.name, is_relationship=False - ): - return True - return False - def _node_property_triggered_by_diff(self, schema: MainSchemaTypes, prop_name: str) -> bool: """Return True if the diff touches a field guarded by the node-level property `prop_name`. @@ -169,9 +108,9 @@ def _node_property_triggered_by_diff(self, schema: MainSchemaTypes, prop_name: s properties default to emitting so a newly-added node-level constraint is never missed. """ if prop_name == "uniqueness_constraints": - return self._diff_triggers_uniqueness(schema=schema) + return self.uniqueness_scoper.requires_validation(schema=schema) if prop_name in ("parent", "children"): - return self._has_relationship_diff(kind=schema.kind, name=prop_name) + return self.node_diff_index.has_relationship_diff(kind=schema.kind, name=prop_name) return True async def _get_property_constraints_for_impacted_kinds(self) -> list[SchemaUpdateConstraintInfo]: @@ -184,7 +123,7 @@ async def _get_property_constraints_for_impacted_kinds(self) -> list[SchemaUpdat for schema in self.schema_branch.get_all(duplicate=False).values(): if schema.kind in impacted_kinds: continue - if self._diff_triggers_uniqueness(schema=schema): + if self.uniqueness_scoper.requires_validation(schema=schema): schemas.append(schema) constraints: list[SchemaUpdateConstraintInfo] = [] @@ -239,7 +178,13 @@ async def _get_property_constraints_for_one_schema( # the diff, so a data change cannot violate it continue - constraints.append(SchemaUpdateConstraintInfo(constraint_name=constraint_name, path=schema_path)) + node_uuids: list[str] | None = None + if prop_name == "uniqueness_constraints": + node_uuids = await self.uniqueness_scoper.affected_node_uuids(schema=schema) + + constraints.append( + SchemaUpdateConstraintInfo(constraint_name=constraint_name, path=schema_path, node_uuids=node_uuids) + ) return constraints async def _get_attribute_constraints_for_one_schema( @@ -247,7 +192,7 @@ async def _get_attribute_constraints_for_one_schema( ) -> list[SchemaUpdateConstraintInfo]: constraints: list[SchemaUpdateConstraintInfo] = [] for field_name in schema.attribute_names: - if self._has_attribute_diff(kind=schema.kind, name=field_name): + if self.node_diff_index.has_attribute_diff(kind=schema.kind, name=field_name): field = schema.get_attribute(field_name) constraints.extend(await self._get_constraints_for_one_field(schema=schema, field=field)) return constraints @@ -257,7 +202,7 @@ async def _get_relationship_constraints_for_one_schema( ) -> list[SchemaUpdateConstraintInfo]: constraints: list[SchemaUpdateConstraintInfo] = [] for field_name in schema.relationship_names: - if self._has_relationship_diff(kind=schema.kind, name=field_name): + if self.node_diff_index.has_relationship_diff(kind=schema.kind, name=field_name): field = schema.get_relationship(field_name) constraints.extend(await self._get_constraints_for_one_field(schema=schema, field=field)) return constraints @@ -319,3 +264,21 @@ async def _get_constraints_for_one_field( constraints.append(SchemaUpdateConstraintInfo(constraint_name=constraint_name, path=schema_path)) return constraints + + +def build_constraint_validator_determiner( + db: InfrahubDatabase, + branch: Branch, + schema_branch: SchemaBranch, + at: Timestamp | str | None = None, +) -> ConstraintValidatorDeterminer: + """Wire a determiner with its node-diff index and uniqueness scoper for a single operation.""" + node_diff_index = NodeDiffIndex() + uniqueness_scoper = UniquenessConstraintScoper( + schema_branch=schema_branch, + dependent_resolver=UniquenessDependentResolver(db=db, branch=branch, at=at), + node_diff_index=node_diff_index, + ) + return ConstraintValidatorDeterminer( + schema_branch=schema_branch, node_diff_index=node_diff_index, uniqueness_scoper=uniqueness_scoper + ) diff --git a/backend/tests/component/core/constraint_validators/test_determiner.py b/backend/tests/component/core/constraint_validators/test_determiner.py index 7e1065f8fca..65fcf09b6cd 100644 --- a/backend/tests/component/core/constraint_validators/test_determiner.py +++ b/backend/tests/component/core/constraint_validators/test_determiner.py @@ -13,10 +13,27 @@ from infrahub.core.schema.schema_branch import SchemaBranch from infrahub.core.validators.determiner import ConstraintValidatorDeterminer from infrahub.core.validators.enum import ConstraintIdentifier +from infrahub.core.validators.node_diff_index import NodeDiffIndex +from infrahub.core.validators.uniqueness.scope import UniquenessConstraintScoper RELATIONSHIP_PROPERTIES = ("peer", "cardinality", "optional", "min_count", "max_count") +class _NoDependentsResolver: + async def resolve(self, node_kind: str, relationship_identifier: str, peer_uuids: list[str]) -> set[str]: + return set() + + +def _build_determiner(schema_branch: SchemaBranch) -> ConstraintValidatorDeterminer: + node_diff_index = NodeDiffIndex() + scoper = UniquenessConstraintScoper( + schema_branch=schema_branch, dependent_resolver=_NoDependentsResolver(), node_diff_index=node_diff_index + ) + return ConstraintValidatorDeterminer( + schema_branch=schema_branch, node_diff_index=node_diff_index, uniqueness_scoper=scoper + ) + + def node_constraint(kind: str, property_name: str) -> SchemaUpdateConstraintInfo: return SchemaUpdateConstraintInfo( constraint_name=f"node.{property_name}.update", @@ -153,7 +170,7 @@ def person_cars_node_diff( class TestConstraintDeterminer: async def test_no_node_diffs(self, car_person_schema: SchemaBranch, default_branch: Branch) -> None: schema_branch = registry.schema.get_schema_branch(name=default_branch.name) - determiner = ConstraintValidatorDeterminer(schema_branch=schema_branch) + determiner = _build_determiner(schema_branch=schema_branch) constraints = await determiner.get_constraints(node_diffs=[]) @@ -166,7 +183,7 @@ async def test_one_attribute_update_node_diff( person_name_node_diff: tuple[NodeDiffFieldSummary, set[SchemaUpdateConstraintInfo]], ) -> None: schema_branch = registry.schema.get_schema_branch(name=default_branch.name) - determiner = ConstraintValidatorDeterminer(schema_branch=schema_branch) + determiner = _build_determiner(schema_branch=schema_branch) node_diff, constraint_info_set = person_name_node_diff constraints = await determiner.get_constraints(node_diffs=[node_diff]) @@ -187,7 +204,7 @@ async def test_many_relationship_update( person_cars_node_diff: tuple[NodeDiffFieldSummary, set[SchemaUpdateConstraintInfo]], ) -> None: schema_branch = registry.schema.get_schema_branch(name=default_branch.name) - determiner = ConstraintValidatorDeterminer(schema_branch=schema_branch) + determiner = _build_determiner(schema_branch=schema_branch) node_diff, constraint_info_set = person_cars_node_diff constraints = await determiner.get_constraints(node_diffs=[node_diff]) @@ -208,7 +225,7 @@ async def test_node_property_constraints_included( name_attr_schema.parameters.max_length = 30 car_schema = schema_branch.get(name="TestCar", duplicate=False) car_schema.uniqueness_constraints = [["owner", "color__value"]] - determiner = ConstraintValidatorDeterminer(schema_branch=schema_branch) + determiner = _build_determiner(schema_branch=schema_branch) node_diff, constraint_info_set = person_name_node_diff max_length_param_constraint_info = SchemaUpdateConstraintInfo( constraint_name=ConstraintIdentifier.ATTRIBUTE_PARAMETERS_MAX_LENGTH_UPDATE.value, @@ -235,7 +252,7 @@ async def test_uniqueness_constraint_on_peer_attribute_included( schema_branch = registry.schema.get_schema_branch(name=default_branch.name) car_schema = schema_branch.get(name="TestCar", duplicate=False) car_schema.uniqueness_constraints = [["owner__name", "color__value"]] - determiner = ConstraintValidatorDeterminer(schema_branch=schema_branch) + determiner = _build_determiner(schema_branch=schema_branch) node_diff, constraint_info_set = person_name_node_diff constraint_info_set.add(node_uniqueness_constraint("TestPerson")) # TestCar's constraint reads the name attribute of the related TestPerson, so a TestPerson @@ -254,7 +271,7 @@ async def test_uniqueness_not_triggered_by_unrelated_field( schema_branch = registry.schema.get_schema_branch(name=default_branch.name) generic_schema = schema_branch.get(name="TestCar", duplicate=False) generic_schema.uniqueness_constraints = [["name__value"]] - determiner = ConstraintValidatorDeterminer(schema_branch=schema_branch) + determiner = _build_determiner(schema_branch=schema_branch) node_diff = NodeDiffFieldSummary(kind="TestElectricCar", attribute_names={"nbr_engine"}) # nbr_engine participates in no uniqueness path, so the uniqueness check must not be # triggered on the implementation or on its generic; only the nbr_engine field constraints remain @@ -276,7 +293,7 @@ async def test_generic_uniqueness_triggered_by_inherited_field( schema_branch = registry.schema.get_schema_branch(name=default_branch.name) generic_schema = schema_branch.get(name="TestCar", duplicate=False) generic_schema.uniqueness_constraints = [["name__value"]] - determiner = ConstraintValidatorDeterminer(schema_branch=schema_branch) + determiner = _build_determiner(schema_branch=schema_branch) # `name` is inherited from the generic; a generic-level uniqueness check spans every # implementing node, so changing name on an implementation must trigger the check on the # generic (TestCar) as well as on the implementation (TestElectricCar) @@ -304,7 +321,7 @@ async def test_uniqueness_triggered_by_generic_peer_implementation( # reading the peer's `name` must fire when an implementation of that generic (LocationSite) # changes `name`, even though the peer kind named in the path (LocationGeneric) has no diff thing_schema.uniqueness_constraints = [["location__name"]] - determiner = ConstraintValidatorDeterminer(schema_branch=schema_branch) + determiner = _build_determiner(schema_branch=schema_branch) node_diff = NodeDiffFieldSummary(kind="LocationSite", attribute_names={"name"}) constraints = await determiner.get_constraints(node_diffs=[node_diff]) @@ -319,7 +336,7 @@ async def test_uniqueness_not_triggered_by_unrelated_peer_attribute( schema_branch = registry.schema.get_schema_branch(name=default_branch.name) car_schema = schema_branch.get(name="TestCar", duplicate=False) car_schema.uniqueness_constraints = [["owner__name", "color__value"]] - determiner = ConstraintValidatorDeterminer(schema_branch=schema_branch) + determiner = _build_determiner(schema_branch=schema_branch) # TestCar's uniqueness constraints read TestPerson.name; a change to an unrelated # TestPerson attribute, height, does not participate, so neither kind's uniqueness # check should trigger @@ -338,7 +355,7 @@ async def test_kind_missing_from_schema_is_skipped( person_name_node_diff: tuple[NodeDiffFieldSummary, set[SchemaUpdateConstraintInfo]], ) -> None: schema_branch = registry.schema.get_schema_branch(name=default_branch.name) - determiner = ConstraintValidatorDeterminer(schema_branch=schema_branch) + determiner = _build_determiner(schema_branch=schema_branch) node_diff, constraint_info_set = person_name_node_diff constraint_info_set.add(node_uniqueness_constraint("TestPerson")) @@ -358,7 +375,7 @@ async def test_internal_schema_kinds_only_when_in_diff( schema_branch = registry.schema.register_schema( schema=SchemaRoot(**internal_schema), branch=default_branch.name ) - determiner = ConstraintValidatorDeterminer(schema_branch=schema_branch) + determiner = _build_determiner(schema_branch=schema_branch) node_diff, constraint_info_set = person_name_node_diff constraint_info_set.add(node_uniqueness_constraint("TestPerson")) @@ -367,7 +384,7 @@ async def test_internal_schema_kinds_only_when_in_diff( assert set(constraints) == constraint_info_set internal_kinds = {"SchemaNode", "SchemaGeneric", "SchemaAttribute", "SchemaRelationship"} - determiner = ConstraintValidatorDeterminer(schema_branch=schema_branch) + determiner = _build_determiner(schema_branch=schema_branch) constraints = await determiner.get_constraints( node_diffs=[ node_diff, @@ -386,7 +403,7 @@ async def test_hierarchy_constraints_selected_for_both_endpoint_kinds( default_branch: Branch, ) -> None: schema_branch = registry.schema.get_schema_branch(name=default_branch.name) - determiner = ConstraintValidatorDeterminer(schema_branch=schema_branch) + determiner = _build_determiner(schema_branch=schema_branch) # A hierarchy edge change surfaces on both endpoints: the child (LocationRack) records its # `parent` change and the parent (LocationSite) records its `children` change. Each endpoint # emits only the hierarchy constraint whose relationship actually changed there, and no @@ -415,7 +432,7 @@ async def test_unparseable_uniqueness_constraint_element_is_skipped_and_logged( schema_branch = registry.schema.get_schema_branch(name=default_branch.name) person_schema = schema_branch.get(name="TestPerson", duplicate=False) person_schema.uniqueness_constraints = [["does_not_exist__value"]] - determiner = ConstraintValidatorDeterminer(schema_branch=schema_branch) + determiner = _build_determiner(schema_branch=schema_branch) # `height` is not a unique attribute, so evaluating uniqueness must fall through to parsing # the (unparseable) constraint group rather than short-circuiting on a unique attribute. node_diff = NodeDiffFieldSummary(kind="TestPerson", attribute_names={"height"}) @@ -433,7 +450,7 @@ async def test_hierarchy_constraint_scoped_to_changed_relationship( default_branch: Branch, ) -> None: schema_branch = registry.schema.get_schema_branch(name=default_branch.name) - determiner = ConstraintValidatorDeterminer(schema_branch=schema_branch) + determiner = _build_determiner(schema_branch=schema_branch) # Re-parenting a rack changes only its `parent` relationship, so only the parent hierarchy # constraint may be emitted for that kind, never the children one. node_diffs = [NodeDiffFieldSummary(kind="LocationSite", relationship_names={"parent"})] From 87a8d525ca41c1379b70379447f1c686b3d643ac Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 17:40:03 -0700 Subject: [PATCH 11/31] add uniqueness constraints to ConstraintIdentifier enum --- backend/infrahub/core/migrations/__init__.py | 4 +++- .../migrations/schema/node_uniqueness_constraints_update.py | 3 ++- backend/infrahub/core/validators/__init__.py | 2 +- backend/infrahub/core/validators/enum.py | 1 + backend/infrahub/core/validators/uniqueness/checker.py | 3 ++- 5 files changed, 9 insertions(+), 4 deletions(-) diff --git a/backend/infrahub/core/migrations/__init__.py b/backend/infrahub/core/migrations/__init__.py index 97c6a35557a..00f0dc09366 100644 --- a/backend/infrahub/core/migrations/__init__.py +++ b/backend/infrahub/core/migrations/__init__.py @@ -1,3 +1,5 @@ +from infrahub.core.validators.enum import ConstraintIdentifier + from .schema.attribute_kind_update import AttributeKindUpdateMigration from .schema.attribute_name_update import AttributeNameUpdateMigration from .schema.attribute_supports_generated_schema import AttributeSupportsGeneratedSchemaMigration @@ -18,7 +20,7 @@ "node.name.update": NodeKindUpdateMigration, "node.namespace.update": NodeKindUpdateMigration, "node.relationship.remove": NodeRelationshipRemoveMigration, - "node.uniqueness_constraints.update": NodeUniquenessConstraintsUpdateMigration, + ConstraintIdentifier.NODE_UNIQUENESS_CONSTRAINTS_UPDATE.value: NodeUniquenessConstraintsUpdateMigration, "attribute.name.update": AttributeNameUpdateMigration, "attribute.branch.update": None, "attribute.kind.update": AttributeKindUpdateMigration, diff --git a/backend/infrahub/core/migrations/schema/node_uniqueness_constraints_update.py b/backend/infrahub/core/migrations/schema/node_uniqueness_constraints_update.py index 7a5b39ed23f..73a02c7aaf9 100644 --- a/backend/infrahub/core/migrations/schema/node_uniqueness_constraints_update.py +++ b/backend/infrahub/core/migrations/schema/node_uniqueness_constraints_update.py @@ -6,6 +6,7 @@ from infrahub.core.path import SchemaPath from infrahub.core.schema.generic_schema import GenericSchema from infrahub.core.schema.node_schema import NodeSchema +from infrahub.core.validators.enum import ConstraintIdentifier from ..query import MigrationBaseQuery # noqa: TC001 from ..shared import AttributeSchemaMigration, MigrationInput, MigrationResult, SchemaMigration @@ -19,7 +20,7 @@ class NodeUniquenessConstraintsUpdateMigration(SchemaMigration): - name: str = "node.uniqueness_constraints.update" + name: str = ConstraintIdentifier.NODE_UNIQUENESS_CONSTRAINTS_UPDATE.value queries: Sequence[type[MigrationBaseQuery]] = [] async def execute( diff --git a/backend/infrahub/core/validators/__init__.py b/backend/infrahub/core/validators/__init__.py index 3ebe9d69f1a..4902e6e4c7a 100644 --- a/backend/infrahub/core/validators/__init__.py +++ b/backend/infrahub/core/validators/__init__.py @@ -44,7 +44,7 @@ "relationship.max_count.update": RelationshipCountChecker, "relationship.common_parent.update": RelationshipPeerParentChecker, "node.inherit_from.update": NodeInheritFromChecker, - "node.uniqueness_constraints.update": UniquenessChecker, + ConstraintIdentifier.NODE_UNIQUENESS_CONSTRAINTS_UPDATE.value: UniquenessChecker, "node.parent.update": NodeHierarchyChecker, "node.children.update": NodeHierarchyChecker, "node.generate_profile.update": NodeGenerateProfileChecker, diff --git a/backend/infrahub/core/validators/enum.py b/backend/infrahub/core/validators/enum.py index 97e619457c8..3f29c4b3c6a 100644 --- a/backend/infrahub/core/validators/enum.py +++ b/backend/infrahub/core/validators/enum.py @@ -10,3 +10,4 @@ class ConstraintIdentifier(StrEnum): ATTRIBUTE_PARAMETERS_EXCLUDED_VALUES_UPDATE = "attribute.parameters.excluded_values.update" ATTRIBUTE_PARAMETERS_END_RANGE_UPDATE = "attribute.parameters.end_range.update" ATTRIBUTE_PARAMETERS_START_RANGE_UPDATE = "attribute.parameters.start_range.update" + NODE_UNIQUENESS_CONSTRAINTS_UPDATE = "node.uniqueness_constraints.update" diff --git a/backend/infrahub/core/validators/uniqueness/checker.py b/backend/infrahub/core/validators/uniqueness/checker.py index 7f51c1ac8cf..69665697587 100644 --- a/backend/infrahub/core/validators/uniqueness/checker.py +++ b/backend/infrahub/core/validators/uniqueness/checker.py @@ -8,6 +8,7 @@ from infrahub.core.schema import AttributeSchema, MainSchemaTypes, RelationshipSchema from infrahub.core.validators.uniqueness.index import UniquenessQueryResultsIndex +from ..enum import ConstraintIdentifier from ..interface import ConstraintCheckerInterface from .model import ( NodeUniquenessQueryRequest, @@ -66,7 +67,7 @@ def __init__( @property def name(self) -> str: - return "node.uniqueness_constraints.update" + return ConstraintIdentifier.NODE_UNIQUENESS_CONSTRAINTS_UPDATE.value def supports(self, request: SchemaConstraintValidatorRequest) -> bool: return request.constraint_name == self.name From 7563e6f4c278127c97eb0e22c44b0f524a386d6f Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 17:42:03 -0700 Subject: [PATCH 12/31] add uniqueness constraints deduplicator --- .../validators/uniqueness/deduplicator.py | 99 +++++++++++ .../test_constraint_deduplicator.py | 163 ++++++++++++++++++ 2 files changed, 262 insertions(+) create mode 100644 backend/infrahub/core/validators/uniqueness/deduplicator.py create mode 100644 backend/tests/unit/core/validators/test_constraint_deduplicator.py diff --git a/backend/infrahub/core/validators/uniqueness/deduplicator.py b/backend/infrahub/core/validators/uniqueness/deduplicator.py new file mode 100644 index 00000000000..8a26463c13e --- /dev/null +++ b/backend/infrahub/core/validators/uniqueness/deduplicator.py @@ -0,0 +1,99 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +from infrahub.core.schema import GenericSchema +from infrahub.core.validators.enum import ConstraintIdentifier +from infrahub.exceptions import SchemaNotFoundError + +if TYPE_CHECKING: + from infrahub.core.models import SchemaUpdateConstraintInfo + from infrahub.core.schema import MainSchemaTypes + from infrahub.core.schema.schema_branch import SchemaBranch + +UNIQUENESS_CONSTRAINT_NAME = ConstraintIdentifier.NODE_UNIQUENESS_CONSTRAINTS_UPDATE.value + + +class UniquenessConstraintDeduplicator: + """Drop node-level uniqueness checks already covered by an implicated generic. + + A generic's uniqueness query spans every implementing node, so when a generic and one of its + implementations are both slated to validate uniqueness, the implementation's check is redundant. + It is dropped only when the generic covers every one of the node's constraint groups and its + validation scope covers the node's, so neither a node-specific group nor a broader + (full-population) check is ever lost. + """ + + def __init__(self, schema_branch: SchemaBranch) -> None: + self.schema_branch = schema_branch + + def deduplicate(self, constraints: list[SchemaUpdateConstraintInfo]) -> list[SchemaUpdateConstraintInfo]: + uniqueness_infos = { + constraint.path.schema_kind: constraint + for constraint in constraints + if constraint.constraint_name == UNIQUENESS_CONSTRAINT_NAME + } + # a node can only be redundant against a generic, so there is nothing to collapse below two + if len(uniqueness_infos) < 2: + return list(constraints) + + redundant_kinds = { + kind + for kind, info in uniqueness_infos.items() + if self._is_covered_by_generic(kind=kind, info=info, uniqueness_infos=uniqueness_infos) + } + if not redundant_kinds: + return list(constraints) + + return [ + constraint + for constraint in constraints + if constraint.constraint_name != UNIQUENESS_CONSTRAINT_NAME + or constraint.path.schema_kind not in redundant_kinds + ] + + def _is_covered_by_generic( + self, kind: str, info: SchemaUpdateConstraintInfo, uniqueness_infos: dict[str, SchemaUpdateConstraintInfo] + ) -> bool: + schema = self._get_schema_or_none(kind=kind) + if schema is None or isinstance(schema, GenericSchema): + # a generic is the coverer, never the covered + return False + node_groups = self._constraint_groups(schema) + if not node_groups: + return False + + covered_groups: set[frozenset[str]] = set() + for generic_kind in schema.inherit_from or []: + generic_info = uniqueness_infos.get(generic_kind) + if generic_info is None: + continue + if not self._scope_covers(node_info=info, generic_info=generic_info): + continue + generic_schema = self._get_schema_or_none(kind=generic_kind) + if generic_schema is None: + continue + covered_groups |= self._constraint_groups(generic_schema) + return node_groups <= covered_groups + + def _scope_covers(self, node_info: SchemaUpdateConstraintInfo, generic_info: SchemaUpdateConstraintInfo) -> bool: + """Return True if the generic's validation scope covers the node's. + + A full-population generic covers everything. A scoped generic cannot stand in for a + full-population node (that would silently narrow the check), and otherwise must validate a + superset of the node's nodes. + """ + if generic_info.node_uuids is None: + return True + if node_info.node_uuids is None: + return False + return set(generic_info.node_uuids) >= set(node_info.node_uuids) + + def _constraint_groups(self, schema: MainSchemaTypes) -> set[frozenset[str]]: + return {frozenset(group) for group in schema.uniqueness_constraints or []} + + def _get_schema_or_none(self, kind: str) -> MainSchemaTypes | None: + try: + return self.schema_branch.get(name=kind, duplicate=False) + except SchemaNotFoundError: + return None diff --git a/backend/tests/unit/core/validators/test_constraint_deduplicator.py b/backend/tests/unit/core/validators/test_constraint_deduplicator.py new file mode 100644 index 00000000000..a8c3bc839b7 --- /dev/null +++ b/backend/tests/unit/core/validators/test_constraint_deduplicator.py @@ -0,0 +1,163 @@ +from infrahub.core.constants import SchemaPathType +from infrahub.core.models import SchemaUpdateConstraintInfo +from infrahub.core.path import SchemaPath +from infrahub.core.schema.generic_schema import GenericSchema +from infrahub.core.schema.node_schema import NodeSchema +from infrahub.core.schema.schema_branch import SchemaBranch +from infrahub.core.validators.enum import ConstraintIdentifier +from infrahub.core.validators.uniqueness.deduplicator import UniquenessConstraintDeduplicator + +UNIQUENESS = ConstraintIdentifier.NODE_UNIQUENESS_CONSTRAINTS_UPDATE.value + + +def _uniqueness_info(kind: str, node_uuids: list[str] | None = None) -> SchemaUpdateConstraintInfo: + return SchemaUpdateConstraintInfo( + constraint_name=UNIQUENESS, + path=SchemaPath( + path_type=SchemaPathType.NODE, + schema_kind=kind, + field_name="uniqueness_constraints", + property_name="uniqueness_constraints", + ), + node_uuids=node_uuids, + ) + + +def _attribute_info(kind: str) -> SchemaUpdateConstraintInfo: + return SchemaUpdateConstraintInfo( + constraint_name="attribute.unique.update", + path=SchemaPath( + path_type=SchemaPathType.ATTRIBUTE, schema_kind=kind, field_name="name", property_name="unique" + ), + ) + + +def _schema_branch() -> SchemaBranch: + """A generic ``TestCar`` and implementations, plus an unrelated standalone ``TestPerson``. + + ElectricCar/GazCar inherit the generic's single constraint group; SpecialCar adds one of its + own; Person shares no inheritance. + """ + branch = SchemaBranch(cache={}, name="test") + branch.set( + name="TestCar", + schema=GenericSchema(name="Car", namespace="Test", uniqueness_constraints=[["name__value"]]), + ) + branch.set( + name="TestElectricCar", + schema=NodeSchema(name="ElectricCar", namespace="Test", inherit_from=["TestCar"], uniqueness_constraints=[["name__value"]]), + ) + branch.set( + name="TestGazCar", + schema=NodeSchema(name="GazCar", namespace="Test", inherit_from=["TestCar"], uniqueness_constraints=[["name__value"]]), + ) + branch.set( + name="TestSpecialCar", + schema=NodeSchema( + name="SpecialCar", + namespace="Test", + inherit_from=["TestCar"], + uniqueness_constraints=[["name__value"], ["special__value"]], + ), + ) + branch.set( + name="TestPerson", + schema=NodeSchema(name="Person", namespace="Test", uniqueness_constraints=[["name__value"]]), + ) + return branch + + +def _remaining_uniqueness_kinds(constraints: list[SchemaUpdateConstraintInfo]) -> set[str]: + return {c.path.schema_kind for c in constraints if c.constraint_name == UNIQUENESS} + + +class TestUniquenessConstraintDeduplicator: + def test_inherited_node_dropped_when_generic_covers_it_full_population(self) -> None: + deduplicator = UniquenessConstraintDeduplicator(schema_branch=_schema_branch()) + constraints = [_uniqueness_info("TestCar"), _uniqueness_info("TestElectricCar")] + + result = deduplicator.deduplicate(constraints) + + assert _remaining_uniqueness_kinds(result) == {"TestCar"} + + def test_all_implementers_dropped_when_generic_covers_them(self) -> None: + # the generic's scope is the union of the implementers' changed nodes + deduplicator = UniquenessConstraintDeduplicator(schema_branch=_schema_branch()) + constraints = [ + _uniqueness_info("TestCar", node_uuids=["e1", "g1"]), + _uniqueness_info("TestElectricCar", node_uuids=["e1"]), + _uniqueness_info("TestGazCar", node_uuids=["g1"]), + ] + + result = deduplicator.deduplicate(constraints) + + assert _remaining_uniqueness_kinds(result) == {"TestCar"} + + def test_node_with_own_group_is_kept(self) -> None: + # SpecialCar has a `special` group the generic does not cover, so it cannot be dropped + deduplicator = UniquenessConstraintDeduplicator(schema_branch=_schema_branch()) + constraints = [_uniqueness_info("TestCar"), _uniqueness_info("TestSpecialCar")] + + result = deduplicator.deduplicate(constraints) + + assert _remaining_uniqueness_kinds(result) == {"TestCar", "TestSpecialCar"} + + def test_full_population_node_not_dropped_for_scoped_generic(self) -> None: + # dropping a full-population node for a scoped generic would silently narrow the check + deduplicator = UniquenessConstraintDeduplicator(schema_branch=_schema_branch()) + constraints = [ + _uniqueness_info("TestCar", node_uuids=["e1"]), + _uniqueness_info("TestElectricCar", node_uuids=None), + ] + + result = deduplicator.deduplicate(constraints) + + assert _remaining_uniqueness_kinds(result) == {"TestCar", "TestElectricCar"} + + def test_node_not_dropped_when_generic_scope_does_not_cover_it(self) -> None: + deduplicator = UniquenessConstraintDeduplicator(schema_branch=_schema_branch()) + constraints = [ + _uniqueness_info("TestCar", node_uuids=["other"]), + _uniqueness_info("TestElectricCar", node_uuids=["e1"]), + ] + + result = deduplicator.deduplicate(constraints) + + assert _remaining_uniqueness_kinds(result) == {"TestCar", "TestElectricCar"} + + def test_scoped_node_dropped_when_generic_scope_is_superset(self) -> None: + deduplicator = UniquenessConstraintDeduplicator(schema_branch=_schema_branch()) + constraints = [ + _uniqueness_info("TestCar", node_uuids=["e1", "e2", "g1"]), + _uniqueness_info("TestElectricCar", node_uuids=["e1", "e2"]), + ] + + result = deduplicator.deduplicate(constraints) + + assert _remaining_uniqueness_kinds(result) == {"TestCar"} + + def test_node_kept_when_generic_absent_from_set(self) -> None: + deduplicator = UniquenessConstraintDeduplicator(schema_branch=_schema_branch()) + constraints = [_uniqueness_info("TestElectricCar")] + + result = deduplicator.deduplicate(constraints) + + assert _remaining_uniqueness_kinds(result) == {"TestElectricCar"} + + def test_standalone_node_is_kept(self) -> None: + deduplicator = UniquenessConstraintDeduplicator(schema_branch=_schema_branch()) + constraints = [_uniqueness_info("TestCar"), _uniqueness_info("TestPerson")] + + result = deduplicator.deduplicate(constraints) + + assert _remaining_uniqueness_kinds(result) == {"TestCar", "TestPerson"} + + def test_non_uniqueness_constraints_pass_through(self) -> None: + deduplicator = UniquenessConstraintDeduplicator(schema_branch=_schema_branch()) + attribute = _attribute_info("TestElectricCar") + constraints = [_uniqueness_info("TestCar"), _uniqueness_info("TestElectricCar"), attribute] + + result = deduplicator.deduplicate(constraints) + + assert _remaining_uniqueness_kinds(result) == {"TestCar"} + assert attribute in result From 5cfaad6fa9c316ad85321e7003bd1de309ae4167 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 17:53:48 -0700 Subject: [PATCH 13/31] add ConstraintInfoMerger --- .../core/validators/constraint_merge.py | 46 ++++++++ .../core/validators/test_constraint_merge.py | 101 ++++++++++++++++++ 2 files changed, 147 insertions(+) create mode 100644 backend/infrahub/core/validators/constraint_merge.py create mode 100644 backend/tests/unit/core/validators/test_constraint_merge.py diff --git a/backend/infrahub/core/validators/constraint_merge.py b/backend/infrahub/core/validators/constraint_merge.py new file mode 100644 index 00000000000..4b1a1c00d80 --- /dev/null +++ b/backend/infrahub/core/validators/constraint_merge.py @@ -0,0 +1,46 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +from infrahub.core.validators.uniqueness.deduplicator import UniquenessConstraintDeduplicator + +if TYPE_CHECKING: + from infrahub.core.models import SchemaUpdateConstraintInfo + from infrahub.core.schema.schema_branch import SchemaBranch + + +class ConstraintInfoMerger: + """Combine constraint infos from several producers into the set of checks to run. + + First the same constraint produced by more than one producer is collapsed onto a single entry; + then node-level uniqueness checks already covered by an implicated generic are dropped. + """ + + def __init__(self, deduplicator: UniquenessConstraintDeduplicator) -> None: + self.deduplicator = deduplicator + + def merge(self, *constraint_lists: list[SchemaUpdateConstraintInfo]) -> list[SchemaUpdateConstraintInfo]: + # Collapse the same constraint from multiple producers onto one entry. A constraint both + # broadened by a schema change (full population) and hit by a data change (node-scoped) must + # re-check every node, so a None node_uuids always wins; two node-scoped entries union. + merged: dict[tuple[str, str], SchemaUpdateConstraintInfo] = {} + for constraint_list in constraint_lists: + for constraint in constraint_list: + key = (constraint.constraint_name, constraint.path.get_path()) + existing = merged.get(key) + if existing is None: + merged[key] = constraint + elif existing.node_uuids is None: + continue + elif constraint.node_uuids is None: + merged[key] = constraint + else: + merged[key] = existing.model_copy( + update={"node_uuids": sorted(set(existing.node_uuids) | set(constraint.node_uuids))} + ) + + return self.deduplicator.deduplicate(list(merged.values())) + + +def build_constraint_info_merger(schema_branch: SchemaBranch) -> ConstraintInfoMerger: + return ConstraintInfoMerger(deduplicator=UniquenessConstraintDeduplicator(schema_branch=schema_branch)) diff --git a/backend/tests/unit/core/validators/test_constraint_merge.py b/backend/tests/unit/core/validators/test_constraint_merge.py new file mode 100644 index 00000000000..269a4eefb27 --- /dev/null +++ b/backend/tests/unit/core/validators/test_constraint_merge.py @@ -0,0 +1,101 @@ +from infrahub.core.constants import SchemaPathType +from infrahub.core.models import SchemaUpdateConstraintInfo +from infrahub.core.path import SchemaPath +from infrahub.core.schema.generic_schema import GenericSchema +from infrahub.core.schema.node_schema import NodeSchema +from infrahub.core.schema.schema_branch import SchemaBranch +from infrahub.core.validators.constraint_merge import build_constraint_info_merger +from infrahub.core.validators.enum import ConstraintIdentifier + +UNIQUENESS = ConstraintIdentifier.NODE_UNIQUENESS_CONSTRAINTS_UPDATE.value + + +def _uniqueness_info(kind: str, node_uuids: list[str] | None) -> SchemaUpdateConstraintInfo: + return SchemaUpdateConstraintInfo( + constraint_name=UNIQUENESS, + path=SchemaPath( + path_type=SchemaPathType.NODE, + schema_kind=kind, + field_name="uniqueness_constraints", + property_name="uniqueness_constraints", + ), + node_uuids=node_uuids, + ) + + +def _schema_branch() -> SchemaBranch: + branch = SchemaBranch(cache={}, name="test") + branch.set( + name="TestCar", schema=GenericSchema(name="Car", namespace="Test", uniqueness_constraints=[["name__value"]]) + ) + branch.set( + name="TestElectricCar", + schema=NodeSchema( + name="ElectricCar", namespace="Test", inherit_from=["TestCar"], uniqueness_constraints=[["name__value"]] + ), + ) + branch.set( + name="TestPerson", + schema=NodeSchema(name="Person", namespace="Test", uniqueness_constraints=[["name__value"]]), + ) + return branch + + +class TestConstraintInfoMergerPrecedence: + """The cross-producer collapse that runs before generic/node deduplication.""" + + def test_full_scan_wins_over_node_scoped(self) -> None: + # a constraint both broadened (schema diff, full scan) and data-changed collapses to a full scan + merger = build_constraint_info_merger(schema_branch=_schema_branch()) + data_diff = [_uniqueness_info("TestCar", node_uuids=["a", "b"])] + schema_diff = [_uniqueness_info("TestCar", node_uuids=None)] + + result = merger.merge(data_diff, schema_diff) + + assert result == [_uniqueness_info("TestCar", node_uuids=None)] + + def test_full_scan_wins_regardless_of_order(self) -> None: + merger = build_constraint_info_merger(schema_branch=_schema_branch()) + schema_diff = [_uniqueness_info("TestCar", node_uuids=None)] + data_diff = [_uniqueness_info("TestCar", node_uuids=["a", "b"])] + + result = merger.merge(schema_diff, data_diff) + + assert result == [_uniqueness_info("TestCar", node_uuids=None)] + + def test_two_node_scoped_entries_union_their_nodes(self) -> None: + merger = build_constraint_info_merger(schema_branch=_schema_branch()) + + result = merger.merge( + [_uniqueness_info("TestCar", node_uuids=["a", "b"])], + [_uniqueness_info("TestCar", node_uuids=["b", "c"])], + ) + + assert result == [_uniqueness_info("TestCar", node_uuids=["a", "b", "c"])] + + def test_distinct_constraints_are_preserved(self) -> None: + merger = build_constraint_info_merger(schema_branch=_schema_branch()) + + result = merger.merge( + [_uniqueness_info("TestCar", node_uuids=["a"])], + [_uniqueness_info("TestPerson", node_uuids=["b"])], + ) + + assert {c.path.schema_kind: c.node_uuids for c in result} == {"TestCar": ["a"], "TestPerson": ["b"]} + + +class TestConstraintInfoMerger: + def test_merges_then_deduplicates(self) -> None: + # the data diff scopes the generic; the schema diff broadens it to the full population; and + # the inherited node check must be dropped as covered by the generic + merger = build_constraint_info_merger(schema_branch=_schema_branch()) + data_diff = [ + _uniqueness_info("TestCar", node_uuids=["e1"]), + _uniqueness_info("TestElectricCar", node_uuids=["e1"]), + ] + schema_diff = [_uniqueness_info("TestCar", node_uuids=None)] + + result = merger.merge(data_diff, schema_diff) + + # merge precedence keeps the full-population generic; dedup then removes the covered node + assert result == [_uniqueness_info("TestCar", node_uuids=None)] From 7bd6cebd4aa654afe2ee53ee22482b3aad1e2fc1 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 18:07:35 -0700 Subject: [PATCH 14/31] pass node_uuids for constraints through PC pipeline --- backend/infrahub/core/models.py | 5 +++++ backend/infrahub/core/validators/model.py | 5 +++++ backend/infrahub/core/validators/tasks.py | 3 +++ backend/infrahub/proposed_change/tasks.py | 24 +++++++++++++---------- 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/backend/infrahub/core/models.py b/backend/infrahub/core/models.py index 531cf76d759..a7394941d47 100644 --- a/backend/infrahub/core/models.py +++ b/backend/infrahub/core/models.py @@ -156,6 +156,11 @@ class SchemaUpdateConstraintInfo(BaseModel): model_config = ConfigDict(extra="forbid") path: SchemaPath constraint_name: str + node_uuids: list[str] | None = None + """When set, restrict validation to these nodes; None means validate the full population. + + node_uuids=None indicates that all possible matching objects must be checked + """ @property def routing_key(self) -> str: diff --git a/backend/infrahub/core/validators/model.py b/backend/infrahub/core/validators/model.py index e030df994fc..6bb62b40f55 100644 --- a/backend/infrahub/core/validators/model.py +++ b/backend/infrahub/core/validators/model.py @@ -16,6 +16,10 @@ class SchemaConstraintValidatorRequest(BaseModel): node_schema: NodeSchema | GenericSchema = Field(..., description="Schema of Node or Generic to validate") schema_path: SchemaPath = Field(..., description="SchemaPath to the element of the schema to validate") schema_branch: SchemaBranch = Field(..., description="SchemaBranch of the element to validate") + node_uuids: list[str] | None = Field( + default=None, + description="When set, restrict validation to these nodes; None validates the full population", + ) @model_serializer() def serialize_model(self) -> dict[str, Any]: @@ -25,6 +29,7 @@ def serialize_model(self) -> dict[str, Any]: "node_schema": self.node_schema.model_dump(), "schema_path": self.schema_path.model_dump(), "schema_branch": self.schema_branch.to_dict_schema_object(), + "node_uuids": self.node_uuids, } @field_validator("schema_branch", mode="before") diff --git a/backend/infrahub/core/validators/tasks.py b/backend/infrahub/core/validators/tasks.py index d43ebab1b33..5326f47992b 100644 --- a/backend/infrahub/core/validators/tasks.py +++ b/backend/infrahub/core/validators/tasks.py @@ -46,6 +46,7 @@ async def schema_validate_migrations(message: SchemaValidateMigrationData) -> li node_schema=schema, schema_path=constraint.path, schema_branch=message.schema_branch, + node_uuids=constraint.node_uuids, database=await get_database(), ) @@ -66,6 +67,7 @@ async def schema_path_validate( schema_path: SchemaPath, schema_branch: SchemaBranch, database: InfrahubDatabase, + node_uuids: list[str] | None = None, ) -> SchemaValidatorPathResponseData: async with database.start_session(read_only=True) as db: constraint_request = SchemaConstraintValidatorRequest( @@ -74,6 +76,7 @@ async def schema_path_validate( node_schema=node_schema, schema_path=schema_path, schema_branch=schema_branch, + node_uuids=node_uuids, ) component_registry = get_component_registry() diff --git a/backend/infrahub/proposed_change/tasks.py b/backend/infrahub/proposed_change/tasks.py index 0abdc37e411..f57e3dab7f9 100644 --- a/backend/infrahub/proposed_change/tasks.py +++ b/backend/infrahub/proposed_change/tasks.py @@ -53,7 +53,8 @@ from infrahub.core.protocols import CoreProposedChange as InternalCoreProposedChange from infrahub.core.timestamp import Timestamp from infrahub.core.validators.checks_runner import run_checks_and_update_validator -from infrahub.core.validators.determiner import ConstraintValidatorDeterminer +from infrahub.core.validators.constraint_merge import build_constraint_info_merger +from infrahub.core.validators.determiner import build_constraint_validator_determiner from infrahub.core.validators.models.validate_migration import SchemaValidateMigrationData from infrahub.core.validators.tasks import schema_validate_migrations from infrahub.dependencies.registry import get_component_registry @@ -525,6 +526,8 @@ async def run_proposed_change_schema_integrity_check(model: RequestProposedChang # In the future it would be good to generate the object SchemaUpdateValidationResult from message.branch_diff await add_tags(branches=[model.source_branch], nodes=[model.proposed_change]) + database = await get_database() + source_schema = registry.schema.get_schema_branch(name=model.source_branch).duplicate() dest_schema = registry.schema.get_schema_branch(name=model.destination_branch).duplicate() @@ -538,7 +541,6 @@ async def run_proposed_change_schema_integrity_check(model: RequestProposedChang parts = error_msg.split(":", 1) kind = parts[0].strip() if len(parts) > 1 and parts[0].strip() else "Unknown" schema_path = f"schema/{kind}" - database = await get_database() async with database.start_transaction() as db: object_conflict_validator_recorder = _build_schema_integrity_validator_recorder(db=db) await object_conflict_validator_recorder.record_conflicts( @@ -561,11 +563,13 @@ async def run_proposed_change_schema_integrity_check(model: RequestProposedChang validation_result = dest_schema.validate_update(other=candidate_schema, diff=schema_diff) diff_summary = await get_diff_summary_cache(pipeline_id=model.branch_diff.pipeline_id) + source_branch = registry.get_branch_from_registry(branch=model.source_branch) constraints_from_data_diff = await _get_proposed_change_schema_integrity_constraints( - schema=candidate_schema, diff_summary=diff_summary + db=database, schema=candidate_schema, diff_summary=diff_summary, branch=source_branch ) constraints_from_schema_diff = validation_result.constraints - constraints = set(constraints_from_data_diff + constraints_from_schema_diff) + merger = build_constraint_info_merger(schema_branch=candidate_schema) + constraints = merger.merge(constraints_from_data_diff, constraints_from_schema_diff) if not constraints: return @@ -573,15 +577,13 @@ async def run_proposed_change_schema_integrity_check(model: RequestProposedChang # ---------------------------------------------------------- # Validate if the new schema is valid with the content of the database # ---------------------------------------------------------- - source_branch = registry.get_branch_from_registry(branch=model.source_branch) responses = await schema_validate_migrations( message=SchemaValidateMigrationData( - branch=source_branch, schema_branch=candidate_schema, constraints=list(constraints) + branch=source_branch, schema_branch=candidate_schema, constraints=constraints ) ) component_registry = get_component_registry() - database = await get_database() async with database.start_session() as db: diff_repository = await component_registry.get_component(DiffRepository, db=db, branch=source_branch) conflicted_fields = await gather_conflicted_fields( @@ -601,7 +603,7 @@ async def run_proposed_change_schema_integrity_check(model: RequestProposedChang async def _get_proposed_change_schema_integrity_constraints( - schema: SchemaBranch, diff_summary: list[NodeDiff] + db: InfrahubDatabase, schema: SchemaBranch, diff_summary: list[NodeDiff], branch: Branch ) -> list[SchemaUpdateConstraintInfo]: node_diff_field_summary_map: dict[str, NodeDiffFieldSummary] = {} @@ -610,6 +612,7 @@ async def _get_proposed_change_schema_integrity_constraints( if node_kind not in node_diff_field_summary_map: node_diff_field_summary_map[node_kind] = NodeDiffFieldSummary(kind=node_kind) field_summary = node_diff_field_summary_map[node_kind] + field_summary.node_uuids.add(node_diff["id"]) for element in node_diff["elements"]: element_name = element["name"] # The SDK diff summary reports element_type using the DiffElementType member name @@ -620,8 +623,9 @@ async def _get_proposed_change_schema_integrity_constraints( elif element_type == DiffElementType.ATTRIBUTE.name: field_summary.attribute_names.add(element_name) - determiner = ConstraintValidatorDeterminer(schema_branch=schema) - return await determiner.get_constraints(node_diffs=list(node_diff_field_summary_map.values())) + async with db.start_session(read_only=True) as session_db: + determiner = build_constraint_validator_determiner(db=session_db, branch=branch, schema_branch=schema) + return await determiner.get_constraints(node_diffs=list(node_diff_field_summary_map.values())) @flow(name="proposed-changed-repository-checks", flow_run_name="Process user defined checks") From 79eb72cd6e52e0cb8932a7d6ce7a9c6622b54c77 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 18:08:48 -0700 Subject: [PATCH 15/31] build and pass constraint classes through rebase flow --- backend/infrahub/core/branch/tasks.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/backend/infrahub/core/branch/tasks.py b/backend/infrahub/core/branch/tasks.py index 239143f9ee7..583353d5e3b 100644 --- a/backend/infrahub/core/branch/tasks.py +++ b/backend/infrahub/core/branch/tasks.py @@ -35,7 +35,8 @@ from infrahub.core.migrations.runner import MigrationRunner from infrahub.core.schema.update_coordinator import MigrationExecutor, SchemaUpdateCoordinator from infrahub.core.timestamp import Timestamp -from infrahub.core.validators.determiner import ConstraintValidatorDeterminer +from infrahub.core.validators.constraint_merge import build_constraint_info_merger +from infrahub.core.validators.determiner import build_constraint_validator_determiner from infrahub.core.validators.models.validate_migration import SchemaValidateMigrationData from infrahub.core.validators.tasks import schema_validate_migrations from infrahub.dependencies.registry import get_component_registry @@ -66,6 +67,7 @@ if TYPE_CHECKING: from logging import Logger, LoggerAdapter + from infrahub.core.models import SchemaUpdateConstraintInfo from infrahub.database import InfrahubDatabase @@ -175,14 +177,19 @@ async def rebase_branch(branch: str, context: InfrahubContext, send_events: bool ) candidate_schema = schema_analyzer.get_candidate_schema() - determiner = ConstraintValidatorDeterminer(schema_branch=candidate_schema) - constraints = await determiner.get_constraints(node_diffs=node_diff_field_summaries) + determiner = build_constraint_validator_determiner( + db=db, branch=user_branch, schema_branch=candidate_schema, at=rebase_at + ) + data_diff_constraints = await determiner.get_constraints(node_diffs=node_diff_field_summaries) # If there are some changes related to the schema between this branch and main, we need to # - Run all the validations to ensure everything is correct before rebasing the branch # - Run all the migrations after the rebase + schema_diff_constraints: list[SchemaUpdateConstraintInfo] = [] if user_branch.has_schema_changes: - constraints += await schema_analyzer.calculate_validations(target_schema=candidate_schema) + schema_diff_constraints = await schema_analyzer.calculate_validations(target_schema=candidate_schema) + merger = build_constraint_info_merger(schema_branch=candidate_schema) + constraints = merger.merge(data_diff_constraints, schema_diff_constraints) if constraints: responses = await schema_validate_migrations( message=SchemaValidateMigrationData( From 5bf5e604f241328f9da42455cc590dd55c553d2e Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 18:26:49 -0700 Subject: [PATCH 16/31] update PC pipeline tests --- .../requests/test_proposed_change.py | 135 +++++------------- ...hema_integrity_relationship_constraints.py | 2 +- 2 files changed, 38 insertions(+), 99 deletions(-) diff --git a/backend/tests/component/message_bus/operations/requests/test_proposed_change.py b/backend/tests/component/message_bus/operations/requests/test_proposed_change.py index 4d2b747564a..02eccdc87b3 100644 --- a/backend/tests/component/message_bus/operations/requests/test_proposed_change.py +++ b/backend/tests/component/message_bus/operations/requests/test_proposed_change.py @@ -128,105 +128,44 @@ async def test_get_proposed_change_schema_integrity_constraints( ) -> None: schema = registry.schema.get_schema_branch(name=default_branch.name) constraints = await _get_proposed_change_schema_integrity_constraints( - schema=schema, diff_summary=branch_diff_01_summary + db=db, schema=schema, diff_summary=branch_diff_01_summary, branch=default_branch ) - non_generate_profile_constraints = [c for c in constraints if c.constraint_name != "node.generate_profile.update"] - assert len(constraints) == 12 - assert len(non_generate_profile_constraints) == 12 - # node-level property constraints must be scoped to the kinds present in the diff - node_constraint_kinds = {c.path.schema_kind for c in constraints if c.path.path_type is SchemaPathType.NODE} - assert node_constraint_kinds == {"TestPerson"} - dumped_constraints = [c.model_dump() for c in non_generate_profile_constraints] - assert { - "constraint_name": "relationship.optional.update", - "path": { - "field_name": "cars", - "path_type": SchemaPathType.RELATIONSHIP, - "property_name": "optional", - "schema_id": None, - "schema_kind": "TestPerson", - }, - } in dumped_constraints - assert { - "constraint_name": "relationship.peer.update", - "path": { - "field_name": "cars", - "path_type": SchemaPathType.RELATIONSHIP, - "property_name": "peer", - "schema_id": None, - "schema_kind": "TestPerson", - }, - } in dumped_constraints - assert { - "constraint_name": "relationship.cardinality.update", - "path": { - "field_name": "cars", - "path_type": SchemaPathType.RELATIONSHIP, - "property_name": "cardinality", - "schema_id": None, - "schema_kind": "TestPerson", - }, - } in dumped_constraints - assert { - "constraint_name": "relationship.min_count.update", - "path": { - "field_name": "cars", - "path_type": SchemaPathType.RELATIONSHIP, - "property_name": "min_count", - "schema_id": None, - "schema_kind": "TestPerson", - }, - } in dumped_constraints - assert { - "constraint_name": "relationship.max_count.update", - "path": { - "field_name": "cars", - "path_type": SchemaPathType.RELATIONSHIP, - "property_name": "max_count", - "schema_id": None, - "schema_kind": "TestPerson", - }, - } in dumped_constraints - assert { - "constraint_name": "attribute.optional.update", - "path": { - "field_name": "height", - "path_type": SchemaPathType.ATTRIBUTE, - "property_name": "optional", - "schema_id": None, - "schema_kind": "TestPerson", - }, - } in dumped_constraints - assert { - "constraint_name": "attribute.unique.update", - "path": { - "field_name": "height", - "path_type": SchemaPathType.ATTRIBUTE, - "property_name": "unique", - "schema_id": None, - "schema_kind": "TestPerson", - }, - } in dumped_constraints - assert { - "constraint_name": "attribute.optional.update", - "path": { - "field_name": "name", - "path_type": SchemaPathType.ATTRIBUTE, - "property_name": "optional", - "schema_id": None, - "schema_kind": "TestPerson", - }, - } in dumped_constraints - assert { - "constraint_name": "attribute.unique.update", - "path": { - "field_name": "name", - "path_type": SchemaPathType.ATTRIBUTE, - "property_name": "unique", - "schema_id": None, - "schema_kind": "TestPerson", - }, - } in dumped_constraints + # the diff changes name on one TestPerson and height+cars on another; both are TestPerson, so the + # uniqueness check (name participates) is scoped to both changed nodes while every field-level + # check spans the population (node_uuids=None) + person_uuids = ("11111111-1111-1111-1111-111111111111", "22222222-2222-2222-2222-222222222222") + actual = { + ( + c.constraint_name, + c.path.schema_kind, + c.path.field_name, + c.path.property_name, + c.path.path_type, + tuple(c.node_uuids) if c.node_uuids is not None else None, + ) + for c in constraints + } + assert actual == { + ("attribute.kind.update", "TestPerson", "name", "kind", SchemaPathType.ATTRIBUTE, None), + ("attribute.kind.update", "TestPerson", "height", "kind", SchemaPathType.ATTRIBUTE, None), + ("attribute.optional.update", "TestPerson", "name", "optional", SchemaPathType.ATTRIBUTE, None), + ("attribute.optional.update", "TestPerson", "height", "optional", SchemaPathType.ATTRIBUTE, None), + ("attribute.unique.update", "TestPerson", "name", "unique", SchemaPathType.ATTRIBUTE, None), + ("attribute.unique.update", "TestPerson", "height", "unique", SchemaPathType.ATTRIBUTE, None), + ("relationship.optional.update", "TestPerson", "cars", "optional", SchemaPathType.RELATIONSHIP, None), + ("relationship.peer.update", "TestPerson", "cars", "peer", SchemaPathType.RELATIONSHIP, None), + ("relationship.cardinality.update", "TestPerson", "cars", "cardinality", SchemaPathType.RELATIONSHIP, None), + ("relationship.min_count.update", "TestPerson", "cars", "min_count", SchemaPathType.RELATIONSHIP, None), + ("relationship.max_count.update", "TestPerson", "cars", "max_count", SchemaPathType.RELATIONSHIP, None), + ( + "node.uniqueness_constraints.update", + "TestPerson", + "uniqueness_constraints", + "uniqueness_constraints", + SchemaPathType.NODE, + person_uuids, + ), + } async def test_schema_integrity( diff --git a/backend/tests/integration/proposed_change/test_schema_integrity_relationship_constraints.py b/backend/tests/integration/proposed_change/test_schema_integrity_relationship_constraints.py index 1a9a94c687a..38f769a7376 100644 --- a/backend/tests/integration/proposed_change/test_schema_integrity_relationship_constraints.py +++ b/backend/tests/integration/proposed_change/test_schema_integrity_relationship_constraints.py @@ -91,7 +91,7 @@ async def test_relationship_change_produces_schema_integrity_constraints( schema_branch = registry.schema.get_schema_branch(name=branch.name) constraints = await _get_proposed_change_schema_integrity_constraints( - schema=schema_branch, diff_summary=diff_summary + db=db, schema=schema_branch, diff_summary=diff_summary, branch=source ) constrained_fields = {(c.path.schema_kind, c.path.field_name) for c in constraints} From 57f47ca4f4652a5a5b1038920085ef6b65dc36dd Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 18:45:30 -0700 Subject: [PATCH 17/31] update MergeConstraintValidator --- backend/infrahub/core/merge/builder.py | 8 ++++- backend/infrahub/core/merge/constraints.py | 30 +++++++++++++------ .../core/diff/test_coordinator_lock.py | 13 ++++++-- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/backend/infrahub/core/merge/builder.py b/backend/infrahub/core/merge/builder.py index 30c7dc3cf44..cc7fb2665e3 100644 --- a/backend/infrahub/core/merge/builder.py +++ b/backend/infrahub/core/merge/builder.py @@ -9,6 +9,7 @@ from infrahub.core.diff.repository.repository import DiffRepository from infrahub.core.registry import registry from infrahub.core.schema.update_coordinator import SchemaUpdateCoordinator +from infrahub.core.validators.tasks import schema_validate_migrations from infrahub.dependencies.registry import get_component_registry from infrahub.workers.dependencies import get_cache, get_event_service, get_workflow @@ -58,7 +59,12 @@ async def build_branch_merge_orchestrator( diff_repository=diff_repository, schema_manager=registry.schema, ) - constraint_validator = MergeConstraintValidator(db=db, branch=source_branch, diff_repository=diff_repository) + constraint_validator = MergeConstraintValidator( + db=db, + branch=source_branch, + diff_repository=diff_repository, + migration_validator=schema_validate_migrations, + ) graph_merger = GraphMerger( db=db, source_branch=source_branch, diff --git a/backend/infrahub/core/merge/constraints.py b/backend/infrahub/core/merge/constraints.py index 9b994e90be6..36672344daf 100644 --- a/backend/infrahub/core/merge/constraints.py +++ b/backend/infrahub/core/merge/constraints.py @@ -5,11 +5,13 @@ from infrahub.core.diff.model.diff import SchemaConflict from infrahub.core.diff.model.path import BranchTrackingId -from infrahub.core.validators.determiner import ConstraintValidatorDeterminer +from infrahub.core.validators.constraint_merge import build_constraint_info_merger +from infrahub.core.validators.determiner import build_constraint_validator_determiner from infrahub.core.validators.models.validate_migration import SchemaValidateMigrationData -from infrahub.core.validators.tasks import schema_validate_migrations if TYPE_CHECKING: + from collections.abc import Awaitable, Callable + from infrahub.core.branch import Branch from infrahub.core.diff.repository.repository import DiffRepository from infrahub.core.models import SchemaUpdateConstraintInfo @@ -18,6 +20,8 @@ from infrahub.core.validators.models.validate_migration import SchemaValidatorPathResponseData from infrahub.database import InfrahubDatabase + MigrationValidator = Callable[[SchemaValidateMigrationData], Awaitable[list[SchemaValidatorPathResponseData]]] + @dataclass class MergeConstraintValidationResult: @@ -95,27 +99,35 @@ class MergeConstraintValidator: during the merge, so its pre-resolution cross-branch state is not a real violation. """ - def __init__(self, db: InfrahubDatabase, branch: Branch, diff_repository: DiffRepository) -> None: + def __init__( + self, + db: InfrahubDatabase, + branch: Branch, + diff_repository: DiffRepository, + migration_validator: MigrationValidator, + ) -> None: self.db = db self.branch = branch self.diff_repository = diff_repository + self.migration_validator = migration_validator async def validate( self, candidate_schema: SchemaBranch, schema_diff_constraints: list[SchemaUpdateConstraintInfo] ) -> MergeConstraintValidationResult: - determiner = ConstraintValidatorDeterminer(schema_branch=candidate_schema) + determiner = build_constraint_validator_determiner( + db=self.db, branch=self.branch, schema_branch=candidate_schema + ) node_field_summaries = await self.diff_repository.get_node_field_summaries( diff_branch_name=self.branch.name, tracking_id=BranchTrackingId(name=self.branch.name) ) data_diff_constraints = await determiner.get_constraints(node_diffs=node_field_summaries) - constraints = set(data_diff_constraints + schema_diff_constraints) + merger = build_constraint_info_merger(schema_branch=candidate_schema) + constraints = merger.merge(data_diff_constraints, schema_diff_constraints) if not constraints: return MergeConstraintValidationResult() - responses = await schema_validate_migrations( - message=SchemaValidateMigrationData( - branch=self.branch, schema_branch=candidate_schema, constraints=list(constraints) - ) + responses = await self.migration_validator( + SchemaValidateMigrationData(branch=self.branch, schema_branch=candidate_schema, constraints=constraints) ) conflicted_fields = await gather_conflicted_fields( diff_repository=self.diff_repository, branch_name=self.branch.name diff --git a/backend/tests/component/core/diff/test_coordinator_lock.py b/backend/tests/component/core/diff/test_coordinator_lock.py index f0030dcdc8a..6cf45c2524a 100644 --- a/backend/tests/component/core/diff/test_coordinator_lock.py +++ b/backend/tests/component/core/diff/test_coordinator_lock.py @@ -19,6 +19,7 @@ from infrahub.core.node import Node from infrahub.core.schema.schema_branch import SchemaBranch from infrahub.core.timestamp import Timestamp +from infrahub.core.validators.tasks import schema_validate_migrations from infrahub.database import InfrahubDatabase, get_db from infrahub.dependencies.registry import get_component_registry @@ -205,7 +206,12 @@ async def test_diff_update_blocks_merge( diff_repository=diff_repository, schema_manager=registry.schema, ), - constraint_validator=MergeConstraintValidator(db=db, branch=diff_branch, diff_repository=diff_repository), + constraint_validator=MergeConstraintValidator( + db=db, + branch=diff_branch, + diff_repository=diff_repository, + migration_validator=schema_validate_migrations, + ), ) results = await asyncio.gather( @@ -258,7 +264,10 @@ async def test_merge_blocks_diff_update( schema_manager=registry.schema, ), constraint_validator=MergeConstraintValidator( - db=db2, branch=diff_branch, diff_repository=diff_repository_2 + db=db2, + branch=diff_branch, + diff_repository=diff_repository_2, + migration_validator=schema_validate_migrations, ), ) From 77b106c151bf650c4f55dce832984ba1b71b7d90 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 18:47:55 -0700 Subject: [PATCH 18/31] formatting --- .../unit/core/validators/test_constraint_deduplicator.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/backend/tests/unit/core/validators/test_constraint_deduplicator.py b/backend/tests/unit/core/validators/test_constraint_deduplicator.py index a8c3bc839b7..f02604e087f 100644 --- a/backend/tests/unit/core/validators/test_constraint_deduplicator.py +++ b/backend/tests/unit/core/validators/test_constraint_deduplicator.py @@ -45,11 +45,15 @@ def _schema_branch() -> SchemaBranch: ) branch.set( name="TestElectricCar", - schema=NodeSchema(name="ElectricCar", namespace="Test", inherit_from=["TestCar"], uniqueness_constraints=[["name__value"]]), + schema=NodeSchema( + name="ElectricCar", namespace="Test", inherit_from=["TestCar"], uniqueness_constraints=[["name__value"]] + ), ) branch.set( name="TestGazCar", - schema=NodeSchema(name="GazCar", namespace="Test", inherit_from=["TestCar"], uniqueness_constraints=[["name__value"]]), + schema=NodeSchema( + name="GazCar", namespace="Test", inherit_from=["TestCar"], uniqueness_constraints=[["name__value"]] + ), ) branch.set( name="TestSpecialCar", From 8fd2de51b53e8183f43c2ad654bdb325569f6630 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 20:57:31 -0700 Subject: [PATCH 19/31] fix uniqueness constraint test expectations for node_uuids field The constraint result model gained a node_uuids field, so model_dump() now emits 'node_uuids': None. Update the two schema-validate tests whose hardcoded expected dicts predate the field. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../tests/component/core/schema_manager/test_manager_schema.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/backend/tests/component/core/schema_manager/test_manager_schema.py b/backend/tests/component/core/schema_manager/test_manager_schema.py index 33aad71b510..26037be5793 100644 --- a/backend/tests/component/core/schema_manager/test_manager_schema.py +++ b/backend/tests/component/core/schema_manager/test_manager_schema.py @@ -2902,6 +2902,7 @@ async def test_schema_branch_validate_check_missing( "schema_id": None, "schema_kind": "TestingCriticality", }, + "node_uuids": None, }, ], "enforce_update_support": True, @@ -3003,6 +3004,7 @@ async def test_schema_branch_validate_add_node_relationships( "schema_id": None, "schema_kind": "TestingCriticality", }, + "node_uuids": None, }, { "constraint_name": "node.relationship.add", @@ -3013,6 +3015,7 @@ async def test_schema_branch_validate_add_node_relationships( "schema_id": None, "schema_kind": "TestingCriticality", }, + "node_uuids": None, }, ], "enforce_update_support": True, From e479df1c3a952b7fc040439a4a46a5492c39a8a5 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 21:01:00 -0700 Subject: [PATCH 20/31] render targeted uniqueness values as strings in error paths The targeted validation path emitted attribute values with their native graph type, so an integer attribute rendered as value=8 while the full-population path rendered '8'. Stringify the value when building the data path so error messages are identical across both paths. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/infrahub/core/validators/uniqueness/checker.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/backend/infrahub/core/validators/uniqueness/checker.py b/backend/infrahub/core/validators/uniqueness/checker.py index 69665697587..9582d35ca99 100644 --- a/backend/infrahub/core/validators/uniqueness/checker.py +++ b/backend/infrahub/core/validators/uniqueness/checker.py @@ -187,11 +187,14 @@ def _violation_to_data_paths( property_name = "id" path_type = PathType.RELATIONSHIP_ONE peer_id: str | None = value + path_value: str | None = value else: field_name = element.active_attribute_schema.name property_name = "value" path_type = PathType.ATTRIBUTE peer_id = None + # value is always a str + path_value = None if value is None else str(value) for node_id in involved_node_ids: data_paths.append( DataPath( @@ -201,7 +204,7 @@ def _violation_to_data_paths( kind=schema.kind, field_name=field_name, property_name=property_name, - value=value, + value=path_value, peer_id=peer_id, ) ) From e436def38fdebd1973ed0e4a5bb366517561cbdb Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 21:06:28 -0700 Subject: [PATCH 21/31] reset uniqueness scoper cache when determiner re-initializes The determiner re-initializes its node-diff index on each get_constraints call, but the uniqueness scoper's per-kind scope cache was never cleared, so a reused determiner could return scopes computed from a previous diff. Add a reset() on the scoper and call it alongside index initialization. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/infrahub/core/validators/determiner.py | 1 + backend/infrahub/core/validators/uniqueness/scope.py | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/backend/infrahub/core/validators/determiner.py b/backend/infrahub/core/validators/determiner.py index 5706dc6e723..5092443daa3 100644 --- a/backend/infrahub/core/validators/determiner.py +++ b/backend/infrahub/core/validators/determiner.py @@ -43,6 +43,7 @@ async def get_constraints( self, node_diffs: list[NodeDiffFieldSummary], filter_invalid: bool = True ) -> list[SchemaUpdateConstraintInfo]: self.node_diff_index.initialize(node_diffs) + self.uniqueness_scoper.reset() constraints: list[SchemaUpdateConstraintInfo] = [] if not node_diffs: return constraints diff --git a/backend/infrahub/core/validators/uniqueness/scope.py b/backend/infrahub/core/validators/uniqueness/scope.py index 7e5f1e14d4d..23dd2268544 100644 --- a/backend/infrahub/core/validators/uniqueness/scope.py +++ b/backend/infrahub/core/validators/uniqueness/scope.py @@ -62,9 +62,13 @@ def __init__( self.dependent_resolver = dependent_resolver self.node_diff_index = node_diff_index # scopes are recomputed for the same kind across the trigger check and the uuid resolution; - # the schema branch and node-diff index are fixed for this scoper's lifetime, so cache them + # the cache is valid only for the node-diff index's current contents self._scope_cache: dict[str, UniquenessScopeForKind] = {} + def reset(self) -> None: + """Drop cached scopes so the next lookup recomputes against the current node-diff index.""" + self._scope_cache = {} + def requires_validation(self, schema: MainSchemaTypes) -> bool: return self._scope(schema=schema).requires_validation From 3c01c9c1fc3d51defb027af6d0f2f55a98de3f71 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 21:18:28 -0700 Subject: [PATCH 22/31] fall back to full-population scan for peer-attribute uniqueness The batched targeted query cannot read an attribute of a related peer (a constraint element such as owner__name) and raises on one. Detect such constraints and validate the whole population for that schema instead of scoping to the changed nodes, since the full-population path supports them. Removes the xfail markers on the two peer-attribute tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/validators/uniqueness/checker.py | 18 +++++++++++++++++- .../test_uniqueness_checker_node_scoped.py | 16 ---------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/backend/infrahub/core/validators/uniqueness/checker.py b/backend/infrahub/core/validators/uniqueness/checker.py index 9582d35ca99..26923cb7c28 100644 --- a/backend/infrahub/core/validators/uniqueness/checker.py +++ b/backend/infrahub/core/validators/uniqueness/checker.py @@ -73,7 +73,9 @@ def supports(self, request: SchemaConstraintValidatorRequest) -> bool: return request.constraint_name == self.name async def check(self, request: SchemaConstraintValidatorRequest) -> list[GroupedDataPaths]: - if request.node_uuids is None: + if request.node_uuids is None or not self._supports_targeted( + schema=request.node_schema, schema_branch=request.schema_branch + ): non_unique_nodes = await self.check_one_schema( schema=request.node_schema, branch=request.branch, schema_branch=request.schema_branch ) @@ -91,6 +93,20 @@ async def check(self, request: SchemaConstraintValidatorRequest) -> list[Grouped ) ] + def _supports_targeted(self, schema: MainSchemaTypes, schema_branch: SchemaBranch) -> bool: + """Whether every uniqueness constraint of the schema can be checked by the targeted query. + + The targeted query compares node attributes by value and cardinality-one relationships by + peer id, but cannot read an attribute of a related peer (a constraint element such as + "owner__name"). A schema with any such element falls back to full-population validation, + which does support it, rather than being scoped to the changed nodes. + """ + for constraint_path in schema.get_unique_constraint_schema_attribute_paths(schema_branch=schema_branch): + for element in constraint_path.attributes_paths: + if element.relationship_schema is not None and element.attribute_schema is not None: + return False + return True + async def _check_targeted( self, schema: MainSchemaTypes, diff --git a/backend/tests/component/core/constraint_validators/test_uniqueness_checker_node_scoped.py b/backend/tests/component/core/constraint_validators/test_uniqueness_checker_node_scoped.py index 96cd095740f..3633abec729 100644 --- a/backend/tests/component/core/constraint_validators/test_uniqueness_checker_node_scoped.py +++ b/backend/tests/component/core/constraint_validators/test_uniqueness_checker_node_scoped.py @@ -1,5 +1,3 @@ -import pytest - from infrahub.core import registry from infrahub.core.branch import Branch from infrahub.core.constants import SchemaPathType @@ -117,13 +115,6 @@ async def test_full_scan_when_node_uuids_is_none( (car_prius_main.id, "nbr_seats", "5"), } - @pytest.mark.xfail( - reason="peer-attribute uniqueness (owner__height) is not supported by the batched targeted " - "query; such constraints are rejected at schema load, so this path is unreachable in a " - "valid schema. Full-population validation still covers it.", - raises=ValueError, - strict=True, - ) async def test_targeted_cross_kind_peer_attribute_collision( self, db: InfrahubDatabase, @@ -147,13 +138,6 @@ async def test_targeted_cross_kind_peer_attribute_collision( assert (car_accord_main.id, "owner", "180") in violations assert (car_camry_main.id, "owner", "180") in violations - @pytest.mark.xfail( - reason="peer-attribute uniqueness (owner__height) is not supported by the batched targeted " - "query; the determiner still resolves the cross-kind change, but the checker rejects the " - "peer-attribute constraint. Such constraints cannot exist in a valid schema.", - raises=ValueError, - strict=True, - ) async def test_cross_kind_peer_change_resolves_and_detects_end_to_end( self, db: InfrahubDatabase, From 7a09ce8b195815b46c2cf1244b409e7ba00bc2f3 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Wed, 22 Jul 2026 21:55:19 -0700 Subject: [PATCH 23/31] scope uniqueness validation to the nodes that changed each field The diff field summary collapsed all changed node uuids per kind, so changing one node's unique field validated every changed node of that kind. Track uuids per changed field through NodeDiffFieldSummary, the diff query, NodeDiffIndex, and the uniqueness scoper, so a uniqueness check is scoped to only the nodes whose changed field participates in it. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/infrahub/core/diff/model/path.py | 37 +++++++++++- .../infrahub/core/diff/query/field_summary.py | 44 +++++++++----- .../core/validators/node_diff_index.py | 37 +++++++----- .../core/validators/uniqueness/scope.py | 36 +++++++++-- backend/infrahub/proposed_change/tasks.py | 6 +- .../constraint_validators/test_determiner.py | 59 ++++++++++++------- .../test_uniqueness_checker_node_scoped.py | 4 +- .../test_uniqueness_scope.py | 31 ++++++++-- .../diff/repository/test_diff_repository.py | 11 ++-- .../requests/test_proposed_change.py | 10 ++-- .../core/validators/test_node_diff_index.py | 34 ++++++----- 11 files changed, 213 insertions(+), 96 deletions(-) diff --git a/backend/infrahub/core/diff/model/path.py b/backend/infrahub/core/diff/model/path.py index 9d0d5eb50a1..6ac24b0a446 100644 --- a/backend/infrahub/core/diff/model/path.py +++ b/backend/infrahub/core/diff/model/path.py @@ -102,10 +102,41 @@ def __str__(self) -> str: @dataclass class NodeDiffFieldSummary: + """Which fields changed for a kind, and which nodes changed each one. + + The per-field maps keep the correlation between a changed field and the nodes that changed it, + so a consumer can scope work to the nodes touching a specific field rather than every changed + node of the kind. ``attribute_names``/``relationship_names``/``node_uuids`` are derived views. + """ + kind: str - attribute_names: set[str] = field(default_factory=set) - relationship_names: set[str] = field(default_factory=set) - node_uuids: set[str] = field(default_factory=set) + attribute_node_uuids: dict[str, set[str]] = field(default_factory=dict) + relationship_node_uuids: dict[str, set[str]] = field(default_factory=dict) + + def add_attribute_node_uuid(self, name: str, node_uuid: str) -> None: + """Record that `node_uuid` is a node of this kind whose attribute `name` changed.""" + self.attribute_node_uuids.setdefault(name, set()).add(node_uuid) + + def add_relationship_node_uuid(self, name: str, node_uuid: str) -> None: + """Record that `node_uuid` is a node of this kind whose relationship `name` changed.""" + self.relationship_node_uuids.setdefault(name, set()).add(node_uuid) + + @property + def attribute_names(self) -> set[str]: + return set(self.attribute_node_uuids) + + @property + def relationship_names(self) -> set[str]: + return set(self.relationship_node_uuids) + + @property + def node_uuids(self) -> set[str]: + uuids: set[str] = set() + for field_uuids in self.attribute_node_uuids.values(): + uuids |= field_uuids + for field_uuids in self.relationship_node_uuids.values(): + uuids |= field_uuids + return uuids @dataclass diff --git a/backend/infrahub/core/diff/query/field_summary.py b/backend/infrahub/core/diff/query/field_summary.py index d2e31a616ff..259f98ff7f1 100644 --- a/backend/infrahub/core/diff/query/field_summary.py +++ b/backend/infrahub/core/diff/query/field_summary.py @@ -1,3 +1,4 @@ +from dataclasses import dataclass from typing import Any from infrahub.core.constants import DiffAction @@ -7,6 +8,14 @@ from ..model.path import NodeDiffFieldSummary, TrackingId +@dataclass +class FieldNodeUuidsRow: + """One changed field of a kind and the uuids of the nodes that changed it, as projected by the query.""" + + name: str + node_uuids: list[str] + + class EnrichedDiffNodeFieldSummaryQuery(Query): """Get node kind and names of all altered attributes and relationships for each kind.""" @@ -42,41 +51,48 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # noqa AND (diff_root.uuid = $diff_id OR $diff_id IS NULL) OPTIONAL MATCH (diff_root)-[:DIFF_HAS_NODE]->(n:DiffNode) WHERE n.action <> $unchanged_str - WITH n.kind AS kind, collect(DISTINCT n.uuid) AS node_uuids + WITH DISTINCT n.kind AS kind CALL (kind) { OPTIONAL MATCH (n:DiffNode {kind: kind})-[:DIFF_HAS_ATTRIBUTE]->(a:DiffAttribute) WHERE n.action <> $unchanged_str AND a.action <> $unchanged_str - WITH DISTINCT a.name AS attr_name - RETURN collect(attr_name) AS attr_names + WITH a.name AS attr_name, collect(DISTINCT n.uuid) AS attr_node_uuids + WHERE attr_name IS NOT NULL + RETURN collect({name: attr_name, node_uuids: attr_node_uuids}) AS attr_name_uuids } - WITH kind, node_uuids, attr_names + WITH kind, attr_name_uuids CALL (kind) { OPTIONAL MATCH (n:DiffNode {kind: kind})-[:DIFF_HAS_RELATIONSHIP]->(r:DiffRelationship) WHERE n.action <> $unchanged_str AND r.action <> $unchanged_str - WITH DISTINCT r.name AS rel_name - RETURN collect(rel_name) AS rel_names + WITH r.name AS rel_name, collect(DISTINCT n.uuid) AS rel_node_uuids + WHERE rel_name IS NOT NULL + RETURN collect({name: rel_name, node_uuids: rel_node_uuids}) AS rel_name_uuids } """ self.add_to_query(query=query) self.order_by = ["kind"] - self.return_labels = ["kind", "node_uuids", "attr_names", "rel_names"] + self.return_labels = ["kind", "attr_name_uuids", "rel_name_uuids"] async def get_field_summaries(self) -> list[NodeDiffFieldSummary]: field_summaries = [] for result in self.get_results(): kind = result.get_as_type(label="kind", return_type=str) - node_uuids = result.get_as_type(label="node_uuids", return_type=list[str]) - attr_names = result.get_as_type(label="attr_names", return_type=list[str]) - rel_names = result.get_as_type(label="rel_names", return_type=list[str]) - if attr_names or rel_names: + attribute_node_uuids = self._to_field_uuids( + result.get_as_list_of_type(label="attr_name_uuids", return_type=FieldNodeUuidsRow) + ) + relationship_node_uuids = self._to_field_uuids( + result.get_as_list_of_type(label="rel_name_uuids", return_type=FieldNodeUuidsRow) + ) + if attribute_node_uuids or relationship_node_uuids: field_summaries.append( NodeDiffFieldSummary( kind=kind, - attribute_names=set(attr_names), - relationship_names=set(rel_names), - node_uuids=set(node_uuids), + attribute_node_uuids=attribute_node_uuids, + relationship_node_uuids=relationship_node_uuids, ) ) return field_summaries + + def _to_field_uuids(self, rows: list[FieldNodeUuidsRow]) -> dict[str, set[str]]: + return {row.name: set(row.node_uuids) for row in rows} diff --git a/backend/infrahub/core/validators/node_diff_index.py b/backend/infrahub/core/validators/node_diff_index.py index 340632c40c0..4da7d4f1d62 100644 --- a/backend/infrahub/core/validators/node_diff_index.py +++ b/backend/infrahub/core/validators/node_diff_index.py @@ -9,26 +9,28 @@ class NodeDiffIndex: """Which fields and nodes changed, per kind, in a data diff. + The node uuids are kept per changed field, not merged per kind, so a consumer can ask which + nodes changed a specific field rather than every node of the kind. + `initialize` must be called for a given diff before any query method is used. """ def __init__(self) -> None: self._initialized: bool = False self._kinds: set[str] = set() - self._attribute_names_by_kind: dict[str, set[str]] = {} - self._relationship_names_by_kind: dict[str, set[str]] = {} - self._node_uuids_by_kind: dict[str, set[str]] = {} + self._attribute_uuids_by_kind_field: dict[tuple[str, str], set[str]] = {} + self._relationship_uuids_by_kind_field: dict[tuple[str, str], set[str]] = {} def initialize(self, node_diffs: list[NodeDiffFieldSummary]) -> None: self._kinds = set() - self._attribute_names_by_kind = {} - self._relationship_names_by_kind = {} - self._node_uuids_by_kind = {} + self._attribute_uuids_by_kind_field = {} + self._relationship_uuids_by_kind_field = {} for node_diff in node_diffs: self._kinds.add(node_diff.kind) - self._attribute_names_by_kind.setdefault(node_diff.kind, set()).update(node_diff.attribute_names) - self._relationship_names_by_kind.setdefault(node_diff.kind, set()).update(node_diff.relationship_names) - self._node_uuids_by_kind.setdefault(node_diff.kind, set()).update(node_diff.node_uuids) + for name, uuids in node_diff.attribute_node_uuids.items(): + self._attribute_uuids_by_kind_field.setdefault((node_diff.kind, name), set()).update(uuids) + for name, uuids in node_diff.relationship_node_uuids.items(): + self._relationship_uuids_by_kind_field.setdefault((node_diff.kind, name), set()).update(uuids) self._initialized = True def _ensure_initialized(self) -> None: @@ -42,15 +44,18 @@ def kinds(self) -> set[str]: def has_attribute_diff(self, kind: str, name: str) -> bool: self._ensure_initialized() - return name in self._attribute_names_by_kind.get(kind, set()) + return (kind, name) in self._attribute_uuids_by_kind_field def has_relationship_diff(self, kind: str, name: str) -> bool: self._ensure_initialized() - return name in self._relationship_names_by_kind.get(kind, set()) + return (kind, name) in self._relationship_uuids_by_kind_field + + def get_uuids_for_attribute(self, kind: str, name: str) -> set[str]: + """UUIDs of the nodes of `kind` whose attribute `name` changed in the diff.""" + self._ensure_initialized() + return set(self._attribute_uuids_by_kind_field.get((kind, name), set())) - def uuids_for_kinds(self, kinds: set[str]) -> set[str]: + def get_uuids_for_relationship(self, kind: str, name: str) -> set[str]: + """UUIDs of the nodes of `kind` whose relationship `name` changed in the diff.""" self._ensure_initialized() - uuids: set[str] = set() - for kind in kinds: - uuids |= self._node_uuids_by_kind.get(kind, set()) - return uuids + return set(self._relationship_uuids_by_kind_field.get((kind, name), set())) diff --git a/backend/infrahub/core/validators/uniqueness/scope.py b/backend/infrahub/core/validators/uniqueness/scope.py index 23dd2268544..171a2a66415 100644 --- a/backend/infrahub/core/validators/uniqueness/scope.py +++ b/backend/infrahub/core/validators/uniqueness/scope.py @@ -104,6 +104,20 @@ def _diffed_kinds_with_field(self, schema: MainSchemaTypes, field_name: str, is_ ) return {kind for kind in kinds if check(kind=kind, name=field_name)} + def _uuids_for_field(self, kinds: set[str], field_name: str, is_relationship: bool) -> set[str]: + """UUIDs of the nodes that changed `field_name`, across the given kinds. + + Scoped to the specific field, so an unrelated change to another field of the same node's + kind does not pull that node into the uniqueness scope. + """ + uuids: set[str] = set() + for kind in kinds: + if is_relationship: + uuids |= self.node_diff_index.get_uuids_for_relationship(kind=kind, name=field_name) + else: + uuids |= self.node_diff_index.get_uuids_for_attribute(kind=kind, name=field_name) + return uuids + def _scope(self, schema: MainSchemaTypes) -> UniquenessScopeForKind: cached = self._scope_cache.get(schema.kind) if cached is None: @@ -122,12 +136,14 @@ def _compute_scope(self, schema: MainSchemaTypes) -> UniquenessScopeForKind: """ scope = UniquenessScopeForKind() for attribute_schema in schema.unique_attributes: - kinds = self._diffed_kinds_with_field( + attr_kinds = self._diffed_kinds_with_field( schema=schema, field_name=attribute_schema.name, is_relationship=False ) - if kinds: + if attr_kinds: scope.requires_validation = True - scope.object_uuids |= self.node_diff_index.uuids_for_kinds(kinds) + scope.object_uuids |= self._uuids_for_field( + kinds=attr_kinds, field_name=attribute_schema.name, is_relationship=False + ) for constraint_group in schema.uniqueness_constraints or []: for constraint_path in constraint_group: try: @@ -141,7 +157,9 @@ def _compute_scope(self, schema: MainSchemaTypes) -> UniquenessScopeForKind: ) if rel_kinds: scope.requires_validation = True - scope.object_uuids |= self.node_diff_index.uuids_for_kinds(rel_kinds) + scope.object_uuids |= self._uuids_for_field( + kinds=rel_kinds, field_name=schema_path.relationship_schema.name, is_relationship=True + ) if schema_path.attribute_schema is not None and schema_path.related_schema is not None: peer_kinds = self._diffed_kinds_with_field( schema=schema_path.related_schema, @@ -153,7 +171,11 @@ def _compute_scope(self, schema: MainSchemaTypes) -> UniquenessScopeForKind: scope.cross_kind_peer_changes.append( CrossKindPeerChange( relationship_identifier=schema_path.relationship_schema.get_identifier(), - changed_peer_uuids=self.node_diff_index.uuids_for_kinds(peer_kinds), + changed_peer_uuids=self._uuids_for_field( + kinds=peer_kinds, + field_name=schema_path.attribute_schema.name, + is_relationship=False, + ), ) ) elif schema_path.attribute_schema is not None: @@ -162,5 +184,7 @@ def _compute_scope(self, schema: MainSchemaTypes) -> UniquenessScopeForKind: ) if attr_kinds: scope.requires_validation = True - scope.object_uuids |= self.node_diff_index.uuids_for_kinds(attr_kinds) + scope.object_uuids |= self._uuids_for_field( + kinds=attr_kinds, field_name=schema_path.attribute_schema.name, is_relationship=False + ) return scope diff --git a/backend/infrahub/proposed_change/tasks.py b/backend/infrahub/proposed_change/tasks.py index f57e3dab7f9..a072db900c9 100644 --- a/backend/infrahub/proposed_change/tasks.py +++ b/backend/infrahub/proposed_change/tasks.py @@ -612,16 +612,16 @@ async def _get_proposed_change_schema_integrity_constraints( if node_kind not in node_diff_field_summary_map: node_diff_field_summary_map[node_kind] = NodeDiffFieldSummary(kind=node_kind) field_summary = node_diff_field_summary_map[node_kind] - field_summary.node_uuids.add(node_diff["id"]) + node_id = node_diff["id"] for element in node_diff["elements"]: element_name = element["name"] # The SDK diff summary reports element_type using the DiffElementType member name # (e.g. "RELATIONSHIP_ONE"), not its value ("RelationshipOne"). element_type = element["element_type"] if element_type in (DiffElementType.RELATIONSHIP_MANY.name, DiffElementType.RELATIONSHIP_ONE.name): - field_summary.relationship_names.add(element_name) + field_summary.add_relationship_node_uuid(name=element_name, node_uuid=node_id) elif element_type == DiffElementType.ATTRIBUTE.name: - field_summary.attribute_names.add(element_name) + field_summary.add_attribute_node_uuid(name=element_name, node_uuid=node_id) async with db.start_session(read_only=True) as session_db: determiner = build_constraint_validator_determiner(db=session_db, branch=branch, schema_branch=schema) diff --git a/backend/tests/component/core/constraint_validators/test_determiner.py b/backend/tests/component/core/constraint_validators/test_determiner.py index 65fcf09b6cd..690ab03c9b5 100644 --- a/backend/tests/component/core/constraint_validators/test_determiner.py +++ b/backend/tests/component/core/constraint_validators/test_determiner.py @@ -18,6 +18,11 @@ RELATIONSHIP_PROPERTIES = ("peer", "cardinality", "optional", "min_count", "max_count") +# uuids of the changed nodes carried by the test node-diffs, so a uniqueness check emitted for a +# directly-changed kind can be asserted to be scoped to exactly those nodes +CHANGED_PERSON_UUID = "person-1" +CHANGED_ELECTRIC_CAR_UUID = "electric-car-1" + class _NoDependentsResolver: async def resolve(self, node_kind: str, relationship_identifier: str, peer_uuids: list[str]) -> set[str]: @@ -46,8 +51,17 @@ def node_constraint(kind: str, property_name: str) -> SchemaUpdateConstraintInfo ) -def node_uniqueness_constraint(kind: str) -> SchemaUpdateConstraintInfo: - return node_constraint(kind, "uniqueness_constraints") +def node_uniqueness_constraint(kind: str, node_uuids: list[str] | None = None) -> SchemaUpdateConstraintInfo: + return SchemaUpdateConstraintInfo( + constraint_name="node.uniqueness_constraints.update", + path=SchemaPath( + path_type=SchemaPathType.NODE, + schema_kind=kind, + field_name="uniqueness_constraints", + property_name="uniqueness_constraints", + ), + node_uuids=node_uuids, + ) def attribute_constraint(kind: str, field_name: str, property_name: str) -> SchemaUpdateConstraintInfo: @@ -78,7 +92,7 @@ def relationship_constraint(kind: str, field_name: str, property_name: str) -> S def person_name_node_diff( person_john_main: Node, default_branch: Branch ) -> tuple[NodeDiffFieldSummary, set[SchemaUpdateConstraintInfo]]: - node_diff = NodeDiffFieldSummary(kind="TestPerson", attribute_names={"name"}) + node_diff = NodeDiffFieldSummary(kind="TestPerson", attribute_node_uuids={"name": {CHANGED_PERSON_UUID}}) schema_updated_constraint_infos = { SchemaUpdateConstraintInfo( path=SchemaPath( @@ -116,7 +130,7 @@ def person_name_node_diff( def person_cars_node_diff( person_john_main: Node, default_branch: Branch ) -> tuple[NodeDiffFieldSummary, set[SchemaUpdateConstraintInfo]]: - node_diff = NodeDiffFieldSummary(kind="TestPerson", relationship_names={"cars"}) + node_diff = NodeDiffFieldSummary(kind="TestPerson", relationship_node_uuids={"cars": set()}) schema_updated_constraint_infos = { SchemaUpdateConstraintInfo( constraint_name="relationship.min_count.update", @@ -236,7 +250,7 @@ async def test_node_property_constraints_included( property_name="parameters.max_length", ), ) - constraint_info_set.add(node_uniqueness_constraint("TestPerson")) + constraint_info_set.add(node_uniqueness_constraint("TestPerson", node_uuids=[CHANGED_PERSON_UUID])) constraint_info_set.add(max_length_param_constraint_info) constraints = await determiner.get_constraints(node_diffs=[node_diff]) @@ -254,9 +268,9 @@ async def test_uniqueness_constraint_on_peer_attribute_included( car_schema.uniqueness_constraints = [["owner__name", "color__value"]] determiner = _build_determiner(schema_branch=schema_branch) node_diff, constraint_info_set = person_name_node_diff - constraint_info_set.add(node_uniqueness_constraint("TestPerson")) + constraint_info_set.add(node_uniqueness_constraint("TestPerson", node_uuids=[CHANGED_PERSON_UUID])) # TestCar's constraint reads the name attribute of the related TestPerson, so a TestPerson - # data change can violate it even though TestCar itself has no diff + # data change can violate it even though TestCar itself has no diff. constraint_info_set.add(node_uniqueness_constraint("TestCar")) constraints = await determiner.get_constraints(node_diffs=[node_diff]) @@ -272,7 +286,7 @@ async def test_uniqueness_not_triggered_by_unrelated_field( generic_schema = schema_branch.get(name="TestCar", duplicate=False) generic_schema.uniqueness_constraints = [["name__value"]] determiner = _build_determiner(schema_branch=schema_branch) - node_diff = NodeDiffFieldSummary(kind="TestElectricCar", attribute_names={"nbr_engine"}) + node_diff = NodeDiffFieldSummary(kind="TestElectricCar", attribute_node_uuids={"nbr_engine": set()}) # nbr_engine participates in no uniqueness path, so the uniqueness check must not be # triggered on the implementation or on its generic; only the nbr_engine field constraints remain constraint_info_set = { @@ -297,13 +311,16 @@ async def test_generic_uniqueness_triggered_by_inherited_field( # `name` is inherited from the generic; a generic-level uniqueness check spans every # implementing node, so changing name on an implementation must trigger the check on the # generic (TestCar) as well as on the implementation (TestElectricCar) - node_diff = NodeDiffFieldSummary(kind="TestElectricCar", attribute_names={"name"}) + node_diff = NodeDiffFieldSummary( + kind="TestElectricCar", attribute_node_uuids={"name": {CHANGED_ELECTRIC_CAR_UUID}} + ) constraints = await determiner.get_constraints(node_diffs=[node_diff]) + # both the generic and the implementation checks are scoped to the changed implementation node expected = { - node_uniqueness_constraint("TestCar"), - node_uniqueness_constraint("TestElectricCar"), + node_uniqueness_constraint("TestCar", node_uuids=[CHANGED_ELECTRIC_CAR_UUID]), + node_uniqueness_constraint("TestElectricCar", node_uuids=[CHANGED_ELECTRIC_CAR_UUID]), attribute_constraint("TestElectricCar", "name", "kind"), attribute_constraint("TestElectricCar", "name", "optional"), attribute_constraint("TestElectricCar", "name", "unique"), @@ -322,7 +339,7 @@ async def test_uniqueness_triggered_by_generic_peer_implementation( # changes `name`, even though the peer kind named in the path (LocationGeneric) has no diff thing_schema.uniqueness_constraints = [["location__name"]] determiner = _build_determiner(schema_branch=schema_branch) - node_diff = NodeDiffFieldSummary(kind="LocationSite", attribute_names={"name"}) + node_diff = NodeDiffFieldSummary(kind="LocationSite", attribute_node_uuids={"name": set()}) constraints = await determiner.get_constraints(node_diffs=[node_diff]) @@ -340,7 +357,7 @@ async def test_uniqueness_not_triggered_by_unrelated_peer_attribute( # TestCar's uniqueness constraints read TestPerson.name; a change to an unrelated # TestPerson attribute, height, does not participate, so neither kind's uniqueness # check should trigger - node_diff = NodeDiffFieldSummary(kind="TestPerson", attribute_names={"height"}) + node_diff = NodeDiffFieldSummary(kind="TestPerson", attribute_node_uuids={"height": set()}) constraints = await determiner.get_constraints(node_diffs=[node_diff]) @@ -357,10 +374,10 @@ async def test_kind_missing_from_schema_is_skipped( schema_branch = registry.schema.get_schema_branch(name=default_branch.name) determiner = _build_determiner(schema_branch=schema_branch) node_diff, constraint_info_set = person_name_node_diff - constraint_info_set.add(node_uniqueness_constraint("TestPerson")) + constraint_info_set.add(node_uniqueness_constraint("TestPerson", node_uuids=[CHANGED_PERSON_UUID])) constraints = await determiner.get_constraints( - node_diffs=[NodeDiffFieldSummary(kind="TestDeleted", attribute_names={"name"}), node_diff] + node_diffs=[NodeDiffFieldSummary(kind="TestDeleted", attribute_node_uuids={"name": set()}), node_diff] ) # TestDeleted is absent from the schema, so it contributes nothing; only TestPerson remains @@ -377,7 +394,7 @@ async def test_internal_schema_kinds_only_when_in_diff( ) determiner = _build_determiner(schema_branch=schema_branch) node_diff, constraint_info_set = person_name_node_diff - constraint_info_set.add(node_uniqueness_constraint("TestPerson")) + constraint_info_set.add(node_uniqueness_constraint("TestPerson", node_uuids=[CHANGED_PERSON_UUID])) constraints = await determiner.get_constraints(node_diffs=[node_diff]) # internal schema kinds are absent from the diff, so none contribute constraints @@ -388,7 +405,7 @@ async def test_internal_schema_kinds_only_when_in_diff( constraints = await determiner.get_constraints( node_diffs=[ node_diff, - *(NodeDiffFieldSummary(kind=kind, attribute_names={"name"}) for kind in internal_kinds), + *(NodeDiffFieldSummary(kind=kind, attribute_node_uuids={"name": set()}) for kind in internal_kinds), ] ) # once an internal schema kind is in the diff, its uniqueness constraint must be validated @@ -409,8 +426,8 @@ async def test_hierarchy_constraints_selected_for_both_endpoint_kinds( # emits only the hierarchy constraint whose relationship actually changed there, and no # uniqueness constraint since `name` (the only unique field) is not in the diff. node_diffs = [ - NodeDiffFieldSummary(kind="LocationRack", relationship_names={"parent"}), - NodeDiffFieldSummary(kind="LocationSite", relationship_names={"children"}), + NodeDiffFieldSummary(kind="LocationRack", relationship_node_uuids={"parent": set()}), + NodeDiffFieldSummary(kind="LocationSite", relationship_node_uuids={"children": set()}), ] expected = { node_constraint("LocationRack", "parent"), @@ -435,7 +452,7 @@ async def test_unparseable_uniqueness_constraint_element_is_skipped_and_logged( determiner = _build_determiner(schema_branch=schema_branch) # `height` is not a unique attribute, so evaluating uniqueness must fall through to parsing # the (unparseable) constraint group rather than short-circuiting on a unique attribute. - node_diff = NodeDiffFieldSummary(kind="TestPerson", attribute_names={"height"}) + node_diff = NodeDiffFieldSummary(kind="TestPerson", attribute_node_uuids={"height": set()}) with caplog.at_level(logging.WARNING): constraints = await determiner.get_constraints(node_diffs=[node_diff]) @@ -453,7 +470,7 @@ async def test_hierarchy_constraint_scoped_to_changed_relationship( determiner = _build_determiner(schema_branch=schema_branch) # Re-parenting a rack changes only its `parent` relationship, so only the parent hierarchy # constraint may be emitted for that kind, never the children one. - node_diffs = [NodeDiffFieldSummary(kind="LocationSite", relationship_names={"parent"})] + node_diffs = [NodeDiffFieldSummary(kind="LocationSite", relationship_node_uuids={"parent": set()})] constraints = await determiner.get_constraints(node_diffs=node_diffs) diff --git a/backend/tests/component/core/constraint_validators/test_uniqueness_checker_node_scoped.py b/backend/tests/component/core/constraint_validators/test_uniqueness_checker_node_scoped.py index 3633abec729..709d8b156f8 100644 --- a/backend/tests/component/core/constraint_validators/test_uniqueness_checker_node_scoped.py +++ b/backend/tests/component/core/constraint_validators/test_uniqueness_checker_node_scoped.py @@ -158,9 +158,7 @@ async def test_cross_kind_peer_change_resolves_and_detects_end_to_end( determiner = build_constraint_validator_determiner( db=db, branch=branch, schema_branch=registry.schema.get_schema_branch(name=branch.name) ) - person_change = NodeDiffFieldSummary( - kind="TestPerson", attribute_names={"height"}, node_uuids={person_john_main.id} - ) + person_change = NodeDiffFieldSummary(kind="TestPerson", attribute_node_uuids={"height": {person_john_main.id}}) constraints = await determiner.get_constraints(node_diffs=[person_change]) diff --git a/backend/tests/component/core/constraint_validators/test_uniqueness_scope.py b/backend/tests/component/core/constraint_validators/test_uniqueness_scope.py index cb1570bbfa4..5ecd2c6d833 100644 --- a/backend/tests/component/core/constraint_validators/test_uniqueness_scope.py +++ b/backend/tests/component/core/constraint_validators/test_uniqueness_scope.py @@ -35,13 +35,34 @@ async def test_same_kind_change_scopes_to_changed_nodes( scoper = _scoper( schema_branch, _RecordingResolver(dependents=set()), - [NodeDiffFieldSummary(kind="TestPerson", attribute_names={"name"}, node_uuids={"person-1", "person-2"})], + [NodeDiffFieldSummary(kind="TestPerson", attribute_node_uuids={"name": {"person-1", "person-2"}})], ) person_schema = schema_branch.get(name="TestPerson") assert scoper.requires_validation(schema=person_schema) is True assert await scoper.affected_node_uuids(schema=person_schema) == ["person-1", "person-2"] + async def test_scopes_only_nodes_that_changed_the_unique_field( + self, car_person_schema: SchemaBranch, default_branch: Branch + ) -> None: + schema_branch = registry.schema.get_schema_branch(name=default_branch.name) + # person-1 changed the unique "name"; person-2 only changed the non-unique "height" + scoper = _scoper( + schema_branch, + _RecordingResolver(dependents=set()), + [ + NodeDiffFieldSummary( + kind="TestPerson", + attribute_node_uuids={"name": {"person-1"}, "height": {"person-2"}}, + ) + ], + ) + person_schema = schema_branch.get(name="TestPerson") + + assert scoper.requires_validation(schema=person_schema) is True + # only person-1 is scoped; person-2's change does not participate in uniqueness + assert await scoper.affected_node_uuids(schema=person_schema) == ["person-1"] + async def test_triggered_without_node_uuids_falls_back_to_full_scan( self, car_person_schema: SchemaBranch, default_branch: Branch ) -> None: @@ -49,7 +70,7 @@ async def test_triggered_without_node_uuids_falls_back_to_full_scan( scoper = _scoper( schema_branch, _RecordingResolver(dependents=set()), - [NodeDiffFieldSummary(kind="TestPerson", attribute_names={"name"})], + [NodeDiffFieldSummary(kind="TestPerson", attribute_node_uuids={"name": set()})], ) person_schema = schema_branch.get(name="TestPerson") @@ -64,7 +85,7 @@ async def test_unrelated_field_change_does_not_trigger( scoper = _scoper( schema_branch, _RecordingResolver(dependents=set()), - [NodeDiffFieldSummary(kind="TestPerson", attribute_names={"height"}, node_uuids={"person-1"})], + [NodeDiffFieldSummary(kind="TestPerson", attribute_node_uuids={"height": {"person-1"}})], ) person_schema = schema_branch.get(name="TestPerson") @@ -85,7 +106,7 @@ async def test_cross_kind_peer_change_resolves_dependents( scoper = _scoper( schema_branch, resolver, - [NodeDiffFieldSummary(kind="TestPerson", attribute_names={"name"}, node_uuids={"person-1"})], + [NodeDiffFieldSummary(kind="TestPerson", attribute_node_uuids={"name": {"person-1"}})], ) car_schema = schema_branch.get(name="TestCar") @@ -109,7 +130,7 @@ async def test_cross_kind_without_known_peer_uuids_falls_back_to_full_scan( scoper = _scoper( schema_branch, resolver, - [NodeDiffFieldSummary(kind="TestPerson", attribute_names={"name"})], + [NodeDiffFieldSummary(kind="TestPerson", attribute_node_uuids={"name": set()})], ) car_schema = schema_branch.get(name="TestCar") diff --git a/backend/tests/component/core/diff/repository/test_diff_repository.py b/backend/tests/component/core/diff/repository/test_diff_repository.py index 35b12c719bc..5a47b8ce773 100644 --- a/backend/tests/component/core/diff/repository/test_diff_repository.py +++ b/backend/tests/component/core/diff/repository/test_diff_repository.py @@ -630,11 +630,12 @@ async def test_get_node_field_summaries(self, diff_repository: DiffRepository) - if node.kind not in expected_map: expected_map[node.kind] = NodeDiffFieldSummary(kind=node.kind) field_summary = expected_map[node.kind] - field_summary.node_uuids.add(node.uuid) - attr_names = {a.name for a in node.attributes if a.action is not DiffAction.UNCHANGED} - field_summary.attribute_names.update(attr_names) - rel_names = {r.name for r in node.relationships if r.action is not DiffAction.UNCHANGED} - field_summary.relationship_names.update(rel_names) + for attr in node.attributes: + if attr.action is not DiffAction.UNCHANGED: + field_summary.add_attribute_node_uuid(name=attr.name, node_uuid=node.uuid) + for rel in node.relationships: + if rel.action is not DiffAction.UNCHANGED: + field_summary.add_relationship_node_uuid(name=rel.name, node_uuid=node.uuid) expected_map = {k: v for k, v in expected_map.items() if v.relationship_names or v.attribute_names} retrieved_node_field_summaries = await diff_repository.get_node_field_summaries( diff --git a/backend/tests/component/message_bus/operations/requests/test_proposed_change.py b/backend/tests/component/message_bus/operations/requests/test_proposed_change.py index 02eccdc87b3..c073c3759d5 100644 --- a/backend/tests/component/message_bus/operations/requests/test_proposed_change.py +++ b/backend/tests/component/message_bus/operations/requests/test_proposed_change.py @@ -130,10 +130,10 @@ async def test_get_proposed_change_schema_integrity_constraints( constraints = await _get_proposed_change_schema_integrity_constraints( db=db, schema=schema, diff_summary=branch_diff_01_summary, branch=default_branch ) - # the diff changes name on one TestPerson and height+cars on another; both are TestPerson, so the - # uniqueness check (name participates) is scoped to both changed nodes while every field-level - # check spans the population (node_uuids=None) - person_uuids = ("11111111-1111-1111-1111-111111111111", "22222222-2222-2222-2222-222222222222") + # the diff changes name on one TestPerson and height+cars on another; only name participates in + # uniqueness, so the uniqueness check is scoped to just the node that changed name, while every + # field-level check spans the population (node_uuids=None) + name_changed_person_uuids = ("11111111-1111-1111-1111-111111111111",) actual = { ( c.constraint_name, @@ -163,7 +163,7 @@ async def test_get_proposed_change_schema_integrity_constraints( "uniqueness_constraints", "uniqueness_constraints", SchemaPathType.NODE, - person_uuids, + name_changed_person_uuids, ), } diff --git a/backend/tests/unit/core/validators/test_node_diff_index.py b/backend/tests/unit/core/validators/test_node_diff_index.py index 24e7f8464e5..ec807faa9f9 100644 --- a/backend/tests/unit/core/validators/test_node_diff_index.py +++ b/backend/tests/unit/core/validators/test_node_diff_index.py @@ -15,20 +15,19 @@ def test_query_before_initialize_raises() -> None: with pytest.raises(RuntimeError, match=match): index.has_relationship_diff(kind="TestCar", name="owner") with pytest.raises(RuntimeError, match=match): - index.uuids_for_kinds({"TestCar"}) + index.get_uuids_for_attribute(kind="TestCar", name="name") -def test_indexes_fields_and_uuids_per_kind() -> None: +def test_indexes_fields_and_uuids_per_field() -> None: index = NodeDiffIndex() index.initialize( [ NodeDiffFieldSummary( kind="TestCar", - attribute_names={"name", "color"}, - relationship_names={"owner"}, - node_uuids={"c1", "c2"}, + attribute_node_uuids={"name": {"c1", "c2"}, "color": {"c1"}}, + relationship_node_uuids={"owner": {"c2"}}, ), - NodeDiffFieldSummary(kind="TestPerson", attribute_names={"height"}, node_uuids={"p1"}), + NodeDiffFieldSummary(kind="TestPerson", attribute_node_uuids={"height": {"p1"}}), ] ) @@ -38,32 +37,37 @@ def test_indexes_fields_and_uuids_per_kind() -> None: assert not index.has_attribute_diff(kind="Unknown", name="name") assert index.has_relationship_diff(kind="TestCar", name="owner") assert not index.has_relationship_diff(kind="TestPerson", name="owner") - assert index.uuids_for_kinds({"TestCar", "TestPerson"}) == {"c1", "c2", "p1"} - assert index.uuids_for_kinds({"Unknown"}) == set() + # uuids are scoped to the specific field, not merged across the kind + assert index.get_uuids_for_attribute(kind="TestCar", name="name") == {"c1", "c2"} + assert index.get_uuids_for_attribute(kind="TestCar", name="color") == {"c1"} + assert index.get_uuids_for_relationship(kind="TestCar", name="owner") == {"c2"} + assert index.get_uuids_for_attribute(kind="TestPerson", name="height") == {"p1"} + assert index.get_uuids_for_attribute(kind="Unknown", name="name") == set() def test_summaries_for_same_kind_are_merged() -> None: index = NodeDiffIndex() index.initialize( [ - NodeDiffFieldSummary(kind="TestCar", attribute_names={"name"}, node_uuids={"c1"}), - NodeDiffFieldSummary(kind="TestCar", attribute_names={"color"}, node_uuids={"c2"}), + NodeDiffFieldSummary(kind="TestCar", attribute_node_uuids={"name": {"c1"}}), + NodeDiffFieldSummary(kind="TestCar", attribute_node_uuids={"name": {"c2"}, "color": {"c3"}}), ] ) assert index.has_attribute_diff(kind="TestCar", name="name") assert index.has_attribute_diff(kind="TestCar", name="color") - assert index.uuids_for_kinds({"TestCar"}) == {"c1", "c2"} + assert index.get_uuids_for_attribute(kind="TestCar", name="name") == {"c1", "c2"} + assert index.get_uuids_for_attribute(kind="TestCar", name="color") == {"c3"} def test_initialize_resets_prior_state() -> None: index = NodeDiffIndex() - index.initialize([NodeDiffFieldSummary(kind="A", attribute_names={"x"}, node_uuids={"a1"})]) - index.initialize([NodeDiffFieldSummary(kind="B", attribute_names={"y"}, node_uuids={"b1"})]) + index.initialize([NodeDiffFieldSummary(kind="A", attribute_node_uuids={"x": {"a1"}})]) + index.initialize([NodeDiffFieldSummary(kind="B", attribute_node_uuids={"y": {"b1"}})]) assert index.kinds == {"B"} assert not index.has_attribute_diff(kind="A", name="x") - assert index.uuids_for_kinds({"A"}) == set() + assert index.get_uuids_for_attribute(kind="A", name="x") == set() def test_empty_diff() -> None: @@ -72,4 +76,4 @@ def test_empty_diff() -> None: assert index.kinds == set() assert not index.has_attribute_diff(kind="A", name="x") - assert index.uuids_for_kinds({"A"}) == set() + assert index.get_uuids_for_attribute(kind="A", name="x") == set() From 25fbe7f73b64247d050d0de20873570f4916b946 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Thu, 23 Jul 2026 21:19:25 -0700 Subject: [PATCH 24/31] consolidate chunks --- PR_DESCRIPTION.md | 73 +++++++++++++++++++ backend/infrahub/computed_attribute/tasks.py | 9 +-- .../core/merge/recompute_coalescing.py | 9 +-- backend/infrahub/core/recompute/bulk_write.py | 10 +-- .../core/validators/uniqueness/checker.py | 11 +-- backend/infrahub/utilities/__init__.py | 0 backend/infrahub/utilities/chunks.py | 22 ++++++ backend/tests/unit/utilities/__init__.py | 0 .../test_chunks.py} | 6 +- 9 files changed, 108 insertions(+), 32 deletions(-) create mode 100644 PR_DESCRIPTION.md create mode 100644 backend/infrahub/utilities/__init__.py create mode 100644 backend/infrahub/utilities/chunks.py create mode 100644 backend/tests/unit/utilities/__init__.py rename backend/tests/unit/{computed_attribute/test_tasks.py => utilities/test_chunks.py} (51%) diff --git a/PR_DESCRIPTION.md b/PR_DESCRIPTION.md new file mode 100644 index 00000000000..e53f4de3ad5 --- /dev/null +++ b/PR_DESCRIPTION.md @@ -0,0 +1,73 @@ +# Why + +Uniqueness validation scanned the **entire population** of a kind on every data change. Merging, rebasing, or proposing a change that touched a handful of nodes of a large kind still ran a population-wide scan — slow on big kinds (hundreds of thousands of nodes) and it shipped large duplicate result sets back to Python. + +**Goal:** scope a data-triggered uniqueness check to only the nodes the diff actually affects, so the work is proportional to the size of the change rather than the size of the kind. + +**Non-goals:** this does not change node-save-time uniqueness (`grouped_uniqueness`), does not touch other constraint validators, and does not change the uniqueness error surfaced to users. + +**Note:** uniqueness constraints do not support attribute of relationships (e.g. `"device__name"`). some parts of the code support this kind of uniqueness constraint, but not all of it and it is still blocked when creating/updating a schema. + +Part of IFC-2796 (epic IFC-2706). + +## What changed + +**Behavioral** + +- Data-triggered uniqueness validation (merge, rebase, proposed-change integrity) now validates **only the changed/affected nodes** via a batched, index-anchored targeted query instead of a full-population scan. +- A uniqueness check is scoped to exactly the nodes whose changed field participates in the constraint — changing one node's unique field no longer drags in every other changed node of the kind. +- A **newly added or broadened** constraint (schema-diff origin, with no affected-node set) still runs the full-population scan, as before. +- Cross-kind constraints that read a peer's attribute (e.g. `owner__name`) fall back to full-population validation, which supports them; the targeted query does not. **Again, this type of uniqueness constraint cannot be added to schemas currently.** +- Uniqueness **error messages are unchanged** — attribute values render identically to the full-population path regardless of their type. + +**Implementation notes** + +- A `node_uuids` carrier is threaded end to end: `None` means full scan, a list means validate exactly those nodes. It rides from the diff summary through the determiner, the constraint info, and the validator request into the checker. +- `NodeDiffFieldSummary`/`NodeDiffIndex` now keep the correlation between each changed field and the nodes that changed it (per-field UUID maps), so scoping targets a specific field's nodes rather than every changed node of the kind. +- New here (constructor-injected, wired by `build_constraint_validator_determiner`): `UniquenessConstraintScoper` decides, per kind, whether uniqueness must run and which nodes it affects (`None` → full scan); the `ConstraintValidatorDeterminer` update threads the affected set through; constraint dedup/merge helpers collapse a node's uniqueness check into its generic's where the generic already covers it. +- `UniquenessChecker` now runs the targeted query for a scoped set, and falls back to the full-population scan when a constraint reads a peer attribute. + +**What stayed the same** + +- The full-population path and the node-save-time uniqueness checker (`grouped_uniqueness`) are untouched. +- No GraphQL/API contract changes, no database schema or migration changes. + +### Suggested review order + +1. `core/diff/model/path.py`, `core/diff/query/field_summary.py`, `core/validators/node_diff_index.py` — how per-field changed-node UUIDs are captured. +2. `core/validators/uniqueness/scope.py` + `core/validators/determiner.py` — how the affected-node set (or `None`) is decided. +3. `core/validators/uniqueness/checker.py` — running the targeted query for a scoped set vs the full-population fallback. +4. `proposed_change/tasks.py`, `merge/constraints.py`, `branch/tasks.py` — the three call sites that thread the affected set through. +5. Tests. + +## How to review + +Focus on the scope decision (`UniquenessConstraintScoper._compute_scope`) and the checker's targeted-vs-full-population branch (`UniquenessChecker.check` / `_supports_targeted`) — those govern correctness. Mechanical: the `attribute_node_uuids` conversions across the test files. + +## How to test + +```bash +# unit + node-scoped component suites (need a running database) +uv run pytest backend/tests/unit/core/validators +uv run pytest backend/tests/component/core/constraint_validators +uv run pytest backend/tests/component/core/diff/repository/test_diff_repository.py::TestDiffRepositorySaveAndLoad::test_get_node_field_summaries +uv run pytest "backend/tests/component/message_bus/operations/requests/test_proposed_change.py::test_get_proposed_change_schema_integrity_constraints" + +# end-to-end schema-validator uniqueness (rebase/merge path) +uv run pytest "backend/tests/integration/schema_lifecycle/test_schema_validator_generic_uniqueness.py::TestSchemaLifecycleValidatorMain" +``` + +## Impact & rollout + +- **Backward compatibility:** no breaking changes; uniqueness error messages are identical to before. +- **Performance:** the headline win. On a large local dataset (≈500k-node kind) the full-population scan ran 8+ minutes before OOM-killing Neo4j; the targeted query validates ~9,400 changed nodes of that kind in ~12s and ~23,500 in ~29s, and all implicated kinds complete in tens of seconds. Work is now bounded by the change, not the population. +- **Config/env changes:** none. +- **Deployment notes:** safe to deploy; no coordinated release or migration required. + +## Checklist + +- [x] Tests added/updated +- [x] Changelog entry added +- [ ] External docs updated (no user-facing surface change) +- [x] Internal .md docs updated (internal knowledge and AI code tools knowledge) +- [x] I have reviewed AI generated content diff --git a/backend/infrahub/computed_attribute/tasks.py b/backend/infrahub/computed_attribute/tasks.py index 2b3cbafb0aa..8e036e99c9e 100644 --- a/backend/infrahub/computed_attribute/tasks.py +++ b/backend/infrahub/computed_attribute/tasks.py @@ -21,6 +21,7 @@ from infrahub.git.repository import get_initialized_repo from infrahub.trigger.models import TriggerSetupReport, TriggerType from infrahub.trigger.setup import setup_triggers, setup_triggers_specific +from infrahub.utilities.chunks import chunked from infrahub.workers.dependencies import get_client, get_component, get_database, get_workflow from infrahub.workflows.catalogue import ( COMPUTED_ATTRIBUTE_PROCESS_JINJA2, @@ -54,10 +55,6 @@ from infrahub.graphql.analyzer import GraphQLQueryReport -def _chunk_ids(ids: list[str], chunk_size: int) -> list[list[str]]: - return [ids[i : i + chunk_size] for i in range(0, len(ids), chunk_size)] - - async def _reconcile_python_computed_attribute_automations(db: InfrahubDatabase) -> None: """Reconcile the node-input (data-path) automations against the current schema. @@ -261,7 +258,7 @@ async def trigger_update_python_computed_attributes( return chunk_size = get_submission_chunk_size() - for chunk in _chunk_ids(object_ids, chunk_size): + for chunk in chunked(object_ids, chunk_size): await get_workflow().submit_workflow( workflow=COMPUTED_ATTRIBUTE_PROCESS_TRANSFORM, context=context, @@ -658,7 +655,7 @@ async def query_transform_targets( chunk_size = get_submission_chunk_size() for (kind, attribute_name), batch_object_ids in batches.items(): - for chunk in _chunk_ids(batch_object_ids, chunk_size): + for chunk in chunked(batch_object_ids, chunk_size): await get_workflow().submit_workflow( workflow=COMPUTED_ATTRIBUTE_PROCESS_TRANSFORM, context=context, diff --git a/backend/infrahub/core/merge/recompute_coalescing.py b/backend/infrahub/core/merge/recompute_coalescing.py index db1f7ee1037..c00bbba574e 100644 --- a/backend/infrahub/core/merge/recompute_coalescing.py +++ b/backend/infrahub/core/merge/recompute_coalescing.py @@ -9,6 +9,7 @@ from infrahub.events.limits import get_submission_chunk_size from infrahub.hfid.scoping import derive_hfid_targets from infrahub.log import get_logger +from infrahub.utilities.chunks import chunked from infrahub.workflows.catalogue import ( COMPUTED_ATTRIBUTE_PROCESS_JINJA2, DISPLAY_LABELS_PROCESS_JINJA2, @@ -173,12 +174,6 @@ class CoalescedSubmission: node_ids: tuple[str, ...] -def _chunk(ids: tuple[str, ...], size: int) -> Iterator[tuple[str, ...]]: - """Yield ``ids`` in contiguous slices of at most ``size``.""" - for start in range(0, len(ids), size): - yield ids[start : start + size] - - class CoalescedRecomputeBuilder: """Derive the deduplicated recompute for a merge or rebase change set from one schema branch.""" @@ -339,7 +334,7 @@ def plan(coalesced: CoalescedRecompute) -> list[CoalescedSubmission]: ) for target in coalesced.targets for lookup in target.reader_lookups - for chunk in _chunk(tuple(sorted(lookup.source_node_ids)), chunk_size) + for chunk in chunked(tuple(sorted(lookup.source_node_ids)), chunk_size) ] return sorted( submissions, diff --git a/backend/infrahub/core/recompute/bulk_write.py b/backend/infrahub/core/recompute/bulk_write.py index 773afd9ec49..6979d4d91df 100644 --- a/backend/infrahub/core/recompute/bulk_write.py +++ b/backend/infrahub/core/recompute/bulk_write.py @@ -10,10 +10,9 @@ from infrahub.events.constants import NodeMutationOrigin from infrahub.events.models import EventMeta from infrahub.events.node_action import NodeUpdatedEvent +from infrahub.utilities.chunks import chunked if TYPE_CHECKING: - from collections.abc import Iterator - from infrahub.core.branch import Branch from infrahub.core.node import Node from infrahub.database import InfrahubDatabase @@ -45,11 +44,6 @@ class WrittenNode: fields: tuple[str, ...] -def _chunks(items: list[str], size: int) -> Iterator[list[str]]: - for start in range(0, len(items), size): - yield items[start : start + size] - - async def _apply(node: Node, write: AttributeValueWrite) -> None: if write.field == DISPLAY_LABEL_FIELD: await node.set_display_label(value=cast("str | None", write.value)) @@ -102,7 +96,7 @@ async def write( user_id = context.account_id or SYSTEM_USER_ID written: list[WrittenNode] = [] async with self.db.start_session() as session: - for chunk in _chunks(list(writes_by_node), self.transaction_chunk_size): + for chunk in chunked(list(writes_by_node), self.transaction_chunk_size): # Load the whole node, not just the written fields: the save recomputes same-node # derived values (display label, hfid, computed siblings) that read a written value, # and it can only do that when they are loaded. diff --git a/backend/infrahub/core/validators/uniqueness/checker.py b/backend/infrahub/core/validators/uniqueness/checker.py index 26923cb7c28..120125a41aa 100644 --- a/backend/infrahub/core/validators/uniqueness/checker.py +++ b/backend/infrahub/core/validators/uniqueness/checker.py @@ -7,6 +7,7 @@ from infrahub.core.path import DataPath, GroupedDataPaths from infrahub.core.schema import AttributeSchema, MainSchemaTypes, RelationshipSchema from infrahub.core.validators.uniqueness.index import UniquenessQueryResultsIndex +from infrahub.utilities.chunks import chunked from ..enum import ConstraintIdentifier from ..interface import ConstraintCheckerInterface @@ -21,8 +22,6 @@ from .query import NodeUniqueAttributeConstraintQuery, TargetedUniquenessValidationQuery if TYPE_CHECKING: - from collections.abc import Iterator - from infrahub.core.branch import Branch from infrahub.core.query import QueryResult from infrahub.core.schema.basenode_schema import SchemaAttributePath @@ -33,11 +32,6 @@ from .query import TargetedUniquenessViolation -def _chunked(items: list[str], size: int) -> Iterator[list[str]]: - for start in range(0, len(items), size): - yield items[start : start + size] - - def get_attribute_path_from_string( path: str, schema: MainSchemaTypes ) -> tuple[AttributeSchema | RelationshipSchema, str | None]: @@ -59,6 +53,7 @@ def __init__( self, db: InfrahubDatabase, max_concurrent_execution: int = 5, + # default to 500 as we commonly do with CALL IN TRANSACTIONS OF 500 ROWS in cypher query_batch_size: int = 500, ) -> None: self.db = db @@ -135,7 +130,7 @@ async def _check_targeted( async with self.db.start_session(read_only=True) as session_db: for constraint_path in constraint_paths: constraint_elements = constraint_path.attributes_paths - for window in _chunked(node_uuids, self.query_batch_size): + for window in chunked(node_uuids, self.query_batch_size): data_paths = await self._query_group_violations( session_db=session_db, schema=schema, diff --git a/backend/infrahub/utilities/__init__.py b/backend/infrahub/utilities/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/backend/infrahub/utilities/chunks.py b/backend/infrahub/utilities/chunks.py new file mode 100644 index 00000000000..73c9ea473bf --- /dev/null +++ b/backend/infrahub/utilities/chunks.py @@ -0,0 +1,22 @@ +"""Split a sequence into contiguous fixed-size batches.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, overload + +if TYPE_CHECKING: + from collections.abc import Iterator + + +@overload +def chunked[T](items: list[T], size: int) -> Iterator[list[T]]: ... + + +@overload +def chunked[T](items: tuple[T, ...], size: int) -> Iterator[tuple[T, ...]]: ... + + +def chunked[T](items: list[T] | tuple[T, ...], size: int) -> Iterator[list[T] | tuple[T, ...]]: + """Yield ``items`` in contiguous slices of at most ``size``, preserving the input type.""" + for start in range(0, len(items), size): + yield items[start : start + size] diff --git a/backend/tests/unit/utilities/__init__.py b/backend/tests/unit/utilities/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/backend/tests/unit/computed_attribute/test_tasks.py b/backend/tests/unit/utilities/test_chunks.py similarity index 51% rename from backend/tests/unit/computed_attribute/test_tasks.py rename to backend/tests/unit/utilities/test_chunks.py index acc9d7a8b4f..6a9b6cb3f5a 100644 --- a/backend/tests/unit/computed_attribute/test_tasks.py +++ b/backend/tests/unit/utilities/test_chunks.py @@ -1,9 +1,9 @@ import pytest -from infrahub.computed_attribute.tasks import _chunk_ids +from infrahub.utilities.chunks import chunked -def test_chunk_ids_rejects_zero_chunk_size() -> None: +def test_chunked_rejects_zero_chunk_size() -> None: """A zero chunk size is invalid and must never reach the chunker.""" with pytest.raises(ValueError, match="must not be zero"): - _chunk_ids(["a", "b"], 0) + list(chunked(["a", "b"], 0)) From 38ff2021cf94ad6a14ccc5edd05122b4c835b7d3 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Thu, 23 Jul 2026 21:19:46 -0700 Subject: [PATCH 25/31] comment -> docstring --- backend/infrahub/core/validators/constraint_merge.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/backend/infrahub/core/validators/constraint_merge.py b/backend/infrahub/core/validators/constraint_merge.py index 4b1a1c00d80..4c68ac68c12 100644 --- a/backend/infrahub/core/validators/constraint_merge.py +++ b/backend/infrahub/core/validators/constraint_merge.py @@ -20,9 +20,12 @@ def __init__(self, deduplicator: UniquenessConstraintDeduplicator) -> None: self.deduplicator = deduplicator def merge(self, *constraint_lists: list[SchemaUpdateConstraintInfo]) -> list[SchemaUpdateConstraintInfo]: - # Collapse the same constraint from multiple producers onto one entry. A constraint both - # broadened by a schema change (full population) and hit by a data change (node-scoped) must - # re-check every node, so a None node_uuids always wins; two node-scoped entries union. + """Collapse the same constraint from multiple producers onto one entry. + + A constraint both broadened by a schema change (full population) and hit by a data change + (node-scoped) must re-check every node, so a ``None`` node_uuids always wins and two + node-scoped entries union. + """ merged: dict[tuple[str, str], SchemaUpdateConstraintInfo] = {} for constraint_list in constraint_lists: for constraint in constraint_list: From cb8356e00bb4810c19bd3e8ee81c7771d6331410 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Thu, 23 Jul 2026 21:49:53 -0700 Subject: [PATCH 26/31] remove accidental PR_DESCRIPTION.md --- PR_DESCRIPTION.md | 73 ----------------------------------------------- 1 file changed, 73 deletions(-) delete mode 100644 PR_DESCRIPTION.md diff --git a/PR_DESCRIPTION.md b/PR_DESCRIPTION.md deleted file mode 100644 index e53f4de3ad5..00000000000 --- a/PR_DESCRIPTION.md +++ /dev/null @@ -1,73 +0,0 @@ -# Why - -Uniqueness validation scanned the **entire population** of a kind on every data change. Merging, rebasing, or proposing a change that touched a handful of nodes of a large kind still ran a population-wide scan — slow on big kinds (hundreds of thousands of nodes) and it shipped large duplicate result sets back to Python. - -**Goal:** scope a data-triggered uniqueness check to only the nodes the diff actually affects, so the work is proportional to the size of the change rather than the size of the kind. - -**Non-goals:** this does not change node-save-time uniqueness (`grouped_uniqueness`), does not touch other constraint validators, and does not change the uniqueness error surfaced to users. - -**Note:** uniqueness constraints do not support attribute of relationships (e.g. `"device__name"`). some parts of the code support this kind of uniqueness constraint, but not all of it and it is still blocked when creating/updating a schema. - -Part of IFC-2796 (epic IFC-2706). - -## What changed - -**Behavioral** - -- Data-triggered uniqueness validation (merge, rebase, proposed-change integrity) now validates **only the changed/affected nodes** via a batched, index-anchored targeted query instead of a full-population scan. -- A uniqueness check is scoped to exactly the nodes whose changed field participates in the constraint — changing one node's unique field no longer drags in every other changed node of the kind. -- A **newly added or broadened** constraint (schema-diff origin, with no affected-node set) still runs the full-population scan, as before. -- Cross-kind constraints that read a peer's attribute (e.g. `owner__name`) fall back to full-population validation, which supports them; the targeted query does not. **Again, this type of uniqueness constraint cannot be added to schemas currently.** -- Uniqueness **error messages are unchanged** — attribute values render identically to the full-population path regardless of their type. - -**Implementation notes** - -- A `node_uuids` carrier is threaded end to end: `None` means full scan, a list means validate exactly those nodes. It rides from the diff summary through the determiner, the constraint info, and the validator request into the checker. -- `NodeDiffFieldSummary`/`NodeDiffIndex` now keep the correlation between each changed field and the nodes that changed it (per-field UUID maps), so scoping targets a specific field's nodes rather than every changed node of the kind. -- New here (constructor-injected, wired by `build_constraint_validator_determiner`): `UniquenessConstraintScoper` decides, per kind, whether uniqueness must run and which nodes it affects (`None` → full scan); the `ConstraintValidatorDeterminer` update threads the affected set through; constraint dedup/merge helpers collapse a node's uniqueness check into its generic's where the generic already covers it. -- `UniquenessChecker` now runs the targeted query for a scoped set, and falls back to the full-population scan when a constraint reads a peer attribute. - -**What stayed the same** - -- The full-population path and the node-save-time uniqueness checker (`grouped_uniqueness`) are untouched. -- No GraphQL/API contract changes, no database schema or migration changes. - -### Suggested review order - -1. `core/diff/model/path.py`, `core/diff/query/field_summary.py`, `core/validators/node_diff_index.py` — how per-field changed-node UUIDs are captured. -2. `core/validators/uniqueness/scope.py` + `core/validators/determiner.py` — how the affected-node set (or `None`) is decided. -3. `core/validators/uniqueness/checker.py` — running the targeted query for a scoped set vs the full-population fallback. -4. `proposed_change/tasks.py`, `merge/constraints.py`, `branch/tasks.py` — the three call sites that thread the affected set through. -5. Tests. - -## How to review - -Focus on the scope decision (`UniquenessConstraintScoper._compute_scope`) and the checker's targeted-vs-full-population branch (`UniquenessChecker.check` / `_supports_targeted`) — those govern correctness. Mechanical: the `attribute_node_uuids` conversions across the test files. - -## How to test - -```bash -# unit + node-scoped component suites (need a running database) -uv run pytest backend/tests/unit/core/validators -uv run pytest backend/tests/component/core/constraint_validators -uv run pytest backend/tests/component/core/diff/repository/test_diff_repository.py::TestDiffRepositorySaveAndLoad::test_get_node_field_summaries -uv run pytest "backend/tests/component/message_bus/operations/requests/test_proposed_change.py::test_get_proposed_change_schema_integrity_constraints" - -# end-to-end schema-validator uniqueness (rebase/merge path) -uv run pytest "backend/tests/integration/schema_lifecycle/test_schema_validator_generic_uniqueness.py::TestSchemaLifecycleValidatorMain" -``` - -## Impact & rollout - -- **Backward compatibility:** no breaking changes; uniqueness error messages are identical to before. -- **Performance:** the headline win. On a large local dataset (≈500k-node kind) the full-population scan ran 8+ minutes before OOM-killing Neo4j; the targeted query validates ~9,400 changed nodes of that kind in ~12s and ~23,500 in ~29s, and all implicated kinds complete in tens of seconds. Work is now bounded by the change, not the population. -- **Config/env changes:** none. -- **Deployment notes:** safe to deploy; no coordinated release or migration required. - -## Checklist - -- [x] Tests added/updated -- [x] Changelog entry added -- [ ] External docs updated (no user-facing surface change) -- [x] Internal .md docs updated (internal knowledge and AI code tools knowledge) -- [x] I have reviewed AI generated content From a0736f2788a0cc3d2001c79a7c6ec63eaa7cdfd0 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Mon, 27 Jul 2026 10:19:05 -0700 Subject: [PATCH 27/31] trim docstring --- backend/infrahub/core/validators/uniqueness/scope.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/backend/infrahub/core/validators/uniqueness/scope.py b/backend/infrahub/core/validators/uniqueness/scope.py index 171a2a66415..fc0b0b970d8 100644 --- a/backend/infrahub/core/validators/uniqueness/scope.py +++ b/backend/infrahub/core/validators/uniqueness/scope.py @@ -92,9 +92,7 @@ def _diffed_kinds_with_field(self, schema: MainSchemaTypes, field_name: str, is_ """Return the diffed kinds where `field_name` changed, on `schema` or a kind that inherits it. An inherited field keeps its name on the implementing kind, so a generic-level constraint - is implicated when an implementation's copy of the field is what changed in the diff. A - generic knows its implementing kinds through `used_by`; the diff check then keeps only the - kinds actually present in the diff. + is implicated when an implementation's copy of the field is what changed in the diff. """ kinds = {schema.kind} if isinstance(schema, GenericSchema): From b7c6994889b3327c2325f8afbfc93a1a178f9f1f Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Mon, 27 Jul 2026 11:09:18 -0700 Subject: [PATCH 28/31] refactor UNiquenessConstraintScoper for readability --- .../core/validators/uniqueness/scope.py | 173 ++++++++++++------ 1 file changed, 116 insertions(+), 57 deletions(-) diff --git a/backend/infrahub/core/validators/uniqueness/scope.py b/backend/infrahub/core/validators/uniqueness/scope.py index fc0b0b970d8..1970c72c853 100644 --- a/backend/infrahub/core/validators/uniqueness/scope.py +++ b/backend/infrahub/core/validators/uniqueness/scope.py @@ -1,6 +1,6 @@ from __future__ import annotations -from dataclasses import dataclass, field +from dataclasses import dataclass from typing import TYPE_CHECKING from infrahub.core.schema import AttributePathParsingError, GenericSchema @@ -9,14 +9,15 @@ LOG = get_logger(__name__) if TYPE_CHECKING: - from infrahub.core.schema import MainSchemaTypes + from infrahub.core.schema import AttributeSchema, MainSchemaTypes, RelationshipSchema + from infrahub.core.schema.basenode_schema import SchemaAttributePath from infrahub.core.schema.schema_branch import SchemaBranch from infrahub.core.validators.node_diff_index import NodeDiffIndex from .dependent_resolver import UniquenessDependentResolverInterface -@dataclass +@dataclass(frozen=True) class CrossKindPeerChange: """A change to a peer kind that can break the uniqueness of the kind pointing at it. @@ -27,19 +28,34 @@ class CrossKindPeerChange: """ relationship_identifier: str - changed_peer_uuids: set[str] + changed_peer_uuids: frozenset[str] -@dataclass +@dataclass(frozen=True) +class UniquenessScopeFragment: + """How a data diff implicates one element of a kind's uniqueness. + + A fragment is produced only for an element the diff actually touches, so the existence of a + fragment is what makes validation necessary — both payloads can be empty when the element + changed but the nodes behind the change are unknown. + """ + + # uuids of directly-changed nodes of the constrained kind + object_uuids: frozenset[str] = frozenset() + # peer-kind changes reached across a relationship, still to be resolved into the constrained kind + cross_kind_peer_changes: tuple[CrossKindPeerChange, ...] = () + + +@dataclass(frozen=True) class UniquenessScopeForKind: """How a data diff implicates a single kind's uniqueness.""" # whether any diffed field participates in the kind's uniqueness (if False, no need to validate) requires_validation: bool = False # uuids of directly-changed nodes of the kind whose changed field participates in its uniqueness - object_uuids: set[str] = field(default_factory=set) + object_uuids: frozenset[str] = frozenset() # peer-kind changes reached across a relationship, still to be resolved into this kind's nodes - cross_kind_peer_changes: list[CrossKindPeerChange] = field(default_factory=list) + cross_kind_peer_changes: tuple[CrossKindPeerChange, ...] = () class UniquenessConstraintScoper: @@ -126,22 +142,31 @@ def _scope(self, schema: MainSchemaTypes) -> UniquenessScopeForKind: def _compute_scope(self, schema: MainSchemaTypes) -> UniquenessScopeForKind: """Compute why and how a data change implicates `schema`'s uniqueness. - Uniqueness spans single unique attributes and multi-field constraint groups. A group - element such as "owner__name" reads an attribute of a related peer, so a data change on - the peer kind can create a violation without any change to the constrained kind itself: - that case yields a cross-kind peer change. Directly changed nodes of the constrained kind - are collected as object uuids. + Uniqueness spans single unique attributes and multi-field constraint groups, each element of + which is scoped on its own. Validation is required as soon as one element is implicated, even + when no node behind it could be identified. """ - scope = UniquenessScopeForKind() - for attribute_schema in schema.unique_attributes: - attr_kinds = self._diffed_kinds_with_field( - schema=schema, field_name=attribute_schema.name, is_relationship=False - ) - if attr_kinds: - scope.requires_validation = True - scope.object_uuids |= self._uuids_for_field( - kinds=attr_kinds, field_name=attribute_schema.name, is_relationship=False - ) + fragments = [ + *self._unique_attribute_fragments(schema=schema), + *self._uniqueness_constraint_fragments(schema=schema), + ] + return UniquenessScopeForKind( + requires_validation=bool(fragments), + object_uuids=frozenset(uuid for fragment in fragments for uuid in fragment.object_uuids), + cross_kind_peer_changes=tuple( + peer_change for fragment in fragments for peer_change in fragment.cross_kind_peer_changes + ), + ) + + def _unique_attribute_fragments(self, schema: MainSchemaTypes) -> list[UniquenessScopeFragment]: + fragments = ( + self._direct_change_fragment(schema=schema, field_name=attribute_schema.name, is_relationship=False) + for attribute_schema in schema.unique_attributes + ) + return [fragment for fragment in fragments if fragment is not None] + + def _uniqueness_constraint_fragments(self, schema: MainSchemaTypes) -> list[UniquenessScopeFragment]: + fragments: list[UniquenessScopeFragment] = [] for constraint_group in schema.uniqueness_constraints or []: for constraint_path in constraint_group: try: @@ -149,40 +174,74 @@ def _compute_scope(self, schema: MainSchemaTypes) -> UniquenessScopeForKind: except AttributePathParsingError: LOG.warning(f"Cannot parse {schema.kind}.uniqueness_constraints element '{constraint_path}'") continue - if schema_path.relationship_schema is not None: - rel_kinds = self._diffed_kinds_with_field( - schema=schema, field_name=schema_path.relationship_schema.name, is_relationship=True - ) - if rel_kinds: - scope.requires_validation = True - scope.object_uuids |= self._uuids_for_field( - kinds=rel_kinds, field_name=schema_path.relationship_schema.name, is_relationship=True - ) - if schema_path.attribute_schema is not None and schema_path.related_schema is not None: - peer_kinds = self._diffed_kinds_with_field( - schema=schema_path.related_schema, - field_name=schema_path.attribute_schema.name, - is_relationship=False, - ) - if peer_kinds: - scope.requires_validation = True - scope.cross_kind_peer_changes.append( - CrossKindPeerChange( - relationship_identifier=schema_path.relationship_schema.get_identifier(), - changed_peer_uuids=self._uuids_for_field( - kinds=peer_kinds, - field_name=schema_path.attribute_schema.name, - is_relationship=False, - ), - ) - ) - elif schema_path.attribute_schema is not None: - attr_kinds = self._diffed_kinds_with_field( + fragments.extend(self._constraint_path_fragments(schema=schema, schema_path=schema_path)) + return fragments + + def _constraint_path_fragments( + self, schema: MainSchemaTypes, schema_path: SchemaAttributePath + ) -> list[UniquenessScopeFragment]: + """Scope one element of a uniqueness constraint group. + + An element such as "owner__name" reads an attribute of a related peer, so it is implicated + both by a change to the relationship on the constrained kind and by a change to the peer's + attribute — the latter creating a violation without any change to the constrained kind. + """ + fragments: list[UniquenessScopeFragment | None] = [] + if schema_path.relationship_schema is None: + if schema_path.attribute_schema is not None: + fragments.append( + self._direct_change_fragment( schema=schema, field_name=schema_path.attribute_schema.name, is_relationship=False ) - if attr_kinds: - scope.requires_validation = True - scope.object_uuids |= self._uuids_for_field( - kinds=attr_kinds, field_name=schema_path.attribute_schema.name, is_relationship=False - ) - return scope + ) + else: + fragments.append( + self._direct_change_fragment( + schema=schema, field_name=schema_path.relationship_schema.name, is_relationship=True + ) + ) + if schema_path.attribute_schema is not None and schema_path.related_schema is not None: + fragments.append( + self._peer_change_fragment( + relationship_schema=schema_path.relationship_schema, + related_schema=schema_path.related_schema, + attribute_schema=schema_path.attribute_schema, + ) + ) + return [fragment for fragment in fragments if fragment is not None] + + def _direct_change_fragment( + self, schema: MainSchemaTypes, field_name: str, is_relationship: bool + ) -> UniquenessScopeFragment | None: + """Scope a uniqueness field carried by the constrained kind itself, None if it did not change.""" + kinds = self._diffed_kinds_with_field(schema=schema, field_name=field_name, is_relationship=is_relationship) + if not kinds: + return None + return UniquenessScopeFragment( + object_uuids=frozenset( + self._uuids_for_field(kinds=kinds, field_name=field_name, is_relationship=is_relationship) + ) + ) + + def _peer_change_fragment( + self, + relationship_schema: RelationshipSchema, + related_schema: MainSchemaTypes, + attribute_schema: AttributeSchema, + ) -> UniquenessScopeFragment | None: + """Scope the peer attribute a constraint element reads, None if the peers did not change.""" + peer_kinds = self._diffed_kinds_with_field( + schema=related_schema, field_name=attribute_schema.name, is_relationship=False + ) + if not peer_kinds: + return None + return UniquenessScopeFragment( + cross_kind_peer_changes=( + CrossKindPeerChange( + relationship_identifier=relationship_schema.get_identifier(), + changed_peer_uuids=frozenset( + self._uuids_for_field(kinds=peer_kinds, field_name=attribute_schema.name, is_relationship=False) + ), + ), + ) + ) From 671b39c60ff86df4d856156caaf4dc386e12bc91 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Mon, 27 Jul 2026 15:08:40 -0700 Subject: [PATCH 29/31] no negative in chunked() --- backend/infrahub/utilities/chunks.py | 10 ++++- backend/tests/unit/utilities/test_chunks.py | 48 +++++++++++++++++++-- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/backend/infrahub/utilities/chunks.py b/backend/infrahub/utilities/chunks.py index 73c9ea473bf..3062fb8bad6 100644 --- a/backend/infrahub/utilities/chunks.py +++ b/backend/infrahub/utilities/chunks.py @@ -17,6 +17,14 @@ def chunked[T](items: tuple[T, ...], size: int) -> Iterator[tuple[T, ...]]: ... def chunked[T](items: list[T] | tuple[T, ...], size: int) -> Iterator[list[T] | tuple[T, ...]]: - """Yield ``items`` in contiguous slices of at most ``size``, preserving the input type.""" + """Yield ``items`` in contiguous slices of at most ``size``, preserving the input type. + + Raises: + ValueError: if ``size`` is not positive. A non-positive size cannot produce slices and + would otherwise either blow up inside ``range`` or silently drop every item. + + """ + if size <= 0: + raise ValueError(f"chunk size must be greater than zero, got {size}") for start in range(0, len(items), size): yield items[start : start + size] diff --git a/backend/tests/unit/utilities/test_chunks.py b/backend/tests/unit/utilities/test_chunks.py index 6a9b6cb3f5a..1ba0c127e07 100644 --- a/backend/tests/unit/utilities/test_chunks.py +++ b/backend/tests/unit/utilities/test_chunks.py @@ -1,9 +1,49 @@ +from dataclasses import dataclass, field + import pytest from infrahub.utilities.chunks import chunked -def test_chunked_rejects_zero_chunk_size() -> None: - """A zero chunk size is invalid and must never reach the chunker.""" - with pytest.raises(ValueError, match="must not be zero"): - list(chunked(["a", "b"], 0)) +@dataclass +class InvalidSizeCase: + name: str + size: int + expected_message: str + + +@dataclass +class ChunkCase: + name: str + items: list[str] | tuple[str, ...] + size: int + expected: list[list[str] | tuple[str, ...]] = field(default_factory=list) + + +@pytest.mark.parametrize( + "test_case", + [ + InvalidSizeCase(name="zero", size=0, expected_message="chunk size must be greater than zero, got 0"), + InvalidSizeCase(name="negative", size=-3, expected_message="chunk size must be greater than zero, got -3"), + ], + ids=lambda test_case: test_case.name, +) +def test_chunked_rejects_non_positive_chunk_size(test_case: InvalidSizeCase) -> None: + """A non-positive chunk size is invalid and must never silently drop items.""" + with pytest.raises(ValueError, match=rf"^{test_case.expected_message}$"): + list(chunked(["a", "b"], test_case.size)) + + +@pytest.mark.parametrize( + "test_case", + [ + ChunkCase(name="empty_list", items=[], size=2, expected=[]), + ChunkCase(name="exact_multiple", items=["a", "b", "c", "d"], size=2, expected=[["a", "b"], ["c", "d"]]), + ChunkCase(name="trailing_partial", items=["a", "b", "c"], size=2, expected=[["a", "b"], ["c"]]), + ChunkCase(name="size_exceeds_length", items=["a", "b"], size=5, expected=[["a", "b"]]), + ChunkCase(name="tuple_input_stays_tuple", items=("a", "b", "c"), size=2, expected=[("a", "b"), ("c",)]), + ], + ids=lambda test_case: test_case.name, +) +def test_chunked_splits_into_contiguous_slices(test_case: ChunkCase) -> None: + assert list(chunked(test_case.items, test_case.size)) == test_case.expected From dbbcad42c1f64bfa8006cf1147668c67a851aa3c Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Mon, 27 Jul 2026 15:35:05 -0700 Subject: [PATCH 30/31] move get_query_arrows() from RelationshipSchema to Query --- backend/infrahub/core/query/__init__.py | 46 ++++++++++++++++++- backend/infrahub/core/query/relationship.py | 6 +-- .../core/schema/relationship_schema.py | 36 --------------- .../core/validators/relationship/peer.py | 6 +-- .../uniqueness/query/targeted_validation.py | 4 +- .../validators/uniqueness/query/validation.py | 2 +- 6 files changed, 54 insertions(+), 46 deletions(-) diff --git a/backend/infrahub/core/query/__init__.py b/backend/infrahub/core/query/__init__.py index 6487df4e1e9..9152b5dbe28 100644 --- a/backend/infrahub/core/query/__init__.py +++ b/backend/infrahub/core/query/__init__.py @@ -15,7 +15,7 @@ from opentelemetry import trace from infrahub import config -from infrahub.core.constants import SYSTEM_USER_ID, PermissionLevel +from infrahub.core.constants import SYSTEM_USER_ID, PermissionLevel, RelationshipDirection from infrahub.core.timestamp import Timestamp from infrahub.exceptions import QueryError @@ -138,6 +138,36 @@ def __str__(self) -> str: return "-%s-" % main_str +@dataclass +class QueryArrow: + start: str + end: str + + +@dataclass +class QueryArrowInbound(QueryArrow): + start: str = "<-" + end: str = "-" + + +@dataclass +class QueryArrowOutbound(QueryArrow): + start: str = "-" + end: str = "->" + + +@dataclass +class QueryArrowBidir(QueryArrow): + start: str = "-" + end: str = "-" + + +@dataclass +class QueryArrows: + left: QueryArrow + right: QueryArrow + + class QueryType(Enum): READ = "read" WRITE = "write" @@ -443,6 +473,20 @@ def get_context(self) -> dict[str, str]: """ return {} + @staticmethod + def get_query_arrows(direction: RelationshipDirection) -> QueryArrows: + """Return the 2 arrows of the node→Relationship→peer path for a relationship direction. + + The edges around a Relationship vertex are created in the direction of the relationship, so + a query must traverse them the same way to tell the two ends of the path apart. + """ + if direction == RelationshipDirection.OUTBOUND: + return QueryArrows(left=QueryArrowOutbound(), right=QueryArrowOutbound()) + if direction == RelationshipDirection.INBOUND: + return QueryArrows(left=QueryArrowInbound(), right=QueryArrowInbound()) + + return QueryArrows(left=QueryArrowOutbound(), right=QueryArrowInbound()) + @staticmethod @lru_cache(maxsize=1024) def _split_query_lines(query: str) -> list[str]: diff --git a/backend/infrahub/core/query/relationship.py b/backend/infrahub/core/query/relationship.py index 65b479da97f..a095ffef75b 100644 --- a/backend/infrahub/core/query/relationship.py +++ b/backend/infrahub/core/query/relationship.py @@ -314,7 +314,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None: # noqa: ARG self.params["rel_prop"] = self.get_relationship_properties_dict( status=RelationshipStatus.ACTIVE, user_id=self.user_id ) - arrows = self.schema.get_query_arrows() + arrows = self.get_query_arrows(direction=self.schema.direction) r1 = f"{arrows.left.start}[r1:IS_RELATED $rel_prop ]{arrows.left.end}" r2 = f"{arrows.right.start}[r2:IS_RELATED $rel_prop ]{arrows.right.end}" @@ -585,7 +585,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None: # noqa: ARG self.params["at"] = self.at.to_string() self.params.update(rel_params) - arrows = self.schema.get_query_arrows() + arrows = self.get_query_arrows(direction=self.schema.direction) r1 = f"{arrows.left.start}[r1:IS_RELATED $rel_prop ]{arrows.left.end}" r2 = f"{arrows.right.start}[r2:IS_RELATED $rel_prop ]{arrows.right.end}" @@ -966,7 +966,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None: # noqa: ARG self.params["peer_kind"] = self.schema.peer self.params["source_kind"] = self.source_kind - arrows = self.schema.get_query_arrows() + arrows = self.get_query_arrows(direction=self.schema.direction) path_str = f"{arrows.left.start}[r1:IS_RELATED]{arrows.left.end}(rl){arrows.right.start}[r2:IS_RELATED]{arrows.right.end}" diff --git a/backend/infrahub/core/schema/relationship_schema.py b/backend/infrahub/core/schema/relationship_schema.py index 30031946f18..586460c5c35 100644 --- a/backend/infrahub/core/schema/relationship_schema.py +++ b/backend/infrahub/core/schema/relationship_schema.py @@ -3,8 +3,6 @@ from enum import Enum from typing import TYPE_CHECKING, Any -from pydantic import BaseModel - from infrahub import config from infrahub.core.constants import RelationshipDirection, RelationshipKind from infrahub.core.query import QueryNode, QueryRel, QueryRelDirection @@ -67,15 +65,6 @@ def get_id(self) -> str: raise InitializationError("The relationship schema has not been saved yet and doesn't have an id") return self.id - def get_query_arrows(self) -> QueryArrows: - """Return (in 4 parts) the 2 arrows for the relationship R1 and R2 based on the direction of the relationship.""" - if self.direction == RelationshipDirection.OUTBOUND: - return QueryArrows(left=QueryArrowOutband(), right=QueryArrowOutband()) - if self.direction == RelationshipDirection.INBOUND: - return QueryArrows(left=QueryArrowInband(), right=QueryArrowInband()) - - return QueryArrows(left=QueryArrowOutband(), right=QueryArrowInband()) - def update_from_generic(self, other: RelationshipSchema) -> None: fields_to_exclude = ("id", "order_weight", "branch", "inherited", "filters") for name in self.__class__.model_fields: @@ -210,28 +199,3 @@ async def get_query_filter( query_params.update(field_params) return query_filter, query_params, query_where - - -class QueryArrow(BaseModel): - start: str - end: str - - -class QueryArrowInband(QueryArrow): - start: str = "<-" - end: str = "-" - - -class QueryArrowOutband(QueryArrow): - start: str = "-" - end: str = "->" - - -class QueryArrowBidir(QueryArrow): - start: str = "-" - end: str = "-" - - -class QueryArrows(BaseModel): - left: QueryArrow - right: QueryArrow diff --git a/backend/infrahub/core/validators/relationship/peer.py b/backend/infrahub/core/validators/relationship/peer.py index 7f23d8ad034..553f3758c51 100644 --- a/backend/infrahub/core/validators/relationship/peer.py +++ b/backend/infrahub/core/validators/relationship/peer.py @@ -150,7 +150,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: dict[str, Any]) -> No self.params["parent_relationship_id"] = self.parent_relationship.identifier self.params["peer_parent_relationship_id"] = self.peer_parent_relationship.identifier - parent_arrows = self.parent_relationship.get_query_arrows() + parent_arrows = self.get_query_arrows(direction=self.parent_relationship.direction) parent_match = ( "MATCH (active_node)%(lstart)s[r1:IS_RELATED]%(lend)s" "(rel:Relationship { name: $parent_relationship_id })%(rstart)s[r2:IS_RELATED]%(rend)s(parent:Node)" @@ -161,7 +161,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: dict[str, Any]) -> No "rend": parent_arrows.right.end, } - peer_parent_arrows = self.relationship.get_query_arrows() + peer_parent_arrows = self.get_query_arrows(direction=self.relationship.direction) peer_match = ( "MATCH (active_node)%(lstart)s[r1:IS_RELATED]%(lend)s" "(r:Relationship {name: $peer_relationship_id })%(rstart)s[r2:IS_RELATED]%(rend)s(peer:Node)" @@ -172,7 +172,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: dict[str, Any]) -> No "rend": peer_parent_arrows.right.end, } - peer_parent_arrows = self.peer_parent_relationship.get_query_arrows() + peer_parent_arrows = self.get_query_arrows(direction=self.peer_parent_relationship.direction) peer_parent_match = ( "MATCH (peer:Node)%(lstart)s[r1:IS_RELATED]%(lend)s" "(r:Relationship {name: $peer_parent_relationship_id})%(rstart)s[r2:IS_RELATED]%(rend)s(peer_parent:Node)" diff --git a/backend/infrahub/core/validators/uniqueness/query/targeted_validation.py b/backend/infrahub/core/validators/uniqueness/query/targeted_validation.py index c85b5a10eab..a9eacbd1c4d 100644 --- a/backend/infrahub/core/validators/uniqueness/query/targeted_validation.py +++ b/backend/infrahub/core/validators/uniqueness/query/targeted_validation.py @@ -158,7 +158,7 @@ def _render_value_resolution( relationship_schema = element.active_relationship_schema rel_identifier_var = f"rel_identifier_{index}" - query_arrows = relationship_schema.get_query_arrows() + query_arrows = self.get_query_arrows(direction=relationship_schema.direction) query = """ CALL (%(source_var)s) { MATCH (%(source_var)s)%(lstart)s[r:IS_RELATED]%(lend)s(rel:Relationship {name: $%(rel_identifier_var)s}) @@ -216,7 +216,7 @@ def _render_anchor_probe( "value_var": value_var, } else: - query_arrows = element.active_relationship_schema.get_query_arrows() + query_arrows = self.get_query_arrows(direction=element.active_relationship_schema.direction) anchor_match = ( "MATCH (candidate:%(kind)s)%(lstart)s[:IS_RELATED]%(lend)s" "(:Relationship {name: $rel_identifier_%(index)s})%(rstart)s[:IS_RELATED]%(rend)s(anchor_peer:Node)\n" diff --git a/backend/infrahub/core/validators/uniqueness/query/validation.py b/backend/infrahub/core/validators/uniqueness/query/validation.py index 50df006c8c8..c382ddfaeb4 100644 --- a/backend/infrahub/core/validators/uniqueness/query/validation.py +++ b/backend/infrahub/core/validators/uniqueness/query/validation.py @@ -393,7 +393,7 @@ def _build_rel_subquery( ) params[attr_name_var] = rel_path.attribute_name params[attr_value_var] = rel_path.attribute_value - query_arrows = rel_path.relationship_schema.get_query_arrows() + query_arrows = self.get_query_arrows(direction=rel_path.relationship_schema.direction) rel_name_var = f"rel_name_{index}" # long path MATCH is required to hit an index on the peer or AttributeValue of the peer first_match = ( From abcd7e724624d606b23cafbb531969b26d2efc07 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Mon, 27 Jul 2026 16:00:51 -0700 Subject: [PATCH 31/31] support relationship direction in UniquenessDependentResolver --- .../uniqueness/dependent_resolver.py | 20 +++- .../uniqueness/query/affected_dependents.py | 28 +++-- .../core/validators/uniqueness/scope.py | 4 + .../constraint_validators/test_determiner.py | 10 +- .../test_uniqueness_dependent_resolver.py | 104 ++++++++++++++++-- .../test_uniqueness_scope.py | 21 +++- 6 files changed, 161 insertions(+), 26 deletions(-) diff --git a/backend/infrahub/core/validators/uniqueness/dependent_resolver.py b/backend/infrahub/core/validators/uniqueness/dependent_resolver.py index fe84de6ded2..1fa25438b06 100644 --- a/backend/infrahub/core/validators/uniqueness/dependent_resolver.py +++ b/backend/infrahub/core/validators/uniqueness/dependent_resolver.py @@ -8,6 +8,7 @@ if TYPE_CHECKING: from infrahub.core.branch import Branch + from infrahub.core.constants import RelationshipDirection from infrahub.core.timestamp import Timestamp from infrahub.database import InfrahubDatabase @@ -20,7 +21,13 @@ class UniquenessDependentResolverInterface(Protocol): must be resolved by traversal before they can be node-scoped. """ - async def resolve(self, node_kind: str, relationship_identifier: str, peer_uuids: list[str]) -> set[str]: ... + async def resolve( + self, + node_kind: str, + relationship_identifier: str, + relationship_direction: RelationshipDirection, + peer_uuids: list[str], + ) -> set[str]: ... class UniquenessDependentResolver: @@ -31,9 +38,17 @@ def __init__(self, db: InfrahubDatabase, branch: Branch, at: Timestamp | str | N self.branch = branch self.at = at - async def resolve(self, node_kind: str, relationship_identifier: str, peer_uuids: list[str]) -> set[str]: + async def resolve( + self, + node_kind: str, + relationship_identifier: str, + relationship_direction: RelationshipDirection, + peer_uuids: list[str], + ) -> set[str]: """Return the uuids of `node_kind` nodes related via `relationship_identifier` to any peer in `peer_uuids`. + The relationship is traversed in `relationship_direction`, so only the nodes on the + constrained side of the peers are returned even when the peer kind is the node kind itself. Only relationships visible from this branch (its own, its base, and the global branch) are considered. The result is a superset of the truly-related nodes and is empty when no peer uuids are given or none are referenced. @@ -46,6 +61,7 @@ async def resolve(self, node_kind: str, relationship_identifier: str, peer_uuids at=self.at, node_kind=node_kind, relationship_identifier=relationship_identifier, + relationship_direction=relationship_direction, peer_uuids=peer_uuids, default_branch_name=registry.default_branch, ) diff --git a/backend/infrahub/core/validators/uniqueness/query/affected_dependents.py b/backend/infrahub/core/validators/uniqueness/query/affected_dependents.py index 54ce40f9ce7..f5402bae7ee 100644 --- a/backend/infrahub/core/validators/uniqueness/query/affected_dependents.py +++ b/backend/infrahub/core/validators/uniqueness/query/affected_dependents.py @@ -6,17 +6,22 @@ from infrahub.core.query import Query, QueryType if TYPE_CHECKING: + from infrahub.core.constants import RelationshipDirection from infrahub.database import InfrahubDatabase class AffectedUniquenessDependentsQuery(Query): """Return the nodes of a kind related, through a named relationship, to any of a set of peer nodes. + The path is traversed in the relationship's own direction, so a kind whose relationship points at + its own kind resolves only the nodes on the constrained side of the peers, not those on the + opposite side. + Considers edges at the timestamp on the input branch, default branch, and global branch, regardless of when the input branch forked from the default branch. That is, changes made on the default branch after the input branch was created WILL be included in the results. - Each hop of the path (peer→relationship and relationship→node) is resolved to a single winner + Each hop of the path (node→relationship and relationship→peer) is resolved to a single winner across the visible branches: the latest edge on the deepest branch decides, so a change on the input branch overrides the default branch. The user branch will always override the default branch if changes conflict. @@ -29,12 +34,14 @@ def __init__( self, node_kind: str, relationship_identifier: str, + relationship_direction: RelationshipDirection, peer_uuids: list[str], default_branch_name: str, **kwargs: Any, ) -> None: self.node_kind = node_kind self.relationship_identifier = relationship_identifier + self.relationship_direction = relationship_direction self.peer_uuids = peer_uuids self.default_branch_name = default_branch_name super().__init__(**kwargs) @@ -48,11 +55,12 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # noqa "default_branch": self.default_branch_name, "global_branch": GLOBAL_BRANCH_NAME, } + query_arrows = self.get_query_arrows(direction=self.relationship_direction) query = """ // -------------------- // start with all possible active Relationship paths on a branch we care about // -------------------- -MATCH (peer)-[r1:IS_RELATED]-(rel:Relationship {name: $rel_identifier})-[r2:IS_RELATED]-(node:%(node_kind)s) +MATCH (node:%(node_kind)s)%(lstart)s[r1:IS_RELATED]%(lend)s(rel:Relationship {name: $rel_identifier})%(rstart)s[r2:IS_RELATED]%(rend)s(peer) WHERE peer.uuid IN $peer_uuids AND peer <> node AND r1.branch IN [$branch, $default_branch, $global_branch] @@ -67,8 +75,8 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # noqa // -------------------- // keep only active edges. the latest edge on the deepest branch wins. // -------------------- -CALL (peer, rel) { - MATCH (peer)-[r:IS_RELATED]-(rel) +CALL (node, rel) { + MATCH (node)%(lstart)s[r:IS_RELATED]%(lend)s(rel) WHERE r.branch IN [$branch, $default_branch, $global_branch] AND r.from <= $at AND (r.to IS NULL OR r.to >= $at) @@ -77,10 +85,10 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # noqa LIMIT 1 WITH is_active WHERE is_active = TRUE - RETURN TRUE AS peer_rel_is_live + RETURN TRUE AS node_rel_is_live } -CALL (rel, node) { - MATCH (rel)-[r:IS_RELATED]-(node) +CALL (rel, peer) { + MATCH (rel)%(rstart)s[r:IS_RELATED]%(rend)s(peer) WHERE r.branch IN [$branch, $default_branch, $global_branch] AND r.from <= $at AND (r.to IS NULL OR r.to >= $at) @@ -89,10 +97,14 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # noqa LIMIT 1 WITH is_active WHERE is_active = TRUE - RETURN TRUE AS rel_node_is_live + RETURN TRUE AS rel_peer_is_live } """ % { "node_kind": self.node_kind, + "lstart": query_arrows.left.start, + "lend": query_arrows.left.end, + "rstart": query_arrows.right.start, + "rend": query_arrows.right.end, } self.add_to_query(query=query) self.return_labels = ["DISTINCT node.uuid AS node_uuid"] diff --git a/backend/infrahub/core/validators/uniqueness/scope.py b/backend/infrahub/core/validators/uniqueness/scope.py index 1970c72c853..85e0733f031 100644 --- a/backend/infrahub/core/validators/uniqueness/scope.py +++ b/backend/infrahub/core/validators/uniqueness/scope.py @@ -9,6 +9,7 @@ LOG = get_logger(__name__) if TYPE_CHECKING: + from infrahub.core.constants import RelationshipDirection from infrahub.core.schema import AttributeSchema, MainSchemaTypes, RelationshipSchema from infrahub.core.schema.basenode_schema import SchemaAttributePath from infrahub.core.schema.schema_branch import SchemaBranch @@ -28,6 +29,7 @@ class CrossKindPeerChange: """ relationship_identifier: str + relationship_direction: RelationshipDirection changed_peer_uuids: frozenset[str] @@ -98,6 +100,7 @@ async def affected_node_uuids(self, schema: MainSchemaTypes) -> list[str] | None node_uuids |= await self.dependent_resolver.resolve( node_kind=schema.kind, relationship_identifier=peer_change.relationship_identifier, + relationship_direction=peer_change.relationship_direction, peer_uuids=sorted(peer_change.changed_peer_uuids), ) if not node_uuids: @@ -239,6 +242,7 @@ def _peer_change_fragment( cross_kind_peer_changes=( CrossKindPeerChange( relationship_identifier=relationship_schema.get_identifier(), + relationship_direction=relationship_schema.direction, changed_peer_uuids=frozenset( self._uuids_for_field(kinds=peer_kinds, field_name=attribute_schema.name, is_relationship=False) ), diff --git a/backend/tests/component/core/constraint_validators/test_determiner.py b/backend/tests/component/core/constraint_validators/test_determiner.py index 690ab03c9b5..cfe2cb6bda8 100644 --- a/backend/tests/component/core/constraint_validators/test_determiner.py +++ b/backend/tests/component/core/constraint_validators/test_determiner.py @@ -4,7 +4,7 @@ from infrahub.core import registry from infrahub.core.branch import Branch -from infrahub.core.constants import SchemaPathType +from infrahub.core.constants import RelationshipDirection, SchemaPathType from infrahub.core.diff.model.path import NodeDiffFieldSummary from infrahub.core.models import SchemaUpdateConstraintInfo from infrahub.core.node import Node @@ -25,7 +25,13 @@ class _NoDependentsResolver: - async def resolve(self, node_kind: str, relationship_identifier: str, peer_uuids: list[str]) -> set[str]: + async def resolve( + self, + node_kind: str, + relationship_identifier: str, + relationship_direction: RelationshipDirection, + peer_uuids: list[str], + ) -> set[str]: return set() diff --git a/backend/tests/component/core/constraint_validators/test_uniqueness_dependent_resolver.py b/backend/tests/component/core/constraint_validators/test_uniqueness_dependent_resolver.py index 88559c3dc75..37e16d4a6fb 100644 --- a/backend/tests/component/core/constraint_validators/test_uniqueness_dependent_resolver.py +++ b/backend/tests/component/core/constraint_validators/test_uniqueness_dependent_resolver.py @@ -1,15 +1,50 @@ +import pytest + from infrahub.core import registry from infrahub.core.branch import Branch +from infrahub.core.constants import RelationshipCardinality, RelationshipDirection from infrahub.core.initialization import create_branch from infrahub.core.manager import NodeManager from infrahub.core.node import Node +from infrahub.core.schema import AttributeSchema, NodeSchema, SchemaRoot +from infrahub.core.schema.relationship_schema import RelationshipSchema from infrahub.core.validators.uniqueness.dependent_resolver import UniquenessDependentResolver from infrahub.database import InfrahubDatabase - -async def _owner_identifier(default_branch: Branch) -> str: +TREE_NODE_KIND = "TestingTreeNode" +TREE_RELATIONSHIP_IDENTIFIER = "testingtreenode__testingtreenode" + +# a kind whose relationship points at its own kind: `parent` and `children` share one identifier and +# are told apart only by the direction of the edges around the Relationship vertex +TREE_NODE = NodeSchema( + name="TreeNode", + namespace="Testing", + generate_profile=False, + attributes=[AttributeSchema(name="name", kind="Text", unique=True)], + relationships=[ + RelationshipSchema( + name="parent", + peer=TREE_NODE_KIND, + identifier=TREE_RELATIONSHIP_IDENTIFIER, + cardinality=RelationshipCardinality.ONE, + direction=RelationshipDirection.OUTBOUND, + optional=True, + ), + RelationshipSchema( + name="children", + peer=TREE_NODE_KIND, + identifier=TREE_RELATIONSHIP_IDENTIFIER, + cardinality=RelationshipCardinality.MANY, + direction=RelationshipDirection.INBOUND, + optional=True, + ), + ], +) + + +def _owner_relationship(default_branch: Branch) -> RelationshipSchema: car_schema = registry.schema.get("TestCar", branch=default_branch) - return car_schema.get_relationship("owner").get_identifier() + return car_schema.get_relationship("owner") class TestUniquenessDependentResolver: @@ -27,7 +62,8 @@ async def test_resolves_nodes_referencing_changed_peers( dependents = await resolver.resolve( node_kind="TestCar", - relationship_identifier=await _owner_identifier(default_branch), + relationship_identifier=_owner_relationship(default_branch).get_identifier(), + relationship_direction=_owner_relationship(default_branch).direction, peer_uuids=[person_john_main.id], ) @@ -42,7 +78,8 @@ async def test_empty_peer_uuids_returns_empty( dependents = await resolver.resolve( node_kind="TestCar", - relationship_identifier=await _owner_identifier(default_branch), + relationship_identifier=_owner_relationship(default_branch).get_identifier(), + relationship_direction=_owner_relationship(default_branch).direction, peer_uuids=[], ) @@ -66,7 +103,8 @@ async def test_covers_default_branch_changes_made_after_the_branch_forked( dependents = await resolver.resolve( node_kind="TestCar", - relationship_identifier=await _owner_identifier(default_branch), + relationship_identifier=_owner_relationship(default_branch).get_identifier(), + relationship_direction=_owner_relationship(default_branch).direction, peer_uuids=[person_john_main.id], ) @@ -93,7 +131,8 @@ async def test_post_fork_default_branch_deletion_excludes_when_branch_is_silent( dependents = await resolver.resolve( node_kind="TestCar", - relationship_identifier=await _owner_identifier(default_branch), + relationship_identifier=_owner_relationship(default_branch).get_identifier(), + relationship_direction=_owner_relationship(default_branch).direction, peer_uuids=[person_john_main.id], ) @@ -117,7 +156,8 @@ async def test_excludes_node_whose_relationship_is_deleted_on_input_branch( dependents = await resolver.resolve( node_kind="TestCar", - relationship_identifier=await _owner_identifier(default_branch), + relationship_identifier=_owner_relationship(default_branch).get_identifier(), + relationship_direction=_owner_relationship(default_branch).direction, peer_uuids=[person_john_main.id], ) @@ -125,3 +165,51 @@ async def test_excludes_node_whose_relationship_is_deleted_on_input_branch( # produce), so accord is excluded; prius keeps its untouched relationship and stays assert car_accord_main.id not in dependents assert car_prius_main.id in dependents + + +class TestUniquenessDependentResolverSelfReferential: + @pytest.fixture + async def tree(self, db: InfrahubDatabase, default_branch: Branch) -> dict[str, Node]: + registry.schema.register_schema(schema=SchemaRoot(nodes=[TREE_NODE]), branch=default_branch.name) + + root = await Node.init(db=db, schema=TREE_NODE_KIND, branch=default_branch) + await root.new(db=db, name="root") + await root.save(db=db) + middle = await Node.init(db=db, schema=TREE_NODE_KIND, branch=default_branch) + await middle.new(db=db, name="middle", parent=root.id) + await middle.save(db=db) + leaf = await Node.init(db=db, schema=TREE_NODE_KIND, branch=default_branch) + await leaf.new(db=db, name="leaf", parent=middle.id) + await leaf.save(db=db) + + return {"root": root, "middle": middle, "leaf": leaf} + + async def test_outbound_relationship_resolves_only_the_child_side( + self, db: InfrahubDatabase, default_branch: Branch, tree: dict[str, Node] + ) -> None: + """A change to `middle` implicates the node whose `parent` is `middle`, not `middle`'s parent.""" + resolver = UniquenessDependentResolver(db=db, branch=default_branch) + + dependents = await resolver.resolve( + node_kind=TREE_NODE_KIND, + relationship_identifier=TREE_RELATIONSHIP_IDENTIFIER, + relationship_direction=RelationshipDirection.OUTBOUND, + peer_uuids=[tree["middle"].id], + ) + + assert dependents == {tree["leaf"].id} + + async def test_inbound_relationship_resolves_only_the_parent_side( + self, db: InfrahubDatabase, default_branch: Branch, tree: dict[str, Node] + ) -> None: + """The same edges seen from `children` implicate the node holding `middle` as a child.""" + resolver = UniquenessDependentResolver(db=db, branch=default_branch) + + dependents = await resolver.resolve( + node_kind=TREE_NODE_KIND, + relationship_identifier=TREE_RELATIONSHIP_IDENTIFIER, + relationship_direction=RelationshipDirection.INBOUND, + peer_uuids=[tree["middle"].id], + ) + + assert dependents == {tree["root"].id} diff --git a/backend/tests/component/core/constraint_validators/test_uniqueness_scope.py b/backend/tests/component/core/constraint_validators/test_uniqueness_scope.py index 5ecd2c6d833..f575554d0bb 100644 --- a/backend/tests/component/core/constraint_validators/test_uniqueness_scope.py +++ b/backend/tests/component/core/constraint_validators/test_uniqueness_scope.py @@ -1,5 +1,6 @@ from infrahub.core import registry from infrahub.core.branch import Branch +from infrahub.core.constants import RelationshipDirection from infrahub.core.diff.model.path import NodeDiffFieldSummary from infrahub.core.schema import SchemaRoot from infrahub.core.schema.schema_branch import SchemaBranch @@ -12,10 +13,16 @@ class _RecordingResolver: def __init__(self, dependents: set[str]) -> None: self.dependents = dependents - self.calls: list[tuple[str, str, list[str]]] = [] - - async def resolve(self, node_kind: str, relationship_identifier: str, peer_uuids: list[str]) -> set[str]: - self.calls.append((node_kind, relationship_identifier, peer_uuids)) + self.calls: list[tuple[str, str, RelationshipDirection, list[str]]] = [] + + async def resolve( + self, + node_kind: str, + relationship_identifier: str, + relationship_direction: RelationshipDirection, + peer_uuids: list[str], + ) -> set[str]: + self.calls.append((node_kind, relationship_identifier, relationship_direction, peer_uuids)) return set(self.dependents) @@ -113,8 +120,10 @@ async def test_cross_kind_peer_change_resolves_dependents( assert scoper.requires_validation(schema=car_schema) is True assert await scoper.affected_node_uuids(schema=car_schema) == ["car-1", "car-2"] # the peer change is routed to the resolver as a single call carrying the changed peer uuids - owner_identifier = car_schema.get_relationship(name="owner").get_identifier() - assert resolver.calls == [("TestCar", owner_identifier, ["person-1"])] + owner_relationship = car_schema.get_relationship(name="owner") + assert resolver.calls == [ + ("TestCar", owner_relationship.get_identifier(), owner_relationship.direction, ["person-1"]) + ] async def test_cross_kind_without_known_peer_uuids_falls_back_to_full_scan( self, car_person_schema: SchemaBranch, default_branch: Branch