VED-457: Refactor backend to include e-tag Header (#650)

* VED-457: Refactor backend to include e-tag Header

* VED-457: change return type and e2e update response

* VED-457: change return type and e2e update response json

* VED-457: update imms call with etag parameter

* VED-457: refactor e-tag headers

* VED-457: refactor e-tag headers data type

* VED-457: fix backoff error

* VED-457: fix backoff error2

* VED-457: fix backoff error 3

* VED-457: fix backoff update error

* VED-457: fix backoff update error2

* VED-457: fix e-tag

* VED-457: fix e-tag2

* VED-457: Send correct E-Tag in update request.

* VED-457: Fix lint error.

* VED-457: Return updated version in two missed places.

* VED-457: improve coverage test

* VED-457: improve coverage for services and repository

* VED-457: Remove unittest skip from test

---------

Co-authored-by: Matt Jarvis <[email protected]>

Author: Akol125

backend/src/fhir_controller.py

@@ -197,7 +197,8 @@ def create_immunization(self, aws_event):
                 return self.create_response(400, json.dumps(exp_error))
             else:
                 location = f"{get_service_url()}/Immunization/{resource.id}"
-                return self.create_response(201, None, {"Location": location})
+                version = "1"
+                return self.create_response(201, None, {"Location": location, "E-Tag": version})
         except ValidationError as error:
             return self.create_response(400, error.to_operation_outcome())
         except IdentifierDuplicationError as duplicate:
@@ -224,7 +225,7 @@ def update_immunization(self, aws_event):
 
         # Validate the imms id - start
         if id_error := self._validate_id(imms_id):
-            return FhirController.create_response(400, json.dumps(id_error))    
+            return FhirController.create_response(400, json.dumps(id_error))
         # Validate the imms id - end
 
         # Validate the body of the request - start
@@ -276,12 +277,17 @@ def update_immunization(self, aws_event):
         # Check vaccine type permissions on the existing record - end
 
         existing_resource_version = int(existing_record["Version"])
+
         try:
             # Validate if the imms resource to be updated is a logically deleted resource - start
             if existing_record["DeletedAt"] == True:
-
-                outcome, resource = self.fhir_service.reinstate_immunization(
-                    imms_id, imms, existing_resource_version, imms_vax_type_perms, supplier_system)
+                outcome, resource, updated_version = self.fhir_service.reinstate_immunization(
+                    imms_id,
+                    imms,
+                    existing_resource_version,
+                    imms_vax_type_perms,
+                    supplier_system
+                )
             # Validate if the imms resource to be updated is a logically deleted resource-end
             else:
                 # Validate if imms resource version is part of the request - start
@@ -330,15 +336,15 @@ def update_immunization(self, aws_event):
 
                 # Check if the record is reinstated record - start
                 if existing_record["Reinstated"] == True:
-                    outcome, resource = self.fhir_service.update_reinstated_immunization(
+                    outcome, resource, updated_version = self.fhir_service.update_reinstated_immunization(
                         imms_id,
                         imms,
                         existing_resource_version,
                         imms_vax_type_perms,
                         supplier_system
                     )
                 else:
-                    outcome, resource = self.fhir_service.update_immunization(
+                    outcome, resource, updated_version = self.fhir_service.update_immunization(
                         imms_id,
                         imms,
                         existing_resource_version,
@@ -358,7 +364,7 @@ def update_immunization(self, aws_event):
                 )
                 return self.create_response(400, json.dumps(exp_error))
             if outcome == UpdateOutcome.UPDATE:
-                return self.create_response(200)
+                return self.create_response(200, {"E-Tag": updated_version}) #include e-tag here, is it not included in the response resource
         except ValidationError as error:
             return self.create_response(400, error.to_operation_outcome())
         except IdentifierDuplicationError as duplicate:
@@ -541,7 +547,7 @@ def _create_bad_request(self, message):
         )
         return self.create_response(400, error)
 
-    
+
     def authorize_request(self, aws_event: dict) -> Optional[dict]:
         try:
             self.authorizer.authorize(aws_event)
@@ -667,5 +673,5 @@ def create_response(status_code, body=None, headers=None):
     def _identify_supplier_system(aws_event):
         supplier_system = aws_event["headers"]["SupplierSystem"]
         if not supplier_system:
-            return self.create_response(403, unauthorized.to_operation_outcome())
+            raise UnauthorizedError("SupplierSystem header is missing")
         return supplier_system

backend/src/fhir_repository.py

@@ -3,7 +3,7 @@
 import time
 import uuid
 from dataclasses import dataclass
-from typing import Optional
+from typing import Optional, Tuple
 
 import boto3
 import botocore.exceptions
@@ -194,9 +194,9 @@ def update_immunization(
         immunization: dict,
         patient: any,
         existing_resource_version: int,
-        imms_vax_type_perms: str,
+        imms_vax_type_perms: list[str],
         supplier_system: str,
-    ) -> dict:
+    ) -> tuple[dict, int]:
         attr = RecordAttributes(immunization, patient)
         self._handle_permissions(imms_vax_type_perms, attr)
         update_exp = self._build_update_expression(is_reinstate=False)
@@ -219,10 +219,10 @@ def reinstate_immunization(
         immunization: dict,
         patient: any,
         existing_resource_version: int,
-        imms_vax_type_perms: str,
+        imms_vax_type_perms: list[str],
         supplier_system: str,
 
-    ) -> dict:
+    ) -> tuple[dict, int]:
         attr = RecordAttributes(immunization, patient)
         self._handle_permissions(imms_vax_type_perms, attr)
         update_exp = self._build_update_expression(is_reinstate=True)
