PRMDR-477 refactor document reference search handler (#209)

* [PRMDR-477] refactor doc ref search lambda
---------

Co-authored-by: NogaNHS <noga.sasson1@nhs.net>

Makefile

@@ -22,8 +22,8 @@ clean-test:
 
 format:
 	./lambdas/venv/bin/python3 -m isort --profile black lambdas/
-	./lambdas/venv/bin/python3 -m black lambdas/ &&\
-	ruff check lambdas/ --fix
+	./lambdas/venv/bin/python3 -m black lambdas/
+	./lambdas/venv/bin/ruff check lambdas/ --fix
 
 sort-requirements:
 	sort -o lambdas/requirements.txt lambdas/requirements.txt

lambdas/handlers/document_reference_search_handler.py

@@ -1,13 +1,7 @@
 import json
-import os
-from json import JSONDecodeError
 
-from botocore.exceptions import ClientError
 from enums.logging_app_interaction import LoggingAppInteraction
-from enums.metadata_field_names import DocumentReferenceMetadataFields
-from models.document_reference import DocumentReference, DocumentReferenceSearchResult
-from pydantic import ValidationError
-from services.document_service import DocumentService
+from services.document_reference_search_service import DocumentReferenceSearchService
 from utils.audit_logging_setup import LoggingService
 from utils.decorators.ensure_env_var import ensure_environment_variables
 from utils.decorators.override_error_check import override_error_check
@@ -16,7 +10,7 @@
     extract_nhs_number_from_event,
     validate_patient_id,
 )
-from utils.exceptions import DynamoServiceException, InvalidResourceIdException
+from utils.lambda_exceptions import DocumentRefSearchException
 from utils.lambda_response import ApiGatewayResponse
 from utils.request_context import request_context
 
@@ -29,84 +23,28 @@
 @override_error_check
 def lambda_handler(event, context):
     request_context.app_interaction = LoggingAppInteraction.VIEW_PATIENT.value
-    nhs_number = extract_nhs_number_from_event(event)
-    request_context.patient_nhs_no = nhs_number
 
     logger.info("Starting document reference search process")
 
-    try:
-        list_of_table_names = json.loads(os.environ["DYNAMODB_TABLE_LIST"])
-    except JSONDecodeError as e:
-        logger.error(str(e), {"Result": f"Unsuccessful viewing docs due to {str(e)}"})
-        return ApiGatewayResponse(
-            500,
-            "An error occurred when parsing `DYNAMODB_TABLE_LIST` env variables",
-            "GET",
-        ).create_api_gateway_response()
+    nhs_number = extract_nhs_number_from_event(event)
+    request_context.patient_nhs_no = nhs_number
 
-    document_service = DocumentService()
+    document_reference_search_service = DocumentReferenceSearchService()
 
     try:
-        results: list[DocumentReference] = []
-        for table_name in list_of_table_names:
-            logger.info(f"Searching for results in {table_name}")
-            documents = document_service.fetch_documents_from_table_with_filter(
-                nhs_number,
-                table_name,
-                attr_filter={DocumentReferenceMetadataFields.DELETED.value: ""},
-            )
-
-            results += documents
-
-    except ValidationError as e:
-        logger.info(e, {"Result": f"Unsuccessful viewing docs due to {str(e)}"})
+        response = document_reference_search_service.get_document_references(nhs_number)
+        logger.info("User is able to view docs", {"Result": "Successful viewing docs"})
+        if response:
+            return ApiGatewayResponse(
+                200, json.dumps(response), "GET"
+            ).create_api_gateway_response()
+        else:
+            return ApiGatewayResponse(
+                204, json.dumps([]), "GET"
+            ).create_api_gateway_response()
+
+    except DocumentRefSearchException as e:
+        logger.error(e.message, {"Result": f"Search unsuccessful due to {e.message}"})
         return ApiGatewayResponse(
-            500,
-            "Failed to parse document reference from from DynamoDb response",
-            "GET",
+            e.status_code, e.message, "GET"
         ).create_api_gateway_response()
