diff --git a/apim/specification.yaml b/apim/specification.yaml index edc13fccd4..46e0856dfb 100644 --- a/apim/specification.yaml +++ b/apim/specification.yaml @@ -605,7 +605,7 @@ components: - DocumentReference id: type: string - pattern: '16521000000101~[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[0-9a-f]{4}-[0-9a-f]{12}' + pattern: '[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[0-9a-f]{4}-[0-9a-f]{12}' description: 'The logical id of the resource, as used in the URL for the resource. Once assigned, this value never changes.' meta: $ref: '#/components/schemas/Meta' @@ -1198,7 +1198,7 @@ components: - div DocumentId: type: string - pattern: '16521000000101~[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[0-9a-f]{4}-[0-9a-f]{12}' + pattern: '[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[0-9a-f]{4}-[0-9a-f]{12}' RequestPathParams: type: object properties: @@ -1269,7 +1269,7 @@ components: examples: valid: summary: Valid ID, document held, user has correct auth (200) - value: 16521000000101~f9ed81db-f90a-42d4-b7e4-d554d8f338fd + value: f9ed81db-f90a-42d4-b7e4-d554d8f338fd invalid_400: summary: Invalid request, no ID present (400) value: 400 diff --git a/lambdas/handlers/get_fhir_document_reference_handler.py b/lambdas/handlers/get_fhir_document_reference_handler.py index 25df6ba44f..c9bf26a2eb 100644 --- a/lambdas/handlers/get_fhir_document_reference_handler.py +++ b/lambdas/handlers/get_fhir_document_reference_handler.py @@ -1,4 +1,6 @@ from enums.lambda_error import LambdaError +from enums.mtls import MtlsCommonNames +from enums.snomed_codes import SnomedCode, SnomedCodes from oauthlib.oauth2 import WebApplicationClient from services.base.ssm_service import SSMService from services.dynamic_configuration_service import DynamicConfigurationService @@ -15,6 +17,7 @@ SearchPatientException, ) from utils.lambda_handler_utils import extract_bearer_token +from utils.lambda_header_utils import validate_common_name_in_mtls from utils.lambda_response import ApiGatewayResponse logger = LoggingService(__name__) @@ -29,44 +32,52 @@ "APPCONFIG_ENVIRONMENT", "PRESIGNED_ASSUME_ROLE", "CLOUDFRONT_URL", - ] + ], ) def lambda_handler(event, context): try: + common_name = validate_common_name_in_mtls(event.get("requestContext")) bearer_token = extract_bearer_token(event, context) selected_role_id = event.get("headers", {}).get("cis2-urid", None) - document_id, snomed_code = extract_document_parameters(event) + snomed_code = _determine_document_type(common_name=common_name) + + document_id = extract_document_parameters(event) get_document_service = GetFhirDocumentReferenceService() document_reference = get_document_service.handle_get_document_reference_request( - snomed_code, document_id + snomed_code.code, + document_id, ) if selected_role_id and bearer_token: verify_user_authorisation( - bearer_token, selected_role_id, document_reference.nhs_number + bearer_token, + selected_role_id, + document_reference.nhs_number, ) document_reference_response = ( get_document_service.create_document_reference_fhir_response( - document_reference + document_reference, ) ) logger.info( - f"Successfully retrieved document reference for document_id: {document_id}, snomed_code: {snomed_code}" + f"Successfully retrieved document reference for document_id: {document_id}, snomed_code: {snomed_code}", ) return ApiGatewayResponse( - status_code=200, body=document_reference_response, methods="GET" + status_code=200, + body=document_reference_response, + methods="GET", ).create_api_gateway_response() except GetFhirDocumentReferenceException as exception: return ApiGatewayResponse( status_code=exception.status_code, body=exception.error.create_error_response().create_error_fhir_response( - exception.error.value.get("fhir_coding") + exception.error.value.get("fhir_coding"), ), methods="GET", ).create_api_gateway_response() @@ -74,16 +85,16 @@ def lambda_handler(event, context): def extract_document_parameters(event): """Extract document ID and SNOMED code from path parameters""" - path_params = event.get("pathParameters", {}).get("id", None) - document_id, snomed_code = get_id_and_snomed_from_path_parameters(path_params) + document_id = event.get("pathParameters", {}).get("id", None) - if not document_id or not snomed_code: - logger.error("Missing document id or snomed code in request path parameters.") + if not document_id: + logger.error("Missing document id in request path parameters.") raise GetFhirDocumentReferenceException( - 400, LambdaError.DocumentReferenceMissingParameters + 400, + LambdaError.DocumentReferenceMissingParameters, ) - return document_id, snomed_code + return document_id def verify_user_authorisation(bearer_token, selected_role_id, nhs_number): @@ -100,27 +111,33 @@ def verify_user_authorisation(bearer_token, selected_role_id, nhs_number): org_ods_code = oidc_service.fetch_user_org_code(userinfo, selected_role_id) smartcard_role_code, _ = oidc_service.fetch_user_role_code( - userinfo, selected_role_id, "R" + userinfo, + selected_role_id, + "R", ) except (OidcApiException, AuthorisationException) as e: logger.error(f"Authorization error: {str(e)}") raise GetFhirDocumentReferenceException( - 403, LambdaError.DocumentReferenceUnauthorised + 403, + LambdaError.DocumentReferenceUnauthorised, ) try: search_patient_service = SearchPatientDetailsService( - smartcard_role_code, org_ods_code + smartcard_role_code, + org_ods_code, ) search_patient_service.handle_search_patient_request(nhs_number, False) except SearchPatientException as e: raise GetFhirDocumentReferenceException(e.status_code, e.error) -def get_id_and_snomed_from_path_parameters(path_parameters): - """Extract document ID and SNOMED code from path parameters""" - if path_parameters: - params = path_parameters.split("~") - if len(params) == 2: - return params[1], params[0] - return None, None +def _determine_document_type(common_name: MtlsCommonNames | None) -> SnomedCode: + if not common_name: + return SnomedCodes.LLOYD_GEORGE.value + + if common_name not in MtlsCommonNames: + logger.error(f"mTLS common name {common_name} - is not supported") + raise GetFhirDocumentReferenceException(400, LambdaError.DocRefInvalidType) + + return SnomedCodes.PATIENT_DATA.value diff --git a/lambdas/models/fhir/R4/fhir_document_reference.py b/lambdas/models/fhir/R4/fhir_document_reference.py index cf86f8d700..cbad417c20 100644 --- a/lambdas/models/fhir/R4/fhir_document_reference.py +++ b/lambdas/models/fhir/R4/fhir_document_reference.py @@ -64,7 +64,7 @@ class ContentStabilityExtensionValueCodeableConcept(CodeableConcept): """CodeableConcept for content stability.""" coding: List[ContentStabilityExtensionCoding] = Field( - default_factory=lambda: [ContentStabilityExtensionCoding()] + default_factory=lambda: [ContentStabilityExtensionCoding()], ) @@ -73,7 +73,7 @@ class ContentStabilityExtension(Extension): url: Literal[CONTENT_STABILITY_URL] = CONTENT_STABILITY_URL valueCodeableConcept: ContentStabilityExtensionValueCodeableConcept = Field( - default_factory=ContentStabilityExtensionValueCodeableConcept + default_factory=ContentStabilityExtensionValueCodeableConcept, ) @@ -136,8 +136,7 @@ def extract_nhs_number_from_fhir(self) -> str: and self.subject.identifier.system == "https://fhir.nhs.uk/Id/nhs-number" ): return self.subject.identifier.value - else: - raise FhirDocumentReferenceException("NHS number was not found") + raise FhirDocumentReferenceException("NHS number was not found") class DocumentReferenceInfo(BaseModel): @@ -167,7 +166,7 @@ def _create_identifier(self, system_suffix: str, value: str) -> Dict[str, Any]: "identifier": { "system": f"{FHIR_BASE_URL}/{system_suffix}", "value": value, - } + }, } def _create_snomed_coding(self, snomed_code: SnomedCode) -> List[Dict[str, str]]: @@ -184,7 +183,7 @@ def _create_snomed_coding(self, snomed_code: SnomedCode) -> List[Dict[str, str]] "system": SNOMED_URL, "code": snomed_code.code, "display": snomed_code.display_name, - } + }, ] def create_nrl_fhir_document_reference_object(self) -> DocumentReference: @@ -204,25 +203,27 @@ def create_nrl_fhir_document_reference_object(self) -> DocumentReference: subject=Reference(**self._create_identifier("nhs-number", self.nhs_number)), content=[DocumentReferenceContent(attachment=self.attachment)], custodian=Reference( - **self._create_identifier("ods-organization-code", self.custodian) + **self._create_identifier("ods-organization-code", self.custodian), ), type=CodeableConcept( - coding=self._create_snomed_coding(self.snomed_code_doc_type) + coding=self._create_snomed_coding(self.snomed_code_doc_type), ), category=[ CodeableConcept( - coding=self._create_snomed_coding(self.snomed_code_category) - ) + coding=self._create_snomed_coding(self.snomed_code_category), + ), ], author=[ Reference( - **self._create_identifier("ods-organization-code", self.custodian) - ) + **self._create_identifier("ods-organization-code", self.custodian), + ), ], context=DocumentReferenceContext( practiceSetting=CodeableConcept( - coding=self._create_snomed_coding(self.snomed_code_practice_setting) - ) + coding=self._create_snomed_coding( + self.snomed_code_practice_setting, + ), + ), ), ) @@ -233,7 +234,8 @@ def create_nrl_fhir_document_reference_object(self) -> DocumentReference: return fhir_document_ref def create_fhir_document_reference_object( - self, document: NdrDocumentReference + self, + document: NdrDocumentReference, ) -> DocumentReference: """Create a FHIR DocumentReference . @@ -245,10 +247,10 @@ def create_fhir_document_reference_object( return DocumentReference( resourceType="DocumentReference", - id=f"{self.snomed_code_doc_type.code}~{document.id}", + id=document.id, docStatus=document.doc_status, type=CodeableConcept( - coding=self._create_snomed_coding(self.snomed_code_doc_type) + coding=self._create_snomed_coding(self.snomed_code_doc_type), ), subject=Reference(**self._create_identifier("nhs-number", self.nhs_number)), content=[DocumentReferenceContent(attachment=self.attachment)], @@ -256,35 +258,40 @@ def create_fhir_document_reference_object( author=[ Reference( **self._create_identifier( - "ods-organization-code", document.author or self.custodian - ) - ) + "ods-organization-code", + document.author or self.custodian, + ), + ), ], custodian=Reference( **self._create_identifier( - "ods-organization-code", document.custodian or self.custodian - ) + "ods-organization-code", + document.custodian or self.custodian, + ), ), meta=Meta(versionId=document.version), ) def create_fhir_document_reference_object_basic( - self, original_id: str, original_version + self, + original_id: str, + original_version, ) -> DocumentReference: return DocumentReference( resourceType="DocumentReference", id=f"{original_id}", type=CodeableConcept( - coding=self._create_snomed_coding(self.snomed_code_doc_type) + coding=self._create_snomed_coding(self.snomed_code_doc_type), ), subject=Reference(**self._create_identifier("nhs-number", self.nhs_number)), content=[DocumentReferenceContent(attachment=self.attachment)], author=[ Reference( **self._create_identifier( - "ods-organization-code", self.author or self.custodian - ) - ) + "ods-organization-code", + self.author or self.custodian, + ), + ), ], meta=Meta(versionId=original_version), ) diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index f53ddc07a9..bfa386f755 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -229,11 +229,7 @@ def create_document_reference_fhir_response( title=document_reference.file_name, creation=document_reference.document_scan_creation or document_reference.created, - url=document_retrieve_endpoint - + "/" - + document_reference.document_snomed_code_type - + "~" - + document_reference.id, + url=document_retrieve_endpoint + "/" + document_reference.id, ) fhir_document_reference = ( DocumentReferenceInfo( diff --git a/lambdas/services/fhir_document_reference_service_base.py b/lambdas/services/fhir_document_reference_service_base.py index 1ae935e126..844c63474c 100644 --- a/lambdas/services/fhir_document_reference_service_base.py +++ b/lambdas/services/fhir_document_reference_service_base.py @@ -8,11 +8,16 @@ from enums.patient_ods_inactive_status import PatientOdsInactiveStatus from enums.snomed_codes import SnomedCode, SnomedCodes from models.document_reference import DocumentReference -from models.fhir.R4.fhir_document_reference import SNOMED_URL, Attachment +from models.fhir.R4.fhir_document_reference import ( + SNOMED_URL, + Attachment, +) from models.fhir.R4.fhir_document_reference import ( DocumentReference as FhirDocumentReference, ) -from models.fhir.R4.fhir_document_reference import DocumentReferenceInfo +from models.fhir.R4.fhir_document_reference import ( + DocumentReferenceInfo, +) from models.pds_models import PatientDetails from services.base.dynamo_service import DynamoDBService from services.base.s3_service import S3Service @@ -43,7 +48,9 @@ def __init__(self): self.doc_router = DocTypeTableRouter() def _store_binary_in_s3( - self, document_reference: DocumentReference, binary_content: bytes + self, + document_reference: DocumentReference, + binary_content: bytes, ) -> None: """Store binary content in S3""" try: @@ -54,7 +61,7 @@ def _store_binary_in_s3( file_key=document_reference.s3_upload_key, ) logger.info( - f"Successfully stored binary content in S3: {document_reference.s3_upload_key}" + f"Successfully stored binary content in S3: {document_reference.s3_upload_key}", ) except (binascii.Error, ValueError) as e: logger.error(f"Failed to decode base64: {str(e)}") @@ -65,28 +72,29 @@ def _store_binary_in_s3( except ClientError as e: logger.error(f"Failed to store binary in S3: {str(e)}") raise FhirDocumentReferenceException( - f"Failed to store binary in S3: {str(e)}" + f"Failed to store binary in S3: {str(e)}", ) except (OSError, IOError) as e: logger.error(f"I/O error when processing binary content: {str(e)}") raise FhirDocumentReferenceException( - f"I/O error when processing binary content: {str(e)}" + f"I/O error when processing binary content: {str(e)}", ) def _create_s3_presigned_url(self, document_reference: DocumentReference) -> str: """Create a pre-signed URL for uploading a file""" try: response = self.s3_service.create_put_presigned_url( - document_reference.s3_bucket_name, document_reference.s3_upload_key + document_reference.s3_bucket_name, + document_reference.s3_upload_key, ) logger.info( - f"Successfully created pre-signed URL for {document_reference.s3_upload_key}" + f"Successfully created pre-signed URL for {document_reference.s3_upload_key}", ) return response except ClientError as e: logger.error(f"Failed to create pre-signed URL: {str(e)}") raise FhirDocumentReferenceException( - f"Failed to create pre-signed URL: {str(e)}" + f"Failed to create pre-signed URL: {str(e)}", ) def _create_document_reference( @@ -138,10 +146,9 @@ def _get_document_reference(self, document_id: str, table) -> DocumentReference: if len(documents) > 0: logger.info("Document found for given id") return documents[0] - else: - raise FhirDocumentReferenceException( - f"Did not find any documents for document ID {document_id}" - ) + raise FhirDocumentReferenceException( + f"Did not find any documents for document ID {document_id}", + ) def _determine_document_type(self, fhir_doc: FhirDocumentReference) -> SnomedCode: """Determine the document type based on SNOMED code in the FHIR document""" @@ -154,7 +161,9 @@ def _determine_document_type(self, fhir_doc: FhirDocumentReference) -> SnomedCod raise FhirDocumentReferenceException("SNOMED code not found in FHIR document") def _save_document_reference_to_dynamo( - self, table_name: str, document_reference: DocumentReference + self, + table_name: str, + document_reference: DocumentReference, ) -> None: """Save document reference to DynamoDB""" try: @@ -166,7 +175,7 @@ def _save_document_reference_to_dynamo( except ClientError as e: logger.error(f"Failed to create document reference: {str(e)}") raise FhirDocumentReferenceException( - f"Failed to create document reference: {str(e)}" + f"Failed to create document reference: {str(e)}", ) def _check_nhs_number_with_pds(self, nhs_number: str) -> PatientDetails: @@ -181,7 +190,7 @@ def _check_nhs_number_with_pds(self, nhs_number: str) -> PatientDetails: ) as e: logger.error(f"Error occurred when fetching patient details: {str(e)}") raise FhirDocumentReferenceException( - f"Error occurred when fetching patient details: {str(e)}" + f"Error occurred when fetching patient details: {str(e)}", ) def _create_fhir_response( @@ -195,14 +204,11 @@ def _create_fhir_response( attachment_url = presigned_url else: document_retrieve_endpoint = os.getenv( - "DOCUMENT_RETRIEVE_ENDPOINT_APIM", "" + "DOCUMENT_RETRIEVE_ENDPOINT_APIM", + "", ) attachment_url = ( - document_retrieve_endpoint - + "/" - + document_reference_ndr.document_snomed_code_type - + "~" - + document_reference_ndr.id + document_retrieve_endpoint + "/" + document_reference_ndr.id ) document_details = Attachment( title=document_reference_ndr.file_name, @@ -216,7 +222,7 @@ def _create_fhir_response( attachment=document_details, custodian=document_reference_ndr.custodian, snomed_code_doc_type=SnomedCodes.find_by_code( - document_reference_ndr.document_snomed_code_type + document_reference_ndr.document_snomed_code_type, ), ) .create_fhir_document_reference_object(document_reference_ndr) diff --git a/lambdas/tests/e2e/api/fhir/conftest.py b/lambdas/tests/e2e/api/fhir/conftest.py index 989ae5aa37..c06b3569b4 100644 --- a/lambdas/tests/e2e/api/fhir/conftest.py +++ b/lambdas/tests/e2e/api/fhir/conftest.py @@ -27,7 +27,12 @@ def test_data(): def fetch_with_retry_mtls( - session, url, headers, condition_func=None, max_retries=5, delay=10 + session, + url, + headers, + condition_func=None, + max_retries=5, + delay=10, ): retries = 0 while retries < max_retries: @@ -47,7 +52,8 @@ def fetch_with_retry_mtls( def create_mtls_session( - client_cert_path=CLIENT_CERT_PATH, client_key_path=CLIENT_KEY_PATH + client_cert_path=CLIENT_CERT_PATH, + client_key_path=CLIENT_KEY_PATH, ): session = requests.Session() session.cert = (client_cert_path, client_key_path) @@ -95,7 +101,7 @@ def get_pdm_document_reference( endpoint_override=None, ): if not endpoint_override: - url = f"https://{MTLS_ENDPOINT}/{resource_type}/{pdm_snomed}~{record_id}" + url = f"https://{MTLS_ENDPOINT}/{resource_type}/{record_id}" else: url = f"https://{MTLS_ENDPOINT}/{resource_type}/{endpoint_override}" headers = { @@ -138,7 +144,9 @@ def create_and_store_pdm_record( ): """Helper to create metadata and resource for a record.""" record = pdm_data_helper.build_record( - nhs_number=nhs_number, doc_status=doc_status, size=size + nhs_number=nhs_number, + doc_status=doc_status, + size=size, ) test_data.append(record) pdm_data_helper.create_metadata(record, **dynamo_kwargs) diff --git a/lambdas/tests/e2e/api/fhir/test_delete_document_reference_fhir_api_success.py b/lambdas/tests/e2e/api/fhir/test_delete_document_reference_fhir_api_success.py index 75d7e38e3e..9926eefd70 100644 --- a/lambdas/tests/e2e/api/fhir/test_delete_document_reference_fhir_api_success.py +++ b/lambdas/tests/e2e/api/fhir/test_delete_document_reference_fhir_api_success.py @@ -1,7 +1,7 @@ from tests.e2e.api.fhir.conftest import ( create_and_store_pdm_record, - get_pdm_document_reference, delete_document_reference, + get_pdm_document_reference, ) from tests.e2e.helpers.data_helper import PdmDataHelper @@ -12,15 +12,15 @@ def test_delete_record_by_patient_details_and_doc_id(test_data): created_record = create_and_store_pdm_record(test_data) expected_record_id = created_record["id"] - get_response_1 = get_pdm_document_reference(expected_record_id) + get_response_1 = get_pdm_document_reference(record_id=expected_record_id) assert get_response_1.status_code == 200 response = delete_document_reference( - f"?subject:identifier=https://fhir.nhs.uk/Id/nhs-number|9912003071&_id={expected_record_id}" + f"?subject:identifier=https://fhir.nhs.uk/Id/nhs-number|9912003071&_id={expected_record_id}", ) assert response.status_code == 204 - get_response = get_pdm_document_reference(expected_record_id) + get_response = get_pdm_document_reference(record_id=expected_record_id) assert get_response.status_code == 404 @@ -31,19 +31,19 @@ def test_delete_only_one_record_by_patient_details_and_doc_id(test_data): created_record_2 = create_and_store_pdm_record(test_data) expected_record_id_2 = created_record_2["id"] - get_response_1 = get_pdm_document_reference(expected_record_id_1) + get_response_1 = get_pdm_document_reference(record_id=expected_record_id_1) assert get_response_1.status_code == 200 - get_response_2 = get_pdm_document_reference(expected_record_id_2) + get_response_2 = get_pdm_document_reference(record_id=expected_record_id_2) assert get_response_2.status_code == 200 response = delete_document_reference( - f"?subject:identifier=https://fhir.nhs.uk/Id/nhs-number|9912003071&_id={expected_record_id_1}" + f"?subject:identifier=https://fhir.nhs.uk/Id/nhs-number|9912003071&_id={expected_record_id_1}", ) assert response.status_code == 204 - get_response_1_deleted = get_pdm_document_reference(expected_record_id_1) + get_response_1_deleted = get_pdm_document_reference(record_id=expected_record_id_1) assert get_response_1_deleted.status_code == 404 - get_response_2_deleted = get_pdm_document_reference(expected_record_id_2) + get_response_2_deleted = get_pdm_document_reference(record_id=expected_record_id_2) assert get_response_2_deleted.status_code == 200 diff --git a/lambdas/tests/e2e/api/fhir/test_retrieve_document_fhir_api_failure.py b/lambdas/tests/e2e/api/fhir/test_retrieve_document_fhir_api_failure.py index e1cfebe33a..0f5ff733de 100644 --- a/lambdas/tests/e2e/api/fhir/test_retrieve_document_fhir_api_failure.py +++ b/lambdas/tests/e2e/api/fhir/test_retrieve_document_fhir_api_failure.py @@ -102,11 +102,31 @@ def test_retrieve_invalid_resource_type(test_data): @pytest.mark.parametrize( "param,expected_status,expected_code", [ - (f"{pdm_data_helper.snomed_code}-{str(uuid.uuid4())}", 400, "MISSING_VALUE"), - (f"{pdm_data_helper.snomed_code}+{str(uuid.uuid4())}", 400, "MISSING_VALUE"), - (f"{pdm_data_helper.snomed_code}&{str(uuid.uuid4())}", 400, "MISSING_VALUE"), - (f"{pdm_data_helper.snomed_code}{str(uuid.uuid4())}", 400, "MISSING_VALUE"), - (f"{str(uuid.uuid4())}~{pdm_data_helper.snomed_code}", 500, "exception"), + ( + f"{pdm_data_helper.snomed_code}-{str(uuid.uuid4())}", + 404, + "RESOURCE_NOT_FOUND", + ), + ( + f"{pdm_data_helper.snomed_code}+{str(uuid.uuid4())}", + 404, + "RESOURCE_NOT_FOUND", + ), + ( + f"{pdm_data_helper.snomed_code}&{str(uuid.uuid4())}", + 404, + "RESOURCE_NOT_FOUND", + ), + ( + f"{pdm_data_helper.snomed_code}{str(uuid.uuid4())}", + 404, + "RESOURCE_NOT_FOUND", + ), + ( + f"{str(uuid.uuid4())}~{pdm_data_helper.snomed_code}", + 404, + "RESOURCE_NOT_FOUND", + ), ], ) def test_incorrectly_formatted_path_param_id( @@ -124,18 +144,6 @@ def test_incorrectly_formatted_path_param_id( _assert_operation_outcome(body=body, code=expected_code) -def test_no_snomed_code_in_path_param_id(test_data): - pdm_record = create_and_store_pdm_record(test_data) - response = get_pdm_document_reference( - record_id=pdm_record["id"], - pdm_snomed="", - ) - - body = response.json() - assert response.status_code == 400 - _assert_operation_outcome(body=body, code="MISSING_VALUE") - - def test_no_document_id_in_path_param_id(): response = get_pdm_document_reference() @@ -144,23 +152,13 @@ def test_no_document_id_in_path_param_id(): _assert_operation_outcome(body=body, code="MISSING_VALUE") -def test_no_snomed_or_document_id_in_path_param_id(): - response = get_pdm_document_reference( - pdm_snomed="", - ) - - body = response.json() - assert response.status_code == 400 - _assert_operation_outcome(body=body, code="MISSING_VALUE") - - # This is not a helpful error message. Update this in ticket NDR-394 def test_extra_parameter_in_id_in_path_param_id(test_data): pdm_record = create_and_store_pdm_record(test_data) response = get_pdm_document_reference( - endpoint_override=f"{pdm_data_helper.snomed_code}~{pdm_record['id']}~thisshouldnotbehere", + endpoint_override=f"{pdm_record['id']}~thisshouldnotbehere", ) body = response.json() - assert response.status_code == 400 - _assert_operation_outcome(body=body, code="MISSING_VALUE") + assert response.status_code == 404 + _assert_operation_outcome(body=body, code="RESOURCE_NOT_FOUND") diff --git a/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py b/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py index 92783b115c..fbc12c775d 100644 --- a/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py +++ b/lambdas/tests/e2e/api/fhir/test_search_patient_fhir_api.py @@ -4,7 +4,6 @@ from enums.document_retention import DocumentRetentionDays from tests.e2e.api.fhir.conftest import ( MTLS_ENDPOINT, - PDM_SNOMED, create_and_store_pdm_record, create_mtls_session, ) @@ -60,18 +59,14 @@ def test_search_patient_details(test_data): # Find the entry with the matching record_id matching_entry = next( - ( - e - for e in entries - if e["resource"].get("id") == f"{PDM_SNOMED}~{expected_record_id}" - ), + (e for e in entries if e["resource"].get("id") == expected_record_id), None, ) assert matching_entry attachment_url = matching_entry["resource"]["content"][0]["attachment"]["url"] assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{PDM_SNOMED}~{expected_record_id}" + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{expected_record_id}" in attachment_url ) @@ -92,11 +87,7 @@ def test_multiple_cancelled_search_patient_details(test_data): # Validate all created records exist and have status cancelled for record_id in record_ids: entry = next( - ( - e - for e in entries - if e["resource"].get("id") == f"{PDM_SNOMED}~{record_id}" - ), + (e for e in entries if e["resource"].get("id") == record_id), None, ) assert entry @@ -178,21 +169,13 @@ def test_search_patient_details_deleted_are_not_returned(test_data): # Find the entry with the matching record_id matching_entry = next( - ( - e - for e in entries - if e["resource"].get("id") == f"{PDM_SNOMED}~{expected_record_id_1}" - ), + (e for e in entries if e["resource"].get("id") == expected_record_id_1), None, ) assert matching_entry # Assert deleted item doesn't exist non_matching_entry = next( - ( - e - for e in entries - if e["resource"].get("id") == f"{PDM_SNOMED}~{expected_record_id_2}" - ), + (e for e in entries if e["resource"].get("id") == expected_record_id_2), None, ) assert non_matching_entry is None diff --git a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py index 930df91133..57f53a1a4a 100644 --- a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py +++ b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_failure.py @@ -47,7 +47,7 @@ def test_create_document_virus(test_data): raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 upload_response = raw_upload_response.json() - record["id"] = upload_response["id"].split("~")[1] + record["id"] = upload_response["id"] test_data.append(record) # Poll until processing/scan completes @@ -59,7 +59,8 @@ def condition(response_json): ) raw_retrieve_response = retrieve_document_with_retry( - upload_response["id"], condition + upload_response["id"], + condition, ) retrieve_response = raw_retrieve_response.json() @@ -84,7 +85,10 @@ def condition(response_json): ], ) def test_search_edge_cases( - nhs_number, expected_status, expected_code, expected_diagnostics + nhs_number, + expected_status, + expected_code, + expected_diagnostics, ): record = { "ods": "H81109", @@ -123,7 +127,10 @@ def test_forbidden_with_invalid_cert(temp_cert_and_key): headers = {"Authorization": "Bearer 123", "X-Correlation-Id": "1234"} response = requests.post( - url, headers=headers, cert=(cert_path, key_path), data=payload + url, + headers=headers, + cert=(cert_path, key_path), + data=payload, ) body = response.json() assert response.status_code == 403 diff --git a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py index dd1a7d4204..6a95edf054 100644 --- a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py +++ b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py @@ -3,11 +3,7 @@ import logging import os -from tests.e2e.api.fhir.conftest import ( - PDM_SNOMED, - retrieve_document_with_retry, - upload_document, -) +from tests.e2e.api.fhir.conftest import retrieve_document_with_retry, upload_document from tests.e2e.conftest import APIM_ENDPOINT from tests.e2e.helpers.data_helper import PdmDataHelper @@ -27,14 +23,14 @@ def test_create_document_base64(test_data): raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 - record["id"] = raw_upload_response.json()["id"].split("~")[1] + record["id"] = raw_upload_response.json()["id"] test_data.append(record) # Validate attachment URL upload_response = raw_upload_response.json() attachment_url = upload_response["content"][0]["attachment"]["url"] assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{PDM_SNOMED}~" + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{record['id']}" in attachment_url ) @@ -66,7 +62,7 @@ def test_create_document_saves_raw(test_data): raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 - record["id"] = raw_upload_response.json()["id"].split("~")[1] + record["id"] = raw_upload_response.json()["id"] test_data.append(record) doc_ref = pdm_data_helper.retrieve_document_reference(record=record) @@ -95,7 +91,7 @@ def test_create_document_without_author_or_type(test_data): assert field not in payload raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 - record["id"] = raw_upload_response.json()["id"].split("~")[1] + record["id"] = raw_upload_response.json()["id"] test_data.append(record) doc_ref = pdm_data_helper.retrieve_document_reference(record=record) @@ -122,7 +118,7 @@ def test_create_document_without_title(test_data): raw_upload_response = upload_document(payload) assert raw_upload_response.status_code == 201 - record["id"] = raw_upload_response.json()["id"].split("~")[1] + record["id"] = raw_upload_response.json()["id"] test_data.append(record) doc_ref = pdm_data_helper.retrieve_document_reference(record=record) diff --git a/lambdas/tests/e2e/api/test_delete_document_reference_fhir_api_success.py b/lambdas/tests/e2e/api/test_delete_document_reference_fhir_api_success.py index 2b44d81071..2c397f99f1 100644 --- a/lambdas/tests/e2e/api/test_delete_document_reference_fhir_api_success.py +++ b/lambdas/tests/e2e/api/test_delete_document_reference_fhir_api_success.py @@ -2,7 +2,7 @@ import uuid import requests -from tests.e2e.conftest import API_ENDPOINT, API_KEY, LLOYD_GEORGE_SNOMED +from tests.e2e.conftest import API_ENDPOINT, API_KEY from tests.e2e.helpers.data_helper import LloydGeorgeDataHelper data_helper = LloydGeorgeDataHelper() @@ -54,7 +54,7 @@ def test_delete_record_by_patient_details_and_get_by_id(test_data): data_helper.create_metadata(lloyd_george_record) data_helper.create_resource(lloyd_george_record) - url = f"https://{API_ENDPOINT}/FhirDocumentReference/{LLOYD_GEORGE_SNOMED}~{lloyd_george_record['id']}" + url = f"https://{API_ENDPOINT}/FhirDocumentReference/{lloyd_george_record['id']}" headers = { "Authorization": "Bearer 123", "X-Api-Key": API_KEY, diff --git a/lambdas/tests/e2e/api/test_retrieve_document_api.py b/lambdas/tests/e2e/api/test_retrieve_document_api.py index 4f464c7b5b..560d7f1fef 100644 --- a/lambdas/tests/e2e/api/test_retrieve_document_api.py +++ b/lambdas/tests/e2e/api/test_retrieve_document_api.py @@ -3,12 +3,7 @@ import requests from syrupy.filters import paths -from tests.e2e.conftest import ( - API_ENDPOINT, - API_KEY, - LLOYD_GEORGE_S3_BUCKET, - LLOYD_GEORGE_SNOMED, -) +from tests.e2e.conftest import API_ENDPOINT, API_KEY, LLOYD_GEORGE_BUCKET from tests.e2e.helpers.data_helper import LloydGeorgeDataHelper data_helper = LloydGeorgeDataHelper() @@ -25,7 +20,7 @@ def test_small_file(test_data, snapshot_json): data_helper.create_metadata(lloyd_george_record) data_helper.create_resource(lloyd_george_record) - url = f"https://{API_ENDPOINT}/FhirDocumentReference/{LLOYD_GEORGE_SNOMED}~{lloyd_george_record['id']}" + url = f"https://{API_ENDPOINT}/FhirDocumentReference/{lloyd_george_record['id']}" headers = { "Authorization": "Bearer 123", "X-Api-Key": API_KEY, @@ -49,7 +44,7 @@ def test_large_file(test_data, snapshot_json): data_helper.create_metadata(lloyd_george_record) data_helper.create_resource(lloyd_george_record) - url = f"https://{API_ENDPOINT}/FhirDocumentReference/{LLOYD_GEORGE_SNOMED}~{lloyd_george_record['id']}" + url = f"https://{API_ENDPOINT}/FhirDocumentReference/{lloyd_george_record['id']}" headers = { "Authorization": "Bearer 123", "X-Api-Key": API_KEY, @@ -59,11 +54,11 @@ def test_large_file(test_data, snapshot_json): response = requests.request("GET", url, headers=headers) json = response.json() - expected_presign_uri = f"https://{LLOYD_GEORGE_S3_BUCKET}.s3.eu-west-2.amazonaws.com/{lloyd_george_record['nhs_number']}/{lloyd_george_record['id']}" + expected_presign_uri = f"https://{LLOYD_GEORGE_BUCKET}.s3.eu-west-2.amazonaws.com/{lloyd_george_record['nhs_number']}/{lloyd_george_record['id']}" assert expected_presign_uri in json["content"][0]["attachment"]["url"] assert json == snapshot_json( - exclude=paths("date", "id", "content.0.attachment.url") + exclude=paths("date", "id", "content.0.attachment.url"), ) @@ -71,7 +66,7 @@ def test_no_file_found(snapshot_json): lloyd_george_record = {} lloyd_george_record["id"] = str(uuid.uuid4()) - url = f"https://{API_ENDPOINT}/FhirDocumentReference/{LLOYD_GEORGE_SNOMED}~{lloyd_george_record['id']}" + url = f"https://{API_ENDPOINT}/FhirDocumentReference/{lloyd_george_record['id']}" headers = { "Authorization": "Bearer 123", "X-Api-Key": API_KEY, @@ -95,7 +90,7 @@ def test_preliminary_file(test_data, snapshot_json): data_helper.create_metadata(lloyd_george_record) data_helper.create_resource(lloyd_george_record) - url = f"https://{API_ENDPOINT}/FhirDocumentReference/{LLOYD_GEORGE_SNOMED}~{lloyd_george_record['id']}" + url = f"https://{API_ENDPOINT}/FhirDocumentReference/{lloyd_george_record['id']}" headers = { "Authorization": "Bearer 123", "X-Api-Key": API_KEY, diff --git a/lambdas/tests/e2e/api/test_search_patient_api.py b/lambdas/tests/e2e/api/test_search_patient_api.py index 2a28349c66..3848f0c31a 100644 --- a/lambdas/tests/e2e/api/test_search_patient_api.py +++ b/lambdas/tests/e2e/api/test_search_patient_api.py @@ -4,7 +4,7 @@ import requests from syrupy.filters import paths -from tests.e2e.conftest import API_ENDPOINT, API_KEY, APIM_ENDPOINT, LLOYD_GEORGE_SNOMED +from tests.e2e.conftest import API_ENDPOINT, API_KEY, APIM_ENDPOINT from tests.e2e.helpers.data_helper import LloydGeorgeDataHelper data_helper = LloydGeorgeDataHelper() @@ -33,7 +33,7 @@ def test_search_patient_details(test_data, snapshot_json): attachment_url = bundle["entry"][0]["resource"]["content"][0]["attachment"]["url"] assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{LLOYD_GEORGE_SNOMED}~" + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{lloyd_george_record['id']}" in attachment_url ) @@ -182,8 +182,7 @@ def test_search_patient_details_deleted_are_not_returned(test_data): ( e for e in entries - if e["resource"].get("id") - == f"{LLOYD_GEORGE_SNOMED}~{lloyd_george_record['id']}" + if e["resource"].get("id") == f"{lloyd_george_record['id']}" ), None, ) @@ -193,8 +192,7 @@ def test_search_patient_details_deleted_are_not_returned(test_data): ( e for e in entries - if e["resource"].get("id") - == f"{LLOYD_GEORGE_SNOMED}~{second_lloyd_george_record['id']}" + if e["resource"].get("id") == f"{second_lloyd_george_record['id']}" ), None, ) diff --git a/lambdas/tests/e2e/api/test_upload_document_api.py b/lambdas/tests/e2e/api/test_upload_document_api.py index 204eea9368..ba173cbaa9 100644 --- a/lambdas/tests/e2e/api/test_upload_document_api.py +++ b/lambdas/tests/e2e/api/test_upload_document_api.py @@ -9,7 +9,7 @@ API_ENDPOINT, API_KEY, APIM_ENDPOINT, - LLOYD_GEORGE_S3_BUCKET, + LLOYD_GEORGE_BUCKET, LLOYD_GEORGE_SNOMED, fetch_with_retry, ) @@ -88,7 +88,7 @@ def test_create_document_base64(test_data, snapshot_json): retrieve_response = requests.post(url, headers=headers, data=payload) upload_response = retrieve_response.json() - lloyd_george_record["id"] = upload_response["id"].split("~")[1] + lloyd_george_record["id"] = upload_response["id"] test_data.append(lloyd_george_record) retrieve_url = ( @@ -104,7 +104,7 @@ def condition(response_json): attachment_url = upload_response["content"][0]["attachment"]["url"] assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{LLOYD_GEORGE_SNOMED}~" + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{lloyd_george_record['id']}" in attachment_url ) @@ -130,7 +130,7 @@ def test_create_document_presign(test_data, snapshot_json): retrieve_response = requests.post(url, headers=headers, data=payload) upload_response = retrieve_response.json() - lloyd_george_record["id"] = upload_response["id"].split("~")[1] + lloyd_george_record["id"] = upload_response["id"] test_data.append(lloyd_george_record) presign_uri = upload_response["content"][0]["attachment"]["url"] del upload_response["content"][0]["attachment"]["url"] @@ -152,7 +152,7 @@ def condition(response_json): raw_retrieve_response = fetch_with_retry(retrieve_url, condition) retrieve_response = raw_retrieve_response.json() - expected_presign_uri = f"https://{LLOYD_GEORGE_S3_BUCKET}.s3.eu-west-2.amazonaws.com/{lloyd_george_record['nhs_number']}/{lloyd_george_record['id']}" + expected_presign_uri = f"https://{LLOYD_GEORGE_BUCKET}.s3.eu-west-2.amazonaws.com/{lloyd_george_record['nhs_number']}/{lloyd_george_record['id']}" assert expected_presign_uri in retrieve_response["content"][0]["attachment"]["url"] assert isinstance(retrieve_response["content"][0]["attachment"]["size"], (int)) @@ -186,7 +186,7 @@ def test_create_document_virus(test_data, snapshot_json): retrieve_response = requests.post(url, headers=headers, data=payload) upload_response = retrieve_response.json() - lloyd_george_record["id"] = upload_response["id"].split("~")[1] + lloyd_george_record["id"] = upload_response["id"] test_data.append(lloyd_george_record) retrieve_url = ( @@ -226,7 +226,7 @@ def test_create_document_does_not_save_raw(test_data): retrieve_response = requests.post(url, headers=headers, data=payload) json_response = retrieve_response.json() - lloyd_george_record["id"] = json_response.get("id").split("~")[1] + lloyd_george_record["id"] = json_response.get("id") test_data.append(lloyd_george_record) doc_ref = data_helper.retrieve_document_reference(record=lloyd_george_record) assert "Item" in doc_ref diff --git a/lambdas/tests/e2e/conftest.py b/lambdas/tests/e2e/conftest.py index 7754574670..8111d3feed 100644 --- a/lambdas/tests/e2e/conftest.py +++ b/lambdas/tests/e2e/conftest.py @@ -13,7 +13,7 @@ API_KEY = os.environ.get("NDR_API_KEY") LG_METADATA_TABLE = data_helper.dynamo_table -LLOYD_GEORGE_S3_BUCKET = data_helper.s3_bucket +LLOYD_GEORGE_BUCKET = data_helper.s3_bucket APIM_ENDPOINT = data_helper.apim_url diff --git a/lambdas/tests/unit/conftest.py b/lambdas/tests/unit/conftest.py index dbd9e699d8..1785f326ac 100644 --- a/lambdas/tests/unit/conftest.py +++ b/lambdas/tests/unit/conftest.py @@ -82,7 +82,7 @@ MOCK_STATISTICS_REPORT_BUCKET_NAME = "test_statistics_report_bucket" REVIEW_SQS_QUEUE_URL = "test_review_queue" TEST_NHS_NUMBER = "9000000009" -TEST_UUID = "1234-4567-8912-HSDF-TEST" +TEST_UUID = "550e8400-e29b-41d4-a716-446655440000" TEST_FILE_KEY = "test_file_key" TEST_FILE_NAME = "test.pdf" TEST_FILE_SIZE = 24000 diff --git a/lambdas/tests/unit/handlers/test_get_fhir_document_reference_handler.py b/lambdas/tests/unit/handlers/test_get_fhir_document_reference_handler.py index cdd3e6f873..5ed0f9bc52 100644 --- a/lambdas/tests/unit/handlers/test_get_fhir_document_reference_handler.py +++ b/lambdas/tests/unit/handlers/test_get_fhir_document_reference_handler.py @@ -6,7 +6,6 @@ from enums.snomed_codes import SnomedCodes from handlers.get_fhir_document_reference_handler import ( extract_document_parameters, - get_id_and_snomed_from_path_parameters, lambda_handler, verify_user_authorisation, ) @@ -28,19 +27,19 @@ "Authorization": f"Bearer {TEST_UUID}", "cis2-urid": TEST_UUID, }, - "pathParameters": {"id": f"{SNOMED_CODE}~{TEST_UUID}"}, + "pathParameters": {"id": TEST_UUID}, "body": None, "requestContext": {}, } MOCK_INVALID_EVENT_ID_MALFORMED = deepcopy(MOCK_CIS2_VALID_EVENT) -MOCK_INVALID_EVENT_ID_MALFORMED["pathParameters"]["id"] = f"~{TEST_UUID}" +MOCK_INVALID_EVENT_ID_MALFORMED["pathParameters"]["id"] = "NOT_A_UUID" MOCK_EVENT_APPLICATION = deepcopy(MOCK_CIS2_VALID_EVENT) MOCK_EVENT_APPLICATION["headers"] = {"Authorization": f"Bearer {TEST_UUID}"} MOCK_DOCUMENT_REFERENCE = DocumentReference.model_validate( - MOCK_SEARCH_RESPONSE["Items"][0] + MOCK_SEARCH_RESPONSE["Items"][0], ) @@ -58,7 +57,7 @@ def mock_oidc_service(mocker): @pytest.fixture def mock_config_service(mocker): mock_config = mocker.patch( - "handlers.get_fhir_document_reference_handler.DynamicConfigurationService" + "handlers.get_fhir_document_reference_handler.DynamicConfigurationService", ) mock_config_instance = mock_config.return_value return mock_config_instance @@ -67,7 +66,7 @@ def mock_config_service(mocker): @pytest.fixture def mock_document_service(mocker): mock_service = mocker.patch( - "handlers.get_fhir_document_reference_handler.GetFhirDocumentReferenceService" + "handlers.get_fhir_document_reference_handler.GetFhirDocumentReferenceService", ) mock_service_instance = mock_service.return_value mock_service_instance.handle_get_document_reference_request.return_value = ( @@ -79,7 +78,7 @@ def mock_document_service(mocker): @pytest.fixture def mock_search_patient_service(mocker): mock_service = mocker.patch( - "handlers.get_fhir_document_reference_handler.SearchPatientDetailsService" + "handlers.get_fhir_document_reference_handler.SearchPatientDetailsService", ) mock_service_instance = mock_service.return_value return mock_service_instance @@ -106,13 +105,15 @@ def test_lambda_handler_happy_path_with_cis2_login( mock_oidc_service.fetch_user_org_code.assert_called_once() mock_oidc_service.fetch_user_role_code.assert_called_once() mock_document_service.handle_get_document_reference_request.assert_called_once_with( - SNOMED_CODE, TEST_UUID + SNOMED_CODE, + TEST_UUID, ) mock_search_patient_service.handle_search_patient_request.assert_called_once_with( - "9000000009", False + "9000000009", + False, ) mock_document_service.create_document_reference_fhir_response.assert_called_once_with( - MOCK_DOCUMENT_REFERENCE + MOCK_DOCUMENT_REFERENCE, ) @@ -137,11 +138,12 @@ def test_lambda_handler_happy_path_with_application_login( mock_oidc_service.fetch_user_org_code.assert_not_called() mock_oidc_service.fetch_user_role_code.assert_not_called() mock_document_service.handle_get_document_reference_request.assert_called_once_with( - SNOMED_CODE, TEST_UUID + SNOMED_CODE, + TEST_UUID, ) mock_search_patient_service.handle_search_patient_request.assert_not_called() mock_document_service.create_document_reference_fhir_response.assert_called_once_with( - MOCK_DOCUMENT_REFERENCE + MOCK_DOCUMENT_REFERENCE, ) @@ -161,20 +163,19 @@ def test_extract_missing_bearer_token(context): def test_extract_document_parameters_valid(): - document_id, snomed_code = extract_document_parameters(MOCK_CIS2_VALID_EVENT) + document_id = extract_document_parameters(MOCK_CIS2_VALID_EVENT) assert document_id == TEST_UUID - assert snomed_code == SNOMED_CODE def test_extract_document_parameters_invalid(): - with pytest.raises(GetFhirDocumentReferenceException) as e: - extract_document_parameters(MOCK_INVALID_EVENT_ID_MALFORMED) - assert e.value.status_code == 400 - assert e.value.error == LambdaError.DocumentReferenceMissingParameters + document_id = extract_document_parameters(MOCK_INVALID_EVENT_ID_MALFORMED) + assert document_id == "NOT_A_UUID" def test_verify_user_authorisation( - mock_oidc_service, mock_config_service, mock_search_patient_service + mock_oidc_service, + mock_config_service, + mock_search_patient_service, ): try: verify_user_authorisation(f"Bearer {TEST_UUID}", TEST_UUID, "9000000009") @@ -183,7 +184,9 @@ def test_verify_user_authorisation( def test_verify_user_authorization_raise_oidc_error( - mock_oidc_service, mock_config_service, mock_search_patient_service + mock_oidc_service, + mock_config_service, + mock_search_patient_service, ): mock_oidc_service.fetch_userinfo.side_effect = OidcApiException("OIDC error") with pytest.raises(GetFhirDocumentReferenceException) as excinfo: @@ -193,7 +196,9 @@ def test_verify_user_authorization_raise_oidc_error( def test_verify_user_authorization_raise_search_patient_error( - mock_oidc_service, mock_config_service, mock_search_patient_service + mock_oidc_service, + mock_config_service, + mock_search_patient_service, ): mock_search_patient_service.handle_search_patient_request.side_effect = ( SearchPatientException(403, LambdaError.MockError) @@ -205,7 +210,11 @@ def test_verify_user_authorization_raise_search_patient_error( def test_lambda_handler_missing_auth( - set_env, mock_oidc_service, mock_config_service, mock_document_service, context + set_env, + mock_oidc_service, + mock_config_service, + mock_document_service, + context, ): event_without_auth = deepcopy(MOCK_CIS2_VALID_EVENT) event_without_auth["headers"] = {} @@ -216,15 +225,22 @@ def test_lambda_handler_missing_auth( def test_lambda_handler_id_malformed( - set_env, mock_oidc_service, mock_config_service, mock_document_service, context + set_env, + mock_oidc_service, + mock_config_service, + mock_document_service, + context, ): response = lambda_handler(MOCK_INVALID_EVENT_ID_MALFORMED, context) - assert response["statusCode"] == 400 - mock_document_service.handle_get_document_reference_request.assert_not_called() + assert response["statusCode"] == 404 def test_lambda_handler_oidc_error( - set_env, mock_config_service, mock_document_service, context, mocker + set_env, + mock_config_service, + mock_document_service, + context, + mocker, ): mocker.patch( "handlers.get_fhir_document_reference_handler.OidcService.set_up_oidc_parameters", @@ -238,14 +254,17 @@ def test_lambda_handler_oidc_error( def test_lambda_handler_invalid_path_parameters( - set_env, mock_oidc_service, mock_config_service, mock_document_service, context + set_env, + mock_oidc_service, + mock_config_service, + mock_document_service, + context, ): event_with_invalid_path = deepcopy(MOCK_CIS2_VALID_EVENT) - event_with_invalid_path["pathParameters"] = {"id": "invalid_format_no_tilde"} + event_with_invalid_path["pathParameters"] = {"id": "invalid_format"} response = lambda_handler(event_with_invalid_path, context) - assert response["statusCode"] == 400 - mock_document_service.handle_get_document_reference_request.assert_not_called() + assert response["statusCode"] == 404 @pytest.mark.parametrize( @@ -312,34 +331,5 @@ def test_lambda_handler_search_service_errors( == LambdaError.MockError.value.get("fhir_coding").display ) assert response_body["issue"][0]["diagnostics"] == LambdaError.MockError.value.get( - "message" + "message", ) - - -def test_get_id_and_snomed_from_path_parameters(): - path_parameter = f"{SNOMED_CODE}~{TEST_UUID}" - - document_id, snomed = get_id_and_snomed_from_path_parameters(path_parameter) - assert document_id == TEST_UUID - assert snomed == SNOMED_CODE - - -def test_get_id_and_snomed_from_path_parameters_no_tilde(): - document_id, snomed = get_id_and_snomed_from_path_parameters("notildePresent") - assert document_id is None - assert snomed is None - - -def test_get_id_and_snomed_from_path_parameters_extra_tildes(): - # Test with extra tildes - path_parameter = f"{SNOMED_CODE}~{TEST_UUID}~extra" - - document_id, snomed = get_id_and_snomed_from_path_parameters(path_parameter) - assert document_id is None - assert snomed is None - - -def test_get_id_and_snomed_from_path_parameters_empty(): - document_id, snomed = get_id_and_snomed_from_path_parameters("") - assert document_id is None - assert snomed is None diff --git a/lambdas/tests/unit/handlers/test_pdm_get_fhir_document_reference_by_id_handler.py b/lambdas/tests/unit/handlers/test_pdm_get_fhir_document_reference_by_id_handler.py index ad1521349f..f127e99981 100644 --- a/lambdas/tests/unit/handlers/test_pdm_get_fhir_document_reference_by_id_handler.py +++ b/lambdas/tests/unit/handlers/test_pdm_get_fhir_document_reference_by_id_handler.py @@ -15,7 +15,7 @@ MOCK_MTLS_VALID_EVENT = { "httpMethod": "GET", "headers": {}, - "pathParameters": {"id": f"{SNOMED_CODE}~{TEST_UUID}"}, + "pathParameters": {"id": TEST_UUID}, "body": None, "requestContext": { "accountId": "123456789012", @@ -39,14 +39,14 @@ } MOCK_DOCUMENT_REFERENCE = DocumentReference.model_validate( - MOCK_SEARCH_RESPONSE["Items"][0] + MOCK_SEARCH_RESPONSE["Items"][0], ) @pytest.fixture def mock_config_service(mocker): mock_config = mocker.patch( - "handlers.get_fhir_document_reference_handler.DynamicConfigurationService" + "handlers.get_fhir_document_reference_handler.DynamicConfigurationService", ) mock_config_instance = mock_config.return_value return mock_config_instance @@ -55,7 +55,7 @@ def mock_config_service(mocker): @pytest.fixture def mock_document_service(mocker): mock_service = mocker.patch( - "handlers.get_fhir_document_reference_handler.GetFhirDocumentReferenceService" + "handlers.get_fhir_document_reference_handler.GetFhirDocumentReferenceService", ) mock_service_instance = mock_service.return_value mock_service_instance.handle_get_document_reference_request.return_value = ( @@ -89,10 +89,11 @@ def test_lambda_handler_happy_path_with_mtls_pdm_login( assert response["body"] == "test_document_reference" # Verify correct method calls mock_document_service.handle_get_document_reference_request.assert_called_once_with( - SNOMED_CODE, TEST_UUID + SNOMED_CODE, + TEST_UUID, ) mock_document_service.create_document_reference_fhir_response.assert_called_once_with( - MOCK_DOCUMENT_REFERENCE + MOCK_DOCUMENT_REFERENCE, ) @@ -102,6 +103,5 @@ def test_extract_bearer_token_when_pdm(context, mock_mtls_common_names): def test_extract_document_parameters_valid_pdm(): - document_id, snomed_code = extract_document_parameters(MOCK_MTLS_VALID_EVENT) + document_id = extract_document_parameters(MOCK_MTLS_VALID_EVENT) assert document_id == TEST_UUID - assert snomed_code == SNOMED_CODE diff --git a/lambdas/tests/unit/services/test_document_reference_search_service.py b/lambdas/tests/unit/services/test_document_reference_search_service.py index 712eb9ac8a..75172d8765 100644 --- a/lambdas/tests/unit/services/test_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_document_reference_search_service.py @@ -328,7 +328,7 @@ def test_create_document_reference_fhir_response(mock_document_service, mocker): ) expected_fhir_response = { - "id": "16521000000101~Y05868-1634567890", + "id": "Y05868-1634567890", "resourceType": "DocumentReference", "status": "current", "docStatus": "final", @@ -373,7 +373,7 @@ def test_create_document_reference_fhir_response(mock_document_service, mocker): mock_attachment.assert_called_once_with( title=mock_document_reference.file_name, creation=mock_document_reference.document_scan_creation, - url=f"{APIM_API_URL}/DocumentReference/{SnomedCodes.LLOYD_GEORGE.value.code}~{mock_document_reference.id}", + url=f"{APIM_API_URL}/DocumentReference/{mock_document_reference.id}", ) mock_doc_ref_info.assert_called_once_with( @@ -408,7 +408,7 @@ def test_create_document_reference_fhir_response_integration( mock_document_reference.version = "1" expected_fhir_response = { - "id": "16521000000101~Y05868-1634567890", + "id": "Y05868-1634567890", "resourceType": "DocumentReference", "status": "current", "docStatus": "final", @@ -426,7 +426,7 @@ def test_create_document_reference_fhir_response_integration( "language": "en-GB", "title": "test_document.pdf", "creation": "2023-05-01", - "url": f"{APIM_API_URL}/DocumentReference/16521000000101~Y05868-1634567890", + "url": f"{APIM_API_URL}/DocumentReference/Y05868-1634567890", }, }, ], diff --git a/lambdas/tests/unit/services/test_fhir_document_reference_service_base.py b/lambdas/tests/unit/services/test_fhir_document_reference_service_base.py index c91c463c07..fa23fe5b98 100644 --- a/lambdas/tests/unit/services/test_fhir_document_reference_service_base.py +++ b/lambdas/tests/unit/services/test_fhir_document_reference_service_base.py @@ -7,15 +7,22 @@ from enums.snomed_codes import SnomedCode, SnomedCodes from models.document_reference import DocumentReference from models.fhir.R4.base_models import Coding, Identifier, Reference -from models.fhir.R4.fhir_document_reference import SNOMED_URL, Attachment +from models.fhir.R4.fhir_document_reference import ( + SNOMED_URL, + Attachment, +) from models.fhir.R4.fhir_document_reference import ( DocumentReference as FhirDocumentReference, ) -from models.fhir.R4.fhir_document_reference import DocumentReferenceContent +from models.fhir.R4.fhir_document_reference import ( + DocumentReferenceContent, +) from services.put_fhir_document_reference_service import ( FhirDocumentReferenceServiceBase, ) -from tests.unit.conftest import APIM_API_URL +from tests.unit.conftest import ( + APIM_API_URL, +) from tests.unit.conftest import ( EXPECTED_PARSED_PATIENT_BASE_CASE as mock_pds_patient_details, ) @@ -111,8 +118,8 @@ def mock_store_binary_in_s3(mocker, mock_service): "system": "http://snomed.info/sct", "code": "invalid-code", "display": "Invalid", - } - ] + }, + ], }, }, # Missing document type @@ -173,7 +180,7 @@ def test_check_nhs_number_with_pds_raise_error(mock_service, mocker): return_value=mock_service_object, ) mock_service_object.fetch_patient_details.side_effect = PatientNotFoundException( - "test test" + "test test", ) with pytest.raises(FhirDocumentReferenceException): mock_service._check_nhs_number_with_pds("9000000009") @@ -207,20 +214,22 @@ def test_create_document_reference_with_author(mock_service, mocker): contentType="application/pdf", title="test-file.pdf", creation="2023-01-01T12:00:00Z", - ) - ) + ), + ), ] fhir_doc.custodian = Reference( identifier=Identifier( - system="https://fhir.nhs.uk/Id/ods-organization-code", value="A12345" - ) + system="https://fhir.nhs.uk/Id/ods-organization-code", + value="A12345", + ), ) fhir_doc.author = [ Reference( identifier=Identifier( - system="https://fhir.nhs.uk/Id/ods-organization-code", value="B67890" - ) - ) + system="https://fhir.nhs.uk/Id/ods-organization-code", + value="B67890", + ), + ), ] doc_type = SnomedCode(code="test-code", display_name="Test Type") @@ -253,15 +262,16 @@ def test_create_document_reference_without_custodian(mock_service, mocker): contentType="application/pdf", title="test-file.pdf", creation="2023-01-01T12:00:00Z", - ) - ) + ), + ), ] fhir_doc.author = [ Reference( identifier=Identifier( - system="https://fhir.nhs.uk/Id/ods-organization-code", value="B67890" - ) - ) + system="https://fhir.nhs.uk/Id/ods-organization-code", + value="B67890", + ), + ), ] fhir_doc.custodian = None @@ -324,12 +334,14 @@ def test_process_fhir_document_reference_with_invalid_base64_data(mock_service): """Test process_fhir_document_reference with invalid base64 data.""" with pytest.raises(FhirDocumentReferenceException): mock_service._store_binary_in_s3( - TEST_DOCUMENT_REFERENCE, b"invalid-base64-data!!!" + TEST_DOCUMENT_REFERENCE, + b"invalid-base64-data!!!", ) def test_determine_document_type_returns_lloyd_george_type( - mock_service, valid_fhir_doc_object + mock_service, + valid_fhir_doc_object, ): """Test that determine_document_type returns the lloyd george type for a lloyd george document""" @@ -339,12 +351,13 @@ def test_determine_document_type_returns_lloyd_george_type( def test_determine_document_type_non_snomed_coding( - mock_service, valid_fhir_doc_object: FhirDocumentReference + mock_service, + valid_fhir_doc_object: FhirDocumentReference, ): """Test that determine_document_type returns the lloyd george type for a lloyd george document""" valid_fhir_doc_object.type.coding.append( - Coding(system="mocked_system", code="mocked_code", display="mocked_display") + Coding(system="mocked_system", code="mocked_code", display="mocked_display"), ) valid_fhir_doc_object.type.coding.reverse() @@ -355,12 +368,13 @@ def test_determine_document_type_non_snomed_coding( def test_determine_document_type_non_george_lloyd_code( - mock_service, valid_fhir_doc_object: FhirDocumentReference + mock_service, + valid_fhir_doc_object: FhirDocumentReference, ): """Test that determine_document_type returns the lloyd george type for a lloyd george document""" valid_fhir_doc_object.type.coding.append( - Coding(system=SNOMED_URL, code="mocked_code", display="mocked_display") + Coding(system=SNOMED_URL, code="mocked_code", display="mocked_display"), ) valid_fhir_doc_object.type.coding.reverse() @@ -398,7 +412,8 @@ def test_get_document_reference_returns_document_reference(mocker, mock_service) def test_create_s3_presigned_url_error(mock_service): """Test that create_s3_presigned_url raises a FhirDocumentReferenceException on AWS S3 ClientError""" mock_service.s3_service.create_put_presigned_url.side_effect = ClientError( - {"Error": {}}, "" + {"Error": {}}, + "", ) document = create_test_lloyd_george_doc_store_refs()[0] @@ -443,7 +458,7 @@ def test_store_binary_in_s3_with_client_error(mock_service): "Error": { "Code": "NoSuchBucket", "Message": "The specified bucket does not exist", - } + }, }, "PutObject", ) @@ -523,7 +538,9 @@ def test_create_fhir_response_with_presigned_url(mock_service, mocker): """Test _create_fhir_response method with a presigned URL.""" mocker.patch.object( - SnomedCodes, "find_by_code", return_value=SnomedCodes.LLOYD_GEORGE.value + SnomedCodes, + "find_by_code", + return_value=SnomedCodes.LLOYD_GEORGE.value, ) document_ref = DocumentReference( @@ -550,7 +567,9 @@ def test_create_fhir_response_without_presigned_url(set_env, mock_service, mocke """Test _create_fhir_response method without a presigned URL (for binary uploads).""" mocker.patch.object( - SnomedCodes, "find_by_code", return_value=SnomedCodes.LLOYD_GEORGE.value + SnomedCodes, + "find_by_code", + return_value=SnomedCodes.LLOYD_GEORGE.value, ) custom_endpoint = f"{APIM_API_URL}/DocumentReference" @@ -570,9 +589,7 @@ def test_create_fhir_response_without_presigned_url(set_env, mock_service, mocke result_json = json.loads(result) assert result_json["resourceType"] == "DocumentReference" - expected_url = ( - f"{custom_endpoint}/{SnomedCodes.LLOYD_GEORGE.value.code}~{document_ref.id}" - ) + expected_url = f"{custom_endpoint}/{document_ref.id}" assert result_json["content"][0]["attachment"]["url"] == expected_url @@ -587,7 +604,9 @@ def test_handle_document_save_returns_presigned_url( """Test _handle_document_save method returns presigned URL for Lloyd George document.""" result = mock_service._handle_document_save( - mock_document_reference, valid_fhir_doc_object, "test_table" + mock_document_reference, + valid_fhir_doc_object, + "test_table", ) assert result == "https://test-presigned-url.com" @@ -603,7 +622,9 @@ def test_handle_document_save_stores_binary_in_s3( ): valid_fhir_doc_object.content[0].attachment.data = "SGVsbG8gV29ybGQ=" result = mock_service._handle_document_save( - mock_document_reference, valid_fhir_doc_object, "test_table" + mock_document_reference, + valid_fhir_doc_object, + "test_table", ) assert result is None mock_service._store_binary_in_s3.assert_called_once() @@ -625,7 +646,9 @@ def test_handle_document_save_store_binary_in_s3_failure( with pytest.raises(DocumentRefException) as excinfo: mock_service._handle_document_save( - mock_document_reference, valid_fhir_doc_object, "test_table" + mock_document_reference, + valid_fhir_doc_object, + "test_table", ) assert excinfo.value.status_code == 500 @@ -652,7 +675,9 @@ def test_handle_document_save_create_s3_failure( with pytest.raises(DocumentRefException) as excinfo: mock_service._handle_document_save( - mock_document_reference, valid_fhir_doc_object, "test_table" + mock_document_reference, + valid_fhir_doc_object, + "test_table", ) assert excinfo.value.status_code == 500 @@ -678,7 +703,9 @@ def test_save_document_reference_to_dynamo_failure( with pytest.raises(DocumentRefException) as excinfo: mock_service._handle_document_save( - mock_document_reference, valid_fhir_doc_object, "test_table" + mock_document_reference, + valid_fhir_doc_object, + "test_table", ) assert excinfo.value.status_code == 500 diff --git a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py index b75c50f199..42f8c73d97 100644 --- a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_search_service.py @@ -167,7 +167,7 @@ def test_create_document_reference_fhir_response(mock_document_service, mocker): mock_attachment.assert_called_once_with( title=mock_document_reference.file_name, creation=mock_document_reference.document_scan_creation, - url=f"{APIM_API_URL}/DocumentReference/{SnomedCodes.PATIENT_DATA.value.code}~{mock_document_reference.id}", + url=f"{APIM_API_URL}/DocumentReference/{mock_document_reference.id}", ) mock_doc_ref_info.assert_called_once_with( @@ -202,7 +202,7 @@ def test_create_document_reference_fhir_response_integration( mock_document_reference.version = "1" expected_fhir_response = { - "id": "717391000000106~Y05868-1634567890", + "id": "Y05868-1634567890", "resourceType": "DocumentReference", "status": "current", "docStatus": "final", @@ -220,7 +220,7 @@ def test_create_document_reference_fhir_response_integration( "language": "en-GB", "title": "test_document.pdf", "creation": "2023-05-01", - "url": f"{APIM_API_URL}/DocumentReference/717391000000106~Y05868-1634567890", + "url": f"{APIM_API_URL}/DocumentReference/Y05868-1634567890", }, }, ], @@ -277,7 +277,7 @@ def test_create_document_reference_fhir_response_no_title( mock_document_reference.version = "1" expected_fhir_response = { - "id": "717391000000106~Y05868-1634567890", + "id": "Y05868-1634567890", "resourceType": "DocumentReference", "status": "current", "docStatus": "final", @@ -294,7 +294,7 @@ def test_create_document_reference_fhir_response_no_title( "contentType": "application/pdf", "language": "en-GB", "creation": "2023-05-01", - "url": f"{APIM_API_URL}/DocumentReference/717391000000106~Y05868-1634567890", + "url": f"{APIM_API_URL}/DocumentReference/Y05868-1634567890", }, }, ],