@@ -245,9 +245,9 @@ def update_reinstated_immunization(
         immunization: dict,
         patient: any,
         existing_resource_version: int,
-        imms_vax_type_perms: str,
+        imms_vax_type_perms: list[str],
         supplier_system: str,
-    ) -> dict:
+    ) -> tuple[dict, int]:
         attr = RecordAttributes(immunization, patient)
         self._handle_permissions(imms_vax_type_perms, attr)
         update_exp = self._build_update_expression(is_reinstate=False)
@@ -299,8 +299,9 @@ def _perform_dynamo_update(
         supplier_system: str,
         deleted_at_required: bool,
         update_reinstated: bool,
-    ) -> dict:
+    ) -> Tuple[dict, int]:
         try:
+            updated_version = existing_resource_version + 1
             condition_expression = Attr("PK").eq(attr.pk) & (
                 Attr("DeletedAt").exists()
                 if deleted_at_required
@@ -313,7 +314,7 @@ def _perform_dynamo_update(
                     ":patient_sk": attr.patient_sk,
                     ":imms_resource_val": json.dumps(attr.resource, use_decimal=True),
                     ":operation": "UPDATE",
-                    ":version": existing_resource_version + 1,
+                    ":version": updated_version,
                     ":supplier_system": supplier_system,
                     ":respawn": "reinstated",
                 }
@@ -324,7 +325,7 @@ def _perform_dynamo_update(
                     ":patient_sk": attr.patient_sk,
                     ":imms_resource_val": json.dumps(attr.resource, use_decimal=True),
                     ":operation": "UPDATE",
-                    ":version": existing_resource_version + 1,
+                    ":version": updated_version,
                     ":supplier_system": supplier_system,
                 }
 
@@ -338,7 +339,7 @@ def _perform_dynamo_update(
                 ReturnValues="ALL_NEW",
                 ConditionExpression=condition_expression,
             )
-            return self._handle_dynamo_response(response)
+            return self._handle_dynamo_response(response), updated_version
         except botocore.exceptions.ClientError as error:
             # Either resource didn't exist or it has already been deleted. See ConditionExpression in the request
             if error.response["Error"]["Code"] == "ConditionalCheckFailedException":
@@ -352,12 +353,12 @@ def _perform_dynamo_update(
     def delete_immunization(
             self, imms_id: str, imms_vax_type_perms: str, supplier_system: str) -> dict:
         now_timestamp = int(time.time())
-        
+
         try:
             item = self.table.get_item(Key={"PK": _make_immunization_pk(imms_id)}).get("Item")
             if not item:
                 raise ResourceNotFoundError(resource_type="Immunization", resource_id=imms_id)
-            
+
             if not item.get("DeletedAt") or item.get("DeletedAt") == "reinstated":
                 vaccine_type = self._vaccine_type(item["PatientSK"])
                 if not validate_permissions(imms_vax_type_perms, ApiOperationCode.DELETE, [vaccine_type]):
@@ -380,7 +381,7 @@ def delete_immunization(
                     (Attr("DeletedAt").not_exists() | Attr("DeletedAt").eq("reinstated"))
                 ),
             )
-            
+
             return self._handle_dynamo_response(response)
         except botocore.exceptions.ClientError as error:
             if error.response["Error"]["Code"] == "ConditionalCheckFailedException":

backend/src/fhir_service.py

@@ -136,13 +136,13 @@ def update_immunization(
         existing_resource_version: int,
         imms_vax_type_perms: list[str],
         supplier_system: str,
-    ) -> tuple[UpdateOutcome, Immunization]:
+    ) -> tuple[UpdateOutcome, Immunization, int]:
         immunization["id"] = imms_id
 
         patient = self._validate_patient(immunization)
         if "diagnostics" in patient:
