Merge branch 'master' into dependabot/github_actions/docker/setup-buildx-action-4.0.0

Author: Thomas-Boyle

lambdas/backend/src/controller/fhir_controller.py

@@ -89,12 +89,17 @@ def create_immunization(self, aws_event: APIGatewayProxyEventV1) -> dict:
         except JSONDecodeError as e:
             raise InvalidJsonError(message=str(f"Request's body contains malformed JSON: {e}"))
 
-        created_resource_id = self.fhir_service.create_immunization(immunisation, supplier_system)
+        created_resource_id, created_resource_version = self.fhir_service.create_immunization(
+            immunisation, supplier_system
+        )
 
         return create_response(
             status_code=201,
             body=None,
-            headers={"Location": f"{self._API_SERVICE_URL}/Immunization/{created_resource_id}", "E-Tag": "1"},
+            headers={
+                "Location": f"{self._API_SERVICE_URL}/Immunization/{created_resource_id}",
+                "E-Tag": str(created_resource_version),
+            },
         )
 
     @fhir_api_exception_handler

lambdas/backend/src/service/fhir_service.py

@@ -3,7 +3,7 @@
 import logging
 import os
 import uuid
-from typing import Any, cast
+from typing import Any
 from uuid import uuid4
 
 from fhir.resources.R4B.bundle import (
@@ -144,7 +144,7 @@ def get_immunization_and_version_by_id(self, imms_id: str, supplier_system: str)
 
         return Immunization.parse_obj(resource), str(immunization_metadata.resource_version)
 
-    def create_immunization(self, immunization: dict, supplier_system: str) -> Id:
+    def create_immunization(self, immunization: dict, supplier_system: str) -> tuple[Id, int]:
         if immunization.get("id") is not None:
             raise CustomValidationError("id field must not be present for CREATE operation")
 
@@ -155,16 +155,32 @@ def create_immunization(self, immunization: dict, supplier_system: str) -> Id:
         if not self.authoriser.authorise(supplier_system, ApiOperationCode.CREATE, {vaccination_type}):
             raise UnauthorizedVaxError()
 
-        # Set ID for the requested new record
-        immunization["id"] = str(uuid.uuid4())
+        identifier = Identifier.parse_obj(immunization["identifier"][0])
+        duplicate_identifier = f"{identifier.system}#{identifier.value}"
 
-        immunization_fhir_entity = Immunization.parse_obj(immunization)
-        identifier = cast(Identifier, immunization_fhir_entity.identifier[0])
+        existing_immunization_resource, existing_immunization_meta = (
+            self.immunization_repo.get_immunization_by_identifier(identifier)
+        )
+        if existing_immunization_resource:
+            if not existing_immunization_meta.is_deleted:
+                raise IdentifierDuplicationError(identifier=duplicate_identifier)
+
+            immunization_id = existing_immunization_resource["id"]
+            immunization["id"] = immunization_id
+            immunization_fhir_entity = Immunization.parse_obj(immunization)
+            updated_version = self.immunization_repo.update_immunization(
+                immunization_id,
+                immunization_fhir_entity,
+                existing_immunization_meta,
+                supplier_system,
+            )
+            return immunization_id, updated_version
 
-        if self.immunization_repo.check_immunization_identifier_exists(identifier.system, identifier.value):
-            raise IdentifierDuplicationError(identifier=f"{identifier.system}#{identifier.value}")
+        immunization["id"] = str(uuid.uuid4())
+        immunization_fhir_entity = Immunization.parse_obj(immunization)
 
-        return self.immunization_repo.create_immunization(immunization_fhir_entity, supplier_system)
+        created_id = self.immunization_repo.create_immunization(immunization_fhir_entity, supplier_system)
+        return created_id, 1
 
     def update_immunization(self, imms_id: str, immunization: dict, supplier_system: str, resource_version: int) -> int:
         self._validate_immunization(immunization)

lambdas/backend/tests/controller/test_fhir_controller.py

@@ -529,7 +529,7 @@ def test_create_immunization(self):
             "headers": {"SupplierSystem": "Test"},
             "body": imms.json(),
         }
-        self.service.create_immunization.return_value = imms_id
+        self.service.create_immunization.return_value = (imms_id, 1)
 
         response = self.controller.create_immunization(aws_event)
 
@@ -538,6 +538,23 @@ def test_create_immunization(self):
         self.assertEqual(response["statusCode"], 201)
         self.assertTrue("body" not in response)
         self.assertTrue(response["headers"]["Location"].endswith(f"Immunization/{imms_id}"))
