diff --git a/changelog/1153.fixed.md b/changelog/1153.fixed.md new file mode 100644 index 000000000..60411a1d0 --- /dev/null +++ b/changelog/1153.fixed.md @@ -0,0 +1 @@ +Fetching or editing a relationship on a node that has no ID (e.g. a node created locally but not yet saved) now raises a clear error explaining the node needs to be saved first, instead of the generic `At least one filter must be provided to get()` message (for `fetch()`) or sending the caller in a circle between `add()` and `fetch()`. diff --git a/infrahub_sdk/node/relationship.py b/infrahub_sdk/node/relationship.py index 7069b13ee..f4bc6f4c0 100644 --- a/infrahub_sdk/node/relationship.py +++ b/infrahub_sdk/node/relationship.py @@ -2,7 +2,7 @@ from collections import defaultdict from collections.abc import Iterable -from typing import TYPE_CHECKING, Any, Generic, cast +from typing import TYPE_CHECKING, Any, Generic, NoReturn, cast from ..exceptions import ( Error, @@ -19,6 +19,25 @@ from .node import InfrahubNode, InfrahubNodeSync +def _raise_missing_identifier(node: InfrahubNode | InfrahubNodeSync, name: str) -> NoReturn: + """Raise a clear error for fetching/editing a relationship on a node with no ID. + + A relationship can only be fetched or edited once the parent node has an ID to look it up + by. When it does not, the underlying ``client.get()`` call would otherwise fail with a + generic "At least one filter must be provided to get()" message that gives the caller no + hint about the real cause. + + Raises: + UninitializedError: Always; the parent node has no ID to look the relationship up by. + + """ + raise UninitializedError( + f"Cannot access the '{name}' relationship because the {node._schema.kind} node has no ID to " + f"look it up by. This usually means the node was created locally but not saved yet — call " + f".save() on it first (or fetch it from Infrahub) before fetching or editing its relationships." + ) + + class RelationshipManagerBase(Generic[PeerT]): """Base class for :class:`RelationshipManager` and :class:`RelationshipManagerSync`. @@ -243,6 +262,8 @@ async def fetch(self) -> None: """ if not self.initialized: + if not self.node.id: + _raise_missing_identifier(self.node, self.name) exclude = self.node._schema.relationship_names + self.node._schema.attribute_names exclude.remove(self.schema.name) node = await self.client.get( @@ -293,6 +314,8 @@ def add(self, data: str | RelatedNode | dict) -> None: """ if not self.initialized: + if not self.node.id: + _raise_missing_identifier(self.node, self.name) raise UninitializedError("Must call fetch() on RelationshipManager before editing members") new_node = cast( "RelatedNode[PeerT]", RelatedNode(schema=self.schema, client=self.client, branch=self.branch, data=data) @@ -337,6 +360,8 @@ def remove(self, data: str | RelatedNode | dict) -> None: """ if not self.initialized: + if not self.node.id: + _raise_missing_identifier(self.node, self.name) raise UninitializedError("Must call fetch() on RelationshipManager before editing members") node_to_remove = RelatedNode(schema=self.schema, client=self.client, branch=self.branch, data=data) @@ -440,6 +465,8 @@ def fetch(self) -> None: """ if not self.initialized: + if not self.node.id: + _raise_missing_identifier(self.node, self.name) exclude = self.node._schema.relationship_names + self.node._schema.attribute_names exclude.remove(self.schema.name) node = self.client.get( @@ -490,6 +517,8 @@ def add(self, data: str | RelatedNodeSync | dict) -> None: """ if not self.initialized: + if not self.node.id: + _raise_missing_identifier(self.node, self.name) raise UninitializedError("Must call fetch() on RelationshipManager before editing members") new_node = cast( "RelatedNodeSync[PeerTSync]", @@ -535,6 +564,8 @@ def remove(self, data: str | RelatedNodeSync | dict) -> None: """ if not self.initialized: + if not self.node.id: + _raise_missing_identifier(self.node, self.name) raise UninitializedError("Must call fetch() on RelationshipManager before editing members") node_to_remove = RelatedNodeSync(schema=self.schema, client=self.client, branch=self.branch, data=data) diff --git a/tests/unit/sdk/test_node.py b/tests/unit/sdk/test_node.py index ab0d72ec0..1d44ce01b 100644 --- a/tests/unit/sdk/test_node.py +++ b/tests/unit/sdk/test_node.py @@ -9,7 +9,7 @@ import pytest -from infrahub_sdk.exceptions import FeatureNotSupportedError, NodeNotFoundError +from infrahub_sdk.exceptions import FeatureNotSupportedError, NodeNotFoundError, UninitializedError from infrahub_sdk.node import ( InfrahubNode, InfrahubNodeBase, @@ -342,6 +342,48 @@ async def test_cardinality_many_accepts_list( assert len(node.tags.peers) == 2 +@pytest.mark.parametrize("client_type", client_types) +async def test_fetch_relationship_without_node_id_raises_clear_error( + client: InfrahubClient, location_schema: NodeSchemaAPI, client_type: str +) -> None: + """fetch() on a node with no ID explains it isn't saved instead of raising the generic get() filter error.""" + if client_type == "standard": + node = InfrahubNode(client=client, schema=location_schema, data={"name": {"value": "JFK1"}}) + with pytest.raises(UninitializedError, match=r"has no ID"): + await node.tags.fetch() + else: + node = InfrahubNodeSync(client=client, schema=location_schema, data={"name": {"value": "JFK1"}}) + with pytest.raises(UninitializedError, match=r"has no ID"): + node.tags.fetch() + + +@pytest.mark.parametrize("client_type", client_types) +async def test_edit_relationship_without_node_id_raises_clear_error( + client: InfrahubClient, location_schema: NodeSchemaAPI, client_type: str +) -> None: + """add() on a node with no ID points the user at .save() instead of at fetch(), avoiding the circular guidance.""" + if client_type == "standard": + node = InfrahubNode(client=client, schema=location_schema, data={"name": {"value": "JFK1"}}) + else: + node = InfrahubNodeSync(client=client, schema=location_schema, data={"name": {"value": "JFK1"}}) + with pytest.raises(UninitializedError, match=r"has no ID"): + node.tags.add("11111111-1111-1111-1111-111111111111") + + +@pytest.mark.parametrize("client_type", client_types) +async def test_edit_relationship_with_node_id_still_requires_fetch( + client: InfrahubClient, location_schema: NodeSchemaAPI, client_type: str +) -> None: + """When the node has an ID but the relationship isn't loaded, the original "call fetch()" guidance is preserved.""" + data = {"id": "22222222-2222-2222-2222-222222222222", "name": {"value": "JFK1"}} + if client_type == "standard": + node = InfrahubNode(client=client, schema=location_schema, data=data) + else: + node = InfrahubNodeSync(client=client, schema=location_schema, data=data) + with pytest.raises(UninitializedError, match=r"Must call fetch"): + node.tags.add("11111111-1111-1111-1111-111111111111") + + @pytest.mark.parametrize("client_type", client_types) async def test_query_data_no_filters_property( clients: BothClients, location_schema: NodeSchemaAPI, client_type: str