From 279ad7013f45256e82ef7e64af9b4de5af623ff4 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Wed, 28 Jan 2026 17:06:46 +0000 Subject: [PATCH 01/20] [CDAPI-95]: Added Initial Composition Resource --- pathology-api/src/pathology_api/fhir/r4/elements.py | 5 +++++ pathology-api/src/pathology_api/fhir/r4/resources.py | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/pathology-api/src/pathology_api/fhir/r4/elements.py b/pathology-api/src/pathology_api/fhir/r4/elements.py index b337bdf..0a1f885 100644 --- a/pathology-api/src/pathology_api/fhir/r4/elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/elements.py @@ -69,3 +69,8 @@ def __init__(self, value: uuid.UUID | None = None): value=str(value or uuid.uuid4()), system=self._expected_system, ) + + +@dataclass(frozen=True) +class LogicalReference[T: Identifier]: + identifier: T diff --git a/pathology-api/src/pathology_api/fhir/r4/resources.py b/pathology-api/src/pathology_api/fhir/r4/resources.py index 13f3d73..e2b1c8a 100644 --- a/pathology-api/src/pathology_api/fhir/r4/resources.py +++ b/pathology-api/src/pathology_api/fhir/r4/resources.py @@ -9,7 +9,7 @@ model_validator, ) -from .elements import Identifier, Meta, UUIDIdentifier +from .elements import Identifier, LogicalReference, Meta, UUIDIdentifier class Resource(BaseModel): @@ -126,3 +126,9 @@ def from_nhs_number(cls, nhs_number: str) -> "Patient.PatientIdentifier": return cls(value=nhs_number) identifier: Annotated[PatientIdentifier, Field(frozen=True)] + + +class Composition(Resource, resource_type="Composition"): + """A FHIR R4 Composition resource.""" + + subject: Annotated[LogicalReference[Patient.PatientIdentifier], Field(frozen=True)] From 6bc114f9cdf69ff46b2b173e5268de1c8b8b3644 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Thu, 29 Jan 2026 09:59:34 +0000 Subject: [PATCH 02/20] [CDAPI-95]: Added initial exception handlers to lambda_handler --- pathology-api/lambda_handler.py | 82 +++++++++++++--------- pathology-api/src/pathology_api/handler.py | 12 +++- 2 files changed, 60 insertions(+), 34 deletions(-) diff --git a/pathology-api/lambda_handler.py b/pathology-api/lambda_handler.py index 6692f6a..8cbe255 100644 --- a/pathology-api/lambda_handler.py +++ b/pathology-api/lambda_handler.py @@ -1,6 +1,5 @@ -import json import logging -from functools import reduce +from collections.abc import Callable from typing import Any from aws_lambda_powertools.event_handler import ( @@ -10,7 +9,6 @@ from aws_lambda_powertools.utilities.typing import LambdaContext from pathology_api.fhir.r4.resources import Bundle from pathology_api.handler import handle_request -from pydantic import ValidationError _INVALID_PAYLOAD_MESSAGE = "Invalid payload provided." @@ -18,6 +16,26 @@ app = APIGatewayHttpResolver() +type _ExceptionHandler[T: Exception] = Callable[[T], Response[str]] + + +def _exception_handler[T: Exception]( + exception_type: type[T], +) -> Callable[[_ExceptionHandler[T]], _ExceptionHandler[T]]: + """ + Exception handler decorator that registers a function as an exception handler with + the created app whilst maintaining type information. + """ + + def decorator(func: _ExceptionHandler[T]) -> _ExceptionHandler[T]: + def wrapper(exception: T) -> Response[str]: + return func(exception) + + app.exception_handler(exception_type)(wrapper) + return wrapper + + return decorator + def _with_default_headers(status_code: int, body: str) -> Response[str]: content_type = "application/fhir+json" if status_code == 200 else "text/plain" @@ -28,6 +46,25 @@ def _with_default_headers(status_code: int, body: str) -> Response[str]: ) +@_exception_handler(ValueError) +def handle_value_error(exception: ValueError) -> Response[str]: + """Exception handler for handling ValueError exceptions.""" + + # LOG014: False positive, we are within an exception handler here. + _logger.warning("ValueError encountered: %s", exception, exc_info=True) # noqa: LOG014 + return _with_default_headers(status_code=400, body="Invalid payload provided.") + + +@_exception_handler(Exception) +def handle_exception(exception: Exception) -> Response[str]: + """General default exception handler for handling any unhandled exceptions.""" + _logger.exception("Unhandled Exception encountered: %s", exception) + return _with_default_headers( + status_code=500, + body="An unexpected error has occurred. Please try again later.", + ) + + @app.get("/_status") def status() -> Response[str]: _logger.debug("Status check endpoint called") @@ -37,42 +74,21 @@ def status() -> Response[str]: @app.post("/FHIR/R4/Bundle") def post_result() -> Response[str]: _logger.debug("Post result endpoint called.") - try: - payload = app.current_event.json_body - except json.JSONDecodeError as err: - _logger.error("Error decoding JSON payload. error: %s", err) - return _with_default_headers(status_code=400, body=_INVALID_PAYLOAD_MESSAGE) + payload = app.current_event.json_body _logger.debug("Payload received: %s", payload) if not payload: _logger.error("No payload provided.") return _with_default_headers(status_code=400, body="No payload provided.") - try: - bundle = Bundle.model_validate(payload, by_alias=True) - except ValidationError as err: - _logger.error( - "Error parsing payload. error: %s issues: %s", - err, - reduce(lambda acc, e: acc + "," + str(e), err.errors(), ""), - ) - return _with_default_headers(status_code=400, body=_INVALID_PAYLOAD_MESSAGE) - except TypeError as err: - _logger.error("Error parsing payload. error: %s", err) - return _with_default_headers(status_code=400, body=_INVALID_PAYLOAD_MESSAGE) - - try: - response = handle_request(bundle) - - return _with_default_headers( - status_code=200, - body=response.model_dump_json(by_alias=True, exclude_none=True), - ) - except ValueError as err: - _logger.error("Error processing payload. error: %s", err) - return _with_default_headers( - status_code=400, body="Error processing provided bundle." - ) + bundle = Bundle.model_validate(payload, by_alias=True) + + response = handle_request(bundle) + + return _with_default_headers( + status_code=200, + body=response.model_dump_json(by_alias=True, exclude_none=True), + ) def handler(data: dict[str, Any], context: LambdaContext) -> dict[str, Any]: diff --git a/pathology-api/src/pathology_api/handler.py b/pathology-api/src/pathology_api/handler.py index f9cc0f0..923c8f8 100644 --- a/pathology-api/src/pathology_api/handler.py +++ b/pathology-api/src/pathology_api/handler.py @@ -2,7 +2,7 @@ from collections.abc import Callable from pathology_api.fhir.r4.elements import Meta, UUIDIdentifier -from pathology_api.fhir.r4.resources import Bundle, Patient +from pathology_api.fhir.r4.resources import Bundle, Composition, Patient _logger = logging.getLogger(__name__) @@ -25,9 +25,19 @@ def _ensure_test_result_references_patient(bundle: Bundle) -> None: ) +def _ensure_bundle_composition(bundle: Bundle) -> None: + compositions = bundle.find_resources(t=Composition) + if len(compositions) != 1: + raise ValueError("Bundle must contain exactly one Composition resource.") + + composition = compositions[0] + _logger.debug("Composition provided: %s", composition) + + type ValidationFunction = Callable[[Bundle], None] _validation_functions: list[ValidationFunction] = [ _ensure_test_result_references_patient, + _ensure_bundle_composition, ] From f177416c7387a522d64ec098358ee56bdf3b6a6b Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Thu, 29 Jan 2026 12:05:32 +0000 Subject: [PATCH 03/20] [CDAPI-95]: Swapped logging implementation to use logger provided by aws-lambda-powertools --- pathology-api/lambda_handler.py | 6 ++---- pathology-api/src/pathology_api/__init__.py | 20 ------------------- pathology-api/src/pathology_api/handler.py | 4 ++-- pathology-api/src/pathology_api/logging.py | 22 +++++++++++++++++++++ 4 files changed, 26 insertions(+), 26 deletions(-) create mode 100644 pathology-api/src/pathology_api/logging.py diff --git a/pathology-api/lambda_handler.py b/pathology-api/lambda_handler.py index 8cbe255..5bcc42b 100644 --- a/pathology-api/lambda_handler.py +++ b/pathology-api/lambda_handler.py @@ -1,4 +1,3 @@ -import logging from collections.abc import Callable from typing import Any @@ -9,10 +8,9 @@ from aws_lambda_powertools.utilities.typing import LambdaContext from pathology_api.fhir.r4.resources import Bundle from pathology_api.handler import handle_request +from pathology_api.logging import get_logger -_INVALID_PAYLOAD_MESSAGE = "Invalid payload provided." - -_logger = logging.getLogger(__name__) +_logger = get_logger(__name__) app = APIGatewayHttpResolver() diff --git a/pathology-api/src/pathology_api/__init__.py b/pathology-api/src/pathology_api/__init__.py index 87a7a95..e69de29 100644 --- a/pathology-api/src/pathology_api/__init__.py +++ b/pathology-api/src/pathology_api/__init__.py @@ -1,20 +0,0 @@ -import logging.config - -logging.config.dictConfig( - { - "version": 1, - "formatters": { - "default": { - "format": "[%(asctime)s] %(levelname)s - %(module)s: %(message)s", - }, - }, - "handlers": { - "stdout": { - "class": "logging.StreamHandler", - "stream": "ext://sys.stdout", - "formatter": "default", - } - }, - "root": {"level": "DEBUG", "handlers": ["stdout"]}, - } -) diff --git a/pathology-api/src/pathology_api/handler.py b/pathology-api/src/pathology_api/handler.py index 923c8f8..d508e1f 100644 --- a/pathology-api/src/pathology_api/handler.py +++ b/pathology-api/src/pathology_api/handler.py @@ -1,10 +1,10 @@ -import logging from collections.abc import Callable from pathology_api.fhir.r4.elements import Meta, UUIDIdentifier from pathology_api.fhir.r4.resources import Bundle, Composition, Patient +from pathology_api.logging import get_logger -_logger = logging.getLogger(__name__) +_logger = get_logger(__name__) def _ensure_test_result_references_patient(bundle: Bundle) -> None: diff --git a/pathology-api/src/pathology_api/logging.py b/pathology-api/src/pathology_api/logging.py new file mode 100644 index 0000000..d094698 --- /dev/null +++ b/pathology-api/src/pathology_api/logging.py @@ -0,0 +1,22 @@ +from typing import Any, Protocol + +from aws_lambda_powertools import Logger + + +class LogProvider(Protocol): + """Protocol defining required contract for a logger.""" + + def debug(self, msg: str, *args: Any, **kwargs: Any) -> None: ... + + def info(self, msg: str, *args: Any, **kwargs: Any) -> None: ... + + def warning(self, msg: str, *args: Any, **kwargs: Any) -> None: ... + + def error(self, msg: str, *args: Any, **kwargs: Any) -> None: ... + + def exception(self, msg: str, *args: Any, **kwargs: Any) -> None: ... + + +def get_logger(service: str) -> LogProvider: + """Get a configured logger instance.""" + return Logger(service=service, level="DEBUG", serialize_stacktrace=True) From 0eb5abb28c24e5443facc0b918f6bad1d21a5a64 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Thu, 29 Jan 2026 15:38:07 +0000 Subject: [PATCH 04/20] [CDAPI-95]: Added additional resource classes to FHIR R4 module --- .../src/pathology_api/fhir/r4/elements.py | 14 +++++ .../src/pathology_api/fhir/r4/resources.py | 39 +++++++++----- .../pathology_api/fhir/r4/test_elements.py | 12 ++++- .../pathology_api/fhir/r4/test_resources.py | 53 +++++++------------ pathology-api/src/pathology_api/handler.py | 21 +------- .../src/pathology_api/test_handler.py | 45 +++++++++------- pathology-api/test_lambda_handler.py | 11 ++-- .../acceptance/steps/bundle_endpoint_steps.py | 11 ++-- pathology-api/tests/integration/test_main.py | 15 +++--- 9 files changed, 113 insertions(+), 108 deletions(-) diff --git a/pathology-api/src/pathology_api/fhir/r4/elements.py b/pathology-api/src/pathology_api/fhir/r4/elements.py index 0a1f885..cb3352f 100644 --- a/pathology-api/src/pathology_api/fhir/r4/elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/elements.py @@ -71,6 +71,20 @@ def __init__(self, value: uuid.UUID | None = None): ) +class PatientIdentifier( + Identifier, expected_system="https://fhir.nhs.uk/Id/nhs-number" +): + """A FHIR R4 Patient Identifier utilising the NHS Number system.""" + + def __init__(self, value: str): + super().__init__(value=value, system=self._expected_system) + + @classmethod + def from_nhs_number(cls, nhs_number: str) -> "PatientIdentifier": + """Create a PatientIdentifier from an NHS number.""" + return cls(value=nhs_number) + + @dataclass(frozen=True) class LogicalReference[T: Identifier]: identifier: T diff --git a/pathology-api/src/pathology_api/fhir/r4/resources.py b/pathology-api/src/pathology_api/fhir/r4/resources.py index e2b1c8a..9cf98e5 100644 --- a/pathology-api/src/pathology_api/fhir/r4/resources.py +++ b/pathology-api/src/pathology_api/fhir/r4/resources.py @@ -9,7 +9,7 @@ model_validator, ) -from .elements import Identifier, LogicalReference, Meta, UUIDIdentifier +from .elements import LogicalReference, Meta, PatientIdentifier, UUIDIdentifier class Resource(BaseModel): @@ -112,23 +112,36 @@ def empty(cls, bundle_type: BundleType) -> "Bundle": class Patient(Resource, resource_type="Patient"): """A FHIR R4 Patient resource.""" - class PatientIdentifier( - Identifier, expected_system="https://fhir.nhs.uk/Id/nhs-number" - ): - """A FHIR R4 Patient Identifier utilising the NHS Number system.""" - def __init__(self, value: str): - super().__init__(value=value, system=self._expected_system) +class ServiceRequest(Resource, resource_type="ServiceRequest"): + """A FHIR R4 ServiceRequest resource.""" - @classmethod - def from_nhs_number(cls, nhs_number: str) -> "Patient.PatientIdentifier": - """Create a PatientIdentifier from an NHS number.""" - return cls(value=nhs_number) - identifier: Annotated[PatientIdentifier, Field(frozen=True)] +class DiagnosticReport(Resource, resource_type="DiagnosticReport"): + """A FHIR R4 DiagnosticReport resource.""" + + +class Organization(Resource, resource_type="Organization"): + """A FHIR R4 Organization resource.""" + + +class Practitioner(Resource, resource_type="Practitioner"): + """A FHIR R4 Practitioner resource.""" + + +class PractitionerRole(Resource, resource_type="PractitionerRole"): + """A FHIR R4 PractitionerRole resource.""" + + +class Observation(Resource, resource_type="Observation"): + """A FHIR R4 Observation resource.""" + + +class Specimen(Resource, resource_type="Specimen"): + """A FHIR R4 Specimen resource.""" class Composition(Resource, resource_type="Composition"): """A FHIR R4 Composition resource.""" - subject: Annotated[LogicalReference[Patient.PatientIdentifier], Field(frozen=True)] + subject: Annotated[LogicalReference[PatientIdentifier], Field(frozen=True)] diff --git a/pathology-api/src/pathology_api/fhir/r4/test_elements.py b/pathology-api/src/pathology_api/fhir/r4/test_elements.py index d5e7c4d..858f9b0 100644 --- a/pathology-api/src/pathology_api/fhir/r4/test_elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/test_elements.py @@ -4,7 +4,7 @@ import pytest from pydantic import BaseModel -from .elements import Identifier, Meta, UUIDIdentifier +from .elements import Identifier, Meta, PatientIdentifier, UUIDIdentifier class TestMeta: @@ -86,3 +86,13 @@ class _TestContainer(BaseModel): _TestContainer.model_validate( {"identifier": {"system": "invalid-system", "value": "some-value"}} ) + + +class TestPatientIdentifier: + def test_create_from_nhs_number(self) -> None: + """Test creating a PatientIdentifier from an NHS number.""" + nhs_number = "1234567890" + identifier = PatientIdentifier.from_nhs_number(nhs_number) + + assert identifier.system == "https://fhir.nhs.uk/Id/nhs-number" + assert identifier.value == nhs_number diff --git a/pathology-api/src/pathology_api/fhir/r4/test_resources.py b/pathology-api/src/pathology_api/fhir/r4/test_resources.py index b0632ac..156d6ea 100644 --- a/pathology-api/src/pathology_api/fhir/r4/test_resources.py +++ b/pathology-api/src/pathology_api/fhir/r4/test_resources.py @@ -4,7 +4,9 @@ import pytest from pydantic import BaseModel -from .resources import Bundle, Patient, Resource +from pathology_api.fhir.r4.elements import PatientIdentifier + +from .resources import Bundle, Composition, Patient, Resource class TestResource: @@ -17,22 +19,24 @@ def test_resource_deserialisation(self) -> None: example_json = json.dumps( { "resource": { - "resourceType": "Patient", - "identifier": { - "system": expected_system, - "value": expected_nhs_number, + "resourceType": "Composition", + "subject": { + "identifier": { + "system": expected_system, + "value": expected_nhs_number, + } }, } } ) created_object = self._TestContainer.model_validate_json(example_json) - assert isinstance(created_object.resource, Patient) + assert isinstance(created_object.resource, Composition) - created_patient = created_object.resource - assert created_patient.identifier is not None - assert created_patient.identifier.system == expected_system - assert created_patient.identifier.value == expected_nhs_number + created_composition = created_object.resource + assert created_composition.subject is not None + assert created_composition.subject.identifier.system == expected_system + assert created_composition.subject.identifier.value == expected_nhs_number def test_resource_deserialisation_unknown_resource(self) -> None: expected_resource_type = "UnknownResourceType" @@ -108,8 +112,8 @@ def test_create(self) -> None: """Test creating a Bundle resource.""" expected_entry = Bundle.Entry( fullUrl="full", - resource=Patient.create( - identifier=Patient.PatientIdentifier.from_nhs_number("nhs_number") + resource=Composition.create( + subject=PatientIdentifier.from_nhs_number("nhs_number") ), ) @@ -130,8 +134,8 @@ def test_create_without_entries(self) -> None: assert bundle.identifier is None assert bundle.entries is None - expected_resource = Patient.create( - identifier=Patient.PatientIdentifier.from_nhs_number("nhs_number") + expected_resource = Composition.create( + subject=PatientIdentifier.from_nhs_number("nhs_number") ) @pytest.mark.parametrize( @@ -199,24 +203,3 @@ def test_find_resources_returns_empty_list(self, bundle: Bundle) -> None: """ result = bundle.find_resources(Patient) assert result == [] - - -class TestPatient: - def test_create(self) -> None: - """Test creating a Patient resource.""" - nhs_number = "1234567890" - - expected_identifier = Patient.PatientIdentifier.from_nhs_number(nhs_number) - patient = Patient.create(identifier=expected_identifier) - - assert patient.identifier == expected_identifier - - -class TestPatientIdentifier: - def test_create_from_nhs_number(self) -> None: - """Test creating a PatientIdentifier from an NHS number.""" - nhs_number = "1234567890" - identifier = Patient.PatientIdentifier.from_nhs_number(nhs_number) - - assert identifier.system == "https://fhir.nhs.uk/Id/nhs-number" - assert identifier.value == nhs_number diff --git a/pathology-api/src/pathology_api/handler.py b/pathology-api/src/pathology_api/handler.py index d508e1f..9aad084 100644 --- a/pathology-api/src/pathology_api/handler.py +++ b/pathology-api/src/pathology_api/handler.py @@ -1,30 +1,12 @@ from collections.abc import Callable from pathology_api.fhir.r4.elements import Meta, UUIDIdentifier -from pathology_api.fhir.r4.resources import Bundle, Composition, Patient +from pathology_api.fhir.r4.resources import Bundle, Composition from pathology_api.logging import get_logger _logger = get_logger(__name__) -def _ensure_test_result_references_patient(bundle: Bundle) -> None: - patient_references = [ - patient.identifier for patient in bundle.find_resources(t=Patient) - ] - if not patient_references: - raise ValueError( - "Test Result Bundle must reference at least one Patient resource." - ) - - _logger.debug("Bundle.entries %s", bundle.entries) - _logger.debug("Patient references found: %s", patient_references) - - if len(patient_references) > 1: - raise ValueError( - "Test Result Bundle must not reference more than one Patient resource." - ) - - def _ensure_bundle_composition(bundle: Bundle) -> None: compositions = bundle.find_resources(t=Composition) if len(compositions) != 1: @@ -36,7 +18,6 @@ def _ensure_bundle_composition(bundle: Bundle) -> None: type ValidationFunction = Callable[[Bundle], None] _validation_functions: list[ValidationFunction] = [ - _ensure_test_result_references_patient, _ensure_bundle_composition, ] diff --git a/pathology-api/src/pathology_api/test_handler.py b/pathology-api/src/pathology_api/test_handler.py index 16504c4..03f7d1e 100644 --- a/pathology-api/src/pathology_api/test_handler.py +++ b/pathology-api/src/pathology_api/test_handler.py @@ -2,8 +2,8 @@ import pytest -from pathology_api.fhir.r4.elements import UUIDIdentifier -from pathology_api.fhir.r4.resources import Bundle, Patient +from pathology_api.fhir.r4.elements import PatientIdentifier, UUIDIdentifier +from pathology_api.fhir.r4.resources import Bundle, Composition from pathology_api.handler import handle_request @@ -15,10 +15,8 @@ def test_handle_request(self) -> None: entry=[ Bundle.Entry( fullUrl="patient", - resource=Patient.create( - identifier=Patient.PatientIdentifier.from_nhs_number( - "nhs_number" - ) + resource=Composition.create( + subject=PatientIdentifier.from_nhs_number("nhs_number") ), ) ], @@ -57,34 +55,40 @@ def test_handle_request_raises_error_when_no_patient_resource(self) -> None: with pytest.raises( ValueError, - match="Test Result Bundle must reference at least one Patient resource.", + match="Test Result Bundle must reference at least one Composition " + "resource.", ): handle_request(bundle) - def test_handle_request_raises_error_when_multiple_patient_resources( + def test_handle_request_raises_error_when_multiple_composition_resources( self, ) -> None: - patient = Patient.create( - identifier=Patient.PatientIdentifier.from_nhs_number("nhs_number_1") + """ + Test that handle_request raises ValueError when bundle has multiple Composition + resources. + """ + # Arrange + composition = Composition.create( + subject=PatientIdentifier.from_nhs_number("nhs_number_1") ) bundle = Bundle.create( type="transaction", entry=[ Bundle.Entry( - fullUrl="patient1", - resource=patient, + fullUrl="composition1", + resource=composition, ), Bundle.Entry( - fullUrl="patient2", - resource=patient, + fullUrl="composition2", + resource=composition, ), ], ) with pytest.raises( ValueError, - match="Test Result Bundle must not reference more than one Patient " + match="Test Result Bundle must not reference more than one Composition " "resource.", ): handle_request(bundle) @@ -92,14 +96,19 @@ def test_handle_request_raises_error_when_multiple_patient_resources( def test_handle_request_raises_error_when_bundle_includes_identifier( self, ) -> None: - patient = Patient.create( - identifier=Patient.PatientIdentifier.from_nhs_number("nhs_number_1") + """ + Test that handle_request raises ValueError when bundle includes identifier + resources. + """ + # Arrange + composition = Composition.create( + subject=PatientIdentifier.from_nhs_number("nhs_number_1") ) bundle = Bundle.create( identifier=UUIDIdentifier(), type="transaction", - entry=[Bundle.Entry(fullUrl="patient1", resource=patient)], + entry=[Bundle.Entry(fullUrl="composition1", resource=composition)], ) with pytest.raises( diff --git a/pathology-api/test_lambda_handler.py b/pathology-api/test_lambda_handler.py index 02d3479..c049ba3 100644 --- a/pathology-api/test_lambda_handler.py +++ b/pathology-api/test_lambda_handler.py @@ -5,7 +5,8 @@ import pytest from aws_lambda_powertools.utilities.typing import LambdaContext from lambda_handler import handler -from pathology_api.fhir.r4.resources import Bundle, Patient +from pathology_api.fhir.r4.elements import PatientIdentifier +from pathology_api.fhir.r4.resources import Bundle, Composition from pydantic import ValidationError @@ -37,11 +38,9 @@ def test_create_test_result_success(self) -> None: type="transaction", entry=[ Bundle.Entry( - fullUrl="patient", - resource=Patient.create( - identifier=Patient.PatientIdentifier.from_nhs_number( - "nhs_number" - ) + fullUrl="composition", + resource=Composition.create( + subject=PatientIdentifier.from_nhs_number("nhs_number") ), ) ], diff --git a/pathology-api/tests/acceptance/steps/bundle_endpoint_steps.py b/pathology-api/tests/acceptance/steps/bundle_endpoint_steps.py index 002d603..6d1b4c6 100644 --- a/pathology-api/tests/acceptance/steps/bundle_endpoint_steps.py +++ b/pathology-api/tests/acceptance/steps/bundle_endpoint_steps.py @@ -1,7 +1,8 @@ """Step definitions for pathology API bundle endpoint feature.""" import requests -from pathology_api.fhir.r4.resources import Bundle, BundleType, Patient +from pathology_api.fhir.r4.elements import PatientIdentifier +from pathology_api.fhir.r4.resources import Bundle, BundleType, Composition from pytest_bdd import given, parsers, then, when from tests.acceptance.conftest import ResponseContext @@ -36,11 +37,9 @@ def step_send_valid_bundle(client: Client, response_context: ResponseContext) -> type="document", entry=[ Bundle.Entry( - fullUrl="patient", - resource=Patient.create( - identifier=Patient.PatientIdentifier.from_nhs_number( - "nhs_number" - ) + fullUrl="composition", + resource=Composition.create( + subject=PatientIdentifier.from_nhs_number("nhs_number") ), ) ], diff --git a/pathology-api/tests/integration/test_main.py b/pathology-api/tests/integration/test_main.py index 15b4551..4ab1291 100644 --- a/pathology-api/tests/integration/test_main.py +++ b/pathology-api/tests/integration/test_main.py @@ -1,6 +1,7 @@ """Integration tests for the pathology API using pytest.""" -from pathology_api.fhir.r4.resources import Bundle, Patient +from pathology_api.fhir.r4.elements import PatientIdentifier +from pathology_api.fhir.r4.resources import Bundle, Composition from tests.conftest import Client @@ -12,10 +13,8 @@ def test_bundle_returns_200(self, client: Client) -> None: entry=[ Bundle.Entry( fullUrl="patient", - resource=Patient.create( - identifier=Patient.PatientIdentifier.from_nhs_number( - "nhs_number" - ) + resource=Composition.create( + subject=PatientIdentifier.from_nhs_number("nhs_number") ), ) ], @@ -71,10 +70,8 @@ def test_invalid_request_method_returns_error(self, client: Client) -> None: entry=[ Bundle.Entry( fullUrl="patient", - resource=Patient.create( - identifier=Patient.PatientIdentifier.from_nhs_number( - "nhs_number" - ) + resource=Composition.create( + subject=PatientIdentifier.from_nhs_number("nhs_number") ), ) ], From b7f11eccc3e5bbdca5df3d0bf76a93ca4f1da30f Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Fri, 30 Jan 2026 12:43:13 +0000 Subject: [PATCH 05/20] [CDAPI-95]: Added OperationOutcome resource and exception handling for Bundle API --- pathology-api/lambda_handler.py | 75 ++++++++++++---- pathology-api/src/pathology_api/exception.py | 9 ++ .../src/pathology_api/fhir/r4/elements.py | 6 +- .../src/pathology_api/fhir/r4/resources.py | 86 +++++++++++++++++-- pathology-api/src/pathology_api/handler.py | 24 ++++-- .../src/pathology_api/test_handler.py | 12 --- 6 files changed, 169 insertions(+), 43 deletions(-) create mode 100644 pathology-api/src/pathology_api/exception.py diff --git a/pathology-api/lambda_handler.py b/pathology-api/lambda_handler.py index 5bcc42b..b7222c6 100644 --- a/pathology-api/lambda_handler.py +++ b/pathology-api/lambda_handler.py @@ -1,12 +1,16 @@ from collections.abc import Callable +from functools import reduce +from json import JSONDecodeError from typing import Any +import pydantic from aws_lambda_powertools.event_handler import ( APIGatewayHttpResolver, Response, ) from aws_lambda_powertools.utilities.typing import LambdaContext -from pathology_api.fhir.r4.resources import Bundle +from pathology_api.exception import ValidationError +from pathology_api.fhir.r4.resources import Bundle, OperationOutcome from pathology_api.handler import handle_request from pathology_api.logging import get_logger @@ -35,22 +39,53 @@ def wrapper(exception: T) -> Response[str]: return decorator -def _with_default_headers(status_code: int, body: str) -> Response[str]: - content_type = "application/fhir+json" if status_code == 200 else "text/plain" +def _with_default_headers(status_code: int, body: pydantic.BaseModel) -> Response[str]: return Response( status_code=status_code, - headers={"Content-Type": content_type}, - body=body, + headers={"Content-Type": "application/fhir+json"}, + body=body.model_dump_json(by_alias=True, exclude_none=True), ) -@_exception_handler(ValueError) -def handle_value_error(exception: ValueError) -> Response[str]: - """Exception handler for handling ValueError exceptions.""" +@_exception_handler(ValidationError) +def handle_validation_error(exception: ValidationError) -> Response[str]: + """Exception handler for handling ValidationError exceptions.""" # LOG014: False positive, we are within an exception handler here. - _logger.warning("ValueError encountered: %s", exception, exc_info=True) # noqa: LOG014 - return _with_default_headers(status_code=400, body="Invalid payload provided.") + _logger.info( + "ValidationError encountered: %s", + exception, + exc_info=True, # noqa: LOG014 + ) + return _with_default_headers( + status_code=400, + body=OperationOutcome.raise_validation_error(exception.message), + ) + + +@_exception_handler(pydantic.ValidationError) +def handle_pydantic_validation_error( + exception: pydantic.ValidationError, +) -> Response[str]: + """Exception handler for handling pydantic.ValidationError exceptions.""" + # LOG014: False positive, we are within an exception handler here. + _logger.info( + "Pydantic ValidationError encountered: %s", + exception, + exc_info=True, # noqa: LOG014 + ) + + operation_outcome = OperationOutcome.raise_validation_error( + reduce( + lambda acc, e: acc + f"{str(e['loc'])} - {e['msg']} \n", + exception.errors(), + "", + ) + ) + return _with_default_headers( + status_code=400, + body=operation_outcome, + ) @_exception_handler(Exception) @@ -59,7 +94,9 @@ def handle_exception(exception: Exception) -> Response[str]: _logger.exception("Unhandled Exception encountered: %s", exception) return _with_default_headers( status_code=500, - body="An unexpected error has occurred. Please try again later.", + body=OperationOutcome.raise_server_error( + "An unexpected error has occurred. Please try again later." + ), ) @@ -72,12 +109,18 @@ def status() -> Response[str]: @app.post("/FHIR/R4/Bundle") def post_result() -> Response[str]: _logger.debug("Post result endpoint called.") - payload = app.current_event.json_body + + try: + payload = app.current_event.json_body + except JSONDecodeError as e: + raise ValidationError("Invalid payload provided.") from e + _logger.debug("Payload received: %s", payload) - if not payload: - _logger.error("No payload provided.") - return _with_default_headers(status_code=400, body="No payload provided.") + if payload is None: + raise ValidationError( + "Resources must be provided as a bundle of type 'document'" + ) bundle = Bundle.model_validate(payload, by_alias=True) @@ -85,7 +128,7 @@ def post_result() -> Response[str]: return _with_default_headers( status_code=200, - body=response.model_dump_json(by_alias=True, exclude_none=True), + body=response, ) diff --git a/pathology-api/src/pathology_api/exception.py b/pathology-api/src/pathology_api/exception.py new file mode 100644 index 0000000..5fcea45 --- /dev/null +++ b/pathology-api/src/pathology_api/exception.py @@ -0,0 +1,9 @@ +class ValidationError(Exception): + """ + Custom exception for validation errors in FHIR resources. + Note that any message here will be provided in the error response returned to users. + """ + + def __init__(self, message: str): + super().__init__(message) + self.message = message diff --git a/pathology-api/src/pathology_api/fhir/r4/elements.py b/pathology-api/src/pathology_api/fhir/r4/elements.py index cb3352f..efbbff4 100644 --- a/pathology-api/src/pathology_api/fhir/r4/elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/elements.py @@ -6,6 +6,8 @@ from pydantic import Field, model_validator +from pathology_api.exception import ValidationError + @dataclass(frozen=True) class Meta: @@ -44,13 +46,13 @@ class Identifier(ABC): _expected_system: ClassVar[str] = "__unknown__" - value: str system: str + value: str | None = None @model_validator(mode="after") def validate_system(self) -> "Identifier": if self.system != self._expected_system: - raise ValueError( + raise ValidationError( f"Identifier system '{self.system}' does not match expected " f"system '{self._expected_system}'." ) diff --git a/pathology-api/src/pathology_api/fhir/r4/resources.py b/pathology-api/src/pathology_api/fhir/r4/resources.py index 9cf98e5..268b14d 100644 --- a/pathology-api/src/pathology_api/fhir/r4/resources.py +++ b/pathology-api/src/pathology_api/fhir/r4/resources.py @@ -9,6 +9,8 @@ model_validator, ) +from pathology_api.exception import ValidationError + from .elements import LogicalReference, Meta, PatientIdentifier, UUIDIdentifier @@ -43,13 +45,13 @@ def validate_with_subtype( return handler(value) if "resourceType" not in value or value["resourceType"] is None: - raise TypeError("resourceType is required for Resource validation.") + raise ValidationError("resourceType must be provided for each Resource.") resource_type = value["resourceType"] subclass = cls.__resource_types.get(resource_type) if subclass is None: - raise TypeError(f"Unknown resource type: {resource_type}") + raise ValidationError(f"Unsupported resourceType: {resource_type}") # Instantiate the subclass using the dictionary values. return subclass.model_validate(value) @@ -67,14 +69,24 @@ def create(cls, **kwargs: Any) -> Self: def _validate_resource_type(cls, value: str) -> str: expected_resource_type = cls.__expected_resource_type[cls] if value != expected_resource_type: - raise ValueError( - f"Resource type '{value}' does not match expected " - f"resource type '{expected_resource_type}'." + raise ValidationError( + f"Provided resourceType '{value}' does not match required " + f"resourceType '{expected_resource_type}'." ) return value -type BundleType = Literal["document", "transaction"] +type BundleType = Literal[ + "document", + "message", + "transaction", + "transaction-response", + "batch", + "batch-response", + "history", + "searchset", + "collection", +] class Bundle(Resource, resource_type="Bundle"): @@ -144,4 +156,64 @@ class Specimen(Resource, resource_type="Specimen"): class Composition(Resource, resource_type="Composition"): """A FHIR R4 Composition resource.""" - subject: Annotated[LogicalReference[PatientIdentifier], Field(frozen=True)] + subject: Annotated[LogicalReference[PatientIdentifier] | None, Field(frozen=True)] + + +class OperationOutcome(BaseModel): + """ + A FHIR R4 OperationOutcome resource. + + Note this class is deliberately not a subclass of Resource so that it is not + accepted as a valid resource for a client. + """ + + resource_type: Literal["OperationOutcome"] = Field( + "OperationOutcome", alias="resourceType", frozen=True + ) + + class Issue(BaseModel): + severity: Literal["fatal", "error", "warning", "information"] = Field( + frozen=True + ) + code: str = Field(frozen=True) + diagnostics: str | None = Field(None, frozen=True) + + issue: list[Issue] = Field(frozen=True) + + @classmethod + def raise_validation_error(cls, diagnostics: str) -> Self: + """ + Create an OperationOutcome with the provided diagnostic as a validation error. + Args: + diagnostics: The diagnostic message for the validation error. + """ + + return cls( + resourceType="OperationOutcome", + issue=[ + cls.Issue( + severity="error", + code="invalid", + diagnostics=diagnostics, + ) + ], + ) + + @classmethod + def raise_server_error(cls, diagnostics: str | None = None) -> Self: + """ + Create an OperationOutcome with the provided diagnostics as a server error. + Args: + diagnostics: any diagnostics to include with the server error. + """ + + return cls( + resourceType="OperationOutcome", + issue=[ + cls.Issue( + severity="fatal", + code="exception", + diagnostics=diagnostics, + ) + ], + ) diff --git a/pathology-api/src/pathology_api/handler.py b/pathology-api/src/pathology_api/handler.py index 9aad084..d0b3261 100644 --- a/pathology-api/src/pathology_api/handler.py +++ b/pathology-api/src/pathology_api/handler.py @@ -1,5 +1,6 @@ from collections.abc import Callable +from pathology_api.exception import ValidationError from pathology_api.fhir.r4.elements import Meta, UUIDIdentifier from pathology_api.fhir.r4.resources import Bundle, Composition from pathology_api.logging import get_logger @@ -10,22 +11,33 @@ def _ensure_bundle_composition(bundle: Bundle) -> None: compositions = bundle.find_resources(t=Composition) if len(compositions) != 1: - raise ValueError("Bundle must contain exactly one Composition resource.") + raise ValidationError("Document must include a single Composition resource") - composition = compositions[0] - _logger.debug("Composition provided: %s", composition) + subject = compositions[0].subject + if subject is None: + raise ValidationError("Composition does not define a valid subject identifier") + + nhs_number = subject.identifier.value + if nhs_number is None: + raise ValidationError("Composition does not define a valid subject identifier") + + +def _validate_bundle(bundle: Bundle) -> None: + if bundle.identifier is not None: + raise ValidationError("Bundle with identifier is not allowed.") + + if bundle.bundle_type != "document": + raise ValidationError("Resource must be a bundle of type 'document'") type ValidationFunction = Callable[[Bundle], None] _validation_functions: list[ValidationFunction] = [ _ensure_bundle_composition, + _validate_bundle, ] def handle_request(bundle: Bundle) -> Bundle: - if bundle.identifier: - raise ValueError("Bundle with identifier is not allowed.") - for validate_function in _validation_functions: validate_function(bundle) diff --git a/pathology-api/src/pathology_api/test_handler.py b/pathology-api/src/pathology_api/test_handler.py index 03f7d1e..b188579 100644 --- a/pathology-api/src/pathology_api/test_handler.py +++ b/pathology-api/src/pathology_api/test_handler.py @@ -22,12 +22,10 @@ def test_handle_request(self) -> None: ], ) - # Act before_call = datetime.datetime.now(tz=datetime.timezone.utc) result_bundle = handle_request(bundle) after_call = datetime.datetime.now(tz=datetime.timezone.utc) - # Assert assert result_bundle is not None assert result_bundle.identifier is not None @@ -63,11 +61,6 @@ def test_handle_request_raises_error_when_no_patient_resource(self) -> None: def test_handle_request_raises_error_when_multiple_composition_resources( self, ) -> None: - """ - Test that handle_request raises ValueError when bundle has multiple Composition - resources. - """ - # Arrange composition = Composition.create( subject=PatientIdentifier.from_nhs_number("nhs_number_1") ) @@ -96,11 +89,6 @@ def test_handle_request_raises_error_when_multiple_composition_resources( def test_handle_request_raises_error_when_bundle_includes_identifier( self, ) -> None: - """ - Test that handle_request raises ValueError when bundle includes identifier - resources. - """ - # Arrange composition = Composition.create( subject=PatientIdentifier.from_nhs_number("nhs_number_1") ) From 8b373a7fb9e3a0b98a85fe3d37bf50074f1e4839 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Mon, 2 Feb 2026 11:44:12 +0000 Subject: [PATCH 06/20] [CDAPI-95]: Renamed _ensure_bundle_composition to _validate_composition --- pathology-api/src/pathology_api/handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pathology-api/src/pathology_api/handler.py b/pathology-api/src/pathology_api/handler.py index d0b3261..fef18ad 100644 --- a/pathology-api/src/pathology_api/handler.py +++ b/pathology-api/src/pathology_api/handler.py @@ -8,7 +8,7 @@ _logger = get_logger(__name__) -def _ensure_bundle_composition(bundle: Bundle) -> None: +def _validate_composition(bundle: Bundle) -> None: compositions = bundle.find_resources(t=Composition) if len(compositions) != 1: raise ValidationError("Document must include a single Composition resource") @@ -32,7 +32,7 @@ def _validate_bundle(bundle: Bundle) -> None: type ValidationFunction = Callable[[Bundle], None] _validation_functions: list[ValidationFunction] = [ - _ensure_bundle_composition, + _validate_composition, _validate_bundle, ] From 143361e8e446c53ba1281713f93526d2d288c7c6 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Mon, 2 Feb 2026 12:08:11 +0000 Subject: [PATCH 07/20] [CDAPI-95]: Updated resource unit tests --- .../pathology_api/fhir/r4/test_resources.py | 85 +++++++++++++++---- 1 file changed, 69 insertions(+), 16 deletions(-) diff --git a/pathology-api/src/pathology_api/fhir/r4/test_resources.py b/pathology-api/src/pathology_api/fhir/r4/test_resources.py index 156d6ea..2187248 100644 --- a/pathology-api/src/pathology_api/fhir/r4/test_resources.py +++ b/pathology-api/src/pathology_api/fhir/r4/test_resources.py @@ -1,12 +1,14 @@ import json from typing import Any +import pydantic import pytest from pydantic import BaseModel -from pathology_api.fhir.r4.elements import PatientIdentifier +from pathology_api.exception import ValidationError -from .resources import Bundle, Composition, Patient, Resource +from .elements import LogicalReference, PatientIdentifier +from .resources import Bundle, Composition, OperationOutcome, Patient, Resource class TestResource: @@ -49,8 +51,8 @@ def test_resource_deserialisation_unknown_resource(self) -> None: ) with pytest.raises( - TypeError, - match=f"Unknown resource type: {expected_resource_type}", + ValidationError, + match=f"Unsupported resourceType: {expected_resource_type}", ): self._TestContainer.model_validate_json(example_json) @@ -70,38 +72,44 @@ def test_resource_deserialisation_without_resource_type( example_json = json.dumps(value) with pytest.raises( - TypeError, - match="resourceType is required for Resource validation.", + ValidationError, + match="resourceType must be provided for each Resource.", ): self._TestContainer.model_validate_json(example_json) @pytest.mark.parametrize( - ("json", "expected_error_message"), + ("json", "expected_error_message", "expected_error_type"), [ pytest.param( json.dumps({"resourceType": "invalid", "type": "document"}), - "Value error, Resource type 'invalid' does not match expected " - "resource type 'Bundle'.", + "Provided resourceType 'invalid' does not match required " + "resourceType 'Bundle'.", + ValidationError, id="Invalid resource type", ), pytest.param( json.dumps({"resourceType": None, "type": "document"}), "1 validation error for Bundle\nresourceType\n " "Input should be a valid string", + pydantic.ValidationError, id="Input should be a valid string", ), pytest.param( json.dumps({"type": "document"}), "1 validation error for Bundle\nresourceType\n Field required", + pydantic.ValidationError, id="Missing resource type", ), ], ) def test_deserialise_wrong_resource_type( - self, json: str, expected_error_message: str + self, + json: str, + expected_error_message: str, + expected_error_type: type[Exception], ) -> None: with pytest.raises( - ValueError, + expected_error_type, match=expected_error_message, ): Bundle.model_validate_json(json, strict=True) @@ -109,11 +117,12 @@ def test_deserialise_wrong_resource_type( class TestBundle: def test_create(self) -> None: - """Test creating a Bundle resource.""" expected_entry = Bundle.Entry( fullUrl="full", resource=Composition.create( - subject=PatientIdentifier.from_nhs_number("nhs_number") + subject=LogicalReference( + PatientIdentifier.from_nhs_number("nhs_number") + ) ), ) @@ -127,7 +136,6 @@ def test_create(self) -> None: assert bundle.entries == [expected_entry] def test_create_without_entries(self) -> None: - """Test creating a Bundle resource without entries.""" bundle = Bundle.empty("document") assert bundle.bundle_type == "document" @@ -135,7 +143,9 @@ def test_create_without_entries(self) -> None: assert bundle.entries is None expected_resource = Composition.create( - subject=PatientIdentifier.from_nhs_number("nhs_number") + subject=LogicalReference( + identifier=PatientIdentifier.from_nhs_number("nhs_number") + ) ) @pytest.mark.parametrize( @@ -172,7 +182,7 @@ def test_find_resources( ) -> None: bundle = Bundle.create(type="document", entry=entries) - result = bundle.find_resources(Patient) + result = bundle.find_resources(Composition) assert result == expected_results @pytest.mark.parametrize( @@ -203,3 +213,46 @@ def test_find_resources_returns_empty_list(self, bundle: Bundle) -> None: """ result = bundle.find_resources(Patient) assert result == [] + + +class TestOperationOutcome: + def test_raise_validation_error(self) -> None: + expected_diagnostics = "Invalid patient identifier format" + + outcome = OperationOutcome.raise_validation_error(expected_diagnostics) + + assert outcome.resource_type == "OperationOutcome" + assert len(outcome.issue) == 1 + + issue = outcome.issue[0] + assert issue.severity == "error" + assert issue.code == "invalid" + assert issue.diagnostics == expected_diagnostics + + @pytest.mark.parametrize( + ("diagnostics", "expected_diagnostics"), + [ + pytest.param( + "Database connection failed", + "Database connection failed", + id="with_diagnostics", + ), + pytest.param( + None, + None, + id="without_diagnostics", + ), + ], + ) + def test_raise_server_error( + self, diagnostics: str | None, expected_diagnostics: str | None + ) -> None: + outcome = OperationOutcome.raise_server_error(diagnostics) + + assert outcome.resource_type == "OperationOutcome" + assert len(outcome.issue) == 1 + + issue = outcome.issue[0] + assert issue.severity == "fatal" + assert issue.code == "exception" + assert issue.diagnostics == expected_diagnostics From 7158c045905b77347f0600f84e8510247b4bb523 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Mon, 2 Feb 2026 13:25:41 +0000 Subject: [PATCH 08/20] [CDAPI-95]: Updated elements unit tests --- .../pathology_api/fhir/r4/test_elements.py | 62 ++++++++++++++++++- 1 file changed, 60 insertions(+), 2 deletions(-) diff --git a/pathology-api/src/pathology_api/fhir/r4/test_elements.py b/pathology-api/src/pathology_api/fhir/r4/test_elements.py index 858f9b0..7257daa 100644 --- a/pathology-api/src/pathology_api/fhir/r4/test_elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/test_elements.py @@ -4,7 +4,15 @@ import pytest from pydantic import BaseModel -from .elements import Identifier, Meta, PatientIdentifier, UUIDIdentifier +from pathology_api.exception import ValidationError + +from .elements import ( + Identifier, + LogicalReference, + Meta, + PatientIdentifier, + UUIDIdentifier, +) class TestMeta: @@ -79,7 +87,7 @@ class _TestContainer(BaseModel): identifier: _TestIdentifier with pytest.raises( - ValueError, + ValidationError, match="Identifier system 'invalid-system' does not match expected " "system 'expected-system'.", ): @@ -96,3 +104,53 @@ def test_create_from_nhs_number(self) -> None: assert identifier.system == "https://fhir.nhs.uk/Id/nhs-number" assert identifier.value == nhs_number + + +class TestLogicalReference: + class _TestContainer(BaseModel): + reference: LogicalReference[PatientIdentifier] + + def test_create_with_patient_identifier(self) -> None: + nhs_number = "nhs_number" + patient_id = PatientIdentifier.from_nhs_number(nhs_number) + + reference = LogicalReference(identifier=patient_id) + + assert reference.identifier == patient_id + assert reference.identifier.system == "https://fhir.nhs.uk/Id/nhs-number" + assert reference.identifier.value == nhs_number + + def test_serialization(self) -> None: + nhs_number = "nhs_number" + patient_id = PatientIdentifier.from_nhs_number(nhs_number) + reference = LogicalReference(identifier=patient_id) + + container = self._TestContainer(reference=reference) + serialized = container.model_dump(by_alias=True) + + expected = { + "reference": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + } + } + assert serialized == expected + + def test_deserialization(self) -> None: + data = { + "reference": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + } + } + + container = self._TestContainer.model_validate(data) + + created_identifier = container.reference.identifier + assert isinstance(created_identifier, PatientIdentifier) + assert created_identifier.system == "https://fhir.nhs.uk/Id/nhs-number" + assert created_identifier.value == "nhs_number" From 7e2eface817163a373b63eeabcb6e3e5b2e2b4d8 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Tue, 3 Feb 2026 15:15:18 +0000 Subject: [PATCH 09/20] [CDAPI-95]: Updated final unit tests to account for handler and exception handling changes --- pathology-api/lambda_handler.py | 17 ++- .../src/pathology_api/fhir/r4/elements.py | 2 +- .../src/pathology_api/fhir/r4/resources.py | 53 ++++--- .../pathology_api/fhir/r4/test_resources.py | 12 +- .../src/pathology_api/test_handler.py | 89 +++++++++-- pathology-api/test_lambda_handler.py | 142 +++++++++++++----- 6 files changed, 237 insertions(+), 78 deletions(-) diff --git a/pathology-api/lambda_handler.py b/pathology-api/lambda_handler.py index b7222c6..5d94489 100644 --- a/pathology-api/lambda_handler.py +++ b/pathology-api/lambda_handler.py @@ -8,6 +8,7 @@ APIGatewayHttpResolver, Response, ) +from aws_lambda_powertools.event_handler.exceptions import NotFoundError from aws_lambda_powertools.utilities.typing import LambdaContext from pathology_api.exception import ValidationError from pathology_api.fhir.r4.resources import Bundle, OperationOutcome @@ -49,8 +50,6 @@ def _with_default_headers(status_code: int, body: pydantic.BaseModel) -> Respons @_exception_handler(ValidationError) def handle_validation_error(exception: ValidationError) -> Response[str]: - """Exception handler for handling ValidationError exceptions.""" - # LOG014: False positive, we are within an exception handler here. _logger.info( "ValidationError encountered: %s", @@ -67,7 +66,6 @@ def handle_validation_error(exception: ValidationError) -> Response[str]: def handle_pydantic_validation_error( exception: pydantic.ValidationError, ) -> Response[str]: - """Exception handler for handling pydantic.ValidationError exceptions.""" # LOG014: False positive, we are within an exception handler here. _logger.info( "Pydantic ValidationError encountered: %s", @@ -88,9 +86,20 @@ def handle_pydantic_validation_error( ) +@app.not_found +def handle_not_found_error(exception: NotFoundError) -> Response[str]: + _logger.info("NotFoundError encountered: %s", exception, exec_info=True) + return _with_default_headers( + status_code=404, + body=OperationOutcome.raise_not_found_error( + "No resource found for requested path. " + f"Path: ({app.current_event.http_method}) {app.current_event.path}" + ), + ) + + @_exception_handler(Exception) def handle_exception(exception: Exception) -> Response[str]: - """General default exception handler for handling any unhandled exceptions.""" _logger.exception("Unhandled Exception encountered: %s", exception) return _with_default_headers( status_code=500, diff --git a/pathology-api/src/pathology_api/fhir/r4/elements.py b/pathology-api/src/pathology_api/fhir/r4/elements.py index efbbff4..f7597f5 100644 --- a/pathology-api/src/pathology_api/fhir/r4/elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/elements.py @@ -78,7 +78,7 @@ class PatientIdentifier( ): """A FHIR R4 Patient Identifier utilising the NHS Number system.""" - def __init__(self, value: str): + def __init__(self, value: str | None = None): super().__init__(value=value, system=self._expected_system) @classmethod diff --git a/pathology-api/src/pathology_api/fhir/r4/resources.py b/pathology-api/src/pathology_api/fhir/r4/resources.py index 268b14d..60c33b4 100644 --- a/pathology-api/src/pathology_api/fhir/r4/resources.py +++ b/pathology-api/src/pathology_api/fhir/r4/resources.py @@ -1,4 +1,5 @@ -from typing import Annotated, Any, ClassVar, Literal, Self +from dataclasses import dataclass +from typing import Annotated, Any, ClassVar, Literal, Self, TypedDict from pydantic import ( BaseModel, @@ -171,12 +172,11 @@ class OperationOutcome(BaseModel): "OperationOutcome", alias="resourceType", frozen=True ) - class Issue(BaseModel): - severity: Literal["fatal", "error", "warning", "information"] = Field( - frozen=True - ) - code: str = Field(frozen=True) - diagnostics: str | None = Field(None, frozen=True) + @dataclass(frozen=True) + class Issue(TypedDict): + severity: Literal["fatal", "error", "warning", "information"] + code: str + diagnostics: str | None issue: list[Issue] = Field(frozen=True) @@ -191,11 +191,11 @@ def raise_validation_error(cls, diagnostics: str) -> Self: return cls( resourceType="OperationOutcome", issue=[ - cls.Issue( - severity="error", - code="invalid", - diagnostics=diagnostics, - ) + { + "severity": "error", + "code": "invalid", + "diagnostics": diagnostics, + } ], ) @@ -210,10 +210,29 @@ def raise_server_error(cls, diagnostics: str | None = None) -> Self: return cls( resourceType="OperationOutcome", issue=[ - cls.Issue( - severity="fatal", - code="exception", - diagnostics=diagnostics, - ) + { + "severity": "fatal", + "code": "exception", + "diagnostics": diagnostics, + } + ], + ) + + @classmethod + def raise_not_found_error(cls, diagnostics: str | None = None) -> Self: + """ + Create an OperationOutcome with the provided diagnostics as a not found error. + Args: + diagnostics: any diagnostics to include with the not found error. + """ + + return cls( + resourceType="OperationOutcome", + issue=[ + { + "severity": "error", + "code": "not-found", + "diagnostics": diagnostics, + } ], ) diff --git a/pathology-api/src/pathology_api/fhir/r4/test_resources.py b/pathology-api/src/pathology_api/fhir/r4/test_resources.py index 2187248..627b552 100644 --- a/pathology-api/src/pathology_api/fhir/r4/test_resources.py +++ b/pathology-api/src/pathology_api/fhir/r4/test_resources.py @@ -225,9 +225,9 @@ def test_raise_validation_error(self) -> None: assert len(outcome.issue) == 1 issue = outcome.issue[0] - assert issue.severity == "error" - assert issue.code == "invalid" - assert issue.diagnostics == expected_diagnostics + assert issue["severity"] == "error" + assert issue["code"] == "invalid" + assert issue["diagnostics"] == expected_diagnostics @pytest.mark.parametrize( ("diagnostics", "expected_diagnostics"), @@ -253,6 +253,6 @@ def test_raise_server_error( assert len(outcome.issue) == 1 issue = outcome.issue[0] - assert issue.severity == "fatal" - assert issue.code == "exception" - assert issue.diagnostics == expected_diagnostics + assert issue["severity"] == "fatal" + assert issue["code"] == "exception" + assert issue["diagnostics"] == expected_diagnostics diff --git a/pathology-api/src/pathology_api/test_handler.py b/pathology-api/src/pathology_api/test_handler.py index b188579..e834072 100644 --- a/pathology-api/src/pathology_api/test_handler.py +++ b/pathology-api/src/pathology_api/test_handler.py @@ -2,7 +2,12 @@ import pytest -from pathology_api.fhir.r4.elements import PatientIdentifier, UUIDIdentifier +from pathology_api.exception import ValidationError +from pathology_api.fhir.r4.elements import ( + LogicalReference, + PatientIdentifier, + UUIDIdentifier, +) from pathology_api.fhir.r4.resources import Bundle, Composition from pathology_api.handler import handle_request @@ -11,12 +16,14 @@ class TestHandleRequest: def test_handle_request(self) -> None: # Arrange bundle = Bundle.create( - type="transaction", + type="document", entry=[ Bundle.Entry( fullUrl="patient", resource=Composition.create( - subject=PatientIdentifier.from_nhs_number("nhs_number") + subject=LogicalReference( + PatientIdentifier.from_nhs_number("nhs_number") + ) ), ) ], @@ -45,16 +52,15 @@ def test_handle_request(self) -> None: assert created_meta.version_id is None - def test_handle_request_raises_error_when_no_patient_resource(self) -> None: + def test_handle_request_raises_error_when_no_composition_resource(self) -> None: bundle = Bundle.create( - type="transaction", + type="document", entry=[], ) with pytest.raises( - ValueError, - match="Test Result Bundle must reference at least one Composition " - "resource.", + ValidationError, + match="Document must include a single Composition resource", ): handle_request(bundle) @@ -62,11 +68,11 @@ def test_handle_request_raises_error_when_multiple_composition_resources( self, ) -> None: composition = Composition.create( - subject=PatientIdentifier.from_nhs_number("nhs_number_1") + subject=LogicalReference(PatientIdentifier.from_nhs_number("nhs_number_1")) ) bundle = Bundle.create( - type="transaction", + type="document", entry=[ Bundle.Entry( fullUrl="composition1", @@ -80,9 +86,42 @@ def test_handle_request_raises_error_when_multiple_composition_resources( ) with pytest.raises( - ValueError, - match="Test Result Bundle must not reference more than one Composition " - "resource.", + ValidationError, + match="Document must include a single Composition resource", + ): + handle_request(bundle) + + @pytest.mark.parametrize( + ("composition", "expected_error_message"), + [ + pytest.param( + Composition.create(subject=None), + "Composition does not define a valid subject identifier", + id="No subject", + ), + pytest.param( + Composition.create(subject=LogicalReference(PatientIdentifier())), + "Composition does not define a valid subject identifier", + id="Subject with no identifier value", + ), + ], + ) + def test_handle_request_raises_error_when_invalid_composition( + self, composition: Composition, expected_error_message: str + ) -> None: + bundle = Bundle.create( + type="document", + entry=[ + Bundle.Entry( + fullUrl="composition", + resource=composition, + ) + ], + ) + + with pytest.raises( + ValidationError, + match=expected_error_message, ): handle_request(bundle) @@ -90,17 +129,35 @@ def test_handle_request_raises_error_when_bundle_includes_identifier( self, ) -> None: composition = Composition.create( - subject=PatientIdentifier.from_nhs_number("nhs_number_1") + subject=LogicalReference(PatientIdentifier.from_nhs_number("nhs_number_1")) ) bundle = Bundle.create( identifier=UUIDIdentifier(), - type="transaction", + type="document", entry=[Bundle.Entry(fullUrl="composition1", resource=composition)], ) with pytest.raises( - ValueError, + ValidationError, match="Bundle with identifier is not allowed.", ): handle_request(bundle) + + def test_handle_request_raises_error_when_bundle_not_document_type( + self, + ) -> None: + composition = Composition.create( + subject=LogicalReference(PatientIdentifier.from_nhs_number("nhs_number_1")) + ) + + bundle = Bundle.create( + type="collection", + entry=[Bundle.Entry(fullUrl="composition1", resource=composition)], + ) + + with pytest.raises( + ValidationError, + match="Resource must be a bundle of type 'document'", + ): + handle_request(bundle) diff --git a/pathology-api/test_lambda_handler.py b/pathology-api/test_lambda_handler.py index c049ba3..2b97482 100644 --- a/pathology-api/test_lambda_handler.py +++ b/pathology-api/test_lambda_handler.py @@ -1,13 +1,13 @@ -import json from typing import Any from unittest.mock import patch +import pydantic import pytest from aws_lambda_powertools.utilities.typing import LambdaContext from lambda_handler import handler -from pathology_api.fhir.r4.elements import PatientIdentifier -from pathology_api.fhir.r4.resources import Bundle, Composition -from pydantic import ValidationError +from pathology_api.exception import ValidationError +from pathology_api.fhir.r4.elements import LogicalReference, PatientIdentifier +from pathology_api.fhir.r4.resources import Bundle, Composition, OperationOutcome class TestHandler: @@ -33,14 +33,23 @@ def _create_test_event( "pathParameters": {"proxy": path_params}, } + def _parse_returned_issue(self, response: str) -> OperationOutcome.Issue: + response_outcome = OperationOutcome.model_validate_json(response) + + assert len(response_outcome.issue) == 1 + returned_issue = response_outcome.issue[0] + return returned_issue + def test_create_test_result_success(self) -> None: bundle = Bundle.create( - type="transaction", + type="document", entry=[ Bundle.Entry( fullUrl="composition", resource=Composition.create( - subject=PatientIdentifier.from_nhs_number("nhs_number") + subject=LogicalReference( + PatientIdentifier.from_nhs_number("nhs_number") + ) ), ) ], @@ -80,8 +89,16 @@ def test_create_test_result_no_payload(self) -> None: response = handler(event, context) assert response["statusCode"] == 400 - assert response["body"] == "No payload provided." - assert response["headers"] == {"Content-Type": "text/plain"} + assert response["headers"] == {"Content-Type": "application/fhir+json"} + + returned_issue = self._parse_returned_issue(response["body"]) + + assert returned_issue["severity"] == "error" + assert returned_issue["code"] == "invalid" + assert ( + returned_issue["diagnostics"] + == "Resources must be provided as a bundle of type 'document'" + ) def test_create_test_result_empty_payload(self) -> None: event = self._create_test_event( @@ -92,8 +109,16 @@ def test_create_test_result_empty_payload(self) -> None: response = handler(event, context) assert response["statusCode"] == 400 - assert response["body"] == "No payload provided." - assert response["headers"] == {"Content-Type": "text/plain"} + assert response["headers"] == {"Content-Type": "application/fhir+json"} + + returned_issue = self._parse_returned_issue(response["body"]) + + assert returned_issue["severity"] == "error" + assert returned_issue["code"] == "invalid" + assert ( + returned_issue["diagnostics"] + == "('resourceType',) - Field required \n('type',) - Field required \n" + ) def test_create_test_result_invalid_json(self) -> None: event = self._create_test_event( @@ -104,44 +129,84 @@ def test_create_test_result_invalid_json(self) -> None: response = handler(event, context) assert response["statusCode"] == 400 - assert response["body"] == "Invalid payload provided." - assert response["headers"] == {"Content-Type": "text/plain"} + assert response["headers"] == {"Content-Type": "application/fhir+json"} - def test_create_test_result_processing_error(self) -> None: - bundle = Bundle.empty(bundle_type="transaction") + returned_issue = self._parse_returned_issue(response["body"]) + assert returned_issue["severity"] == "error" + assert returned_issue["code"] == "invalid" + assert returned_issue["diagnostics"] == "Invalid payload provided." + + @pytest.mark.parametrize( + ("error", "expected_issue", "expected_status_code"), + [ + pytest.param( + ValidationError("Test processing error"), + OperationOutcome.Issue( + severity="error", + code="invalid", + diagnostics="Test processing error", + ), + 400, + id="ValidationError", + ), + pytest.param( + Exception("Test general error"), + { + "severity": "fatal", + "code": "exception", + "diagnostics": "An unexpected error has occurred. " + "Please try again later.", + }, + 500, + id="Unexpected exception", + ), + ], + ) + def test_create_test_result_processing_error( + self, + error: type[Exception], + expected_issue: OperationOutcome.Issue, + expected_status_code: int, + ) -> None: + bundle = Bundle.empty(bundle_type="document") event = self._create_test_event( body=bundle.model_dump_json(by_alias=True), path_params="FHIR/R4/Bundle", request_method="POST", ) context = LambdaContext() - error_message = "Test processing error" - expected_error = ValueError(error_message) - with patch("lambda_handler.handle_request", side_effect=expected_error): + with patch("lambda_handler.handle_request", side_effect=error): response = handler(event, context) - assert response["statusCode"] == 400 - assert response["body"] == "Error processing provided bundle." - assert response["headers"] == {"Content-Type": "text/plain"} + assert response["statusCode"] == expected_status_code + assert response["headers"] == {"Content-Type": "application/fhir+json"} + + returned_issue = self._parse_returned_issue(response["body"]) + assert returned_issue == expected_issue @pytest.mark.parametrize( - "expected_error", + ("expected_error", "expected_diagnostic"), [ pytest.param( - TypeError("Test type error"), - id="TypeError", + ValidationError("Test validation error"), + "Test validation error", + id="ValidationError", ), pytest.param( - ValidationError("Test validation error", []), - id="ValidationError", + pydantic.ValidationError.from_exception_data( + "Test validation error", + [{"type": "missing", "loc": ("field",), "input": "is invalid"}], + ), + "('field',) - Field required \n", + id="Pydantic ValidationError", ), ], ) def test_create_test_result_parse_json_error( - self, expected_error: Exception + self, expected_error: Exception, expected_diagnostic: str ) -> None: - bundle = Bundle.empty(bundle_type="transaction") + bundle = Bundle.empty(bundle_type="document") event = self._create_test_event( body=bundle.model_dump_json(by_alias=True), path_params="FHIR/R4/Bundle", @@ -156,8 +221,12 @@ def test_create_test_result_parse_json_error( response = handler(event, context) assert response["statusCode"] == 400 - assert response["body"] == "Invalid payload provided." - assert response["headers"] == {"Content-Type": "text/plain"} + assert response["headers"] == {"Content-Type": "application/fhir+json"} + + returned_issue = self._parse_returned_issue(response["body"]) + assert returned_issue["severity"] == "error" + assert returned_issue["code"] == "invalid" + assert returned_issue["diagnostics"] == expected_diagnostic def test_status_success(self) -> None: event = self._create_test_event(path_params="_status", request_method="GET") @@ -186,8 +255,13 @@ def test_invalid_request(self, request_method: str, request_parameter: str) -> N response = handler(event, context) assert response["statusCode"] == 404 - assert json.loads(response["body"]) == { - "statusCode": 404, - "message": "Not found", - } - assert response["headers"] == {"Content-Type": "application/json"} + assert response["headers"] == {"Content-Type": "application/fhir+json"} + + returned_issue = self._parse_returned_issue(response["body"]) + + assert returned_issue["severity"] == "error" + assert returned_issue["code"] == "not-found" + assert ( + returned_issue["diagnostics"] == "No resource found for requested path. " + f"Path: ({request_method}) /{request_parameter}" + ) From 9817f23bfc17a51da7ecb0c4416cc9a861d5bcc9 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Wed, 4 Feb 2026 10:05:38 +0000 Subject: [PATCH 10/20] [CDAPI-95]: Allowed for additional unparsed fields to be included within the FHIR payload as part of the Test Result endpoint --- pathology-api/src/pathology_api/fhir/r4/elements.py | 1 + pathology-api/src/pathology_api/fhir/r4/resources.py | 4 ++++ pathology-api/src/pathology_api/handler.py | 10 ++++++---- pathology-api/src/pathology_api/test_handler.py | 11 ++++------- pathology-api/test_lambda_handler.py | 6 +----- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/pathology-api/src/pathology_api/fhir/r4/elements.py b/pathology-api/src/pathology_api/fhir/r4/elements.py index f7597f5..a9032ea 100644 --- a/pathology-api/src/pathology_api/fhir/r4/elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/elements.py @@ -20,6 +20,7 @@ class Meta: last_updated: Annotated[datetime.datetime | None, Field(alias="lastUpdated")] = None version_id: Annotated[str | None, Field(alias="versionId")] = None + profile: list[str] | None = None @classmethod def with_last_updated(cls, last_updated: datetime.datetime | None = None) -> "Meta": diff --git a/pathology-api/src/pathology_api/fhir/r4/resources.py b/pathology-api/src/pathology_api/fhir/r4/resources.py index 60c33b4..5085be0 100644 --- a/pathology-api/src/pathology_api/fhir/r4/resources.py +++ b/pathology-api/src/pathology_api/fhir/r4/resources.py @@ -3,6 +3,7 @@ from pydantic import ( BaseModel, + ConfigDict, Field, SerializeAsAny, ValidatorFunctionWrapHandler, @@ -18,10 +19,13 @@ class Resource(BaseModel): """A FHIR R4 Resource base class.""" + model_config = ConfigDict(extra="allow") + # class variable to hold class mappings per resource_type __resource_types: ClassVar[dict[str, type["Resource"]]] = {} __expected_resource_type: ClassVar[dict[type["Resource"], str]] = {} + id: Annotated[str | None, Field(frozen=True)] = None meta: Annotated[Meta | None, Field(alias="meta", frozen=True)] = None resource_type: str = Field(alias="resourceType", frozen=True) diff --git a/pathology-api/src/pathology_api/handler.py b/pathology-api/src/pathology_api/handler.py index fef18ad..5cb7e11 100644 --- a/pathology-api/src/pathology_api/handler.py +++ b/pathology-api/src/pathology_api/handler.py @@ -1,7 +1,8 @@ +import uuid from collections.abc import Callable from pathology_api.exception import ValidationError -from pathology_api.fhir.r4.elements import Meta, UUIDIdentifier +from pathology_api.fhir.r4.elements import Meta from pathology_api.fhir.r4.resources import Bundle, Composition from pathology_api.logging import get_logger @@ -23,8 +24,8 @@ def _validate_composition(bundle: Bundle) -> None: def _validate_bundle(bundle: Bundle) -> None: - if bundle.identifier is not None: - raise ValidationError("Bundle with identifier is not allowed.") + if bundle.id is not None: + raise ValidationError("Bundles cannot be defined with an existing ID") if bundle.bundle_type != "document": raise ValidationError("Resource must be a bundle of type 'document'") @@ -43,8 +44,9 @@ def handle_request(bundle: Bundle) -> Bundle: _logger.debug("Bundle entries: %s", bundle.entries) return_bundle = Bundle.create( + id=str(uuid.uuid4()), meta=Meta.with_last_updated(), - identifier=UUIDIdentifier(), + identifier=bundle.identifier, type=bundle.bundle_type, entry=bundle.entries, ) diff --git a/pathology-api/src/pathology_api/test_handler.py b/pathology-api/src/pathology_api/test_handler.py index e834072..7a9d5b0 100644 --- a/pathology-api/src/pathology_api/test_handler.py +++ b/pathology-api/src/pathology_api/test_handler.py @@ -6,7 +6,6 @@ from pathology_api.fhir.r4.elements import ( LogicalReference, PatientIdentifier, - UUIDIdentifier, ) from pathology_api.fhir.r4.resources import Bundle, Composition from pathology_api.handler import handle_request @@ -35,9 +34,7 @@ def test_handle_request(self) -> None: assert result_bundle is not None - assert result_bundle.identifier is not None - result_identifier = result_bundle.identifier - assert result_identifier.system == "https://tools.ietf.org/html/rfc4122" + assert result_bundle.id is not None assert result_bundle.bundle_type == bundle.bundle_type assert result_bundle.entries == bundle.entries @@ -125,7 +122,7 @@ def test_handle_request_raises_error_when_invalid_composition( ): handle_request(bundle) - def test_handle_request_raises_error_when_bundle_includes_identifier( + def test_handle_request_raises_error_when_bundle_includes_id( self, ) -> None: composition = Composition.create( @@ -133,14 +130,14 @@ def test_handle_request_raises_error_when_bundle_includes_identifier( ) bundle = Bundle.create( - identifier=UUIDIdentifier(), + id="id", type="document", entry=[Bundle.Entry(fullUrl="composition1", resource=composition)], ) with pytest.raises( ValidationError, - match="Bundle with identifier is not allowed.", + match="Bundles cannot be defined with an existing ID", ): handle_request(bundle) diff --git a/pathology-api/test_lambda_handler.py b/pathology-api/test_lambda_handler.py index 2b97482..caf2ce8 100644 --- a/pathology-api/test_lambda_handler.py +++ b/pathology-api/test_lambda_handler.py @@ -73,12 +73,8 @@ def test_create_test_result_success(self) -> None: assert response_bundle.bundle_type == bundle.bundle_type assert response_bundle.entries == bundle.entries - assert response_bundle.identifier is not None - assert ( - response_bundle.identifier.system == "https://tools.ietf.org/html/rfc4122" - ) # A UUID value so can only check its presence. - assert response_bundle.identifier.value is not None + assert response_bundle.id is not None def test_create_test_result_no_payload(self) -> None: event = self._create_test_event( From 967f311772982a0d430a3a9f54b313155aa7371d Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Wed, 4 Feb 2026 15:31:02 +0000 Subject: [PATCH 11/20] [CDAPI-95]: Updated Integration Tests to account for OperationOutcome changes --- pathology-api/tests/integration/test_main.py | 76 ++++++++++++++++---- 1 file changed, 61 insertions(+), 15 deletions(-) diff --git a/pathology-api/tests/integration/test_main.py b/pathology-api/tests/integration/test_main.py index 4ab1291..26b3fd2 100644 --- a/pathology-api/tests/integration/test_main.py +++ b/pathology-api/tests/integration/test_main.py @@ -1,6 +1,6 @@ """Integration tests for the pathology API using pytest.""" -from pathology_api.fhir.r4.elements import PatientIdentifier +from pathology_api.fhir.r4.elements import LogicalReference, PatientIdentifier from pathology_api.fhir.r4.resources import Bundle, Composition from tests.conftest import Client @@ -14,7 +14,9 @@ def test_bundle_returns_200(self, client: Client) -> None: Bundle.Entry( fullUrl="patient", resource=Composition.create( - subject=PatientIdentifier.from_nhs_number("nhs_number") + subject=LogicalReference( + PatientIdentifier.from_nhs_number("nhs_number") + ) ), ) ], @@ -35,11 +37,8 @@ def test_bundle_returns_200(self, client: Client) -> None: assert response_bundle.bundle_type == bundle.bundle_type assert response_bundle.entries == bundle.entries - assert response_bundle.identifier is not None - response_identifier = response_bundle.identifier - assert response_identifier.system == "https://tools.ietf.org/html/rfc4122" # A UUID value so can only check its presence. - assert response_identifier.value is not None + assert response_bundle.id is not None assert response_bundle.meta is not None response_meta = response_bundle.meta @@ -52,17 +51,41 @@ def test_no_payload_returns_error(self, client: Client) -> None: ) assert response.status_code == 400 - response_data = response.text - assert response_data == "No payload provided." + response_data = response.json() + assert response_data == { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "invalid", + "diagnostics": "Resources must be provided as a bundle of type" + " 'document'", + } + ], + } + assert response.headers["Content-Type"] == "application/fhir+json" assert response.status_code == 400 def test_empty_name_returns_error(self, client: Client) -> None: response = client.send(data="", request_method="POST", path="FHIR/R4/Bundle") assert response.status_code == 400 - response_data = response.text - assert response_data == "No payload provided." + response_data = response.json() + assert response_data == { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "invalid", + "diagnostics": "Resources must be provided as a bundle of type" + " 'document'", + } + ], + } + + assert response.headers["Content-Type"] == "application/fhir+json" + assert response.status_code == 400 def test_invalid_request_method_returns_error(self, client: Client) -> None: bundle = Bundle.create( @@ -71,7 +94,9 @@ def test_invalid_request_method_returns_error(self, client: Client) -> None: Bundle.Entry( fullUrl="patient", resource=Composition.create( - subject=PatientIdentifier.from_nhs_number("nhs_number") + subject=LogicalReference( + PatientIdentifier.from_nhs_number("nhs_number") + ) ), ) ], @@ -83,8 +108,18 @@ def test_invalid_request_method_returns_error(self, client: Client) -> None: path="FHIR/R4/Bundle", ) assert response.status_code == 404 - assert response.headers["Content-Type"] == "application/json" - assert response.json() == {"message": "Not found", "statusCode": 404} + assert response.headers["Content-Type"] == "application/fhir+json" + assert response.json() == { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "not-found", + "diagnostics": "No resource found for requested path. Path: " + "(GET) /FHIR/R4/Bundle", + } + ], + } class TestStatusEndpoint: @@ -102,6 +137,17 @@ def test_invalid_request_method(self, client: Client) -> None: request_method="POST", path="_status", ) + assert response.status_code == 404 - assert response.headers["Content-Type"] == "application/json" - assert response.json() == {"message": "Not found", "statusCode": 404} + assert response.headers["Content-Type"] == "application/fhir+json" + assert response.json() == { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "not-found", + "diagnostics": "No resource found for requested path. Path: " + "(POST) /_status", + } + ], + } From 4c772d323d633a5031c672686e5bef0bd262acfa Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Wed, 4 Feb 2026 15:58:36 +0000 Subject: [PATCH 12/20] [CDAPI-95]: Updated contract tests --- ...ologyAPIConsumer-PathologyAPIProvider.json | 37 ++++++++++--------- .../tests/contract/test_consumer_contract.py | 29 ++++++++------- 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/pathology-api/tests/contract/pacts/PathologyAPIConsumer-PathologyAPIProvider.json b/pathology-api/tests/contract/pacts/PathologyAPIConsumer-PathologyAPIProvider.json index 771dc4f..21e359d 100644 --- a/pathology-api/tests/contract/pacts/PathologyAPIConsumer-PathologyAPIProvider.json +++ b/pathology-api/tests/contract/pacts/PathologyAPIConsumer-PathologyAPIProvider.json @@ -23,13 +23,15 @@ "content": { "entry": [ { - "fullUrl": "patient", + "fullUrl": "composition", "resource": { - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number" - }, - "resourceType": "Patient" + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number" + } + } } } ], @@ -52,20 +54,19 @@ "content": { "entry": [ { - "fullUrl": "patient", + "fullUrl": "composition", "resource": { - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number" - }, - "resourceType": "Patient" + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number" + } + } } } ], - "identifier": { - "system": "https://tools.ietf.org/html/rfc4122", - "value": null - }, + "id": null, "meta": { "lastUpdated": "2026-01-16T12:00:00.000Z" }, @@ -77,7 +78,7 @@ }, "generators": { "body": { - "$.identifier.value": { + "$.id": { "type": "Uuid" } } @@ -89,7 +90,7 @@ }, "matchingRules": { "body": { - "$.identifier.value": { + "$.id": { "combine": "AND", "matchers": [ { diff --git a/pathology-api/tests/contract/test_consumer_contract.py b/pathology-api/tests/contract/test_consumer_contract.py index 7c44873..71be872 100644 --- a/pathology-api/tests/contract/test_consumer_contract.py +++ b/pathology-api/tests/contract/test_consumer_contract.py @@ -25,12 +25,14 @@ def test_post_bundle(self) -> None: "type": "document", "entry": [ { - "fullUrl": "patient", + "fullUrl": "composition", "resource": { - "resourceType": "Patient", - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number", + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + }, }, }, } @@ -42,20 +44,19 @@ def test_post_bundle(self) -> None: "type": "document", "entry": [ { - "fullUrl": "patient", + "fullUrl": "composition", "resource": { - "resourceType": "Patient", - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number", + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + }, }, }, } ], - "identifier": { - "system": "https://tools.ietf.org/html/rfc4122", - "value": match.uuid(), - }, + "id": match.uuid(), "meta": { "lastUpdated": match.datetime( "2026-01-16T12:00:00.000Z", format="%Y-%m-%dT%H:%M:%S.%fZ" From 3ac04f979ac04bf287da0a4fbcfb7e18dcb4f003 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Wed, 4 Feb 2026 16:03:43 +0000 Subject: [PATCH 13/20] [CDAPI-95]: Updated Acceptance tests --- .../tests/acceptance/steps/bundle_endpoint_steps.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pathology-api/tests/acceptance/steps/bundle_endpoint_steps.py b/pathology-api/tests/acceptance/steps/bundle_endpoint_steps.py index 6d1b4c6..9333e7e 100644 --- a/pathology-api/tests/acceptance/steps/bundle_endpoint_steps.py +++ b/pathology-api/tests/acceptance/steps/bundle_endpoint_steps.py @@ -1,7 +1,7 @@ """Step definitions for pathology API bundle endpoint feature.""" import requests -from pathology_api.fhir.r4.elements import PatientIdentifier +from pathology_api.fhir.r4.elements import LogicalReference, PatientIdentifier from pathology_api.fhir.r4.resources import Bundle, BundleType, Composition from pytest_bdd import given, parsers, then, when @@ -39,7 +39,9 @@ def step_send_valid_bundle(client: Client, response_context: ResponseContext) -> Bundle.Entry( fullUrl="composition", resource=Composition.create( - subject=PatientIdentifier.from_nhs_number("nhs_number") + subject=LogicalReference( + PatientIdentifier.from_nhs_number("nhs_number") + ) ), ) ], @@ -120,9 +122,7 @@ def step_check_response_contains_valid_bundle( f"Expected bundle type '{expected_type}', got: '{bundle.bundle_type}'" ) - assert bundle.identifier is not None, "Bundle identifier is missing." - assert bundle.identifier.system == "https://tools.ietf.org/html/rfc4122" - assert bundle.identifier.value is not None, "Bundle identifier value is missing." + assert bundle.id is not None, "Bundle ID is missing." def _validate_response_set(response_context: ResponseContext) -> requests.Response: assert response_context.response is not None, "Response has not been set." From dc17f45d02c684bbc099ecb0968dd0aa1ff3c487 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Fri, 6 Feb 2026 13:52:00 +0000 Subject: [PATCH 14/20] [CDAPI-95]: Added additional integration tests --- .../src/pathology_api/fhir/r4/resources.py | 4 +- .../tests/integration/test_endpoints.py | 304 ++++++++++++++++++ pathology-api/tests/integration/test_main.py | 153 --------- 3 files changed, 307 insertions(+), 154 deletions(-) create mode 100644 pathology-api/tests/integration/test_endpoints.py delete mode 100644 pathology-api/tests/integration/test_main.py diff --git a/pathology-api/src/pathology_api/fhir/r4/resources.py b/pathology-api/src/pathology_api/fhir/r4/resources.py index 5085be0..89741a2 100644 --- a/pathology-api/src/pathology_api/fhir/r4/resources.py +++ b/pathology-api/src/pathology_api/fhir/r4/resources.py @@ -161,7 +161,9 @@ class Specimen(Resource, resource_type="Specimen"): class Composition(Resource, resource_type="Composition"): """A FHIR R4 Composition resource.""" - subject: Annotated[LogicalReference[PatientIdentifier] | None, Field(frozen=True)] + subject: Annotated[ + LogicalReference[PatientIdentifier] | None, Field(frozen=True) + ] = None class OperationOutcome(BaseModel): diff --git a/pathology-api/tests/integration/test_endpoints.py b/pathology-api/tests/integration/test_endpoints.py new file mode 100644 index 0000000..f9f715b --- /dev/null +++ b/pathology-api/tests/integration/test_endpoints.py @@ -0,0 +1,304 @@ +"""Integration tests for the pathology API using pytest.""" + +import json +from typing import Any + +import pytest +from pathology_api.fhir.r4.elements import LogicalReference, PatientIdentifier +from pathology_api.fhir.r4.resources import Bundle, Composition + +from tests.conftest import Client + + +class TestBundleEndpoint: + def test_bundle_returns_200(self, client: Client) -> None: + bundle = Bundle.create( + type="document", + entry=[ + Bundle.Entry( + fullUrl="patient", + resource=Composition.create( + subject=LogicalReference( + PatientIdentifier.from_nhs_number("nhs_number") + ) + ), + ) + ], + ) + + response = client.send( + data=bundle.model_dump_json(by_alias=True), + path="FHIR/R4/Bundle", + request_method="POST", + ) + + assert response.status_code == 200 + assert response.headers["Content-Type"] == "application/fhir+json" + + response_data = response.json() + response_bundle = Bundle.model_validate(response_data, by_alias=True) + + assert response_bundle.bundle_type == bundle.bundle_type + assert response_bundle.entries == bundle.entries + + # A UUID value so can only check its presence. + assert response_bundle.id is not None + + assert response_bundle.meta is not None + response_meta = response_bundle.meta + assert response_meta.last_updated is not None + assert response_meta.version_id is None + + def test_no_payload_returns_error(self, client: Client) -> None: + response = client.send_without_payload( + request_method="POST", path="FHIR/R4/Bundle" + ) + assert response.status_code == 400 + + response_data = response.json() + assert response_data == { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "invalid", + "diagnostics": "Resources must be provided as a bundle of type" + " 'document'", + } + ], + } + + assert response.headers["Content-Type"] == "application/fhir+json" + assert response.status_code == 400 + + def test_empty_payload_returns_error(self, client: Client) -> None: + response = client.send(data="", request_method="POST", path="FHIR/R4/Bundle") + assert response.status_code == 400 + + response_data = response.json() + assert response_data == { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "invalid", + "diagnostics": "Resources must be provided as a bundle of type" + " 'document'", + } + ], + } + + assert response.headers["Content-Type"] == "application/fhir+json" + assert response.status_code == 400 + + @pytest.mark.parametrize( + ("payload", "expected_diagnostic"), + [ + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [], + }, + "Document must include a single Composition resource", + id="no composition", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "composition", + "resource": {"resourceType": "Composition"}, + } + ], + }, + "Composition does not define a valid subject identifier", + id="composition with no subject", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": {"identifier": {"value": "nhs_number"}}, + }, + } + ], + }, + "('entry', 0, 'resource', 'subject', 'identifier', 'system') " + "- Field required \n", + id="composition with subject but no system", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number" + } + }, + }, + } + ], + }, + "Composition does not define a valid subject identifier", + id="composition with subject but identifier has no value", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + }, + }, + { + "fullUrl": "invalid-resource", + "resource": {"resourceType": "InvalidResourceType"}, + }, + ], + }, + "Unsupported resourceType: InvalidResourceType", + id="bundle with unexpected resource type", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + }, + }, + { + "fullUrl": "composition-2", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + }, + }, + ], + }, + "Document must include a single Composition resource", + id="bundle with multiple compositions", + ), + ], + ) + def test_invalid_payload_returns_error( + self, client: Client, payload: dict[str, Any], expected_diagnostic: str + ) -> None: + response = client.send( + data=json.dumps(payload), request_method="POST", path="FHIR/R4/Bundle" + ) + assert response.status_code == 400 + + response_data = response.json() + assert response_data == { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "invalid", + "diagnostics": expected_diagnostic, + } + ], + } + + def test_invalid_request_method_returns_error(self, client: Client) -> None: + bundle = Bundle.create( + type="document", + entry=[ + Bundle.Entry( + fullUrl="patient", + resource=Composition.create( + subject=LogicalReference( + PatientIdentifier.from_nhs_number("nhs_number") + ) + ), + ) + ], + ) + + response = client.send( + data=bundle.model_dump_json(by_alias=True), + request_method="GET", + path="FHIR/R4/Bundle", + ) + assert response.status_code == 404 + assert response.headers["Content-Type"] == "application/fhir+json" + assert response.json() == { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "not-found", + "diagnostics": "No resource found for requested path. Path: " + "(GET) /FHIR/R4/Bundle", + } + ], + } + + +class TestStatusEndpoint: + def test_status_returns_200(self, client: Client) -> None: + response = client.send_without_payload(request_method="GET", path="_status") + assert response.status_code == 200 + assert response.headers["Content-Type"] == "text/plain" + + response_data = response.text + assert response_data == "OK" + + def test_invalid_request_method(self, client: Client) -> None: + response = client.send( + data="", + request_method="POST", + path="_status", + ) + + assert response.status_code == 404 + assert response.headers["Content-Type"] == "application/fhir+json" + assert response.json() == { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "not-found", + "diagnostics": "No resource found for requested path. Path: " + "(POST) /_status", + } + ], + } diff --git a/pathology-api/tests/integration/test_main.py b/pathology-api/tests/integration/test_main.py deleted file mode 100644 index 26b3fd2..0000000 --- a/pathology-api/tests/integration/test_main.py +++ /dev/null @@ -1,153 +0,0 @@ -"""Integration tests for the pathology API using pytest.""" - -from pathology_api.fhir.r4.elements import LogicalReference, PatientIdentifier -from pathology_api.fhir.r4.resources import Bundle, Composition - -from tests.conftest import Client - - -class TestBundleEndpoint: - def test_bundle_returns_200(self, client: Client) -> None: - bundle = Bundle.create( - type="document", - entry=[ - Bundle.Entry( - fullUrl="patient", - resource=Composition.create( - subject=LogicalReference( - PatientIdentifier.from_nhs_number("nhs_number") - ) - ), - ) - ], - ) - - response = client.send( - data=bundle.model_dump_json(by_alias=True), - path="FHIR/R4/Bundle", - request_method="POST", - ) - - assert response.status_code == 200 - assert response.headers["Content-Type"] == "application/fhir+json" - - response_data = response.json() - response_bundle = Bundle.model_validate(response_data, by_alias=True) - - assert response_bundle.bundle_type == bundle.bundle_type - assert response_bundle.entries == bundle.entries - - # A UUID value so can only check its presence. - assert response_bundle.id is not None - - assert response_bundle.meta is not None - response_meta = response_bundle.meta - assert response_meta.last_updated is not None - assert response_meta.version_id is None - - def test_no_payload_returns_error(self, client: Client) -> None: - response = client.send_without_payload( - request_method="POST", path="FHIR/R4/Bundle" - ) - assert response.status_code == 400 - - response_data = response.json() - assert response_data == { - "resourceType": "OperationOutcome", - "issue": [ - { - "severity": "error", - "code": "invalid", - "diagnostics": "Resources must be provided as a bundle of type" - " 'document'", - } - ], - } - - assert response.headers["Content-Type"] == "application/fhir+json" - assert response.status_code == 400 - - def test_empty_name_returns_error(self, client: Client) -> None: - response = client.send(data="", request_method="POST", path="FHIR/R4/Bundle") - assert response.status_code == 400 - - response_data = response.json() - assert response_data == { - "resourceType": "OperationOutcome", - "issue": [ - { - "severity": "error", - "code": "invalid", - "diagnostics": "Resources must be provided as a bundle of type" - " 'document'", - } - ], - } - - assert response.headers["Content-Type"] == "application/fhir+json" - assert response.status_code == 400 - - def test_invalid_request_method_returns_error(self, client: Client) -> None: - bundle = Bundle.create( - type="document", - entry=[ - Bundle.Entry( - fullUrl="patient", - resource=Composition.create( - subject=LogicalReference( - PatientIdentifier.from_nhs_number("nhs_number") - ) - ), - ) - ], - ) - - response = client.send( - data=bundle.model_dump_json(by_alias=True), - request_method="GET", - path="FHIR/R4/Bundle", - ) - assert response.status_code == 404 - assert response.headers["Content-Type"] == "application/fhir+json" - assert response.json() == { - "resourceType": "OperationOutcome", - "issue": [ - { - "severity": "error", - "code": "not-found", - "diagnostics": "No resource found for requested path. Path: " - "(GET) /FHIR/R4/Bundle", - } - ], - } - - -class TestStatusEndpoint: - def test_status_returns_200(self, client: Client) -> None: - response = client.send_without_payload(request_method="GET", path="_status") - assert response.status_code == 200 - assert response.headers["Content-Type"] == "text/plain" - - response_data = response.text - assert response_data == "OK" - - def test_invalid_request_method(self, client: Client) -> None: - response = client.send( - data="", - request_method="POST", - path="_status", - ) - - assert response.status_code == 404 - assert response.headers["Content-Type"] == "application/fhir+json" - assert response.json() == { - "resourceType": "OperationOutcome", - "issue": [ - { - "severity": "error", - "code": "not-found", - "diagnostics": "No resource found for requested path. Path: " - "(POST) /_status", - } - ], - } From fed873549a7e4d4cb115cddddec1a332e021d560 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Fri, 6 Feb 2026 16:58:34 +0000 Subject: [PATCH 15/20] [CDAPI-95]: Added Schemathesis hooks to fix schema tests --- pathology-api/openapi.yaml | 237 ++++++++++++++++--------- pathology-api/tests/schema/conftest.py | 62 +++++++ 2 files changed, 217 insertions(+), 82 deletions(-) create mode 100644 pathology-api/tests/schema/conftest.py diff --git a/pathology-api/openapi.yaml b/pathology-api/openapi.yaml index 1be90a7..ed23195 100644 --- a/pathology-api/openapi.yaml +++ b/pathology-api/openapi.yaml @@ -1,4 +1,4 @@ -openapi: 3.0.3 +openapi: 3.1.2 info: title: Pathology and Laboratory Medicine Reporting - FHIR API description: Clinical Data Pathology API @@ -40,117 +40,190 @@ paths: type: string enum: - document - - transaction description: The type of the bundle example: document entry: type: array description: Entries in the bundle - minItems: 1 - maxItems: 1 items: type: object required: - fullUrl - resource + contains: + type: object + properties: + resource: + type: object + required: + - resourceType + resourceType: + type: string + const: Composition + maxContains: 1 properties: fullUrl: type: string description: URI for resource - example: "patient" + example: "composition" resource: - type: object - required: - - resourceType - - identifier - properties: - resourceType: - type: string - description: Type of FHIR resource (Always "Patient") - enum: - - Patient - example: Patient - identifier: - type: object + anyOf: + - $ref: "#/components/schemas/CompositionResource" + - type: object required: - - system - - value + - resourceType properties: - system: + resourceType: type: string + description: Type of FHIR resource (e.g. "Patient", "Observation", etc.) enum: - - "https://fhir.nhs.uk/Id/nhs-number" - example: "https://fhir.nhs.uk/Id/nhs-number" - value: - type: string - example: "9999999999" - + - Patient + - Observation + - DiagnosticReport + - Specimen + - ServiceRequest + - Practitioner + - PractitionerRole + - Organization + example: Patient responses: '200': description: Successful response content: application/fhir+json: schema: - schema: - type: object - required: - - resourceType - - type - properties: - resourceType: - type: string - enum: - - Bundle - description: FHIR resource type (always "Bundle") - meta: - type: object - description: Metadata about the resource - nullable: true - type: - type: string - enum: - - document - - transaction - description: The type of the bundle - identifier: - type: object - nullable: true - description: Persistent identifier for the bundle (UUID) - properties: - system: - type: string - format: uri - value: - type: string - format: uuid - entry: - type: array - nullable: true - description: Entries in the bundle - items: + type: object + required: + - resourceType + - type + properties: + resourceType: + type: string + enum: + - Bundle + description: FHIR resource type (always "Bundle") + meta: type: object - required: - - fullUrl - - resource + description: Metadata about the resource + nullable: true + type: + type: string + enum: + - document + - transaction + description: The type of the bundle + identifier: + type: object + nullable: true + description: Persistent identifier for the bundle (UUID) properties: - fullUrl: + system: type: string - description: URI for resource - resource: - type: object - required: - - resourceType - description: The Patient a test result is for - properties: - resourceType: - type: string - description: Type of FHIR resource (always "Patient") - enum: - - Patient - example: Patient + format: uri + value: + type: string + format: uuid + entry: + type: array + nullable: true + description: Entries in the bundle + items: + type: object + required: + - fullUrl + - resource + properties: + fullUrl: + type: string + description: URI for resource + resource: + type: object + required: + - resourceType '400': description: Invalid request content: - text/plain: + application/fhir+json: + schema: + $ref: "#/components/schemas/OperationOutcome" + '404': + description: Unknown resource requested + content: + application/fhir+json: schema: + $ref: "#/components/schemas/OperationOutcome" + '500': + description: Unexpected internal error + content: + application/fhir+json: + schema: + $ref: "#/components/schemas/OperationOutcome" + +components: + schemas: + CompositionResource: + type: object + required: + - resourceType + - subject + properties: + resourceType: + type: string + description: Type of FHIR resource (Always "Composition") + enum: + - Composition + example: Composition + subject: + type: object + required: + - identifier + properties: + identifier: + type: object + required: + - system + - value + properties: + system: + type: string + enum: + - "https://fhir.nhs.uk/Id/nhs-number" + example: "https://fhir.nhs.uk/Id/nhs-number" + value: + type: string + example: "9999999999" + OperationOutcome: + type: object + required: + - resourceType + - issue + properties: + resourceType: + type: string + enum: + - OperationOutcome + example: OperationOutcome + issue: + type: array + items: + type: object + required: + - severity + - code + - diagnostics + properties: + severity: + type: string + enum: + - fatal + - error + example: error + code: + type: string + enum: + - invalid + - not-found + - exception + example: invalid + diagnostics: type: string diff --git a/pathology-api/tests/schema/conftest.py b/pathology-api/tests/schema/conftest.py new file mode 100644 index 0000000..0bc35ec --- /dev/null +++ b/pathology-api/tests/schema/conftest.py @@ -0,0 +1,62 @@ +from typing import Any + +import schemathesis + + +def _find_entries(body: dict[str, Any]) -> list[dict[str, Any]]: + if "entry" in body and isinstance(body["entry"], list): + return body["entry"] + return [] + + +def _find_compositions(entries: list[dict[str, Any]]) -> list[dict[str, Any]]: + return [ + item["resource"] + for item in entries + if item.get("resource") is not None + and isinstance(item["resource"], dict) + and item["resource"].get("resourceType") == "Composition" + ] + + +@schemathesis.hook("before_call") +def ensure_composition_in_body( + _ctx: schemathesis.HookContext, case: schemathesis.Case, *_kwargs: Any +) -> schemathesis.Case: + """ + Hook to ensure that when schemathesis generates a request body, + it always contains a Composition resource. + """ + if isinstance(case.body, dict): + entries = _find_entries(case.body) + if len(_find_compositions(entries)) == 0: + # If no Composition resource is found, add a valid entry to satisfy + # the schema + entries.append( + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + }, + } + ) + case.body["entry"] = entries + return case + + +@schemathesis.hook("filter_body") +def ignore_multiple_composition_requests( + _: schemathesis.HookContext, body: dict[str, Any] +) -> bool: + """ + Hook to filter out any requests generated by schemathesis that contain more + than one Composition resource. + """ + composition_resources = _find_compositions(_find_entries(body)) + return len(composition_resources) < 2 From 25530c3464083182c5acce09ae8254f7b6c16861 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Mon, 9 Feb 2026 12:00:28 +0000 Subject: [PATCH 16/20] [CDAPI-95]: Swapped OAS file back to version 3.0.3 and added initial documentation around required resource types --- pathology-api/openapi.yaml | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/pathology-api/openapi.yaml b/pathology-api/openapi.yaml index ed23195..72e2072 100644 --- a/pathology-api/openapi.yaml +++ b/pathology-api/openapi.yaml @@ -1,4 +1,4 @@ -openapi: 3.1.2 +openapi: 3.0.3 info: title: Pathology and Laboratory Medicine Reporting - FHIR API description: Clinical Data Pathology API @@ -44,23 +44,16 @@ paths: example: document entry: type: array - description: Entries in the bundle + description: | + Entries that form a Test Result. Any included entries should comply with the [Pathology FHIR Implementation Guide|https://simplifier.net/guide/pathology-fhir-implementation-guide/Home?version=0.2.0], though the resources listed below are additionally required by the Pathology API: + | Resource Type | Required Fields | + |---------------|-----------------| + | Composition | - `subject` - Required to identify the Patient a given Test Result is for. The Subject should include an `identifier` referencing the `https://fhir.nhs.uk/Id/nhs-number` System value. | items: type: object required: - fullUrl - resource - contains: - type: object - properties: - resource: - type: object - required: - - resourceType - resourceType: - type: string - const: Composition - maxContains: 1 properties: fullUrl: type: string From 4489779dc37d55846c0d645ffa39a6102677aad3 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Mon, 9 Feb 2026 12:05:19 +0000 Subject: [PATCH 17/20] [CDAPI-95]: Moved security schemes into main components section of OpenAPI spec --- pathology-api/openapi.yaml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pathology-api/openapi.yaml b/pathology-api/openapi.yaml index 72e2072..77428b0 100644 --- a/pathology-api/openapi.yaml +++ b/pathology-api/openapi.yaml @@ -8,10 +8,6 @@ info: servers: - url: http://localhost:5002 description: Local development server -components: - securitySchemes: - app-level3: - $ref: https://proxygen.ptl.api.platform.nhs.uk/components/securitySchemes/app-level3 paths: /FHIR/R4/Bundle: post: @@ -153,6 +149,9 @@ paths: $ref: "#/components/schemas/OperationOutcome" components: + securitySchemes: + app-level3: + $ref: https://proxygen.ptl.api.platform.nhs.uk/components/securitySchemes/app-level3 schemas: CompositionResource: type: object From 3832035ed63aff7ebd2c45b1b6c4c96812ff3f28 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Mon, 9 Feb 2026 15:31:40 +0000 Subject: [PATCH 18/20] [CDAPI-95]: Minor tidy up before submitting for review --- pathology-api/lambda_handler.py | 8 +++--- .../src/pathology_api/fhir/r4/resources.py | 6 ++--- .../pathology_api/fhir/r4/test_resources.py | 25 ++++++++++++++----- pathology-api/test_lambda_handler.py | 4 +-- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/pathology-api/lambda_handler.py b/pathology-api/lambda_handler.py index 5d94489..f284536 100644 --- a/pathology-api/lambda_handler.py +++ b/pathology-api/lambda_handler.py @@ -58,7 +58,7 @@ def handle_validation_error(exception: ValidationError) -> Response[str]: ) return _with_default_headers( status_code=400, - body=OperationOutcome.raise_validation_error(exception.message), + body=OperationOutcome.create_validation_error(exception.message), ) @@ -73,7 +73,7 @@ def handle_pydantic_validation_error( exc_info=True, # noqa: LOG014 ) - operation_outcome = OperationOutcome.raise_validation_error( + operation_outcome = OperationOutcome.create_validation_error( reduce( lambda acc, e: acc + f"{str(e['loc'])} - {e['msg']} \n", exception.errors(), @@ -91,7 +91,7 @@ def handle_not_found_error(exception: NotFoundError) -> Response[str]: _logger.info("NotFoundError encountered: %s", exception, exec_info=True) return _with_default_headers( status_code=404, - body=OperationOutcome.raise_not_found_error( + body=OperationOutcome.create_not_found_error( "No resource found for requested path. " f"Path: ({app.current_event.http_method}) {app.current_event.path}" ), @@ -103,7 +103,7 @@ def handle_exception(exception: Exception) -> Response[str]: _logger.exception("Unhandled Exception encountered: %s", exception) return _with_default_headers( status_code=500, - body=OperationOutcome.raise_server_error( + body=OperationOutcome.create_server_error( "An unexpected error has occurred. Please try again later." ), ) diff --git a/pathology-api/src/pathology_api/fhir/r4/resources.py b/pathology-api/src/pathology_api/fhir/r4/resources.py index 89741a2..052052e 100644 --- a/pathology-api/src/pathology_api/fhir/r4/resources.py +++ b/pathology-api/src/pathology_api/fhir/r4/resources.py @@ -187,7 +187,7 @@ class Issue(TypedDict): issue: list[Issue] = Field(frozen=True) @classmethod - def raise_validation_error(cls, diagnostics: str) -> Self: + def create_validation_error(cls, diagnostics: str) -> Self: """ Create an OperationOutcome with the provided diagnostic as a validation error. Args: @@ -206,7 +206,7 @@ def raise_validation_error(cls, diagnostics: str) -> Self: ) @classmethod - def raise_server_error(cls, diagnostics: str | None = None) -> Self: + def create_server_error(cls, diagnostics: str | None = None) -> Self: """ Create an OperationOutcome with the provided diagnostics as a server error. Args: @@ -225,7 +225,7 @@ def raise_server_error(cls, diagnostics: str | None = None) -> Self: ) @classmethod - def raise_not_found_error(cls, diagnostics: str | None = None) -> Self: + def create_not_found_error(cls, diagnostics: str | None = None) -> Self: """ Create an OperationOutcome with the provided diagnostics as a not found error. Args: diff --git a/pathology-api/src/pathology_api/fhir/r4/test_resources.py b/pathology-api/src/pathology_api/fhir/r4/test_resources.py index 627b552..1d96870 100644 --- a/pathology-api/src/pathology_api/fhir/r4/test_resources.py +++ b/pathology-api/src/pathology_api/fhir/r4/test_resources.py @@ -216,10 +216,10 @@ def test_find_resources_returns_empty_list(self, bundle: Bundle) -> None: class TestOperationOutcome: - def test_raise_validation_error(self) -> None: + def test_create_validation_error(self) -> None: expected_diagnostics = "Invalid patient identifier format" - outcome = OperationOutcome.raise_validation_error(expected_diagnostics) + outcome = OperationOutcome.create_validation_error(expected_diagnostics) assert outcome.resource_type == "OperationOutcome" assert len(outcome.issue) == 1 @@ -229,12 +229,25 @@ def test_raise_validation_error(self) -> None: assert issue["code"] == "invalid" assert issue["diagnostics"] == expected_diagnostics + def test_create_not_found_error(self) -> None: + expected_diagnostics = "Requested resource not found" + + outcome = OperationOutcome.create_not_found_error(expected_diagnostics) + + assert outcome.resource_type == "OperationOutcome" + assert len(outcome.issue) == 1 + + issue = outcome.issue[0] + assert issue["severity"] == "error" + assert issue["code"] == "not-found" + assert issue["diagnostics"] == expected_diagnostics + @pytest.mark.parametrize( ("diagnostics", "expected_diagnostics"), [ pytest.param( - "Database connection failed", - "Database connection failed", + "Unexpected error", + "Unexpected error", id="with_diagnostics", ), pytest.param( @@ -244,10 +257,10 @@ def test_raise_validation_error(self) -> None: ), ], ) - def test_raise_server_error( + def test_create_server_error( self, diagnostics: str | None, expected_diagnostics: str | None ) -> None: - outcome = OperationOutcome.raise_server_error(diagnostics) + outcome = OperationOutcome.create_server_error(diagnostics) assert outcome.resource_type == "OperationOutcome" assert len(outcome.issue) == 1 diff --git a/pathology-api/test_lambda_handler.py b/pathology-api/test_lambda_handler.py index caf2ce8..7a31dfb 100644 --- a/pathology-api/test_lambda_handler.py +++ b/pathology-api/test_lambda_handler.py @@ -199,7 +199,7 @@ def test_create_test_result_processing_error( ), ], ) - def test_create_test_result_parse_json_error( + def test_create_test_result_model_validate_error( self, expected_error: Exception, expected_diagnostic: str ) -> None: bundle = Bundle.empty(bundle_type="document") @@ -242,7 +242,7 @@ def test_status_success(self) -> None: pytest.param("POST", "_status", id="Unknown POST method"), ], ) - def test_invalid_request(self, request_method: str, request_parameter: str) -> None: + def test_unknown_request(self, request_method: str, request_parameter: str) -> None: event = self._create_test_event( path_params=request_parameter, request_method=request_method ) From da022f5d2ef3213a57bfe3aa6eebb93e62afba4e Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Mon, 9 Feb 2026 17:42:46 +0000 Subject: [PATCH 19/20] [CDAPI-95]: Updated Bruno collection to account for changes to Test Result endpoint Also reconciled APIM and PDM auth call to be only held within APIM collection, with the generated access token being available to all collections via a global environment variable. --- bruno/APIM/Get_Auth_Token.bru | 46 +++++- bruno/APIM/Post_Document_Bundle_via_APIM.bru | 18 +- bruno/APIM/README.md | 29 +++- bruno/APIM/environments/APIM.bru | 2 +- .../Bundle/Post_a_Batch_Bundle_with_gets.bru | 2 +- .../PDM/Bundle/Post_a_Transaction_Bundle.bru | 2 +- bruno/PDM/Bundle/folder.bru | 2 +- bruno/PDM/Document/Post_a_Document.bru | 2 +- ...ogy-Bundle-CRP-Report-Document-Example.bru | 2 +- bruno/PDM/Document/Retrieve_Document.bru | 2 +- bruno/PDM/Document/folder.bru | 2 +- bruno/PDM/Get_Auth_Token.bru | 30 ---- bruno/PDM/Observation/Create_Observation.bru | 2 +- .../PDM/Observation/Retrieve_Observation.bru | 2 +- bruno/PDM/Observation/folder.bru | 2 +- bruno/PDM/README.md | 8 +- bruno/PDM/Specimen/Create_Specimen.bru | 2 +- bruno/PDM/Specimen/Retrieve_Specimen.bru | 2 +- bruno/PDM/Specimen/folder.bru | 2 +- bruno/PDM/environments/PDM.bru | 7 +- bruno/PDM/package-lock.json | 154 ------------------ bruno/PDM/package.json | 15 -- bruno/common/auth-token.js | 47 ------ 23 files changed, 90 insertions(+), 292 deletions(-) delete mode 100644 bruno/PDM/Get_Auth_Token.bru delete mode 100644 bruno/PDM/package-lock.json delete mode 100644 bruno/PDM/package.json delete mode 100644 bruno/common/auth-token.js diff --git a/bruno/APIM/Get_Auth_Token.bru b/bruno/APIM/Get_Auth_Token.bru index 05f25d1..74257fa 100644 --- a/bruno/APIM/Get_Auth_Token.bru +++ b/bruno/APIM/Get_Auth_Token.bru @@ -5,7 +5,7 @@ meta { } post { - url: https://internal-dev.api.service.nhs.uk/oauth2/token + url: https://{{APIM_ENV}}.api.service.nhs.uk/oauth2/token body: formUrlEncoded auth: none } @@ -16,12 +16,50 @@ body:form-urlencoded { } script:pre-request { - const { generateAuthToken } = require("../common/auth-token"); - generateAuthToken(bru, req, "https://internal-dev.api.service.nhs.uk/oauth2/token", "kid-1"); + function generateAuthToken(bru, req, audienceUrl, kid) { + const jwt = require("jsonwebtoken"); + const fs = require("node:fs"); + const crypto = require("node:crypto"); + + const secret = bru.getEnvVar("JWT_SECRET"); + const privateKeyPath = bru.getEnvVar("PRIVATE_KEY_PATH"); + + if (!secret) { + throw new Error("JWT_SECRET environment variable is missing."); + } + if (!privateKeyPath) { + throw new Error("PRIVATE_KEY_PATH environment variable is missing."); + } + + const privateKey = fs.readFileSync(privateKeyPath); + + const payload = { + sub: secret, + iss: secret, + jti: crypto.randomUUID(), + aud: audienceUrl, + exp: (Date.now() / 1000) + 300 + }; + + const options = { + algorithm: 'RS512', + header: { kid: kid } + }; + + const token = jwt.sign(payload, privateKey, options); + + let new_body = req.getBody(); + new_body.push({ name: "client_assertion", value: token }); + + req.setBody(new_body); + } + + const environment = bru.getGlobalEnvVar("APIM_ENV") + generateAuthToken(bru, req, `https://${environment}.api.service.nhs.uk/oauth2/token`, bru.getEnvVar("KID")); } script:post-response { - bru.setEnvVar("auth_token", res.getBody().access_token) + bru.setGlobalEnvVar("auth_token", res.getBody().access_token) } settings { diff --git a/bruno/APIM/Post_Document_Bundle_via_APIM.bru b/bruno/APIM/Post_Document_Bundle_via_APIM.bru index 18e078d..9f51f6c 100644 --- a/bruno/APIM/Post_Document_Bundle_via_APIM.bru +++ b/bruno/APIM/Post_Document_Bundle_via_APIM.bru @@ -5,7 +5,7 @@ meta { } post { - url: https://internal-dev.api.service.nhs.uk/pathology-laboratory-reporting-pr-{{PR_NUMBER}}/FHIR/R4/Bundle + url: https://{{APIM_ENV}}.api.service.nhs.uk/pathology-laboratory-reporting-pr-{{PR_NUMBER}}/FHIR/R4/Bundle body: json auth: inherit } @@ -20,14 +20,16 @@ body:json { "type": "document", "entry": [ { - "fullUrl": "patient", - "resource": { - "resourceType": "Patient", - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "test-nhs-number" - } + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "test-nhs-number" + } } + } } ] } diff --git a/bruno/APIM/README.md b/bruno/APIM/README.md index ffabc36..7b33dac 100644 --- a/bruno/APIM/README.md +++ b/bruno/APIM/README.md @@ -14,13 +14,24 @@ Your feature branch must have an open pull request (draft or ready for review) t The following environment variables will need to be configured in Bruno: -| Variable | Description | Example | -| ------------------ | ---------------------------------------------------- | ----------------------------- | -| `PRIVATE_KEY_PATH` | Path to your private key file on your local machine | `/home/user/.ssh/api-key.pem` | -| `JWT_SECRET` | Active API Key from your Developer Hub application | `your-api-key-here` | -| `PR_NUMBER` | The pull request number for your preview environment | `123` | +| Variable | Description | Example | +| ------------------ | ---------------------------------------------------- | ----------------------------- | +| `PRIVATE_KEY_PATH` | Path to your private key file on your local machine | `/home/user/.ssh/api-key.pem` | +| `JWT_SECRET` | Active API Key from your Developer Hub application | `your-api-key-here` | +| `PR_NUMBER` | The pull request number for your preview environment | `123` | +| `APIM_ENV` | The APIM environment you're testing against | `internal-dev` | +| `KID` | The Key ID to utilise when generating an access token | `INT-1` | -### 3. Developer Hub Application Setup +### 3. Bruno Global Environment Variables + +The following environment variables also need to be configured as global variables in Bruno: + +| Variable | Description | Example | Secret | +| ------------------ | ----------------------------------------------------- | ----------------------------- | ------ | +| `APIM_ENV` | The APIM environment you're testing against | `internal-dev` | | +| `auth_token` | The auth token to use when accessing APIM | `your-auth-token-here` | x | + +### 4. Developer Hub Application Setup Register an application on the [Internal Developer Hub](https://dos-internal.ptl.api.platform.nhs.uk/Index): @@ -28,10 +39,10 @@ Register an application on the [Internal Developer Hub](https://dos-internal.ptl 2. Upload the public key to your application 3. Copy the **Active API Key** and set it as the `JWT_SECRET` environment variable in Bruno -### 4. Configure Proxy Endpoint +### 5. Configure Proxy Endpoint -The POST request URL automatically targets your preview environment proxy using the `PR_NUMBER` environment variable, which you will need to set. The URL follows this format: +The POST request URL automatically targets your preview environment proxy using the `PR_NUMBER` and `APIM_ENV` environment variables, which you will need to set. The URL follows this format: ```text -https://internal-dev.api.service.nhs.uk/pathology-laboratory-reporting-pr-{{PR_NUMBER}}/FHIR/R4/Bundle +https://{{APIM_ENV}}.api.service.nhs.uk/pathology-laboratory-reporting-pr-{{PR_NUMBER}}/FHIR/R4/Bundle ``` diff --git a/bruno/APIM/environments/APIM.bru b/bruno/APIM/environments/APIM.bru index c3033ec..7e5d0a6 100644 --- a/bruno/APIM/environments/APIM.bru +++ b/bruno/APIM/environments/APIM.bru @@ -2,5 +2,5 @@ vars:secret [ PRIVATE_KEY_PATH, JWT_SECRET, PR_NUMBER, - auth_token + KID ] diff --git a/bruno/PDM/Bundle/Post_a_Batch_Bundle_with_gets.bru b/bruno/PDM/Bundle/Post_a_Batch_Bundle_with_gets.bru index 5cad76b..ab62d6c 100644 --- a/bruno/PDM/Bundle/Post_a_Batch_Bundle_with_gets.bru +++ b/bruno/PDM/Bundle/Post_a_Batch_Bundle_with_gets.bru @@ -5,7 +5,7 @@ meta { } post { - url: https://int.api.service.nhs.uk/patient-data-manager/FHIR/R4/ + url: https://{{APIM_ENV}}.api.service.nhs.uk/patient-data-manager/FHIR/R4/ body: json auth: inherit } diff --git a/bruno/PDM/Bundle/Post_a_Transaction_Bundle.bru b/bruno/PDM/Bundle/Post_a_Transaction_Bundle.bru index 459ae42..1f289b1 100644 --- a/bruno/PDM/Bundle/Post_a_Transaction_Bundle.bru +++ b/bruno/PDM/Bundle/Post_a_Transaction_Bundle.bru @@ -5,7 +5,7 @@ meta { } post { - url: https://int.api.service.nhs.uk/patient-data-manager/FHIR/R4/ + url: https://{{APIM_ENV}}.api.service.nhs.uk/patient-data-manager/FHIR/R4/ body: json auth: inherit } diff --git a/bruno/PDM/Bundle/folder.bru b/bruno/PDM/Bundle/folder.bru index a9ae5e9..d403d4f 100644 --- a/bruno/PDM/Bundle/folder.bru +++ b/bruno/PDM/Bundle/folder.bru @@ -1,6 +1,6 @@ meta { name: Bundle - seq: 6 + seq: 4 } auth { diff --git a/bruno/PDM/Document/Post_a_Document.bru b/bruno/PDM/Document/Post_a_Document.bru index a76c22f..c8435ff 100644 --- a/bruno/PDM/Document/Post_a_Document.bru +++ b/bruno/PDM/Document/Post_a_Document.bru @@ -5,7 +5,7 @@ meta { } post { - url: https://int.api.service.nhs.uk/patient-data-manager/FHIR/R4/Bundle + url: https://{{APIM_ENV}}.api.service.nhs.uk/patient-data-manager/FHIR/R4/Bundle body: json auth: inherit } diff --git a/bruno/PDM/Document/Post_a_Document_Pathology-Bundle-CRP-Report-Document-Example.bru b/bruno/PDM/Document/Post_a_Document_Pathology-Bundle-CRP-Report-Document-Example.bru index a9ad773..4b0f606 100644 --- a/bruno/PDM/Document/Post_a_Document_Pathology-Bundle-CRP-Report-Document-Example.bru +++ b/bruno/PDM/Document/Post_a_Document_Pathology-Bundle-CRP-Report-Document-Example.bru @@ -5,7 +5,7 @@ meta { } post { - url: https://int.api.service.nhs.uk/patient-data-manager/FHIR/R4/Bundle + url: https://{{APIM_ENV}}.api.service.nhs.uk/patient-data-manager/FHIR/R4/Bundle body: json auth: inherit } diff --git a/bruno/PDM/Document/Retrieve_Document.bru b/bruno/PDM/Document/Retrieve_Document.bru index c5831f6..cb9ead8 100644 --- a/bruno/PDM/Document/Retrieve_Document.bru +++ b/bruno/PDM/Document/Retrieve_Document.bru @@ -5,7 +5,7 @@ meta { } get { - url: https://int.api.service.nhs.uk/patient-data-manager/FHIR/R4/Bundle/6a0c6a4d-9941-35cf-b83d-76fa4b880a85 + url: https://{{APIM_ENV}}.api.service.nhs.uk/patient-data-manager/FHIR/R4/Bundle/6a0c6a4d-9941-35cf-b83d-76fa4b880a85 body: none auth: inherit } diff --git a/bruno/PDM/Document/folder.bru b/bruno/PDM/Document/folder.bru index 0139902..b67b74f 100644 --- a/bruno/PDM/Document/folder.bru +++ b/bruno/PDM/Document/folder.bru @@ -1,6 +1,6 @@ meta { name: Document - seq: 5 + seq: 3 } auth { diff --git a/bruno/PDM/Get_Auth_Token.bru b/bruno/PDM/Get_Auth_Token.bru deleted file mode 100644 index c8e43e4..0000000 --- a/bruno/PDM/Get_Auth_Token.bru +++ /dev/null @@ -1,30 +0,0 @@ -meta { - name: Get Auth Token - type: http - seq: 2 -} - -post { - url: https://int.api.service.nhs.uk/oauth2/token - body: formUrlEncoded - auth: none -} - -body:form-urlencoded { - grant_type: client_credentials - client_assertion_type: urn:ietf:params:oauth:client-assertion-type:jwt-bearer -} - -script:pre-request { - const { generateAuthToken } = require("../common/auth-token"); - generateAuthToken(bru, req, "https://int.api.service.nhs.uk/oauth2/token", "INT-1"); -} - -script:post-response { - bru.setEnvVar("auth_token", res.getBody().access_token) -} - -settings { - encodeUrl: true - timeout: 0 -} diff --git a/bruno/PDM/Observation/Create_Observation.bru b/bruno/PDM/Observation/Create_Observation.bru index 15cdb7a..6d1d62b 100644 --- a/bruno/PDM/Observation/Create_Observation.bru +++ b/bruno/PDM/Observation/Create_Observation.bru @@ -5,7 +5,7 @@ meta { } post { - url: https://int.api.service.nhs.uk/patient-data-manager/FHIR/R4/Observation + url: https://{{APIM_ENV}}.api.service.nhs.uk/patient-data-manager/FHIR/R4/Observation body: json auth: inherit } diff --git a/bruno/PDM/Observation/Retrieve_Observation.bru b/bruno/PDM/Observation/Retrieve_Observation.bru index fceaaee..6fe4755 100644 --- a/bruno/PDM/Observation/Retrieve_Observation.bru +++ b/bruno/PDM/Observation/Retrieve_Observation.bru @@ -5,7 +5,7 @@ meta { } get { - url: https://int.api.service.nhs.uk/patient-data-manager/FHIR/R4/Observation/ec5e3a08-a4fe-462b-b627-d553b53a66f2 + url: https://{{APIM_ENV}}.api.service.nhs.uk/patient-data-manager/FHIR/R4/Observation/ec5e3a08-a4fe-462b-b627-d553b53a66f2 body: none auth: inherit } diff --git a/bruno/PDM/Observation/folder.bru b/bruno/PDM/Observation/folder.bru index 91b09ac..15fb638 100644 --- a/bruno/PDM/Observation/folder.bru +++ b/bruno/PDM/Observation/folder.bru @@ -1,6 +1,6 @@ meta { name: Observation - seq: 3 + seq: 1 } auth { diff --git a/bruno/PDM/README.md b/bruno/PDM/README.md index b6e4e7e..e14c8a7 100644 --- a/bruno/PDM/README.md +++ b/bruno/PDM/README.md @@ -1,13 +1,9 @@ # PDM Collection Setup -## Install dependencies - -While in the dev container, navigate to the PDM collection directory and run the command `npm install` - ## Authentication Setup -Follow the instructions on this [confluence page](https://nhsd-confluence.digital.nhs.uk/x/ixnIT) to setup Bruno with the PDM INT Environment +Follow the instructions on this [confluence page](https://nhsd-confluence.digital.nhs.uk/x/ixnIT) to setup Bruno with the PDM INT Environment. Authentication can the be completed via the `Get Auth Token` call within the `APIM` collection. ## Getting Auth Token -Once you have completed the previous instructions you should be able to run the Get Auth Token request, once the request is complete it should copy the returned token into the `auth_token` environment variable. The collection has been setup to automatically use this variable to authenticate requests +Once the request is complete it should copy the returned token into the `auth_token` global environment variable. The collection has been setup to automatically use this variable to authenticate requests diff --git a/bruno/PDM/Specimen/Create_Specimen.bru b/bruno/PDM/Specimen/Create_Specimen.bru index 24b9f59..5a3bcc3 100644 --- a/bruno/PDM/Specimen/Create_Specimen.bru +++ b/bruno/PDM/Specimen/Create_Specimen.bru @@ -5,7 +5,7 @@ meta { } post { - url: https://int.api.service.nhs.uk/patient-data-manager/FHIR/R4/Specimen + url: https://{{APIM_ENV}}.api.service.nhs.uk/patient-data-manager/FHIR/R4/Specimen body: json auth: inherit } diff --git a/bruno/PDM/Specimen/Retrieve_Specimen.bru b/bruno/PDM/Specimen/Retrieve_Specimen.bru index cd7e02f..5eac1ff 100644 --- a/bruno/PDM/Specimen/Retrieve_Specimen.bru +++ b/bruno/PDM/Specimen/Retrieve_Specimen.bru @@ -5,7 +5,7 @@ meta { } get { - url: https://int.api.service.nhs.uk/patient-data-manager/FHIR/R4/Specimen/6a0c6a4d-9941-35cf-b83d-76fa4b880a85 + url: https://{{APIM_ENV}}.api.service.nhs.uk/patient-data-manager/FHIR/R4/Specimen/6a0c6a4d-9941-35cf-b83d-76fa4b880a85 body: none auth: inherit } diff --git a/bruno/PDM/Specimen/folder.bru b/bruno/PDM/Specimen/folder.bru index 02d6445..a5d21ca 100644 --- a/bruno/PDM/Specimen/folder.bru +++ b/bruno/PDM/Specimen/folder.bru @@ -1,6 +1,6 @@ meta { name: Specimen - seq: 4 + seq: 2 } auth { diff --git a/bruno/PDM/environments/PDM.bru b/bruno/PDM/environments/PDM.bru index 9048c2a..9d1c9da 100644 --- a/bruno/PDM/environments/PDM.bru +++ b/bruno/PDM/environments/PDM.bru @@ -1,5 +1,2 @@ -vars:secret [ - PRIVATE_KEY_PATH, - JWT_SECRET, - auth_token -] +vars { +} diff --git a/bruno/PDM/package-lock.json b/bruno/PDM/package-lock.json deleted file mode 100644 index 45ecd83..0000000 --- a/bruno/PDM/package-lock.json +++ /dev/null @@ -1,154 +0,0 @@ -{ - "name": "pdm", - "version": "1.0.0", - "lockfileVersion": 3, - "requires": true, - "packages": { - "": { - "name": "pdm", - "version": "1.0.0", - "license": "ISC", - "dependencies": { - "jsonwebtoken": "^9.0.3" - } - }, - "node_modules/buffer-equal-constant-time": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/buffer-equal-constant-time/-/buffer-equal-constant-time-1.0.1.tgz", - "integrity": "sha512-zRpUiDwd/xk6ADqPMATG8vc9VPrkck7T07OIx0gnjmJAnHnTVXNQG3vfvWNuiZIkwu9KrKdA1iJKfsfTVxE6NA==", - "license": "BSD-3-Clause" - }, - "node_modules/ecdsa-sig-formatter": { - "version": "1.0.11", - "resolved": "https://registry.npmjs.org/ecdsa-sig-formatter/-/ecdsa-sig-formatter-1.0.11.tgz", - "integrity": "sha512-nagl3RYrbNv6kQkeJIpt6NJZy8twLB/2vtz6yN9Z4vRKHN4/QZJIEbqohALSgwKdnksuY3k5Addp5lg8sVoVcQ==", - "license": "Apache-2.0", - "dependencies": { - "safe-buffer": "^5.0.1" - } - }, - "node_modules/jsonwebtoken": { - "version": "9.0.3", - "resolved": "https://registry.npmjs.org/jsonwebtoken/-/jsonwebtoken-9.0.3.tgz", - "integrity": "sha512-MT/xP0CrubFRNLNKvxJ2BYfy53Zkm++5bX9dtuPbqAeQpTVe0MQTFhao8+Cp//EmJp244xt6Drw/GVEGCUj40g==", - "license": "MIT", - "dependencies": { - "jws": "^4.0.1", - "lodash.includes": "^4.3.0", - "lodash.isboolean": "^3.0.3", - "lodash.isinteger": "^4.0.4", - "lodash.isnumber": "^3.0.3", - "lodash.isplainobject": "^4.0.6", - "lodash.isstring": "^4.0.1", - "lodash.once": "^4.0.0", - "ms": "^2.1.1", - "semver": "^7.5.4" - }, - "engines": { - "node": ">=12", - "npm": ">=6" - } - }, - "node_modules/jwa": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/jwa/-/jwa-2.0.1.tgz", - "integrity": "sha512-hRF04fqJIP8Abbkq5NKGN0Bbr3JxlQ+qhZufXVr0DvujKy93ZCbXZMHDL4EOtodSbCWxOqR8MS1tXA5hwqCXDg==", - "license": "MIT", - "dependencies": { - "buffer-equal-constant-time": "^1.0.1", - "ecdsa-sig-formatter": "1.0.11", - "safe-buffer": "^5.0.1" - } - }, - "node_modules/jws": { - "version": "4.0.1", - "resolved": "https://registry.npmjs.org/jws/-/jws-4.0.1.tgz", - "integrity": "sha512-EKI/M/yqPncGUUh44xz0PxSidXFr/+r0pA70+gIYhjv+et7yxM+s29Y+VGDkovRofQem0fs7Uvf4+YmAdyRduA==", - "license": "MIT", - "dependencies": { - "jwa": "^2.0.1", - "safe-buffer": "^5.0.1" - } - }, - "node_modules/lodash.includes": { - "version": "4.3.0", - "resolved": "https://registry.npmjs.org/lodash.includes/-/lodash.includes-4.3.0.tgz", - "integrity": "sha512-W3Bx6mdkRTGtlJISOvVD/lbqjTlPPUDTMnlXZFnVwi9NKJ6tiAk6LVdlhZMm17VZisqhKcgzpO5Wz91PCt5b0w==", - "license": "MIT" - }, - "node_modules/lodash.isboolean": { - "version": "3.0.3", - "resolved": "https://registry.npmjs.org/lodash.isboolean/-/lodash.isboolean-3.0.3.tgz", - "integrity": "sha512-Bz5mupy2SVbPHURB98VAcw+aHh4vRV5IPNhILUCsOzRmsTmSQ17jIuqopAentWoehktxGd9e/hbIXq980/1QJg==", - "license": "MIT" - }, - "node_modules/lodash.isinteger": { - "version": "4.0.4", - "resolved": "https://registry.npmjs.org/lodash.isinteger/-/lodash.isinteger-4.0.4.tgz", - "integrity": "sha512-DBwtEWN2caHQ9/imiNeEA5ys1JoRtRfY3d7V9wkqtbycnAmTvRRmbHKDV4a0EYc678/dia0jrte4tjYwVBaZUA==", - "license": "MIT" - }, - "node_modules/lodash.isnumber": { - "version": "3.0.3", - "resolved": "https://registry.npmjs.org/lodash.isnumber/-/lodash.isnumber-3.0.3.tgz", - "integrity": "sha512-QYqzpfwO3/CWf3XP+Z+tkQsfaLL/EnUlXWVkIk5FUPc4sBdTehEqZONuyRt2P67PXAk+NXmTBcc97zw9t1FQrw==", - "license": "MIT" - }, - "node_modules/lodash.isplainobject": { - "version": "4.0.6", - "resolved": "https://registry.npmjs.org/lodash.isplainobject/-/lodash.isplainobject-4.0.6.tgz", - "integrity": "sha512-oSXzaWypCMHkPC3NvBEaPHf0KsA5mvPrOPgQWDsbg8n7orZ290M0BmC/jgRZ4vcJ6DTAhjrsSYgdsW/F+MFOBA==", - "license": "MIT" - }, - "node_modules/lodash.isstring": { - "version": "4.0.1", - "resolved": "https://registry.npmjs.org/lodash.isstring/-/lodash.isstring-4.0.1.tgz", - "integrity": "sha512-0wJxfxH1wgO3GrbuP+dTTk7op+6L41QCXbGINEmD+ny/G/eCqGzxyCsh7159S+mgDDcoarnBw6PC1PS5+wUGgw==", - "license": "MIT" - }, - "node_modules/lodash.once": { - "version": "4.1.1", - "resolved": "https://registry.npmjs.org/lodash.once/-/lodash.once-4.1.1.tgz", - "integrity": "sha512-Sb487aTOCr9drQVL8pIxOzVhafOjZN9UU54hiN8PU3uAiSV7lx1yYNpbNmex2PK6dSJoNTSJUUswT651yww3Mg==", - "license": "MIT" - }, - "node_modules/ms": { - "version": "2.1.3", - "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz", - "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==", - "license": "MIT" - }, - "node_modules/safe-buffer": { - "version": "5.2.1", - "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.2.1.tgz", - "integrity": "sha512-rp3So07KcdmmKbGvgaNxQSJr7bGVSVk5S9Eq1F+ppbRo70+YeaDxkw5Dd8NPN+GD6bjnYm2VuPuCXmpuYvmCXQ==", - "funding": [ - { - "type": "github", - "url": "https://github.com/sponsors/feross" - }, - { - "type": "patreon", - "url": "https://www.patreon.com/feross" - }, - { - "type": "consulting", - "url": "https://feross.org/support" - } - ], - "license": "MIT" - }, - "node_modules/semver": { - "version": "7.7.3", - "resolved": "https://registry.npmjs.org/semver/-/semver-7.7.3.tgz", - "integrity": "sha512-SdsKMrI9TdgjdweUSR9MweHA4EJ8YxHn8DFaDisvhVlUOe4BF1tLD7GAj0lIqWVl+dPb/rExr0Btby5loQm20Q==", - "license": "ISC", - "bin": { - "semver": "bin/semver.js" - }, - "engines": { - "node": ">=10" - } - } - } -} diff --git a/bruno/PDM/package.json b/bruno/PDM/package.json deleted file mode 100644 index 6f38dac..0000000 --- a/bruno/PDM/package.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "name": "pdm", - "version": "1.0.0", - "description": "", - "main": "index.js", - "scripts": { - }, - "keywords": [], - "author": "", - "license": "ISC", - "type": "commonjs", - "dependencies": { - "jsonwebtoken": "^9.0.3" - } -} diff --git a/bruno/common/auth-token.js b/bruno/common/auth-token.js deleted file mode 100644 index ebec877..0000000 --- a/bruno/common/auth-token.js +++ /dev/null @@ -1,47 +0,0 @@ -/** - * Generate JWT client assertion for OAuth2 authentication - * - * @param {object} bru - Bruno runtime object - * @param {object} req - Request object - * @param {string} audienceUrl - The audience URL for the token - * @param {string} kid - The key ID for the JWT header - */ -function generateAuthToken(bru, req, audienceUrl, kid) { - const jwt = require("jsonwebtoken"); - const fs = require("fs"); - const crypto = require("crypto"); - - const secret = bru.getEnvVar("JWT_SECRET"); - const privateKeyPath = bru.getEnvVar("PRIVATE_KEY_PATH"); - - if (!secret) { - throw new Error("JWT_SECRET environment variable is missing."); - } - if (!privateKeyPath) { - throw new Error("PRIVATE_KEY_PATH environment variable is missing."); - } - - const privateKey = fs.readFileSync(privateKeyPath); - - const payload = { - sub: secret, - iss: secret, - jti: crypto.randomUUID(), - aud: audienceUrl, - exp: (Date.now() / 1000) + 300 - }; - - const options = { - algorithm: 'RS512', - header: { kid: kid } - }; - - const token = jwt.sign(payload, privateKey, options); - - let new_body = req.getBody(); - new_body.push({ name: "client_assertion", value: token }); - - req.setBody(new_body); -} - -module.exports = { generateAuthToken }; From 431411f42062ffdd3f4906274f783791a239d20f Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Tue, 10 Feb 2026 15:44:18 +0000 Subject: [PATCH 20/20] [CDAPI-95]: Swapped Identifier.value to be mandatory within pydantic model --- .../src/pathology_api/fhir/r4/elements.py | 4 ++-- .../pathology_api/fhir/r4/test_elements.py | 23 ++++++++++++++----- pathology-api/src/pathology_api/handler.py | 4 ---- .../src/pathology_api/test_handler.py | 7 +----- .../tests/integration/test_endpoints.py | 3 ++- 5 files changed, 22 insertions(+), 19 deletions(-) diff --git a/pathology-api/src/pathology_api/fhir/r4/elements.py b/pathology-api/src/pathology_api/fhir/r4/elements.py index a9032ea..7ac3f1d 100644 --- a/pathology-api/src/pathology_api/fhir/r4/elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/elements.py @@ -48,7 +48,7 @@ class Identifier(ABC): _expected_system: ClassVar[str] = "__unknown__" system: str - value: str | None = None + value: str @model_validator(mode="after") def validate_system(self) -> "Identifier": @@ -79,7 +79,7 @@ class PatientIdentifier( ): """A FHIR R4 Patient Identifier utilising the NHS Number system.""" - def __init__(self, value: str | None = None): + def __init__(self, value: str): super().__init__(value=value, system=self._expected_system) @classmethod diff --git a/pathology-api/src/pathology_api/fhir/r4/test_elements.py b/pathology-api/src/pathology_api/fhir/r4/test_elements.py index 7257daa..dc0da35 100644 --- a/pathology-api/src/pathology_api/fhir/r4/test_elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/test_elements.py @@ -1,6 +1,7 @@ import datetime import uuid +import pydantic import pytest from pydantic import BaseModel @@ -78,14 +79,15 @@ def test_create_without_value(self) -> None: assert parsed_uuid.version == 4 -class TestIdentifier: - def test_invalid_system(self) -> None: - class _TestIdentifier(Identifier, expected_system="expected-system"): - pass +class _TestContainer(BaseModel): + identifier: "IdentifierStub" - class _TestContainer(BaseModel): - identifier: _TestIdentifier + class IdentifierStub(Identifier, expected_system="expected-system"): + pass + +class TestIdentifier: + def test_invalid_system(self) -> None: with pytest.raises( ValidationError, match="Identifier system 'invalid-system' does not match expected " @@ -95,6 +97,15 @@ class _TestContainer(BaseModel): {"identifier": {"system": "invalid-system", "value": "some-value"}} ) + def test_without_value(self) -> None: + with pytest.raises( + pydantic.ValidationError, + match="1 validation error for _TestContainer\nidentifier.value\n " + "Field required [type=missing, input_value={'system': 'expected-system'}," + " input_type=dict]*", + ): + _TestContainer.model_validate({"identifier": {"system": "expected-system"}}) + class TestPatientIdentifier: def test_create_from_nhs_number(self) -> None: diff --git a/pathology-api/src/pathology_api/handler.py b/pathology-api/src/pathology_api/handler.py index 5cb7e11..6a4a7d8 100644 --- a/pathology-api/src/pathology_api/handler.py +++ b/pathology-api/src/pathology_api/handler.py @@ -18,10 +18,6 @@ def _validate_composition(bundle: Bundle) -> None: if subject is None: raise ValidationError("Composition does not define a valid subject identifier") - nhs_number = subject.identifier.value - if nhs_number is None: - raise ValidationError("Composition does not define a valid subject identifier") - def _validate_bundle(bundle: Bundle) -> None: if bundle.id is not None: diff --git a/pathology-api/src/pathology_api/test_handler.py b/pathology-api/src/pathology_api/test_handler.py index 7a9d5b0..d649d4a 100644 --- a/pathology-api/src/pathology_api/test_handler.py +++ b/pathology-api/src/pathology_api/test_handler.py @@ -95,12 +95,7 @@ def test_handle_request_raises_error_when_multiple_composition_resources( Composition.create(subject=None), "Composition does not define a valid subject identifier", id="No subject", - ), - pytest.param( - Composition.create(subject=LogicalReference(PatientIdentifier())), - "Composition does not define a valid subject identifier", - id="Subject with no identifier value", - ), + ) ], ) def test_handle_request_raises_error_when_invalid_composition( diff --git a/pathology-api/tests/integration/test_endpoints.py b/pathology-api/tests/integration/test_endpoints.py index f9f715b..8d633e8 100644 --- a/pathology-api/tests/integration/test_endpoints.py +++ b/pathology-api/tests/integration/test_endpoints.py @@ -153,7 +153,8 @@ def test_empty_payload_returns_error(self, client: Client) -> None: } ], }, - "Composition does not define a valid subject identifier", + "('entry', 0, 'resource', 'subject', 'identifier', 'value')" + " - Field required \n", id="composition with subject but identifier has no value", ), pytest.param(