restrict diff field summaries to the single diff being queried

backend/infrahub/core/diff/query/field_summary.py

@@ -42,17 +42,17 @@ 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
-        CALL (kind) {
-            OPTIONAL MATCH (n:DiffNode {kind: kind})-[:DIFF_HAS_ATTRIBUTE]->(a:DiffAttribute)
+        WITH DISTINCT diff_root, n.kind AS kind
+        CALL (diff_root, kind) {
+            OPTIONAL MATCH (diff_root)-[:DIFF_HAS_NODE]->(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 kind, attr_names
-        CALL (kind) {
-            OPTIONAL MATCH (n:DiffNode {kind: kind})-[:DIFF_HAS_RELATIONSHIP]->(r:DiffRelationship)
+        WITH diff_root, kind, attr_names
+        CALL (diff_root, kind) {
+            OPTIONAL MATCH (diff_root)-[:DIFF_HAS_NODE]->(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

backend/tests/component/core/diff/repository/test_diff_field_summaries.py

@@ -0,0 +1,217 @@
+import random
+from typing import Generator
+
+import pytest
+
+from infrahub import config
+from infrahub.core.constants import DiffAction
+from infrahub.core.diff.model.path import (
+    BranchTrackingId,
+    EnrichedDiffNode,
+    NameTrackingId,
+    NodeDiffFieldSummary,
+)
+from infrahub.core.diff.parent_node_adder import DiffParentNodeAdder
+from infrahub.core.diff.repository.deserializer import EnrichedDiffDeserializer
+from infrahub.core.diff.repository.repository import DiffRepository
+from infrahub.core.timestamp import Timestamp
+from infrahub.database import InfrahubDatabase
+
+from ..factories import (
+    EnrichedAttributeFactory,
+    EnrichedNodeFactory,
+    EnrichedRelationshipGroupFactory,
+    EnrichedRootFactory,
+)
+from .base import DiffRepositoryTestBase
+
+
+class TestDiffNodeFieldSummaries(DiffRepositoryTestBase):
+    base_branch_name: str = "main"
+    diff_branch_name: str = "diff"
+    diff_from_time = Timestamp("2024-06-15T18:35:20Z")
+    diff_to_time = Timestamp("2024-06-15T18:49:40Z")
+
+    @pytest.fixture
+    def diff_repository(self, db: InfrahubDatabase) -> Generator[DiffRepository, None, None]:
+        original_depth = config.SETTINGS.database.max_depth_search_hierarchy
+        original_size = config.SETTINGS.database.query_size_limit
+        config.SETTINGS.database.max_depth_search_hierarchy = 10
+        config.SETTINGS.database.query_size_limit = 50
+        diff_repository = DiffRepository(
+            db=db, deserializer=EnrichedDiffDeserializer(DiffParentNodeAdder()), max_save_batch_size=30
+        )
+        yield diff_repository
+        config.SETTINGS.database.max_depth_search_hierarchy = original_depth
+        config.SETTINGS.database.query_size_limit = original_size
+
+    def _build_named_field_node(
+        self,
+        kind: str,
+        node_action: DiffAction,
+        attribute_actions: dict[str, DiffAction],
+        relationship_actions: dict[str, DiffAction],
+    ) -> EnrichedDiffNode:
+        return EnrichedNodeFactory.build(
+            kind=kind,
+            action=node_action,
+            attributes={
+                EnrichedAttributeFactory.build(name=name, action=action, properties=set())
+                for name, action in attribute_actions.items()
+            },
+            relationships={
+                EnrichedRelationshipGroupFactory.build(name=name, action=action, relationships=set(), nodes=set())
+                for name, action in relationship_actions.items()
+            },
+        )
+
+    async def test_get_node_field_summaries(self, diff_repository: DiffRepository) -> None:
+        diff_nodes = self._build_nodes(num_nodes=5, num_sub_fields=2)
+        for diff_node in list(diff_nodes)[:3]:
+            same_kind_diff_node = self.build_diff_node(num_sub_fields=3, no_recurse=True)
+            same_kind_diff_node.identifier.kind = diff_node.identifier.kind
+            same_attr_names = random.sample([a.name for a in diff_node.attributes], k=min(len(diff_node.attributes), 2))
+            for attr_diff, attr_name in zip(list(same_kind_diff_node.attributes)[:2], same_attr_names, strict=False):
+                attr_diff.name = attr_name
+            same_rel_names = random.sample(
+                [r.name for r in diff_node.relationships], k=min(len(diff_node.relationships), 2)
+            )
+            for rel_diff, rel_name in zip(list(same_kind_diff_node.relationships)[:2], same_rel_names, strict=False):
+                rel_diff.name = rel_name
+            diff_nodes.add(same_kind_diff_node)
+        diff_root = EnrichedRootFactory.build(nodes=diff_nodes)
+        diff_root.tracking_id = BranchTrackingId(name=diff_root.diff_branch_name)
+        await self._save_single_diff(diff_repository=diff_repository, enriched_diff=diff_root, do_summary_counts=False)
+
+        expected_map: dict[str, NodeDiffFieldSummary] = {}
+        for node in diff_root.nodes:
+            if node.action is DiffAction.UNCHANGED:
+                continue
+            if node.kind not in expected_map:
+                expected_map[node.kind] = NodeDiffFieldSummary(kind=node.kind)
+            field_summary = expected_map[node.kind]
+            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)
+        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_branch_name=diff_root.diff_branch_name, tracking_id=diff_root.tracking_id
+        )
+        retrieved_map = {summary.kind: summary for summary in retrieved_node_field_summaries}
+        assert expected_map == retrieved_map
+
+        retrieved_node_field_summaries = await diff_repository.get_node_field_summaries(
+            diff_branch_name=diff_root.diff_branch_name, diff_id=diff_root.uuid
+        )
+        retrieved_map = {summary.kind: summary for summary in retrieved_node_field_summaries}
+        assert expected_map == retrieved_map
+
+    async def test_get_node_field_summaries_excludes_other_diffs(
+        self, diff_repository: DiffRepository, reset_database: None
+    ) -> None:
+        """Only the changed fields of the requested diff are summarized.
+
+        Other diffs covering the same node kind must not contribute field names, and must not make a field
+        name that is unchanged in the requested diff look changed.
+        """
+        shared_kind = "TestingSharedKind"
+        requested_branch_name = "requested-branch"
+        requested_tracking_id = BranchTrackingId(name=requested_branch_name)
+
+        requested_diff = EnrichedRootFactory.build(
+            base_branch_name=self.base_branch_name,
+            diff_branch_name=requested_branch_name,
+            from_time=self.diff_from_time,
+            to_time=self.diff_to_time,
+            nodes={
+                self._build_named_field_node(
+                    kind=shared_kind,
+                    node_action=DiffAction.UPDATED,
+                    attribute_actions={"changed_attr": DiffAction.UPDATED, "quiet_attr": DiffAction.UNCHANGED},
+                    relationship_actions={"changed_rel": DiffAction.ADDED, "quiet_rel": DiffAction.UNCHANGED},
+                ),
+                # an unchanged node of the same kind contributes nothing, even with changed fields of its own
+                self._build_named_field_node(
+                    kind=shared_kind,
+                    node_action=DiffAction.UNCHANGED,
+                    attribute_actions={"quiet_node_attr": DiffAction.UPDATED},
+                    relationship_actions={"quiet_node_rel": DiffAction.REMOVED},
+                ),
+            },
+            tracking_id=requested_tracking_id,
+        )
+        await self._save_single_diff(
+            diff_repository=diff_repository, enriched_diff=requested_diff, do_summary_counts=False
+        )
+
+        other_branch_diff = EnrichedRootFactory.build(
+            base_branch_name=self.base_branch_name,
+            diff_branch_name="other-branch",
+            from_time=self.diff_from_time,
+            to_time=self.diff_to_time,
+            nodes={
+                self._build_named_field_node(
+                    kind=shared_kind,
+                    node_action=DiffAction.UPDATED,
+                    attribute_actions={
+                        # changed here, unchanged in the requested diff
+                        "quiet_attr": DiffAction.REMOVED,
+                        # unchanged here, changed in the requested diff
+                        "changed_attr": DiffAction.UNCHANGED,
+                        "other_branch_attr": DiffAction.ADDED,
+                    },
+                    relationship_actions={
+                        "quiet_rel": DiffAction.UPDATED,
+                        "changed_rel": DiffAction.UNCHANGED,
+                        "other_branch_rel": DiffAction.ADDED,
+                    },
+                )
+            },
+            tracking_id=BranchTrackingId(name="other-branch"),
+        )
+        await self._save_single_diff(
+            diff_repository=diff_repository, enriched_diff=other_branch_diff, do_summary_counts=False
+        )
+
+        merged_tracking_id = NameTrackingId(name="already-merged")
+        merged_diff = EnrichedRootFactory.build(
+            base_branch_name=self.base_branch_name,
+            diff_branch_name=requested_branch_name,
+            from_time=self.diff_from_time,
+            to_time=self.diff_to_time,
+            nodes={
+                self._build_named_field_node(
+                    kind=shared_kind,
+                    node_action=DiffAction.UPDATED,
+                    attribute_actions={
+                        "quiet_node_attr": DiffAction.UPDATED,
+                        "merged_attr": DiffAction.UPDATED,
+                    },
+                    relationship_actions={
+                        "quiet_node_rel": DiffAction.UPDATED,
+                        "merged_rel": DiffAction.UPDATED,
+                    },
+                )
+            },
+            tracking_id=merged_tracking_id,
+        )
+        await self._save_single_diff(
+            diff_repository=diff_repository, enriched_diff=merged_diff, do_summary_counts=False
+        )
+        await diff_repository.mark_tracking_ids_merged(tracking_ids=[merged_tracking_id])
+
+        expected_summaries = [
+            NodeDiffFieldSummary(kind=shared_kind, attribute_names={"changed_attr"}, relationship_names={"changed_rel"})
+        ]
+
+        retrieved_by_tracking_id = await diff_repository.get_node_field_summaries(
+            diff_branch_name=requested_branch_name, tracking_id=requested_tracking_id
+        )
+        assert retrieved_by_tracking_id == expected_summaries
+
+        retrieved_by_diff_id = await diff_repository.get_node_field_summaries(
+            diff_branch_name=requested_branch_name, diff_id=requested_diff.uuid
+        )
+        assert retrieved_by_diff_id == expected_summaries

backend/tests/component/core/diff/repository/test_diff_repository.py

@@ -15,7 +15,6 @@
     EnrichedDiffs,
     FrozenTrackingId,
     NameTrackingId,
-    NodeDiffFieldSummary,
 )
 from infrahub.core.diff.parent_node_adder import DiffParentNodeAdder
 from infrahub.core.diff.repository.deserializer import EnrichedDiffDeserializer
@@ -605,49 +604,6 @@ async def test_get_one_multiple_results_raises_distinct_error(
             await diff_repository.get_one(diff_branch_name=self.diff_branch_name)
         assert not issubclass(ResourceMultipleFoundError, ResourceNotFoundError)
 
-    async def test_get_node_field_summaries(self, diff_repository: DiffRepository) -> None:
-        diff_nodes = self._build_nodes(num_nodes=5, num_sub_fields=2)
-        for diff_node in list(diff_nodes)[:3]:
-            same_kind_diff_node = self.build_diff_node(num_sub_fields=3, no_recurse=True)
-            same_kind_diff_node.identifier.kind = diff_node.identifier.kind
-            same_attr_names = random.sample([a.name for a in diff_node.attributes], k=min(len(diff_node.attributes), 2))
-            for attr_diff, attr_name in zip(list(same_kind_diff_node.attributes)[:2], same_attr_names, strict=False):
-                attr_diff.name = attr_name
-            same_rel_names = random.sample(
-                [r.name for r in diff_node.relationships], k=min(len(diff_node.relationships), 2)
-            )
-            for rel_diff, rel_name in zip(list(same_kind_diff_node.relationships)[:2], same_rel_names, strict=False):
-                rel_diff.name = rel_name
-            diff_nodes.add(same_kind_diff_node)
-        diff_root = EnrichedRootFactory.build(nodes=diff_nodes)
-        diff_root.tracking_id = BranchTrackingId(name=diff_root.diff_branch_name)
-        await self._save_single_diff(diff_repository=diff_repository, enriched_diff=diff_root, do_summary_counts=False)
-
-        expected_map: dict[str, NodeDiffFieldSummary] = {}
-        for node in diff_root.nodes:
-            if node.action is DiffAction.UNCHANGED:
-                continue
-            if node.kind not in expected_map:
-                expected_map[node.kind] = NodeDiffFieldSummary(kind=node.kind)
-            field_summary = expected_map[node.kind]
-            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)
-        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_branch_name=diff_root.diff_branch_name, tracking_id=diff_root.tracking_id
-        )
-        retrieved_map = {summary.kind: summary for summary in retrieved_node_field_summaries}
-        assert expected_map == retrieved_map
-
-        retrieved_node_field_summaries = await diff_repository.get_node_field_summaries(
-            diff_branch_name=diff_root.diff_branch_name, diff_id=diff_root.uuid
-        )
-        retrieved_map = {summary.kind: summary for summary in retrieved_node_field_summaries}
-        assert expected_map == retrieved_map
-
     async def test_merge_tracking_ids(self, diff_repository: DiffRepository, reset_database: None) -> None:
         base_branch_name = "main"
         tracking_id_diff_1 = EnrichedRootFactory.build(base_branch_name=base_branch_name)

changelog/+diff-field-summary-cross-diff-leak.fixed.md

@@ -0,0 +1 @@
+Branch rebase and merge no longer validate schema constraints for attributes and relationships that were not actually modified on the branch. The query collecting the changed fields of a diff could match nodes belonging to other branches' diffs and to already-merged diffs, which would inflate the set of constraints to check.