diff --git a/backend/infrahub/core/changelog/models.py b/backend/infrahub/core/changelog/models.py index d78fbc48512..151c23edbad 100644 --- a/backend/infrahub/core/changelog/models.py +++ b/backend/infrahub/core/changelog/models.py @@ -591,7 +591,7 @@ def _process_added_peers( primary_changelog: NodeChangelog, ) -> list[NodeChangelog]: secondaries: list[NodeChangelog] = [] - peer_relation = peer_schema.get_relationship_by_identifier(id=str(rel_schema.identifier), raise_on_error=False) + peer_relation = peer_schema.get_reverse_relationship(relationship=rel_schema) if peer_relation: node_changelog = NodeChangelog(node_id=peer_id, node_kind=peer_kind, display_label="n/a") if peer_relation.cardinality == RelationshipCardinality.ONE: @@ -625,7 +625,7 @@ def _process_removed_peers( primary_changelog: NodeChangelog, ) -> list[NodeChangelog]: secondaries: list[NodeChangelog] = [] - peer_relation = peer_schema.get_relationship_by_identifier(id=str(rel_schema.identifier), raise_on_error=False) + peer_relation = peer_schema.get_reverse_relationship(relationship=rel_schema) if peer_relation: node_changelog = NodeChangelog(node_id=peer_id, node_kind=peer_kind, display_label="n/a") if peer_relation.cardinality == RelationshipCardinality.ONE: diff --git a/backend/infrahub/core/schema/basenode_schema.py b/backend/infrahub/core/schema/basenode_schema.py index ff2dbd43575..40653218944 100644 --- a/backend/infrahub/core/schema/basenode_schema.py +++ b/backend/infrahub/core/schema/basenode_schema.py @@ -11,7 +11,12 @@ from pydantic import ConfigDict, ValidationError, field_validator from infrahub.computed_attribute.jinja2 import InfrahubJinja2Template -from infrahub.core.constants import HashableModelState, RelationshipCardinality, RelationshipKind +from infrahub.core.constants import ( + HashableModelState, + RelationshipCardinality, + RelationshipDirection, + RelationshipKind, +) from infrahub.core.models import HashableModel, HashableModelDiff from .attribute_schema import AttributeSchema, get_attribute_schema_class_for_kind @@ -378,6 +383,31 @@ def get_relationships_by_identifier(self, id: str) -> list[RelationshipSchema]: return rels + def get_reverse_relationship(self, relationship: RelationshipSchema) -> RelationshipSchema | None: + """Return the relationship on this schema that traverses back along the provided relationship. + + Sharing an identifier is not enough: two kinds can both declare the same identifier on the same + side, and a single kind can declare both sides of it. The peer must point the opposite way, and a + bidirectional relationship is never its own reverse because both sides are stored asymmetrically. + + Returns None when this schema declares no relationship pointing back. + """ + expected_direction = relationship.direction.neighbor_direction + + for candidate in self.get_relationships_by_identifier(id=relationship.get_identifier()): + # A peer has to point the opposite way, which rules out every candidate on the same side. + # Both sides of a bidirectional pair share BIDIR, so there the name is the only thing that + # separates a real peer from the relationship we started on: an identical name means two + # kinds inherited the same relationship from a common generic (CoreGroup.members on both + # CoreStandardGroup and CoreGeneratorGroup), not two sides of one relationship. + if candidate.direction != expected_direction or ( + candidate.direction == RelationshipDirection.BIDIR and candidate.name == relationship.name + ): + continue + return candidate + + return None + def get_relationships_of_kind(self, relationship_kinds: Iterable[RelationshipKind]) -> list[RelationshipSchema]: return [r for r in self.relationships if r.kind in relationship_kinds] diff --git a/backend/tests/component/graphql/mutations/test_group_event_collection.py b/backend/tests/component/graphql/mutations/test_group_event_collection.py index 465ed39ca6d..8f38d3b827b 100644 --- a/backend/tests/component/graphql/mutations/test_group_event_collection.py +++ b/backend/tests/component/graphql/mutations/test_group_event_collection.py @@ -1,6 +1,12 @@ +import pytest + from infrahub.auth.session import AccountSession +from infrahub.core import registry from infrahub.core.branch import Branch +from infrahub.core.constants import InfrahubKind +from infrahub.core.manager import NodeManager from infrahub.core.node import Node +from infrahub.core.schema import AttributeSchema, NodeSchema, SchemaRoot from infrahub.core.schema.schema_branch import SchemaBranch from infrahub.database import InfrahubDatabase from infrahub.events.group_action import GroupMemberAddedEvent, GroupMemberRemovedEvent @@ -184,3 +190,88 @@ async def test_node_mutation_to_group_event( assert len(orphan_group_event.members) == 1 assert EventNode(id=person_id, kind="TestPerson") in orphan_group_event.members assert len(orphan_group_event.ancestors) == 0 + + +@pytest.fixture +async def tracking_group_schema( + db: InfrahubDatabase, default_branch: Branch, node_group_schema: None, standard_group_schema: None +) -> None: + """Register a second group kind, so group-to-group enrolment can be exercised across two kinds.""" + schema = SchemaRoot( + nodes=[ + NodeSchema( + name="TrackingGroup", + namespace="Test", + inherit_from=[InfrahubKind.GENERICGROUP], + attributes=[AttributeSchema(name="name", kind="Text", unique=True)], + ) + ] + ) + registry.schema.register_schema(schema=schema, branch=default_branch.name) + + +async def test_group_enrolled_into_group_only_emits_event_for_enclosing_group( + db: InfrahubDatabase, + default_branch: Branch, + tracking_group_schema: None, + session_first_account: AccountSession, +) -> None: + """Enrolling a group into another group reports the enclosing group as the one that gained a member. + + The enrolled group gains nothing, so it must not be reported as having gained a member itself. The two + groups are of different kinds, which is how a repository import enrols one group into another. + """ + enclosing_group = await Node.init(db=db, schema="TestTrackingGroup", branch=default_branch) + await enclosing_group.new(db=db, name="enclosing_group") + await enclosing_group.save(db=db) + enrolled_group = await Node.init(db=db, schema="CoreStandardGroup", branch=default_branch) + await enrolled_group.new(db=db, name="enrolled_group") + await enrolled_group.save(db=db) + + memory_event = MemoryInfrahubEvent() + service = await InfrahubServices.new(event=memory_event) + default_branch.update_schema_hash() + gql_params = await prepare_graphql_params( + db=db, branch=default_branch, service=service, account_session=session_first_account + ) + + update_query = """ + mutation($group: String!, $member: String!) { + TestTrackingGroupUpdate(data: + { + id: $group, + members: [{id: $member}] + } + ) { + ok + } + } + """ + result = await graphql( + schema=gql_params.schema, + source=update_query, + context_value=gql_params.context, + root_value=None, + variable_values={"group": enclosing_group.get_id(), "member": enrolled_group.get_id()}, + ) + + assert not result.errors + assert gql_params.context.background + await gql_params.context.background() + + member_added_events = [event for event in memory_event.events if isinstance(event, GroupMemberAddedEvent)] + assert [event.node_id for event in member_added_events] == [enclosing_group.get_id()] + assert member_added_events[0].members == [EventNode(id=enrolled_group.get_id(), kind="CoreStandardGroup")] + assert member_added_events[0].kind == "TestTrackingGroup" + + node_updated_events = [event for event in memory_event.events if isinstance(event, NodeUpdatedEvent)] + assert [event.node_id for event in node_updated_events] == [enclosing_group.get_id()] + + assert len(memory_event.events) == 2 + + reloaded_enclosing = await NodeManager.get_one(db=db, id=enclosing_group.get_id(), prefetch_relationships=True) + reloaded_enrolled = await NodeManager.get_one(db=db, id=enrolled_group.get_id(), prefetch_relationships=True) + assert list(await reloaded_enclosing.members.get_peers(db=db)) == [enrolled_group.get_id()] + # Both group kinds read the one stored membership through the same relationship, so it is visible + # from either side and cannot say which group gained a member: only the mutation says that. + assert list(await reloaded_enrolled.members.get_peers(db=db)) == [enclosing_group.get_id()] diff --git a/changelog/10314.fixed.md b/changelog/10314.fixed.md new file mode 100644 index 00000000000..ed95a598ce0 --- /dev/null +++ b/changelog/10314.fixed.md @@ -0,0 +1 @@ +Adding a group as a member of another group no longer reports the added group as the one that gained a member, which previously made every repository import start a Generator run that failed.