-    except InvalidResourceIdException:
-        return ApiGatewayResponse(
-            500, "No data was requested to be returned in query", "GET"
-        ).create_api_gateway_response()
-    except ClientError as e:
-        logger.error(
-            f"Unable to connect to DynamoDB: {str(e)}",
-            {"Result": f"Unsuccessful viewing docs due to {str(e)}"},
-        )
-        return ApiGatewayResponse(
-            500, "An error occurred when searching for available documents", "GET"
-        ).create_api_gateway_response()
-    except DynamoServiceException as e:
-        logger.error(
-            f"An error occurred when querying DynamoDB: {str(e)}",
-            {"Result": f"Unsuccessful viewing docs due to {str(e)}"},
-        )
-        return ApiGatewayResponse(
-            500,
-            "An error occurred when searching for available documents",
-            "GET",
-        ).create_api_gateway_response()
-
-    try:
-        response = [
-            dict(DocumentReferenceSearchResult.model_validate(dict(result)))
-            for result in results
-        ]
-    except ValidationError as e:
-        logger.info(e, {"Result": f"Unsuccessful viewing docs due to {str(e)}"})
-        return ApiGatewayResponse(
-            500,
-            "Failed to parse document reference search results",
-            "GET",
-        ).create_api_gateway_response()
-    logger.info("User is able to view docs", {"Result": "Successful viewing docs"})
-
-    if not response:
-        return ApiGatewayResponse(
-            204, json.dumps([]), "GET"
-        ).create_api_gateway_response()
-
-    return ApiGatewayResponse(
-        200, json.dumps(response), "GET"
-    ).create_api_gateway_response()

lambdas/models/document_reference.py

@@ -6,24 +6,24 @@
 from utils.exceptions import InvalidDocumentReferenceException
 
 
-class DocumentReferenceSearchResult(BaseModel):
-    created: str = Field(..., alias="created")
-    fileName: str = Field(..., alias="file_name")
-    virusScannerResult: str = Field(..., alias="virus_scanner_result")
-
-
 class DocumentReference(BaseModel):
     id: str = Field(..., alias=str(DocumentReferenceMetadataFields.ID.value))
     content_type: str = Field(
         ..., alias=str(DocumentReferenceMetadataFields.CONTENT_TYPE.value)
     )
-    created: str = Field(..., alias=str(DocumentReferenceMetadataFields.CREATED.value))
+    created: str = Field(
+        ...,
+        alias=str(DocumentReferenceMetadataFields.CREATED.value),
+        serialization_alias="created",
+    )
     deleted: str = Field(..., alias=str(DocumentReferenceMetadataFields.DELETED.value))
     file_location: str = Field(
         ..., alias=str(DocumentReferenceMetadataFields.FILE_LOCATION.value)
     )
     file_name: str = Field(
-        ..., alias=str(DocumentReferenceMetadataFields.FILE_NAME.value)
+        ...,
+        alias=str(DocumentReferenceMetadataFields.FILE_NAME.value),
+        serialization_alias="fileName",
     )
     nhs_number: str = Field(
         ..., alias=str(DocumentReferenceMetadataFields.NHS_NUMBER.value)
@@ -32,7 +32,9 @@ class DocumentReference(BaseModel):
         alias=str(DocumentReferenceMetadataFields.TTL.value), default=None
     )
     virus_scanner_result: str = Field(
-        ..., alias=str(DocumentReferenceMetadataFields.VIRUS_SCANNER_RESULT.value)
+        ...,
+        alias=str(DocumentReferenceMetadataFields.VIRUS_SCANNER_RESULT.value),
+        serialization_alias="virusScannerResult",
     )
 
     def get_file_name_path(self):

lambdas/services/document_reference_search_service.py

