From c13444182f99a55b6ea07f50a99e99f4e9b4cf68 Mon Sep 17 00:00:00 2001 From: Mikhail Yohman Date: Wed, 31 Dec 2025 13:06:27 -0700 Subject: [PATCH 1/6] Fix code for HFID casting of strings that aren't UUIDs so user doesn't have to provide HFID directly if single value HFID. --- infrahub_sdk/spec/object.py | 30 +++- tests/unit/sdk/spec/test_object.py | 247 +++++++++++++++++++++++++++++ 2 files changed, 275 insertions(+), 2 deletions(-) diff --git a/infrahub_sdk/spec/object.py b/infrahub_sdk/spec/object.py index 0df3f95c..3c52621a 100644 --- a/infrahub_sdk/spec/object.py +++ b/infrahub_sdk/spec/object.py @@ -7,6 +7,7 @@ from ..exceptions import ObjectValidationError, ValidationError from ..schema import GenericSchemaAPI, RelationshipKind, RelationshipSchema +from ..utils import is_valid_uuid from ..yaml import InfrahubFile, InfrahubFileKind from .models import InfrahubObjectParameters from .processors.factory import DataProcessorFactory @@ -33,6 +34,28 @@ def validate_list_of_objects(value: list[Any]) -> bool: return all(isinstance(item, dict) for item in value) +def normalize_hfid_reference(value: str | list[str]) -> list[str]: + """Normalize a reference value to HFID format. + + If the value is a string and not a valid UUID, wrap it in a list to treat it as a single-component HFID. + If the value is already a list, return it as-is. + If the value is a UUID string, return it as-is (will be treated as an ID). + """ + if isinstance(value, list): + return value + if is_valid_uuid(value): + return value # type: ignore[return-value] + return [value] + + +def normalize_hfid_references(values: list[str | list[str]]) -> list[str | list[str]]: + """Normalize a list of reference values to HFID format. + + Each string that is not a valid UUID will be wrapped in a list to treat it as a single-component HFID. + """ + return [normalize_hfid_reference(v) for v in values] + + class RelationshipDataFormat(str, Enum): UNKNOWN = "unknown" @@ -445,9 +468,12 @@ async def create_node( # - if the relationship is bidirectional and is not mandatory on the other side, then we need should create the related object First # - if the relationship is not bidirectional, then we need to create the related object First if rel_info.is_reference and isinstance(value, list): - clean_data[key] = value + # Normalize string HFIDs to list format: "name" -> ["name"] + # UUIDs are left as-is since they are treated as IDs + clean_data[key] = normalize_hfid_references(value) elif rel_info.format == RelationshipDataFormat.ONE_REF and isinstance(value, str): - clean_data[key] = [value] + # Normalize string to HFID format if not a UUID + clean_data[key] = [normalize_hfid_reference(value)] elif not rel_info.is_reference and rel_info.is_bidirectional and rel_info.is_mandatory: remaining_rels.append(key) elif not rel_info.is_reference and not rel_info.is_mandatory: diff --git a/tests/unit/sdk/spec/test_object.py b/tests/unit/sdk/spec/test_object.py index 1af02ac3..c3f27cbd 100644 --- a/tests/unit/sdk/spec/test_object.py +++ b/tests/unit/sdk/spec/test_object.py @@ -263,3 +263,250 @@ async def test_parameters_non_dict(client_with_schema_01: InfrahubClient, locati obj = ObjectFile(location="some/path", content=location_with_non_dict_parameters) with pytest.raises(ValidationError): await obj.validate_format(client=client_with_schema_01) + + +class TestHfidNormalizationInObjectLoading: + """Tests to verify HFID normalization works correctly through the object loading code path.""" + + @pytest.fixture + def location_with_cardinality_one_string_hfid(self, root_location: dict) -> dict: + """Location with a cardinality-one relationship using string HFID.""" + data = [{"name": "Mexico", "type": "Country", "primary_tag": "Important"}] + location = root_location.copy() + location["spec"]["data"] = data + return location + + @pytest.fixture + def location_with_cardinality_one_list_hfid(self, root_location: dict) -> dict: + """Location with a cardinality-one relationship using list HFID.""" + data = [{"name": "Mexico", "type": "Country", "primary_tag": ["Important"]}] + location = root_location.copy() + location["spec"]["data"] = data + return location + + @pytest.fixture + def location_with_cardinality_one_uuid(self, root_location: dict) -> dict: + """Location with a cardinality-one relationship using UUID.""" + data = [ + {"name": "Mexico", "type": "Country", "primary_tag": "550e8400-e29b-41d4-a716-446655440000"} + ] + location = root_location.copy() + location["spec"]["data"] = data + return location + + @pytest.fixture + def location_with_cardinality_many_string_hfids(self, root_location: dict) -> dict: + """Location with a cardinality-many relationship using string HFIDs.""" + data = [{"name": "Mexico", "type": "Country", "tags": ["Important", "Active"]}] + location = root_location.copy() + location["spec"]["data"] = data + return location + + @pytest.fixture + def location_with_cardinality_many_list_hfids(self, root_location: dict) -> dict: + """Location with a cardinality-many relationship using list HFIDs.""" + data = [{"name": "Mexico", "type": "Country", "tags": [["Important"], ["Active"]]}] + location = root_location.copy() + location["spec"]["data"] = data + return location + + @pytest.fixture + def location_with_cardinality_many_mixed_hfids(self, root_location: dict) -> dict: + """Location with a cardinality-many relationship using mixed string and list HFIDs.""" + data = [{"name": "Mexico", "type": "Country", "tags": ["Important", ["namespace", "name"]]}] + location = root_location.copy() + location["spec"]["data"] = data + return location + + @pytest.fixture + def location_with_cardinality_many_uuids(self, root_location: dict) -> dict: + """Location with a cardinality-many relationship using UUIDs.""" + data = [ + { + "name": "Mexico", + "type": "Country", + "tags": ["550e8400-e29b-41d4-a716-446655440000", "6ba7b810-9dad-11d1-80b4-00c04fd430c8"], + } + ] + location = root_location.copy() + location["spec"]["data"] = data + return location + + async def test_cardinality_one_string_hfid_normalized( + self, client_with_schema_01: InfrahubClient, location_with_cardinality_one_string_hfid: dict + ) -> None: + """String HFID for cardinality-one should be wrapped in a list.""" + obj = ObjectFile(location="some/path", content=location_with_cardinality_one_string_hfid) + await obj.validate_format(client=client_with_schema_01) + + # Track calls to client.create + create_calls = [] + original_create = client_with_schema_01.create + + async def mock_create(kind, branch=None, data=None, **kwargs): + create_calls.append({"kind": kind, "data": data}) + # Return a mock node that has the required methods + node = await original_create(kind=kind, branch=branch, data=data, **kwargs) + return node + + client_with_schema_01.create = mock_create + + # Mock the save method to avoid API calls + from unittest.mock import AsyncMock, patch + + with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): + await obj.process(client=client_with_schema_01) + + # Verify the data passed to create has the normalized HFID + assert len(create_calls) == 1 + assert create_calls[0]["data"]["primary_tag"] == [["Important"]] + + async def test_cardinality_one_list_hfid_unchanged( + self, client_with_schema_01: InfrahubClient, location_with_cardinality_one_list_hfid: dict + ) -> None: + """List HFID for cardinality-one should remain unchanged.""" + obj = ObjectFile(location="some/path", content=location_with_cardinality_one_list_hfid) + await obj.validate_format(client=client_with_schema_01) + + create_calls = [] + + async def mock_create(kind, branch=None, data=None, **kwargs): + create_calls.append({"kind": kind, "data": data}) + original_create = client_with_schema_01.__class__.create + return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) + + client_with_schema_01.create = mock_create + + from unittest.mock import AsyncMock, patch + + with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): + await obj.process(client=client_with_schema_01) + + assert len(create_calls) == 1 + assert create_calls[0]["data"]["primary_tag"] == [["Important"]] + + async def test_cardinality_one_uuid_unchanged( + self, client_with_schema_01: InfrahubClient, location_with_cardinality_one_uuid: dict + ) -> None: + """UUID for cardinality-one should remain as a string (not wrapped in list).""" + obj = ObjectFile(location="some/path", content=location_with_cardinality_one_uuid) + await obj.validate_format(client=client_with_schema_01) + + create_calls = [] + + async def mock_create(kind, branch=None, data=None, **kwargs): + create_calls.append({"kind": kind, "data": data}) + original_create = client_with_schema_01.__class__.create + return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) + + client_with_schema_01.create = mock_create + + from unittest.mock import AsyncMock, patch + + with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): + await obj.process(client=client_with_schema_01) + + assert len(create_calls) == 1 + # UUID should be passed as-is (not wrapped in a list) + assert create_calls[0]["data"]["primary_tag"] == ["550e8400-e29b-41d4-a716-446655440000"] + + async def test_cardinality_many_string_hfids_normalized( + self, client_with_schema_01: InfrahubClient, location_with_cardinality_many_string_hfids: dict + ) -> None: + """String HFIDs for cardinality-many should each be wrapped in a list.""" + obj = ObjectFile(location="some/path", content=location_with_cardinality_many_string_hfids) + await obj.validate_format(client=client_with_schema_01) + + create_calls = [] + + async def mock_create(kind, branch=None, data=None, **kwargs): + create_calls.append({"kind": kind, "data": data}) + original_create = client_with_schema_01.__class__.create + return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) + + client_with_schema_01.create = mock_create + + from unittest.mock import AsyncMock, patch + + with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): + await obj.process(client=client_with_schema_01) + + assert len(create_calls) == 1 + assert create_calls[0]["data"]["tags"] == [["Important"], ["Active"]] + + async def test_cardinality_many_list_hfids_unchanged( + self, client_with_schema_01: InfrahubClient, location_with_cardinality_many_list_hfids: dict + ) -> None: + """List HFIDs for cardinality-many should remain unchanged.""" + obj = ObjectFile(location="some/path", content=location_with_cardinality_many_list_hfids) + await obj.validate_format(client=client_with_schema_01) + + create_calls = [] + + async def mock_create(kind, branch=None, data=None, **kwargs): + create_calls.append({"kind": kind, "data": data}) + original_create = client_with_schema_01.__class__.create + return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) + + client_with_schema_01.create = mock_create + + from unittest.mock import AsyncMock, patch + + with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): + await obj.process(client=client_with_schema_01) + + assert len(create_calls) == 1 + assert create_calls[0]["data"]["tags"] == [["Important"], ["Active"]] + + async def test_cardinality_many_mixed_hfids_normalized( + self, client_with_schema_01: InfrahubClient, location_with_cardinality_many_mixed_hfids: dict + ) -> None: + """Mixed string and list HFIDs for cardinality-many should be normalized correctly.""" + obj = ObjectFile(location="some/path", content=location_with_cardinality_many_mixed_hfids) + await obj.validate_format(client=client_with_schema_01) + + create_calls = [] + + async def mock_create(kind, branch=None, data=None, **kwargs): + create_calls.append({"kind": kind, "data": data}) + original_create = client_with_schema_01.__class__.create + return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) + + client_with_schema_01.create = mock_create + + from unittest.mock import AsyncMock, patch + + with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): + await obj.process(client=client_with_schema_01) + + assert len(create_calls) == 1 + # "Important" should be wrapped, ["namespace", "name"] should remain unchanged + assert create_calls[0]["data"]["tags"] == [["Important"], ["namespace", "name"]] + + async def test_cardinality_many_uuids_unchanged( + self, client_with_schema_01: InfrahubClient, location_with_cardinality_many_uuids: dict + ) -> None: + """UUIDs for cardinality-many should remain as strings (not wrapped).""" + obj = ObjectFile(location="some/path", content=location_with_cardinality_many_uuids) + await obj.validate_format(client=client_with_schema_01) + + create_calls = [] + + async def mock_create(kind, branch=None, data=None, **kwargs): + create_calls.append({"kind": kind, "data": data}) + original_create = client_with_schema_01.__class__.create + return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) + + client_with_schema_01.create = mock_create + + from unittest.mock import AsyncMock, patch + + with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): + await obj.process(client=client_with_schema_01) + + assert len(create_calls) == 1 + # UUIDs should remain as-is + assert create_calls[0]["data"]["tags"] == [ + "550e8400-e29b-41d4-a716-446655440000", + "6ba7b810-9dad-11d1-80b4-00c04fd430c8", + ] From b409d99b1c41943d7620fd7f5e31fc9b4e459a98 Mon Sep 17 00:00:00 2001 From: Mikhail Yohman Date: Wed, 31 Dec 2025 13:18:58 -0700 Subject: [PATCH 2/6] Fix linting issues. --- tests/unit/sdk/spec/test_object.py | 90 ++++++++++++++++++------------ 1 file changed, 54 insertions(+), 36 deletions(-) diff --git a/tests/unit/sdk/spec/test_object.py b/tests/unit/sdk/spec/test_object.py index c3f27cbd..728b9442 100644 --- a/tests/unit/sdk/spec/test_object.py +++ b/tests/unit/sdk/spec/test_object.py @@ -1,6 +1,7 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any +from unittest.mock import AsyncMock, patch import pytest @@ -9,6 +10,7 @@ if TYPE_CHECKING: from infrahub_sdk.client import InfrahubClient + from infrahub_sdk.node import InfrahubNode @pytest.fixture @@ -287,9 +289,7 @@ def location_with_cardinality_one_list_hfid(self, root_location: dict) -> dict: @pytest.fixture def location_with_cardinality_one_uuid(self, root_location: dict) -> dict: """Location with a cardinality-one relationship using UUID.""" - data = [ - {"name": "Mexico", "type": "Country", "primary_tag": "550e8400-e29b-41d4-a716-446655440000"} - ] + data = [{"name": "Mexico", "type": "Country", "primary_tag": "550e8400-e29b-41d4-a716-446655440000"}] location = root_location.copy() location["spec"]["data"] = data return location @@ -340,20 +340,20 @@ async def test_cardinality_one_string_hfid_normalized( await obj.validate_format(client=client_with_schema_01) # Track calls to client.create - create_calls = [] + create_calls: list[dict[str, Any]] = [] original_create = client_with_schema_01.create - async def mock_create(kind, branch=None, data=None, **kwargs): + async def mock_create( + kind: str, + branch: str | None = None, + data: dict | None = None, + **kwargs: Any, # noqa: ANN401 + ) -> InfrahubNode: create_calls.append({"kind": kind, "data": data}) - # Return a mock node that has the required methods - node = await original_create(kind=kind, branch=branch, data=data, **kwargs) - return node + return await original_create(kind=kind, branch=branch, data=data, **kwargs) client_with_schema_01.create = mock_create - # Mock the save method to avoid API calls - from unittest.mock import AsyncMock, patch - with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): await obj.process(client=client_with_schema_01) @@ -368,17 +368,20 @@ async def test_cardinality_one_list_hfid_unchanged( obj = ObjectFile(location="some/path", content=location_with_cardinality_one_list_hfid) await obj.validate_format(client=client_with_schema_01) - create_calls = [] + create_calls: list[dict[str, Any]] = [] - async def mock_create(kind, branch=None, data=None, **kwargs): + async def mock_create( + kind: str, + branch: str | None = None, + data: dict | None = None, + **kwargs: Any, # noqa: ANN401 + ) -> InfrahubNode: create_calls.append({"kind": kind, "data": data}) original_create = client_with_schema_01.__class__.create return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) client_with_schema_01.create = mock_create - from unittest.mock import AsyncMock, patch - with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): await obj.process(client=client_with_schema_01) @@ -392,17 +395,20 @@ async def test_cardinality_one_uuid_unchanged( obj = ObjectFile(location="some/path", content=location_with_cardinality_one_uuid) await obj.validate_format(client=client_with_schema_01) - create_calls = [] + create_calls: list[dict[str, Any]] = [] - async def mock_create(kind, branch=None, data=None, **kwargs): + async def mock_create( + kind: str, + branch: str | None = None, + data: dict | None = None, + **kwargs: Any, # noqa: ANN401 + ) -> InfrahubNode: create_calls.append({"kind": kind, "data": data}) original_create = client_with_schema_01.__class__.create return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) client_with_schema_01.create = mock_create - from unittest.mock import AsyncMock, patch - with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): await obj.process(client=client_with_schema_01) @@ -417,17 +423,20 @@ async def test_cardinality_many_string_hfids_normalized( obj = ObjectFile(location="some/path", content=location_with_cardinality_many_string_hfids) await obj.validate_format(client=client_with_schema_01) - create_calls = [] + create_calls: list[dict[str, Any]] = [] - async def mock_create(kind, branch=None, data=None, **kwargs): + async def mock_create( + kind: str, + branch: str | None = None, + data: dict | None = None, + **kwargs: Any, # noqa: ANN401 + ) -> InfrahubNode: create_calls.append({"kind": kind, "data": data}) original_create = client_with_schema_01.__class__.create return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) client_with_schema_01.create = mock_create - from unittest.mock import AsyncMock, patch - with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): await obj.process(client=client_with_schema_01) @@ -441,17 +450,20 @@ async def test_cardinality_many_list_hfids_unchanged( obj = ObjectFile(location="some/path", content=location_with_cardinality_many_list_hfids) await obj.validate_format(client=client_with_schema_01) - create_calls = [] + create_calls: list[dict[str, Any]] = [] - async def mock_create(kind, branch=None, data=None, **kwargs): + async def mock_create( + kind: str, + branch: str | None = None, + data: dict | None = None, + **kwargs: Any, # noqa: ANN401 + ) -> InfrahubNode: create_calls.append({"kind": kind, "data": data}) original_create = client_with_schema_01.__class__.create return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) client_with_schema_01.create = mock_create - from unittest.mock import AsyncMock, patch - with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): await obj.process(client=client_with_schema_01) @@ -465,17 +477,20 @@ async def test_cardinality_many_mixed_hfids_normalized( obj = ObjectFile(location="some/path", content=location_with_cardinality_many_mixed_hfids) await obj.validate_format(client=client_with_schema_01) - create_calls = [] + create_calls: list[dict[str, Any]] = [] - async def mock_create(kind, branch=None, data=None, **kwargs): + async def mock_create( + kind: str, + branch: str | None = None, + data: dict | None = None, + **kwargs: Any, # noqa: ANN401 + ) -> InfrahubNode: create_calls.append({"kind": kind, "data": data}) original_create = client_with_schema_01.__class__.create return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) client_with_schema_01.create = mock_create - from unittest.mock import AsyncMock, patch - with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): await obj.process(client=client_with_schema_01) @@ -490,17 +505,20 @@ async def test_cardinality_many_uuids_unchanged( obj = ObjectFile(location="some/path", content=location_with_cardinality_many_uuids) await obj.validate_format(client=client_with_schema_01) - create_calls = [] + create_calls: list[dict[str, Any]] = [] - async def mock_create(kind, branch=None, data=None, **kwargs): + async def mock_create( + kind: str, + branch: str | None = None, + data: dict | None = None, + **kwargs: Any, # noqa: ANN401 + ) -> InfrahubNode: create_calls.append({"kind": kind, "data": data}) original_create = client_with_schema_01.__class__.create return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) client_with_schema_01.create = mock_create - from unittest.mock import AsyncMock, patch - with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): await obj.process(client=client_with_schema_01) From a5852e958d5606a0ce7ea9b006702061bafb1564 Mon Sep 17 00:00:00 2001 From: Mikhail Yohman Date: Wed, 31 Dec 2025 13:45:13 -0700 Subject: [PATCH 3/6] Fix card one from being double wrapped list. --- infrahub_sdk/spec/object.py | 4 +++- tests/unit/sdk/spec/test_object.py | 10 ++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/infrahub_sdk/spec/object.py b/infrahub_sdk/spec/object.py index 3c52621a..a423c89a 100644 --- a/infrahub_sdk/spec/object.py +++ b/infrahub_sdk/spec/object.py @@ -473,7 +473,9 @@ async def create_node( clean_data[key] = normalize_hfid_references(value) elif rel_info.format == RelationshipDataFormat.ONE_REF and isinstance(value, str): # Normalize string to HFID format if not a UUID - clean_data[key] = [normalize_hfid_reference(value)] + # For cardinality-one, we pass the normalized value directly (not wrapped in a list) + # The RelatedNode class will interpret a list as {"hfid": list} and a string as {"id": string} + clean_data[key] = normalize_hfid_reference(value) elif not rel_info.is_reference and rel_info.is_bidirectional and rel_info.is_mandatory: remaining_rels.append(key) elif not rel_info.is_reference and not rel_info.is_mandatory: diff --git a/tests/unit/sdk/spec/test_object.py b/tests/unit/sdk/spec/test_object.py index 728b9442..6c220020 100644 --- a/tests/unit/sdk/spec/test_object.py +++ b/tests/unit/sdk/spec/test_object.py @@ -358,8 +358,9 @@ async def mock_create( await obj.process(client=client_with_schema_01) # Verify the data passed to create has the normalized HFID + # For cardinality-one, string "Important" becomes ["Important"] (list HFID format) assert len(create_calls) == 1 - assert create_calls[0]["data"]["primary_tag"] == [["Important"]] + assert create_calls[0]["data"]["primary_tag"] == ["Important"] async def test_cardinality_one_list_hfid_unchanged( self, client_with_schema_01: InfrahubClient, location_with_cardinality_one_list_hfid: dict @@ -385,8 +386,9 @@ async def mock_create( with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): await obj.process(client=client_with_schema_01) + # List HFID ["Important"] remains unchanged assert len(create_calls) == 1 - assert create_calls[0]["data"]["primary_tag"] == [["Important"]] + assert create_calls[0]["data"]["primary_tag"] == ["Important"] async def test_cardinality_one_uuid_unchanged( self, client_with_schema_01: InfrahubClient, location_with_cardinality_one_uuid: dict @@ -412,9 +414,9 @@ async def mock_create( with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): await obj.process(client=client_with_schema_01) + # UUID should be passed as-is (string, not wrapped in a list) assert len(create_calls) == 1 - # UUID should be passed as-is (not wrapped in a list) - assert create_calls[0]["data"]["primary_tag"] == ["550e8400-e29b-41d4-a716-446655440000"] + assert create_calls[0]["data"]["primary_tag"] == "550e8400-e29b-41d4-a716-446655440000" async def test_cardinality_many_string_hfids_normalized( self, client_with_schema_01: InfrahubClient, location_with_cardinality_many_string_hfids: dict From 68b8112730449bca8fd18062dba530cbf87a31c9 Mon Sep 17 00:00:00 2001 From: Mikhail Yohman Date: Wed, 31 Dec 2025 13:53:38 -0700 Subject: [PATCH 4/6] Update to not double wrap a list for a card one rel. --- infrahub_sdk/spec/object.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/infrahub_sdk/spec/object.py b/infrahub_sdk/spec/object.py index a423c89a..ed69311b 100644 --- a/infrahub_sdk/spec/object.py +++ b/infrahub_sdk/spec/object.py @@ -467,14 +467,15 @@ async def create_node( # - if the relationship is bidirectional and is mandatory on the other side, then we need to create this object First # - if the relationship is bidirectional and is not mandatory on the other side, then we need should create the related object First # - if the relationship is not bidirectional, then we need to create the related object First - if rel_info.is_reference and isinstance(value, list): - # Normalize string HFIDs to list format: "name" -> ["name"] + if rel_info.format == RelationshipDataFormat.MANY_REF and isinstance(value, list): + # Cardinality-many: normalize each string HFID to list format: "name" -> ["name"] # UUIDs are left as-is since they are treated as IDs clean_data[key] = normalize_hfid_references(value) - elif rel_info.format == RelationshipDataFormat.ONE_REF and isinstance(value, str): - # Normalize string to HFID format if not a UUID - # For cardinality-one, we pass the normalized value directly (not wrapped in a list) + elif rel_info.format == RelationshipDataFormat.ONE_REF: + # Cardinality-one: normalize string to HFID format if not a UUID + # For cardinality-one, we pass the normalized value directly # The RelatedNode class will interpret a list as {"hfid": list} and a string as {"id": string} + # Value can be either a string (e.g., "Jane Smith") or a list (e.g., ["Jane Smith"]) clean_data[key] = normalize_hfid_reference(value) elif not rel_info.is_reference and rel_info.is_bidirectional and rel_info.is_mandatory: remaining_rels.append(key) From fc9b258e274d60a3a0cd4896c71bc9c4e0bb5455 Mon Sep 17 00:00:00 2001 From: Mikhail Yohman Date: Wed, 31 Dec 2025 14:14:26 -0700 Subject: [PATCH 5/6] Fixed typing error. --- infrahub_sdk/spec/object.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/infrahub_sdk/spec/object.py b/infrahub_sdk/spec/object.py index ed69311b..83af5ff8 100644 --- a/infrahub_sdk/spec/object.py +++ b/infrahub_sdk/spec/object.py @@ -34,17 +34,21 @@ def validate_list_of_objects(value: list[Any]) -> bool: return all(isinstance(item, dict) for item in value) -def normalize_hfid_reference(value: str | list[str]) -> list[str]: +def normalize_hfid_reference(value: str | list[str]) -> str | list[str]: """Normalize a reference value to HFID format. - If the value is a string and not a valid UUID, wrap it in a list to treat it as a single-component HFID. - If the value is already a list, return it as-is. - If the value is a UUID string, return it as-is (will be treated as an ID). + Args: + value: Either a string (ID or single-component HFID) or a list of strings (multi-component HFID). + + Returns: + - If value is already a list: returns it unchanged as list[str] + - If value is a valid UUID string: returns it unchanged as str (will be treated as an ID) + - If value is a non-UUID string: wraps it in a list as list[str] (single-component HFID) """ if isinstance(value, list): return value if is_valid_uuid(value): - return value # type: ignore[return-value] + return value return [value] From adb5eb318458c92cfa99e05cb4d2b7d9370870d3 Mon Sep 17 00:00:00 2001 From: Mikhail Yohman Date: Mon, 5 Jan 2026 12:23:16 -0700 Subject: [PATCH 6/6] Sessived code comments. Fixturized testing. --- infrahub_sdk/spec/object.py | 5 +- tests/unit/sdk/spec/test_object.py | 336 ++++++++--------------------- 2 files changed, 88 insertions(+), 253 deletions(-) diff --git a/infrahub_sdk/spec/object.py b/infrahub_sdk/spec/object.py index 83af5ff8..548b5fc3 100644 --- a/infrahub_sdk/spec/object.py +++ b/infrahub_sdk/spec/object.py @@ -476,10 +476,7 @@ async def create_node( # UUIDs are left as-is since they are treated as IDs clean_data[key] = normalize_hfid_references(value) elif rel_info.format == RelationshipDataFormat.ONE_REF: - # Cardinality-one: normalize string to HFID format if not a UUID - # For cardinality-one, we pass the normalized value directly - # The RelatedNode class will interpret a list as {"hfid": list} and a string as {"id": string} - # Value can be either a string (e.g., "Jane Smith") or a list (e.g., ["Jane Smith"]) + # Cardinality-one: normalize string to HFID list format: "name" -> ["name"] or keep as string (UUID) clean_data[key] = normalize_hfid_reference(value) elif not rel_info.is_reference and rel_info.is_bidirectional and rel_info.is_mandatory: remaining_rels.append(key) diff --git a/tests/unit/sdk/spec/test_object.py b/tests/unit/sdk/spec/test_object.py index 6c220020..581b2572 100644 --- a/tests/unit/sdk/spec/test_object.py +++ b/tests/unit/sdk/spec/test_object.py @@ -1,5 +1,6 @@ from __future__ import annotations +from dataclasses import dataclass from typing import TYPE_CHECKING, Any from unittest.mock import AsyncMock, patch @@ -267,266 +268,103 @@ async def test_parameters_non_dict(client_with_schema_01: InfrahubClient, locati await obj.validate_format(client=client_with_schema_01) -class TestHfidNormalizationInObjectLoading: - """Tests to verify HFID normalization works correctly through the object loading code path.""" - - @pytest.fixture - def location_with_cardinality_one_string_hfid(self, root_location: dict) -> dict: - """Location with a cardinality-one relationship using string HFID.""" - data = [{"name": "Mexico", "type": "Country", "primary_tag": "Important"}] - location = root_location.copy() - location["spec"]["data"] = data - return location - - @pytest.fixture - def location_with_cardinality_one_list_hfid(self, root_location: dict) -> dict: - """Location with a cardinality-one relationship using list HFID.""" - data = [{"name": "Mexico", "type": "Country", "primary_tag": ["Important"]}] - location = root_location.copy() - location["spec"]["data"] = data - return location - - @pytest.fixture - def location_with_cardinality_one_uuid(self, root_location: dict) -> dict: - """Location with a cardinality-one relationship using UUID.""" - data = [{"name": "Mexico", "type": "Country", "primary_tag": "550e8400-e29b-41d4-a716-446655440000"}] - location = root_location.copy() - location["spec"]["data"] = data - return location - - @pytest.fixture - def location_with_cardinality_many_string_hfids(self, root_location: dict) -> dict: - """Location with a cardinality-many relationship using string HFIDs.""" - data = [{"name": "Mexico", "type": "Country", "tags": ["Important", "Active"]}] - location = root_location.copy() - location["spec"]["data"] = data - return location - - @pytest.fixture - def location_with_cardinality_many_list_hfids(self, root_location: dict) -> dict: - """Location with a cardinality-many relationship using list HFIDs.""" - data = [{"name": "Mexico", "type": "Country", "tags": [["Important"], ["Active"]]}] - location = root_location.copy() - location["spec"]["data"] = data - return location - - @pytest.fixture - def location_with_cardinality_many_mixed_hfids(self, root_location: dict) -> dict: - """Location with a cardinality-many relationship using mixed string and list HFIDs.""" - data = [{"name": "Mexico", "type": "Country", "tags": ["Important", ["namespace", "name"]]}] - location = root_location.copy() - location["spec"]["data"] = data - return location - - @pytest.fixture - def location_with_cardinality_many_uuids(self, root_location: dict) -> dict: - """Location with a cardinality-many relationship using UUIDs.""" - data = [ +@dataclass +class HfidLoadTestCase: + """Test case for HFID normalization in object loading.""" + + name: str + data: list[dict[str, Any]] + expected_primary_tag: str | list[str] | None + expected_tags: list[str] | list[list[str]] | None + + +HFID_NORMALIZATION_TEST_CASES = [ + HfidLoadTestCase( + name="cardinality_one_string_hfid_normalized", + data=[{"name": "Mexico", "type": "Country", "primary_tag": "Important"}], + expected_primary_tag=["Important"], + expected_tags=None, + ), + HfidLoadTestCase( + name="cardinality_one_list_hfid_unchanged", + data=[{"name": "Mexico", "type": "Country", "primary_tag": ["Important"]}], + expected_primary_tag=["Important"], + expected_tags=None, + ), + HfidLoadTestCase( + name="cardinality_one_uuid_unchanged", + data=[{"name": "Mexico", "type": "Country", "primary_tag": "550e8400-e29b-41d4-a716-446655440000"}], + expected_primary_tag="550e8400-e29b-41d4-a716-446655440000", + expected_tags=None, + ), + HfidLoadTestCase( + name="cardinality_many_string_hfids_normalized", + data=[{"name": "Mexico", "type": "Country", "tags": ["Important", "Active"]}], + expected_primary_tag=None, + expected_tags=[["Important"], ["Active"]], + ), + HfidLoadTestCase( + name="cardinality_many_list_hfids_unchanged", + data=[{"name": "Mexico", "type": "Country", "tags": [["Important"], ["Active"]]}], + expected_primary_tag=None, + expected_tags=[["Important"], ["Active"]], + ), + HfidLoadTestCase( + name="cardinality_many_mixed_hfids_normalized", + data=[{"name": "Mexico", "type": "Country", "tags": ["Important", ["namespace", "name"]]}], + expected_primary_tag=None, + expected_tags=[["Important"], ["namespace", "name"]], + ), + HfidLoadTestCase( + name="cardinality_many_uuids_unchanged", + data=[ { "name": "Mexico", "type": "Country", "tags": ["550e8400-e29b-41d4-a716-446655440000", "6ba7b810-9dad-11d1-80b4-00c04fd430c8"], } - ] - location = root_location.copy() - location["spec"]["data"] = data - return location - - async def test_cardinality_one_string_hfid_normalized( - self, client_with_schema_01: InfrahubClient, location_with_cardinality_one_string_hfid: dict - ) -> None: - """String HFID for cardinality-one should be wrapped in a list.""" - obj = ObjectFile(location="some/path", content=location_with_cardinality_one_string_hfid) - await obj.validate_format(client=client_with_schema_01) - - # Track calls to client.create - create_calls: list[dict[str, Any]] = [] - original_create = client_with_schema_01.create - - async def mock_create( - kind: str, - branch: str | None = None, - data: dict | None = None, - **kwargs: Any, # noqa: ANN401 - ) -> InfrahubNode: - create_calls.append({"kind": kind, "data": data}) - return await original_create(kind=kind, branch=branch, data=data, **kwargs) - - client_with_schema_01.create = mock_create - - with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): - await obj.process(client=client_with_schema_01) - - # Verify the data passed to create has the normalized HFID - # For cardinality-one, string "Important" becomes ["Important"] (list HFID format) - assert len(create_calls) == 1 - assert create_calls[0]["data"]["primary_tag"] == ["Important"] - - async def test_cardinality_one_list_hfid_unchanged( - self, client_with_schema_01: InfrahubClient, location_with_cardinality_one_list_hfid: dict - ) -> None: - """List HFID for cardinality-one should remain unchanged.""" - obj = ObjectFile(location="some/path", content=location_with_cardinality_one_list_hfid) - await obj.validate_format(client=client_with_schema_01) - - create_calls: list[dict[str, Any]] = [] - - async def mock_create( - kind: str, - branch: str | None = None, - data: dict | None = None, - **kwargs: Any, # noqa: ANN401 - ) -> InfrahubNode: - create_calls.append({"kind": kind, "data": data}) - original_create = client_with_schema_01.__class__.create - return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) - - client_with_schema_01.create = mock_create - - with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): - await obj.process(client=client_with_schema_01) - - # List HFID ["Important"] remains unchanged - assert len(create_calls) == 1 - assert create_calls[0]["data"]["primary_tag"] == ["Important"] - - async def test_cardinality_one_uuid_unchanged( - self, client_with_schema_01: InfrahubClient, location_with_cardinality_one_uuid: dict - ) -> None: - """UUID for cardinality-one should remain as a string (not wrapped in list).""" - obj = ObjectFile(location="some/path", content=location_with_cardinality_one_uuid) - await obj.validate_format(client=client_with_schema_01) - - create_calls: list[dict[str, Any]] = [] - - async def mock_create( - kind: str, - branch: str | None = None, - data: dict | None = None, - **kwargs: Any, # noqa: ANN401 - ) -> InfrahubNode: - create_calls.append({"kind": kind, "data": data}) - original_create = client_with_schema_01.__class__.create - return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) - - client_with_schema_01.create = mock_create - - with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): - await obj.process(client=client_with_schema_01) - - # UUID should be passed as-is (string, not wrapped in a list) - assert len(create_calls) == 1 - assert create_calls[0]["data"]["primary_tag"] == "550e8400-e29b-41d4-a716-446655440000" - - async def test_cardinality_many_string_hfids_normalized( - self, client_with_schema_01: InfrahubClient, location_with_cardinality_many_string_hfids: dict - ) -> None: - """String HFIDs for cardinality-many should each be wrapped in a list.""" - obj = ObjectFile(location="some/path", content=location_with_cardinality_many_string_hfids) - await obj.validate_format(client=client_with_schema_01) - - create_calls: list[dict[str, Any]] = [] - - async def mock_create( - kind: str, - branch: str | None = None, - data: dict | None = None, - **kwargs: Any, # noqa: ANN401 - ) -> InfrahubNode: - create_calls.append({"kind": kind, "data": data}) - original_create = client_with_schema_01.__class__.create - return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) - - client_with_schema_01.create = mock_create - - with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): - await obj.process(client=client_with_schema_01) - - assert len(create_calls) == 1 - assert create_calls[0]["data"]["tags"] == [["Important"], ["Active"]] - - async def test_cardinality_many_list_hfids_unchanged( - self, client_with_schema_01: InfrahubClient, location_with_cardinality_many_list_hfids: dict - ) -> None: - """List HFIDs for cardinality-many should remain unchanged.""" - obj = ObjectFile(location="some/path", content=location_with_cardinality_many_list_hfids) - await obj.validate_format(client=client_with_schema_01) - - create_calls: list[dict[str, Any]] = [] - - async def mock_create( - kind: str, - branch: str | None = None, - data: dict | None = None, - **kwargs: Any, # noqa: ANN401 - ) -> InfrahubNode: - create_calls.append({"kind": kind, "data": data}) - original_create = client_with_schema_01.__class__.create - return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) - - client_with_schema_01.create = mock_create - - with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): - await obj.process(client=client_with_schema_01) + ], + expected_primary_tag=None, + expected_tags=["550e8400-e29b-41d4-a716-446655440000", "6ba7b810-9dad-11d1-80b4-00c04fd430c8"], + ), +] - assert len(create_calls) == 1 - assert create_calls[0]["data"]["tags"] == [["Important"], ["Active"]] - async def test_cardinality_many_mixed_hfids_normalized( - self, client_with_schema_01: InfrahubClient, location_with_cardinality_many_mixed_hfids: dict - ) -> None: - """Mixed string and list HFIDs for cardinality-many should be normalized correctly.""" - obj = ObjectFile(location="some/path", content=location_with_cardinality_many_mixed_hfids) - await obj.validate_format(client=client_with_schema_01) +@pytest.mark.parametrize("test_case", HFID_NORMALIZATION_TEST_CASES, ids=lambda tc: tc.name) +async def test_hfid_normalization_in_object_loading( + client_with_schema_01: InfrahubClient, test_case: HfidLoadTestCase +) -> None: + """Test that HFIDs are normalized correctly based on cardinality and format.""" - create_calls: list[dict[str, Any]] = [] + root_location = {"apiVersion": "infrahub.app/v1", "kind": "Object", "spec": {"kind": "BuiltinLocation", "data": []}} + location = { + "apiVersion": root_location["apiVersion"], + "kind": root_location["kind"], + "spec": {"kind": root_location["spec"]["kind"], "data": test_case.data}, + } - async def mock_create( - kind: str, - branch: str | None = None, - data: dict | None = None, - **kwargs: Any, # noqa: ANN401 - ) -> InfrahubNode: - create_calls.append({"kind": kind, "data": data}) - original_create = client_with_schema_01.__class__.create - return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) + obj = ObjectFile(location="some/path", content=location) + await obj.validate_format(client=client_with_schema_01) - client_with_schema_01.create = mock_create + create_calls: list[dict[str, Any]] = [] - with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): - await obj.process(client=client_with_schema_01) + async def mock_create( + kind: str, + branch: str | None = None, + data: dict | None = None, + **kwargs: Any, # noqa: ANN401 + ) -> InfrahubNode: + create_calls.append({"kind": kind, "data": data}) + original_create = client_with_schema_01.__class__.create + return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) - assert len(create_calls) == 1 - # "Important" should be wrapped, ["namespace", "name"] should remain unchanged - assert create_calls[0]["data"]["tags"] == [["Important"], ["namespace", "name"]] + client_with_schema_01.create = mock_create - async def test_cardinality_many_uuids_unchanged( - self, client_with_schema_01: InfrahubClient, location_with_cardinality_many_uuids: dict - ) -> None: - """UUIDs for cardinality-many should remain as strings (not wrapped).""" - obj = ObjectFile(location="some/path", content=location_with_cardinality_many_uuids) - await obj.validate_format(client=client_with_schema_01) + with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): + await obj.process(client=client_with_schema_01) - create_calls: list[dict[str, Any]] = [] - - async def mock_create( - kind: str, - branch: str | None = None, - data: dict | None = None, - **kwargs: Any, # noqa: ANN401 - ) -> InfrahubNode: - create_calls.append({"kind": kind, "data": data}) - original_create = client_with_schema_01.__class__.create - return await original_create(client_with_schema_01, kind=kind, branch=branch, data=data, **kwargs) - - client_with_schema_01.create = mock_create - - with patch("infrahub_sdk.node.InfrahubNode.save", new_callable=AsyncMock): - await obj.process(client=client_with_schema_01) - - assert len(create_calls) == 1 - # UUIDs should remain as-is - assert create_calls[0]["data"]["tags"] == [ - "550e8400-e29b-41d4-a716-446655440000", - "6ba7b810-9dad-11d1-80b4-00c04fd430c8", - ] + assert len(create_calls) == 1 + if test_case.expected_primary_tag is not None: + assert create_calls[0]["data"]["primary_tag"] == test_case.expected_primary_tag + if test_case.expected_tags is not None: + assert create_calls[0]["data"]["tags"] == test_case.expected_tags