skip running node-level uniqueness constraints when possible (#5954)

* skip running node-level uniqueness constraints when possible

* fix unit test for new order of constraint execution

* add changelog

* add schema's to vale

Author: ajtmccarty

.vale/styles/spelling-exceptions.txt

@@ -109,6 +109,7 @@ regenerate_host_artifact
 repo
 REST
 resources
+schema's
 schema_mapping
 sdk
 subcommand

backend/infrahub/core/node/constraints/grouped_uniqueness.py

@@ -177,12 +177,25 @@ async def _check_one_schema(
         await self._check_results(updated_node=node, path_groups=path_groups, query_results=query.get_results())
 
     async def check(self, node: Node, at: Optional[Timestamp] = None, filters: Optional[list[str]] = None) -> None:
+        def _frozen_constraints(schema: MainSchemaTypes) -> frozenset[frozenset[str]]:
+            if not schema.uniqueness_constraints:
+                return frozenset()
+            return frozenset(frozenset(uc) for uc in schema.uniqueness_constraints)
+
         node_schema = node.get_schema()
-        schemas_to_check: list[MainSchemaTypes] = [node_schema]
+        include_node_schema = True
+        frozen_node_constraints = _frozen_constraints(node_schema)
+        schemas_to_check: list[MainSchemaTypes] = []
         if node_schema.inherit_from:
             for parent_schema_name in node_schema.inherit_from:
                 parent_schema = self.schema_branch.get(name=parent_schema_name, duplicate=False)
-                if parent_schema.uniqueness_constraints:
-                    schemas_to_check.append(parent_schema)
+                if not parent_schema.uniqueness_constraints:
+                    continue
+                schemas_to_check.append(parent_schema)
+                frozen_parent_constraints = _frozen_constraints(parent_schema)
+                if frozen_node_constraints <= frozen_parent_constraints:
+                    include_node_schema = False
+        if include_node_schema:
+            schemas_to_check.append(node_schema)
         for schema in schemas_to_check:
             await self._check_one_schema(node=node, node_schema=schema, at=at, filters=filters)

backend/tests/unit/core/constraint_validators/test_node_grouped_uniqueness.py

@@ -1,9 +1,12 @@
+from unittest.mock import patch
+
 import pytest
 
 from infrahub.core import registry
 from infrahub.core.branch import Branch
 from infrahub.core.node import Node
 from infrahub.core.node.constraints.grouped_uniqueness import NodeGroupedUniquenessConstraint
+from infrahub.core.validators.uniqueness.query import NodeUniqueAttributeConstraintQuery
 from infrahub.database import InfrahubDatabase
 from infrahub.exceptions import ValidationError
 
@@ -197,6 +200,86 @@ async def test_uniqueness_constraint_no_conflict_relationship_and_attribute(
 
         await self.__call_system_under_test(db=db, branch=default_branch, node=car_node)
 
+    @pytest.mark.parametrize(
+        ["node_constraints", "parent_constraints", "node_query_should_run"],
+        [
+            (
+                [
+                    ["nbr_seats__value", "name__value"],
+                    ["previous_owner", "nbr_seats__value"],
+                ],
+                [
+                    ["nbr_seats__value", "name__value"],
+                    ["previous_owner", "nbr_seats__value"],
+                ],
+                False,
+            ),
+            (
+                [
+                    ["nbr_seats__value", "name__value"],
+                ],
+                [
+                    ["nbr_seats__value", "name__value"],
+                    ["previous_owner", "nbr_seats__value"],
+                ],
+                False,
+            ),
+            (
+                [
+                    ["previous_owner", "name__value"],
+                ],
+                [
+                    ["nbr_seats__value", "name__value"],
+                    ["previous_owner", "nbr_seats__value"],
+                ],
+                True,
+            ),
+            (
+                [
+                    ["nbr_seats__value", "name__value"],
+                    ["previous_owner", "nbr_seats__value", "color__value"],
+                ],
+                [
+                    ["nbr_seats__value", "name__value"],
+                    ["previous_owner", "nbr_seats__value"],
+                ],
+                True,
+            ),
+        ],
+    )
+    async def test_uniqueness_constraint_skips_overlapping_constraints(
+        self,
+        db: InfrahubDatabase,
+        default_branch: Branch,
+        car_person_generics_data_simple,
+        node_constraints: list[list[str]],
+        parent_constraints: list[list[str]],
+        node_query_should_run: bool,
+    ):
+        car_node: Node = car_person_generics_data_simple["c1"]
+        car_schema = car_node.get_schema()
+        schema_branch = registry.schema.get_schema_branch(name=default_branch.name)
+        parent_kind = car_schema.inherit_from[0]
+        parent_schema = schema_branch.get(name=parent_kind, duplicate=False)
+        car_schema.uniqueness_constraints = node_constraints
+        parent_schema.uniqueness_constraints = parent_constraints
+
+        # make sure we only use this query once if the constraints overlap
+        with patch(
+            "infrahub.core.node.constraints.grouped_uniqueness.NodeUniqueAttributeConstraintQuery",
+            wraps=NodeUniqueAttributeConstraintQuery,
+        ) as wrapped_query:
+            await self.__call_system_under_test(db=db, branch=default_branch, node=car_node)
+            if node_query_should_run:
+                assert len(wrapped_query.init.call_args_list) == 2
+            else:
+                assert len(wrapped_query.init.call_args_list) == 1
+            query_kinds = {init_call[1]["query_request"].kind for init_call in wrapped_query.init.call_args_list}
+            if node_query_should_run:
+                assert query_kinds == {car_schema.kind, parent_kind}
+            else:
+                assert query_kinds == {parent_kind}
+
     async def test_uniqueness_constraint_conflict_relationship_and_attribute(
         self, db: InfrahubDatabase, default_branch: Branch, car_person_generics_data_simple
     ):

backend/tests/unit/core/constraint_validators/test_node_uniqueness.py

@@ -42,8 +42,8 @@ async def test_hierarchical_uniqueness_constraint(
     site_schema.uniqueness_constraints = [["parent", "name__value"]]
 
     rack_schema = hierarchical_location_schema_simple_unregistered.get(name="LocationRack")
-    rack_schema.human_friendly_id = ["parent__name__value", "name__value"]
-    rack_schema.uniqueness_constraints = [["parent", "name__value"]]
+    rack_schema.human_friendly_id = ["parent__name__value", "status__value"]
+    rack_schema.uniqueness_constraints = [["parent", "status__value"]]
 
     registry.schema.register_schema(schema=hierarchical_location_schema_simple_unregistered, branch=default_branch.name)
     constraint = NodeGroupedUniquenessConstraint(db=db, branch=default_branch)
@@ -67,7 +67,7 @@ async def test_hierarchical_uniqueness_constraint(
     await ld6.save(db=db)
     await constraint.check(ld6)
 
-    ld6 = await Node.init(db=db, schema="LocationRack", branch=default_branch)
-    await ld6.new(db=db, name="ld6-ldn", parent=uk)
-    with pytest.raises(ValidationError, match=r"Violates uniqueness constraint 'parent-name' at parent"):
-        await constraint.check(ld6)
+    ld62 = await Node.init(db=db, schema="LocationRack", branch=default_branch)
+    await ld62.new(db=db, name="ld6-ldn2", parent=uk)
+    with pytest.raises(ValidationError, match=r"Violates uniqueness constraint 'parent-status' at status"):
+        await constraint.check(ld62)

changelog/+uniqueness-constraint-overlap.fixed.md

@@ -0,0 +1 @@
+Reduces the number of database queries we run when checking a uniqueness constraint during a node update or create mutation if that node uses a schema that inherits from a generic schema and the node schema's uniqueness constraints are contained within the generic schema's uniqueness constraints