diff --git a/.github/workflows/base-lambdas-reusable-deploy-all.yml b/.github/workflows/base-lambdas-reusable-deploy-all.yml index ee41d2cd3..04793ab16 100644 --- a/.github/workflows/base-lambdas-reusable-deploy-all.yml +++ b/.github/workflows/base-lambdas-reusable-deploy-all.yml @@ -488,6 +488,20 @@ jobs: secrets: AWS_ASSUME_ROLE: ${{ secrets.AWS_ASSUME_ROLE }} + deploy_get_document_review_lambda: + name: Deploy get document review lambda + uses: ./.github/workflows/base-lambdas-reusable-deploy.yml + with: + environment: ${{ inputs.environment}} + python_version: ${{ inputs.python_version }} + build_branch: ${{ inputs.build_branch}} + sandbox: ${{ inputs.sandbox }} + lambda_handler_name: get_document_review_handler + lambda_aws_name: GetDocumentReview + lambda_layer_names: "core_lambda_layer" + secrets: + AWS_ASSUME_ROLE: ${{ secrets.AWS_ASSUME_ROLE }} + deploy_edge_presign_lambda: name: Deploy edge presign cloudfront lambda uses: ./.github/workflows/base-lambdas-edge-deploy.yml diff --git a/lambdas/enums/logging_app_interaction.py b/lambdas/enums/logging_app_interaction.py index 23c3c1707..c1d7fc73f 100644 --- a/lambdas/enums/logging_app_interaction.py +++ b/lambdas/enums/logging_app_interaction.py @@ -19,3 +19,4 @@ class LoggingAppInteraction(Enum): VIRUS_SCAN = "Virus Scan" UPLOAD_CONFIRMATION = "Upload confirmation" UPDATE_UPLOAD_STATE = "Update upload state" + GET_REVIEW_DOCUMENTS = "Get review documents" diff --git a/lambdas/handlers/authoriser_handler.py b/lambdas/handlers/authoriser_handler.py index 3fef0e90f..8c7c6daad 100644 --- a/lambdas/handlers/authoriser_handler.py +++ b/lambdas/handlers/authoriser_handler.py @@ -41,7 +41,7 @@ def lambda_handler(event, context): auth_token = headers.get("authorization") or headers.get("Authorization") if event.get("methodArn") is None: return {"Error": "methodArn is not defined"} - _, _, _, region, aws_account_id, api_gateway_arn = event.get("methodArn").split(":") + _, _, _, region, aws_account_id, api_gateway_arn = event.get("methodArn").split(":", 5) api_id, stage, _http_verb, _resource_name = api_gateway_arn.split("/", 3) path = "/" + _resource_name diff --git a/lambdas/handlers/get_document_review_handler.py b/lambdas/handlers/get_document_review_handler.py new file mode 100644 index 000000000..13ed5621c --- /dev/null +++ b/lambdas/handlers/get_document_review_handler.py @@ -0,0 +1,95 @@ +import json + +from enums.feature_flags import FeatureFlags +from enums.lambda_error import LambdaError +from enums.logging_app_interaction import LoggingAppInteraction +from services.feature_flags_service import FeatureFlagService +from services.get_document_review_service import GetDocumentReviewService +from utils.audit_logging_setup import LoggingService +from utils.decorators.ensure_env_var import ensure_environment_variables +from utils.decorators.handle_lambda_exceptions import handle_lambda_exceptions +from utils.decorators.override_error_check import override_error_check +from utils.decorators.set_audit_arg import set_request_context_for_logging +from utils.decorators.validate_patient_id import validate_patient_id +from utils.lambda_exceptions import GetDocumentReviewException +from utils.lambda_response import ApiGatewayResponse +from utils.request_context import request_context + +logger = LoggingService(__name__) + + +@set_request_context_for_logging +@validate_patient_id +@ensure_environment_variables( + names=[ + "DOCUMENT_REVIEW_DYNAMODB_NAME", + "PRESIGNED_ASSUME_ROLE", + "EDGE_REFERENCE_TABLE", + "CLOUDFRONT_URL", + ] +) +@override_error_check +@handle_lambda_exceptions +def lambda_handler(event, context): + request_context.app_interaction = LoggingAppInteraction.GET_REVIEW_DOCUMENTS.value + + logger.info("Get Document Review handler has been triggered") + feature_flag_service = FeatureFlagService() + upload_lambda_enabled_flag_object = feature_flag_service.get_feature_flags_by_flag( + FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED + ) + + if not upload_lambda_enabled_flag_object[FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED]: + logger.info("Feature flag not enabled, event will not be processed") + raise GetDocumentReviewException(404, LambdaError.FeatureFlagDisabled) + + # Extract patient_id from query string parameters + query_params = event.get("queryStringParameters", {}) + patient_id = query_params.get("patientId", "") + + if not patient_id: + logger.error("Missing patient_id in query string parameters") + raise GetDocumentReviewException( + 400, LambdaError.DocumentReferenceMissingParameters + ) + + # Extract id from path parameters + path_params = event.get("pathParameters", {}) + document_id = path_params.get("id") + + if not document_id: + logger.error("Missing id in path parameters") + raise GetDocumentReviewException( + 400, LambdaError.DocumentReferenceMissingParameters + ) + + request_context.patient_nhs_no = patient_id + + logger.info( + f"Retrieving document review for patient_id: {patient_id}, document_id: {document_id}" + ) + + # Get document review service + document_review_service = GetDocumentReviewService() + document_review = document_review_service.get_document_review( + patient_id=patient_id, document_id=document_id + ) + + if document_review: + logger.info( + "Document review retrieved successfully", + {"Result": "Successful document review retrieval"}, + ) + return ApiGatewayResponse( + 200, json.dumps(document_review), "GET" + ).create_api_gateway_response() + else: + logger.error( + "Document review not found", + {"Result": "No document review available"}, + ) + return ApiGatewayResponse( + 404, + LambdaError.DocumentReferenceNotFound.create_error_body(), + "GET", + ).create_api_gateway_response() diff --git a/lambdas/models/document_review.py b/lambdas/models/document_review.py index 0d2c698fb..90cec0cfd 100644 --- a/lambdas/models/document_review.py +++ b/lambdas/models/document_review.py @@ -17,6 +17,7 @@ class DocumentReviewFileDetails(BaseModel): file_name: str file_location: str + presigned_url: str | None = None class DocumentUploadReviewReference(BaseModel): @@ -36,12 +37,12 @@ class DocumentUploadReviewReference(BaseModel): default=DocumentReviewStatus.PENDING_REVIEW ) review_reason: str - review_date: int = Field(default=None) - reviewer: str = Field(default=None) + review_date: int | None = Field(default=None) + reviewer: str | None = Field(default=None) upload_date: int - files: list[DocumentReviewFileDetails] + files: list[DocumentReviewFileDetails] = Field(min_length=1) nhs_number: str - ttl: Optional[int] = Field( + ttl: int | None = Field( alias=str(DocumentReferenceMetadataFields.TTL.value), default=None ) document_reference_id: str = Field(default=None) diff --git a/lambdas/services/authoriser_service.py b/lambdas/services/authoriser_service.py index ab9794b70..49f6aa376 100644 --- a/lambdas/services/authoriser_service.py +++ b/lambdas/services/authoriser_service.py @@ -121,6 +121,10 @@ def deny_access_policy(self, path, user_role, nhs_number: str = None): deny_resource = ( not patient_access_is_allowed or is_user_gp_clinical or is_user_pcse ) + case path if path.startswith("/DocumentReview/"): + deny_resource = ( + not patient_access_is_allowed + ) case "/UploadState": deny_resource = ( diff --git a/lambdas/services/document_service.py b/lambdas/services/document_service.py index 3fd420704..1dc1e257c 100644 --- a/lambdas/services/document_service.py +++ b/lambdas/services/document_service.py @@ -1,5 +1,6 @@ import os from datetime import datetime, timezone +from typing import Optional from boto3.dynamodb.conditions import Attr, ConditionBase from enums.metadata_field_names import DocumentReferenceMetadataFields @@ -104,12 +105,47 @@ def fetch_documents_from_table( continue return documents + def get_item( + self, + document_id: str, + table_name: str = None, + model_class: type[BaseModel] = None, + ) -> Optional[BaseModel]: + """Fetch a single document by ID from specified or configured table. + + Args: + document_id: The document ID to retrieve. + table_name: Optional table name, defaults to self.table_name. + model_class: Optional model class, defaults to self.model_class. + + Returns: + Document object if found, None otherwise. + """ + table_to_use = table_name or self.table_name + model_to_use = model_class or self.model_class + + try: + response = self.dynamo_service.get_item( + table_name=table_to_use, key={"ID": document_id} + ) + + if "Item" not in response: + logger.info(f"No document found for document_id: {document_id}") + return None + + document = model_to_use.model_validate(response["Item"]) + return document + + except ValidationError as e: + logger.error(f"Validation error on document: {response.get('Item')}") + logger.error(f"{e}") + return None + def get_nhs_numbers_based_on_ods_code( self, ods_code: str, table_name: str | None = None ) -> list[str]: """Get unique NHS numbers for patients with given ODS code.""" table_name = table_name or self.table_name - documents = self.fetch_documents_from_table( index_name="OdsCodeIndex", search_key=DocumentReferenceMetadataFields.CURRENT_GP_ODS.value, diff --git a/lambdas/services/get_document_review_service.py b/lambdas/services/get_document_review_service.py new file mode 100644 index 000000000..034b60975 --- /dev/null +++ b/lambdas/services/get_document_review_service.py @@ -0,0 +1,122 @@ +import os +import uuid +from datetime import datetime, timezone +from typing import Optional + +from enums.lambda_error import LambdaError +from services.base.s3_service import S3Service +from services.document_upload_review_service import DocumentUploadReviewService +from utils.audit_logging_setup import LoggingService +from utils.exceptions import DynamoServiceException +from utils.lambda_exceptions import GetDocumentReviewException +from utils.utilities import format_cloudfront_url + +logger = LoggingService(__name__) + + +class GetDocumentReviewService: + """ + Service for retrieving document reviews. + """ + + def __init__(self): + presigned_assume_role = os.getenv("PRESIGNED_ASSUME_ROLE") + self.s3_service = S3Service(custom_aws_role=presigned_assume_role) + self.document_review_service = DocumentUploadReviewService() + self.cloudfront_table_name = os.environ.get("EDGE_REFERENCE_TABLE") + self.cloudfront_url = os.environ.get("CLOUDFRONT_URL") + + def get_document_review(self, patient_id: str, document_id: str) -> Optional[dict]: + """Retrieve a document review for a given patient and document. + + Args: + patient_id: The patient ID (NHS number). + document_id: The document ID to retrieve. + + Returns: + Dictionary containing the document review details, or None if not found. + """ + try: + logger.info( + f"Fetching document review for patient_id: {patient_id}, document_id: {document_id}" + ) + + document_review_item = self.document_review_service.get_item(document_id) + + if not document_review_item: + logger.info(f"No document review found for document_id: {document_id}") + return None + + if document_review_item.nhs_number != patient_id: + logger.warning( + f"Document {document_id} does not belong to patient {patient_id}" + ) + return None + + if document_review_item.files: + for file_detail in document_review_item.files: + presigned_url = self.create_cloudfront_presigned_url( + file_detail.file_location + ) + file_detail.presigned_url = presigned_url + + document_review = document_review_item.model_dump( + by_alias=True, + include={ + "id": True, + "upload_date": True, + "files": {"__all__": {"file_name": True, "presigned_url": True}}, + "document_snomed_code_type": True, + }, + ) + + logger.info( + f"Successfully retrieved document review for document_id: {document_id}" + ) + + return document_review + + except DynamoServiceException as e: + logger.error( + f"{LambdaError.DocRefClient.to_str()}: {str(e)}", + {"Result": "Failed to retrieve document review"}, + ) + raise GetDocumentReviewException(500, LambdaError.DocRefClient) + except Exception as e: + logger.error( + f"Unexpected error retrieving document review: {str(e)}", + {"Result": "Failed to retrieve document review"}, + ) + raise GetDocumentReviewException(500, LambdaError.DocRefClient) + + def create_cloudfront_presigned_url(self, file_location: str) -> str: + """Create a CloudFront obfuscated pre-signed URL for a file. + + Args: + file_location: The S3 file key/location. + + Returns: + CloudFront URL that obfuscates the actual pre-signed URL. + """ + s3_bucket_name, file_key = file_location.removeprefix("s3://").split("/", 1) + presign_url_response = self.s3_service.create_download_presigned_url( + s3_bucket_name=s3_bucket_name, + file_key=file_key, + ) + + presigned_id = "review/" + str(uuid.uuid4()) + + deletion_date = datetime.now(timezone.utc) + ttl_half_an_hour_in_seconds = self.s3_service.presigned_url_expiry + dynamo_item_ttl = int(deletion_date.timestamp() + ttl_half_an_hour_in_seconds) + + self.document_review_service.dynamo_service.create_item( + self.cloudfront_table_name, + { + "ID": f"{presigned_id}", + "presignedUrl": presign_url_response, + "TTL": dynamo_item_ttl, + }, + ) + + return format_cloudfront_url(presigned_id, self.cloudfront_url) diff --git a/lambdas/tests/unit/conftest.py b/lambdas/tests/unit/conftest.py index 184a0a7d4..aa6a762ca 100644 --- a/lambdas/tests/unit/conftest.py +++ b/lambdas/tests/unit/conftest.py @@ -138,6 +138,7 @@ MOCK_ALERTING_SLACK_CHANNEL_ID = "slack_channel_id" MOCK_DOCUMENT_REVIEW_TABLE = "test_document_review" MOCK_DOCUMENT_REVIEW_BUCKET = "test_document_review_bucket" +MOCK_EDGE_TABLE = "test_edge_reference_table" @pytest.fixture def set_env(monkeypatch): @@ -228,10 +229,10 @@ def set_env(monkeypatch): monkeypatch.setenv("ITOC_TESTING_ODS_CODES", MOCK_ITOC_ODS_CODES) monkeypatch.setenv("DOCUMENT_REVIEW_DYNAMODB_NAME", MOCK_DOCUMENT_REVIEW_TABLE) monkeypatch.setenv("DOCUMENT_REVIEW_S3_BUCKET_NAME", MOCK_DOCUMENT_REVIEW_BUCKET) + monkeypatch.setenv("EDGE_REFERENCE_TABLE", MOCK_EDGE_TABLE) monkeypatch.setenv("STAGING_STORE_BUCKET_NAME", MOCK_STAGING_STORE_BUCKET) monkeypatch.setenv("METADATA_SQS_QUEUE_URL", MOCK_LG_METADATA_SQS_QUEUE) - EXPECTED_PARSED_PATIENT_BASE_CASE = PatientDetails( givenName=["Jane"], familyName="Smith", diff --git a/lambdas/tests/unit/handlers/conftest.py b/lambdas/tests/unit/handlers/conftest.py index e839fc4c9..f5f29ab3d 100755 --- a/lambdas/tests/unit/handlers/conftest.py +++ b/lambdas/tests/unit/handlers/conftest.py @@ -1,4 +1,5 @@ import pytest +from enums.feature_flags import FeatureFlags from services.feature_flags_service import FeatureFlagService @@ -138,3 +139,13 @@ def mock_validation_strict_disabled(mocker): "lloydGeorgeValidationStrictModeEnabled": False } yield mock_upload_lambda_feature_flag + + +@pytest.fixture +def mock_upload_document_iteration_3_enabled(mocker): + mock_function = mocker.patch.object(FeatureFlagService, "get_feature_flags_by_flag") + mock_feature_flag = mock_function.return_value = { + FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED: True + } + yield mock_feature_flag + diff --git a/lambdas/tests/unit/handlers/test_get_document_review_handler.py b/lambdas/tests/unit/handlers/test_get_document_review_handler.py new file mode 100644 index 000000000..145568817 --- /dev/null +++ b/lambdas/tests/unit/handlers/test_get_document_review_handler.py @@ -0,0 +1,175 @@ +import json + +import pytest +from enums.feature_flags import FeatureFlags +from enums.lambda_error import LambdaError +from handlers.get_document_review_handler import lambda_handler +from utils.lambda_exceptions import GetDocumentReviewException +from utils.lambda_response import ApiGatewayResponse + +MOCK_DOCUMENT_REVIEW_RESPONSE = { + "ID": "test-document-id", + "UploadDate": 1699000000, + "Files": [ + { + "FileName": "test-file-1.pdf", + "PresignedUrl": "https://mock-cloudfront-url.com/presigned1", + }, + { + "FileName": "test-file-2.pdf", + "PresignedUrl": "https://mock-cloudfront-url.com/presigned2", + }, + ], + "DocumentSnomedCodeType": "734163000", +} + + +@pytest.fixture +def valid_get_document_review_event(): + api_gateway_proxy_event = { + "httpMethod": "GET", + "queryStringParameters": {"patientId": "9000000009"}, + "pathParameters": {"id": "test-document-id"}, + } + return api_gateway_proxy_event + + +@pytest.fixture +def missing_patient_id_event(): + api_gateway_proxy_event = { + "httpMethod": "GET", + "queryStringParameters": {}, + "pathParameters": {"id": "test-document-id"}, + } + return api_gateway_proxy_event + + +@pytest.fixture +def missing_document_id_event(): + api_gateway_proxy_event = { + "httpMethod": "GET", + "queryStringParameters": {"patientId": "9000000009"}, + "pathParameters": {}, + } + return api_gateway_proxy_event + + +@pytest.fixture +def invalid_patient_id_event(): + api_gateway_proxy_event = { + "httpMethod": "GET", + "queryStringParameters": {"patientId": "900000000900"}, + "pathParameters": {"id": "test-document-id"}, + } + return api_gateway_proxy_event + + + +@pytest.fixture +def mocked_service(set_env, mocker): + mocked_class = mocker.patch( + "handlers.get_document_review_handler.GetDocumentReviewService" + ) + mocked_service = mocked_class.return_value + yield mocked_service + + +def test_lambda_handler_returns_200_with_document_review( + mocked_service, valid_get_document_review_event, context, set_env, mock_upload_document_iteration_3_enabled +): + mocked_service.get_document_review.return_value = MOCK_DOCUMENT_REVIEW_RESPONSE + + expected = ApiGatewayResponse( + 200, json.dumps(MOCK_DOCUMENT_REVIEW_RESPONSE), "GET" + ).create_api_gateway_response() + + actual = lambda_handler(valid_get_document_review_event, context) + + assert expected == actual + mocked_service.get_document_review.assert_called_once_with( + patient_id="9000000009", document_id="test-document-id" + ) + + +def test_lambda_handler_returns_404_when_document_review_not_found( + mocked_service, valid_get_document_review_event, context, mock_upload_document_iteration_3_enabled +): + mocked_service.get_document_review.return_value = None + + expected_body = json.dumps( + { + "message": "Document reference not found", + "err_code": "NRL_DR_4041", + "interaction_id": "88888888-4444-4444-4444-121212121212", + } + ) + expected = ApiGatewayResponse( + 404, expected_body, "GET" + ).create_api_gateway_response() + + actual = lambda_handler(valid_get_document_review_event, context) + + assert expected == actual + + +def test_lambda_handler_raises_exception_returns_500( + mocked_service, valid_get_document_review_event, context, mock_upload_document_iteration_3_enabled +): + mocked_service.get_document_review.side_effect = GetDocumentReviewException( + 500, LambdaError.MockError + ) + actual = lambda_handler(valid_get_document_review_event, context) + + expected = ApiGatewayResponse( + 500, + LambdaError.MockError.create_error_body(), + "GET", + ).create_api_gateway_response() + assert expected == actual + + +def test_lambda_handler_when_patient_id_not_valid_returns_400( + set_env, invalid_patient_id_event, context, mock_upload_document_iteration_3_enabled +): + expected_body = json.dumps( + { + "message": "Invalid patient number 900000000900", + "err_code": "PN_4001", + "interaction_id": "88888888-4444-4444-4444-121212121212", + } + ) + expected = ApiGatewayResponse( + 400, expected_body, "GET" + ).create_api_gateway_response() + actual = lambda_handler(invalid_patient_id_event, context) + assert expected == actual + + +def test_lambda_handler_when_document_id_not_supplied_returns_400( + set_env, missing_document_id_event, context, mock_upload_document_iteration_3_enabled +): + actual = lambda_handler(missing_document_id_event, context) + + expected = ApiGatewayResponse( + 400, LambdaError.DocumentReferenceMissingParameters.create_error_body(), "GET" + ).create_api_gateway_response() + assert expected == actual + + +def test_lambda_handler_missing_environment_variables_returns_500( + set_env, monkeypatch, valid_get_document_review_event, context, mock_upload_document_iteration_3_enabled +): + monkeypatch.delenv("DOCUMENT_REVIEW_DYNAMODB_NAME") + + expected_body = json.dumps( + { + "message": "An error occurred due to missing environment variable: 'DOCUMENT_REVIEW_DYNAMODB_NAME'", + "err_code": "ENV_5001", + "interaction_id": "88888888-4444-4444-4444-121212121212", + } + ) + expected = ApiGatewayResponse( + 500, expected_body, "GET" + ).create_api_gateway_response() + actual = lambda_handler(valid_get_document_review_event, context) + assert expected == actual diff --git a/lambdas/tests/unit/services/test_document_service.py b/lambdas/tests/unit/services/test_document_service.py index 5d5d97ddf..c92aee904 100644 --- a/lambdas/tests/unit/services/test_document_service.py +++ b/lambdas/tests/unit/services/test_document_service.py @@ -9,6 +9,7 @@ from enums.supported_document_types import SupportedDocumentTypes from freezegun import freeze_time from models.document_reference import DocumentReference +from models.document_review import DocumentUploadReviewReference from services.document_service import DocumentService from tests.unit.conftest import ( MOCK_ARF_TABLE_NAME, @@ -419,3 +420,122 @@ def test_get_batch_document_references_by_id_client_error( with pytest.raises(ClientError): mock_service.get_batch_document_references_by_id(document_ids, doc_type) + + +@pytest.mark.parametrize( + "document_id,table_name,file_name,expected_table", + [ + ("test-doc-id-123", None, "test-file.pdf", MOCK_LG_TABLE_NAME), + ("test-doc-id-456", MOCK_ARF_TABLE_NAME, "arf-file.pdf", MOCK_ARF_TABLE_NAME), + ], +) +def test_get_item_success( + mock_service, mock_dynamo_service, document_id, table_name, file_name, expected_table +): + """Test successful retrieval of a document with default or custom table.""" + mock_dynamo_response = { + "Item": { + "ID": document_id, + "NhsNumber": TEST_NHS_NUMBER, + "FileName": file_name, + "Created": "2023-01-01T00:00:00Z", + "Deleted": "", + "VirusScannerResult": "Clean", + } + } + + mock_dynamo_service.get_item.return_value = mock_dynamo_response + + if table_name: + result = mock_service.get_item(document_id, table_name=table_name) + else: + result = mock_service.get_item(document_id) + + mock_dynamo_service.get_item.assert_called_once_with( + table_name=expected_table, key={"ID": document_id} + ) + assert result is not None + assert isinstance(result, DocumentReference) + assert result.id == document_id + assert result.nhs_number == TEST_NHS_NUMBER + + +@pytest.mark.parametrize( + "document_id,mock_response,expected_log", + [ + ("non-existent-doc", {}, "No document found for document_id: non-existent-doc"), + ( + "invalid-doc", + {"Item": {"ID": "invalid-doc", "InvalidField": "invalid-value"}}, + "Validation error on document:", + ), + ], +) +def test_get_item_returns_none( + mock_service, mock_dynamo_service, caplog, document_id, mock_response, expected_log +): + """Test get_item returns None for not found or invalid documents.""" + mock_dynamo_service.get_item.return_value = mock_response + + result = mock_service.get_item(document_id) + + mock_dynamo_service.get_item.assert_called_once_with( + table_name=MOCK_LG_TABLE_NAME, key={"ID": document_id} + ) + assert result is None + assert expected_log in caplog.text + + +@pytest.mark.parametrize( + "document_id,table_name,file_name,expected_table", + [ + ("review-doc-id", None, "test.pdf", MOCK_LG_TABLE_NAME), + ("custom-review-doc", "custom-review-table", "custom.pdf", "custom-review-table"), + ], +) +def test_get_item_with_custom_model_class( + mock_service, mock_dynamo_service, document_id, table_name, file_name, expected_table +): + """Test retrieval using a custom model class with default or custom table.""" + + mock_dynamo_response = { + "Item": { + "ID": document_id, + "Author": "Y12345", + "Custodian": "Y12345", + "ReviewStatus": "PENDING_REVIEW", + "ReviewReason": "Test reason", + "UploadDate": 1699000000, + "Files": [ + { + "FileName": file_name, + "FileLocation": f"s3://bucket/{file_name}", + } + ], + "NhsNumber": TEST_NHS_NUMBER, + "DocumentSnomedCodeType": "734163000", + } + } + + mock_dynamo_service.get_item.return_value = mock_dynamo_response + + if table_name: + result = mock_service.get_item( + document_id, + table_name=table_name, + model_class=DocumentUploadReviewReference, + ) + else: + result = mock_service.get_item( + document_id, model_class=DocumentUploadReviewReference + ) + + mock_dynamo_service.get_item.assert_called_once_with( + table_name=expected_table, key={"ID": document_id} + ) + assert result is not None + assert isinstance(result, DocumentUploadReviewReference) + assert result.id == document_id + assert result.nhs_number == TEST_NHS_NUMBER + + diff --git a/lambdas/tests/unit/services/test_document_upload_review_service.py b/lambdas/tests/unit/services/test_document_upload_review_service.py index e399a6ee3..c4a0b53ce 100644 --- a/lambdas/tests/unit/services/test_document_upload_review_service.py +++ b/lambdas/tests/unit/services/test_document_upload_review_service.py @@ -3,7 +3,11 @@ import pytest from models.document_review import DocumentUploadReviewReference from services.document_upload_review_service import DocumentUploadReviewService -from tests.unit.conftest import TEST_NHS_NUMBER, MOCK_DOCUMENT_REVIEW_TABLE, MOCK_DOCUMENT_REVIEW_BUCKET +from tests.unit.conftest import ( + MOCK_DOCUMENT_REVIEW_BUCKET, + MOCK_DOCUMENT_REVIEW_TABLE, + TEST_NHS_NUMBER, +) TEST_ODS_CODE = "Y12345" NEW_ODS_CODE = "Z98765" diff --git a/lambdas/tests/unit/services/test_get_document_review_service.py b/lambdas/tests/unit/services/test_get_document_review_service.py new file mode 100644 index 000000000..53396790f --- /dev/null +++ b/lambdas/tests/unit/services/test_get_document_review_service.py @@ -0,0 +1,270 @@ +from unittest.mock import MagicMock + +import pytest +from enums.document_review_status import DocumentReviewStatus +from enums.lambda_error import LambdaError +from enums.snomed_codes import SnomedCodes +from freezegun import freeze_time +from models.document_review import ( + DocumentReviewFileDetails, + DocumentUploadReviewReference, +) +from services.get_document_review_service import GetDocumentReviewService +from tests.unit.conftest import ( + MOCK_DOCUMENT_REVIEW_BUCKET, + MOCK_EDGE_TABLE, + TEST_NHS_NUMBER, +) +from utils.exceptions import DynamoServiceException +from utils.lambda_exceptions import GetDocumentReviewException + +TEST_DOCUMENT_ID = "test-document-id-123" +TEST_DIFFERENT_NHS_NUMBER = "9000000010" +TEST_ODS_CODE = "Y12345" +TEST_FILE_LOCATION_1 = f"s3://{MOCK_DOCUMENT_REVIEW_BUCKET}/file1.pdf" +TEST_FILE_LOCATION_2 = f"s3://{MOCK_DOCUMENT_REVIEW_BUCKET}/file2.pdf" +TEST_PRESIGNED_URL_1 = "https://s3.amazonaws.com/presigned1?signature=abc123" +TEST_PRESIGNED_URL_2 = "https://s3.amazonaws.com/presigned2?signature=def456" +TEST_CLOUDFRONT_URL = "https://mock-cloudfront-url.com" + + +@pytest.fixture +def mock_service(set_env, mocker): + """Fixture to create a GetDocumentReviewService with mocked dependencies.""" + mocker.patch("services.get_document_review_service.S3Service") + mocker.patch("services.get_document_review_service.DocumentUploadReviewService") + + service = GetDocumentReviewService() + service.s3_service = MagicMock() + service.s3_service.presigned_url_expiry = 1800 # 30 minutes + service.document_review_service = MagicMock() + service.document_review_service.dynamo_service = MagicMock() + service.cloudfront_url = TEST_CLOUDFRONT_URL + + yield service + + +@pytest.fixture +def mock_document_review(): + """Create a mock document review reference.""" + files = [ + DocumentReviewFileDetails( + file_name="file1.pdf", + file_location=TEST_FILE_LOCATION_1, + ), + DocumentReviewFileDetails( + file_name="file2.pdf", + file_location=TEST_FILE_LOCATION_2, + ), + ] + + review = DocumentUploadReviewReference( + id=TEST_DOCUMENT_ID, + author=TEST_ODS_CODE, + custodian=TEST_ODS_CODE, + review_status=DocumentReviewStatus.PENDING_REVIEW, + review_reason="Uploaded for review", + upload_date=1699000000, + files=files, + nhs_number=TEST_NHS_NUMBER, + document_snomed_code_type=SnomedCodes.LLOYD_GEORGE.value.code, + ) + + return review + + +def test_get_document_review_success(mock_service, mock_document_review, mocker): + """Test successful retrieval of a document review with pre-signed URLs.""" + mock_service.document_review_service.get_item.return_value = mock_document_review + + mock_service.s3_service.create_download_presigned_url.side_effect = [ + TEST_PRESIGNED_URL_1, + TEST_PRESIGNED_URL_2, + ] + + mocker.patch( + "services.get_document_review_service.format_cloudfront_url", + side_effect=lambda presigned_id, url: f"{url}/{presigned_id}", + ) + + result = mock_service.get_document_review( + patient_id=TEST_NHS_NUMBER, document_id=TEST_DOCUMENT_ID + ) + assert result is not None + assert result["ID"] == TEST_DOCUMENT_ID + assert result["UploadDate"] == 1699000000 + assert result["DocumentSnomedCodeType"] == SnomedCodes.LLOYD_GEORGE.value.code + assert len(result["Files"]) == 2 + + assert "Author" not in result + assert "Custodian" not in result + assert "ReviewStatus" not in result + assert "NhsNumber" not in result + + assert result["Files"][0]["FileName"] == "file1.pdf" + assert result["Files"][0]["PresignedUrl"].startswith(TEST_CLOUDFRONT_URL) + assert result["Files"][1]["FileName"] == "file2.pdf" + assert result["Files"][1]["PresignedUrl"].startswith(TEST_CLOUDFRONT_URL) + + mock_service.document_review_service.get_item.assert_called_once_with( + TEST_DOCUMENT_ID + ) + + assert mock_service.s3_service.create_download_presigned_url.call_count == 2 + mock_service.s3_service.create_download_presigned_url.assert_any_call( + s3_bucket_name=MOCK_DOCUMENT_REVIEW_BUCKET, + file_key="file1.pdf", + ) + mock_service.s3_service.create_download_presigned_url.assert_any_call( + s3_bucket_name=MOCK_DOCUMENT_REVIEW_BUCKET, + file_key="file2.pdf", + ) + + assert ( + mock_service.document_review_service.dynamo_service.create_item.call_count == 2 + ) + + +def test_get_document_review_not_found(mock_service): + """Test when a document review is not found in DynamoDB.""" + mock_service.document_review_service.get_item.return_value = None + + result = mock_service.get_document_review( + patient_id=TEST_NHS_NUMBER, document_id=TEST_DOCUMENT_ID + ) + + assert result is None + mock_service.document_review_service.get_item.assert_called_once_with( + TEST_DOCUMENT_ID + ) + + +def test_get_document_review_nhs_number_mismatch(mock_service, mock_document_review): + """Test when document review exists but the NHS number doesn't match.""" + mock_service.document_review_service.get_item.return_value = mock_document_review + + result = mock_service.get_document_review( + patient_id=TEST_DIFFERENT_NHS_NUMBER, document_id=TEST_DOCUMENT_ID + ) + + assert result is None + mock_service.document_review_service.get_item.assert_called_once_with( + TEST_DOCUMENT_ID + ) + + mock_service.s3_service.create_download_presigned_url.assert_not_called() + + +def test_get_document_review_dynamo_service_exception(mock_service): + """Test handling of DynamoServiceException.""" + mock_service.document_review_service.get_item.side_effect = DynamoServiceException( + "DynamoDB error" + ) + + with pytest.raises(GetDocumentReviewException) as exc_info: + mock_service.get_document_review( + patient_id=TEST_NHS_NUMBER, document_id=TEST_DOCUMENT_ID + ) + + assert exc_info.value.status_code == 500 + assert exc_info.value.error == LambdaError.DocRefClient + + +def test_get_document_review_unexpected_exception(mock_service): + """Test handling of unexpected exceptions.""" + mock_service.document_review_service.get_item.side_effect = Exception( + "Unexpected error" + ) + + with pytest.raises(GetDocumentReviewException) as exc_info: + mock_service.get_document_review( + patient_id=TEST_NHS_NUMBER, document_id=TEST_DOCUMENT_ID + ) + + assert exc_info.value.status_code == 500 + assert exc_info.value.error == LambdaError.DocRefClient + + +@freeze_time("2023-11-03T12:00:00Z") +def test_create_cloudfront_presigned_url(mock_service, mock_uuid, mocker): + """Test creating a CloudFront presigned URL.""" + mock_service.s3_service.create_download_presigned_url.return_value = ( + TEST_PRESIGNED_URL_1 + ) + + mock_format_url = mocker.patch( + "services.get_document_review_service.format_cloudfront_url", + return_value=f"{TEST_CLOUDFRONT_URL}/review/{mock_uuid}", + ) + + result = mock_service.create_cloudfront_presigned_url(TEST_FILE_LOCATION_1) + + assert result == f"{TEST_CLOUDFRONT_URL}/review/{mock_uuid}" + + mock_service.s3_service.create_download_presigned_url.assert_called_once_with( + s3_bucket_name=MOCK_DOCUMENT_REVIEW_BUCKET, + file_key="file1.pdf", + ) + + mock_service.document_review_service.dynamo_service.create_item.assert_called_once() + call_args = ( + mock_service.document_review_service.dynamo_service.create_item.call_args + ) + assert call_args[0][0] == MOCK_EDGE_TABLE + assert call_args[0][1]["ID"] == f"review/{mock_uuid}" + assert call_args[0][1]["presignedUrl"] == TEST_PRESIGNED_URL_1 + assert "TTL" in call_args[0][1] + + mock_format_url.assert_called_once_with(f"review/{mock_uuid}", TEST_CLOUDFRONT_URL) + + +@freeze_time("2023-11-03T12:00:00Z") +def test_create_cloudfront_presigned_url_with_nested_path( + mock_service, mock_uuid, mocker +): + """Test creating a CloudFront presigned URL with a nested S3 path.""" + file_location = f"s3://{MOCK_DOCUMENT_REVIEW_BUCKET}/nested/path/to/file.pdf" + mock_service.s3_service.create_download_presigned_url.return_value = ( + TEST_PRESIGNED_URL_1 + ) + + mocker.patch( + "services.get_document_review_service.format_cloudfront_url", + return_value=f"{TEST_CLOUDFRONT_URL}/review/{mock_uuid}", + ) + + result = mock_service.create_cloudfront_presigned_url(file_location) + + assert result == f"{TEST_CLOUDFRONT_URL}/review/{mock_uuid}" + + mock_service.s3_service.create_download_presigned_url.assert_called_once_with( + s3_bucket_name=MOCK_DOCUMENT_REVIEW_BUCKET, + file_key="nested/path/to/file.pdf", + ) + + +@freeze_time("2023-11-03T12:00:00Z") +def test_create_cloudfront_presigned_url_calculates_correct_ttl( + mock_service, mock_uuid, mocker +): + """Test that TTL is calculated correctly based on presigned URL expiry.""" + mock_service.s3_service.create_download_presigned_url.return_value = ( + TEST_PRESIGNED_URL_1 + ) + mock_service.s3_service.presigned_url_expiry = 3600 # 1 hour + mocker.patch( + "services.get_document_review_service.uuid.uuid4", return_value=mock_uuid + ) + + mocker.patch( + "services.get_document_review_service.format_cloudfront_url", + return_value=f"{TEST_CLOUDFRONT_URL}/review/{mock_uuid}", + ) + + mock_service.create_cloudfront_presigned_url(TEST_FILE_LOCATION_1) + + call_args = ( + mock_service.document_review_service.dynamo_service.create_item.call_args + ) + expected_ttl = 1699012800 + 3600 # frozen time timestamp + 1 hour + assert call_args[0][1]["TTL"] == expected_ttl diff --git a/lambdas/utils/lambda_exceptions.py b/lambdas/utils/lambda_exceptions.py index 7f8442c56..cc55d6c10 100644 --- a/lambdas/utils/lambda_exceptions.py +++ b/lambdas/utils/lambda_exceptions.py @@ -103,3 +103,8 @@ class PdfStitchingException(LambdaException): class UpdateFhirDocumentReferenceException(LambdaException): pass + + +class GetDocumentReviewException(LambdaException): + pass +