diff --git a/.github/workflows/preview-env.yml b/.github/workflows/preview-env.yml index 06ca3dec..28901d2a 100644 --- a/.github/workflows/preview-env.yml +++ b/.github/workflows/preview-env.yml @@ -55,8 +55,9 @@ jobs: SANITIZED_BRANCH=$( printf '%s' "$RAW_BRANCH" \ | tr '[:upper:]' '[:lower:]' \ - | tr -c 'a-z0-9._-' '-' \ - | sed -E 's/^[^a-z0-9]+//; s/[^a-z0-9]+$//' + | tr '._' '-' \ + | tr -c 'a-z0-9-' '-' \ + | sed -E 's/-{2,}/-/g; s/^-+//; s/-+$//' ) # Last resort fallback if everything got stripped diff --git a/.gitignore b/.gitignore index 4b30addd..4a5191c9 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ **/__pycache__ **/.ruff_cache **/.mypy_cache +**/.idea # Testing artefacts **/.hypothesis diff --git a/gateway-api/poetry.lock b/gateway-api/poetry.lock index d125513f..338577d4 100644 --- a/gateway-api/poetry.lock +++ b/gateway-api/poetry.lock @@ -2243,4 +2243,4 @@ propcache = ">=0.2.1" [metadata] lock-version = "2.1" python-versions = ">3.13,<4.0.0" -content-hash = "ec013475c1d46e197243aad2c6ac8eb56957da032015c466694978937b784a61" +content-hash = "67e8839de72625c8f7c4d42aea6ea55afaf9f738aef2267bb4dac2f83a389f8e" diff --git a/gateway-api/pyproject.toml b/gateway-api/pyproject.toml index 3a5cd476..2242551f 100644 --- a/gateway-api/pyproject.toml +++ b/gateway-api/pyproject.toml @@ -7,14 +7,13 @@ authors = [ ] readme = "README.md" requires-python = ">3.13,<4.0.0" -dependencies = [ -] [tool.poetry.dependencies] clinical-data-common = { git = "https://github.com/NHSDigital/clinical-data-common.git", tag = "v0.1.0" } [tool.poetry] -packages = [{include = "gateway_api", from = "src"}] +packages = [{include = "gateway_api", from = "src"}, + {include = "stubs", from = "stubs"}] [tool.coverage.run] relative_files = true diff --git a/gateway-api/src/gateway_api/pds_search.py b/gateway-api/src/gateway_api/pds_search.py new file mode 100644 index 00000000..cddcc056 --- /dev/null +++ b/gateway-api/src/gateway_api/pds_search.py @@ -0,0 +1,394 @@ +""" +PDS (Personal Demographics Service) FHIR R4 patient lookup client. + +Contracts enforced by the helper functions: + +* ``Patient.name[]`` records passed to :func:`find_current_name_record` must contain:: + + record["period"]["start"] + record["period"]["end"] + +* ``Patient.generalPractitioner[]`` records passed to :func:`find_current_record` must + contain:: + + record["identifier"]["period"]["start"] + record["identifier"]["period"]["end"] + +If required keys are missing, a ``KeyError`` is raised intentionally. This is treated as +malformed upstream data (or malformed test fixtures) and should be corrected at source. +""" + +from __future__ import annotations + +import uuid +from dataclasses import dataclass +from datetime import date, datetime, timezone +from typing import cast + +import requests + +# Recursive JSON-like structure typing used for parsed FHIR bodies. +type ResultStructure = str | dict[str, "ResultStructure"] | list["ResultStructure"] +type ResultStructureDict = dict[str, ResultStructure] +type ResultList = list[ResultStructureDict] + + +class ExternalServiceError(Exception): + """ + Raised when the downstream PDS request fails. + + This module catches :class:`requests.HTTPError` thrown by + ``response.raise_for_status()`` and re-raises it as ``ExternalServiceError`` so + callers are not coupled to ``requests`` exception types. + """ + + +@dataclass +class SearchResults: + """ + A single extracted patient record. + + Only a small subset of the PDS Patient fields are currently required by this + gateway. More will be added in later phases. + + :param given_names: Given names from the *current* ``Patient.name`` record, + concatenated with spaces. + :param family_name: Family name from the *current* ``Patient.name`` record. + :param nhs_number: NHS number (``Patient.id``). + :param gp_ods_code: The ODS code of the *current* GP, extracted from + ``Patient.generalPractitioner[].identifier.value`` if a current GP record exists + otherwise ``None``. + """ + + given_names: str + family_name: str + nhs_number: str + gp_ods_code: str | None + + +class PdsClient: + """ + Simple client for PDS FHIR R4 patient retrieval. + + The client currently supports one operation: + + * :meth:`search_patient_by_nhs_number` - calls ``GET /Patient/{nhs_number}`` + + This method returns a :class:`SearchResults` instance when a patient can be + extracted, otherwise ``None``. + + **Usage example**:: + + pds = PdsClient( + auth_token="YOUR_ACCESS_TOKEN", + end_user_org_ods="A12345", + base_url="https://sandbox.api.service.nhs.uk/personal-demographics/FHIR/R4", + ) + + result = pds.search_patient_by_nhs_number(9000000009) + + if result: + print(result) + """ + + # URLs for different PDS environments. Requires authentication to use live. + SANDBOX_URL = "https://sandbox.api.service.nhs.uk/personal-demographics/FHIR/R4" + INT_URL = "https://int.api.service.nhs.uk/personal-demographics/FHIR/R4" + PROD_URL = "https://api.service.nhs.uk/personal-demographics/FHIR/R4" + + def __init__( + self, + auth_token: str, + end_user_org_ods: str, + base_url: str = SANDBOX_URL, + nhsd_session_urid: str | None = None, + timeout: int = 10, + ignore_dates: bool = False, + ) -> None: + """ + Create a PDS client. + + :param auth_token: OAuth2 bearer token (without the ``"Bearer "`` prefix). + :param end_user_org_ods: NHSD End User Organisation ODS code. + :param base_url: Base URL for the PDS API (one of :attr:`SANDBOX_URL`, + :attr:`INT_URL`, :attr:`PROD_URL`). Trailing slashes are stripped. + :param nhsd_session_urid: Optional ``NHSD-Session-URID`` header value. + :param timeout: Default timeout in seconds for HTTP calls. + :param ignore_dates: If ``True`` just get the most recent name or GP record, + ignoring the date ranges. + """ + self.auth_token = auth_token + self.end_user_org_ods = end_user_org_ods + self.base_url = base_url.rstrip("/") + self.nhsd_session_urid = nhsd_session_urid + self.timeout = timeout + self.ignore_dates = ignore_dates + + def _build_headers( + self, + request_id: str | None = None, + correlation_id: str | None = None, + ) -> dict[str, str]: + """ + Build mandatory and optional headers for a PDS request. + + :param request_id: Optional ``X-Request-ID``. If not supplied a new UUID is + generated. + :param correlation_id: Optional ``X-Correlation-ID`` for cross-system tracing. + :return: Dictionary of HTTP headers for the outbound request. + """ + headers = { + "X-Request-ID": request_id or str(uuid.uuid4()), + "NHSD-End-User-Organisation-ODS": self.end_user_org_ods, + "Accept": "application/fhir+json", + } + + # Trying to pass an auth token to the sandbox makes PDS unhappy + if self.base_url != self.SANDBOX_URL: + headers["Authorization"] = f"Bearer {self.auth_token}" + + # NHSD-Session-URID is required in some flows; include only if configured. + if self.nhsd_session_urid: + headers["NHSD-Session-URID"] = self.nhsd_session_urid + + # Correlation ID is used to track the same request across multiple systems. + # Can be safely omitted, mirrored back in response if included. + if correlation_id: + headers["X-Correlation-ID"] = correlation_id + + return headers + + def search_patient_by_nhs_number( + self, + nhs_number: int, + request_id: str | None = None, + correlation_id: str | None = None, + timeout: int | None = None, + ) -> SearchResults | None: + """ + Retrieve a patient by NHS number. + + Calls ``GET /Patient/{nhs_number}``, which returns a single FHIR Patient + resource on success, then extracts a single :class:`SearchResults`. + + :param nhs_number: NHS number to search for. + :param request_id: Optional request ID to reuse for retries; if not supplied a + UUID is generated. + :param correlation_id: Optional correlation ID for tracing. + :param timeout: Optional per-call timeout in seconds. If not provided, + :attr:`timeout` is used. + :return: A :class:`SearchResults` instance if a patient can be extracted, + otherwise ``None``. + :raises ExternalServiceError: If the HTTP request returns an error status and + ``raise_for_status()`` raises :class:`requests.HTTPError`. + """ + headers = self._build_headers( + request_id=request_id, + correlation_id=correlation_id, + ) + + url = f"{self.base_url}/Patient/{nhs_number}" + + response = requests.get( + url, + headers=headers, + params={}, + timeout=timeout or self.timeout, + ) + + try: + # In production, failures surface here (4xx/5xx -> HTTPError). + response.raise_for_status() + except requests.HTTPError as err: + raise ExternalServiceError( + f"PDS request failed: {err.response.reason}" + ) from err + + body = response.json() + return self._extract_single_search_result(body) + + # --------------- internal helpers for result extraction ----------------- + + def _get_gp_ods_code(self, general_practitioners: ResultList) -> str | None: + """ + Extract the current GP ODS code from ``Patient.generalPractitioner``. + + This function implements the business rule: + + * If the list is empty, return ``None``. + * If the list is non-empty and no record is current, return ``None``. + * If exactly one record is current, return its ``identifier.value``. + + In future this may change to return the most recent record if none is current. + + :param general_practitioners: List of ``generalPractitioner`` records from a + Patient resource. + :return: ODS code string if a current record exists, otherwise ``None``. + :raises KeyError: If a record is missing required ``identifier.period`` fields. + """ + if len(general_practitioners) == 0: + return None + + gp = self.find_current_gp(general_practitioners) + if gp is None: + return None + + identifier = cast("ResultStructureDict", gp.get("identifier", {})) + ods_code = str(identifier.get("value", None)) + + # Avoid returning the literal string "None" if identifier.value is absent. + return None if ods_code == "None" else ods_code + + def _extract_single_search_result( + self, body: ResultStructureDict + ) -> SearchResults | None: + """ + Extract a single :class:`SearchResults` from a Patient response. + + This helper accepts either: + * a single FHIR Patient resource (as returned by ``GET /Patient/{id}``), or + * a FHIR Bundle containing Patient entries (as typically returned by searches). + + For Bundle inputs, the code assumes either zero matches (empty entry list) or a + single match; if multiple entries are present, the first entry is used. + :param body: Parsed JSON body containing either a Patient resource or a Bundle + whose first entry contains a Patient resource under ``resource``. + :return: A populated :class:`SearchResults` if extraction succeeds, otherwise + ``None``. + """ + # Accept either: + # 1) Patient (GET /Patient/{id}) + # 2) Bundle with Patient in entry[0].resource (search endpoints) + if str(body.get("resourceType", "")) == "Patient": + patient = body + else: + entries: ResultList = cast("ResultList", body.get("entry", [])) + if not entries: + raise RuntimeError("PDS response contains no patient entries") + + # Use the first patient entry. Search by NHS number is unique. Search by + # demographics for an application is allowed to return max one entry from + # PDS. Search by a human can return more, but presumably we count as an + # application. + # See MaxResults parameter in the PDS OpenAPI spec. + entry = entries[0] + patient = cast("ResultStructureDict", entry.get("resource", {})) + + nhs_number = str(patient.get("id", "")).strip() + if not nhs_number: + raise RuntimeError("PDS patient resource missing NHS number") + + # Select current name record and extract names. + names = cast("ResultList", patient.get("name", [])) + current_name = self.find_current_name_record(names) + + if current_name is not None: + given_names_list = cast("list[str]", current_name.get("given", [])) + family_name = str(current_name.get("family", "")) or "" + given_names_str = " ".join(given_names_list).strip() + else: + given_names_str = "" + family_name = "" + + # Extract GP ODS code if a current GP record exists. + gp_list = cast("ResultList", patient.get("generalPractitioner", [])) + gp_ods_code = self._get_gp_ods_code(gp_list) + + return SearchResults( + given_names=given_names_str, + family_name=family_name, + nhs_number=nhs_number, + gp_ods_code=gp_ods_code, + ) + + def find_current_gp( + self, records: ResultList, today: date | None = None + ) -> ResultStructureDict | None: + """ + Select the current record from a ``generalPractitioner`` list. + + A record is "current" if its ``identifier.period`` covers ``today`` (inclusive): + + ``start <= today <= end`` + + Or else if self.ignore_dates is True, the last record in the list is returned. + + The list may be in any of the following states: + + * empty + * contains one or more records, none current + * contains one or more records, exactly one current + + :param records: List of ``generalPractitioner`` records. + :param today: Optional override date, intended for deterministic tests. + If not supplied, the current UTC date is used. + :return: The first record whose ``identifier.period`` covers ``today``, or + ``None`` if no record is current. + :raises KeyError: If required keys are missing for a record being evaluated. + :raises ValueError: If ``start`` or ``end`` are not valid ISO date strings. + """ + if today is None: + today = datetime.now(timezone.utc).date() + + if self.ignore_dates: + if len(records) > 0: + return records[-1] + else: + return None + + for record in records: + identifier = cast("ResultStructureDict", record["identifier"]) + periods = cast("dict[str, str]", identifier["period"]) + start_str = periods["start"] + end_str = periods["end"] + + start = date.fromisoformat(start_str) + end = date.fromisoformat(end_str) + + if start <= today <= end: + return record + + return None + + def find_current_name_record( + self, records: ResultList, today: date | None = None + ) -> ResultStructureDict | None: + """ + Select the current record from a ``Patient.name`` list. + + A record is "current" if its ``period`` covers ``today`` (inclusive): + + ``start <= today <= end`` + + Or else if self.ignore_dates is True, the last record in the list is returned. + + :param records: List of ``Patient.name`` records. + :param today: Optional override date, intended for deterministic tests. + If not supplied, the current UTC date is used. + :return: The first name record whose ``period`` covers ``today``, or ``None`` if + no record is current. + :raises KeyError: If required keys (``period.start`` / ``period.end``) are + missing. + :raises ValueError: If ``start`` or ``end`` are not valid ISO date strings. + """ + if today is None: + today = datetime.now(timezone.utc).date() + + if self.ignore_dates: + if len(records) > 0: + return records[-1] + else: + return None + + for record in records: + periods = cast("dict[str, str]", record["period"]) + start_str = periods["start"] + end_str = periods["end"] + + start = date.fromisoformat(start_str) + end = date.fromisoformat(end_str) + + if start <= today <= end: + return record + + return None diff --git a/gateway-api/src/gateway_api/test_pds_search.py b/gateway-api/src/gateway_api/test_pds_search.py new file mode 100644 index 00000000..78ed9e73 --- /dev/null +++ b/gateway-api/src/gateway_api/test_pds_search.py @@ -0,0 +1,660 @@ +""" +Unit tests for :mod:`gateway_api.pds_search`. +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass +from datetime import date +from typing import Any, cast +from uuid import uuid4 + +import pytest +import requests +from stubs.stub_pds import PdsFhirApiStub + +from gateway_api.pds_search import ( + ExternalServiceError, + PdsClient, + ResultList, +) + + +@dataclass +class FakeResponse: + """ + Minimal substitute for :class:`requests.Response` used by tests. + + Only the methods accessed by :class:`gateway_api.pds_search.PdsClient` are + implemented. + + :param status_code: HTTP status code. + :param headers: Response headers. + :param _json: Parsed JSON body returned by :meth:`json`. + """ + + status_code: int + headers: dict[str, str] + _json: dict[str, Any] + reason: str = "" + + def json(self) -> dict[str, Any]: + """ + Return the response JSON body. + + :return: Parsed JSON body. + """ + return self._json + + def raise_for_status(self) -> None: + """ + Emulate :meth:`requests.Response.raise_for_status`. + + :return: ``None``. + :raises requests.HTTPError: If the response status is not 200. + """ + if self.status_code != 200: + err = requests.HTTPError(f"{self.status_code} Error") + # requests attaches a Response to HTTPError.response; the client expects it + err.response = self + raise err + + +@pytest.fixture +def stub() -> PdsFhirApiStub: + """ + Create a stub backend instance. + + :return: A :class:`stubs.stub_pds.PdsFhirApiStub` with strict header validation + enabled. + """ + # Strict header validation helps ensure PdsClient sends X-Request-ID correctly. + return PdsFhirApiStub(strict_headers=True) + + +@pytest.fixture +def mock_requests_get( + monkeypatch: pytest.MonkeyPatch, stub: PdsFhirApiStub +) -> dict[str, Any]: + """ + Patch ``requests.get`` so calls are routed into :meth:`PdsFhirApiStub.get_patient`. + + The fixture returns a "capture" dict recording the most recent request information. + This is used by header-related tests. + + :param monkeypatch: Pytest monkeypatch fixture. + :param stub: Stub backend used to serve GET requests. + :return: A capture dictionary containing the last call details + (url/headers/params/timeout). + """ + capture: dict[str, Any] = {} + + def _fake_get( + url: str, + headers: dict[str, str] | None = None, + params: Any = None, + timeout: Any = None, + ) -> FakeResponse: + """ + Replacement function for :func:`requests.get`. + + :param url: URL passed by the client. + :param headers: Headers passed by the client. + :param params: Query parameters (recorded, not interpreted for + GET /Patient/{id}). + :param timeout: Timeout (recorded). + :return: A :class:`FakeResponse` whose behaviour mimics ``requests.Response``. + """ + headers = headers or {} + capture["url"] = url + capture["headers"] = dict(headers) + capture["params"] = params + capture["timeout"] = timeout + + # The client under test is expected to call GET {base_url}/Patient/{id}. + m = re.match(r"^(?P.+)/Patient/(?P\d+)$", url) + if not m: + raise AssertionError(f"Unexpected URL called by client: {url}") + + nhs_number = m.group("nhs") + + # Route the "HTTP" request into the in-memory stub. + stub_resp = stub.get_patient( + nhs_number=nhs_number, + request_id=headers.get("X-Request-ID"), + correlation_id=headers.get("X-Correlation-ID"), + authorization=headers.get("Authorization"), + end_user_org_ods=headers.get("NHSD-End-User-Organisation-ODS"), + ) + + # GET /Patient/{id} returns a single Patient resource on success. + body = stub_resp.json + # Populate a reason phrase so PdsClient can surface it in ExternalServiceError. + reason = "" + if stub_resp.status_code != 200: + # Try to use OperationOutcome display text if present. + issue0 = (stub_resp.json.get("issue") or [{}])[0] + details = issue0.get("details") or {} + coding0 = (details.get("coding") or [{}])[0] + reason = str(coding0.get("display") or "") + if not reason: + reason = {400: "Bad Request", 404: "Not Found"}.get( + stub_resp.status_code, "" + ) + + return FakeResponse( + status_code=stub_resp.status_code, + headers=stub_resp.headers, + _json=body, + reason=reason, + ) + + monkeypatch.setattr(requests, "get", _fake_get) + return capture + + +def _insert_basic_patient( + stub: PdsFhirApiStub, + nhs_number: str, + family: str, + given: list[str], + general_practitioner: list[dict[str, Any]] | None = None, +) -> None: + """ + Insert a basic Patient record into the stub. + + :param stub: Stub backend to insert into. + :param nhs_number: NHS number (10-digit string). + :param family: Family name for the Patient.name record. + :param given: Given names for the Patient.name record. + :param general_practitioner: Optional list stored under + ``Patient.generalPractitioner``. + :return: ``None``. + """ + stub.upsert_patient( + nhs_number=nhs_number, + patient={ + "resourceType": "Patient", + "name": [ + { + "use": "official", + "family": family, + "given": given, + "period": {"start": "1900-01-01", "end": "9999-12-31"}, + } + ], + "generalPractitioner": general_practitioner or [], + }, + version_id=1, + ) + + +def test_search_patient_by_nhs_number_get_patient_success( + stub: PdsFhirApiStub, + mock_requests_get: dict[str, Any], +) -> None: + """ + Verify ``GET /Patient/{nhs_number}`` returns 200 and demographics are extracted. + + This test explicitly inserts the patient into the stub and asserts that the client + returns a populated :class:`gateway_api.pds_search.SearchResults`. + + :param stub: Stub backend fixture. + :param mock_requests_get: Patched ``requests.get`` fixture + (ensures patching is active). + :return: ``None``. + """ + _insert_basic_patient( + stub=stub, + nhs_number="9000000009", + family="Smith", + given=["Jane"], + general_practitioner=[], + ) + + client = PdsClient( + auth_token="test-token", # noqa: S106 (test token hardcoded) + end_user_org_ods="A12345", + base_url="https://example.test/personal-demographics/FHIR/R4", + nhsd_session_urid="test-urid", + ) + + result = client.search_patient_by_nhs_number(9000000009) + + assert result is not None + assert result.nhs_number == "9000000009" + assert result.family_name == "Smith" + assert result.given_names == "Jane" + assert result.gp_ods_code is None + + +def test_search_patient_by_nhs_number_no_current_gp_returns_gp_ods_code_none( + stub: PdsFhirApiStub, + mock_requests_get: dict[str, Any], +) -> None: + """ + Verify that ``gp_ods_code`` is ``None`` when no GP record is current. + + The generalPractitioner list may be: + * empty + * non-empty with no current record + * non-empty with exactly one current record + + This test covers the "non-empty, none current" case by + inserting only a historical GP record. + + :param monkeypatch: Pytest monkeypatch fixture. + :param stub: Stub backend fixture. + :param mock_requests_get: Patched ``requests.get`` fixture. + :return: ``None``. + """ + _insert_basic_patient( + stub=stub, + nhs_number="9000000018", + family="Taylor", + given=["Ben"], + general_practitioner=[ + { + "id": "1", + "type": "Organization", + "identifier": { + "value": "OLDGP", + "period": {"start": "2010-01-01", "end": "2012-01-01"}, + }, + } + ], + ) + + client = PdsClient( + auth_token="test-token", # noqa: S106 (test token hardcoded) + end_user_org_ods="A12345", + base_url="https://example.test/personal-demographics/FHIR/R4", + ) + + result = client.search_patient_by_nhs_number(9000000018) + + assert result is not None + assert result.nhs_number == "9000000018" + assert result.family_name == "Taylor" + assert result.given_names == "Ben" + assert result.gp_ods_code is None + + +def test_search_patient_by_nhs_number_sends_expected_headers( + stub: PdsFhirApiStub, + mock_requests_get: dict[str, Any], +) -> None: + """ + Verify that the client sends the expected headers to PDS. + + Asserts that the request contains: + * Authorization header + * NHSD-End-User-Organisation-ODS header + * Accept header + * caller-provided X-Request-ID and X-Correlation-ID headers + + :param stub: Stub backend fixture. + :param mock_requests_get: Patched ``requests.get`` fixture capturing outbound + headers. + :return: ``None``. + """ + _insert_basic_patient( + stub=stub, + nhs_number="9000000009", + family="Smith", + given=["Jane"], + general_practitioner=[], + ) + + client = PdsClient( + auth_token="test-token", # noqa: S106 + end_user_org_ods="A12345", + base_url="https://example.test/personal-demographics/FHIR/R4", + ) + + req_id = str(uuid4()) + corr_id = "corr-123" + + result = client.search_patient_by_nhs_number( + 9000000009, + request_id=req_id, + correlation_id=corr_id, + ) + assert result is not None + + headers = mock_requests_get["headers"] + assert headers["Authorization"] == "Bearer test-token" + assert headers["NHSD-End-User-Organisation-ODS"] == "A12345" + assert headers["Accept"] == "application/fhir+json" + assert headers["X-Request-ID"] == req_id + assert headers["X-Correlation-ID"] == corr_id + + +def test_search_patient_by_nhs_number_generates_request_id( + stub: PdsFhirApiStub, + mock_requests_get: dict[str, Any], +) -> None: + """ + Verify that the client generates an X-Request-ID when not provided. + + The stub is in strict mode, so a missing or invalid X-Request-ID would cause a 400. + This test confirms a request ID is present and looks UUID-like. + + :param stub: Stub backend fixture. + :param mock_requests_get: Patched ``requests.get`` fixture capturing outbound + headers. + :return: ``None``. + """ + _insert_basic_patient( + stub=stub, + nhs_number="9000000009", + family="Smith", + given=["Jane"], + general_practitioner=[], + ) + + client = PdsClient( + auth_token="test-token", # noqa: S106 + end_user_org_ods="A12345", + base_url="https://example.test/personal-demographics/FHIR/R4", + ) + + result = client.search_patient_by_nhs_number(9000000009) + assert result is not None + + headers = mock_requests_get["headers"] + assert "X-Request-ID" in headers + assert isinstance(headers["X-Request-ID"], str) + assert len(headers["X-Request-ID"]) >= 32 + + +def test_search_patient_by_nhs_number_not_found_raises_error( + stub: PdsFhirApiStub, + mock_requests_get: dict[str, Any], +) -> None: + """ + Verify that a 404 response results in :class:`ExternalServiceError`. + + The stub returns a 404 OperationOutcome for unknown NHS numbers. The client calls + ``raise_for_status()``, which raises ``requests.HTTPError``; the client wraps that + into :class:`ExternalServiceError`. + + :param stub: Stub backend fixture. + :param mock_requests_get: Patched ``requests.get`` fixture. + :return: ``None``. + """ + pds = PdsClient( + auth_token="test-token", # noqa: S106 + end_user_org_ods="A12345", + base_url="https://example.test/personal-demographics/FHIR/R4", + ) + + with pytest.raises(ExternalServiceError): + pds.search_patient_by_nhs_number(9900000001) + + +def test_search_patient_by_nhs_number_extracts_current_gp_ods_code( + stub: PdsFhirApiStub, + mock_requests_get: dict[str, Any], +) -> None: + """ + Verify that a current GP record is selected and its ODS code returned. + + The test inserts a patient with two GP records: + * one historical (not current) + * one current (period covers today) + + :param monkeypatch: Pytest monkeypatch fixture. + :param stub: Stub backend fixture. + :param mock_requests_get: Patched ``requests.get`` fixture. + :return: ``None``. + """ + stub.upsert_patient( + nhs_number="9000000017", + patient={ + "resourceType": "Patient", + "name": [ + { + "use": "official", + "family": "Taylor", + "given": ["Ben", "A."], + "period": {"start": "1900-01-01", "end": "9999-12-31"}, + } + ], + "generalPractitioner": [ + # Old + { + "id": "1", + "type": "Organization", + "identifier": { + "value": "OLDGP", + "period": {"start": "2010-01-01", "end": "2012-01-01"}, + }, + }, + # Current + { + "id": "2", + "type": "Organization", + "identifier": { + "value": "CURRGP", + "period": {"start": "2020-01-01", "end": "9999-01-01"}, + }, + }, + ], + }, + version_id=1, + ) + + client = PdsClient( + auth_token="test-token", # noqa: S106 + end_user_org_ods="A12345", + base_url="https://example.test/personal-demographics/FHIR/R4", + ) + + result = client.search_patient_by_nhs_number(9000000017) + assert result is not None + assert result.nhs_number == "9000000017" + assert result.family_name == "Taylor" + assert result.given_names == "Ben A." + assert result.gp_ods_code == "CURRGP" + + +def test_find_current_gp_with_today_override() -> None: + """ + Verify that ``find_current_gp`` honours an explicit ``today`` value. + + :return: ``None``. + """ + pds = PdsClient("test-token", "A12345") + pds_ignore_dates = PdsClient("test-token", "A12345", ignore_dates=True) + + records = cast( + "ResultList", + [ + { + "identifier": { + "value": "a", + "period": {"start": "2020-01-01", "end": "2020-12-31"}, + } + }, + { + "identifier": { + "value": "b", + "period": {"start": "2021-01-01", "end": "2021-12-31"}, + } + }, + ], + ) + + assert pds.find_current_gp(records, today=date(2020, 6, 1)) == records[0] + assert pds.find_current_gp(records, today=date(2021, 6, 1)) == records[1] + assert pds.find_current_gp(records, today=date(2019, 6, 1)) is None + assert pds_ignore_dates.find_current_gp(records, today=date(2019, 6, 1)) is not None + + +def test_find_current_name_record_no_current_name() -> None: + """ + Verify that ``find_current_name_record`` returns ``None`` when no current name + exists. + + :return: ``None``. + """ + pds = PdsClient("test-token", "A12345") + pds_ignore_date = PdsClient("test-token", "A12345", ignore_dates=True) + + records = cast( + "ResultList", + [ + { + "use": "official", + "family": "Doe", + "given": ["John"], + "period": {"start": "2000-01-01", "end": "2010-12-31"}, + }, + { + "use": "official", + "family": "Smith", + "given": ["John"], + "period": {"start": "2011-01-01", "end": "2020-12-31"}, + }, + ], + ) + + assert pds.find_current_name_record(records) is None + assert pds_ignore_date.find_current_name_record(records) is not None + + +def test_extract_single_search_result_invalid_body_raises_runtime_error() -> None: + """ + Verify that ``PdsClient._extract_single_search_result`` raises ``RuntimeError`` when + mandatory patient content is missing. + + This test asserts that a ``RuntimeError`` is raised when: + + * The body is a bundle containing no entries (``entry`` is empty). + * The body is a patient resource with no NHS number (missing/blank ``id``). + * The body is a patient resource with an NHS number, + but the patient has no *current* + """ + client = PdsClient( + auth_token="test-token", # noqa: S106 (test token hardcoded) + end_user_org_ods="A12345", + base_url="https://example.test/personal-demographics/FHIR/R4", + ) + + # 1) Bundle contains no entries. + bundle_no_entries: Any = {"resourceType": "Bundle", "entry": []} + with pytest.raises(RuntimeError): + client._extract_single_search_result(bundle_no_entries) # noqa SLF001 (testing private method) + + # 2) Patient has no NHS number (Patient.id missing/blank). + patient_missing_nhs_number: Any = { + "resourceType": "Patient", + "name": [ + { + "use": "official", + "family": "Smith", + "given": ["Jane"], + "period": {"start": "1900-01-01", "end": "9999-12-31"}, + } + ], + "generalPractitioner": [], + } + with pytest.raises(RuntimeError): + client._extract_single_search_result(patient_missing_nhs_number) # noqa SLF001 (testing private method) + + # 3) Bundle entry exists with NHS number, but no current name record. + bundle_no_current_name: Any = { + "resourceType": "Bundle", + "entry": [ + { + "resource": { + "resourceType": "Patient", + "id": "9000000009", + "name": [ + { + "use": "official", + "family": "Smith", + "given": ["Jane"], + "period": {"start": "1900-01-01", "end": "1900-12-31"}, + } + ], + "generalPractitioner": [], + } + } + ], + } + + # No current name record is tolerated by PdsClient; names are returned as empty. + result = client._extract_single_search_result(bundle_no_current_name) # noqa SLF001 (testing private method) + assert result is not None + assert result.nhs_number == "9000000009" + assert result.given_names == "" + assert result.family_name == "" + + +def test_find_current_name_record_ignore_dates_returns_last_or_none() -> None: + """ + If ignore_dates=True: + * returns the last name record even if none are current + * returns None when the list is empty + """ + pds_ignore = PdsClient("test-token", "A12345", ignore_dates=True) + + records = cast( + "ResultList", + [ + { + "use": "official", + "family": "Old", + "given": ["First"], + "period": {"start": "1900-01-01", "end": "1900-12-31"}, + }, + { + "use": "official", + "family": "Newer", + "given": ["Second"], + "period": {"start": "1901-01-01", "end": "1901-12-31"}, + }, + ], + ) + + # Pick a date that is not covered by any record; ignore_dates should still pick last + chosen = pds_ignore.find_current_name_record(records, today=date(2026, 1, 1)) + assert chosen == records[-1] + + assert pds_ignore.find_current_name_record(cast("ResultList", [])) is None + + +def test_find_current_gp_ignore_dates_returns_last_or_none() -> None: + """ + If ignore_dates=True: + * returns the last GP record even if none are current + * returns None when the list is empty + """ + pds_ignore = PdsClient("test-token", "A12345", ignore_dates=True) + + records = cast( + "ResultList", + [ + { + "identifier": { + "value": "GP-OLD", + "period": {"start": "1900-01-01", "end": "1900-12-31"}, + } + }, + { + "identifier": { + "value": "GP-NEWER", + "period": {"start": "1901-01-01", "end": "1901-12-31"}, + } + }, + ], + ) + + # Pick a date that is not covered by any record; ignore_dates should still pick last + chosen = pds_ignore.find_current_gp(records, today=date(2026, 1, 1)) + assert chosen == records[-1] + + assert pds_ignore.find_current_gp(cast("ResultList", [])) is None diff --git a/gateway-api/stubs/stubs/__init__.py b/gateway-api/stubs/stubs/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/gateway-api/stubs/stubs/py.typed b/gateway-api/stubs/stubs/py.typed new file mode 100644 index 00000000..e69de29b diff --git a/gateway-api/stubs/stubs/stub_pds.py b/gateway-api/stubs/stubs/stub_pds.py new file mode 100644 index 00000000..dba6c1b9 --- /dev/null +++ b/gateway-api/stubs/stubs/stub_pds.py @@ -0,0 +1,323 @@ +""" +In-memory PDS FHIR R4 API stub. + +The stub does **not** implement the full PDS API surface, nor full FHIR validation. +""" + +from __future__ import annotations + +import re +import uuid +from dataclasses import dataclass +from datetime import datetime, timezone +from typing import Any + + +@dataclass(frozen=True) +class StubResponse: + """ + Minimal response object returned by :class:`PdsFhirApiStub`. + + :param status_code: HTTP-like status code for the response. + :param headers: HTTP-like response headers. + :param json: Parsed JSON response body. + """ + + status_code: int + headers: dict[str, str] + json: dict[str, Any] + + +class PdsFhirApiStub: + """ + Minimal in-memory stub for the PDS FHIR API, implementing only ``GET /Patient/{id}`` + + Contract elements modelled from the PDS OpenAPI spec: + + * ``/Patient/{id}`` where ``id`` is the patient's NHS number (10 digits) + * ``X-Request-ID`` is mandatory (in strict mode) and echoed back as ``X-Request-Id`` + * ``X-Correlation-ID`` is optional and echoed back as ``X-Correlation-Id`` + if supplied + * ``ETag`` follows ``W/""`` and corresponds to ``Patient.meta.versionId`` + + See: + https://github.com/NHSDigital/personal-demographics-service-api/blob/master/specification/personal-demographics.yaml + """ + + def __init__(self, strict_headers: bool = True) -> None: + """ + Create a new stub instance. + + :param strict_headers: If ``True``, enforce presence and UUID format of + ``X-Request-ID``. If ``False``, header validation is relaxed. + """ + self.strict_headers = strict_headers + + # Internal store: nhs_number -> (patient_resource, version_id_int) + self._patients: dict[str, tuple[dict[str, Any], int]] = {} + + # Seed a deterministic example matching the spec's id example. + # Tests may overwrite this record via upsert_patient. + self.upsert_patient( + nhs_number="9000000009", + patient={ + "resourceType": "Patient", + "id": "9000000009", + "meta": { + "versionId": "1", + "lastUpdated": "2020-01-01T00:00:00Z", + }, + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "9000000009", + } + ], + "name": [ + { + "use": "official", + "family": "Smith", + "given": ["Jane"], + "period": {"start": "1900-01-01", "end": "9999-12-31"}, + } + ], + "gender": "female", + "birthDate": "1970-01-01", + }, + version_id=1, + ) + + # --------------------------- + # Public API for tests + # --------------------------- + + def upsert_patient( + self, + nhs_number: str, + patient: dict[str, Any] | None = None, + version_id: int = 1, + ) -> None: + """ + Insert or replace a patient record in the stub store. + + :param nhs_number: NHS number as a 10-digit string. + :param patient: Patient resource dictionary. If ``None``, an empty Patient dict + is created and populated with required keys. + :param version_id: Version integer recorded into + ``patient["meta"]["versionId"]`` and used to generate the ETag on retrieval. + :return: ``None``. + :raises TypeError: If ``nhs_number`` is not a string. + :raises ValueError: If ``nhs_number`` is not 10 digits or fails validation. + """ + try: + nhsnum_match = re.fullmatch(r"(\d{10})", nhs_number) + except TypeError as err: + raise TypeError("NHS Number must be a string") from err + + if not nhsnum_match: + raise ValueError("NHS Number must be exactly 10 digits") + + if not self._is_valid_nhs_number(nhs_number): + raise ValueError("NHS Number is not valid") + + if patient is None: + patient = {} + + patient.setdefault("resourceType", "Patient") + patient["id"] = nhs_number + patient.setdefault("meta", {}) + patient["meta"]["versionId"] = str(version_id) + patient["meta"].setdefault("lastUpdated", self._now_fhir_instant()) + + self._patients[nhs_number] = (patient, version_id) + + def get_patient( + self, + nhs_number: str, + request_id: str | None = None, + correlation_id: str | None = None, + authorization: str | None = None, # noqa: F841 # NOSONAR S1172 (ignored in stub) + role_id: str | None = None, # noqa: F841 # NOSONAR S1172 (ignored in stub) + end_user_org_ods: str | None = None, # noqa: F841 # NOSONAR S1172 (ignored in stub) + ) -> StubResponse: + """ + Implements ``GET /Patient/{id}``. + + :param nhs_number: The NHS number path parameter. + :param request_id: The ``X-Request-ID`` header value. Required + (and must be UUID) when ``strict_headers=True``. + :param correlation_id: Optional ``X-Correlation-ID`` header value. + :param authorization: Authorization header (ignored by the stub). + :param role_id: Role header (ignored by the stub). + :param end_user_org_ods: End-user ODS header (ignored by the stub). + :return: A :class:`StubResponse` representing either: + * ``200`` with Patient JSON + * ``404`` with OperationOutcome JSON + * ``400`` with OperationOutcome JSON (validation failures) + """ + headers_out: dict[str, str] = {} + + # Header validation mirrors key behaviour enforced by the real service. + if self.strict_headers: + if not request_id: + return self._bad_request( + "Missing X-Request-ID", + request_id=request_id, + correlation_id=correlation_id, + ) + if not self._is_uuid(request_id): + return self._bad_request( + "Invalid X-Request-ID (must be a UUID)", + request_id=request_id, + correlation_id=correlation_id, + ) + + # Echo trace headers back (note casing). + if request_id: + headers_out["X-Request-Id"] = request_id + if correlation_id: + headers_out["X-Correlation-Id"] = correlation_id + + # Path parameter validation: must be 10 digits and pass NHS-number validation. + if not re.fullmatch( + r"\d{10}", nhs_number or "" + ) or not self._is_valid_nhs_number(nhs_number): + return self._operation_outcome( + status_code=400, + headers=headers_out, + spine_code="INVALID_RESOURCE_ID", + display="Resource Id is invalid", + ) + + # Lookup: not present => 404 OperationOutcome. + if nhs_number not in self._patients: + return self._operation_outcome( + status_code=404, + headers=headers_out, + spine_code="RESOURCE_NOT_FOUND", + display="Patient not found", + ) + + patient, version_id = self._patients[nhs_number] + + # ETag mirrors the "W/\"\"" shape and aligns to meta.versionId. + headers_out["ETag"] = f'W/"{version_id}"' + return StubResponse(status_code=200, headers=headers_out, json=patient) + + # --------------------------- + # Internal helpers + # --------------------------- + + @staticmethod + def _now_fhir_instant() -> str: + """ + Generate a FHIR instant timestamp in UTC with seconds precision. + + :return: Timestamp string in the format ``YYYY-MM-DDTHH:MM:SSZ``. + """ + return ( + datetime.now(timezone.utc) + .replace(microsecond=0) + .isoformat() + .replace("+00:00", "Z") + ) + + @staticmethod + def _is_uuid(value: str) -> bool: + """ + Determine whether a string can be parsed as a UUID. + + :param value: Candidate value. + :return: ``True`` if the value parses as a UUID, otherwise ``False``. + """ + try: + uuid.UUID(value) + return True + except Exception: + return False + + @staticmethod + def _is_valid_nhs_number(nhs_number: str) -> bool: + """ + Validate an NHS number. + + The intended logic is check-digit validation (mod 11), rejecting cases where the + computed check digit is 10. + + :param nhs_number: NHS number string. + :return: ``True`` if considered valid. + + .. note:: + This stub currently returns ``True`` for all values to keep unit test data + setup lightweight. Uncomment the implementation below if stricter validation + is desired. + """ + return True + + # digits = [int(c) for c in nhs_number] # NOSONAR S125 (May be wanted later) + # total = sum(digits[i] * (10 - i) for i in range(9)) # weights 10..2 + # remainder = total % 11 + # check = 11 - remainder + # if check == 11: + # check = 0 + # if check == 10: + # return False + # return digits[9] == check + + def _bad_request( + self, message: str, *, request_id: str | None, correlation_id: str | None + ) -> StubResponse: + """ + Build a 400 OperationOutcome response. + + :param message: Human-readable error message. + :param request_id: Optional request ID to echo back. + :param correlation_id: Optional correlation ID to echo back. + :return: A 400 :class:`StubResponse` containing an OperationOutcome. + """ + headers: dict[str, str] = {} + if request_id: + headers["X-Request-Id"] = request_id + if correlation_id: + headers["X-Correlation-Id"] = correlation_id + + return self._operation_outcome( + status_code=400, + headers=headers, + spine_code="INVALID_REQUEST", + display=message, + ) + + @staticmethod + def _operation_outcome( + *, status_code: int, headers: dict[str, str], spine_code: str, display: str + ) -> StubResponse: + """ + Construct an OperationOutcome response body. + + :param status_code: HTTP-like status code. + :param headers: Response headers. + :param spine_code: Spine error/warning code. + :param display: Human-readable display message. + :return: A :class:`StubResponse` containing an OperationOutcome JSON body. + """ + body = { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "value", + "details": { + "coding": [ + { + "system": "https://fhir.nhs.uk/R4/CodeSystem/Spine-ErrorOrWarningCode", + "version": "1", + "code": spine_code, + "display": display, + } + ] + }, + } + ], + } + return StubResponse(status_code=status_code, headers=dict(headers), json=body) diff --git a/infrastructure/environments/preview/main.tf b/infrastructure/environments/preview/main.tf index 334669dd..cfa32753 100644 --- a/infrastructure/environments/preview/main.tf +++ b/infrastructure/environments/preview/main.tf @@ -62,7 +62,7 @@ locals { ############################ resource "aws_lb_target_group" "branch" { - name = substr(replace(local.effective_host_name, ".", "-"), 0, 32) + name = trim(substr(replace(var.branch_name, ".", "-"), 0, 32), "-") port = var.container_port protocol = "HTTP" target_type = "ip" @@ -179,7 +179,7 @@ resource "aws_ecs_task_definition" "branch" { task_role_arn = aws_iam_role.task.arn runtime_platform { - cpu_architecture = "ARM64" + cpu_architecture = "X86_64" operating_system_family = "LINUX" } diff --git a/sonar-project.properties b/sonar-project.properties index 43ad5fd7..4ce10922 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -8,6 +8,6 @@ sonar.sourceEncoding=UTF-8 sonar.sources=.github,gateway-api,infrastructure,scripts,Makefile sonar.tests=tests,gateway-api/tests sonar.exclusions=docs/**,gateway-api/test-artefacts/**,gateway-api/coverage-html/**,gateway-api/tests/**,**/__pycache__/**,**/.venv/**,**/*.drawio -sonar.coverage.exclusions=**/tests/**,**/features/**,**/test_*.py +sonar.coverage.exclusions=**/tests/**,**/features/**,**/test_*.py,**/stubs/** # Set Python version for more precise analysis sonar.python.version=3.14