From a3be32e678df92c380211874da8304e00b800391 Mon Sep 17 00:00:00 2001 From: Vadim Laletin Date: Tue, 27 Aug 2024 15:16:44 +0200 Subject: [PATCH] Preserve null values for save with fields passed --- CHANGELOG.md | 4 +++ fhirpy/__init__.py | 2 +- fhirpy/base/lib_async.py | 10 ++---- fhirpy/base/lib_sync.py | 10 ++---- tests/test_lib_async.py | 73 ++++++++++++++++++++++------------------ tests/test_lib_sync.py | 69 ++++++++++++++++++++----------------- 6 files changed, 90 insertions(+), 78 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ffa2fe..12b6f21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 2.0.7 + +* Preserve null values for resource in save() with fields passed + ## 2.0.6 * Fix type inference for client.resource diff --git a/fhirpy/__init__.py b/fhirpy/__init__.py index 27de57b..0b9985f 100644 --- a/fhirpy/__init__.py +++ b/fhirpy/__init__.py @@ -1,7 +1,7 @@ from .lib import AsyncFHIRClient, SyncFHIRClient __title__ = "fhir-py" -__version__ = "2.0.6" +__version__ = "2.0.7" __author__ = "beda.software" __license__ = "None" __copyright__ = "Copyright 2024 beda.software" diff --git a/fhirpy/base/lib_async.py b/fhirpy/base/lib_async.py index 04cc19d..19bd06a 100644 --- a/fhirpy/base/lib_async.py +++ b/fhirpy/base/lib_async.py @@ -109,7 +109,7 @@ async def save( # _as_dict is a private api used internally _as_dict: bool = False, ) -> Union[TResource, Any]: - data = serialize(resource) + data = serialize(resource, drop_dict_null_values=fields is None) if fields: if not resource.id: raise TypeError("Resource `id` is required for update operation") @@ -287,13 +287,9 @@ async def update(self) -> TResource: # type: ignore async def patch(self, **kwargs) -> TResource: if not self.id: - raise TypeError("Resource `id` is required for delete operation") + raise TypeError("Resource `id` is required for patch operation") super(BaseResource, self).update(**kwargs) - response_data = await self.__client__.patch(self.reference, **kwargs) - - resource_type = self.resource_type - super(BaseResource, self).clear() - super(BaseResource, self).update(**self.__client__.resource(resource_type, **response_data)) + await self.save(fields=list(kwargs.keys())) return cast(TResource, self) diff --git a/fhirpy/base/lib_sync.py b/fhirpy/base/lib_sync.py index 33bece6..6db9980 100644 --- a/fhirpy/base/lib_sync.py +++ b/fhirpy/base/lib_sync.py @@ -109,7 +109,7 @@ def save( # _as_dict is a private api used internally _as_dict: bool = False, ) -> Union[TResource, Any]: - data = serialize(resource) + data = serialize(resource, drop_dict_null_values=fields is None) if fields: if not resource.id: raise TypeError("Resource `id` is required for update operation") @@ -281,13 +281,9 @@ def update(self) -> TResource: # type: ignore def patch(self, **kwargs) -> TResource: if not self.id: - raise TypeError("Resource `id` is required for delete operation") + raise TypeError("Resource `id` is required for patch operation") super(BaseResource, self).update(**kwargs) - response_data = self.__client__.patch(self.reference, **kwargs) - - resource_type = self.resource_type - super(BaseResource, self).clear() - super(BaseResource, self).update(**self.__client__.resource(resource_type, **response_data)) + self.save(fields=list(kwargs.keys())) return cast(TResource, self) diff --git a/tests/test_lib_async.py b/tests/test_lib_async.py index b3cf7af..5eab52b 100644 --- a/tests/test_lib_async.py +++ b/tests/test_lib_async.py @@ -135,20 +135,26 @@ async def test_client_save_existing(self): @pytest.mark.asyncio() async def test_client_save_partial_update(self): - patient = await self.create_patient_model() + patient = await self.create_patient_model( + managingOrganization=Reference(reference="urn:organization") + ) patient.identifier = [ *patient.identifier, Identifier(system="url", value="value"), ] patient.name[0].text = "New patient" + patient.managingOrganization = None - updated_patient = await self.client.save(patient, fields=["identifier"]) + updated_patient = await self.client.save( + patient, fields=["identifier", "managingOrganization"] + ) assert isinstance(updated_patient, Patient) assert updated_patient.id == patient.id assert len(updated_patient.identifier) == 2 # noqa: PLR2004 assert updated_patient.name[0].text == "My patient" + assert updated_patient.managingOrganization is None @pytest.mark.asyncio() async def test_client_save_partial_update_fails_without_id(self): @@ -408,7 +414,7 @@ async def test_conditional_operations__fail_on_multiple_matches(self): ) @pytest.mark.asyncio() - async def test_update_with_params__no_match(self): + async def test_conditional_update__no_match(self): patient = await self.create_resource("Patient", id="patient", active=True) patient_to_update = self.client.resource( @@ -427,7 +433,7 @@ async def test_update_with_params__no_match(self): assert created is True @pytest.mark.asyncio() - async def test_update_with_params__one_match(self): + async def test_conditional_update__one_match(self): patient = await self.create_resource("Patient", id="patient", active=True) patient_to_update = self.client.resource( @@ -450,7 +456,7 @@ async def test_update_with_params__one_match(self): assert patient.get("active") is None @pytest.mark.asyncio() - async def test_patch_with_params__no_match(self): + async def test_conditional_patch__no_match(self): patient_to_patch = self.client.resource( "Patient", identifier=[{"system": "http://example.com/env", "value": "other"}, self.identifier[0]], @@ -462,7 +468,7 @@ async def test_patch_with_params__no_match(self): ) @pytest.mark.asyncio() - async def test_patch_with_params__one_match(self): + async def test_conditional_patch__one_match(self): patient = await self.create_resource( "Patient", id="patient", @@ -494,7 +500,7 @@ async def test_patch_with_params__one_match(self): assert patient.get("managingOrganization") is None @pytest.mark.asyncio() - async def test_patch_with_params__one_match_deprecated(self): + async def test_conditional_patch__one_match_deprecated(self): patient = await self.create_resource("Patient", id="patient", active=True) patient_to_patch = self.client.resource( @@ -569,7 +575,7 @@ async def test_delete_without_id_failed(self): await patient.delete() @pytest.mark.asyncio() - async def test_delete_with_params__no_match(self): + async def test_conditional_delete__no_match(self): await self.create_resource("Patient", id="patient") _, status_code = await self.client.resources("Patient").search(identifier="other").delete() @@ -578,7 +584,7 @@ async def test_delete_with_params__no_match(self): assert status_code == 204 # noqa: PLR2004 @pytest.mark.asyncio() - async def test_delete_with_params__one_match(self): + async def test_conditional_delete__one_match(self): patient = self.client.resource( "Patient", id="patient", @@ -595,7 +601,7 @@ async def test_delete_with_params__one_match(self): assert status_code == 200 # noqa: PLR2004 @pytest.mark.asyncio() - async def test_delete_with_params__multiple_matches(self): + async def test_conditional_delete__multiple_matches(self): await self.create_resource("Patient", id="patient-1") await self.create_resource("Patient", id="patient-2") @@ -859,18 +865,42 @@ async def test_save_fields(self): active=False, birthDate="1998-01-01", name=[{"text": "Abc"}], + managingOrganization={"reference": "urn:organization"}, ) patient["gender"] = "male" patient["birthDate"] = "1998-02-02" patient["active"] = True patient["name"] = [{"text": "Bcd"}] - await patient.save(fields=["gender", "birthDate"]) + patient["managingOrganization"] = None + await patient.save(fields=["gender", "birthDate", "managingOrganization"]) patient_refreshed = await patient.to_reference().to_resource() assert patient_refreshed["gender"] == patient["gender"] assert patient_refreshed["birthDate"] == patient["birthDate"] assert patient_refreshed["active"] is False assert patient_refreshed["name"] == [{"text": "Abc"}] + assert patient_refreshed.get("managingOrganization") is None + + @pytest.mark.asyncio() + async def test_update_patch_without_id(self): + patient = self.client.resource( + "Patient", identifier=self.identifier, name=[{"text": "J London"}] + ) + new_name = [ + { + "text": "Jack London", + "family": "London", + "given": ["Jack"], + } + ] + with pytest.raises(TypeError): + await patient.update() + with pytest.raises(TypeError): + await patient.patch(active=True, name=new_name) + patient["name"] = new_name + with pytest.raises(TypeError): + await patient.save(fields=["name"]) + await patient.save() @pytest.mark.asyncio() async def test_update(self): @@ -918,27 +948,6 @@ async def test_patch(self): assert patient_instance_1_refreshed["name"] == new_name assert patient_instance_1_refreshed.get("managingOrganization") is None - @pytest.mark.asyncio() - async def test_update_without_id(self): - patient = self.client.resource( - "Patient", identifier=self.identifier, name=[{"text": "J London"}] - ) - new_name = [ - { - "text": "Jack London", - "family": "London", - "given": ["Jack"], - } - ] - with pytest.raises(TypeError): - await patient.update() - with pytest.raises(TypeError): - await patient.patch(active=True, name=new_name) - patient["name"] = new_name - with pytest.raises(TypeError): - await patient.save(fields=["name"]) - await patient.save() - @pytest.mark.asyncio() async def test_refresh(self): patient_id = "refresh-patient-id" diff --git a/tests/test_lib_sync.py b/tests/test_lib_sync.py index 10dedb4..098fa17 100644 --- a/tests/test_lib_sync.py +++ b/tests/test_lib_sync.py @@ -132,20 +132,24 @@ def test_client_save_existing(self): assert len(updated_patient.identifier) == 2 # noqa: PLR2004 def test_client_save_partial_update(self): - patient = self.create_patient_model() + patient = self.create_patient_model( + managingOrganization=Reference(reference="urn:organization") + ) patient.identifier = [ *patient.identifier, Identifier(system="url", value="value"), ] patient.name[0].text = "New patient" + patient.managingOrganization = None - updated_patient = self.client.save(patient, fields=["identifier"]) + updated_patient = self.client.save(patient, fields=["identifier", "managingOrganization"]) assert isinstance(updated_patient, Patient) assert updated_patient.id == patient.id assert len(updated_patient.identifier) == 2 # noqa: PLR2004 assert updated_patient.name[0].text == "My patient" + assert updated_patient.managingOrganization is None def test_client_save_partial_update_fails_without_id(self): patient = self.create_patient_model() @@ -372,7 +376,7 @@ def test_conditional_operations__fail_on_multiple_matches(self): with pytest.raises(MultipleResourcesFound): self.client.resources("Patient").search(identifier="fhirpy").patch(patient_to_save) - def test_update_with_params__no_match(self): + def test_conditional_update__no_match(self): patient = self.create_resource("Patient", id="patient", active=True) patient_to_update = self.client.resource( @@ -390,7 +394,7 @@ def test_update_with_params__no_match(self): assert new_patient.active is False assert created is True - def test_update_with_params__one_match(self): + def test_conditional_update__one_match(self): patient = self.create_resource("Patient", id="patient", active=True) patient_to_update = self.client.resource( @@ -412,7 +416,7 @@ def test_update_with_params__one_match(self): ) assert patient.get("active") is None - def test_patch_with_params__no_match(self): + def test_conditional_patch__no_match(self): patient_to_patch = self.client.resource( "Patient", identifier=[{"system": "http://example.com/env", "value": "other"}, self.identifier[0]], @@ -421,7 +425,7 @@ def test_patch_with_params__no_match(self): with pytest.raises(ResourceNotFound): self.client.resources("Patient").search(identifier="other").patch(patient_to_patch) - def test_patch_with_params__one_match(self): + def test_conditional_patch__one_match(self): patient = self.create_resource( "Patient", id="patient", @@ -452,7 +456,7 @@ def test_patch_with_params__one_match(self): assert patient.active is True assert patient.get("managingOrganization") is None - def test_patch_with_params__one_match_deprecated(self): + def test_conditional_patch__one_match_deprecated(self): patient = self.create_resource("Patient", id="patient", active=True) patient_to_patch = self.client.resource( @@ -520,7 +524,7 @@ def test_delete_without_id_failed(self): with pytest.raises(TypeError): patient.delete() - def test_delete_with_params__no_match(self): + def test_conditional_delete__no_match(self): self.create_resource("Patient", id="patient") _, status_code = self.client.resources("Patient").search(identifier="other").delete() @@ -528,7 +532,7 @@ def test_delete_with_params__no_match(self): self.get_search_set("Patient").search(_id="patient").get() assert status_code == 204 # noqa: PLR2004 - def test_delete_with_params__one_match(self): + def test_conditional_delete__one_match(self): patient = self.client.resource( "Patient", id="patient", @@ -542,7 +546,7 @@ def test_delete_with_params__one_match(self): self.get_search_set("Patient").search(_id="patient").get() assert status_code == 200 # noqa: PLR2004 - def test_delete_with_params__multiple_matches(self): + def test_conditional_delete__multiple_matches(self): self.create_resource("Patient", id="patient-1") self.create_resource("Patient", id="patient-2") @@ -776,18 +780,41 @@ def test_save_fields(self): active=False, birthDate="1998-01-01", name=[{"text": "Abc"}], + managingOrganization={"reference": "urn:organization"}, ) patient["gender"] = "male" patient["birthDate"] = "1998-02-02" patient["active"] = True patient["name"] = [{"text": "Bcd"}] - patient.save(fields=["gender", "birthDate"]) + patient["managingOrganization"] = None + patient.save(fields=["gender", "birthDate", "managingOrganization"]) patient_refreshed = patient.to_reference().to_resource() assert patient_refreshed["gender"] == patient["gender"] assert patient_refreshed["birthDate"] == patient["birthDate"] assert patient_refreshed["active"] is False assert patient_refreshed["name"] == [{"text": "Abc"}] + assert patient_refreshed.get("managingOrganization") is None + + def test_update_patch_without_id(self): + patient = self.client.resource( + "Patient", identifier=self.identifier, name=[{"text": "J London"}] + ) + new_name = [ + { + "text": "Jack London", + "family": "London", + "given": ["Jack"], + } + ] + with pytest.raises(TypeError): + patient.update() + with pytest.raises(TypeError): + patient.patch(active=True, name=new_name) + patient["name"] = new_name + with pytest.raises(TypeError): + patient.save(fields=["name"]) + patient.save() def test_update(self): patient_id = "patient_to_update" @@ -852,26 +879,6 @@ def test_reference_patch(self): assert patient["active"] is True assert patient["name"] == new_name - def test_update_without_id(self): - patient = self.client.resource( - "Patient", identifier=self.identifier, name=[{"text": "J London"}] - ) - new_name = [ - { - "text": "Jack London", - "family": "London", - "given": ["Jack"], - } - ] - with pytest.raises(TypeError): - patient.update() - with pytest.raises(TypeError): - patient.patch(active=True, name=new_name) - patient["name"] = new_name - with pytest.raises(TypeError): - patient.save(fields=["name"]) - patient.save() - def test_refresh(self): patient_id = "refresh-patient-id" patient = self.create_resource("Patient", id=patient_id, active=True)