Merge pull request #5817 from opsmill/pog-merge-changelog-conflict-attributes-IFC-1256

Add support in DiffChangelogCollector for attribute value conflicts

Author: ogenstad

backend/infrahub/core/changelog/diff.py

@@ -7,6 +7,7 @@
 
 from infrahub.core.constants import DiffAction, RelationshipCardinality
 from infrahub.core.constants.database import DatabaseEdgeType
+from infrahub.core.diff.model.path import ConflictSelection
 
 from .models import (
     AttributeChangelog,
@@ -21,6 +22,7 @@
     from infrahub.core.diff.model.path import (
         EnrichedDiffAttribute,
         EnrichedDiffNode,
+        EnrichedDiffProperty,
         EnrichedDiffRelationship,
         EnrichedDiffRoot,
     )
@@ -86,8 +88,9 @@ def _process_node_attribute(
             match attr_property.property_type:
                 case DatabaseEdgeType.HAS_VALUE:
                     # TODO deserialize correct value type from string
-                    changelog_attribute.value = attr_property.new_value
-                    changelog_attribute.value_previous = attr_property.previous_value
+                    if _keep_branch_update(diff_property=attr_property):
+                        changelog_attribute.value = attr_property.new_value
+                        changelog_attribute.value_previous = attr_property.previous_value
                 case DatabaseEdgeType.IS_PROTECTED:
                     changelog_attribute.add_property(
                         name="is_protected",
@@ -113,7 +116,7 @@ def _process_node_attribute(
                         value_previous=attr_property.previous_value,
                     )
 
-        node.attributes[attribute.name] = changelog_attribute
+        node.add_attribute(attribute=changelog_attribute)
 
     def _process_node_relationship(self, node: NodeChangelog, relationship: EnrichedDiffRelationship) -> None:
         match relationship.cardinality:
@@ -229,4 +232,10 @@ def collect_changelogs(self) -> Sequence[tuple[DiffAction, NodeChangelog]]:
             for node in self._diff.nodes
             if node.action != DiffAction.UNCHANGED
         ]
-        return changelogs
+        return [(action, node_changelog) for action, node_changelog in changelogs if node_changelog.has_changes]
+
+
+def _keep_branch_update(diff_property: EnrichedDiffProperty) -> bool:
+    if diff_property.conflict and diff_property.conflict.selected_branch == ConflictSelection.BASE_BRANCH:
+        return False
+    return True

backend/infrahub/core/changelog/models.py

@@ -304,7 +304,8 @@ def delete_relationship(self, relationship: Relationship) -> None:
             )
 
     def add_attribute(self, attribute: AttributeChangelog) -> None:
-        self.attributes[attribute.name] = attribute
+        if attribute.has_updates:
+            self.attributes[attribute.name] = attribute
 
     def add_relationship(
         self, relationship: RelationshipCardinalityOneChangelog | RelationshipCardinalityManyChangelog

backend/tests/unit/core/changelog/test_diff.py

@@ -1,9 +1,17 @@
+from typing import Literal
+from unittest.mock import AsyncMock
+
+import pytest
+
 from infrahub.core.branch import Branch
 from infrahub.core.changelog.diff import DiffChangelogCollector
 from infrahub.core.changelog.models import RelationshipCardinalityManyChangelog, RelationshipCardinalityOneChangelog
 from infrahub.core.constants import DiffAction
 from infrahub.core.diff.coordinator import DiffCoordinator
+from infrahub.core.diff.data_check_synchronizer import DiffDataCheckSynchronizer
 from infrahub.core.diff.merger.merger import DiffMerger
+from infrahub.core.diff.model.path import ConflictSelection
+from infrahub.core.diff.repository.repository import DiffRepository
 from infrahub.core.initialization import create_branch
 from infrahub.core.manager import NodeManager
 from infrahub.core.node import Node
@@ -133,3 +141,74 @@ async def test_merge_diff_changelogs(db: InfrahubDatabase, default_branch, car_p
     assert c2_changelog.relationships["owner"].properties["owner"].value_previous is None
     assert c2_changelog.relationships["owner"].properties["source"].value == p1.id
     assert c2_changelog.relationships["owner"].properties["source"].value_previous is None
+
+
+class TestConflict:
+    async def _get_diff_coordinator(self, db: InfrahubDatabase, branch: Branch) -> DiffCoordinator:
+        component_registry = get_component_registry()
+        diff_coordinator = await component_registry.get_component(DiffCoordinator, db=db, branch=branch)
+        diff_coordinator.data_check_synchronizer = AsyncMock(spec=DiffDataCheckSynchronizer)
+        return diff_coordinator
+
+    async def _get_diff_merger(self, db: InfrahubDatabase, branch: Branch) -> DiffMerger:
+        component_registry = get_component_registry()
+        return await component_registry.get_component(DiffMerger, db=db, branch=branch)
+
+    @pytest.fixture
+    async def diff_repository(self, db: InfrahubDatabase, default_branch: Branch) -> DiffRepository:
+        component_registry = get_component_registry()
+        return await component_registry.get_component(DiffRepository, db=db, branch=default_branch)
+
+    @pytest.mark.parametrize(
+        "conflict_selection,expected_value",
+        [(ConflictSelection.BASE_BRANCH, "John-main"), (ConflictSelection.DIFF_BRANCH, "John-branch")],
+    )
+    async def test_diff_and_merge_with_attribute_value_conflict(
+        self,
+        db: InfrahubDatabase,
+        default_branch: Branch,
+        diff_repository: DiffRepository,
+        person_john_main: Node,
+        person_jane_main: Node,
+        person_alfred_main: Node,
+        car_accord_main: Node,
+        conflict_selection: ConflictSelection,
+        expected_value: Literal["John-main", "John-branch"],
+    ):
+        branch2 = await create_branch(db=db, branch_name="branch2")
+        john_main = await NodeManager.get_one(db=db, id=person_john_main.id)
+        john_main.name.value = "John-main"
+        await john_main.save(db=db)
+        john_branch = await NodeManager.get_one(db=db, branch=branch2, id=person_john_main.id)
+        john_branch.name.value = "John-branch"
+        await john_branch.save(db=db)
+
+        at = Timestamp()
+        diff_coordinator = await self._get_diff_coordinator(db=db, branch=branch2)
+        enriched_diff_metadata = await diff_coordinator.update_branch_diff(
+            base_branch=default_branch, diff_branch=branch2
+        )
+        enriched_diff = await diff_repository.get_one(
+            diff_branch_name=enriched_diff_metadata.diff_branch_name, diff_id=enriched_diff_metadata.uuid
+        )
+        conflicts_map = enriched_diff.get_all_conflicts()
+        assert len(conflicts_map) == 1
+        conflict = next(iter(conflicts_map.values()))
+        await diff_repository.update_conflict_by_id(conflict_id=conflict.uuid, selection=conflict_selection)
+        diff_merger = await self._get_diff_merger(db=db, branch=branch2)
+        diff = await diff_merger.merge_graph(at=at)
+        diff_events = DiffChangelogCollector(diff=diff, db=db, branch=branch2)
+        events = diff_events.collect_changelogs()
+
+        match conflict_selection:
+            case ConflictSelection.BASE_BRANCH:
+                # When we want to keep the conflict in the base branch we don't expect to see any updates after the merge
+                assert len(events) == 0
+            case ConflictSelection.DIFF_BRANCH:
+                # Expect to see changes on the diff branch when we keep changes from that branch
+                assert len(events) == 1
+                event = events[0]
+                action, node_changelog = event
+                assert action == DiffAction.UPDATED
+                assert node_changelog.attributes["name"].value == "John-branch"
+                assert node_changelog.attributes["name"].value_previous == "John"