From cabda0e3466aa4a68ba4001f58a9e03824f87f07 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Mon, 30 Mar 2026 15:44:35 +0000 Subject: [PATCH 01/28] [CDAPI-110]: Added validation for retrieving the requesting organisation from a Test Result --- .../src/pathology_api/fhir/r4/elements.py | 99 ++++++++++-- .../src/pathology_api/fhir/r4/resources.py | 83 +++++++++- pathology-api/src/pathology_api/handler.py | 147 +++++++++++++++--- 3 files changed, 294 insertions(+), 35 deletions(-) diff --git a/pathology-api/src/pathology_api/fhir/r4/elements.py b/pathology-api/src/pathology_api/fhir/r4/elements.py index 7ac3f1d9..56abbae8 100644 --- a/pathology-api/src/pathology_api/fhir/r4/elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/elements.py @@ -2,9 +2,15 @@ import uuid from abc import ABC from dataclasses import dataclass -from typing import Annotated, ClassVar +from typing import Annotated, Any, ClassVar -from pydantic import Field, model_validator +from pydantic import ( + BaseModel, + ConfigDict, + Field, + ValidatorFunctionWrapHandler, + model_validator, +) from pathology_api.exception import ValidationError @@ -36,8 +42,7 @@ def with_last_updated(cls, last_updated: datetime.datetime | None = None) -> "Me ) -@dataclass(frozen=True) -class Identifier(ABC): +class Identifier(ABC, BaseModel): """ A FHIR R4 Identifier element. See https://hl7.org/fhir/R4/datatypes.html#Identifier. Attributes: @@ -45,23 +50,59 @@ class Identifier(ABC): value: The value that is unique within the system. """ - _expected_system: ClassVar[str] = "__unknown__" + __system_types: ClassVar[dict[str, type["Identifier"]]] = {} + + _validate_system: ClassVar[bool] = True + expected_system: ClassVar[str] = "__unknown__" - system: str - value: str + system: str = Field(..., frozen=True) + value: str = Field(..., frozen=True) @model_validator(mode="after") def validate_system(self) -> "Identifier": - if self.system != self._expected_system: + if self._validate_system and self.system != self.expected_system: raise ValidationError( f"Identifier system '{self.system}' does not match expected " - f"system '{self._expected_system}'." + f"system '{self.expected_system}'." ) return self @classmethod - def __init_subclass__(cls, expected_system: str) -> None: - cls._expected_system = expected_system + def __init_subclass__( + cls, expected_system: str = "__unknown__", validate_system: bool = True + ) -> None: + cls.expected_system = expected_system + cls._validate_system = validate_system + + cls.__system_types[expected_system] = cls + + @model_validator(mode="wrap") + @classmethod + def validate_with_system( + cls, value: dict[str, Any], handler: ValidatorFunctionWrapHandler + ) -> Any: + """ + Provides a model validator that instantiates the correct Identifier type based + on the supplied system. If either no system is provided, or the system provided + is not supported, the UnknownIdentifier type will be utilised. + """ + + if cls != Identifier or not isinstance(value, dict): + return handler(value) + + system = value.get("system") + if system is None: + return UnknownIdentifier.model_validate(value) + + identifier_cls = cls.__system_types.get(system) + if identifier_cls is None: + return UnknownIdentifier.model_validate(value) + + return identifier_cls.model_validate(value) + + +class UnknownIdentifier(Identifier, validate_system=False): + pass class UUIDIdentifier(Identifier, expected_system="https://tools.ietf.org/html/rfc4122"): @@ -70,7 +111,7 @@ class UUIDIdentifier(Identifier, expected_system="https://tools.ietf.org/html/rf def __init__(self, value: uuid.UUID | None = None): super().__init__( value=str(value or uuid.uuid4()), - system=self._expected_system, + system=self.expected_system, ) @@ -80,7 +121,7 @@ class PatientIdentifier( """A FHIR R4 Patient Identifier utilising the NHS Number system.""" def __init__(self, value: str): - super().__init__(value=value, system=self._expected_system) + super().__init__(value=value, system=self.expected_system) @classmethod def from_nhs_number(cls, nhs_number: str) -> "PatientIdentifier": @@ -88,6 +129,38 @@ def from_nhs_number(cls, nhs_number: str) -> "PatientIdentifier": return cls(value=nhs_number) +class OrganizationIdentifier( + Identifier, expected_system="https://fhir.nhs.uk/Id/ods-organization-code" +): + """A FHIR R4 Organization Identifier utilising the ODS Organization Code system.""" + + def __init__(self, value: str): + super().__init__(value=value, system=self.expected_system) + + @classmethod + def from_ods_code(cls, ods_code: str) -> "OrganizationIdentifier": + """Create an OrganizationIdentifier from an ODS code.""" + return cls(value=ods_code) + + @dataclass(frozen=True) class LogicalReference[T: Identifier]: identifier: T + + +@dataclass(frozen=True) +class LiteralReference: + reference: str + + +class Extension(BaseModel): + model_config = ConfigDict(arbitrary_types_allowed=True) + url: str = Field(..., frozen=True) + + +class StringExtension(Extension): + value: Annotated[str, Field(alias="valueString", frozen=True)] + + +class ReferenceExtension(Extension): + value: Annotated[LiteralReference, Field(alias="valueReference", frozen=True)] diff --git a/pathology-api/src/pathology_api/fhir/r4/resources.py b/pathology-api/src/pathology_api/fhir/r4/resources.py index e8b4fa8e..3acfc162 100644 --- a/pathology-api/src/pathology_api/fhir/r4/resources.py +++ b/pathology-api/src/pathology_api/fhir/r4/resources.py @@ -13,7 +13,16 @@ from pathology_api.exception import ValidationError -from .elements import LogicalReference, Meta, PatientIdentifier, UUIDIdentifier +from .elements import ( + Extension, + LiteralReference, + LogicalReference, + Meta, + OrganizationIdentifier, + PatientIdentifier, + UnknownIdentifier, + UUIDIdentifier, +) class Resource(BaseModel): @@ -28,6 +37,9 @@ class Resource(BaseModel): 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) + extension: Annotated[list[SerializeAsAny[Extension]] | None, Field(frozen=True)] = ( + None + ) def __init_subclass__(cls, resource_type: str, **kwargs: Any) -> None: cls.__resource_types[resource_type] = cls @@ -35,6 +47,25 @@ def __init_subclass__(cls, resource_type: str, **kwargs: Any) -> None: super().__init_subclass__(**kwargs) + def find_extension[T: Extension]( + self, url: str, required_type: type[T] + ) -> T | None: + extensions = [ext for ext in self.extension or [] if ext.url == url] + if not extensions: + return None + + if len(extensions) > 1: + raise ValidationError(f"Multiple extensions provided with same url: {url}") + + extension = extensions[0] + if not isinstance(extension, required_type): + raise ValidationError( + f"Extension with url {url} is not of expected type " + f"{required_type.__name__}" + ) + + return extension + @model_validator(mode="wrap") @classmethod def validate_with_subtype( @@ -120,6 +151,48 @@ def find_resources[T: Resource](self, t: type[T]) -> list[T]: if isinstance(entry.resource, t) ] + def has_resource[T: Resource](self, t: type[T]) -> bool: + """ + Check if the bundle contains at least one resource of a given type in its + entries. + If the bundle has no entries, False is returned. + Args: + t: The resource type to search for. + Returns: + True if at least one resource of the specified type is found, otherwise + False. + """ + return self.find_resources(t) != [] + + def get_resource[T: Resource](self, url: str, t: type[T]) -> T | None: + """ + Get the resource of a given type in the bundle entries with the specified + fullUrl. + If no matching resource is found, or if the matching resource is not of the + expected type, a ValidationError is raised. + Args: + url: The fullUrl of the resource to find. + t: The expected type of the resource. + Returns: + The resource with the specified fullUrl and type. Or None if not resource is + found with the provided url and required type. + """ + resources = [ + entry.resource + for entry in self.entries or [] + if entry.full_url == url and isinstance(entry.resource, t) + ] + + if not resources: + return None + + if len(resources) > 1: + raise ValidationError( + f"Multiple resources provided with same fullUrl: {url}" + ) + + return resources[0] + @classmethod def empty(cls, bundle_type: BundleType) -> "Bundle": """Create an empty Bundle of the specified type.""" @@ -133,6 +206,8 @@ class Patient(Resource, resource_type="Patient"): class ServiceRequest(Resource, resource_type="ServiceRequest"): """A FHIR R4 ServiceRequest resource.""" + requester: LiteralReference | None = Field(None, frozen=True) + class DiagnosticReport(Resource, resource_type="DiagnosticReport"): """A FHIR R4 DiagnosticReport resource.""" @@ -141,6 +216,10 @@ class DiagnosticReport(Resource, resource_type="DiagnosticReport"): class Organization(Resource, resource_type="Organization"): """A FHIR R4 Organization resource.""" + identifier: list[OrganizationIdentifier | UnknownIdentifier] | None = Field( + None, frozen=True + ) + class Practitioner(Resource, resource_type="Practitioner"): """A FHIR R4 Practitioner resource.""" @@ -149,6 +228,8 @@ class Practitioner(Resource, resource_type="Practitioner"): class PractitionerRole(Resource, resource_type="PractitionerRole"): """A FHIR R4 PractitionerRole resource.""" + organization: LiteralReference | None = Field(None, frozen=True) + class Observation(Resource, resource_type="Observation"): """A FHIR R4 Observation resource.""" diff --git a/pathology-api/src/pathology_api/handler.py b/pathology-api/src/pathology_api/handler.py index 96340325..f44d8c05 100644 --- a/pathology-api/src/pathology_api/handler.py +++ b/pathology-api/src/pathology_api/handler.py @@ -1,5 +1,4 @@ import uuid -from collections.abc import Callable import requests from aws_lambda_powertools.utilities import parameters @@ -11,8 +10,19 @@ get_optional_environment_variable, ) from pathology_api.exception import ValidationError -from pathology_api.fhir.r4.elements import Meta -from pathology_api.fhir.r4.resources import Bundle, Composition +from pathology_api.fhir.r4.elements import ( + LiteralReference, + Meta, + OrganizationIdentifier, + ReferenceExtension, +) +from pathology_api.fhir.r4.resources import ( + Bundle, + Composition, + Organization, + PractitionerRole, + ServiceRequest, +) from pathology_api.http import ClientCertificate, SessionManager from pathology_api.logging import get_logger @@ -69,16 +79,6 @@ def _create_client_certificate( ) -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") - - subject = compositions[0].subject - if subject is None: - raise ValidationError("Composition does not define a valid subject identifier") - - def _validate_bundle(bundle: Bundle) -> None: if bundle.id is not None: raise ValidationError("Bundles cannot be defined with an existing ID") @@ -86,19 +86,124 @@ def _validate_bundle(bundle: Bundle) -> None: if bundle.bundle_type != "document": raise ValidationError("Resource must be a bundle of type 'document'") + if not bundle.has_resource(ServiceRequest): + raise ValidationError("Document must include a ServiceRequest resource") -type ValidationFunction = Callable[[Bundle], None] -_validation_functions: list[ValidationFunction] = [ - _validate_composition, - _validate_bundle, -] + if not bundle.has_resource(PractitionerRole): + raise ValidationError("Document must include a PractitionerRole resource") + if not bundle.has_resource(Organization): + raise ValidationError("Document must include an Organization resource") -def handle_request(bundle: Bundle) -> Bundle: - for validate_function in _validation_functions: - validate_function(bundle) +def _fetch_composition(bundle: Bundle) -> Composition: + compositions = bundle.find_resources(Composition) + if len(compositions) != 1: + raise ValidationError("Document must include a single Composition resource") + + return compositions[0] + + +def _fetch_service_request(composition: Composition, bundle: Bundle) -> ServiceRequest: + request_reference = composition.find_extension( + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + required_type=ReferenceExtension, + ) + + if request_reference is None: + raise ValidationError( + "Composition does not define a valid basedOn-order-or-requisition extension" + ) + + service_request = bundle.get_resource( + url=request_reference.value.reference, t=ServiceRequest + ) + if service_request is None: + raise ValidationError( + "ServiceRequest resource not found with provided reference. " + f"Provided reference: {request_reference.value.reference}" + ) + + return service_request + + +def _fetch_requesting_organisation( + requester_reference: LiteralReference, bundle: Bundle +) -> OrganizationIdentifier: + requester = bundle.get_resource( + url=requester_reference.reference, t=PractitionerRole + ) + if requester is None: + raise ValidationError( + "PractitionerRole resource not found with provided reference. Provided " + f"reference: {requester_reference.reference}" + ) + + if requester.organization is None: + raise ValidationError( + f"PractitionerRole ({requester_reference.reference}) does not define a " + "valid Organization reference" + ) + + requesting_organisation = bundle.get_resource( + url=requester.organization.reference, t=Organization + ) + if requesting_organisation is None: + raise ValidationError( + "Organization resource not found with provided reference. " + f"Provided reference: {requester.organization.reference}" + ) + + if not requesting_organisation.identifier: + raise ValidationError( + f"Organisation ({requester.organization.reference}) does not " + "define a valid subject identifier" + ) + + organisation_identifiers = [ + identifier + for identifier in requesting_organisation.identifier + if isinstance(identifier, OrganizationIdentifier) + ] + + if not organisation_identifiers: + raise ValidationError( + f"Organization ({requester.organization.reference}) does not define a " + "supported identifier. " + f"Supported system '{OrganizationIdentifier.expected_system}'" + ) + + if len(organisation_identifiers) > 1: + raise ValidationError( + f"Organization ({requester.organization.reference}) defines multiple " + "identifier values. Identifier values: " + f"{[identifier.value for identifier in organisation_identifiers]}" + ) + + return organisation_identifiers[0] + + +def handle_request(bundle: Bundle) -> Bundle: _logger.debug("Bundle entries: %s", bundle.entries) + _validate_bundle(bundle) + + composition = _fetch_composition(bundle) + _logger.debug("Found composition resource: %s", composition) + + service_request = _fetch_service_request(composition, bundle) + + if service_request.requester is None: + raise ValidationError("ServiceRequest does not define a valid requester") + + requesting_organisation = _fetch_requesting_organisation( + service_request.requester, bundle + ) + _logger.debug("Requesting organization: %s", requesting_organisation) + + subject = composition.subject + if subject is None: + raise ValidationError("Composition does not define a valid subject identifier") + return_bundle = Bundle.create( id=str(uuid.uuid4()), meta=Meta.with_last_updated(), From 2dd04e00bc1976e7472760f343bf29ef5afa8efc Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Mon, 30 Mar 2026 16:19:04 +0000 Subject: [PATCH 02/28] [CDAPI-110]: Updated Identifier classes to support system being provided within constructor --- .../src/pathology_api/fhir/r4/elements.py | 18 +++++++----------- .../src/pathology_api/fhir/r4/test_elements.py | 4 ++-- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/pathology-api/src/pathology_api/fhir/r4/elements.py b/pathology-api/src/pathology_api/fhir/r4/elements.py index 56abbae8..7d9f61eb 100644 --- a/pathology-api/src/pathology_api/fhir/r4/elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/elements.py @@ -108,10 +108,12 @@ class UnknownIdentifier(Identifier, validate_system=False): class UUIDIdentifier(Identifier, expected_system="https://tools.ietf.org/html/rfc4122"): """A UUID identifier utilising the standard RFC 4122 system.""" - def __init__(self, value: uuid.UUID | None = None): - super().__init__( + @classmethod + def create_with_uuid(cls, value: uuid.UUID | None = None) -> "UUIDIdentifier": + """Create a UUIDIdentifier with the provided UUID value.""" + return cls( value=str(value or uuid.uuid4()), - system=self.expected_system, + system=cls.expected_system, ) @@ -120,13 +122,10 @@ class PatientIdentifier( ): """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) + return cls(value=nhs_number, system=cls.expected_system) class OrganizationIdentifier( @@ -134,13 +133,10 @@ class OrganizationIdentifier( ): """A FHIR R4 Organization Identifier utilising the ODS Organization Code system.""" - def __init__(self, value: str): - super().__init__(value=value, system=self.expected_system) - @classmethod def from_ods_code(cls, ods_code: str) -> "OrganizationIdentifier": """Create an OrganizationIdentifier from an ODS code.""" - return cls(value=ods_code) + return cls(value=ods_code, system=cls.expected_system) @dataclass(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 dc0da354..a8b86b61 100644 --- a/pathology-api/src/pathology_api/fhir/r4/test_elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/test_elements.py @@ -65,13 +65,13 @@ def test_with_last_updated_defaults_to_now(self) -> None: class TestUUIDIdentifier: def test_create_with_value(self) -> None: expected_uuid = uuid.UUID("12345678-1234-5678-1234-567812345678") - identifier = UUIDIdentifier(value=expected_uuid) + identifier = UUIDIdentifier.create_with_uuid(expected_uuid) assert identifier.system == "https://tools.ietf.org/html/rfc4122" assert identifier.value == str(expected_uuid) def test_create_without_value(self) -> None: - identifier = UUIDIdentifier() + identifier = UUIDIdentifier.create_with_uuid() assert identifier.system == "https://tools.ietf.org/html/rfc4122" # Validates that value is a valid UUID v4 From bc81ea506fc960bf1c09ed724cdcbb6ed8d67566 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Mon, 30 Mar 2026 17:07:34 +0000 Subject: [PATCH 03/28] [CDAPI-110]: Added model validator for Extension --- .../src/pathology_api/fhir/r4/elements.py | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/pathology-api/src/pathology_api/fhir/r4/elements.py b/pathology-api/src/pathology_api/fhir/r4/elements.py index 7d9f61eb..abe95305 100644 --- a/pathology-api/src/pathology_api/fhir/r4/elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/elements.py @@ -151,8 +151,40 @@ class LiteralReference: class Extension(BaseModel): model_config = ConfigDict(arbitrary_types_allowed=True) + + __extension_types: ClassVar[dict[str, type["Extension"]]] = {} + url: str = Field(..., frozen=True) + def __init_subclass__(cls, type_name: str) -> None: + cls.__extension_types[type_name] = cls + super().__init_subclass__() + + @model_validator(mode="wrap") + @classmethod + def validate_with_type( + cls, value: dict[str, Any], handler: ValidatorFunctionWrapHandler + ) -> Any: + """ + Provides a model validator that instantiates the correct Extension type based on + the valueX field provided. + If an Extension subclass cannot be found, the default handler is utilised + instead. + """ + + # If we're not validating an Extension, or the value is not a dict, + # delegate to the default handler + if cls != Extension or not isinstance(value, dict): + return handler(value) + + for key in value: + if key.startswith("value"): + type_name = key.split("value", 1)[1].lower() + if (extension_cls := cls.__extension_types.get(type_name)) is not None: + return extension_cls.model_validate(value) + + return handler(value) + class StringExtension(Extension): value: Annotated[str, Field(alias="valueString", frozen=True)] From 8cf496dffc7f18459ae2cc963930a2e6d6e8395e Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Tue, 31 Mar 2026 08:54:20 +0000 Subject: [PATCH 04/28] [CDAPI-110]: Added type_name to Extension subclasses --- pathology-api/src/pathology_api/fhir/r4/elements.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pathology-api/src/pathology_api/fhir/r4/elements.py b/pathology-api/src/pathology_api/fhir/r4/elements.py index abe95305..659e08ee 100644 --- a/pathology-api/src/pathology_api/fhir/r4/elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/elements.py @@ -186,9 +186,9 @@ def validate_with_type( return handler(value) -class StringExtension(Extension): +class StringExtension(Extension, type_name="string"): value: Annotated[str, Field(alias="valueString", frozen=True)] -class ReferenceExtension(Extension): +class ReferenceExtension(Extension, type_name="reference"): value: Annotated[LiteralReference, Field(alias="valueReference", frozen=True)] From 48307ed0ef14433f0b13b160cf539c9db631e825 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Tue, 31 Mar 2026 11:01:24 +0000 Subject: [PATCH 05/28] [CDAPI-110]: Minor updates to VSCode settings Excluding generated files and setting default python manager to be system. --- .vscode/settings.json | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 31386651..c2fa5b2f 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -25,6 +25,12 @@ "python.telemetry.enable": false, "debugpy.telemetry.enable": false, "python.experiments.enabled": false, + // Exclude generated files from analysis + "python.analysis.exclude": [ + "**/target/**", + "**/__pycache__/**", + "**/dist/**" + ], // Enabling Ruff formatter for python files. "[python]": { "editor.formatOnSave": true, @@ -64,5 +70,6 @@ "projectKey": "NHSDigital_clinical-data-pathology-api" }, // Disabling automatic port forwarding as the devcontainer should already have access to any required ports. - "remote.autoForwardPorts": false + "remote.autoForwardPorts": false, + "python-envs.defaultEnvManager": "ms-python.python:system" } From 0078ef6f5230c393a4c9db11c09f13f456ad1b7b Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Tue, 31 Mar 2026 11:03:07 +0000 Subject: [PATCH 06/28] [CDAPI-110]: Added allow extra fields to Extension class --- pathology-api/src/pathology_api/fhir/r4/elements.py | 2 +- 1 file changed, 1 insertion(+), 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 659e08ee..328f1b7b 100644 --- a/pathology-api/src/pathology_api/fhir/r4/elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/elements.py @@ -150,7 +150,7 @@ class LiteralReference: class Extension(BaseModel): - model_config = ConfigDict(arbitrary_types_allowed=True) + model_config = ConfigDict(arbitrary_types_allowed=True, extra="allow") __extension_types: ClassVar[dict[str, type["Extension"]]] = {} From 26bdf1550e5bd9d3a6c34fefd4a1dc4d0f60406a Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Tue, 31 Mar 2026 11:13:35 +0000 Subject: [PATCH 07/28] [CDAPI-110]: Improved error message when incorrect extension type is provided --- pathology-api/src/pathology_api/fhir/r4/elements.py | 2 ++ pathology-api/src/pathology_api/fhir/r4/resources.py | 2 +- 2 files changed, 3 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 328f1b7b..8710d286 100644 --- a/pathology-api/src/pathology_api/fhir/r4/elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/elements.py @@ -153,10 +153,12 @@ class Extension(BaseModel): model_config = ConfigDict(arbitrary_types_allowed=True, extra="allow") __extension_types: ClassVar[dict[str, type["Extension"]]] = {} + type_name: ClassVar[str] = "__unknown__" url: str = Field(..., frozen=True) def __init_subclass__(cls, type_name: str) -> None: + cls.type_name = type_name cls.__extension_types[type_name] = cls super().__init_subclass__() diff --git a/pathology-api/src/pathology_api/fhir/r4/resources.py b/pathology-api/src/pathology_api/fhir/r4/resources.py index 3acfc162..e79d1376 100644 --- a/pathology-api/src/pathology_api/fhir/r4/resources.py +++ b/pathology-api/src/pathology_api/fhir/r4/resources.py @@ -61,7 +61,7 @@ def find_extension[T: Extension]( if not isinstance(extension, required_type): raise ValidationError( f"Extension with url {url} is not of expected type " - f"{required_type.__name__}" + f"{required_type.type_name}" ) return extension From ee8d2732d4d1c4cba51cb783349d0ba5b23b6eab Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Tue, 31 Mar 2026 12:29:53 +0000 Subject: [PATCH 08/28] [CDAPI-110]: Removed unnecessary StringExtension class --- pathology-api/src/pathology_api/fhir/r4/elements.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pathology-api/src/pathology_api/fhir/r4/elements.py b/pathology-api/src/pathology_api/fhir/r4/elements.py index 8710d286..a6bdafc1 100644 --- a/pathology-api/src/pathology_api/fhir/r4/elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/elements.py @@ -188,9 +188,5 @@ def validate_with_type( return handler(value) -class StringExtension(Extension, type_name="string"): - value: Annotated[str, Field(alias="valueString", frozen=True)] - - class ReferenceExtension(Extension, type_name="reference"): value: Annotated[LiteralReference, Field(alias="valueReference", frozen=True)] From 5471465f6abcaa34c2988110781180e6fcc1e965 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Tue, 31 Mar 2026 12:36:58 +0000 Subject: [PATCH 09/28] [CDAPI-110]: Swapped Organization to reference Identifier class instead of subclasses --- pathology-api/src/pathology_api/fhir/r4/resources.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/pathology-api/src/pathology_api/fhir/r4/resources.py b/pathology-api/src/pathology_api/fhir/r4/resources.py index e79d1376..a95ca1f6 100644 --- a/pathology-api/src/pathology_api/fhir/r4/resources.py +++ b/pathology-api/src/pathology_api/fhir/r4/resources.py @@ -15,12 +15,11 @@ from .elements import ( Extension, + Identifier, LiteralReference, LogicalReference, Meta, - OrganizationIdentifier, PatientIdentifier, - UnknownIdentifier, UUIDIdentifier, ) @@ -216,9 +215,7 @@ class DiagnosticReport(Resource, resource_type="DiagnosticReport"): class Organization(Resource, resource_type="Organization"): """A FHIR R4 Organization resource.""" - identifier: list[OrganizationIdentifier | UnknownIdentifier] | None = Field( - None, frozen=True - ) + identifier: SerializeAsAny[list[Identifier]] | None = Field(None, frozen=True) class Practitioner(Resource, resource_type="Practitioner"): From 846db5890b93ef948658fb194146625c5e9748ce Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Tue, 31 Mar 2026 12:47:32 +0000 Subject: [PATCH 10/28] [CDAPI-110]: Bruno collection update to account for new validation --- bruno/APIM/Post_Document_Bundle_via_APIM.bru | 38 ++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/bruno/APIM/Post_Document_Bundle_via_APIM.bru b/bruno/APIM/Post_Document_Bundle_via_APIM.bru index 9f51f6cf..f01750fd 100644 --- a/bruno/APIM/Post_Document_Bundle_via_APIM.bru +++ b/bruno/APIM/Post_Document_Bundle_via_APIM.bru @@ -23,6 +23,14 @@ body:json { "fullUrl": "composition", "resource": { "resourceType": "Composition", + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest" + } + } + ], "subject": { "identifier": { "system": "https://fhir.nhs.uk/Id/nhs-number", @@ -30,6 +38,36 @@ body:json { } } } + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": { + "reference": "practitionerrole" + } + } + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": { + "reference": "organization" + } + } + }, + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "testOrg" + } + ] + } } ] } From e3db3ba6483f6ea5ddbc1ed82ef9ed00b7eb26f6 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Tue, 31 Mar 2026 12:58:26 +0000 Subject: [PATCH 11/28] [CDAPI-110]: Further minor tweak to vscode python analysis settings --- .vscode/settings.json | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index c2fa5b2f..f9a37b07 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -27,9 +27,11 @@ "python.experiments.enabled": false, // Exclude generated files from analysis "python.analysis.exclude": [ - "**/target/**", - "**/__pycache__/**", - "**/dist/**" + "**/target", + "**/__pycache__", + "**/dist", + "**/node_modules", + "**/.*" ], // Enabling Ruff formatter for python files. "[python]": { From ad234a9a5f1f670db061bbc1d25f0ad888a3722d Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Tue, 31 Mar 2026 16:13:25 +0000 Subject: [PATCH 12/28] [CDAPI-110]: Added additional handle_request unit tests and fixed existing tests --- .../src/pathology_api/fhir/r4/elements.py | 2 +- .../src/pathology_api/test_handler.py | 547 ++++++++++++++++-- 2 files changed, 504 insertions(+), 45 deletions(-) diff --git a/pathology-api/src/pathology_api/fhir/r4/elements.py b/pathology-api/src/pathology_api/fhir/r4/elements.py index a6bdafc1..6bf937c1 100644 --- a/pathology-api/src/pathology_api/fhir/r4/elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/elements.py @@ -189,4 +189,4 @@ def validate_with_type( class ReferenceExtension(Extension, type_name="reference"): - value: Annotated[LiteralReference, Field(alias="valueReference", frozen=True)] + value: LiteralReference = Field(..., alias="valueReference", frozen=True) diff --git a/pathology-api/src/pathology_api/test_handler.py b/pathology-api/src/pathology_api/test_handler.py index 265834fe..4c1fb89c 100644 --- a/pathology-api/src/pathology_api/test_handler.py +++ b/pathology-api/src/pathology_api/test_handler.py @@ -5,6 +5,7 @@ from unittest.mock import MagicMock, Mock, call, patch import pytest +from pydantic import Field from requests.exceptions import RequestException os.environ["CLIENT_TIMEOUT"] = "1s" @@ -17,10 +18,20 @@ from pathology_api.exception import ValidationError from pathology_api.fhir.r4.elements import ( + Extension, + LiteralReference, LogicalReference, + OrganizationIdentifier, PatientIdentifier, + ReferenceExtension, +) +from pathology_api.fhir.r4.resources import ( + Bundle, + Composition, + Organization, + PractitionerRole, + ServiceRequest, ) -from pathology_api.fhir.r4.resources import Bundle, Composition mock_session = Mock() @@ -48,26 +59,271 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: from pathology_api.handler import _create_client_certificate, handle_request +def _missing_resource_scenarios() -> list[Any]: + organisation_entry = Bundle.Entry( + fullUrl="organisation", + resource=Organization.create( + identifier=[OrganizationIdentifier.from_ods_code("ods_code")] + ), + ) + + practitioner_role_entry = Bundle.Entry( + fullUrl="practitioner_role", + resource=PractitionerRole.create( + organization=LiteralReference(reference=organisation_entry.full_url) + ), + ) + + service_request_entry = Bundle.Entry( + fullUrl="service_request", + resource=ServiceRequest.create( + requester=LiteralReference(reference=practitioner_role_entry.full_url) + ), + ) + return [ + pytest.param( + Bundle.create( + type="document", + entry=[ + practitioner_role_entry, + service_request_entry, + organisation_entry, + ], + ), + "Document must include a single Composition resource", + id="Missing composition resource", + ), + pytest.param( + Bundle.create( + type="document", entry=[practitioner_role_entry, service_request_entry] + ), + "Document must include an Organization resource", + id="Missing organization resource", + ), + pytest.param( + Bundle.create( + type="document", entry=[organisation_entry, service_request_entry] + ), + "Document must include a PractitionerRole resource", + id="Missing practitioner role resource", + ), + pytest.param( + Bundle.create( + type="document", entry=[practitioner_role_entry, organisation_entry] + ), + "Document must include a ServiceRequest resource", + id="Missing service request resource", + ), + ] + + +def _invalid_composition_scenarios() -> list[Any]: + class _InvalidExtension(Extension, type_name="invalid_extension"): + value: str = Field(..., frozen=True) + + return [ + pytest.param( + lambda service_request_url: Composition.create( + subject=None, + extension=[ + ReferenceExtension( + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + valueReference=LiteralReference(reference=service_request_url), + ) + ], + ), + "Composition does not define a valid subject identifier", + id="Composition with no subject", + ), + pytest.param( + lambda _: Composition.create( + subject=LogicalReference( + PatientIdentifier.from_nhs_number("nhs_number") + ), + extension=None, + ), + "Composition does not define a valid basedOn-order-or-requisition " + "extension", + id="Composition with no extensions", + ), + pytest.param( + lambda service_request_url: Composition.create( + subject=LogicalReference( + PatientIdentifier.from_nhs_number("nhs_number") + ), + extension=[ + _InvalidExtension( + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + value=service_request_url, + ) + ], + ), + "Extension with url " + "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition" + " is not of expected type reference", + id="Composition with invalid extension", + ), + pytest.param( + lambda service_request_url: Composition.create( + subject=LogicalReference( + PatientIdentifier.from_nhs_number("nhs_number") + ), + extension=[ + ReferenceExtension( + url="invalid", + valueReference=LiteralReference(service_request_url), + ) + ], + ), + "Composition does not define a valid basedOn-order-or-requisition" + " extension", + id="Composition with invalid extension URL", + ), + pytest.param( + lambda _: Composition.create( + subject=LogicalReference( + PatientIdentifier.from_nhs_number("nhs_number") + ), + extension=[ + ReferenceExtension( + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + valueReference=LiteralReference("invalid"), + ) + ], + ), + "ServiceRequest resource not found with provided reference. " + "Provided reference: invalid", + id="Composition with invalid service request reference", + ), + ] + + +def _invalid_service_request_scenarios() -> list[Any]: + return [ + pytest.param( + ServiceRequest.create(requester=None), + "ServiceRequest does not define a valid requester", + id="ServiceRequest with no requester", + ), + pytest.param( + ServiceRequest.create(requester=LiteralReference("invalid")), + "PractitionerRole resource not found with provided reference. Provided " + "reference: invalid", + id="ServiceRequest with invalid requester", + ), + ] + + +def _invalid_practitioner_role_scenarios() -> list[Any]: + return [ + pytest.param( + PractitionerRole.create(organization=None), + r"PractitionerRole \(practitioner_role\) does not define a valid" + " Organization reference", + id="PractitionerRole with no organization", + ), + pytest.param( + PractitionerRole.create(organization=LiteralReference("invalid")), + "Organization resource not found with provided reference. " + "Provided reference: invalid", + id="PractitionerRole with invalid organization reference", + ), + pytest.param( + PractitionerRole.create(organization=LiteralReference("service_request")), + "Organization resource not found with provided reference. " + "Provided reference: service_request", + id="PractitionerRole with non-Organization resource reference", + ), + ] + + +def _invalid_organization_scenarios() -> list[Any]: + return [ + pytest.param( + Organization.create(identifier=None), + r"Organisation \(organisation\) does not define a valid subject identifier", + id="Organization with no identifier", + ), + pytest.param( + Organization.create(identifier=[]), + r"Organisation \(organisation\) does not define a valid subject identifier", + id="Organization with empty identifier list", + ), + pytest.param( + Organization.create( + identifier=[PatientIdentifier.from_nhs_number("nhs_number")] + ), + r"Organization \(organisation\) does not define a supported identifier\. " + r"Supported system 'https://fhir\.nhs\.uk/Id/ods-organization-code'", + id="Organization with unsupported identifier system", + ), + pytest.param( + Organization.create( + identifier=[ + OrganizationIdentifier.from_ods_code("ods_code_1"), + OrganizationIdentifier.from_ods_code("ods_code_2"), + ] + ), + r"Organization \(organisation\) defines multiple identifier values\. " + r"Identifier values: \['ods_code_1', 'ods_code_2'\]", + id="Organization with multiple ODS identifiers", + ), + ] + + class TestHandleRequest: - def setup_method(self) -> None: - mock_session.reset() + def _build_valid_test_result(self) -> Bundle: + organisation_entry = Bundle.Entry( + fullUrl="organisation", + resource=Organization.create( + identifier=[OrganizationIdentifier.from_ods_code("ods_code")] + ), + ) - def test_handle_request(self) -> None: - # Arrange - bundle = Bundle.create( + practitioner_role_entry = Bundle.Entry( + fullUrl="practitioner_role", + resource=PractitionerRole.create( + organization=LiteralReference(reference=organisation_entry.full_url) + ), + ) + + service_request_entry = Bundle.Entry( + fullUrl="service_request", + resource=ServiceRequest.create( + requester=LiteralReference(reference=practitioner_role_entry.full_url) + ), + ) + + composition = Composition.create( + subject=LogicalReference(PatientIdentifier.from_nhs_number("nhs_number_1")), + extension=[ + ReferenceExtension( + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + valueReference=LiteralReference(service_request_entry.full_url), + ) + ], + ) + + return Bundle.create( type="document", entry=[ + organisation_entry, + practitioner_role_entry, + service_request_entry, Bundle.Entry( - fullUrl="patient", - resource=Composition.create( - subject=LogicalReference( - PatientIdentifier.from_nhs_number("nhs_number") - ) - ), - ) + fullUrl="composition", + resource=composition, + ), ], ) + def setup_method(self) -> None: + mock_session.reset() + + def test_handle_request(self) -> None: + # Arrange + bundle = self._build_valid_test_result() + before_call = datetime.datetime.now(tz=datetime.timezone.utc) result_bundle = handle_request(bundle) after_call = datetime.datetime.now(tz=datetime.timezone.utc) @@ -106,19 +362,7 @@ def test_handle_request(self) -> None: def test_handle_request_raises_error_when_send_request_fails(self) -> None: # Arrange - bundle = Bundle.create( - type="document", - entry=[ - Bundle.Entry( - fullUrl="patient", - resource=Composition.create( - subject=LogicalReference( - PatientIdentifier.from_nhs_number("nhs_number") - ) - ), - ) - ], - ) + bundle = self._build_valid_test_result() expected_error_message = "Failed to send request" mock_session.post.side_effect = RequestException(expected_error_message) @@ -126,15 +370,15 @@ def test_handle_request_raises_error_when_send_request_fails(self) -> None: with pytest.raises(RequestException, match=expected_error_message): handle_request(bundle) - def test_handle_request_raises_error_when_no_composition_resource(self) -> None: - bundle = Bundle.create( - type="document", - entry=[], - ) - + @pytest.mark.parametrize( + ("bundle", "expected_error_message"), _missing_resource_scenarios() + ) + def test_handle_request_raises_error_when_missing_resource( + self, bundle: Bundle, expected_error_message: str + ) -> None: with pytest.raises( ValidationError, - match="Document must include a single Composition resource", + match=expected_error_message, ): handle_request(bundle) @@ -145,9 +389,33 @@ def test_handle_request_raises_error_when_multiple_composition_resources( subject=LogicalReference(PatientIdentifier.from_nhs_number("nhs_number_1")) ) + organisation_entry = Bundle.Entry( + fullUrl="organisation", + resource=Organization.create( + identifier=[OrganizationIdentifier.from_ods_code("ods_code")] + ), + ) + + practitioner_role_entry = Bundle.Entry( + fullUrl="practitioner_role", + resource=PractitionerRole.create( + organization=LiteralReference(reference=organisation_entry.full_url) + ), + ) + + service_request_entry = Bundle.Entry( + fullUrl="service_request", + resource=ServiceRequest.create( + requester=LiteralReference(reference=practitioner_role_entry.full_url) + ), + ) + bundle = Bundle.create( type="document", entry=[ + organisation_entry, + practitioner_role_entry, + service_request_entry, Bundle.Entry( fullUrl="composition1", resource=composition, @@ -166,25 +434,216 @@ def test_handle_request_raises_error_when_multiple_composition_resources( 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", - ) - ], + ("composition_func", "expected_error_message"), + _invalid_composition_scenarios(), ) def test_handle_request_raises_error_when_invalid_composition( - self, composition: Composition, expected_error_message: str + self, + composition_func: Callable[[str], Composition], + expected_error_message: str, ) -> None: + organisation_entry = Bundle.Entry( + fullUrl="organisation", + resource=Organization.create( + identifier=[OrganizationIdentifier.from_ods_code("ods_code")] + ), + ) + + practitioner_role_entry = Bundle.Entry( + fullUrl="practitioner_role", + resource=PractitionerRole.create( + organization=LiteralReference(reference=organisation_entry.full_url) + ), + ) + + service_request_entry = Bundle.Entry( + fullUrl="service_request", + resource=ServiceRequest.create( + requester=LiteralReference(reference=practitioner_role_entry.full_url) + ), + ) + bundle = Bundle.create( type="document", entry=[ + organisation_entry, + practitioner_role_entry, + service_request_entry, Bundle.Entry( fullUrl="composition", - resource=composition, - ) + resource=composition_func(service_request_entry.full_url), + ), + ], + ) + + with pytest.raises( + ValidationError, + match=expected_error_message, + ): + handle_request(bundle) + + @pytest.mark.parametrize( + ("service_request", "expected_error_message"), + _invalid_service_request_scenarios(), + ) + def test_handle_request_raises_error_when_invalid_service_request( + self, + service_request: ServiceRequest, + expected_error_message: str, + ) -> None: + organisation_entry = Bundle.Entry( + fullUrl="organisation", + resource=Organization.create( + identifier=[OrganizationIdentifier.from_ods_code("ods_code")] + ), + ) + + practitioner_role_entry = Bundle.Entry( + fullUrl="practitioner_role", + resource=PractitionerRole.create( + organization=LiteralReference(reference=organisation_entry.full_url) + ), + ) + + composition_entry = Bundle.Entry( + fullUrl="composition", + resource=Composition.create( + subject=LogicalReference( + PatientIdentifier.from_nhs_number("nhs_number_1") + ), + extension=[ + ReferenceExtension( + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + valueReference=LiteralReference(reference="service_request"), + ) + ], + ), + ) + + bundle = Bundle.create( + type="document", + entry=[ + organisation_entry, + practitioner_role_entry, + composition_entry, + Bundle.Entry( + fullUrl="service_request", + resource=service_request, + ), + ], + ) + + with pytest.raises( + ValidationError, + match=expected_error_message, + ): + handle_request(bundle) + + @pytest.mark.parametrize( + ("practitioner_role", "expected_error_message"), + _invalid_practitioner_role_scenarios(), + ) + def test_handle_request_raises_error_when_invalid_practitioner_role( + self, + practitioner_role: PractitionerRole, + expected_error_message: str, + ) -> None: + organisation_entry = Bundle.Entry( + fullUrl="organisation", + resource=Organization.create( + identifier=[OrganizationIdentifier.from_ods_code("ods_code")] + ), + ) + + service_request_entry = Bundle.Entry( + fullUrl="service_request", + resource=ServiceRequest.create( + requester=LiteralReference(reference="practitioner_role") + ), + ) + + composition_entry = Bundle.Entry( + fullUrl="composition", + resource=Composition.create( + subject=LogicalReference( + PatientIdentifier.from_nhs_number("nhs_number_1") + ), + extension=[ + ReferenceExtension( + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + valueReference=LiteralReference(reference="service_request"), + ) + ], + ), + ) + + bundle = Bundle.create( + type="document", + entry=[ + organisation_entry, + composition_entry, + service_request_entry, + Bundle.Entry( + fullUrl="practitioner_role", + resource=practitioner_role, + ), + ], + ) + + with pytest.raises( + ValidationError, + match=expected_error_message, + ): + handle_request(bundle) + + @pytest.mark.parametrize( + ("organization", "expected_error_message"), + _invalid_organization_scenarios(), + ) + def test_handle_request_raises_error_when_invalid_organization( + self, + organization: Organization, + expected_error_message: str, + ) -> None: + practitioner_role_entry = Bundle.Entry( + fullUrl="practitioner_role", + resource=PractitionerRole.create( + organization=LiteralReference(reference="organisation") + ), + ) + + service_request_entry = Bundle.Entry( + fullUrl="service_request", + resource=ServiceRequest.create( + requester=LiteralReference(reference=practitioner_role_entry.full_url) + ), + ) + + composition_entry = Bundle.Entry( + fullUrl="composition", + resource=Composition.create( + subject=LogicalReference( + PatientIdentifier.from_nhs_number("nhs_number_1") + ), + extension=[ + ReferenceExtension( + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + valueReference=LiteralReference(reference="service_request"), + ) + ], + ), + ) + + bundle = Bundle.create( + type="document", + entry=[ + Bundle.Entry( + fullUrl="organisation", + resource=organization, + ), + practitioner_role_entry, + service_request_entry, + composition_entry, ], ) From 38bad17662cbc3feac263e7aef6053c6f7f1f19c Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Tue, 31 Mar 2026 17:14:17 +0000 Subject: [PATCH 13/28] [CDAPI-110]: Adding additional unit tests for new components --- .../src/pathology_api/fhir/r4/elements.py | 8 +- .../src/pathology_api/fhir/r4/resources.py | 2 +- .../pathology_api/fhir/r4/test_elements.py | 43 +++- .../pathology_api/fhir/r4/test_resources.py | 216 +++++++++++++++++- 4 files changed, 254 insertions(+), 15 deletions(-) diff --git a/pathology-api/src/pathology_api/fhir/r4/elements.py b/pathology-api/src/pathology_api/fhir/r4/elements.py index 6bf937c1..d9d9b525 100644 --- a/pathology-api/src/pathology_api/fhir/r4/elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/elements.py @@ -92,7 +92,9 @@ def validate_with_system( system = value.get("system") if system is None: - return UnknownIdentifier.model_validate(value) + # This condition is unreachable as Pydantic will validate the presence of + # the required 'system' field before this validator is called. + raise ValueError("Identifier provided without a system attribute.") identifier_cls = cls.__system_types.get(system) if identifier_cls is None: @@ -181,12 +183,12 @@ def validate_with_type( for key in value: if key.startswith("value"): - type_name = key.split("value", 1)[1].lower() + type_name = key.split("value", 1)[1] if (extension_cls := cls.__extension_types.get(type_name)) is not None: return extension_cls.model_validate(value) return handler(value) -class ReferenceExtension(Extension, type_name="reference"): +class ReferenceExtension(Extension, type_name="Reference"): value: LiteralReference = Field(..., alias="valueReference", frozen=True) diff --git a/pathology-api/src/pathology_api/fhir/r4/resources.py b/pathology-api/src/pathology_api/fhir/r4/resources.py index a95ca1f6..00b4d3a4 100644 --- a/pathology-api/src/pathology_api/fhir/r4/resources.py +++ b/pathology-api/src/pathology_api/fhir/r4/resources.py @@ -59,7 +59,7 @@ def find_extension[T: Extension]( extension = extensions[0] if not isinstance(extension, required_type): raise ValidationError( - f"Extension with url {url} is not of expected type " + f"Extension with url {url} is not expected type " f"{required_type.type_name}" ) 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 a8b86b61..9c17e6f5 100644 --- a/pathology-api/src/pathology_api/fhir/r4/test_elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/test_elements.py @@ -12,6 +12,7 @@ LogicalReference, Meta, PatientIdentifier, + UnknownIdentifier, UUIDIdentifier, ) @@ -79,13 +80,17 @@ def test_create_without_value(self) -> None: assert parsed_uuid.version == 4 -class _TestContainer(BaseModel): +class _TestIdentifierContainer(BaseModel): identifier: "IdentifierStub" class IdentifierStub(Identifier, expected_system="expected-system"): pass +class _TestIdentifierListContainer(BaseModel): + identifier: list[Identifier] + + class TestIdentifier: def test_invalid_system(self) -> None: with pytest.raises( @@ -93,18 +98,48 @@ def test_invalid_system(self) -> None: match="Identifier system 'invalid-system' does not match expected " "system 'expected-system'.", ): - _TestContainer.model_validate( + _TestIdentifierContainer.model_validate( {"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 " + match="1 validation error for _TestIdentifierContainer" + "\nidentifier.value\n " "Field required [type=missing, input_value={'system': 'expected-system'}," " input_type=dict]*", ): - _TestContainer.model_validate({"identifier": {"system": "expected-system"}}) + _TestIdentifierContainer.model_validate( + {"identifier": {"system": "expected-system"}} + ) + + def test_unknown_system(self) -> None: + + result = _TestIdentifierListContainer.model_validate( + {"identifier": [{"system": "unknown-system", "value": "some-value"}]} + ) + + assert isinstance(result.identifier[0], UnknownIdentifier) + + def test_deserialises_by_system(self) -> None: + result = _TestIdentifierListContainer.model_validate( + { + "identifier": [ + { + "system": "unknown-system", + "value": "some-value", + }, + { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "second-value", + }, + ] + } + ) + + assert isinstance(result.identifier[0], UnknownIdentifier) + assert isinstance(result.identifier[1], PatientIdentifier) class TestPatientIdentifier: 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 13fea55a..bb5e9da1 100644 --- a/pathology-api/src/pathology_api/fhir/r4/test_resources.py +++ b/pathology-api/src/pathology_api/fhir/r4/test_resources.py @@ -7,7 +7,13 @@ from pathology_api.exception import ValidationError -from .elements import LogicalReference, PatientIdentifier +from .elements import ( + Extension, + LiteralReference, + LogicalReference, + PatientIdentifier, + ReferenceExtension, +) from .resources import Bundle, Composition, OperationOutcome, Patient, Resource @@ -114,6 +120,88 @@ def test_deserialise_wrong_resource_type( ): Bundle.model_validate_json(json, strict=True) + expected_extension = ReferenceExtension( + valueReference=LiteralReference(reference="expected_reference"), + url="expected_url", + ) + + @pytest.mark.parametrize( + ("extensions", "expected_result"), + [ + pytest.param( + [ + expected_extension, + ], + expected_extension, + id="Single extension", + ), + pytest.param( + [ + expected_extension, + ReferenceExtension( + valueReference=LiteralReference(reference="second_reference"), + url="second_url", + ), + ], + expected_extension, + id="Multiple extensions", + ), + ], + ) + def test_find_extension( + self, extensions: list[Extension], expected_result: Extension + ) -> None: + bundle = Bundle.create(type="document", extension=extensions, entry=[]) + + assert ( + bundle.find_extension(url="expected_url", required_type=ReferenceExtension) + == expected_result + ) + + assert ( + bundle.find_extension(url="expected_url", required_type=Extension) + == expected_result + ) + + def test_find_extension_no_extensions(self) -> None: + bundle = Bundle.create(type="document", entry=[]) + + assert ( + bundle.find_extension(url="expected_url", required_type=Extension) is None + ) + + def test_find_extension_duplicate_url(self) -> None: + bundle = Bundle.create( + type="document", + extension=[ + self.expected_extension, + self.expected_extension, + ], + entry=[], + ) + + with pytest.raises( + ValidationError, + match="Multiple extensions provided with same url: expected_url", + ): + bundle.find_extension(url="expected_url", required_type=Extension) + + def test_find_extension_wrong_type(self) -> None: + class _ExtensionStub(Extension, type_name="stub"): + pass + + bundle = Bundle.create( + type="document", + extension=[_ExtensionStub(url="expected_url")], + entry=[], + ) + + with pytest.raises( + ValidationError, + match="Extension with url expected_url is not expected type Reference", + ): + bundle.find_extension(url="expected_url", required_type=ReferenceExtension) + class TestBundle: def test_create(self) -> None: @@ -142,7 +230,7 @@ def test_create_without_entries(self) -> None: assert bundle.identifier is None assert bundle.entries is None - expected_resource = Composition.create( + expected_composition = Composition.create( subject=LogicalReference( identifier=PatientIdentifier.from_nhs_number("nhs_number") ) @@ -155,24 +243,24 @@ def test_create_without_entries(self) -> None: [ Bundle.Entry( fullUrl="fullUrl", - resource=expected_resource, + resource=expected_composition, ), Bundle.Entry( fullUrl="fullUrl", - resource=expected_resource, + resource=expected_composition, ), ], - [expected_resource, expected_resource], + [expected_composition, expected_composition], id="Duplicate resources", ), pytest.param( [ Bundle.Entry( fullUrl="fullUrl", - resource=expected_resource, + resource=expected_composition, ), ], - [expected_resource], + [expected_composition], id="Single resource", ), ], @@ -219,6 +307,120 @@ def test_deserialise_without_type(self) -> None: ): Bundle.model_validate_json('{"resourceType": "Bundle"}') + def test_has_resource(self) -> None: + expected_resource = Patient.create( + identifier=PatientIdentifier.from_nhs_number("nhs_number") + ) + + bundle = Bundle.create( + type="document", + entry=[Bundle.Entry(fullUrl="fullUrl", resource=expected_resource)], + ) + + assert bundle.has_resource(Patient) is True + assert bundle.has_resource(Resource) is True + assert bundle.has_resource(Composition) is False + + def test_has_resource_no_resources(self) -> None: + bundle = Bundle.empty("document") + + assert bundle.has_resource(Resource) is False + + expected_patient = Patient.create( + identifier=PatientIdentifier.from_nhs_number("nhs_number") + ) + + @pytest.mark.parametrize( + ("bundle", "expected_resource"), + [ + pytest.param( + Bundle.create( + type="document", + entry=[ + Bundle.Entry( + fullUrl="fullUrl", + resource=expected_patient, + ) + ], + ), + expected_patient, + id="Bundle with single resource", + ), + pytest.param( + Bundle.create( + type="document", + entry=[ + Bundle.Entry( + fullUrl="fullUrl", + resource=expected_patient, + ), + Bundle.Entry( + fullUrl="secondUrl", + resource=Patient.create( + identifier=PatientIdentifier.from_nhs_number( + "second_nhs_number" + ) + ), + ), + ], + ), + expected_patient, + id="Bundle with multiple resources", + ), + ], + ) + def test_get_resource(self, bundle: Bundle, expected_resource: Patient) -> None: + assert bundle.get_resource(url="fullUrl", t=Patient) == expected_resource + assert bundle.get_resource(url="fullUrl", t=Resource) == expected_resource + + def test_get_resource_no_resources(self) -> None: + bundle = Bundle.empty("document") + + assert bundle.get_resource(url="fullUrl", t=Resource) is None + + def test_get_resource_wrong_type(self) -> None: + expected_resource = Patient.create( + identifier=PatientIdentifier.from_nhs_number("nhs_number") + ) + + bundle = Bundle.create( + type="document", + entry=[Bundle.Entry(fullUrl="fullUrl", resource=expected_resource)], + ) + + assert bundle.get_resource(url="fullUrl", t=Composition) is None + + def test_get_resource_wrong_url(self) -> None: + expected_resource = Patient.create( + identifier=PatientIdentifier.from_nhs_number("nhs_number") + ) + + bundle = Bundle.create( + type="document", + entry=[Bundle.Entry(fullUrl="fullUrl", resource=expected_resource)], + ) + + assert bundle.get_resource(url="wrongUrl", t=Patient) is None + + def test_get_resource_multiple_resources_same_url(self) -> None: + expected_resource = Patient.create( + identifier=PatientIdentifier.from_nhs_number("nhs_number") + ) + + bundle = Bundle.create( + type="document", + entry=[ + Bundle.Entry(fullUrl="fullUrl", resource=expected_resource), + Bundle.Entry(fullUrl="fullUrl", resource=expected_resource), + ], + ) + + with pytest.raises( + ValidationError, + match="Multiple resources provided with same fullUrl: fullUrl", + ): + bundle.get_resource(url="fullUrl", t=Patient) + class TestOperationOutcome: def test_create_validation_error(self) -> None: From 4e1faa69aff3d0ce41beb917a2c699f06079dddd Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Wed, 1 Apr 2026 08:58:32 +0000 Subject: [PATCH 14/28] [CDAPI-110]: Adding additional unit tests around element classes --- .../src/pathology_api/fhir/r4/elements.py | 2 +- .../pathology_api/fhir/r4/test_elements.py | 46 +++++++++++++++++++ .../src/pathology_api/test_handler.py | 2 +- 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/pathology-api/src/pathology_api/fhir/r4/elements.py b/pathology-api/src/pathology_api/fhir/r4/elements.py index d9d9b525..969f1097 100644 --- a/pathology-api/src/pathology_api/fhir/r4/elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/elements.py @@ -104,7 +104,7 @@ def validate_with_system( class UnknownIdentifier(Identifier, validate_system=False): - pass + """Provides a fallback Identifier type for an unknown system.""" class UUIDIdentifier(Identifier, expected_system="https://tools.ietf.org/html/rfc4122"): 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 9c17e6f5..e06773b5 100644 --- a/pathology-api/src/pathology_api/fhir/r4/test_elements.py +++ b/pathology-api/src/pathology_api/fhir/r4/test_elements.py @@ -8,10 +8,13 @@ from pathology_api.exception import ValidationError from .elements import ( + Extension, Identifier, LogicalReference, Meta, + OrganizationIdentifier, PatientIdentifier, + ReferenceExtension, UnknownIdentifier, UUIDIdentifier, ) @@ -142,6 +145,16 @@ def test_deserialises_by_system(self) -> None: assert isinstance(result.identifier[1], PatientIdentifier) +class TestUnknownIdentifier: + def test_does_not_validate_system(self) -> None: + result = UnknownIdentifier.model_validate( + {"system": "any-system", "value": "some-value"} + ) + + assert result.system == "any-system" + assert result.value == "some-value" + + class TestPatientIdentifier: def test_create_from_nhs_number(self) -> None: """Test creating a PatientIdentifier from an NHS number.""" @@ -152,6 +165,15 @@ def test_create_from_nhs_number(self) -> None: assert identifier.value == nhs_number +class TestOrganizationIdentifier: + def test_create_from_ods_code(self) -> None: + expected_ods_code = "ods_code" + identifier = OrganizationIdentifier.from_ods_code(expected_ods_code) + + assert identifier.system == "https://fhir.nhs.uk/Id/ods-organization-code" + assert identifier.value == expected_ods_code + + class TestLogicalReference: class _TestContainer(BaseModel): reference: LogicalReference[PatientIdentifier] @@ -200,3 +222,27 @@ def test_deserialization(self) -> None: assert isinstance(created_identifier, PatientIdentifier) assert created_identifier.system == "https://fhir.nhs.uk/Id/nhs-number" assert created_identifier.value == "nhs_number" + + +class TestExtension: + def test_deserialises_on_type(self) -> None: + result = Extension.model_validate( + {"url": "test-extension", "valueReference": {"reference": "test-reference"}} + ) + + assert isinstance(result, ReferenceExtension) + + unknown_type = Extension.model_validate( + {"url": "unknown-extension", "valueString": "test-value"} + ) + + assert isinstance(unknown_type, Extension) + assert not isinstance(unknown_type, ReferenceExtension) + + def test_deserialises_wrong_casing(self) -> None: + result = Extension.model_validate( + {"url": "test-extension", "valuereference": {"reference": "test-reference"}} + ) + + assert isinstance(result, Extension) + assert not isinstance(result, ReferenceExtension) diff --git a/pathology-api/src/pathology_api/test_handler.py b/pathology-api/src/pathology_api/test_handler.py index 4c1fb89c..e093f7b4 100644 --- a/pathology-api/src/pathology_api/test_handler.py +++ b/pathology-api/src/pathology_api/test_handler.py @@ -160,7 +160,7 @@ class _InvalidExtension(Extension, type_name="invalid_extension"): ), "Extension with url " "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition" - " is not of expected type reference", + " is not expected type Reference", id="Composition with invalid extension", ), pytest.param( From a0ee9d73a081d19c249530815daac7248ea31df1 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Wed, 1 Apr 2026 16:20:34 +0000 Subject: [PATCH 15/28] [CDAPI-110]: Fixed contract tests and existing integration tests --- ...ologyAPIConsumer-PathologyAPIProvider.json | 76 +++++++++ .../tests/contract/test_consumer_contract.py | 72 ++++++++- .../tests/integration/test_endpoints.py | 144 ++++++++++++++++-- 3 files changed, 276 insertions(+), 16 deletions(-) diff --git a/pathology-api/tests/contract/pacts/PathologyAPIConsumer-PathologyAPIProvider.json b/pathology-api/tests/contract/pacts/PathologyAPIConsumer-PathologyAPIProvider.json index ab85e626..f56d9d33 100644 --- a/pathology-api/tests/contract/pacts/PathologyAPIConsumer-PathologyAPIProvider.json +++ b/pathology-api/tests/contract/pacts/PathologyAPIConsumer-PathologyAPIProvider.json @@ -13,6 +13,14 @@ { "fullUrl": "composition", "resource": { + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest" + } + } + ], "resourceType": "Composition", "subject": { "identifier": { @@ -21,6 +29,36 @@ } } } + }, + { + "fullUrl": "servicerequest", + "resource": { + "requester": { + "reference": "practitionerrole" + }, + "resourceType": "ServiceRequest" + } + }, + { + "fullUrl": "practitionerrole", + "resource": { + "organization": { + "reference": "organization" + }, + "resourceType": "PractitionerRole" + } + }, + { + "fullUrl": "organization", + "resource": { + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code" + } + ], + "resourceType": "Organization" + } } ], "resourceType": "Bundle", @@ -47,6 +85,14 @@ { "fullUrl": "composition", "resource": { + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest" + } + } + ], "resourceType": "Composition", "subject": { "identifier": { @@ -55,6 +101,36 @@ } } } + }, + { + "fullUrl": "servicerequest", + "resource": { + "requester": { + "reference": "practitionerrole" + }, + "resourceType": "ServiceRequest" + } + }, + { + "fullUrl": "practitionerrole", + "resource": { + "organization": { + "reference": "organization" + }, + "resourceType": "PractitionerRole" + } + }, + { + "fullUrl": "organization", + "resource": { + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code" + } + ], + "resourceType": "Organization" + } } ], "id": null, diff --git a/pathology-api/tests/contract/test_consumer_contract.py b/pathology-api/tests/contract/test_consumer_contract.py index 66b96fab..8e3c3fc7 100644 --- a/pathology-api/tests/contract/test_consumer_contract.py +++ b/pathology-api/tests/contract/test_consumer_contract.py @@ -34,8 +34,42 @@ def test_post_bundle(self) -> None: "value": "nhs_number", }, }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], }, - } + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, ], } @@ -53,8 +87,42 @@ def test_post_bundle(self) -> None: "value": "nhs_number", }, }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], }, - } + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, ], "id": match.uuid(), "meta": { diff --git a/pathology-api/tests/integration/test_endpoints.py b/pathology-api/tests/integration/test_endpoints.py index 99c0a631..680753af 100644 --- a/pathology-api/tests/integration/test_endpoints.py +++ b/pathology-api/tests/integration/test_endpoints.py @@ -4,8 +4,20 @@ from typing import Any, Literal import pytest -from pathology_api.fhir.r4.elements import LogicalReference, PatientIdentifier -from pathology_api.fhir.r4.resources import Bundle, Composition +from pathology_api.fhir.r4.elements import ( + LiteralReference, + LogicalReference, + OrganizationIdentifier, + PatientIdentifier, + ReferenceExtension, +) +from pathology_api.fhir.r4.resources import ( + Bundle, + Composition, + Organization, + PractitionerRole, + ServiceRequest, +) from pydantic import BaseModel, HttpUrl from tests.conftest import Client @@ -13,17 +25,53 @@ class TestBundleEndpoint: def test_bundle_returns_200(self, client: Client) -> None: + organisation_entry = Bundle.Entry( + fullUrl="organization", + resource=Organization.create( + identifier=[ + OrganizationIdentifier.from_ods_code("ods_code"), + ] + ), + ) + + practitioner_role_entry = Bundle.Entry( + fullUrl="practitionerrole", + resource=PractitionerRole.create( + organization=LiteralReference(reference=organisation_entry.full_url) + ), + ) + + service_request_entry = Bundle.Entry( + fullUrl="servicerequest", + resource=ServiceRequest.create( + requester=LiteralReference(reference=practitioner_role_entry.full_url) + ), + ) + + composition_entry = Bundle.Entry( + fullUrl="patient", + resource=Composition.create( + subject=LogicalReference( + PatientIdentifier.from_nhs_number("nhs_number") + ), + extension=[ + ReferenceExtension( + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + valueReference=LiteralReference( + reference=service_request_entry.full_url + ), + ), + ], + ), + ) + bundle = Bundle.create( type="document", entry=[ - Bundle.Entry( - fullUrl="patient", - resource=Composition.create( - subject=LogicalReference( - PatientIdentifier.from_nhs_number("nhs_number") - ) - ), - ) + composition_entry, + organisation_entry, + practitioner_role_entry, + service_request_entry, ], ) @@ -106,12 +154,12 @@ def test_empty_payload_returns_error(self, client: Client) -> None: "type": "document", "entry": [], }, - "Document must include a single Composition resource", + "Document must include a ServiceRequest resource", id="empty entries list", ), pytest.param( {"resourceType": "Bundle", "type": "document"}, - "Document must include a single Composition resource", + "Document must include a ServiceRequest resource", id="missing entries list", ), pytest.param( @@ -119,10 +167,46 @@ def test_empty_payload_returns_error(self, client: Client) -> None: "resourceType": "Bundle", "type": "document", "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, { "fullUrl": "composition", - "resource": {"resourceType": "Composition"}, - } + "resource": { + "resourceType": "Composition", + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, ], }, "Composition does not define a valid subject identifier", @@ -199,6 +283,32 @@ def test_empty_payload_returns_error(self, client: Client) -> None: "resourceType": "Bundle", "type": "document", "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, { "fullUrl": "composition", "resource": { @@ -265,6 +375,8 @@ def test_status_returns_200(self, client: Client) -> None: assert response.status_code == 200 assert response.headers["Content-Type"] == "application/json" + print("Received /_status response:", response.json()) + parsed = StatusResponse.model_validate(response.json()) assert parsed.status == "pass" @@ -275,6 +387,10 @@ class StatusLinks(BaseModel): self: HttpUrl +class HealthCheckOutcome(BaseModel): + status: Literal["pass", "fail"] + + class HealthCheck(BaseModel): status: Literal["pass", "fail"] timeout: Literal["true", "false"] From 128d0db23c9e2f8f05244d809996d9b7303bbc1d Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Wed, 1 Apr 2026 16:23:09 +0000 Subject: [PATCH 16/28] [CDAPI-110]: Added optional stage parameter to test-remote make target --- scripts/tests/test.mk | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/tests/test.mk b/scripts/tests/test.mk index a98fb19e..c53b0cf6 100644 --- a/scripts/tests/test.mk +++ b/scripts/tests/test.mk @@ -143,9 +143,10 @@ test-local: env-local # Run tests against remote lambda, exporting APIGEE_ACCESS_TOKEN only test-remote: env-remote + @echo "Running test stage: $${stage:-all}" @echo "Obtaining APIGEE access token..." @set -a && source .env && set +a && \ APIGEE_ACCESS_TOKEN="$$(./scripts/get_apigee_token.sh)" && \ BASE_URL="$${BASE_URL}-pr-$${PR_NUMBER}" && \ export APIGEE_ACCESS_TOKEN BASE_URL && \ - $(MAKE) test + $(MAKE) test$(if $(stage),-$(stage),) From 77c75d78ca1c808caafbc41042d687fe2989aa53 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Wed, 1 Apr 2026 16:23:31 +0000 Subject: [PATCH 17/28] [CDAPI-110]: Further additions to python analysis exclusions within vscode settings --- .vscode/settings.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.vscode/settings.json b/.vscode/settings.json index f9a37b07..6069540f 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -30,6 +30,11 @@ "**/target", "**/__pycache__", "**/dist", + "**/build", + "**/.venv", + "**/.mypy_cache", + "**/.pytest_cache", + "**/.ruff_cache", "**/node_modules", "**/.*" ], From a949cbd244cdaad0ebc1dbb41e3c0e3dbcba02f8 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Thu, 2 Apr 2026 10:42:06 +0000 Subject: [PATCH 18/28] [CDAPI-110]: Added new integration test scenarios for invalid Organization, PractitionerRole, ServiceRequest, and Composition resources --- .../tests/integration/test_endpoints.py | 936 ++++++++++++++++++ 1 file changed, 936 insertions(+) diff --git a/pathology-api/tests/integration/test_endpoints.py b/pathology-api/tests/integration/test_endpoints.py index 680753af..83a43bf7 100644 --- a/pathology-api/tests/integration/test_endpoints.py +++ b/pathology-api/tests/integration/test_endpoints.py @@ -212,6 +212,283 @@ def test_empty_payload_returns_error(self, client: Client) -> None: "Composition does not define a valid subject identifier", id="composition with no subject", ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + }, + }, + ], + }, + "Composition does not define a valid basedOn-order-or-requisition " + "extension", + id="composition with no extension", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "unknown-resource", + }, + } + ], + }, + }, + ], + }, + "ServiceRequest resource not found with provided reference. " + "Provided reference: unknown-resource", + id="composition with based on extension referencing unknown resource", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "practitionerrole", + }, + } + ], + }, + }, + ], + }, + "ServiceRequest resource not found with provided reference. " + "Provided reference: practitionerrole", + id="composition with based on extension referencing wrong resource", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueString": "servicerequest", + } + ], + }, + }, + ], + }, + "Extension with url " + "http://hl7.eu/fhir/StructureDefinition" + "/composition-basedOn-order-or-requisition " + "is not expected type Reference", + id="composition with based on extension using wrong type", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "wrong-url", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, + ], + }, + "Composition does not define a valid basedOn-order-or-requisition " + "extension", + id="composition with based on extension using wrong url", + ), pytest.param( { "resourceType": "Bundle", @@ -338,6 +615,665 @@ def test_empty_payload_returns_error(self, client: Client) -> None: "Document must include a single Composition resource", id="bundle with multiple compositions", ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, + ], + }, + "Document must include an Organization resource", + id="Missing Organization resource", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, + ], + }, + "Document must include a PractitionerRole resource", + id="Missing PractitionerRole resource", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, + ], + }, + "Document must include a ServiceRequest resource", + id="Missing ServiceRequest resource", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + # No identifier field + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, + ], + }, + "Organisation (organization) does not define a valid subject " + "identifier", + id="organization with no identifier", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code_1", + }, + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code_2", + }, + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, + ], + }, + "Organization (organization) defines multiple identifier values. " + "Identifier values: ['ods_code_1', 'ods_code_2']", + id="organization with multiple identifiers", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://example.com/unknown-system", + "value": "some_value", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, + ], + }, + "Organization (organization) does not define a supported identifier. " + "Supported system 'https://fhir.nhs.uk/Id/ods-organization-code'", + id="organization with unknown identifier system", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + # No organization field + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, + ], + }, + "PractitionerRole (practitionerrole) does not define a valid " + "Organization reference", + id="PractitionerRole without organization field", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": { + "reference": "nonexistent-organization" + }, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, + ], + }, + "Organization resource not found with provided reference. " + "Provided reference: nonexistent-organization", + id="PractitionerRole organization does not reference an " + "Organization resource", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": "invalid", + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, + ], + }, + "('entry', 1, 'resource', 'organization') - Input should be a " + "dictionary or an instance of LiteralReference \n", + id="PractitionerRole organization field is invalid", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + # No requester field + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, + ], + }, + "ServiceRequest does not define a valid requester", + id="ServiceRequest without requester field", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": { + "reference": "nonexistent-practitionerrole" + }, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, + ], + }, + "PractitionerRole resource not found with provided reference. " + "Provided reference: nonexistent-practitionerrole", + id="ServiceRequest requester does not reference a PractitionerRole " + "resource", + ), + pytest.param( + { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": "invalid", + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, + ], + }, + "('entry', 2, 'resource', 'requester') - Input should be a " + "dictionary or an instance of LiteralReference \n", + id="ServiceRequest requester field invalid type", + ), ], ) def test_invalid_payload_returns_error( From f22edb0b9e6cecd9f7bdc58d5dadf9ccacb28973 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Thu, 2 Apr 2026 10:55:37 +0000 Subject: [PATCH 19/28] [CDAPI-110]: Fixed acceptance tests to account for new validation --- .../acceptance/steps/bundle_endpoint_steps.py | 64 ++++++++++++++++--- 1 file changed, 54 insertions(+), 10 deletions(-) diff --git a/pathology-api/tests/acceptance/steps/bundle_endpoint_steps.py b/pathology-api/tests/acceptance/steps/bundle_endpoint_steps.py index 135c634e..928f460a 100644 --- a/pathology-api/tests/acceptance/steps/bundle_endpoint_steps.py +++ b/pathology-api/tests/acceptance/steps/bundle_endpoint_steps.py @@ -1,8 +1,21 @@ """Step definitions for pathology API bundle endpoint feature.""" import requests -from pathology_api.fhir.r4.elements import LogicalReference, PatientIdentifier -from pathology_api.fhir.r4.resources import Bundle, BundleType, Composition +from pathology_api.fhir.r4.elements import ( + LiteralReference, + LogicalReference, + OrganizationIdentifier, + PatientIdentifier, + ReferenceExtension, +) +from pathology_api.fhir.r4.resources import ( + Bundle, + BundleType, + Composition, + Organization, + PractitionerRole, + ServiceRequest, +) from pytest_bdd import given, parsers, then, when from tests.acceptance.conftest import ResponseContext @@ -31,20 +44,51 @@ def step_send_valid_bundle(client: Client, response_context: ResponseContext) -> client: Test client response_context: Context to store the response """ + + organisation_entry = Bundle.Entry( + fullUrl="organization", + resource=Organization.create( + identifier=[OrganizationIdentifier.from_ods_code("ods-code")] + ), + ) + + practitioner_role_entry = Bundle.Entry( + fullUrl="practitionerrole", + resource=PractitionerRole.create( + organization=LiteralReference(organisation_entry.full_url), + ), + ) + + service_request_entry = Bundle.Entry( + fullUrl="servicerequest", + resource=ServiceRequest.create( + requester=LiteralReference(practitioner_role_entry.full_url) + ), + ) + + composition_entry = Bundle.Entry( + fullUrl="composition", + resource=Composition.create( + subject=LogicalReference(PatientIdentifier.from_nhs_number("nhs_number")), + extension=[ + ReferenceExtension( + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + valueReference=LiteralReference(service_request_entry.full_url), + ) + ], + ), + ) + response_context.response = client.send( path="FHIR/R4/Bundle", request_method="POST", data=Bundle.create( type="document", entry=[ - Bundle.Entry( - fullUrl="composition", - resource=Composition.create( - subject=LogicalReference( - PatientIdentifier.from_nhs_number("nhs_number") - ) - ), - ) + composition_entry, + service_request_entry, + practitioner_role_entry, + organisation_entry, ], ).model_dump_json(by_alias=True, exclude_none=True), ) From 7f5a5e17d3d5dcc25c88786498cfd2b4233a7194 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Thu, 2 Apr 2026 12:19:17 +0000 Subject: [PATCH 20/28] [CDAPI-110]: Minor unit test fixes most rebase Also included new top level conftest file for including test fixtures that are available across all tests --- pathology-api/conftest.py | 67 +++++++++++++++ .../src/pathology_api/test_handler.py | 12 ++- pathology-api/test_lambda_handler.py | 26 ++---- .../acceptance/steps/bundle_endpoint_steps.py | 83 +++++-------------- 4 files changed, 105 insertions(+), 83 deletions(-) create mode 100644 pathology-api/conftest.py diff --git a/pathology-api/conftest.py b/pathology-api/conftest.py new file mode 100644 index 00000000..3c2be56b --- /dev/null +++ b/pathology-api/conftest.py @@ -0,0 +1,67 @@ +from collections.abc import Callable + +import pytest +from pathology_api.fhir.r4.elements import ( + LiteralReference, + LogicalReference, + OrganizationIdentifier, + PatientIdentifier, + ReferenceExtension, +) +from pathology_api.fhir.r4.resources import ( + Bundle, + Composition, + Organization, + PractitionerRole, + ServiceRequest, +) + + +@pytest.fixture(scope="session") +def build_valid_test_result() -> Callable[[str, str], Bundle]: + def builder_function(patient: str, ods_code: str) -> Bundle: + organisation_entry = Bundle.Entry( + fullUrl="organisation", + resource=Organization.create( + identifier=[OrganizationIdentifier.from_ods_code(ods_code)] + ), + ) + + practitioner_role_entry = Bundle.Entry( + fullUrl="practitioner_role", + resource=PractitionerRole.create( + organization=LiteralReference(reference=organisation_entry.full_url) + ), + ) + + service_request_entry = Bundle.Entry( + fullUrl="service_request", + resource=ServiceRequest.create( + requester=LiteralReference(reference=practitioner_role_entry.full_url) + ), + ) + + composition = Composition.create( + subject=LogicalReference(PatientIdentifier.from_nhs_number(patient)), + extension=[ + ReferenceExtension( + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + valueReference=LiteralReference(service_request_entry.full_url), + ) + ], + ) + + return Bundle.create( + type="document", + entry=[ + organisation_entry, + practitioner_role_entry, + service_request_entry, + Bundle.Entry( + fullUrl="composition", + resource=composition, + ), + ], + ) + + return builder_function diff --git a/pathology-api/src/pathology_api/test_handler.py b/pathology-api/src/pathology_api/test_handler.py index e093f7b4..ca87b991 100644 --- a/pathology-api/src/pathology_api/test_handler.py +++ b/pathology-api/src/pathology_api/test_handler.py @@ -320,9 +320,11 @@ def _build_valid_test_result(self) -> Bundle: def setup_method(self) -> None: mock_session.reset() - def test_handle_request(self) -> None: + def test_handle_request( + self, build_valid_test_result: Callable[[str, str], Bundle] + ) -> None: # Arrange - bundle = self._build_valid_test_result() + bundle = build_valid_test_result("nhs_number_1", "ods_code") before_call = datetime.datetime.now(tz=datetime.timezone.utc) result_bundle = handle_request(bundle) @@ -360,9 +362,11 @@ def test_handle_request(self) -> None: session_manager=session_manager_mock.return_value, ) - def test_handle_request_raises_error_when_send_request_fails(self) -> None: + def test_handle_request_raises_error_when_send_request_fails( + self, build_valid_test_result: Callable[[str, str], Bundle] + ) -> None: # Arrange - bundle = self._build_valid_test_result() + bundle = build_valid_test_result("nhs_number_1", "ods_code") expected_error_message = "Failed to send request" mock_session.post.side_effect = RequestException(expected_error_message) diff --git a/pathology-api/test_lambda_handler.py b/pathology-api/test_lambda_handler.py index a177cd80..9c9e6611 100644 --- a/pathology-api/test_lambda_handler.py +++ b/pathology-api/test_lambda_handler.py @@ -1,5 +1,6 @@ import logging import os +from collections.abc import Callable from typing import Any from unittest.mock import MagicMock, patch @@ -20,8 +21,8 @@ from lambda_handler import handler from pathology_api.exception import ValidationError -from pathology_api.fhir.r4.elements import LogicalReference, Meta, PatientIdentifier -from pathology_api.fhir.r4.resources import Bundle, Composition, OperationOutcome +from pathology_api.fhir.r4.elements import Meta +from pathology_api.fhir.r4.resources import Bundle, OperationOutcome TEST_CORRELATION_ID = "b876145d-1ebf-4e22-8ff8-275b570c1ec4" @@ -59,20 +60,8 @@ def _parse_returned_issue(self, response: str) -> OperationOutcome.Issue: return returned_issue @pytest.fixture - def bundle(self) -> Bundle: - return Bundle.create( - type="document", - entry=[ - Bundle.Entry( - fullUrl="composition", - resource=Composition.create( - subject=LogicalReference( - PatientIdentifier.from_nhs_number("nhs_number") - ) - ), - ) - ], - ) + def bundle(self, build_valid_test_result: Callable[[str, str], Bundle]) -> Bundle: + return build_valid_test_result("nhs_number", "ods_code") @pytest.fixture def context(self) -> LambdaContext: @@ -223,7 +212,10 @@ def test_empty_correlation_id_header_returns_500( ], ) def test_malicious_correlation_id_values_are_rejected( - self, malicious_value: str, bundle: Bundle, context: LambdaContext + self, + malicious_value: str, + bundle: Bundle, + context: LambdaContext, ) -> None: event = self._create_test_event( body=bundle.model_dump_json(by_alias=True), diff --git a/pathology-api/tests/acceptance/steps/bundle_endpoint_steps.py b/pathology-api/tests/acceptance/steps/bundle_endpoint_steps.py index 928f460a..4bf3ed1c 100644 --- a/pathology-api/tests/acceptance/steps/bundle_endpoint_steps.py +++ b/pathology-api/tests/acceptance/steps/bundle_endpoint_steps.py @@ -1,26 +1,19 @@ """Step definitions for pathology API bundle endpoint feature.""" +from collections.abc import Callable + import requests -from pathology_api.fhir.r4.elements import ( - LiteralReference, - LogicalReference, - OrganizationIdentifier, - PatientIdentifier, - ReferenceExtension, -) from pathology_api.fhir.r4.resources import ( Bundle, BundleType, - Composition, - Organization, - PractitionerRole, - ServiceRequest, ) from pytest_bdd import given, parsers, then, when from tests.acceptance.conftest import ResponseContext from tests.conftest import Client +BUNDLE_ENDPOINT = "FHIR/R4/Bundle" + @given("the API is running") def step_api_is_running(client: Client) -> None: @@ -36,61 +29,26 @@ def step_api_is_running(client: Client) -> None: @when("I send a valid Bundle to the Pathology API") -def step_send_valid_bundle(client: Client, response_context: ResponseContext) -> None: +def step_send_valid_bundle( + client: Client, + response_context: ResponseContext, + build_valid_test_result: Callable[[str, str], Bundle], +) -> None: """ Send a valid Bundle to the API. Args: client: Test client response_context: Context to store the response + build_valid_test_result: Function to build a valid test result """ - organisation_entry = Bundle.Entry( - fullUrl="organization", - resource=Organization.create( - identifier=[OrganizationIdentifier.from_ods_code("ods-code")] - ), - ) - - practitioner_role_entry = Bundle.Entry( - fullUrl="practitionerrole", - resource=PractitionerRole.create( - organization=LiteralReference(organisation_entry.full_url), - ), - ) - - service_request_entry = Bundle.Entry( - fullUrl="servicerequest", - resource=ServiceRequest.create( - requester=LiteralReference(practitioner_role_entry.full_url) - ), - ) - - composition_entry = Bundle.Entry( - fullUrl="composition", - resource=Composition.create( - subject=LogicalReference(PatientIdentifier.from_nhs_number("nhs_number")), - extension=[ - ReferenceExtension( - url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - valueReference=LiteralReference(service_request_entry.full_url), - ) - ], - ), - ) - response_context.response = client.send( - path="FHIR/R4/Bundle", + path=BUNDLE_ENDPOINT, request_method="POST", - data=Bundle.create( - type="document", - entry=[ - composition_entry, - service_request_entry, - practitioner_role_entry, - organisation_entry, - ], - ).model_dump_json(by_alias=True, exclude_none=True), + data=build_valid_test_result("nhs_number_1", "ods_code").model_dump_json( + by_alias=True, exclude_none=True + ), ) @@ -108,7 +66,7 @@ def step_send_invalid_bundle(client: Client, response_context: ResponseContext) ) response_context.response = client.send( - path="FHIR/R4/Bundle", request_method="POST", data=bundle + path=BUNDLE_ENDPOINT, request_method="POST", data=bundle ) @@ -122,7 +80,7 @@ def step_send_bundle_without_composition( ) response_context.response = client.send( - path="FHIR/R4/Bundle", + path=BUNDLE_ENDPOINT, request_method="POST", data=bundle.model_dump_json(by_alias=True, exclude_none=True), ) @@ -140,7 +98,7 @@ def step_send_bundle_wrong_type( ) response_context.response = client.send( - path="FHIR/R4/Bundle", + path=BUNDLE_ENDPOINT, request_method="POST", data=bundle.model_dump_json(by_alias=True, exclude_none=True), ) @@ -155,7 +113,7 @@ def step_check_status_code( """Verify the response status code matches expected value. Args: - context: Behave context containing the response + response_context: Context containing the response expected_status: Expected HTTP status code """ response = _validate_response_set(response_context) @@ -173,7 +131,7 @@ def step_check_response_contains( """Verify the response contains the expected text. Args: - context: Behave context containing the response + response_context: Context containing the response expected_text: Text that should be in the response """ response = _validate_response_set(response_context) @@ -190,7 +148,8 @@ def step_check_response_contains_valid_bundle( """Verify the response contains a valid FHIR Bundle. Args: - context: Behave context containing the response + response_context: Context containing the response + expected_type: Expected Bundle type """ response = _validate_response_set(response_context) From f9c2e386355f4a01adba17cd624f952a630f3235 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Thu, 2 Apr 2026 14:12:08 +0000 Subject: [PATCH 21/28] [CDAPI-110]: Minor sonar fixes --- pathology-api/conftest.py | 3 +- pathology-api/src/pathology_api/handler.py | 3 +- .../tests/integration/test_endpoints.py | 66 ++----------------- 3 files changed, 9 insertions(+), 63 deletions(-) diff --git a/pathology-api/conftest.py b/pathology-api/conftest.py index 3c2be56b..c9021841 100644 --- a/pathology-api/conftest.py +++ b/pathology-api/conftest.py @@ -44,8 +44,9 @@ def builder_function(patient: str, ods_code: str) -> Bundle: composition = Composition.create( subject=LogicalReference(PatientIdentifier.from_nhs_number(patient)), extension=[ + # Using HTTP to match profile required by implementation guide. ReferenceExtension( - url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", # noqa: python:S5332 valueReference=LiteralReference(service_request_entry.full_url), ) ], diff --git a/pathology-api/src/pathology_api/handler.py b/pathology-api/src/pathology_api/handler.py index f44d8c05..60fd3383 100644 --- a/pathology-api/src/pathology_api/handler.py +++ b/pathology-api/src/pathology_api/handler.py @@ -106,7 +106,8 @@ def _fetch_composition(bundle: Bundle) -> Composition: def _fetch_service_request(composition: Composition, bundle: Bundle) -> ServiceRequest: request_reference = composition.find_extension( - url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + # Using HTTP to match profile required by implementation guide. + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", # noqa: python:S5332 required_type=ReferenceExtension, ) diff --git a/pathology-api/tests/integration/test_endpoints.py b/pathology-api/tests/integration/test_endpoints.py index 83a43bf7..be1312d6 100644 --- a/pathology-api/tests/integration/test_endpoints.py +++ b/pathology-api/tests/integration/test_endpoints.py @@ -1,22 +1,12 @@ """Integration tests for the pathology API using pytest.""" import json +from collections.abc import Callable from typing import Any, Literal import pytest -from pathology_api.fhir.r4.elements import ( - LiteralReference, - LogicalReference, - OrganizationIdentifier, - PatientIdentifier, - ReferenceExtension, -) from pathology_api.fhir.r4.resources import ( Bundle, - Composition, - Organization, - PractitionerRole, - ServiceRequest, ) from pydantic import BaseModel, HttpUrl @@ -24,56 +14,10 @@ class TestBundleEndpoint: - def test_bundle_returns_200(self, client: Client) -> None: - organisation_entry = Bundle.Entry( - fullUrl="organization", - resource=Organization.create( - identifier=[ - OrganizationIdentifier.from_ods_code("ods_code"), - ] - ), - ) - - practitioner_role_entry = Bundle.Entry( - fullUrl="practitionerrole", - resource=PractitionerRole.create( - organization=LiteralReference(reference=organisation_entry.full_url) - ), - ) - - service_request_entry = Bundle.Entry( - fullUrl="servicerequest", - resource=ServiceRequest.create( - requester=LiteralReference(reference=practitioner_role_entry.full_url) - ), - ) - - composition_entry = Bundle.Entry( - fullUrl="patient", - resource=Composition.create( - subject=LogicalReference( - PatientIdentifier.from_nhs_number("nhs_number") - ), - extension=[ - ReferenceExtension( - url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - valueReference=LiteralReference( - reference=service_request_entry.full_url - ), - ), - ], - ), - ) - - bundle = Bundle.create( - type="document", - entry=[ - composition_entry, - organisation_entry, - practitioner_role_entry, - service_request_entry, - ], - ) + def test_bundle_returns_200( + self, client: Client, build_valid_test_result: Callable[[str, str], Bundle] + ) -> None: + bundle = build_valid_test_result("nhs_number", "ods_code") response = client.send( data=bundle.model_dump_json(by_alias=True), From 105a7555d4dd0b614ef6ab8fb1a90f25361ba611 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Thu, 2 Apr 2026 14:34:02 +0000 Subject: [PATCH 22/28] [CDAPI-110]: Fixing noqa comments for sonar --- pathology-api/conftest.py | 2 +- pathology-api/src/pathology_api/handler.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pathology-api/conftest.py b/pathology-api/conftest.py index c9021841..87af1542 100644 --- a/pathology-api/conftest.py +++ b/pathology-api/conftest.py @@ -46,7 +46,7 @@ def builder_function(patient: str, ods_code: str) -> Bundle: extension=[ # Using HTTP to match profile required by implementation guide. ReferenceExtension( - url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", # noqa: python:S5332 + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", # noqa: S5332 valueReference=LiteralReference(service_request_entry.full_url), ) ], diff --git a/pathology-api/src/pathology_api/handler.py b/pathology-api/src/pathology_api/handler.py index 60fd3383..60f6e314 100644 --- a/pathology-api/src/pathology_api/handler.py +++ b/pathology-api/src/pathology_api/handler.py @@ -107,7 +107,7 @@ def _fetch_composition(bundle: Bundle) -> Composition: def _fetch_service_request(composition: Composition, bundle: Bundle) -> ServiceRequest: request_reference = composition.find_extension( # Using HTTP to match profile required by implementation guide. - url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", # noqa: python:S5332 + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", # noqa: S5332 required_type=ReferenceExtension, ) From ab6745e3c9a304c216ec955eae23786cc9e7f755 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Thu, 2 Apr 2026 15:03:47 +0000 Subject: [PATCH 23/28] [CDAPI-110]: Split out invalid composition integration scenarios into separate test --- .../tests/integration/test_endpoints.py | 569 +++++++----------- 1 file changed, 202 insertions(+), 367 deletions(-) diff --git a/pathology-api/tests/integration/test_endpoints.py b/pathology-api/tests/integration/test_endpoints.py index be1312d6..a51829c3 100644 --- a/pathology-api/tests/integration/test_endpoints.py +++ b/pathology-api/tests/integration/test_endpoints.py @@ -106,373 +106,6 @@ def test_empty_payload_returns_error(self, client: Client) -> None: "Document must include a ServiceRequest resource", id="missing entries list", ), - pytest.param( - { - "resourceType": "Bundle", - "type": "document", - "entry": [ - { - "fullUrl": "organization", - "resource": { - "resourceType": "Organization", - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", - "value": "ods_code", - } - ], - }, - }, - { - "fullUrl": "practitionerrole", - "resource": { - "resourceType": "PractitionerRole", - "organization": {"reference": "organization"}, - }, - }, - { - "fullUrl": "servicerequest", - "resource": { - "resourceType": "ServiceRequest", - "requester": {"reference": "practitionerrole"}, - }, - }, - { - "fullUrl": "composition", - "resource": { - "resourceType": "Composition", - "extension": [ - { - "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - "valueReference": { - "reference": "servicerequest", - }, - } - ], - }, - }, - ], - }, - "Composition does not define a valid subject identifier", - id="composition with no subject", - ), - pytest.param( - { - "resourceType": "Bundle", - "type": "document", - "entry": [ - { - "fullUrl": "organization", - "resource": { - "resourceType": "Organization", - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", - "value": "ods_code", - } - ], - }, - }, - { - "fullUrl": "practitionerrole", - "resource": { - "resourceType": "PractitionerRole", - "organization": {"reference": "organization"}, - }, - }, - { - "fullUrl": "servicerequest", - "resource": { - "resourceType": "ServiceRequest", - "requester": {"reference": "practitionerrole"}, - }, - }, - { - "fullUrl": "composition", - "resource": { - "resourceType": "Composition", - "subject": { - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number", - } - }, - }, - }, - ], - }, - "Composition does not define a valid basedOn-order-or-requisition " - "extension", - id="composition with no extension", - ), - pytest.param( - { - "resourceType": "Bundle", - "type": "document", - "entry": [ - { - "fullUrl": "organization", - "resource": { - "resourceType": "Organization", - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", - "value": "ods_code", - } - ], - }, - }, - { - "fullUrl": "practitionerrole", - "resource": { - "resourceType": "PractitionerRole", - "organization": {"reference": "organization"}, - }, - }, - { - "fullUrl": "servicerequest", - "resource": { - "resourceType": "ServiceRequest", - "requester": {"reference": "practitionerrole"}, - }, - }, - { - "fullUrl": "composition", - "resource": { - "resourceType": "Composition", - "subject": { - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number", - } - }, - "extension": [ - { - "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - "valueReference": { - "reference": "unknown-resource", - }, - } - ], - }, - }, - ], - }, - "ServiceRequest resource not found with provided reference. " - "Provided reference: unknown-resource", - id="composition with based on extension referencing unknown resource", - ), - pytest.param( - { - "resourceType": "Bundle", - "type": "document", - "entry": [ - { - "fullUrl": "organization", - "resource": { - "resourceType": "Organization", - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", - "value": "ods_code", - } - ], - }, - }, - { - "fullUrl": "practitionerrole", - "resource": { - "resourceType": "PractitionerRole", - "organization": {"reference": "organization"}, - }, - }, - { - "fullUrl": "servicerequest", - "resource": { - "resourceType": "ServiceRequest", - "requester": {"reference": "practitionerrole"}, - }, - }, - { - "fullUrl": "composition", - "resource": { - "resourceType": "Composition", - "subject": { - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number", - } - }, - "extension": [ - { - "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - "valueReference": { - "reference": "practitionerrole", - }, - } - ], - }, - }, - ], - }, - "ServiceRequest resource not found with provided reference. " - "Provided reference: practitionerrole", - id="composition with based on extension referencing wrong resource", - ), - pytest.param( - { - "resourceType": "Bundle", - "type": "document", - "entry": [ - { - "fullUrl": "organization", - "resource": { - "resourceType": "Organization", - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", - "value": "ods_code", - } - ], - }, - }, - { - "fullUrl": "practitionerrole", - "resource": { - "resourceType": "PractitionerRole", - "organization": {"reference": "organization"}, - }, - }, - { - "fullUrl": "servicerequest", - "resource": { - "resourceType": "ServiceRequest", - "requester": {"reference": "practitionerrole"}, - }, - }, - { - "fullUrl": "composition", - "resource": { - "resourceType": "Composition", - "subject": { - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number", - } - }, - "extension": [ - { - "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - "valueString": "servicerequest", - } - ], - }, - }, - ], - }, - "Extension with url " - "http://hl7.eu/fhir/StructureDefinition" - "/composition-basedOn-order-or-requisition " - "is not expected type Reference", - id="composition with based on extension using wrong type", - ), - pytest.param( - { - "resourceType": "Bundle", - "type": "document", - "entry": [ - { - "fullUrl": "organization", - "resource": { - "resourceType": "Organization", - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", - "value": "ods_code", - } - ], - }, - }, - { - "fullUrl": "practitionerrole", - "resource": { - "resourceType": "PractitionerRole", - "organization": {"reference": "organization"}, - }, - }, - { - "fullUrl": "servicerequest", - "resource": { - "resourceType": "ServiceRequest", - "requester": {"reference": "practitionerrole"}, - }, - }, - { - "fullUrl": "composition", - "resource": { - "resourceType": "Composition", - "subject": { - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number", - } - }, - "extension": [ - { - "url": "wrong-url", - "valueReference": { - "reference": "servicerequest", - }, - } - ], - }, - }, - ], - }, - "Composition does not define a valid basedOn-order-or-requisition " - "extension", - id="composition with based on extension using wrong url", - ), - 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" - } - }, - }, - } - ], - }, - "('entry', 0, 'resource', 'subject', 'identifier', 'value')" - " - Field required \n", - id="composition with subject but identifier has no value", - ), pytest.param( { "resourceType": "Bundle", @@ -1240,6 +873,208 @@ def test_invalid_payload_returns_error( ], } + @pytest.mark.parametrize( + ("composition_builder", "expected_diagnostic"), + [ + pytest.param( + lambda service_request_reference: { + "resourceType": "Composition", + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": service_request_reference, + }, + } + ], + }, + "Composition does not define a valid subject identifier", + id="composition with no subject", + ), + pytest.param( + lambda _: { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + }, + "Composition does not define a valid basedOn-order-or-requisition " + "extension", + id="composition with no extension", + ), + pytest.param( + lambda _: { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "unknown-resource", + }, + } + ], + }, + "ServiceRequest resource not found with provided reference. " + "Provided reference: unknown-resource", + id="composition with based on extension referencing unknown resource", + ), + pytest.param( + lambda _: { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "practitionerrole", + }, + } + ], + }, + "ServiceRequest resource not found with provided reference. " + "Provided reference: practitionerrole", + id="composition with based on extension referencing wrong resource", + ), + pytest.param( + lambda service_request_url: { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueString": service_request_url, + } + ], + }, + "Extension with url " + "http://hl7.eu/fhir/StructureDefinition" + "/composition-basedOn-order-or-requisition " + "is not expected type Reference", + id="composition with based on extension using wrong type", + ), + pytest.param( + lambda service_request_url: { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "wrong-url", + "valueReference": { + "reference": service_request_url, + }, + } + ], + }, + "Composition does not define a valid basedOn-order-or-requisition " + "extension", + id="composition with based on extension using wrong url", + ), + pytest.param( + lambda _: { + "resourceType": "Composition", + "subject": {"identifier": {"value": "nhs_number"}}, + }, + "('entry', 3, 'resource', 'subject', 'identifier', 'system') " + "- Field required \n", + id="composition with subject but no system", + ), + pytest.param( + lambda _: { + "resourceType": "Composition", + "subject": { + "identifier": {"system": "https://fhir.nhs.uk/Id/nhs-number"} + }, + }, + "('entry', 3, 'resource', 'subject', 'identifier', 'value')" + " - Field required \n", + id="composition with subject but identifier has no value", + ), + ], + ) + def test_invalid_composition_resource( + self, + composition_builder: Callable[[str], dict[str, Any]], + expected_diagnostic: str, + client: Client, + ) -> None: + bundle = { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "composition", + "resource": composition_builder("servicerequest"), + }, + ], + } + + response = client.send( + data=json.dumps(bundle), 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, + } + ], + } + @pytest.mark.remote_only class TestStatusEndpoint: From 910109ab926dfd51e60fadd263e1bb8fb8a5eb7b Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Thu, 2 Apr 2026 15:17:25 +0000 Subject: [PATCH 24/28] [CDAPI-110]: Moved invalid ServiceRequest scenarios into separate integration test --- .../tests/integration/test_endpoints.py | 278 +++++++----------- 1 file changed, 105 insertions(+), 173 deletions(-) diff --git a/pathology-api/tests/integration/test_endpoints.py b/pathology-api/tests/integration/test_endpoints.py index a51829c3..2b86b530 100644 --- a/pathology-api/tests/integration/test_endpoints.py +++ b/pathology-api/tests/integration/test_endpoints.py @@ -678,179 +678,6 @@ def test_empty_payload_returns_error(self, client: Client) -> None: "dictionary or an instance of LiteralReference \n", id="PractitionerRole organization field is invalid", ), - pytest.param( - { - "resourceType": "Bundle", - "type": "document", - "entry": [ - { - "fullUrl": "organization", - "resource": { - "resourceType": "Organization", - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", - "value": "ods_code", - } - ], - }, - }, - { - "fullUrl": "practitionerrole", - "resource": { - "resourceType": "PractitionerRole", - "organization": {"reference": "organization"}, - }, - }, - { - "fullUrl": "servicerequest", - "resource": { - "resourceType": "ServiceRequest", - # No requester field - }, - }, - { - "fullUrl": "composition", - "resource": { - "resourceType": "Composition", - "subject": { - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number", - } - }, - "extension": [ - { - "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - "valueReference": { - "reference": "servicerequest", - }, - } - ], - }, - }, - ], - }, - "ServiceRequest does not define a valid requester", - id="ServiceRequest without requester field", - ), - pytest.param( - { - "resourceType": "Bundle", - "type": "document", - "entry": [ - { - "fullUrl": "organization", - "resource": { - "resourceType": "Organization", - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", - "value": "ods_code", - } - ], - }, - }, - { - "fullUrl": "practitionerrole", - "resource": { - "resourceType": "PractitionerRole", - "organization": {"reference": "organization"}, - }, - }, - { - "fullUrl": "servicerequest", - "resource": { - "resourceType": "ServiceRequest", - "requester": { - "reference": "nonexistent-practitionerrole" - }, - }, - }, - { - "fullUrl": "composition", - "resource": { - "resourceType": "Composition", - "subject": { - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number", - } - }, - "extension": [ - { - "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - "valueReference": { - "reference": "servicerequest", - }, - } - ], - }, - }, - ], - }, - "PractitionerRole resource not found with provided reference. " - "Provided reference: nonexistent-practitionerrole", - id="ServiceRequest requester does not reference a PractitionerRole " - "resource", - ), - pytest.param( - { - "resourceType": "Bundle", - "type": "document", - "entry": [ - { - "fullUrl": "organization", - "resource": { - "resourceType": "Organization", - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", - "value": "ods_code", - } - ], - }, - }, - { - "fullUrl": "practitionerrole", - "resource": { - "resourceType": "PractitionerRole", - "organization": {"reference": "organization"}, - }, - }, - { - "fullUrl": "servicerequest", - "resource": { - "resourceType": "ServiceRequest", - "requester": "invalid", - }, - }, - { - "fullUrl": "composition", - "resource": { - "resourceType": "Composition", - "subject": { - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number", - } - }, - "extension": [ - { - "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - "valueReference": { - "reference": "servicerequest", - }, - } - ], - }, - }, - ], - }, - "('entry', 2, 'resource', 'requester') - Input should be a " - "dictionary or an instance of LiteralReference \n", - id="ServiceRequest requester field invalid type", - ), ], ) def test_invalid_payload_returns_error( @@ -1075,6 +902,111 @@ def test_invalid_composition_resource( ], } + @pytest.mark.parametrize( + ("service_request", "expected_diagnostic"), + [ + pytest.param( + { + "resourceType": "ServiceRequest", + # No requester field + }, + "ServiceRequest does not define a valid requester", + id="ServiceRequest without requester field", + ), + pytest.param( + { + "resourceType": "ServiceRequest", + "requester": {"reference": "nonexistent-practitionerrole"}, + }, + "PractitionerRole resource not found with provided reference. " + "Provided reference: nonexistent-practitionerrole", + id="ServiceRequest requester does not reference a PractitionerRole " + "resource", + ), + pytest.param( + { + "resourceType": "ServiceRequest", + "requester": "invalid", + }, + "('entry', 2, 'resource', 'requester') - Input should be a " + "dictionary or an instance of LiteralReference \n", + id="ServiceRequest requester field invalid type", + ), + ], + ) + def test_invalid_service_request_resource( + self, + service_request: dict[str, Any], + expected_diagnostic: str, + client: Client, + ) -> None: + bundle = { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": service_request, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, + ], + } + + response = client.send( + data=json.dumps(bundle), 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, + } + ], + } + @pytest.mark.remote_only class TestStatusEndpoint: From 7049535327466292605114a705dfd5a4dfd51ba8 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Thu, 2 Apr 2026 15:23:53 +0000 Subject: [PATCH 25/28] [CDAPI-110]: Separated invalid PractitionerRole resource tests into a dedicated test function --- .../tests/integration/test_endpoints.py | 280 +++++++----------- 1 file changed, 106 insertions(+), 174 deletions(-) diff --git a/pathology-api/tests/integration/test_endpoints.py b/pathology-api/tests/integration/test_endpoints.py index 2b86b530..c73e7667 100644 --- a/pathology-api/tests/integration/test_endpoints.py +++ b/pathology-api/tests/integration/test_endpoints.py @@ -504,180 +504,6 @@ def test_empty_payload_returns_error(self, client: Client) -> None: "Supported system 'https://fhir.nhs.uk/Id/ods-organization-code'", id="organization with unknown identifier system", ), - pytest.param( - { - "resourceType": "Bundle", - "type": "document", - "entry": [ - { - "fullUrl": "organization", - "resource": { - "resourceType": "Organization", - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", - "value": "ods_code", - } - ], - }, - }, - { - "fullUrl": "practitionerrole", - "resource": { - "resourceType": "PractitionerRole", - # No organization field - }, - }, - { - "fullUrl": "servicerequest", - "resource": { - "resourceType": "ServiceRequest", - "requester": {"reference": "practitionerrole"}, - }, - }, - { - "fullUrl": "composition", - "resource": { - "resourceType": "Composition", - "subject": { - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number", - } - }, - "extension": [ - { - "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - "valueReference": { - "reference": "servicerequest", - }, - } - ], - }, - }, - ], - }, - "PractitionerRole (practitionerrole) does not define a valid " - "Organization reference", - id="PractitionerRole without organization field", - ), - pytest.param( - { - "resourceType": "Bundle", - "type": "document", - "entry": [ - { - "fullUrl": "organization", - "resource": { - "resourceType": "Organization", - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", - "value": "ods_code", - } - ], - }, - }, - { - "fullUrl": "practitionerrole", - "resource": { - "resourceType": "PractitionerRole", - "organization": { - "reference": "nonexistent-organization" - }, - }, - }, - { - "fullUrl": "servicerequest", - "resource": { - "resourceType": "ServiceRequest", - "requester": {"reference": "practitionerrole"}, - }, - }, - { - "fullUrl": "composition", - "resource": { - "resourceType": "Composition", - "subject": { - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number", - } - }, - "extension": [ - { - "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - "valueReference": { - "reference": "servicerequest", - }, - } - ], - }, - }, - ], - }, - "Organization resource not found with provided reference. " - "Provided reference: nonexistent-organization", - id="PractitionerRole organization does not reference an " - "Organization resource", - ), - pytest.param( - { - "resourceType": "Bundle", - "type": "document", - "entry": [ - { - "fullUrl": "organization", - "resource": { - "resourceType": "Organization", - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", - "value": "ods_code", - } - ], - }, - }, - { - "fullUrl": "practitionerrole", - "resource": { - "resourceType": "PractitionerRole", - "organization": "invalid", - }, - }, - { - "fullUrl": "servicerequest", - "resource": { - "resourceType": "ServiceRequest", - "requester": {"reference": "practitionerrole"}, - }, - }, - { - "fullUrl": "composition", - "resource": { - "resourceType": "Composition", - "subject": { - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number", - } - }, - "extension": [ - { - "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - "valueReference": { - "reference": "servicerequest", - }, - } - ], - }, - }, - ], - }, - "('entry', 1, 'resource', 'organization') - Input should be a " - "dictionary or an instance of LiteralReference \n", - id="PractitionerRole organization field is invalid", - ), ], ) def test_invalid_payload_returns_error( @@ -1007,6 +833,112 @@ def test_invalid_service_request_resource( ], } + @pytest.mark.parametrize( + ("practitioner_role", "expected_diagnostic"), + [ + pytest.param( + { + "resourceType": "PractitionerRole", + # No organization field + }, + "PractitionerRole (practitionerrole) does not define a valid " + "Organization reference", + id="PractitionerRole without organization field", + ), + pytest.param( + { + "resourceType": "PractitionerRole", + "organization": {"reference": "nonexistent-organization"}, + }, + "Organization resource not found with provided reference. " + "Provided reference: nonexistent-organization", + id="PractitionerRole organization does not reference an " + "Organization resource", + ), + pytest.param( + { + "resourceType": "PractitionerRole", + "organization": "invalid", + }, + "('entry', 1, 'resource', 'organization') - Input should be a " + "dictionary or an instance of LiteralReference \n", + id="PractitionerRole organization field is invalid", + ), + ], + ) + def test_invalid_practitioner_role_resource( + self, + practitioner_role: dict[str, Any], + expected_diagnostic: str, + client: Client, + ) -> None: + bundle = { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code", + } + ], + }, + }, + { + "fullUrl": "practitionerrole", + "resource": practitioner_role, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, + ], + } + + response = client.send( + data=json.dumps(bundle), 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, + } + ], + } + @pytest.mark.remote_only class TestStatusEndpoint: From a39632dadbd12fc28e210fb14366b86408ac6647 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Thu, 2 Apr 2026 15:59:05 +0000 Subject: [PATCH 26/28] [CDAPI-110]: Moved invalid organization resource integration tests into a separate test function --- .../tests/integration/test_endpoints.py | 281 +++++++----------- 1 file changed, 111 insertions(+), 170 deletions(-) diff --git a/pathology-api/tests/integration/test_endpoints.py b/pathology-api/tests/integration/test_endpoints.py index c73e7667..1b793ea6 100644 --- a/pathology-api/tests/integration/test_endpoints.py +++ b/pathology-api/tests/integration/test_endpoints.py @@ -334,176 +334,6 @@ def test_empty_payload_returns_error(self, client: Client) -> None: "Document must include a ServiceRequest resource", id="Missing ServiceRequest resource", ), - pytest.param( - { - "resourceType": "Bundle", - "type": "document", - "entry": [ - { - "fullUrl": "organization", - "resource": { - "resourceType": "Organization", - # No identifier field - }, - }, - { - "fullUrl": "practitionerrole", - "resource": { - "resourceType": "PractitionerRole", - "organization": {"reference": "organization"}, - }, - }, - { - "fullUrl": "servicerequest", - "resource": { - "resourceType": "ServiceRequest", - "requester": {"reference": "practitionerrole"}, - }, - }, - { - "fullUrl": "composition", - "resource": { - "resourceType": "Composition", - "subject": { - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number", - } - }, - "extension": [ - { - "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - "valueReference": { - "reference": "servicerequest", - }, - } - ], - }, - }, - ], - }, - "Organisation (organization) does not define a valid subject " - "identifier", - id="organization with no identifier", - ), - pytest.param( - { - "resourceType": "Bundle", - "type": "document", - "entry": [ - { - "fullUrl": "organization", - "resource": { - "resourceType": "Organization", - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", - "value": "ods_code_1", - }, - { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", - "value": "ods_code_2", - }, - ], - }, - }, - { - "fullUrl": "practitionerrole", - "resource": { - "resourceType": "PractitionerRole", - "organization": {"reference": "organization"}, - }, - }, - { - "fullUrl": "servicerequest", - "resource": { - "resourceType": "ServiceRequest", - "requester": {"reference": "practitionerrole"}, - }, - }, - { - "fullUrl": "composition", - "resource": { - "resourceType": "Composition", - "subject": { - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number", - } - }, - "extension": [ - { - "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - "valueReference": { - "reference": "servicerequest", - }, - } - ], - }, - }, - ], - }, - "Organization (organization) defines multiple identifier values. " - "Identifier values: ['ods_code_1', 'ods_code_2']", - id="organization with multiple identifiers", - ), - pytest.param( - { - "resourceType": "Bundle", - "type": "document", - "entry": [ - { - "fullUrl": "organization", - "resource": { - "resourceType": "Organization", - "identifier": [ - { - "system": "https://example.com/unknown-system", - "value": "some_value", - } - ], - }, - }, - { - "fullUrl": "practitionerrole", - "resource": { - "resourceType": "PractitionerRole", - "organization": {"reference": "organization"}, - }, - }, - { - "fullUrl": "servicerequest", - "resource": { - "resourceType": "ServiceRequest", - "requester": {"reference": "practitionerrole"}, - }, - }, - { - "fullUrl": "composition", - "resource": { - "resourceType": "Composition", - "subject": { - "identifier": { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "nhs_number", - } - }, - "extension": [ - { - "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - "valueReference": { - "reference": "servicerequest", - }, - } - ], - }, - }, - ], - }, - "Organization (organization) does not define a supported identifier. " - "Supported system 'https://fhir.nhs.uk/Id/ods-organization-code'", - id="organization with unknown identifier system", - ), ], ) def test_invalid_payload_returns_error( @@ -939,6 +769,117 @@ def test_invalid_practitioner_role_resource( ], } + @pytest.mark.parametrize( + ("organization", "expected_diagnostic"), + [ + pytest.param( + { + "resourceType": "Organization", + # No identifier field + }, + "Organisation (organization) does not define a valid subject " + "identifier", + id="organization with no identifier", + ), + pytest.param( + { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code_1", + }, + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "ods_code_2", + }, + ], + }, + "Organization (organization) defines multiple identifier values. " + "Identifier values: ['ods_code_1', 'ods_code_2']", + id="organization with multiple identifiers", + ), + pytest.param( + { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://example.com/unknown-system", + "value": "some_value", + } + ], + }, + "Organization (organization) does not define a supported identifier. " + "Supported system 'https://fhir.nhs.uk/Id/ods-organization-code'", + id="organization with unknown identifier system", + ), + ], + ) + def test_invalid_organization_resource( + self, organization: dict[str, Any], expected_diagnostic: str, client: Client + ) -> None: + bundle = { + "resourceType": "Bundle", + "type": "document", + "entry": [ + { + "fullUrl": "organization", + "resource": organization, + }, + { + "fullUrl": "practitionerrole", + "resource": { + "resourceType": "PractitionerRole", + "organization": {"reference": "organization"}, + }, + }, + { + "fullUrl": "servicerequest", + "resource": { + "resourceType": "ServiceRequest", + "requester": {"reference": "practitionerrole"}, + }, + }, + { + "fullUrl": "composition", + "resource": { + "resourceType": "Composition", + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "nhs_number", + } + }, + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": "servicerequest", + }, + } + ], + }, + }, + ], + } + + response = client.send( + data=json.dumps(bundle), 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, + } + ], + } + @pytest.mark.remote_only class TestStatusEndpoint: From 3b7945f20ba33f8791aed1d701f02759809f2478 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Wed, 8 Apr 2026 16:54:04 +0000 Subject: [PATCH 27/28] [CDAPI-110]: Added BundleBuilder test utility class --- pathology-api/conftest.py | 58 +--- .../src/pathology_api/test_handler.py | 301 ++---------------- pathology-api/src/pathology_api/test_utils.py | 101 ++++++ 3 files changed, 135 insertions(+), 325 deletions(-) create mode 100644 pathology-api/src/pathology_api/test_utils.py diff --git a/pathology-api/conftest.py b/pathology-api/conftest.py index 87af1542..f886d044 100644 --- a/pathology-api/conftest.py +++ b/pathology-api/conftest.py @@ -12,57 +12,27 @@ Bundle, Composition, Organization, - PractitionerRole, - ServiceRequest, ) +from pathology_api.test_utils import BundleBuilder @pytest.fixture(scope="session") def build_valid_test_result() -> Callable[[str, str], Bundle]: def builder_function(patient: str, ods_code: str) -> Bundle: - organisation_entry = Bundle.Entry( - fullUrl="organisation", - resource=Organization.create( - identifier=[OrganizationIdentifier.from_ods_code(ods_code)] - ), - ) - - practitioner_role_entry = Bundle.Entry( - fullUrl="practitioner_role", - resource=PractitionerRole.create( - organization=LiteralReference(reference=organisation_entry.full_url) + return BundleBuilder.with_defaults( + composition_func=lambda service_request_url: Composition.create( + subject=LogicalReference(PatientIdentifier.from_nhs_number(patient)), + extension=[ + # Using HTTP to match profile required by implementation guide. + ReferenceExtension( + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", # noqa: S5332 + valueReference=LiteralReference(service_request_url), + ) + ], ), - ) - - service_request_entry = Bundle.Entry( - fullUrl="service_request", - resource=ServiceRequest.create( - requester=LiteralReference(reference=practitioner_role_entry.full_url) + organisation_func=lambda: Organization.create( + identifier=[OrganizationIdentifier.from_ods_code(ods_code)] ), - ) - - composition = Composition.create( - subject=LogicalReference(PatientIdentifier.from_nhs_number(patient)), - extension=[ - # Using HTTP to match profile required by implementation guide. - ReferenceExtension( - url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", # noqa: S5332 - valueReference=LiteralReference(service_request_entry.full_url), - ) - ], - ) - - return Bundle.create( - type="document", - entry=[ - organisation_entry, - practitioner_role_entry, - service_request_entry, - Bundle.Entry( - fullUrl="composition", - resource=composition, - ), - ], - ) + ).build() return builder_function diff --git a/pathology-api/src/pathology_api/test_handler.py b/pathology-api/src/pathology_api/test_handler.py index ca87b991..bd23c3d6 100644 --- a/pathology-api/src/pathology_api/test_handler.py +++ b/pathology-api/src/pathology_api/test_handler.py @@ -32,6 +32,7 @@ PractitionerRole, ServiceRequest, ) +from pathology_api.test_utils import BundleBuilder mock_session = Mock() @@ -60,57 +61,24 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: def _missing_resource_scenarios() -> list[Any]: - organisation_entry = Bundle.Entry( - fullUrl="organisation", - resource=Organization.create( - identifier=[OrganizationIdentifier.from_ods_code("ods_code")] - ), - ) - - practitioner_role_entry = Bundle.Entry( - fullUrl="practitioner_role", - resource=PractitionerRole.create( - organization=LiteralReference(reference=organisation_entry.full_url) - ), - ) - - service_request_entry = Bundle.Entry( - fullUrl="service_request", - resource=ServiceRequest.create( - requester=LiteralReference(reference=practitioner_role_entry.full_url) - ), - ) return [ pytest.param( - Bundle.create( - type="document", - entry=[ - practitioner_role_entry, - service_request_entry, - organisation_entry, - ], - ), + BundleBuilder.with_defaults(composition_func=lambda _: None).build(), "Document must include a single Composition resource", id="Missing composition resource", ), pytest.param( - Bundle.create( - type="document", entry=[practitioner_role_entry, service_request_entry] - ), + BundleBuilder.with_defaults(organisation_func=lambda: None).build(), "Document must include an Organization resource", id="Missing organization resource", ), pytest.param( - Bundle.create( - type="document", entry=[organisation_entry, service_request_entry] - ), + BundleBuilder.with_defaults(practitioner_role_func=lambda _: None).build(), "Document must include a PractitionerRole resource", id="Missing practitioner role resource", ), pytest.param( - Bundle.create( - type="document", entry=[practitioner_role_entry, organisation_entry] - ), + BundleBuilder.with_defaults(service_request_func=lambda _: None).build(), "Document must include a ServiceRequest resource", id="Missing service request resource", ), @@ -272,51 +240,6 @@ def _invalid_organization_scenarios() -> list[Any]: class TestHandleRequest: - def _build_valid_test_result(self) -> Bundle: - organisation_entry = Bundle.Entry( - fullUrl="organisation", - resource=Organization.create( - identifier=[OrganizationIdentifier.from_ods_code("ods_code")] - ), - ) - - practitioner_role_entry = Bundle.Entry( - fullUrl="practitioner_role", - resource=PractitionerRole.create( - organization=LiteralReference(reference=organisation_entry.full_url) - ), - ) - - service_request_entry = Bundle.Entry( - fullUrl="service_request", - resource=ServiceRequest.create( - requester=LiteralReference(reference=practitioner_role_entry.full_url) - ), - ) - - composition = Composition.create( - subject=LogicalReference(PatientIdentifier.from_nhs_number("nhs_number_1")), - extension=[ - ReferenceExtension( - url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - valueReference=LiteralReference(service_request_entry.full_url), - ) - ], - ) - - return Bundle.create( - type="document", - entry=[ - organisation_entry, - practitioner_role_entry, - service_request_entry, - Bundle.Entry( - fullUrl="composition", - resource=composition, - ), - ], - ) - def setup_method(self) -> None: mock_session.reset() @@ -393,42 +316,10 @@ def test_handle_request_raises_error_when_multiple_composition_resources( subject=LogicalReference(PatientIdentifier.from_nhs_number("nhs_number_1")) ) - organisation_entry = Bundle.Entry( - fullUrl="organisation", - resource=Organization.create( - identifier=[OrganizationIdentifier.from_ods_code("ods_code")] - ), - ) - - practitioner_role_entry = Bundle.Entry( - fullUrl="practitioner_role", - resource=PractitionerRole.create( - organization=LiteralReference(reference=organisation_entry.full_url) - ), - ) - - service_request_entry = Bundle.Entry( - fullUrl="service_request", - resource=ServiceRequest.create( - requester=LiteralReference(reference=practitioner_role_entry.full_url) - ), - ) - - bundle = Bundle.create( - type="document", - entry=[ - organisation_entry, - practitioner_role_entry, - service_request_entry, - Bundle.Entry( - fullUrl="composition1", - resource=composition, - ), - Bundle.Entry( - fullUrl="composition2", - resource=composition, - ), - ], + bundle = ( + BundleBuilder.with_defaults(composition_func=lambda _: composition) + .include_resource("composition2", composition) + .build() ) with pytest.raises( @@ -446,39 +337,7 @@ def test_handle_request_raises_error_when_invalid_composition( composition_func: Callable[[str], Composition], expected_error_message: str, ) -> None: - organisation_entry = Bundle.Entry( - fullUrl="organisation", - resource=Organization.create( - identifier=[OrganizationIdentifier.from_ods_code("ods_code")] - ), - ) - - practitioner_role_entry = Bundle.Entry( - fullUrl="practitioner_role", - resource=PractitionerRole.create( - organization=LiteralReference(reference=organisation_entry.full_url) - ), - ) - - service_request_entry = Bundle.Entry( - fullUrl="service_request", - resource=ServiceRequest.create( - requester=LiteralReference(reference=practitioner_role_entry.full_url) - ), - ) - - bundle = Bundle.create( - type="document", - entry=[ - organisation_entry, - practitioner_role_entry, - service_request_entry, - Bundle.Entry( - fullUrl="composition", - resource=composition_func(service_request_entry.full_url), - ), - ], - ) + bundle = BundleBuilder.with_defaults(composition_func=composition_func).build() with pytest.raises( ValidationError, @@ -495,47 +354,9 @@ def test_handle_request_raises_error_when_invalid_service_request( service_request: ServiceRequest, expected_error_message: str, ) -> None: - organisation_entry = Bundle.Entry( - fullUrl="organisation", - resource=Organization.create( - identifier=[OrganizationIdentifier.from_ods_code("ods_code")] - ), - ) - - practitioner_role_entry = Bundle.Entry( - fullUrl="practitioner_role", - resource=PractitionerRole.create( - organization=LiteralReference(reference=organisation_entry.full_url) - ), - ) - - composition_entry = Bundle.Entry( - fullUrl="composition", - resource=Composition.create( - subject=LogicalReference( - PatientIdentifier.from_nhs_number("nhs_number_1") - ), - extension=[ - ReferenceExtension( - url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - valueReference=LiteralReference(reference="service_request"), - ) - ], - ), - ) - - bundle = Bundle.create( - type="document", - entry=[ - organisation_entry, - practitioner_role_entry, - composition_entry, - Bundle.Entry( - fullUrl="service_request", - resource=service_request, - ), - ], - ) + bundle = BundleBuilder.with_defaults( + service_request_func=lambda _: service_request + ).build() with pytest.raises( ValidationError, @@ -552,47 +373,10 @@ def test_handle_request_raises_error_when_invalid_practitioner_role( practitioner_role: PractitionerRole, expected_error_message: str, ) -> None: - organisation_entry = Bundle.Entry( - fullUrl="organisation", - resource=Organization.create( - identifier=[OrganizationIdentifier.from_ods_code("ods_code")] - ), - ) - - service_request_entry = Bundle.Entry( - fullUrl="service_request", - resource=ServiceRequest.create( - requester=LiteralReference(reference="practitioner_role") - ), - ) - composition_entry = Bundle.Entry( - fullUrl="composition", - resource=Composition.create( - subject=LogicalReference( - PatientIdentifier.from_nhs_number("nhs_number_1") - ), - extension=[ - ReferenceExtension( - url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - valueReference=LiteralReference(reference="service_request"), - ) - ], - ), - ) - - bundle = Bundle.create( - type="document", - entry=[ - organisation_entry, - composition_entry, - service_request_entry, - Bundle.Entry( - fullUrl="practitioner_role", - resource=practitioner_role, - ), - ], - ) + bundle = BundleBuilder.with_defaults( + practitioner_role_func=lambda _: practitioner_role + ).build() with pytest.raises( ValidationError, @@ -609,47 +393,9 @@ def test_handle_request_raises_error_when_invalid_organization( organization: Organization, expected_error_message: str, ) -> None: - practitioner_role_entry = Bundle.Entry( - fullUrl="practitioner_role", - resource=PractitionerRole.create( - organization=LiteralReference(reference="organisation") - ), - ) - - service_request_entry = Bundle.Entry( - fullUrl="service_request", - resource=ServiceRequest.create( - requester=LiteralReference(reference=practitioner_role_entry.full_url) - ), - ) - - composition_entry = Bundle.Entry( - fullUrl="composition", - resource=Composition.create( - subject=LogicalReference( - PatientIdentifier.from_nhs_number("nhs_number_1") - ), - extension=[ - ReferenceExtension( - url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", - valueReference=LiteralReference(reference="service_request"), - ) - ], - ), - ) - - bundle = Bundle.create( - type="document", - entry=[ - Bundle.Entry( - fullUrl="organisation", - resource=organization, - ), - practitioner_role_entry, - service_request_entry, - composition_entry, - ], - ) + bundle = BundleBuilder.with_defaults( + organisation_func=lambda: organization + ).build() with pytest.raises( ValidationError, @@ -679,14 +425,7 @@ def test_handle_request_raises_error_when_bundle_includes_id( 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)], - ) + bundle = BundleBuilder.with_defaults().with_type("collection").build() with pytest.raises( ValidationError, diff --git a/pathology-api/src/pathology_api/test_utils.py b/pathology-api/src/pathology_api/test_utils.py new file mode 100644 index 00000000..26ad8602 --- /dev/null +++ b/pathology-api/src/pathology_api/test_utils.py @@ -0,0 +1,101 @@ +from collections.abc import Callable + +from pathology_api.fhir.r4.elements import ( + LiteralReference, + LogicalReference, + OrganizationIdentifier, + PatientIdentifier, + ReferenceExtension, +) +from pathology_api.fhir.r4.resources import ( + Bundle, + BundleType, + Composition, + Organization, + PractitionerRole, + Resource, + ServiceRequest, +) + + +def _build_composition(service_request_url: str) -> Composition: + return Composition.create( + subject=LogicalReference(PatientIdentifier.from_nhs_number("nhs_number")), + extension=[ + # Using HTTP to match profile required by implementation guide. + ReferenceExtension( + url="http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", # noqa: S5332 + valueReference=LiteralReference(service_request_url), + ) + ], + ) + + +def _build_service_request(practitioner_role_url: str) -> ServiceRequest: + return ServiceRequest.create(requester=LiteralReference(practitioner_role_url)) + + +def _build_practitioner_role(organisation_url: str) -> PractitionerRole: + return PractitionerRole.create( + organization=LiteralReference(reference=organisation_url) + ) + + +def _build_organisation() -> Organization: + return Organization.create( + identifier=[OrganizationIdentifier.from_ods_code("ods_code")] + ) + + +class BundleBuilder: + def __init__(self) -> None: + self._entries: list[Bundle.Entry] = [] + self._type: BundleType | None = None + + def include_resource(self, full_url: str, resource: Resource) -> "BundleBuilder": + self._entries.append(Bundle.Entry(fullUrl=full_url, resource=resource)) + return self + + def with_type(self, bundle_type: BundleType) -> "BundleBuilder": + self._type = bundle_type + return self + + def build(self) -> Bundle: + if self._type is None: + raise ValueError("Bundle type must be set before building the Bundle") + return Bundle.create( + type=self._type, + entry=self._entries, + ) + + @staticmethod + def with_defaults( + composition_func: Callable[[str], Composition | None] = _build_composition, + service_request_func: Callable[ + [str], ServiceRequest | None + ] = _build_service_request, + practitioner_role_func: Callable[ + [str], PractitionerRole | None + ] = _build_practitioner_role, + organisation_func: Callable[[], Organization | None] = _build_organisation, + ) -> "BundleBuilder": + organisation_url = "organisation" + practitioner_role_url = "practitioner_role" + service_request_url = "service_request" + composition_url = "composition" + + bundle = BundleBuilder().with_type("document") + + if (organisation := organisation_func()) is not None: + bundle.include_resource(organisation_url, organisation) + + if (practitioner_role := practitioner_role_func(organisation_url)) is not None: + bundle.include_resource(practitioner_role_url, practitioner_role) + + if (service_request := service_request_func(practitioner_role_url)) is not None: + bundle.include_resource(service_request_url, service_request) + + if (composition := composition_func(service_request_url)) is not None: + bundle.include_resource(composition_url, composition) + + return bundle From ea61045cd84b69ce2f7b85b584aaef7af5ca6046 Mon Sep 17 00:00:00 2001 From: Jack Wainwright <79214177+nhsd-jack-wainwright@users.noreply.github.com> Date: Thu, 16 Apr 2026 15:15:25 +0000 Subject: [PATCH 28/28] [CDAPI-150]: Added validation ensuring an empty string cannot be provided as a subject identifier or ODS code --- pathology-api/src/pathology_api/handler.py | 4 +- .../src/pathology_api/test_handler.py | 40 +++++++++++++++++++ .../tests/integration/test_endpoints.py | 36 +++++++++++++++++ 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/pathology-api/src/pathology_api/handler.py b/pathology-api/src/pathology_api/handler.py index 60f6e314..149bf099 100644 --- a/pathology-api/src/pathology_api/handler.py +++ b/pathology-api/src/pathology_api/handler.py @@ -164,7 +164,7 @@ def _fetch_requesting_organisation( organisation_identifiers = [ identifier for identifier in requesting_organisation.identifier - if isinstance(identifier, OrganizationIdentifier) + if isinstance(identifier, OrganizationIdentifier) and identifier.value ] if not organisation_identifiers: @@ -202,7 +202,7 @@ def handle_request(bundle: Bundle) -> Bundle: _logger.debug("Requesting organization: %s", requesting_organisation) subject = composition.subject - if subject is None: + if subject is None or not subject.identifier.value: raise ValidationError("Composition does not define a valid subject identifier") return_bundle = Bundle.create( diff --git a/pathology-api/src/pathology_api/test_handler.py b/pathology-api/src/pathology_api/test_handler.py index bd23c3d6..83499a06 100644 --- a/pathology-api/src/pathology_api/test_handler.py +++ b/pathology-api/src/pathology_api/test_handler.py @@ -285,6 +285,46 @@ def test_handle_request( session_manager=session_manager_mock.return_value, ) + def test_handle_request_with_empty_subject( + self, build_valid_test_result: Callable[[str, str], Bundle] + ) -> None: + bundle = build_valid_test_result("", "ods_code") + + with pytest.raises( + ValidationError, + match="Composition does not define a valid subject identifier", + ): + handle_request(bundle) + + def test_handle_request_with_empty_ods_code( + self, build_valid_test_result: Callable[[str, str], Bundle] + ) -> None: + bundle = build_valid_test_result("nhs_number_1", "") + + if ( + created_practitioner_role := bundle.get_resource( + url="practitioner_role", t=PractitionerRole + ) + ) is None: + raise ValueError( + "Test setup error: PractitionerRole resource not found in bundle" + ) + + if ( + expected_organisation_reference := created_practitioner_role.organization + ) is None: + raise ValueError( + "Test setup error: PractitionerRole resource does not define an " + "organization reference" + ) + + with pytest.raises( + ValidationError, + match=rf"Organization \({expected_organisation_reference.reference}\) does " + "not define a supported identifier. Supported system 'https://fhir.nhs.uk/Id/ods-organization-code'", + ): + handle_request(bundle) + def test_handle_request_raises_error_when_send_request_fails( self, build_valid_test_result: Callable[[str, str], Bundle] ) -> None: diff --git a/pathology-api/tests/integration/test_endpoints.py b/pathology-api/tests/integration/test_endpoints.py index 1b793ea6..3acd1f4d 100644 --- a/pathology-api/tests/integration/test_endpoints.py +++ b/pathology-api/tests/integration/test_endpoints.py @@ -374,6 +374,27 @@ def test_invalid_payload_returns_error( "Composition does not define a valid subject identifier", id="composition with no subject", ), + pytest.param( + lambda service_request_reference: { + "resourceType": "Composition", + "extension": [ + { + "url": "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition", + "valueReference": { + "reference": service_request_reference, + }, + } + ], + "subject": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "", + } + }, + }, + "Composition does not define a valid subject identifier", + id="composition with subject with empty identifier value", + ), pytest.param( lambda _: { "resourceType": "Composition", @@ -813,6 +834,21 @@ def test_invalid_practitioner_role_resource( "Supported system 'https://fhir.nhs.uk/Id/ods-organization-code'", id="organization with unknown identifier system", ), + pytest.param( + { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "", + } + ], + }, + "Organization (organization) does not define a " + "supported identifier. " + r"Supported system 'https://fhir.nhs.uk/Id/ods-organization-code'", + id="organization with identifier with empty value", + ), ], ) def test_invalid_organization_resource(