From 8953e43c7aa3e354ca02835e091c587a991ab8dc Mon Sep 17 00:00:00 2001 From: Vadim Laletin Date: Fri, 30 Aug 2024 08:38:13 +0200 Subject: [PATCH] Get rid of built-in pydantic dump for CRUD and for patch. Rename dump to dump_resource --- CHANGELOG.md | 12 ++++++++++++ fhirpy/base/client.py | 9 +++++++++ fhirpy/base/lib_async.py | 16 ++++++++-------- fhirpy/base/lib_sync.py | 16 ++++++++-------- fhirpy/base/resource.py | 27 --------------------------- fhirpy/base/resource_protocol.py | 4 ---- tests/test_lib_async.py | 28 +++++++++++++++++++++------- tests/test_lib_base.py | 28 ++++++++-------------------- tests/test_lib_sync.py | 26 +++++++++++++++++++------- tests/utils.py | 12 ++++++++++++ 10 files changed, 97 insertions(+), 81 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bf742d..73776de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,15 @@ +## 2.0.11 + +* Rename dump to dump_resource +* Get rid of built-in dumping for dict-like structure +* Get rid of built-in dumping for patch +* Some breaking changes here, but the functional was introduced recently +that's why the version is not increased according to semver + * Migration guide for pydantic-users: + * Pass `dump_resource=lambda d: d.model_dump()` + * Manually dump your models for patch + + ## 2.0.10 * Update serializer with recursive cleaning and removing null values diff --git a/fhirpy/base/client.py b/fhirpy/base/client.py index e64f31d..9b037b4 100644 --- a/fhirpy/base/client.py +++ b/fhirpy/base/client.py @@ -1,5 +1,6 @@ import warnings from abc import ABC, abstractmethod +from collections.abc import Callable from typing import Any, TypeVar, Union from yarl import URL @@ -14,6 +15,7 @@ class AbstractClient(ABC): url: str authorization: Union[str, None] extra_headers: Union[dict, None] + dump_resource: Callable[[Any], Any] # Deprecated @property # pragma: no cover @@ -30,10 +32,17 @@ def __init__( url: str, authorization: Union[str, None] = None, extra_headers: Union[dict, None] = None, + *, + dump_resource: Callable[[Any], dict] = lambda x: dict(x), ): + """ + dump kwarg is a function that is called for all CRUD operations. + It's needed for custom typing model like pydantic + """ self.url = url self.authorization = authorization self.extra_headers = extra_headers + self.dump_resource = dump_resource def __str__(self) -> str: return f"<{self.__class__.__name__} {self.url}>" diff --git a/fhirpy/base/lib_async.py b/fhirpy/base/lib_async.py index b0005dd..db29c6c 100644 --- a/fhirpy/base/lib_async.py +++ b/fhirpy/base/lib_async.py @@ -29,12 +29,12 @@ def __init__( authorization: Union[str, None] = None, extra_headers: Union[dict, None] = None, aiohttp_config: Union[dict, None] = None, - dump: Callable[[Any], Any] = lambda x: x, + *, + dump_resource: Callable[[Any], dict] = lambda x: dict(x), ): self.aiohttp_config = aiohttp_config or {} - self.dump = dump - super().__init__(url, authorization, extra_headers) + super().__init__(url, authorization, extra_headers, dump_resource=dump_resource) async def execute( self, @@ -111,7 +111,7 @@ async def save( # _as_dict is a private api used internally _as_dict: bool = False, ) -> Union[TResource, Any]: - data = serialize(self.dump(resource), drop_nulls_from_dicts=fields is None) + data = serialize(self.dump_resource(resource), drop_nulls_from_dicts=fields is None) if fields: if not resource.id: raise TypeError("Resource `id` is required for update operation") @@ -171,7 +171,7 @@ async def patch( response_data = await self._do_request( "patch", f"{resource_type}/{resource_id}", - data=serialize(self.dump(kwargs), drop_nulls_from_dicts=False), + data=serialize(kwargs, drop_nulls_from_dicts=False), ) if custom_resource_class: @@ -444,7 +444,7 @@ async def get_or_create(self, resource: TResource) -> tuple[TResource, bool]: response_data, status_code = await self.client._do_request( "post", self.resource_type, - serialize(self.client.dump(resource)), + serialize(self.client.dump_resource(resource)), self.params, returning_status=True, ) @@ -457,7 +457,7 @@ async def update(self, resource: TResource) -> tuple[TResource, bool]: response_data, status_code = await self.client._do_request( "put", self.resource_type, - serialize(self.client.dump(resource)), + serialize(self.client.dump_resource(resource)), self.params, returning_status=True, ) @@ -472,7 +472,7 @@ async def patch(self, _resource: Any = None, **kwargs) -> TResource: stacklevel=2, ) data = serialize( - self.client.dump(_resource if _resource is not None else kwargs), + self.client.dump_resource(_resource) if _resource is not None else kwargs, drop_nulls_from_dicts=False, ) response_data = await self.client._do_request( diff --git a/fhirpy/base/lib_sync.py b/fhirpy/base/lib_sync.py index d6d0f11..60787ed 100644 --- a/fhirpy/base/lib_sync.py +++ b/fhirpy/base/lib_sync.py @@ -29,12 +29,12 @@ def __init__( authorization: Union[str, None] = None, extra_headers: Union[dict, None] = None, requests_config: Union[dict, None] = None, - dump: Callable[[Any], Any] = lambda x: x, + *, + dump_resource: Callable[[Any], dict] = lambda x: dict(x), ): self.requests_config = requests_config or {} - self.dump = dump - super().__init__(url, authorization, extra_headers) + super().__init__(url, authorization, extra_headers, dump_resource=dump_resource) def execute( self, @@ -111,7 +111,7 @@ def save( # _as_dict is a private api used internally _as_dict: bool = False, ) -> Union[TResource, Any]: - data = serialize(self.dump(resource), drop_nulls_from_dicts=fields is None) + data = serialize(self.dump_resource(resource), drop_nulls_from_dicts=fields is None) if fields: if not resource.id: raise TypeError("Resource `id` is required for update operation") @@ -167,7 +167,7 @@ def patch( response_data = self._do_request( "patch", f"{resource_type}/{resource_id}", - data=serialize(self.dump(kwargs), drop_nulls_from_dicts=False), + data=serialize(kwargs, drop_nulls_from_dicts=False), ) if custom_resource_class: @@ -442,7 +442,7 @@ def get_or_create(self, resource: TResource) -> tuple[TResource, int]: response_data, status_code = self.client._do_request( "post", self.resource_type, - serialize(self.client.dump(resource)), + serialize(self.client.dump_resource(resource)), self.params, returning_status=True, ) @@ -455,7 +455,7 @@ def update(self, resource: TResource) -> tuple[TResource, int]: response_data, status_code = self.client._do_request( "put", self.resource_type, - serialize(self.client.dump(resource)), + serialize(self.client.dump_resource(resource)), self.params, returning_status=True, ) @@ -472,7 +472,7 @@ def patch(self, _resource: Any = None, **kwargs) -> TResource: ) data = serialize( - self.client.dump(_resource if _resource is not None else kwargs), + self.client.dump_resource(_resource) if _resource is not None else kwargs, drop_nulls_from_dicts=False, ) response_data = self.client._do_request("patch", self.resource_type, data, self.params) diff --git a/fhirpy/base/resource.py b/fhirpy/base/resource.py index 4afb27e..881864d 100644 --- a/fhirpy/base/resource.py +++ b/fhirpy/base/resource.py @@ -1,5 +1,4 @@ from abc import ABC, abstractmethod -from collections.abc import Iterable, Sequence from typing import Any, Generic, Union from fhirpy.base.client import TClient @@ -265,10 +264,6 @@ def convert_fn(item): if isinstance(item, BaseReference): return serialize(item), True - if _is_serializable_dict_like(item): - # Handle dict-serializable structures like pydantic Model - return dict(item), False - return item, False converted_values = convert_values(dict(resource), convert_fn) @@ -277,25 +272,3 @@ def convert_fn(item): converted_values = remove_nulls_from_dicts(converted_values) return clean_empty_values(converted_values) - - -def _is_serializable_dict_like(item): - """ - >>> _is_serializable_dict_like({}) - True - >>> _is_serializable_dict_like([]) - False - >>> _is_serializable_dict_like(()) - False - >>> _is_serializable_dict_like(set()) - False - >>> _is_serializable_dict_like("string") - False - >>> _is_serializable_dict_like(42) - False - >>> _is_serializable_dict_like(True) - False - >>> _is_serializable_dict_like(None) - False - """ - return isinstance(item, Iterable) and not isinstance(item, (Sequence, set)) diff --git a/fhirpy/base/resource_protocol.py b/fhirpy/base/resource_protocol.py index 7a747e6..876731b 100644 --- a/fhirpy/base/resource_protocol.py +++ b/fhirpy/base/resource_protocol.py @@ -1,4 +1,3 @@ -from collections.abc import Iterator from typing import Any, Protocol, TypeVar, Union, get_args, get_type_hints @@ -6,9 +5,6 @@ class ResourceProtocol(Protocol): resourceType: Any # noqa: N815 id: Union[str, None] - def __iter__(self) -> Iterator: - ... - TResource = TypeVar("TResource", bound=ResourceProtocol) TReference = TypeVar("TReference") diff --git a/tests/test_lib_async.py b/tests/test_lib_async.py index 5eab52b..f93b42d 100644 --- a/tests/test_lib_async.py +++ b/tests/test_lib_async.py @@ -13,6 +13,7 @@ from .config import FHIR_SERVER_AUTHORIZATION, FHIR_SERVER_URL from .types import HumanName, Identifier, Patient, Reference +from .utils import dump_resource class TestLibAsyncCase: @@ -33,7 +34,9 @@ async def _clear_db(self): @classmethod def setup_class(cls): - cls.client = AsyncFHIRClient(cls.URL, authorization=FHIR_SERVER_AUTHORIZATION) + cls.client = AsyncFHIRClient( + cls.URL, authorization=FHIR_SERVER_AUTHORIZATION, dump_resource=dump_resource + ) async def create_resource(self, resource_type, **kwargs): return await self.client.resource( @@ -204,7 +207,7 @@ async def test_client_patch_specifying_reference(self): patched_patient = await self.client.patch( f"{patient.resourceType}/{patient.id}", - identifier=new_identifier, + identifier=[x.model_dump(exclude_none=True) for x in new_identifier], managingOrganization=None, ) @@ -220,7 +223,9 @@ async def test_client_patch_specifying_resource_type_str_and_id(self): new_identifier = [*patient.identifier, Identifier(system="url", value="value")] patched_patient = await self.client.patch( - patient.resourceType, patient.id, identifier=new_identifier + patient.resourceType, + patient.id, + identifier=[x.model_dump(exclude_none=True) for x in new_identifier], ) assert isinstance(patched_patient, dict) @@ -231,7 +236,11 @@ async def test_client_patch_specifying_resource_type_type_and_id(self): patient = await self.create_patient_model() new_identifier = [*patient.identifier, Identifier(system="url", value="value")] - patched_patient = await self.client.patch(Patient, patient.id, identifier=new_identifier) + patched_patient = await self.client.patch( + Patient, + patient.id, + identifier=[x.model_dump(exclude_none=True) for x in new_identifier], + ) assert isinstance(patched_patient, Patient) assert len(patched_patient.identifier) == 2 # noqa: PLR2004 @@ -241,7 +250,9 @@ async def test_client_patch_specifying_resource(self): patient = await self.create_patient_model() new_identifier = [*patient.identifier, Identifier(system="url", value="value")] - patched_patient = await self.client.patch(patient, identifier=new_identifier) + patched_patient = await self.client.patch( + patient, identifier=[x.model_dump(exclude_none=True) for x in new_identifier] + ) assert isinstance(patched_patient, Patient) assert len(patched_patient.identifier) == 2 # noqa: PLR2004 @@ -1228,8 +1239,11 @@ async def test_typed_patch(self): .search(name=name) .patch( identifier=[ - Identifier(system="url", value="value"), - Identifier(**self.identifier[0]), + x.model_dump(exclude_none=True) + for x in [ + Identifier(system="url", value="value"), + Identifier(**self.identifier[0]), + ] ], ) ) diff --git a/tests/test_lib_base.py b/tests/test_lib_base.py index ddc7a02..0cb8174 100644 --- a/tests/test_lib_base.py +++ b/tests/test_lib_base.py @@ -3,14 +3,20 @@ import pytest from fhirpy import AsyncFHIRClient, SyncFHIRClient -from fhirpy.base.resource import serialize from fhirpy.base.utils import AttrDict, SearchList, parse_pagination_url, set_by_path from fhirpy.lib import BaseFHIRReference from .types import HumanName, Identifier, Patient +from .utils import dump_resource -@pytest.mark.parametrize("client", [SyncFHIRClient("mock"), AsyncFHIRClient("mock")]) +@pytest.mark.parametrize( + "client", + [ + SyncFHIRClient("mock", dump_resource=dump_resource), + AsyncFHIRClient("mock", dump_resource=dump_resource), + ], +) class TestLibBase: def test_to_reference_for_reference(self, client: Union[SyncFHIRClient, AsyncFHIRClient]): reference = client.reference("Patient", "p1") @@ -272,24 +278,6 @@ def test_pluggable_type_model_resource_instantiation( assert isinstance(patient.name[0], HumanName) assert patient.name[0].text == "Name" - def test_pluggable_type_model_serialize_with_dict_null_values( - self, client: Union[SyncFHIRClient, AsyncFHIRClient] - ): - patient = client.resource( - Patient, - **{ - "resourceType": "Patient", - "identifier": [{"system": "url", "value": "value"}], - "name": [{"text": "Name"}], - "managingOrganization": None, - }, - ) - assert serialize(patient) == { - "resourceType": "Patient", - "identifier": [{"system": "url", "value": "value"}], - "name": [{"text": "Name"}], - } - def test_resource_resource_type_setter(self, client: Union[SyncFHIRClient, AsyncFHIRClient]): patient = client.resource("Patient", id="p1") patient.resourceType = "Patient" diff --git a/tests/test_lib_sync.py b/tests/test_lib_sync.py index 098fa17..927d3b2 100644 --- a/tests/test_lib_sync.py +++ b/tests/test_lib_sync.py @@ -17,7 +17,7 @@ from .config import FHIR_SERVER_AUTHORIZATION, FHIR_SERVER_URL from .types import HumanName, Identifier, Patient, Reference -from .utils import MockRequestsResponse +from .utils import MockRequestsResponse, dump_resource class TestLibSyncCase: @@ -43,6 +43,7 @@ def setup_class(cls): cls.URL, authorization=FHIR_SERVER_AUTHORIZATION, extra_headers={"Access-Control-Allow-Origin": "*"}, + dump_resource=dump_resource, ) def create_resource(self, resource_type, **kwargs): @@ -193,7 +194,7 @@ def test_client_patch_specifying_reference(self): patched_patient = self.client.patch( f"{patient.resourceType}/{patient.id}", - identifier=new_identifier, + identifier=[x.model_dump(exclude_none=True) for x in new_identifier], managingOrganization=None, ) @@ -208,7 +209,9 @@ def test_client_patch_specifying_resource_type_str_and_id(self): new_identifier = [*patient.identifier, Identifier(system="url", value="value")] patched_patient = self.client.patch( - patient.resourceType, patient.id, identifier=new_identifier + patient.resourceType, + patient.id, + identifier=[x.model_dump(exclude_none=True) for x in new_identifier], ) assert isinstance(patched_patient, dict) @@ -218,7 +221,11 @@ def test_client_patch_specifying_resource_type_type_and_id(self): patient = self.create_patient_model() new_identifier = [*patient.identifier, Identifier(system="url", value="value")] - patched_patient = self.client.patch(Patient, patient.id, identifier=new_identifier) + patched_patient = self.client.patch( + Patient, + patient.id, + identifier=[x.model_dump(exclude_none=True) for x in new_identifier], + ) assert isinstance(patched_patient, Patient) assert len(patched_patient.identifier) == 2 # noqa: PLR2004 @@ -227,7 +234,9 @@ def test_client_patch_specifying_resource(self): patient = self.create_patient_model() new_identifier = [*patient.identifier, Identifier(system="url", value="value")] - patched_patient = self.client.patch(patient, identifier=new_identifier) + patched_patient = self.client.patch( + patient, identifier=[x.model_dump(exclude_none=True) for x in new_identifier] + ) assert isinstance(patched_patient, Patient) assert len(patched_patient.identifier) == 2 # noqa: PLR2004 @@ -1138,8 +1147,11 @@ def test_typed_patch(self): .search(name=name) .patch( identifier=[ - Identifier(system="url", value="value"), - Identifier(**self.identifier[0]), + x.model_dump(exclude_none=True) + for x in [ + Identifier(system="url", value="value"), + Identifier(**self.identifier[0]), + ] ], ) ) diff --git a/tests/utils.py b/tests/utils.py index 481b10a..e6a17f8 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,3 +1,8 @@ +from typing import Any + +from pydantic import BaseModel + + class MockAiohttpResponse: def __init__(self, text, status): self._text = text @@ -18,3 +23,10 @@ def __init__(self, text, status_code): # self.json_data = json_data self.status_code = status_code self.content = text + + +def dump_resource(d: Any) -> dict: + if isinstance(d, BaseModel): + return d.model_dump() + + return dict(d)