diff --git a/backend/infrahub/core/diff/conflicts_enricher.py b/backend/infrahub/core/diff/conflicts_enricher.py index ac8e8e0710..303269b302 100644 --- a/backend/infrahub/core/diff/conflicts_enricher.py +++ b/backend/infrahub/core/diff/conflicts_enricher.py @@ -3,7 +3,6 @@ from infrahub.core.constants import DiffAction, RelationshipCardinality from infrahub.core.constants.database import DatabaseEdgeType -from .managed_relationship_checker import ManagedRelationshipChecker from .model.path import ( EnrichedDiffAttribute, EnrichedDiffConflict, @@ -16,8 +15,7 @@ class ConflictsEnricher: - def __init__(self, managed_relationship_checker: ManagedRelationshipChecker) -> None: - self.managed_relationship_checker = managed_relationship_checker + def __init__(self) -> None: self._base_branch_name: str | None = None self._diff_branch_name: str | None = None @@ -38,7 +36,6 @@ async def add_conflicts_to_branch_diff( ) -> None: self._base_branch_name = branch_diff_root.base_branch_name self._diff_branch_name = branch_diff_root.diff_branch_name - self.managed_relationship_checker.reset() base_node_map = {n.uuid: n for n in base_diff_root.nodes} branch_node_map = {n.uuid: n for n in branch_diff_root.nodes} @@ -82,7 +79,6 @@ def _add_node_conflicts(self, base_node: EnrichedDiffNode, branch_node: Enriched self._add_relationship_conflicts( base_relationship=base_relationship, branch_relationship=branch_relationship, - node_kind=branch_node.kind, ) else: branch_relationship.clear_conflicts() @@ -137,11 +133,8 @@ def _add_attribute_conflicts( ) def _add_relationship_conflicts( - self, base_relationship: EnrichedDiffRelationship, branch_relationship: EnrichedDiffRelationship, node_kind: str + self, base_relationship: EnrichedDiffRelationship, branch_relationship: EnrichedDiffRelationship ) -> None: - is_managed = self.managed_relationship_checker.check( - node_kind=node_kind, relationship_name=branch_relationship.name - ) is_cardinality_one = branch_relationship.cardinality is RelationshipCardinality.ONE if is_cardinality_one: if not base_relationship.relationships or not branch_relationship.relationships: @@ -153,7 +146,7 @@ def _add_relationship_conflicts( base_element=base_element, branch_element=branch_element, is_cardinality_one=is_cardinality_one, - is_managed=is_managed, + is_managed=branch_relationship.is_managed, ) return base_peer_id_map = {element.peer_id: element for element in base_relationship.relationships} @@ -168,7 +161,7 @@ def _add_relationship_conflicts( base_element=base_element, branch_element=branch_element, is_cardinality_one=is_cardinality_one, - is_managed=is_managed, + is_managed=branch_relationship.is_managed, ) def _add_relationship_conflicts_for_one_peer( diff --git a/backend/infrahub/core/diff/query_parser.py b/backend/infrahub/core/diff/query_parser.py index afc89110ad..8967af83f8 100644 --- a/backend/infrahub/core/diff/query_parser.py +++ b/backend/infrahub/core/diff/query_parser.py @@ -558,10 +558,7 @@ def parse(self, include_unchanged: bool = False) -> None: def _parse_path(self, database_path: DatabasePath) -> None: diff_root = self._get_diff_root(database_path=database_path) diff_node = self._get_diff_node(database_path=database_path, diff_root=diff_root) - is_base_branch = False - if self.base_branch_name != self.diff_branch_name and diff_root.branch == self.base_branch_name: - is_base_branch = True - self._update_attribute_level(database_path=database_path, diff_node=diff_node, is_base_branch=is_base_branch) + self._update_attribute_level(database_path=database_path, diff_node=diff_node) def _get_diff_root(self, database_path: DatabasePath) -> DiffRootIntermediate: branch = database_path.deepest_branch @@ -595,9 +592,7 @@ def _get_relationship_schema( return rel_schema return None - def _update_attribute_level( - self, database_path: DatabasePath, diff_node: DiffNodeIntermediate, is_base_branch: bool - ) -> None: + def _update_attribute_level(self, database_path: DatabasePath, diff_node: DiffNodeIntermediate) -> None: node_schema = self.db.schema.get( name=database_path.node_kind, branch=database_path.deepest_branch, duplicate=False ) @@ -608,9 +603,7 @@ def _update_attribute_level( relationship_schema = self._get_relationship_schema(database_path=database_path, node_schema=node_schema) if not relationship_schema: return - diff_relationship = self._get_diff_relationship( - diff_node=diff_node, relationship_schema=relationship_schema, is_base_branch=is_base_branch - ) + diff_relationship = self._get_diff_relationship(diff_node=diff_node, relationship_schema=relationship_schema) diff_relationship.add_path( database_path=database_path, diff_from_time=self.from_time, diff_to_time=self.to_time ) @@ -647,15 +640,13 @@ def _update_attribute_property( ) def _get_diff_relationship( - self, diff_node: DiffNodeIntermediate, relationship_schema: RelationshipSchema, is_base_branch: bool + self, diff_node: DiffNodeIntermediate, relationship_schema: RelationshipSchema ) -> DiffRelationshipIntermediate: diff_relationship = diff_node.relationships_by_name.get(relationship_schema.name) if not diff_relationship: - is_managed = False - if is_base_branch and self.managed_relationship_checker.check( + is_managed = self.managed_relationship_checker.check( node_kind=diff_node.kind, relationship_name=relationship_schema.name - ): - is_managed = True + ) diff_relationship = DiffRelationshipIntermediate( name=relationship_schema.name, cardinality=relationship_schema.cardinality, diff --git a/backend/infrahub/dependencies/builder/diff/conflicts_enricher.py b/backend/infrahub/dependencies/builder/diff/conflicts_enricher.py index 4ca3a13ca0..9c8e8e514b 100644 --- a/backend/infrahub/dependencies/builder/diff/conflicts_enricher.py +++ b/backend/infrahub/dependencies/builder/diff/conflicts_enricher.py @@ -1,12 +1,8 @@ from infrahub.core.diff.conflicts_enricher import ConflictsEnricher from infrahub.dependencies.interface import DependencyBuilder, DependencyBuilderContext -from .managed_relationship_checker import ManagedRelationshipCheckerDependency - class DiffConflictsEnricherDependency(DependencyBuilder[ConflictsEnricher]): @classmethod def build(cls, context: DependencyBuilderContext) -> ConflictsEnricher: - return ConflictsEnricher( - managed_relationship_checker=ManagedRelationshipCheckerDependency.build(context=context) - ) + return ConflictsEnricher() diff --git a/backend/tests/integration/ipam/test_ipam_rebase_reconcile.py b/backend/tests/integration/ipam/test_ipam_rebase_reconcile.py index 1ce9c8436e..4d17e90cde 100644 --- a/backend/tests/integration/ipam/test_ipam_rebase_reconcile.py +++ b/backend/tests/integration/ipam/test_ipam_rebase_reconcile.py @@ -16,6 +16,21 @@ from infrahub.database import InfrahubDatabase + +BRANCH_CREATE = """ + mutation($branch: String!) { + BranchCreate(data: { + name: $branch + }) { + ok + object { + id + name + } + } + } +""" + CREATE_IPPREFIX = """ mutation CreatePrefix($prefix: String!, $namespace_id: String!) { IpamIPPrefixCreate( @@ -85,7 +100,15 @@ async def test_step02_add_delete_prefix( initial_dataset, client: InfrahubClient, ) -> None: - branch = await create_branch(db=db, branch_name="delete_prefix") + gql_params = prepare_graphql_params(db=db, include_subscription=False, branch=registry.default_branch) + result = await graphql( + schema=gql_params.schema, + source=BRANCH_CREATE, + context_value=gql_params.context, + variable_values={"branch": "delete_prefix"}, + ) + assert not result.errors + branch = await registry.get_branch(db=db, branch="delete_prefix") gql_params = prepare_graphql_params(db=db, include_subscription=False, branch=registry.default_branch) result = await graphql( diff --git a/backend/tests/unit/core/diff/factories.py b/backend/tests/unit/core/diff/factories.py index 4306f0291c..07fd982006 100644 --- a/backend/tests/unit/core/diff/factories.py +++ b/backend/tests/unit/core/diff/factories.py @@ -48,7 +48,6 @@ class EnrichedRelationshipGroupFactory(DataclassFactory[EnrichedDiffRelationship num_conflicts = 0 nodes = set() contains_conflict = False - is_managed = False class EnrichedRelationshipElementFactory(DataclassFactory[EnrichedDiffSingleRelationship]): diff --git a/backend/tests/unit/core/diff/test_conflicts_enricher.py b/backend/tests/unit/core/diff/test_conflicts_enricher.py index bd0a8c4798..aa8a046d94 100644 --- a/backend/tests/unit/core/diff/test_conflicts_enricher.py +++ b/backend/tests/unit/core/diff/test_conflicts_enricher.py @@ -1,5 +1,4 @@ import random -from unittest.mock import AsyncMock from uuid import uuid4 import pytest @@ -8,7 +7,6 @@ from infrahub.core.constants import DiffAction, RelationshipCardinality from infrahub.core.constants.database import DatabaseEdgeType from infrahub.core.diff.conflicts_enricher import ConflictsEnricher -from infrahub.core.diff.managed_relationship_checker import ManagedRelationshipChecker from infrahub.core.diff.model.path import EnrichedDiffConflict from infrahub.core.initialization import create_branch from infrahub.core.timestamp import Timestamp @@ -33,9 +31,7 @@ def setup_method(self): self.to_time = Timestamp() async def __call_system_under_test(self, db: InfrahubDatabase, base_enriched_diff, branch_enriched_diff) -> None: - mock_rel_checker = AsyncMock(spec=ManagedRelationshipChecker) - mock_rel_checker.check.return_value = False - conflicts_enricher = ConflictsEnricher(managed_relationship_checker=mock_rel_checker) + conflicts_enricher = ConflictsEnricher() return await conflicts_enricher.add_conflicts_to_branch_diff( base_diff_root=base_enriched_diff, branch_diff_root=branch_enriched_diff ) @@ -172,7 +168,8 @@ async def test_one_attribute_conflict(self, db: InfrahubDatabase): else: assert prop.conflict is None - async def test_cardinality_one_conflicts(self, db: InfrahubDatabase, car_person_schema): + @pytest.mark.parametrize("is_managed", [True, False]) + async def test_cardinality_one_conflicts(self, db: InfrahubDatabase, car_person_schema, is_managed: bool): branch = await create_branch(db=db, branch_name="branch") property_type = DatabaseEdgeType.IS_RELATED relationship_name = "owner" @@ -199,6 +196,7 @@ async def test_cardinality_one_conflicts(self, db: InfrahubDatabase, car_person_ ) }, cardinality=RelationshipCardinality.ONE, + is_managed=is_managed, ) } base_nodes = { @@ -227,6 +225,7 @@ async def test_cardinality_one_conflicts(self, db: InfrahubDatabase, car_person_ ) }, cardinality=RelationshipCardinality.ONE, + is_managed=is_managed, ) } branch_nodes = { @@ -253,6 +252,7 @@ async def test_cardinality_one_conflicts(self, db: InfrahubDatabase, car_person_ node.uuid == node_uuid and rel.name == relationship_name and rel_element.peer_id == previous_peer_id + and not is_managed ): assert rel_element.conflict assert rel_element.conflict == EnrichedDiffConflict( @@ -265,6 +265,8 @@ async def test_cardinality_one_conflicts(self, db: InfrahubDatabase, car_person_ diff_branch_changed_at=branch_conflict_property.changed_at, selected_branch=None, ) + else: + assert rel_element.conflict is None async def test_cardinality_many_conflicts(self, db: InfrahubDatabase, car_person_schema): branch = await create_branch(db=db, branch_name="branch") @@ -309,6 +311,7 @@ async def test_cardinality_many_conflicts(self, db: InfrahubDatabase, car_person EnrichedRelationshipElementFactory.build(peer_id=peer_id_2, properties=base_properties_2), }, cardinality=RelationshipCardinality.MANY, + is_managed=False, ) } base_nodes = { @@ -349,6 +352,7 @@ async def test_cardinality_many_conflicts(self, db: InfrahubDatabase, car_person EnrichedRelationshipElementFactory.build(peer_id=peer_id_2, properties=branch_properties_2), }, cardinality=RelationshipCardinality.MANY, + is_managed=False, ) } branch_nodes = { @@ -564,6 +568,7 @@ async def test_manually_fixed_cardinality_one_conflict_cleared(self, db: Infrahu ) }, cardinality=RelationshipCardinality.ONE, + is_managed=False, ) } base_nodes = { @@ -596,6 +601,7 @@ async def test_manually_fixed_cardinality_one_conflict_cleared(self, db: Infrahu ) }, cardinality=RelationshipCardinality.ONE, + is_managed=False, ) } branch_nodes = { @@ -646,6 +652,7 @@ async def test_unchanged_cardinality_one_clears_conflicts( ) }, cardinality=RelationshipCardinality.ONE, + is_managed=False, ) } base_nodes = { @@ -680,6 +687,7 @@ async def test_unchanged_cardinality_one_clears_conflicts( ) }, cardinality=RelationshipCardinality.ONE, + is_managed=False, ) } branch_nodes = { @@ -747,6 +755,7 @@ async def test_unchanged_cardinality_many_rel_clears_conflict(self, db: Infrahub EnrichedRelationshipElementFactory.build(peer_id=peer_id_2, properties=base_properties_2), }, cardinality=RelationshipCardinality.MANY, + is_managed=False, ) } base_nodes = { @@ -788,6 +797,7 @@ async def test_unchanged_cardinality_many_rel_clears_conflict(self, db: Infrahub EnrichedRelationshipElementFactory.build(peer_id=peer_id_2, properties=branch_properties_2), }, cardinality=RelationshipCardinality.MANY, + is_managed=False, ) } branch_nodes = { @@ -861,3 +871,87 @@ async def test_manually_fixed_node_conflict_cleared(self, db: InfrahubDatabase): for node in branch_root.nodes: assert node.conflict is None + + async def test_managed_rel_cannot_create_conflict(self, db: InfrahubDatabase, car_person_schema): + base_action = DiffAction.UPDATED + branch_action = DiffAction.REMOVED + branch = await create_branch(db=db, branch_name="branch") + property_type = DatabaseEdgeType.IS_RELATED + relationship_name = "owner" + node_uuid = str(uuid4()) + node_kind = "TestCar" + peer_id = str(uuid4()) + base_properties = { + EnrichedPropertyFactory.build(property_type=DatabaseEdgeType.IS_VISIBLE), + EnrichedPropertyFactory.build(property_type=property_type, action=base_action), + } + base_relationships = { + EnrichedRelationshipGroupFactory.build( + name=relationship_name, + relationships={ + EnrichedRelationshipElementFactory.build( + peer_id=peer_id, properties=base_properties, action=DiffAction.UPDATED + ) + }, + cardinality=RelationshipCardinality.ONE, + is_managed=True, + ) + } + base_nodes = { + EnrichedNodeFactory.build( + uuid=node_uuid, + kind=node_kind, + action=DiffAction.UPDATED, + relationships=base_relationships, + ), + EnrichedNodeFactory.build(relationships=set()), + } + base_root = EnrichedRootFactory.build(nodes=base_nodes) + branch_conflict_property = EnrichedPropertyFactory.build( + property_type=property_type, + previous_value=peer_id, + action=branch_action, + conflict=EnrichedConflictFactory.build(), + ) + branch_properties = { + branch_conflict_property, + EnrichedPropertyFactory.build(property_type=DatabaseEdgeType.HAS_OWNER), + } + branch_relationships = { + EnrichedRelationshipGroupFactory.build( + name=relationship_name, + relationships={ + EnrichedRelationshipElementFactory.build( + peer_id=peer_id, + properties=branch_properties, + action=DiffAction.REMOVED, + conflict=EnrichedConflictFactory.build(), + ) + }, + cardinality=RelationshipCardinality.ONE, + is_managed=True, + ) + } + branch_nodes = { + EnrichedNodeFactory.build( + uuid=node_uuid, + kind=node_kind, + action=DiffAction.UPDATED, + relationships=branch_relationships, + ), + EnrichedNodeFactory.build(relationships=set()), + } + branch_root = EnrichedRootFactory.build(nodes=branch_nodes, diff_branch_name=branch.name) + + await self.__call_system_under_test(db=db, base_enriched_diff=base_root, branch_enriched_diff=branch_root) + + for node in branch_root.nodes: + assert node.conflict is None + for attribute in node.attributes: + for prop in attribute.properties: + assert prop.conflict is None + for rel in node.relationships: + for rel_element in rel.relationships: + assert rel_element.conflict is None + for prop in rel_element.properties: + assert prop.conflict is None diff --git a/backend/tests/unit/core/diff/test_diff_calculator.py b/backend/tests/unit/core/diff/test_diff_calculator.py index e44e4d7f25..4a9e8891bc 100644 --- a/backend/tests/unit/core/diff/test_diff_calculator.py +++ b/backend/tests/unit/core/diff/test_diff_calculator.py @@ -2881,3 +2881,55 @@ async def test_diff_relationship_property_update_on_main( diff_rel = node_diff.relationships.pop() assert diff_rel.name == "owner" assert diff_rel.action is DiffAction.ADDED + + +async def test_managed_relationship_identified( + db: InfrahubDatabase, + default_branch: Branch, + person_alfred_main: Node, + person_john_main: Node, + car_accord_main: Node, +): + branch = await create_branch(db=db, branch_name="branch") + from_time = Timestamp() + car_branch = await NodeManager.get_one(db=db, branch=branch, id=car_accord_main.id) + await car_branch.owner.update(db=db, data=person_alfred_main) + await car_branch.save(db=db) + + component_registry = get_component_registry() + diff_calculator = await component_registry.get_component(DiffCalculator, db=db, branch=branch) + + class MockManagedRelChecker: + def reset(self): ... + def check(self, node_kind: str, relationship_name: str) -> bool: + return node_kind == "TestCar" and relationship_name == "owner" + + diff_calculator.query_parser.managed_relationship_checker = MockManagedRelChecker() + + calculated_diffs = await diff_calculator.calculate_diff( + base_branch=default_branch, + diff_branch=branch, + from_time=from_time, + to_time=Timestamp(), + ) + + base_branch_diff = calculated_diffs.base_branch_diff + assert len(base_branch_diff.nodes) == 0 + diff_branch_diff = calculated_diffs.diff_branch_diff + nodes_by_id = {n.uuid: n for n in diff_branch_diff.nodes} + assert set(nodes_by_id.keys()) == {car_accord_main.id, person_john_main.id, person_alfred_main.id} + + car_diff = nodes_by_id[car_accord_main.id] + assert len(car_diff.relationships) == 1 + owner_rel_diff = car_diff.relationships.pop() + assert owner_rel_diff.is_managed is True + + john_diff = nodes_by_id[person_john_main.id] + assert len(john_diff.relationships) == 1 + cars_rel_diff = john_diff.relationships.pop() + assert cars_rel_diff.is_managed is False + + alfred_diff = nodes_by_id[person_alfred_main.id] + assert len(alfred_diff.relationships) == 1 + cars_rel_diff = alfred_diff.relationships.pop() + assert cars_rel_diff.is_managed is False diff --git a/backend/tests/unit/core/diff/test_diff_combiner.py b/backend/tests/unit/core/diff/test_diff_combiner.py index 9e2bd7d709..ff1a3b2e4a 100644 --- a/backend/tests/unit/core/diff/test_diff_combiner.py +++ b/backend/tests/unit/core/diff/test_diff_combiner.py @@ -455,6 +455,7 @@ async def test_relationship_one_combined(self, with_schema_manager): action=DiffAction.ADDED, path_identifier=later_relationship.path_identifier, relationships={expected_relationship_element}, + is_managed=later_relationship.is_managed, ) expected_node = EnrichedDiffNode( uuid=later_node.uuid, @@ -560,6 +561,7 @@ async def test_relationship_many_combined(self, with_schema_manager): cardinality=RelationshipCardinality.MANY, relationships={added_element_1, removed_element_1, updated_element_1, canceled_element_1}, nodes=set(), + is_managed=True, ) relationship_group_2 = EnrichedRelationshipGroupFactory.build( name=relationship_name, @@ -568,6 +570,7 @@ async def test_relationship_many_combined(self, with_schema_manager): relationships={added_element_2, removed_element_2, updated_element_2, canceled_element_2}, changed_at=Timestamp(), nodes=set(), + is_managed=True, ) node_1 = EnrichedNodeFactory.build( kind="TestPerson", action=DiffAction.UPDATED, relationships={relationship_group_1} @@ -623,6 +626,7 @@ async def test_relationship_many_combined(self, with_schema_manager): action=DiffAction.UPDATED, path_identifier=relationship_group_2.path_identifier, relationships={expected_added_element, expected_removed_element, expected_updated_element}, + is_managed=True, ) expected_node = EnrichedDiffNode( uuid=node_1.uuid, @@ -688,6 +692,7 @@ async def test_relationship_with_only_nodes(self, with_schema_manager): path_identifier=later_relationship.path_identifier, relationships={element_1}, nodes={later_parent_node}, + is_managed=later_relationship.is_managed, ) expected_node = EnrichedDiffNode( uuid=later_node.uuid, @@ -857,6 +862,7 @@ async def test_unchanged_parents_correctly_updated(self): action=DiffAction.UNCHANGED, relationships=set(), nodes={expected_parent_node}, + is_managed=parent_rel_2.is_managed, ) expected_child_node = EnrichedDiffNode( uuid=child_node_uuid, @@ -929,6 +935,7 @@ async def test_updated_parents_correctly_updated(self): action=DiffAction.UPDATED, relationships={child_element_1}, nodes={expected_parent_2}, + is_managed=child_rel_2.is_managed, ) expected_child_node = EnrichedDiffNode( uuid=child_node_uuid, @@ -1088,6 +1095,7 @@ async def test_resetting_relationship_one_makes_it_unchanged(self, with_schema_m action=DiffAction.UPDATED, path_identifier=later_relationship.path_identifier, relationships={expected_relationship_element}, + is_managed=later_relationship.is_managed, ) expected_node = EnrichedDiffNode( uuid=later_node.uuid,