+        self.assertEqual(response["headers"]["E-Tag"], "1")
+
+    def test_create_immunization_returns_current_version_when_reinstating_deleted_record(self):
+        """it should return the existing resource location and current version for a reinstated create request"""
+        imms_id = str(uuid.uuid4())
+        imms = create_covid_immunization(imms_id)
+        aws_event = {
+            "headers": {"SupplierSystem": "Test"},
+            "body": imms.json(),
+        }
+        self.service.create_immunization.return_value = (imms_id, 3)
+
+        response = self.controller.create_immunization(aws_event)
+
+        self.assertEqual(response["statusCode"], 201)
+        self.assertTrue(response["headers"]["Location"].endswith(f"Immunization/{imms_id}"))
+        self.assertEqual(response["headers"]["E-Tag"], "3")
 
     def test_create_immunization_returns_unauthorised_error_when_supplier_system_header_missing(self):
         """it should return unauthorized error"""

lambdas/backend/tests/service/test_fhir_service.py

@@ -349,31 +349,33 @@ def test_create_immunization(self):
         self.mock_redis.hget.return_value = "COVID"
         self.mock_redis_getter.return_value = self.mock_redis
         self.authoriser.authorise.return_value = True
-        self.imms_repo.check_immunization_identifier_exists.return_value = False
+        self.imms_repo.get_immunization_by_identifier.return_value = (None, None)
         self.imms_repo.create_immunization.return_value = self._MOCK_NEW_UUID
 
         nhs_number = VALID_NHS_NUMBER
         req_imms = create_covid_immunization_dict_no_id(nhs_number)
 
         # When
-        created_id = self.fhir_service.create_immunization(req_imms, "Test")
+        created_id, created_version = self.fhir_service.create_immunization(req_imms, "Test")
 
         # Then
         self.authoriser.authorise.assert_called_once_with("Test", ApiOperationCode.CREATE, {"COVID"})
-        self.imms_repo.check_immunization_identifier_exists.assert_called_once_with(
-            "https://supplierABC/identifiers/vacc", "ACME-vacc123456"
-        )
+        self.imms_repo.get_immunization_by_identifier.assert_called_once()
+        create_identifier = self.imms_repo.get_immunization_by_identifier.call_args.args[0]
+        self.assertEqual(create_identifier.system, "https://supplierABC/identifiers/vacc")
+        self.assertEqual(create_identifier.value, "ACME-vacc123456")
         self.imms_repo.create_immunization.assert_called_once_with(Immunization.parse_obj(req_imms), "Test")
 
         self.validator.validate.assert_called_once_with(req_imms)
         self.assertEqual(self._MOCK_NEW_UUID, created_id)
+        self.assertEqual(1, created_version)
 
     def test_create_immunization_keeps_first_site_and_route_snomed_coding(self):
         """it should keep the first SNOMED coding for site and route during API create"""
         self.mock_redis.hget.return_value = "COVID"
         self.mock_redis_getter.return_value = self.mock_redis
         self.authoriser.authorise.return_value = True
-        self.imms_repo.check_immunization_identifier_exists.return_value = False
+        self.imms_repo.get_immunization_by_identifier.return_value = (None, None)
         self.imms_repo.create_immunization.return_value = self._MOCK_NEW_UUID
 
         req_imms = create_covid_immunization_dict_no_id(VALID_NHS_NUMBER)
@@ -382,9 +384,10 @@ def test_create_immunization_keeps_first_site_and_route_snomed_coding(self):
             req_imms, "route", "888888888", "Replacement route that should be ignored"
         )
 
-        created_id = self.pre_validate_fhir_service.create_immunization(req_imms, "Test")
+        created_id, created_version = self.pre_validate_fhir_service.create_immunization(req_imms, "Test")
 
         self.assertEqual(self._MOCK_NEW_UUID, created_id)
+        self.assertEqual(1, created_version)
         self.assertEqual(req_imms["site"]["coding"], [first_site_coding])
         self.assertEqual(req_imms["route"]["coding"], [first_route_coding])
         self.imms_repo.create_immunization.assert_called_once_with(Immunization.parse_obj(req_imms), "Test")
@@ -496,7 +499,15 @@ def test_raises_duplicate_error_if_identifier_already_exits(self):
         self.mock_redis.hget.return_value = "COVID"
         self.mock_redis_getter.return_value = self.mock_redis
         self.authoriser.authorise.return_value = True
