Skip to content

Commit

Permalink
Get rid of built-in pydantic dump for CRUD and for patch. Rename dump…
Browse files Browse the repository at this point in the history
… to dump_resource
  • Loading branch information
ruscoder committed Aug 30, 2024
1 parent d16a930 commit 8953e43
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 81 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
9 changes: 9 additions & 0 deletions fhirpy/base/client.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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}>"
Expand Down
16 changes: 8 additions & 8 deletions fhirpy/base/lib_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,
)
Expand All @@ -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,
)
Expand All @@ -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(
Expand Down
16 changes: 8 additions & 8 deletions fhirpy/base/lib_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,
)
Expand All @@ -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,
)
Expand All @@ -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)
Expand Down
27 changes: 0 additions & 27 deletions fhirpy/base/resource.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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))
4 changes: 0 additions & 4 deletions fhirpy/base/resource_protocol.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
from collections.abc import Iterator
from typing import Any, Protocol, TypeVar, Union, get_args, get_type_hints


class ResourceProtocol(Protocol):
resourceType: Any # noqa: N815
id: Union[str, None]

def __iter__(self) -> Iterator:
...


TResource = TypeVar("TResource", bound=ResourceProtocol)
TReference = TypeVar("TReference")
Expand Down
28 changes: 21 additions & 7 deletions tests/test_lib_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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(
Expand Down Expand Up @@ -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,
)

Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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]),
]
],
)
)
Expand Down
28 changes: 8 additions & 20 deletions tests/test_lib_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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"
Expand Down
Loading

0 comments on commit 8953e43

Please sign in to comment.