PRMDR-785: Upload retry logic in the event of upload failure (#356)

* Add Upload Retry logic in the event of upload failure

* fix pip-audit issues and format

* pr changes

app/src/helpers/requests/uploadDocument.test.ts

@@ -12,7 +12,7 @@ describe('[POST] updateDocumentState', () => {
         const document = buildDocument(buildTextFile('test1'), documentUploadStates.SUCCEEDED);
         mockedAxios.post.mockImplementation(() => Promise.resolve({ status: 200 }));
         const args = {
-            document,
+            documents: [document],
             uploadingState: true,
             documentReference: 'test/test/test/key',
             baseUrl: '/test',

app/src/helpers/requests/uploadDocuments.ts

@@ -33,7 +33,7 @@ type DocRefResponse = {
 };
 
 type UpdateStateArgs = {
-    document: UploadDocument;
+    documents: UploadDocument[];
     uploadingState: boolean;
     baseUrl: string;
     baseHeaders: AuthHeaders;
@@ -187,21 +187,20 @@ const uploadDocuments = async ({
         throw error;
     }
 };
+
 export const updateDocumentState = async ({
-    document,
+    documents,
     uploadingState,
     baseUrl,
     baseHeaders,
 }: UpdateStateArgs) => {
     const updateUploadStateUrl = baseUrl + endpoints.UPLOAD_DOCUMENT_STATE;
     const body = {
-        files: [
-            {
-                reference: document.ref,
-                type: document.docType,
-                fields: { Uploading: uploadingState },
-            },
-        ],
+        files: documents.map((document) => ({
+            reference: document.ref,
+            type: document.docType,
+            fields: { Uploading: uploadingState },
+        })),
     };
     try {
         return await axios.post(updateUploadStateUrl, body, {

app/src/pages/lloydGeorgeUploadPage/LloydGeorgeUploadPage.tsx

@@ -80,6 +80,15 @@ function LloydGeorgeUploadPage() {
         const hasNoVirus =
             documents.length && documents.every((d) => d.state === DOCUMENT_UPLOAD_STATE.CLEAN);
 
+        const setUploadStateFailed = async () => {
+            await updateDocumentState({
+                documents: documents,
+                uploadingState: false,
+                baseUrl,
+                baseHeaders,
+            });
+        };
+
         const confirmUpload = async () => {
             if (uploadSession) {
                 setStage(LG_UPLOAD_STAGE.CONFIRMATION);
@@ -105,16 +114,19 @@ function LloydGeorgeUploadPage() {
                         navigate(routes.SESSION_EXPIRED);
                         return;
                     }
+                    await setUploadStateFailed();
                     setStage(LG_UPLOAD_STAGE.FAILED);
                 }
             }
         };
 
         if (hasExceededUploadAttempts) {
             window.clearInterval(intervalTimer);
+            void setUploadStateFailed();
             setStage(LG_UPLOAD_STAGE.FAILED);
         } else if (hasVirus) {
             window.clearInterval(intervalTimer);
+            void setUploadStateFailed();
             setStage(LG_UPLOAD_STAGE.INFECTED);
         } else if (hasNoVirus && !confirmed.current) {
             confirmed.current = true;
@@ -161,19 +173,12 @@ function LloydGeorgeUploadPage() {
                     progress: 100,
                 });
             } catch (e) {
-                window.clearInterval(intervalTimer);
                 setDocument(setDocuments, {
                     id: document.id,
                     state: DOCUMENT_UPLOAD_STATE.FAILED,
                     attempts: document.attempts + 1,
                     progress: 0,
                 });
-                await updateDocumentState({
-                    document,
-                    uploadingState: false,
-                    baseUrl,
-                    baseHeaders,
-                });
             }
         });
     };
@@ -233,21 +238,25 @@ function LloydGeorgeUploadPage() {
         setDocuments([]);
         setStage(LG_UPLOAD_STAGE.SELECT);
     };
+
     const startIntervalTimer = (uploadDocuments: Array<UploadDocument>) => {
         return window.setInterval(() => {
-            uploadDocuments.forEach(async (document) => {
-                try {
-                    await updateDocumentState({
-                        document,
-                        uploadingState: true,
-                        baseUrl,
-                        baseHeaders,
-                    });
-                } catch (e) {}
-            });
+            void setDocumentUploadingState(uploadDocuments, true);
         }, 120000);
     };
 
+    const setDocumentUploadingState = async (
+        uploadDocuments: Array<UploadDocument>,
+        uploadingState: boolean,
+    ) => {
+        await updateDocumentState({
+            documents: uploadDocuments,
+            uploadingState: uploadingState,
+            baseUrl,
+            baseHeaders,
+        });
+    };
+
     switch (stage) {
         case LG_UPLOAD_STAGE.SELECT:
             return (

lambdas/models/auth_policy.py

@@ -9,6 +9,7 @@
 This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and limitations under the License.
 """
+
 import os
 import re
 

lambdas/models/document_reference.py

@@ -7,6 +7,16 @@
 from utils.exceptions import InvalidDocumentReferenceException
 
 
+class UploadDocumentReference(BaseModel):
+    reference: str = Field(...)
+    doc_type: str = Field(..., alias="type")
+    fields: dict[str, bool] = Field(...)
+
+
+class UploadDocumentReferences(BaseModel):
+    files: list[UploadDocumentReference] = Field(...)
+
+
 class DocumentReference(BaseModel):
     id: str = Field(..., alias=str(DocumentReferenceMetadataFields.ID.value))
     content_type: str = Field(

lambdas/models/pds_models.py

@@ -111,9 +111,11 @@ def get_patient_details(self, nhs_number) -> PatientDetails:
             givenName=self.get_current_usual_name().given,
             familyName=self.get_current_usual_name().family,
             birthDate=self.birth_date,
-            postalCode=self.get_current_home_address().postal_code
-            if self.is_unrestricted()
-            else "",
+            postalCode=(
+                self.get_current_home_address().postal_code
+                if self.is_unrestricted()
+                else ""
+            ),
             nhsNumber=self.id,
             superseded=bool(nhs_number == id),
             restricted=not self.is_unrestricted(),
@@ -128,9 +130,9 @@ def get_minimum_patient_details(self, nhs_number) -> PatientDetails:
             givenName=self.get_current_usual_name().given,
             familyName=self.get_current_usual_name().family,
             birthDate=self.birth_date,
-            generalPracticeOds=self.get_active_ods_code_for_gp()
-            if self.is_unrestricted()
-            else "",
+            generalPracticeOds=(
+                self.get_active_ods_code_for_gp() if self.is_unrestricted() else ""
+            ),
             nhsNumber=self.id,
             superseded=bool(nhs_number == id),
             restricted=not self.is_unrestricted(),

lambdas/models/staging_metadata.py

@@ -1,6 +1,6 @@
 from typing import Optional
 
-from pydantic import BaseModel, ConfigDict, Field, FieldValidationInfo, field_validator
+from pydantic import BaseModel, ConfigDict, Field, ValidationInfo, field_validator
 from pydantic_core import PydanticCustomError
 
 METADATA_FILENAME = "metadata.csv"
@@ -36,7 +36,7 @@ class MetadataFile(BaseModel):
     @field_validator("gp_practice_code")
     @classmethod
     def ensure_gp_practice_code_non_empty(
-        cls, gp_practice_code: str, info: FieldValidationInfo
+        cls, gp_practice_code: str, info: ValidationInfo
     ) -> str:
         if not gp_practice_code:
             patient_nhs_number = info.data.get("nhs_number", "")

lambdas/requirements.txt

@@ -11,13 +11,13 @@ charset-normalizer==3.2.0
 cryptography==42.0.4
 email-validator==2.1.0.post1
 fhir.resources[xml]==7.1.0
-idna==3.4
+idna==3.7
 inflection==0.5.1
 jmespath==1.0.1
 oauthlib==3.2.2
 packaging==23.0.0
 pycparser==2.21
-pydantic==2.3.0
+pydantic==2.4.0
 pypdf==3.17.2
 python-dateutil==2.8.2
 requests==2.31.0

lambdas/scripts/batch_update_ods_code.py

@@ -192,9 +192,9 @@ def build_progress_dict(self, dynamodb_records: list[dict]) -> dict:
                 progress_dict[nhs_number].doc_ref_ids.append(doc_ref_id)
                 pds_code_at_current_row = ods_code
                 if progress_dict[nhs_number].prev_ods_code != pds_code_at_current_row:
-                    progress_dict[
-                        nhs_number
-                    ].prev_ods_code = "[multiple ods codes in records]"
+                    progress_dict[nhs_number].prev_ods_code = (
+                        "[multiple ods codes in records]"
+                    )
 
         self.logger.info(f"Totally {len(progress_dict)} patients found in record.")
         return progress_dict

lambdas/services/create_document_reference_service.py

@@ -70,9 +70,9 @@ def create_document_reference_request(
                             400, LambdaError.CreateDocInvalidType
                         )
 
-                url_responses[
-                    document_reference.file_name
-                ] = self.prepare_pre_signed_url(document_reference)
+                url_responses[document_reference.file_name] = (
+                    self.prepare_pre_signed_url(document_reference)
+                )
 
             if lg_documents:
                 validate_lg_files(lg_documents, nhs_number)

lambdas/services/document_reference_search_service.py

@@ -33,12 +33,12 @@ def get_document_references(self, nhs_number: str):
             for table_name in list_of_table_names:
                 logger.info(f"Searching for results in {table_name}")
 
-                documents: list[
-                    DocumentReference
-                ] = self.fetch_documents_from_table_with_filter(
-                    nhs_number,
-                    table_name,
-                    query_filter=delete_filter_expression,
+                documents: list[DocumentReference] = (
+                    self.fetch_documents_from_table_with_filter(
+                        nhs_number,
+                        table_name,
+                        query_filter=delete_filter_expression,
+                    )
                 )
 
                 results.extend(

lambdas/services/update_upload_state_service.py

@@ -1,12 +1,17 @@
-import os
 from datetime import datetime, timezone
 
 from botocore.exceptions import ClientError
 from enums.lambda_error import LambdaError
 from enums.metadata_field_names import DocumentReferenceMetadataFields
 from enums.supported_document_types import SupportedDocumentTypes
+from models.document_reference import UploadDocumentReferences
+from pydantic import ValidationError
 from services.base.dynamo_service import DynamoDBService
 from utils.audit_logging_setup import LoggingService
+from utils.decorators.validate_document_type import (
+    doc_type_is_valid,
+    extract_document_type_as_enum,
+)
 from utils.lambda_exceptions import UpdateUploadStateException
 
 logger = LoggingService(__name__)
@@ -18,50 +23,41 @@
 class UpdateUploadStateService:
     def __init__(self):
         self.dynamo_service = DynamoDBService()
-        self.lg_dynamo_table = os.getenv("LLOYD_GEORGE_DYNAMODB_NAME")
-        self.arf_dynamo_table = os.getenv("DOCUMENT_STORE_DYNAMODB_NAME")
 
     def handle_update_state(self, event_body: dict):
         try:
-            files = event_body["files"]
-            for file in files:
-                doc_ref = file["reference"]
-                doc_type = file["type"]
-                uploaded = file["fields"][Fields.UPLOADING.value]
-                not_valid = (
-                    not doc_type
-                    or not doc_ref
-                    or not uploaded
-                    or not isinstance(uploaded, bool)
-                )
-                if not_valid:
-                    raise UpdateUploadStateException(
-                        400, LambdaError.UpdateUploadStateValidation
-                    )
-                elif doc_type not in [
-                    SupportedDocumentTypes.ARF.value,
-                    SupportedDocumentTypes.LG.value,
-                ]:
+            upload_document_references = UploadDocumentReferences.model_validate(
+                event_body
+            )
+            for file in upload_document_references.files:
+                if not doc_type_is_valid(file.doc_type):
                     raise UpdateUploadStateException(
                         400, LambdaError.UpdateUploadStateDocType
                     )
-                else:
-                    self.update_document(doc_ref, doc_type, uploaded)
+
+                self.update_document(
+                    file.reference,
+                    extract_document_type_as_enum(file.doc_type),
+                    file.fields[str(Fields.UPLOADING.value)],
+                )
         except (KeyError, TypeError) as e:
             logger.error(
                 f"{LambdaError.UpdateUploadStateKey.to_str()} :{str(e)}",
             )
             raise UpdateUploadStateException(400, LambdaError.UpdateUploadStateKey)
+        except ValidationError as e:
+            logger.error(
+                f"{LambdaError.UpdateUploadStateValidation.to_str()} :{str(e)}",
+            )
+            raise UpdateUploadStateException(
+                400, LambdaError.UpdateUploadStateValidation
+            )
 
     def update_document(
         self, doc_ref: str, doc_type: SupportedDocumentTypes, uploaded: bool
     ):
         updated_fields = self.format_update({Fields.UPLOADING.value: uploaded})
-        table = (
-            self.lg_dynamo_table
-            if doc_type == SupportedDocumentTypes.LG.value
-            else self.arf_dynamo_table
-        )
+        table = SupportedDocumentTypes.get_dynamodb_table_name(doc_type)
         try:
             self.dynamo_service.update_item(
                 table_name=table,

lambdas/tests/unit/helpers/data/update_upload_state.py

@@ -8,23 +8,21 @@
 
 MOCK_DOCUMENT_REFERENCE = TEST_FILE_KEY
 
-MOCK_LG_DOCTYPE = SupportedDocumentTypes.LG.value
 MOCK_LG_DOCUMENTS_REQUEST = {
     "files": [
         {
             "reference": TEST_FILE_KEY,
-            "type": SupportedDocumentTypes.LG.value,
+            "type": str(SupportedDocumentTypes.LG.value),
             "fields": {Fields.UPLOADING.value: True},
         }
     ]
 }
 
-MOCK_ARF_DOCTYPE = SupportedDocumentTypes.ARF.value
 MOCK_ARF_DOCUMENTS_REQUEST = {
     "files": [
         {
             "reference": TEST_FILE_KEY,
-            "type": SupportedDocumentTypes.ARF.value,
+            "type": str(SupportedDocumentTypes.ARF.value),
             "fields": {Fields.UPLOADING.value: True},
         }
     ]
@@ -34,7 +32,7 @@
     "files": [
         {
             "reference": TEST_FILE_KEY,
-            "type": SupportedDocumentTypes.ALL.value,
+            "type": str(SupportedDocumentTypes.ALL.value),
             "fields": {Fields.UPLOADING.value: True},
         }
     ]