-        self.imms_repo.check_immunization_identifier_exists.return_value = True
+        existing_immunisation = create_covid_immunization_dict("existing-id", VALID_NHS_NUMBER)
+        identifier = Identifier(
+            system=existing_immunisation["identifier"][0]["system"],
+            value=existing_immunisation["identifier"][0]["value"],
+        )
+        self.imms_repo.get_immunization_by_identifier.return_value = (
+            existing_immunisation,
+            ImmunizationRecordMetadata(identifier=identifier, resource_version=1, is_deleted=False, is_reinstated=False),
+        )
 
         nhs_number = VALID_NHS_NUMBER
         req_imms = create_covid_immunization_dict_no_id(nhs_number)
@@ -507,16 +518,53 @@ def test_raises_duplicate_error_if_identifier_already_exits(self):
 
         # Then
         self.authoriser.authorise.assert_called_once_with("Test", ApiOperationCode.CREATE, {"COVID"})
-        self.imms_repo.check_immunization_identifier_exists.assert_called_once_with(
-            "https://supplierABC/identifiers/vacc", "ACME-vacc123456"
-        )
+        self.imms_repo.get_immunization_by_identifier.assert_called_once()
+        duplicate_identifier = self.imms_repo.get_immunization_by_identifier.call_args.args[0]
+        self.assertEqual(duplicate_identifier.system, "https://supplierABC/identifiers/vacc")
+        self.assertEqual(duplicate_identifier.value, "ACME-vacc123456")
         self.imms_repo.create_immunization.assert_not_called()
+        self.imms_repo.update_immunization.assert_not_called()
         self.validator.validate.assert_called_once_with(req_imms)
         self.assertEqual(
             "The provided identifier: https://supplierABC/identifiers/vacc#ACME-vacc123456 is duplicated",
             str(error.exception),
         )
 
+    def test_reinstates_deleted_duplicate_identifier_on_create(self):
+        """it should reinstate a deleted record when a create request matches a deleted identifier"""
+        self.mock_redis.hget.return_value = "COVID"
+        self.mock_redis_getter.return_value = self.mock_redis
+        self.authoriser.authorise.return_value = True
+
+        existing_immunisation = create_covid_immunization_dict("existing-id", VALID_NHS_NUMBER)
+        identifier = Identifier(
+            system=existing_immunisation["identifier"][0]["system"],
+            value=existing_immunisation["identifier"][0]["value"],
+        )
+        existing_metadata = ImmunizationRecordMetadata(
+            identifier=identifier,
+            resource_version=2,
+            is_deleted=True,
+            is_reinstated=False,
+        )
+        self.imms_repo.get_immunization_by_identifier.return_value = (existing_immunisation, existing_metadata)
+        self.imms_repo.update_immunization.return_value = 3
+
+        req_imms = create_covid_immunization_dict_no_id(VALID_NHS_NUMBER)
+
+        reinstated_id, reinstated_version = self.fhir_service.create_immunization(req_imms, "Test")
+
+        self.authoriser.authorise.assert_called_once_with("Test", ApiOperationCode.CREATE, {"COVID"})
+        self.imms_repo.create_immunization.assert_not_called()
+        self.imms_repo.update_immunization.assert_called_once_with(
+            "existing-id",
+            Immunization.parse_obj(req_imms),
+            existing_metadata,
+            "Test",
+        )
+        self.assertEqual("existing-id", reinstated_id)
+        self.assertEqual(3, reinstated_version)
+
 
 class TestUpdateImmunization(TestFhirServiceBase):
     """Tests for FhirService.update_immunization"""

lambdas/recordforwarder/src/repository/fhir_batch_repository.py

@@ -92,14 +92,28 @@ def create_immunization(
         table: any,
         imms_pk: str | None,
     ) -> str:
+        identifier = self._identifier_response(immunization)
+        query_response = _query_identifier(table, identifier, imms_pk)
+        if query_response is not None:
+            deleted_at_required, update_reinstated, is_reinstate = self._get_record_status(query_response)
+            if not deleted_at_required or update_reinstated:
+                raise IdentifierDuplicationError(identifier=identifier)
+
+            return self._update_existing_immunization(
+                immunization,
+                supplier_system,
+                vax_type,
+                table,
+                query_response,
+                deleted_at_required,
+                update_reinstated,
+                is_reinstate,
+            )
+
         new_id = str(uuid.uuid4())
         immunization["id"] = new_id
         attr = RecordAttributes(immunization, vax_type, supplier_system, 0)
 
