From a7d3e1c243e42432946a719ebdfc7de8fe699026 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Tue, 7 Apr 2026 11:19:59 +0000 Subject: [PATCH 1/2] [CDAPI-113]: Added MNS integration for the publication of events --- .github/workflows/preview-env.yaml | 30 ++- .../Post_Document_Bundle_via_APIM_INT.bru | 79 +++++++ mocks/src/mns_mock/handler.py | 42 ++-- pathology-api/lambda_handler.py | 10 + .../src/pathology_api/environment.py | 132 +++++++++++ pathology-api/src/pathology_api/handler.py | 78 +------ pathology-api/src/pathology_api/http.py | 32 ++- pathology-api/src/pathology_api/mns.py | 79 +++++++ .../src/pathology_api/test_environment.py | 113 ++++++++++ .../src/pathology_api/test_handler.py | 132 +++++------ pathology-api/src/pathology_api/test_http.py | 38 +++- .../src/pathology_api/test_logging.py | 5 +- pathology-api/src/pathology_api/test_mns.py | 210 ++++++++++++++++++ pathology-api/test_lambda_handler.py | 60 ++--- .../tests/integration/test_endpoints.py | 46 ++++ 15 files changed, 865 insertions(+), 221 deletions(-) create mode 100644 bruno/APIM/Post_Document_Bundle_via_APIM_INT.bru create mode 100644 pathology-api/src/pathology_api/environment.py create mode 100644 pathology-api/src/pathology_api/mns.py create mode 100644 pathology-api/src/pathology_api/test_environment.py create mode 100644 pathology-api/src/pathology_api/test_mns.py diff --git a/.github/workflows/preview-env.yaml b/.github/workflows/preview-env.yaml index b6cb0c15..db4f6e5f 100644 --- a/.github/workflows/preview-env.yaml +++ b/.github/workflows/preview-env.yaml @@ -183,7 +183,7 @@ jobs: APIM_KEY_ID=$KEY_ID, \ APIM_TOKEN_URL=$MOCK_URL/apim/oauth2/token, \ PDM_BUNDLE_URL=$MOCK_URL/apim/check_auth, \ - MNS_EVENT_URL=$MOCK_URL/mns, \ + MNS_EVENT_URL=$MOCK_URL/mns/events, \ CLIENT_TIMEOUT=$CLIENT_TIMEOUT, \ JWKS_SECRET_NAME=$JWKS_SECRET}" || true wait_for_lambda_ready @@ -206,7 +206,7 @@ jobs: APIM_MTLS_KEY_NAME=$MTLS_KEY, \ APIM_TOKEN_URL=$MOCK_URL/apim/oauth2/token, \ PDM_BUNDLE_URL=$MOCK_URL/apim/check_auth, \ - MNS_EVENT_URL=$MOCK_URL/mns, \ + MNS_EVENT_URL=$MOCK_URL/mns/events, \ CLIENT_TIMEOUT=$CLIENT_TIMEOUT, \ JWKS_SECRET_NAME=$JWKS_SECRET}" \ --publish @@ -229,13 +229,15 @@ jobs: - name: Create or update preview Lambda with integration (on open/sync/reopen) if: github.event.action != 'closed' && github.event.pull_request.user.login != 'dependabot[bot]' env: - MOCK_URL: ${{ steps.names.outputs.mock_preview_url }} + APIM_INT_URL: ${{ vars.APIM_INT_URL }} TOKEN_EXPIRY_THRESHOLD: ${{ secrets.APIM_TOKEN_EXPIRY_THRESHOLD }} JWKS_SECRET_NAME: ${{ secrets.JWKS_SECRET }} APIM_PRIVATE_KEY: ${{ secrets.APIM_PRIVATE_KEY }} APIM_APIKEY: ${{ secrets.APIM_APIKEY }} API_MTLS_CERT: ${{ secrets.API_MTLS_CERT }} API_MTLS_KEY: ${{ secrets.API_MTLS_KEY }} + APIM_KEY_ID: ${{ secrets.APIM_KEY_ID }} + CLIENT_REQUEST_TIMEOUT: ${{ secrets.CLIENT_REQUEST_TIMEOUT }} run: | cd pathology-api/target/ FN="${{ steps.names.outputs.int_function_name }}" @@ -245,6 +247,8 @@ jobs: API_KEY="${APIM_APIKEY:-/cds/pathology/dev/apim/api-key}" MTLS_CERT="${API_MTLS_CERT:-/cds/pathology/dev/mtls/client1-key-public}" MTLS_KEY="${API_MTLS_KEY:-/cds/pathology/dev/mtls/client1-key-secret}" + KEY_ID="${APIM_KEY_ID:-DEV-1}" + CLIENT_TIMEOUT="${CLIENT_REQUEST_TIMEOUT:-10s}" echo "Deploying preview function: $FN" wait_for_lambda_ready() { while true; do @@ -266,14 +270,18 @@ jobs: wait_for_lambda_ready aws lambda update-function-configuration --function-name "$FN" \ --handler "${{ env.LAMBDA_HANDLER }}" \ + --memory-size 512 \ + --timeout 30 \ --environment "Variables={APIM_TOKEN_EXPIRY_THRESHOLD=$EXPIRY_THRESHOLD, \ APIM_PRIVATE_KEY_NAME=$PRIVATE_KEY, \ APIM_API_KEY_NAME=$API_KEY, \ APIM_MTLS_CERT_NAME=$MTLS_CERT, \ APIM_MTLS_KEY_NAME=$MTLS_KEY, \ - APIM_TOKEN_URL=$MOCK_URL/apim, \ - PDM_BUNDLE_URL=$MOCK_URL/pdm, \ - MNS_EVENT_URL=$MOCK_URL/mns, \ + APIM_KEY_ID=$KEY_ID, \ + APIM_TOKEN_URL=$APIM_INT_URL/oauth2/token, \ + PDM_BUNDLE_URL=$APIM_INT_URL/patient-data-manager/FHIR/R4/Bundle, \ + MNS_EVENT_URL=$APIM_INT_URL/multicast-notification-service/events, \ + CLIENT_TIMEOUT=$CLIENT_TIMEOUT, \ JWKS_SECRET_NAME=$JWKS_SECRET}" || true wait_for_lambda_ready aws lambda update-function-code --function-name "$FN" \ @@ -285,14 +293,18 @@ jobs: --handler "${{ env.LAMBDA_HANDLER }}" \ --zip-file "fileb://artifact.zip" \ --role "${{ steps.role-select.outputs.lambda_role }}" \ + --memory-size 512 \ + --timeout 30 \ --environment "Variables={APIM_TOKEN_EXPIRY_THRESHOLD=$EXPIRY_THRESHOLD, \ APIM_PRIVATE_KEY_NAME=$PRIVATE_KEY, \ APIM_API_KEY_NAME=$API_KEY, \ APIM_MTLS_CERT_NAME=$MTLS_CERT, \ APIM_MTLS_KEY_NAME=$MTLS_KEY, \ - APIM_TOKEN_URL=$MOCK_URL/apim, \ - PDM_BUNDLE_URL=$MOCK_URL/pdm, \ - MNS_EVENT_URL=$MOCK_URL/mns, \ + APIM_KEY_ID=$KEY_ID, \ + APIM_TOKEN_URL=$APIM_INT_URL/oauth2/token, \ + PDM_BUNDLE_URL=$APIM_INT_URL/patient-data-manager/FHIR/R4/Bundle, \ + MNS_EVENT_URL=$APIM_INT_URL/multicast-notification-service/events, \ + CLIENT_TIMEOUT=$CLIENT_TIMEOUT, \ JWKS_SECRET_NAME=$JWKS_SECRET}" \ --publish wait_for_lambda_ready diff --git a/bruno/APIM/Post_Document_Bundle_via_APIM_INT.bru b/bruno/APIM/Post_Document_Bundle_via_APIM_INT.bru new file mode 100644 index 00000000..2d24c9be --- /dev/null +++ b/bruno/APIM/Post_Document_Bundle_via_APIM_INT.bru @@ -0,0 +1,79 @@ +meta { + name: Post Document Bundle via APIM - INT + type: http + seq: 4 +} + +post { + url: https://{{APIM_ENV}}.api.service.nhs.uk/pathology-laboratory-reporting-pri-{{PR_NUMBER}}/FHIR/R4/Bundle + body: json + auth: inherit +} + +headers { + Content-Type: application/fhir+json +} + +body:json { + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest" + } + } + ], + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "test-nhs-number" + } + } + } + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": { + "reference": "practitionerrole" + } + } + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": { + "reference": "organization" + } + } + }, + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "testOrg" + } + ] + } + } + ] + } +} + +settings { + encodeUrl: true + timeout: 0 +} diff --git a/mocks/src/mns_mock/handler.py b/mocks/src/mns_mock/handler.py index 6e356c0e..9c58a77b 100644 --- a/mocks/src/mns_mock/handler.py +++ b/mocks/src/mns_mock/handler.py @@ -24,7 +24,7 @@ class MNSResponse(TypedDict): status_code: int - response: dict[str, Any] + response: dict[str, Any] | None class EventItem(BaseMockItem): @@ -35,27 +35,25 @@ class EventItem(BaseMockItem): type RequestHandler = Callable[[], MNSResponse] -def _create_operation_outcome( - status_code: int, response: dict[str, Any] +def _create_response( + status_code: int, response: dict[str, Any] | None = None ) -> MNSResponse: return {"status_code": status_code, "response": response} def _raise_validation_error(event_type: str) -> RequestHandler: - return lambda: _create_operation_outcome( - 400, {"validationErrors": {"type": event_type}} - ) + return lambda: _create_response(400, {"validationErrors": {"type": event_type}}) def _raise_authentication_error(fault_string: str, error_code: str) -> RequestHandler: - return lambda: _create_operation_outcome( + return lambda: _create_response( 401, {"fault": {"faultstring": fault_string, "detail": {"errorcode": error_code}}}, ) -def _raise_server_error(status_code: int, errors: str) -> RequestHandler: - return lambda: _create_operation_outcome(status_code, {"errors": errors}) +def _raise_error(status_code: int, errors: str) -> RequestHandler: + return lambda: _create_response(status_code, {"errors": errors}) REQUEST_HANDLERS: dict[str, RequestHandler] = { @@ -65,7 +63,12 @@ def _raise_server_error(status_code: int, errors: str) -> RequestHandler: "MNS_AUTHENTICATION_ERROR": _raise_authentication_error( "Invalid access token", "oauth.v2.InvalidAccessToken" ), - "MNS_SERVER_ERROR": _raise_server_error(500, "Internal server error"), + "MNS_AUTHORIZATION_ERROR": _raise_error( + 403, "User is not authorized to handle the requested event type" + ), + "MNS_SERVER_ERROR": _raise_error(500, "Internal server error"), + "MNS_BAD_GATEWAY_ERROR": lambda: _create_response(502), + "MNS_GATEWAY_TIMEOUT_ERROR": lambda: _create_response(504), } @@ -102,11 +105,14 @@ def _find_events_in_table(subject: str) -> list[EventItem]: def _with_default_headers(response: MNSResponse) -> Response[str]: - return Response( - body=json.dumps(response["response"]), - status_code=response["status_code"], - headers={"Content-Type": "application/fhir+json"}, - ) + if response["response"] is not None: + body = json.dumps(response["response"]) + headers = {"Content-Type": "application/fhir+json"} + else: + body = None + headers = None + + return Response(body=body, status_code=response["status_code"], headers=headers) @mns_routes.post("/mns/events") @@ -122,7 +128,7 @@ def create_event() -> Response[str]: except json.JSONDecodeError as err: _logger.error("Error decoding JSON payload. Error: %s", err) return _with_default_headers( - _create_operation_outcome( + _create_response( 400, {"validationErrors": {"type": "Invalid payload provided"}} ) ) @@ -130,9 +136,7 @@ def create_event() -> Response[str]: if not payload: _logger.error("No payload provided.") return _with_default_headers( - _create_operation_outcome( - 400, {"validationErrors": {"type": "No payload provided"}} - ) + _create_response(400, {"validationErrors": {"type": "No payload provided"}}) ) _logger.debug( diff --git a/pathology-api/lambda_handler.py b/pathology-api/lambda_handler.py index 377a8bb4..3cbb93c5 100644 --- a/pathology-api/lambda_handler.py +++ b/pathology-api/lambda_handler.py @@ -13,6 +13,7 @@ from pathology_api.fhir.r4.resources import Bundle, OperationOutcome from pathology_api.handler import handle_request from pathology_api.logging import get_logger +from pathology_api.mns import MnsException from pathology_api.request_context import reset_correlation_id, set_correlation_id _logger = get_logger(__name__) @@ -111,6 +112,15 @@ def handle_exception(exception: Exception) -> Response[str]: ) +@_exception_handler(MnsException) +def handle_mns_exception(exception: MnsException) -> Response[str]: + _logger.exception("Failed to publish MNS event: %s", exception) + return _with_default_headers( + status_code=500, + body=OperationOutcome.create_server_error("Failed to publish an event"), + ) + + @app.get("/_status") def status() -> Response[str]: _logger.debug("Status check endpoint called") diff --git a/pathology-api/src/pathology_api/environment.py b/pathology-api/src/pathology_api/environment.py new file mode 100644 index 00000000..daf6806a --- /dev/null +++ b/pathology-api/src/pathology_api/environment.py @@ -0,0 +1,132 @@ +from typing import TypedDict + +from aws_lambda_powertools.utilities import parameters + +from pathology_api.apim import ApimAuthenticator +from pathology_api.config import ( + Duration, + get_environment_variable, + get_optional_environment_variable, +) +from pathology_api.http import ClientCertificate, SessionManager + +__all__ = [ + "apim_authenticator", + "values", + "session_manager", +] + + +class Environment(TypedDict): + client_timeout: Duration + apim_token_url: str + apim_private_key_name: str + apim_api_key_name: str + apim_token_expiry_threshold: Duration + apim_key_id: str + pdm_url: str + mns_url: str + + +_environment: Environment | None = None +_session_manager: SessionManager | None = None +_apim_authenticator: ApimAuthenticator | None = None + + +def _create_client_certificate( + certificate_name: str, key_name: str +) -> ClientCertificate: + certificate = parameters.get_secret(certificate_name) + key = parameters.get_secret(key_name) + + return { + "certificate": certificate, + "key": key, + } + + +def values() -> Environment: + global _environment + if _environment is None: + _environment = Environment( + client_timeout=get_environment_variable( + "CLIENT_TIMEOUT", + Duration, + ), + apim_token_url=get_environment_variable( + "APIM_TOKEN_URL", + str, + ), + apim_private_key_name=get_environment_variable( + "APIM_PRIVATE_KEY_NAME", + str, + ), + apim_api_key_name=get_environment_variable( + "APIM_API_KEY_NAME", + str, + ), + apim_token_expiry_threshold=get_environment_variable( + "APIM_TOKEN_EXPIRY_THRESHOLD", + Duration, + ), + apim_key_id=get_environment_variable( + "APIM_KEY_ID", + str, + ), + pdm_url=get_environment_variable( + "PDM_BUNDLE_URL", + str, + ), + mns_url=get_environment_variable( + "MNS_EVENT_URL", + str, + ), + ) + + return _environment + + +def session_manager() -> SessionManager: + global _session_manager + + if _session_manager is None: + if ( + client_certificate_name := get_optional_environment_variable( + "APIM_MTLS_CERT_NAME", str + ) + ) and ( + client_key_name := get_optional_environment_variable( + "APIM_MTLS_KEY_NAME", str + ) + ): + client_certificate: ClientCertificate | None = _create_client_certificate( + client_certificate_name, client_key_name + ) + else: + client_certificate = None + + _session_manager = SessionManager( + client_timeout=get_environment_variable( + "CLIENT_TIMEOUT", Duration + ).timedelta, + client_certificate=client_certificate, + ) + + return _session_manager + + +def apim_authenticator() -> ApimAuthenticator: + global _apim_authenticator + + if _apim_authenticator is None: + env = values() + _apim_authenticator = ApimAuthenticator( + private_key=parameters.get_secret(env["apim_private_key_name"]), + key_id=env["apim_key_id"], + api_key=parameters.get_secret(env["apim_api_key_name"]), + token_endpoint=env["apim_token_url"], + token_validity_threshold=env["apim_token_expiry_threshold"].timedelta, + session_manager=session_manager(), + ) + + return _apim_authenticator diff --git a/pathology-api/src/pathology_api/handler.py b/pathology-api/src/pathology_api/handler.py index 60f6e314..b73c782e 100644 --- a/pathology-api/src/pathology_api/handler.py +++ b/pathology-api/src/pathology_api/handler.py @@ -1,14 +1,5 @@ import uuid -import requests -from aws_lambda_powertools.utilities import parameters - -from pathology_api.apim import ApimAuthenticator -from pathology_api.config import ( - Duration, - get_environment_variable, - get_optional_environment_variable, -) from pathology_api.exception import ValidationError from pathology_api.fhir.r4.elements import ( LiteralReference, @@ -23,61 +14,11 @@ PractitionerRole, ServiceRequest, ) -from pathology_api.http import ClientCertificate, SessionManager from pathology_api.logging import get_logger +from pathology_api.mns import create_event _logger = get_logger(__name__) -CLIENT_TIMEOUT = get_environment_variable("CLIENT_TIMEOUT", Duration) - -CLIENT_CERTIFICATE_NAME = get_optional_environment_variable("APIM_MTLS_CERT_NAME", str) -CLIENT_KEY_NAME = get_optional_environment_variable("APIM_MTLS_KEY_NAME", str) - -APIM_TOKEN_URL = get_environment_variable("APIM_TOKEN_URL", str) -APIM_PRIVATE_KEY_NAME = get_environment_variable("APIM_PRIVATE_KEY_NAME", str) -APIM_API_KEY_NAME = get_environment_variable("APIM_API_KEY_NAME", str) -APIM_TOKEN_EXPIRY_THRESHOLD = get_environment_variable( - "APIM_TOKEN_EXPIRY_THRESHOLD", Duration -) -APIM_KEY_ID = get_environment_variable("APIM_KEY_ID", str) - -PDM_URL = get_environment_variable("PDM_BUNDLE_URL", str) - - -def _create_client_certificate( - certificate_name: str, key_name: str -) -> ClientCertificate: - certificate = parameters.get_secret(certificate_name) - key = parameters.get_secret(key_name) - - return { - "certificate": certificate, - "key": key, - } - - -if CLIENT_CERTIFICATE_NAME and CLIENT_KEY_NAME: - CLIENT_CERTIFICATE: ClientCertificate | None = _create_client_certificate( - CLIENT_CERTIFICATE_NAME, CLIENT_KEY_NAME - ) -else: - CLIENT_CERTIFICATE = None - - -session_manager = SessionManager( - client_timeout=CLIENT_TIMEOUT.timedelta, - client_certificate=CLIENT_CERTIFICATE, -) - -apim_authenticator = ApimAuthenticator( - private_key=parameters.get_secret(APIM_PRIVATE_KEY_NAME), - key_id=APIM_KEY_ID, - api_key=parameters.get_secret(APIM_API_KEY_NAME), - token_endpoint=APIM_TOKEN_URL, - token_validity_threshold=APIM_TOKEN_EXPIRY_THRESHOLD.timedelta, - session_manager=session_manager, -) - def _validate_bundle(bundle: Bundle) -> None: if bundle.id is not None: @@ -214,16 +155,13 @@ def handle_request(bundle: Bundle) -> Bundle: ) _logger.debug("Return bundle: %s", return_bundle) - auth_response = _send_request(PDM_URL) - _logger.debug( - "Result of authenticated request. status_code=%s data=%s", - auth_response.status_code, - auth_response.text, + if return_bundle.id is None: + raise ValueError("Bundle returned from PDM does not include an ID.") + + create_event( + requesting_org=requesting_organisation.value, + nhs_number=subject.identifier.value, + bundle_id=return_bundle.id, ) return return_bundle - - -@apim_authenticator.auth -def _send_request(session: requests.Session, url: str) -> requests.Response: - return session.post(url) diff --git a/pathology-api/src/pathology_api/http.py b/pathology-api/src/pathology_api/http.py index 0d39e4a2..5c8ec272 100644 --- a/pathology-api/src/pathology_api/http.py +++ b/pathology-api/src/pathology_api/http.py @@ -9,6 +9,7 @@ from requests.adapters import HTTPAdapter from pathology_api.logging import get_logger +from pathology_api.request_context import get_correlation_id _logger = get_logger(__name__) @@ -40,11 +41,29 @@ def send( *args: Any, **kwargs: Any, ) -> requests.Response: - _logger.debug( - "Applying default timeout of %s seconds to request", self._timeout - ) kwargs["timeout"] = self._timeout - return super().send(request, *args, **kwargs) + if "X-Correlation-ID" not in request.headers: + request.headers["X-Correlation-ID"] = get_correlation_id() + + _logger.info( + "Sending HTTP request. method=%s url=%s headers=%s", + request.method, + request.url, + # Omit Authorization header from logging + { + k: v + for k, v in request.headers.items() + if k.lower() != "authorization" + }, + ) + response = super().send(request, *args, **kwargs) + _logger.info( + "Received HTTP response. status_code=%s response_headers=%s", + response.status_code, + response.headers, + ) + + return response def __init__( self, @@ -58,16 +77,12 @@ def with_session[**P, S](self, func: RequestMethod[P, S]) -> Callable[P, S]: @functools.wraps(func) def wrapper(*args: Any, **kwargs: Any) -> Any: with ExitStack() as stack: - _logger.debug("Creating new session for request") session = requests.Session() stack.enter_context(session) - _logger.debug("Mounted default settings to session") session.mount("https://", self._client_adapter) if self._client_certificate is not None: - _logger.debug("Configuring session with client certificate...") - # File added to Exit stack and will be automatically cleaned up with # the stack. cert_file = tempfile.NamedTemporaryFile( # noqa: SIM115 @@ -88,7 +103,6 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: key_file.flush() session.cert = (cert_file.name, key_file.name) - _logger.debug("Client certificate added.") return func(session, *args, **kwargs) diff --git a/pathology-api/src/pathology_api/mns.py b/pathology-api/src/pathology_api/mns.py new file mode 100644 index 00000000..d1a24d83 --- /dev/null +++ b/pathology-api/src/pathology_api/mns.py @@ -0,0 +1,79 @@ +import uuid +from datetime import datetime, timezone +from json import JSONDecodeError +from typing import Any +from urllib.parse import urljoin + +import requests + +from pathology_api import environment +from pathology_api.logging import get_logger + +_logger = get_logger(__name__) + + +class MnsException(Exception): + """ + Standard exception indicating that an issue occurred whilst interacting with MNS. + """ + + +@environment.apim_authenticator().auth +def create_event( + session: requests.Session, + requesting_org: str, + nhs_number: str, + bundle_id: str, +) -> None: + _logger.debug( + "Publishing MNS event. requesting_org=%s bundle_id=%s", + requesting_org, + bundle_id, + ) + + event: dict[str, Any] = { + "specversion": "1.0", + "id": str(uuid.uuid4()), + "source": "uk.nhs.pathology-laboratory-reporting", + "type": "pathology-laboratory-reporting-test-result-stored-1", + "time": datetime.now(timezone.utc).isoformat(), + "dataref": urljoin(environment.values()["pdm_url"] + "/", bundle_id), + "subject": nhs_number, + "filtering": {"requestingOrganisationODS": requesting_org}, + } + + _logger.debug( + "MNS event payload: %s", + # Mask subject value in logs + {k: "*" * len(v) if k == "subject" else v for k, v in event.items()}, + ) + + try: + response = session.post( + environment.values()["mns_url"], + json=event, + headers={"Content-Type": "application/cloudevents+json"}, + ) + except requests.RequestException as e: + raise MnsException("Failed to send request to MNS") from e + + if not response.ok: + raise MnsException( + f"Failed to create MNS event. status_code={response.status_code} " + f"response={response.text}" + ) + + try: + response_data = response.json() + except JSONDecodeError as e: + raise MnsException( + f"Failed to decode MNS response as JSON. response={response.text}" + ) from e + + if "id" not in response_data: + raise MnsException( + "MNS response does not contain a valid 'id' field. " + f"response={response_data}" + ) + + _logger.debug("MNS event published. event_id=%s", response_data["id"]) diff --git a/pathology-api/src/pathology_api/test_environment.py b/pathology-api/src/pathology_api/test_environment.py new file mode 100644 index 00000000..6f348511 --- /dev/null +++ b/pathology-api/src/pathology_api/test_environment.py @@ -0,0 +1,113 @@ +import os +from datetime import timedelta +from unittest.mock import MagicMock, call, patch + +from pathology_api import environment +from pathology_api.config import Duration, DurationUnit +from pathology_api.http import SessionManager + + +class TestEnvironment: + def setup_method(self) -> None: + # Clear any set environment variables + os.environ.clear() + + @patch("pathology_api.environment.parameters.get_secret") + def test_session_manager_with_mtls(self, get_secret_mock: MagicMock) -> None: + environment._session_manager = ( # noqa SLF001 - access private variable for testing purposes + None # reset session manager to force reinitialisation + ) + + get_secret_mock.side_effect = lambda secret_name: { + "mtls_cert_name": "mtls_cert", + "mtls_key_name": "mtls_key", + }[secret_name] + + os.environ["APIM_MTLS_CERT_NAME"] = "mtls_cert_name" + os.environ["APIM_MTLS_KEY_NAME"] = "mtls_key_name" + os.environ["CLIENT_TIMEOUT"] = "30s" + + certificate_name = "mtls_cert_name" + key_name = "mtls_key_name" + + session_manager = environment.session_manager() + client_certificate = session_manager._client_certificate # noqa SLF001 - access private attribute for testing purposes + + assert client_certificate == { + "certificate": "mtls_cert", + "key": "mtls_key", + } + + get_secret_mock.assert_has_calls([call(certificate_name), call(key_name)]) + + @patch("pathology_api.environment.parameters.get_secret") + def test_session_manager(self, get_secret_mock: MagicMock) -> None: + environment._session_manager = ( # noqa SLF001 - access private variable for testing purposes + None # reset session manager to force reinitialisation + ) + + os.environ["CLIENT_TIMEOUT"] = "30s" + + session_manager = environment.session_manager() + assert session_manager._client_certificate is None # noqa SLF001 - access private attribute for testing purposes + + get_secret_mock.assert_not_called() + + def test_values(self) -> None: + os.environ["CLIENT_TIMEOUT"] = "30s" + os.environ["APIM_TOKEN_URL"] = "token_url" # noqa S105 - dummy value + os.environ["APIM_PRIVATE_KEY_NAME"] = "private_key_name" + os.environ["APIM_API_KEY_NAME"] = "api_key_name" + os.environ["APIM_TOKEN_EXPIRY_THRESHOLD"] = "60s" # noqa S105 - dummy value + os.environ["APIM_KEY_ID"] = "key_id" + os.environ["PDM_BUNDLE_URL"] = "pdm_url" + os.environ["MNS_EVENT_URL"] = "mns_url" + + environ = environment.values() + + assert environ["client_timeout"].timedelta == timedelta(seconds=30) + assert environ["apim_token_url"] == "token_url" # noqa S105 - dummy value + assert environ["apim_private_key_name"] == "private_key_name" + assert environ["apim_api_key_name"] == "api_key_name" + assert environ["apim_token_expiry_threshold"].timedelta == timedelta(seconds=60) + assert environ["apim_key_id"] == "key_id" + assert environ["pdm_url"] == "pdm_url" + assert environ["mns_url"] == "mns_url" + + @patch("pathology_api.environment.parameters.get_secret") + @patch("pathology_api.environment.values") + @patch("pathology_api.environment.session_manager") + def test_apim_authenticator( + self, + session_manager_mock: MagicMock, + values_mock: MagicMock, + get_secret_mock: MagicMock, + ) -> None: + expected_session_manager = SessionManager(client_timeout=timedelta(seconds=30)) + session_manager_mock.return_value = expected_session_manager + + environ: environment.Environment = { + "apim_private_key_name": "private_key_name", + "apim_api_key_name": "api_key_name", + "apim_key_id": "key_id", + "apim_token_expiry_threshold": Duration(DurationUnit.SECONDS, 60), + "apim_token_url": "token_url", + "client_timeout": Duration(DurationUnit.SECONDS, 30), + "pdm_url": "pdm_url", + "mns_url": "mns_url", + } + + values_mock.return_value = environ + + get_secret_mock.side_effect = lambda secret_name: { + "private_key_name": "private_key", + "api_key_name": "api_key", + }[secret_name] + + apim_authenticator = environment.apim_authenticator() + assert apim_authenticator._private_key == "private_key" # noqa SLF001 - access private attribute for testing purposes + assert apim_authenticator._api_key == "api_key" # noqa SLF001 + assert apim_authenticator._key_id == "key_id" # noqa SLF001 + assert apim_authenticator._token_validity_threshold == timedelta(seconds=60) # noqa SLF001 + assert apim_authenticator._token_endpoint == "token_url" # noqa SLF001 + assert apim_authenticator._session_manager == expected_session_manager # noqa SLF001 diff --git a/pathology-api/src/pathology_api/test_handler.py b/pathology-api/src/pathology_api/test_handler.py index bd23c3d6..53337026 100644 --- a/pathology-api/src/pathology_api/test_handler.py +++ b/pathology-api/src/pathology_api/test_handler.py @@ -1,20 +1,10 @@ import datetime -import os from collections.abc import Callable from typing import Any -from unittest.mock import MagicMock, Mock, call, patch +from unittest.mock import patch import pytest from pydantic import Field -from requests.exceptions import RequestException - -os.environ["CLIENT_TIMEOUT"] = "1s" -os.environ["APIM_TOKEN_URL"] = "apim_url" # noqa S105 - dummy value -os.environ["APIM_PRIVATE_KEY_NAME"] = "apim_private_key_name" -os.environ["APIM_API_KEY_NAME"] = "apim_api_key_name" -os.environ["APIM_TOKEN_EXPIRY_THRESHOLD"] = "1s" # noqa S105 - dummy value -os.environ["APIM_KEY_ID"] = "apim_key" -os.environ["PDM_BUNDLE_URL"] = "pdm_bundle_url" from pathology_api.exception import ValidationError from pathology_api.fhir.r4.elements import ( @@ -34,30 +24,12 @@ ) from pathology_api.test_utils import BundleBuilder -mock_session = Mock() - - -def mock_auth(func: Callable[..., Any]) -> Callable[..., Any]: - - def wrapper(*args: Any, **kwargs: Any) -> Any: - return func(mock_session, *args, **kwargs) - - return wrapper - - with ( - patch("aws_lambda_powertools.utilities.parameters.get_secret") as get_secret_mock, - patch("pathology_api.apim.ApimAuthenticator") as apim_authenticator_mock, - patch("pathology_api.http.SessionManager") as session_manager_mock, + patch("pathology_api.environment.apim_authenticator"), + patch("pathology_api.mns.create_event") as create_event_mock, ): - apim_authenticator_mock.return_value.auth = mock_auth - get_secret_mock.side_effect = lambda secret_name: { - os.environ["APIM_PRIVATE_KEY_NAME"]: "private_key", - os.environ["APIM_API_KEY_NAME"]: "api_key", - "mtls_cert_name": "mtls_cert", - "mtls_key_name": "mtls_key", - }[secret_name] - from pathology_api.handler import _create_client_certificate, handle_request + from pathology_api.handler import handle_request + from pathology_api.mns import MnsException def _missing_resource_scenarios() -> list[Any]: @@ -240,11 +212,54 @@ def _invalid_organization_scenarios() -> list[Any]: class TestHandleRequest: - def setup_method(self) -> None: - mock_session.reset() + def _build_valid_test_result(self) -> Bundle: + organisation_entry = Bundle.Entry( + fullUrl="organisation", + resource=Organization.create( + identifier=[OrganizationIdentifier.from_ods_code("ods_code")] + ), + ) + + practitioner_role_entry = Bundle.Entry( + fullUrl="practitioner_role", + resource=PractitionerRole.create( + organization=LiteralReference(reference=organisation_entry.full_url) + ), + ) + + service_request_entry = Bundle.Entry( + fullUrl="service_request", + resource=ServiceRequest.create( + requester=LiteralReference(reference=practitioner_role_entry.full_url) + ), + ) + + composition = Composition.create( + subject=LogicalReference(PatientIdentifier.from_nhs_number("nhs_number_1")), + extension=[ + ReferenceExtension( + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + valueReference=LiteralReference(service_request_entry.full_url), + ) + ], + ) + + return Bundle.create( + type="document", + entry=[ + organisation_entry, + practitioner_role_entry, + service_request_entry, + Bundle.Entry( + fullUrl="composition", + resource=composition, + ), + ], + ) def test_handle_request( - self, build_valid_test_result: Callable[[str, str], Bundle] + self, + build_valid_test_result: Callable[[str, str], Bundle], ) -> None: # Arrange bundle = build_valid_test_result("nhs_number_1", "ods_code") @@ -270,31 +285,22 @@ def test_handle_request( assert created_meta.version_id is None - mock_session.post.assert_called_once_with(os.environ["PDM_BUNDLE_URL"]) - - session_manager_mock.assert_called_once_with( - client_timeout=datetime.timedelta(seconds=1), client_certificate=None + create_event_mock.assert_called_once_with( + requesting_org="ods_code", + nhs_number="nhs_number_1", + bundle_id=result_bundle.id, ) - apim_authenticator_mock.assert_called_once_with( - private_key="private_key", - key_id=os.environ["APIM_KEY_ID"], - api_key="api_key", - token_endpoint=os.environ["APIM_TOKEN_URL"], - token_validity_threshold=datetime.timedelta(seconds=1), - session_manager=session_manager_mock.return_value, - ) - - def test_handle_request_raises_error_when_send_request_fails( + def test_handle_request_raises_error_when_create_bundle_fails( self, build_valid_test_result: Callable[[str, str], Bundle] ) -> None: # Arrange bundle = build_valid_test_result("nhs_number_1", "ods_code") - expected_error_message = "Failed to send request" - mock_session.post.side_effect = RequestException(expected_error_message) + expected_error_message = "Failed to create bundle" + create_event_mock.side_effect = MnsException(expected_error_message) - with pytest.raises(RequestException, match=expected_error_message): + with pytest.raises(MnsException, match=expected_error_message): handle_request(bundle) @pytest.mark.parametrize( @@ -432,23 +438,3 @@ def test_handle_request_raises_error_when_bundle_not_document_type( match="Resource must be a bundle of type 'document'", ): handle_request(bundle) - - -@patch("pathology_api.handler.parameters.get_secret") -def test_create_client_certificate(get_secret_mock: MagicMock) -> None: - get_secret_mock.side_effect = lambda secret_name: { - "mtls_cert_name": "mtls_cert", - "mtls_key_name": "mtls_key", - }[secret_name] - - certificate_name = "mtls_cert_name" - key_name = "mtls_key_name" - - client_certificate = _create_client_certificate(certificate_name, key_name) - - assert client_certificate == { - "certificate": "mtls_cert", - "key": "mtls_key", - } - - get_secret_mock.assert_has_calls([call(certificate_name), call(key_name)]) diff --git a/pathology-api/src/pathology_api/test_http.py b/pathology-api/src/pathology_api/test_http.py index cc537f89..319a7883 100644 --- a/pathology-api/src/pathology_api/test_http.py +++ b/pathology-api/src/pathology_api/test_http.py @@ -6,6 +6,7 @@ import requests.adapters from pathology_api.http import SessionManager +from pathology_api.request_context import set_correlation_id class TestSessionManager: @@ -143,14 +144,18 @@ def mock_function(_: requests.Session) -> None: mock_session.return_value.mount.assert_called_once() mock_session.return_value.__exit__.assert_called_once() - def test_adapter_applies_timeout(self) -> None: + def test_adapter_applies_defaults(self) -> None: with patch.object( requests.adapters.HTTPAdapter, "send", autospec=True ) as mock_send: + expected_correlation_id = "correaltion-id" + set_correlation_id(expected_correlation_id) + expected_timeout = timedelta(seconds=30) adapter = SessionManager._Adapter(timeout=expected_timeout.total_seconds()) # noqa: SLF001 - Private access for testing mock_request = Mock() + mock_request.headers = {} expected_response = Mock() mock_send.return_value = expected_response @@ -165,6 +170,34 @@ def test_adapter_applies_timeout(self) -> None: timeout=expected_timeout.total_seconds(), ) + assert mock_request.headers["X-Correlation-ID"] == expected_correlation_id + + def test_adapter_overrides_defaults(self) -> None: + with patch.object( + requests.adapters.HTTPAdapter, "send", autospec=True + ) as mock_send: + expected_timeout = timedelta(seconds=30) + adapter = SessionManager._Adapter(timeout=expected_timeout.total_seconds()) # noqa: SLF001 - Private access for testing + + mock_request = Mock() + expected_correlation_id = "correaltion-id" + mock_request.headers = {"X-Correlation-ID": expected_correlation_id} + + expected_response = Mock() + mock_send.return_value = expected_response + + response = adapter.send(mock_request, verify=True) + assert response == expected_response + + mock_send.assert_called_once_with( + adapter, + mock_request, + verify=True, + timeout=expected_timeout.total_seconds(), + ) + + assert mock_request.headers["X-Correlation-ID"] == expected_correlation_id + def test_adapter_request_error(self) -> None: with patch.object( requests.adapters.HTTPAdapter, "send", autospec=True @@ -174,6 +207,9 @@ def test_adapter_request_error(self) -> None: adapter = SessionManager._Adapter(timeout=expected_timeout.total_seconds()) # noqa: SLF001 - Private access for testing mock_request = Mock() + mock_request.headers = {} + + set_correlation_id("test-correlation-id") with pytest.raises(requests.RequestException, match="request failed"): adapter.send(mock_request) diff --git a/pathology-api/src/pathology_api/test_logging.py b/pathology-api/src/pathology_api/test_logging.py index a204f339..b2c577ee 100644 --- a/pathology-api/src/pathology_api/test_logging.py +++ b/pathology-api/src/pathology_api/test_logging.py @@ -21,6 +21,10 @@ def _make_log_record() -> logging.LogRecord: class TestCorrelationIdFilter: + def setup_method(self) -> None: + # Ensure correlation ID is reset before each test + reset_correlation_id() + def test_filter_is_a_logging_filter_subclass(self) -> None: assert issubclass(_CorrelationIdFilter, logging.Filter) @@ -40,7 +44,6 @@ def test_filter_injects_active_correlation_id(self) -> None: record = _make_log_record() set_correlation_id("abc-123") f.filter(record) - reset_correlation_id() assert record.correlation_id == "abc-123" # type: ignore[attr-defined] def test_filter_injects_empty_string_after_correlation_id_reset( diff --git a/pathology-api/src/pathology_api/test_mns.py b/pathology-api/src/pathology_api/test_mns.py new file mode 100644 index 00000000..230b8a49 --- /dev/null +++ b/pathology-api/src/pathology_api/test_mns.py @@ -0,0 +1,210 @@ +import importlib +import uuid +from collections.abc import Callable +from datetime import datetime, timezone +from json import JSONDecodeError +from typing import Any +from unittest.mock import MagicMock, Mock, patch + +import pytest +import requests + +mock_session = Mock() + + +def _mock_auth() -> Callable[..., Any]: + def _auth_decorator(func: Callable[..., Any]) -> Callable[..., Any]: + def wrapper(*args: Any, **kwargs: Any) -> Any: + return func(mock_session, *args, **kwargs) + + return wrapper + + return _auth_decorator + + +with patch("pathology_api.environment.apim_authenticator") as apim_authenticator_mock: + import pathology_api.mns + + apim_authenticator_mock.return_value.auth = _mock_auth() + + # Reload the module to ensure the patched authenticator is used in case it has + # already been imported + importlib.reload(pathology_api.mns) + from pathology_api.mns import MnsException, create_event + + +class TestMns: + def setup_method(self) -> None: + mock_session.reset_mock(return_value=True, side_effect=True) + + @patch("pathology_api.environment.values") + @patch("pathology_api.mns.datetime") + @patch("pathology_api.mns.uuid") + def test_create_event( + self, uuid_mock: MagicMock, datetime_mock: MagicMock, env_values_mock: MagicMock + ) -> None: + mock_env: dict[str, Any] = { + "pdm_url": "pdm_url", + "mns_url": "mns_url", + } + + env_values_mock.return_value = mock_env + + response_value: dict[str, Any] = {"id": "event_id"} + mock_session.post.return_value.json.return_value = response_value + mock_session.post.return_value.ok = True + + datetime_mock.now.return_value = datetime(2026, 4, 10, tzinfo=timezone.utc) + + expected_uuid = uuid.uuid4() + uuid_mock.uuid4.return_value = expected_uuid + + create_event( + requesting_org="test_org", nhs_number="nhs_number", bundle_id="bundle_id" + ) + + mock_session.post.assert_called_once() + + assert mock_session.post.call_args.args[0] == mock_env["mns_url"] + assert mock_session.post.call_args.kwargs["headers"] == { + "Content-Type": "application/cloudevents+json" + } + + supplied_event = mock_session.post.call_args.kwargs["json"] + assert supplied_event["source"] == "uk.nhs.pathology-laboratory-reporting" + assert ( + supplied_event["type"] + == "pathology-laboratory-reporting-test-result-stored-1" + ) + assert supplied_event["dataref"] == mock_env["pdm_url"] + "/bundle_id" + assert supplied_event["subject"] == "nhs_number" + assert supplied_event["filtering"] == {"requestingOrganisationODS": "test_org"} + assert supplied_event["time"] == "2026-04-10T00:00:00+00:00" + + assert supplied_event["id"] == str(expected_uuid) + + @patch("pathology_api.environment.values") + @patch("pathology_api.mns.datetime") + @patch("pathology_api.mns.uuid") + def test_create_event_request_failure( + self, uuid_mock: MagicMock, datetime_mock: MagicMock, env_values_mock: MagicMock + ) -> None: + mock_env: dict[str, Any] = { + "pdm_url": "pdm_url", + "mns_url": "mns_url", + } + + env_values_mock.return_value = mock_env + + mock_session.post.return_value.status_code = 400 + mock_session.post.return_value.text = "Response text" + mock_session.post.return_value.ok = False + + datetime_mock.now.return_value = datetime(2026, 4, 10, tzinfo=timezone.utc) + + expected_uuid = uuid.uuid4() + uuid_mock.uuid4.return_value = expected_uuid + + with pytest.raises( + MnsException, + match=r"Failed to create MNS event. status_code=400 response=Response text", + ): + create_event( + requesting_org="test_org", + nhs_number="nhs_number", + bundle_id="bundle_id", + ) + + @patch("pathology_api.environment.values") + @patch("pathology_api.mns.datetime") + @patch("pathology_api.mns.uuid") + def test_create_event_request_throws_exception( + self, uuid_mock: MagicMock, datetime_mock: MagicMock, env_values_mock: MagicMock + ) -> None: + mock_env: dict[str, Any] = { + "pdm_url": "pdm_url", + "mns_url": "mns_url", + } + + env_values_mock.return_value = mock_env + + mock_session.post.side_effect = requests.RequestException("Request failed") + + datetime_mock.now.return_value = datetime(2026, 4, 10, tzinfo=timezone.utc) + + expected_uuid = uuid.uuid4() + uuid_mock.uuid4.return_value = expected_uuid + + with pytest.raises(MnsException, match="Failed to send request to MNS"): + create_event( + requesting_org="test_org", + nhs_number="nhs_number", + bundle_id="bundle_id", + ) + + @patch("pathology_api.environment.values") + @patch("pathology_api.mns.datetime") + @patch("pathology_api.mns.uuid") + def test_create_event_json_exception( + self, uuid_mock: MagicMock, datetime_mock: MagicMock, env_values_mock: MagicMock + ) -> None: + mock_env: dict[str, Any] = { + "pdm_url": "pdm_url", + "mns_url": "mns_url", + } + + env_values_mock.return_value = mock_env + + mock_session.post.return_value.json.side_effect = JSONDecodeError( + "Expecting value", "doc", 0 + ) + mock_session.post.return_value.text = "Response text" + mock_session.post.return_value.ok = True + + datetime_mock.now.return_value = datetime(2026, 4, 10, tzinfo=timezone.utc) + + expected_uuid = uuid.uuid4() + uuid_mock.uuid4.return_value = expected_uuid + + with pytest.raises( + MnsException, + match=r"Failed to decode MNS response as JSON. response=Response text", + ): + create_event( + requesting_org="test_org", + nhs_number="nhs_number", + bundle_id="bundle_id", + ) + + @patch("pathology_api.environment.values") + @patch("pathology_api.mns.datetime") + @patch("pathology_api.mns.uuid") + def test_create_event_response_missing_id( + self, uuid_mock: MagicMock, datetime_mock: MagicMock, env_values_mock: MagicMock + ) -> None: + mock_env: dict[str, Any] = { + "pdm_url": "pdm_url", + "mns_url": "mns_url", + } + + env_values_mock.return_value = mock_env + + mock_session.post.return_value.json.return_value = {"not_id": "value"} + mock_session.post.return_value.text = "Response text" + mock_session.post.return_value.ok = True + + datetime_mock.now.return_value = datetime(2026, 4, 10, tzinfo=timezone.utc) + + expected_uuid = uuid.uuid4() + uuid_mock.uuid4.return_value = expected_uuid + + with pytest.raises( + MnsException, + match=r"MNS response does not contain a valid 'id' field. " + r"response={'not_id': 'value'}", + ): + create_event( + requesting_org="test_org", + nhs_number="nhs_number", + bundle_id="bundle_id", + ) diff --git a/pathology-api/test_lambda_handler.py b/pathology-api/test_lambda_handler.py index 9c9e6611..fe8608be 100644 --- a/pathology-api/test_lambda_handler.py +++ b/pathology-api/test_lambda_handler.py @@ -1,24 +1,17 @@ import logging -import os from collections.abc import Callable from typing import Any from unittest.mock import MagicMock, patch import pydantic import pytest - -os.environ["CLIENT_TIMEOUT"] = "1s" -os.environ["APIM_TOKEN_URL"] = "apim_url" # noqa S105 - dummy value -os.environ["APIM_PRIVATE_KEY_NAME"] = "apim_private_key_name" -os.environ["APIM_API_KEY_NAME"] = "apim_api_key_name" -os.environ["APIM_TOKEN_EXPIRY_THRESHOLD"] = "1s" # noqa S105 - dummy value -os.environ["APIM_KEY_ID"] = "apim_key" -os.environ["PDM_BUNDLE_URL"] = "pdm_bundle_url" - from aws_lambda_powertools.utilities.typing import LambdaContext -with patch("aws_lambda_powertools.utilities.parameters.get_secret") as get_secret_mock: +with ( + patch("pathology_api.environment.apim_authenticator"), +): from lambda_handler import handler + from pathology_api.mns import MnsException from pathology_api.exception import ValidationError from pathology_api.fhir.r4.elements import Meta @@ -114,7 +107,7 @@ def test_correlation_id_is_set_on_all_log_records_during_request( ) with ( - patch("pathology_api.handler._send_request"), + patch("pathology_api.mns.create_event"), caplog.at_level(logging.DEBUG), ): handler(event, context) @@ -137,7 +130,7 @@ def test_correlation_id_is_cleared_after_request( headers={"nhsd-correlation-id": "c876145d-1ebf-4e22-8ff8-275b570c1ec4"}, ) with ( - patch("pathology_api.handler._send_request"), + patch("pathology_api.mns.create_event"), caplog.at_level(logging.DEBUG), ): handler(event, context) @@ -151,7 +144,7 @@ def test_correlation_id_is_cleared_after_request( headers={"nhsd-correlation-id": "d876145d-1ebf-4e22-8ff8-275b570c1ec4"}, ) with ( - patch("pathology_api.handler._send_request"), + patch("pathology_api.mns.create_event"), caplog.at_level(logging.DEBUG), ): handler(event2, context) @@ -203,29 +196,6 @@ def test_empty_correlation_id_header_returns_500( == "Missing required header: nhsd-correlation-id" ) - @pytest.mark.parametrize( - "malicious_value", - [ - "abc\ndef", # newline injection - '{"key": "value"}', # JSON fragment injection - "a" * 200, # oversized input - ], - ) - def test_malicious_correlation_id_values_are_rejected( - self, - malicious_value: str, - bundle: Bundle, - context: LambdaContext, - ) -> None: - event = self._create_test_event( - body=bundle.model_dump_json(by_alias=True), - path_params="FHIR/R4/Bundle", - request_method="POST", - headers={"nhsd-correlation-id": malicious_value}, - ) - response = handler(event, context) - assert response["statusCode"] == 500 - def test_correlation_id_is_cleared_after_exception_mid_handler( self, context: LambdaContext ) -> None: @@ -327,18 +297,30 @@ def test_create_test_result_invalid_json(self, context: LambdaContext) -> None: 500, id="Unexpected exception", ), + pytest.param( + MnsException("Test MNS error"), + { + "severity": "fatal", + "code": "exception", + "diagnostics": "Failed to publish an event", + }, + 500, + id="MnsException", + ), ], ) + @patch("lambda_handler.handle_request") def test_create_test_result_processing_error( self, + handle_request_mock: MagicMock, error: type[Exception], expected_issue: OperationOutcome.Issue, expected_status_code: int, post_event: dict[str, Any], context: LambdaContext, ) -> None: - with patch("lambda_handler.handle_request", side_effect=error): - response = handler(post_event, context) + handle_request_mock.side_effect = error + response = handler(post_event, context) assert response["statusCode"] == expected_status_code assert response["headers"]["Content-Type"] == "application/fhir+json" diff --git a/pathology-api/tests/integration/test_endpoints.py b/pathology-api/tests/integration/test_endpoints.py index 1b793ea6..c11e4a2e 100644 --- a/pathology-api/tests/integration/test_endpoints.py +++ b/pathology-api/tests/integration/test_endpoints.py @@ -7,6 +7,7 @@ import pytest from pathology_api.fhir.r4.resources import ( Bundle, + OperationOutcome, ) from pydantic import BaseModel, HttpUrl @@ -880,6 +881,51 @@ def test_invalid_organization_resource( ], } + @pytest.mark.parametrize( + ("subject"), + [ + "MNS_VALIDATION_ERROR", + "MNS_AUTHENTICATION_ERROR", + "MNS_SERVER_ERROR", + "MNS_AUTHORIZATION_ERROR", + "MNS_BAD_GATEWAY_ERROR", + "MNS_GATEWAY_TIMEOUT_ERROR", + ], + ) + def test_unexpected_mns_response( + self, + subject: str, + client: Client, + build_valid_test_result: Callable[[str, str], Bundle], + ) -> None: + bundle = build_valid_test_result(subject, "ods_code") + response = client.send( + data=bundle.model_dump_json(by_alias=True), + path="FHIR/R4/Bundle", + request_method="POST", + headers={"X-Correlation-ID": "bb038f9a-dc45-49e1-bcfd-3ab3c3de5e16"}, + ) + + assert response.status_code == 500 + assert response.headers["Content-Type"] == "application/fhir+json" + assert ( + response.headers["X-Correlation-ID"] + == "bb038f9a-dc45-49e1-bcfd-3ab3c3de5e16" + ) + + assert response.status_code == 500 + + response_data = response.json() + operation_outcome = OperationOutcome.model_validate(response_data) + + issue: OperationOutcome.Issue = { + "severity": "fatal", + "code": "exception", + "diagnostics": "Failed to publish an event", + } + + assert operation_outcome.issue == [issue] + @pytest.mark.remote_only class TestStatusEndpoint: From 3e0fcc8d27378e9c4154cda96fb75fcbdd39e3c8 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Thu, 16 Apr 2026 16:26:51 +0000 Subject: [PATCH 2/2] [CDAPI-113]: Added missing unit test coverage for MNS mock when returning empty payloads --- mocks/src/mns_mock/test_handler.py | 31 ++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/mocks/src/mns_mock/test_handler.py b/mocks/src/mns_mock/test_handler.py index 53474239..80ec68b3 100644 --- a/mocks/src/mns_mock/test_handler.py +++ b/mocks/src/mns_mock/test_handler.py @@ -124,6 +124,14 @@ def test_handle_post_request_success( "MNS_SERVER_ERROR", {"status_code": 500, "response": {"errors": "Internal server error"}}, ), + ( + "MNS_BAD_GATEWAY_ERROR", + {"status_code": 502, "response": None}, + ), + ( + "MNS_GATEWAY_TIMEOUT_ERROR", + {"status_code": 504, "response": None}, + ), ], ) def test_handle_post_request_error_responses( @@ -196,6 +204,29 @@ def test_create_event_success( assert response["headers"] == {"Content-Type": "application/fhir+json"} assert json.loads(response["body"]) == {"id": basic_event_payload["id"]} + @patch("mns_mock.handler.check_authenticated", new=MagicMock(return_value=None)) + def test_create_event_without_response_payload( + self, + basic_event_payload: dict[str, Any], + lambda_app: APIGatewayHttpResolver, + ) -> None: + payload = basic_event_payload | {"subject": "MNS_BAD_GATEWAY_ERROR"} + + event = self._create_test_event( + body=json.dumps(payload), + path_params="mns/events", + request_method="POST", + headers={"Authorization": "Bearer token"}, + ) + context = LambdaContext() + + with patch("mns_mock.handler.storage_helper"): + response = lambda_app.resolve(event, context) + + assert response["statusCode"] == 502 + assert not response["headers"] + assert response["body"] is None + @patch("mns_mock.handler.check_authenticated") def test_create_event_fails_authentication( self,