-            return (None, patient)
-        imms = self.immunization_repo.update_immunization(
+            return (None, patient, None)
+        imms, updated_version = self.immunization_repo.update_immunization(
             imms_id,
             immunization,
             patient,
@@ -151,7 +151,7 @@ def update_immunization(
             supplier_system
         )
 
-        return UpdateOutcome.UPDATE, Immunization.parse_obj(imms)
+        return UpdateOutcome.UPDATE, Immunization.parse_obj(imms), updated_version
 
     def reinstate_immunization(
         self,
@@ -160,12 +160,12 @@ def reinstate_immunization(
         existing_resource_version: int,
         imms_vax_type_perms: list[str],
         supplier_system: str,
-    ) -> tuple[UpdateOutcome, Immunization]:
+    ) -> tuple[UpdateOutcome, Immunization, int]:
         immunization["id"] = imms_id
         patient = self._validate_patient(immunization)
         if "diagnostics" in patient:
-            return (None, patient)
-        imms = self.immunization_repo.reinstate_immunization(
+            return (None, patient, None)
+        imms, updated_version = self.immunization_repo.reinstate_immunization(
             imms_id,
             immunization,
             patient,
@@ -174,7 +174,7 @@ def reinstate_immunization(
             supplier_system,
         )
 
-        return UpdateOutcome.UPDATE, Immunization.parse_obj(imms)
+        return UpdateOutcome.UPDATE, Immunization.parse_obj(imms), updated_version
 
     def update_reinstated_immunization(
         self,
@@ -183,14 +183,12 @@ def update_reinstated_immunization(
         existing_resource_version: int,
         imms_vax_type_perms: list[str],
         supplier_system: str,
-    ) -> tuple[UpdateOutcome, Immunization]:
+    ) -> tuple[UpdateOutcome, Immunization, int]:
         immunization["id"] = imms_id
-
-        patient = None
         patient = self._validate_patient(immunization)
         if "diagnostics" in patient:
-            return (None, patient)
-        imms = self.immunization_repo.update_reinstated_immunization(
+            return (None, patient, None)
+        imms, updated_version = self.immunization_repo.update_reinstated_immunization(
             imms_id,
             immunization,
             patient,
@@ -199,7 +197,7 @@ def update_reinstated_immunization(
             supplier_system,
         )
 
-        return UpdateOutcome.UPDATE, Immunization.parse_obj(imms)
+        return UpdateOutcome.UPDATE, Immunization.parse_obj(imms), updated_version
 
     def delete_immunization(self, imms_id, imms_vax_type_perms, supplier_system) -> Immunization:
         """