-        query_response = _query_identifier(table, attr.identifier, imms_pk)
-        if query_response is not None:
-            raise IdentifierDuplicationError(identifier=attr.identifier)
-
         try:
             response = table.put_item(
                 Item={
@@ -140,20 +154,17 @@ def update_immunization(
         query_response = _query_identifier(table, identifier, imms_pk)
         if query_response is None:
             raise ResourceNotFoundError(resource_type="Immunization", resource_id=identifier)
-        old_id, version = self._get_id_version(query_response)
         deleted_at_required, update_reinstated, is_reinstate = self._get_record_status(query_response)
 
-        immunization["id"] = old_id.split("#")[1]
-        attr = RecordAttributes(immunization, vax_type, supplier_system, version)
-
-        update_exp = self._build_update_expression(is_reinstate=is_reinstate)
-
-        return self._perform_dynamo_update(
-            update_exp,
-            attr,
-            deleted_at_required=deleted_at_required,
-            update_reinstated=update_reinstated,
-            table=table,
+        return self._update_existing_immunization(
+            immunization,
+            supplier_system,
+            vax_type,
+            table,
+            query_response,
+            deleted_at_required,
+            update_reinstated,
+            is_reinstate,
         )
 
     def delete_immunization(
@@ -250,6 +261,30 @@ def _build_update_expression(is_reinstate: bool) -> str:
                 "Operation = :operation, Version = :version, SupplierSystem = :supplier_system "
             )
 
+    def _update_existing_immunization(
+        self,
+        immunization: any,
+        supplier_system: str,
+        vax_type: str,
+        table: any,
+        query_response: any,
+        deleted_at_required: bool,
+        update_reinstated: bool,
+        is_reinstate: bool,
+    ) -> str:
+        old_id, version = self._get_id_version(query_response)
+        immunization["id"] = old_id.split("#")[1]
+        attr = RecordAttributes(immunization, vax_type, supplier_system, version)
+        update_exp = self._build_update_expression(is_reinstate=is_reinstate)
+
+        return self._perform_dynamo_update(
+            update_exp,
+            attr,
+            deleted_at_required=deleted_at_required,
+            update_reinstated=update_reinstated,
+            table=table,
+        )
+
     def _perform_dynamo_update(
         self,
         update_exp: str,

lambdas/recordforwarder/tests/repository/test_fhir_batch_repository.py

@@ -94,15 +94,57 @@ def test_create_immunization_duplicate(self):
         """it should not create Immunization since the request is duplicate"""
         self.table.query = MagicMock(
             return_value={
-                "id": imms_id,
-                "identifier": [{"system": "test-system", "value": "12345"}],
-                "contained": [{"resourceType": "Patient", "identifier": [{"value": "98765"}]}],
                 "Count": 1,
+                "Items": [
+                    {
+                        "PK": _make_immunization_pk(imms_id),
+                        "Version": 1,
+                    }
+                ],
             }
         )
         with self.assertRaises(IdentifierDuplicationError):
             self.repository.create_immunization(self.immunization, "supplier", "vax-type", self.table, None)
         self.table.put_item.assert_not_called()
+        self.table.update_item.assert_not_called()
+
+    def test_create_immunization_reinstates_deleted_duplicate(self):
+        """it should reinstate a deleted record when a create request matches a deleted identifier"""
+        existing_pk = _make_immunization_pk(imms_id)
+        self.table.query = MagicMock(
+            return_value={
+                "Count": 1,
+                "Items": [
+                    {
+                        "PK": existing_pk,
+                        "Version": 2,
+                        "DeletedAt": "20210101",
+                    }
+                ],
+            }
+        )
+
+        response = self.repository.create_immunization(self.immunization, "supplier", "vax-type", self.table, None)
+
+        self.table.put_item.assert_not_called()
+        self.table.update_item.assert_called_once_with(
+            Key={"PK": existing_pk},
+            UpdateExpression=ANY,
+            ExpressionAttributeNames={"#imms_resource": "Resource"},
+            ExpressionAttributeValues={
+                ":timestamp": ANY,
+                ":patient_pk": ANY,
+                ":patient_sk": ANY,
+                ":imms_resource_val": json.dumps(self.immunization),
+                ":operation": "UPDATE",
+                ":version": 3,
+                ":supplier_system": "supplier",
+                ":respawn": "reinstated",
+            },
+            ReturnValues=ANY,
+            ConditionExpression=ANY,
+        )
+        self.assertEqual(response, existing_pk)
 
     def test_create_should_catch_dynamo_error(self):
         """it should throw UnhandledResponse when the response from dynamodb can't be handled"""