@@ -0,0 +1,52 @@
+import json
+import os
+from json import JSONDecodeError
+
+from botocore.exceptions import ClientError
+from enums.metadata_field_names import DocumentReferenceMetadataFields
+from models.document_reference import DocumentReference
+from pydantic import ValidationError
+from services.document_service import DocumentService
+from utils.audit_logging_setup import LoggingService
+from utils.exceptions import DynamoServiceException
+from utils.lambda_exceptions import DocumentRefSearchException
+
+logger = LoggingService(__name__)
+
+
+class DocumentReferenceSearchService(DocumentService):
+    def get_document_references(self, nhs_number: str):
+        try:
+            list_of_table_names = json.loads(os.environ["DYNAMODB_TABLE_LIST"])
+
+            results: list[dict] = []
+            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,
+                    attr_filter={DocumentReferenceMetadataFields.DELETED.value: ""},
+                )
+
+                results.extend(
+                    document.model_dump(
+                        include={"file_name", "created", "virus_scanner_result"},
+                        by_alias=True,
+                    )
+                    for document in documents
+                )
+            return results
+        except (
+            JSONDecodeError,
+            ValidationError,
+            ClientError,
+            DynamoServiceException,
+        ) as e:
+            logger.error(
+                f"An error occurred when using document reference search service: {str(e)}",
+            )
+            raise DocumentRefSearchException(
+                500, "An error occurred when searching for available documents"
+            )

lambdas/tests/unit/handlers/test_document_reference_search_handler.py

@@ -1,119 +1,64 @@
 import json
 
-from botocore.exceptions import ClientError
+import pytest
 from handlers.document_reference_search_handler import lambda_handler
-from tests.unit.helpers.data.dynamo_responses import (
-    EXPECTED_RESPONSE,
-    MOCK_EMPTY_RESPONSE,
-    MOCK_SEARCH_RESPONSE,
-)
-from utils.exceptions import DynamoServiceException, InvalidResourceIdException
+from tests.unit.helpers.data.dynamo_responses import EXPECTED_RESPONSE
+from utils.lambda_exceptions import DocumentRefSearchException
 from utils.lambda_response import ApiGatewayResponse
 
 
-def test_lambda_handler_returns_items_from_dynamo_returns_200(
-    set_env, valid_id_event_without_auth_header, context, mocker
-):
-    mock_dynamo = mocker.patch(
-        "services.base.dynamo_service.DynamoDBService.query_with_requested_fields"
+@pytest.fixture
+def mocked_service(set_env, mocker):
+    mocked_class = mocker.patch(
+        "handlers.document_reference_search_handler.DocumentReferenceSearchService"
     )
-    mock_dynamo.return_value = MOCK_SEARCH_RESPONSE
-
-    expected = ApiGatewayResponse(
-        200, json.dumps(EXPECTED_RESPONSE * 2), "GET"
-    ).create_api_gateway_response()
+    mocked_service = mocked_class.return_value
+    yield mocked_service
 
-    actual = lambda_handler(valid_id_event_without_auth_header, context)
-
-    assert expected == actual
 
-
-def test_lambda_handler_returns_items_from_doc_store_only_returns_200(
-    set_env, valid_id_event_without_auth_header, context, mocker
+def test_lambda_handler_returns_200(
+    mocked_service, valid_id_event_without_auth_header, context
 ):
-    mock_dynamo = mocker.patch(
-        "services.base.dynamo_service.DynamoDBService.query_with_requested_fields"
-    )
-    mock_dynamo.side_effect = [MOCK_SEARCH_RESPONSE, MOCK_EMPTY_RESPONSE]
+    mocked_service.get_document_references.return_value = EXPECTED_RESPONSE * 2
 
     expected = ApiGatewayResponse(
-        200, json.dumps(EXPECTED_RESPONSE), "GET"
+        200, json.dumps(EXPECTED_RESPONSE * 2), "GET"
     ).create_api_gateway_response()
 
     actual = lambda_handler(valid_id_event_without_auth_header, context)
 
     assert expected == actual
 
 
-def test_lambda_handler_when_dynamo_returns_no_records_returns_204(
-    set_env, valid_id_event_without_auth_header, context, mocker
+def test_lambda_handler_returns_204(
+    mocked_service, valid_id_event_without_auth_header, context
 ):
-    mock_dynamo = mocker.patch(
-        "services.base.dynamo_service.DynamoDBService.query_with_requested_fields"
-    )
-    mock_dynamo.return_value = MOCK_EMPTY_RESPONSE
-
-    expected = ApiGatewayResponse(204, "[]", "GET").create_api_gateway_response()
-
-    actual = lambda_handler(valid_id_event_without_auth_header, context)
-
-    assert expected == actual
-
-
-def test_lambda_handler_raises_dynamo_exception_returns_500(
-    set_env, valid_id_event_without_auth_header, context, mocker
-):
-    mock_dynamo = mocker.patch(
-        "services.base.dynamo_service.DynamoDBService.query_with_requested_fields"
-    )
-    mock_dynamo.side_effect = DynamoServiceException
+    mocked_service.get_document_references.return_value = []
 
     expected = ApiGatewayResponse(
-        500,
-        "An error occurred when searching for available documents",
-        "GET",
+        204, json.dumps([]), "GET"
     ).create_api_gateway_response()
+
     actual = lambda_handler(valid_id_event_without_auth_header, context)
+
     assert expected == actual
 
 
-def test_lambda_handler_raises_ClientError_returns_500(
-    set_env, valid_id_event_without_auth_header, context, mocker
+def test_lambda_handler_raises_exception_returns_500(
+    mocked_service, valid_id_event_without_auth_header, context
 ):
-    mock_dynamo = mocker.patch(
-        "services.base.dynamo_service.DynamoDBService.query_with_requested_fields"
-    )
-    mock_dynamo.return_value = MOCK_EMPTY_RESPONSE
-    mock_dynamo.side_effect = ClientError(
-        {"Error": {"Code": 500, "Message": "test error"}}, "testing"
+    mocked_service.get_document_references.side_effect = DocumentRefSearchException(
+        500, "test_string"
     )
-
     expected = ApiGatewayResponse(
         500,
-        "An error occurred when searching for available documents",
+        "test_string",
         "GET",
     ).create_api_gateway_response()
     actual = lambda_handler(valid_id_event_without_auth_header, context)
     assert expected == actual
 
 
-def test_lambda_handler_raises_InvalidResourceIdException_returns_500(
-    set_env, valid_id_event_without_auth_header, context, mocker
-):
-    exception = InvalidResourceIdException
-
-    mock_dynamo = mocker.patch(
-        "services.base.dynamo_service.DynamoDBService.query_with_requested_fields"
-    )
-    mock_dynamo.side_effect = exception
-
-    expected = ApiGatewayResponse(
-        500, "No data was requested to be returned in query", "GET"
-    ).create_api_gateway_response()
-    actual = lambda_handler(valid_id_event_without_auth_header, context)
-    assert expected == actual
-
-
 def test_lambda_handler_when_id_not_valid_returns_400(
     set_env, invalid_id_event, context
 ):

lambdas/tests/unit/services/base/test_dynamo_service.py

@@ -207,6 +207,21 @@ def test_create_item_is_called_with_correct_parameters(mock_service, mock_table)
     )
 
 
+def test_create_item_raise_client_error(mock_service, mock_table):
+    mock_service.create_item(MOCK_TABLE_NAME, {"NhsNumber": TEST_NHS_NUMBER})
+    mock_table.return_value.put_item.side_effect = MOCK_CLIENT_ERROR
+
+    mock_table.assert_called_with(MOCK_TABLE_NAME)
+    mock_table.return_value.put_item.assert_called_once_with(
+        Item={"NhsNumber": TEST_NHS_NUMBER}
+    )
+
+    with pytest.raises(ClientError) as actual_response:
+        mock_service.create_item(MOCK_TABLE_NAME, {"NhsNumber": TEST_NHS_NUMBER})
+
+    assert MOCK_CLIENT_ERROR == actual_response.value
+
+
 def test_delete_item_is_called_with_correct_parameters(mock_service, mock_table):
     mock_service.delete_item(MOCK_TABLE_NAME, {"NhsNumber": TEST_NHS_NUMBER})