From cc6d5c34b5ca3eed047fd162cec273fb23aa0754 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 5 Feb 2026 12:55:27 +0000 Subject: [PATCH 01/27] [GPCAPIM-275]: Remove duplicate lines. --- gateway-api/src/gateway_api/test_app.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/gateway-api/src/gateway_api/test_app.py b/gateway-api/src/gateway_api/test_app.py index fdf77815..6e469151 100644 --- a/gateway-api/src/gateway_api/test_app.py +++ b/gateway-api/src/gateway_api/test_app.py @@ -16,9 +16,6 @@ if TYPE_CHECKING: from fhir.parameters import Parameters -if TYPE_CHECKING: - from fhir.parameters import Parameters - @pytest.fixture def client() -> Generator[FlaskClient[Flask], None, None]: From 4ebe273e91c327e3a51f98c16cda92f00cdb1749 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 5 Feb 2026 15:09:00 +0000 Subject: [PATCH 02/27] [GPCAPIM-275]: Import modules consistently. --- gateway-api/src/gateway_api/test_app.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/gateway-api/src/gateway_api/test_app.py b/gateway-api/src/gateway_api/test_app.py index 6e469151..718466fd 100644 --- a/gateway-api/src/gateway_api/test_app.py +++ b/gateway-api/src/gateway_api/test_app.py @@ -3,13 +3,15 @@ import json import os from collections.abc import Generator -from typing import TYPE_CHECKING +from datetime import datetime, timezone +from typing import TYPE_CHECKING, Any import pytest from flask import Flask from flask.testing import FlaskClient from gateway_api.app import app, get_app_host, get_app_port +from gateway_api.common.common import FlaskResponse from gateway_api.controller import Controller from gateway_api.get_structured_record.request import GetStructuredRecordRequest @@ -58,10 +60,6 @@ def test_get_structured_record_returns_200_with_bundle( valid_simple_request_payload: "Parameters", ) -> None: """Test that successful controller response is returned correctly.""" - from datetime import datetime, timezone - from typing import Any - - from gateway_api.common.common import FlaskResponse # Mock the controller to return a successful FlaskResponse with a Bundle mock_bundle_data: Any = { From 742f2fd823082f700f949fea71cdb436b01a8282 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 5 Feb 2026 15:22:33 +0000 Subject: [PATCH 03/27] [GPCAPIM-275]: Enable other test modules to use a valid Bundle. --- gateway-api/src/gateway_api/conftest.py | 28 +++++++++++++++++++ gateway-api/src/gateway_api/test_app.py | 37 ++++--------------------- 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/gateway-api/src/gateway_api/conftest.py b/gateway-api/src/gateway_api/conftest.py index 05307c86..b9866638 100644 --- a/gateway-api/src/gateway_api/conftest.py +++ b/gateway-api/src/gateway_api/conftest.py @@ -1,6 +1,9 @@ """Pytest configuration and shared fixtures for gateway API tests.""" +from datetime import datetime, timezone + import pytest +from fhir.bundle import Bundle from fhir.parameters import Parameters @@ -18,3 +21,28 @@ def valid_simple_request_payload() -> Parameters: }, ], } + + +@pytest.fixture +def valid_simple_response_payload() -> Bundle: + return { + "resourceType": "Bundle", + "id": "example-patient-bundle", + "type": "collection", + "timestamp": datetime.now(timezone.utc).isoformat(), + "entry": [ + { + "fullUrl": "http://example.com/Patient/9999999999", + "resource": { + "name": [{"family": "Alice", "given": ["Johnson"], "use": "Ally"}], + "gender": "female", + "birthDate": "1990-05-15", + "resourceType": "Patient", + "id": "9999999999", + "identifier": [ + {"value": "9999999999", "system": "urn:nhs:numbers"} + ], + }, + } + ], + } diff --git a/gateway-api/src/gateway_api/test_app.py b/gateway-api/src/gateway_api/test_app.py index 718466fd..97d2f46f 100644 --- a/gateway-api/src/gateway_api/test_app.py +++ b/gateway-api/src/gateway_api/test_app.py @@ -3,10 +3,10 @@ import json import os from collections.abc import Generator -from datetime import datetime, timezone -from typing import TYPE_CHECKING, Any import pytest +from fhir.bundle import Bundle +from fhir.parameters import Parameters from flask import Flask from flask.testing import FlaskClient @@ -15,9 +15,6 @@ from gateway_api.controller import Controller from gateway_api.get_structured_record.request import GetStructuredRecordRequest -if TYPE_CHECKING: - from fhir.parameters import Parameters - @pytest.fixture def client() -> Generator[FlaskClient[Flask], None, None]: @@ -57,42 +54,18 @@ def test_get_structured_record_returns_200_with_bundle( self, client: FlaskClient[Flask], monkeypatch: pytest.MonkeyPatch, - valid_simple_request_payload: "Parameters", + valid_simple_request_payload: Parameters, + valid_simple_response_payload: Bundle, ) -> None: """Test that successful controller response is returned correctly.""" - # Mock the controller to return a successful FlaskResponse with a Bundle - mock_bundle_data: Any = { - "resourceType": "Bundle", - "id": "example-patient-bundle", - "type": "collection", - "timestamp": datetime.now(timezone.utc).isoformat(), - "entry": [ - { - "fullUrl": "http://example.com/Patient/9999999999", - "resource": { - "name": [ - {"family": "Alice", "given": ["Johnson"], "use": "Ally"} - ], - "gender": "female", - "birthDate": "1990-05-15", - "resourceType": "Patient", - "id": "9999999999", - "identifier": [ - {"value": "9999999999", "system": "urn:nhs:numbers"} - ], - }, - } - ], - } - def mock_run( self: Controller, # noqa: ARG001 request: GetStructuredRecordRequest, # noqa: ARG001 ) -> FlaskResponse: return FlaskResponse( status_code=200, - data=json.dumps(mock_bundle_data), + data=json.dumps(valid_simple_response_payload), headers={"Content-Type": "application/fhir+json"}, ) From 41437aced651a600da455bb0b6ea764fb2ccca6a Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 5 Feb 2026 16:13:24 +0000 Subject: [PATCH 04/27] [GPCAPIM-275]: Reduce mocking complexity. Move common required headers to fixture. --- gateway-api/poetry.lock | 32 ++++++-- gateway-api/pyproject.toml | 2 + gateway-api/src/gateway_api/conftest.py | 8 ++ gateway-api/src/gateway_api/test_app.py | 101 +++++++++++------------- 4 files changed, 82 insertions(+), 61 deletions(-) diff --git a/gateway-api/poetry.lock b/gateway-api/poetry.lock index 88b054f5..b4c4e471 100644 --- a/gateway-api/poetry.lock +++ b/gateway-api/poetry.lock @@ -349,7 +349,7 @@ files = [ {file = "colorama-0.4.6-py2.py3-none-any.whl", hash = "sha256:4f1d9991f5acc0ca119f9d443620b77f9d6b33703e51011c16baf57afb285fc6"}, {file = "colorama-0.4.6.tar.gz", hash = "sha256:08695f5cb7ed6e0531a20572697297273c47b8cae5a63ffc6d6ed5c201be6e44"}, ] -markers = {main = "platform_system == \"Windows\""} +markers = {main = "platform_system == \"Windows\" or sys_platform == \"win32\""} [[package]] name = "coverage" @@ -684,7 +684,7 @@ version = "2.3.0" description = "brain-dead simple config-ini parsing" optional = false python-versions = ">=3.10" -groups = ["dev"] +groups = ["main", "dev"] files = [ {file = "iniconfig-2.3.0-py3-none-any.whl", hash = "sha256:f631c04d2c48c52b84d0d0549c99ff3859c98df65b3101406327ecc7d53fbf12"}, {file = "iniconfig-2.3.0.tar.gz", hash = "sha256:c76315c77db068650d49c5b56314774a7804df16fee4402c1f19d6d15d8c4730"}, @@ -1196,7 +1196,7 @@ version = "25.0" description = "Core utilities for Python packages" optional = false python-versions = ">=3.8" -groups = ["dev"] +groups = ["main", "dev"] files = [ {file = "packaging-25.0-py3-none-any.whl", hash = "sha256:29572ef2b1f17581046b3a2227d5c611fb25ec70ca1ba8554b24b0e69331a484"}, {file = "packaging-25.0.tar.gz", hash = "sha256:d443872c98d677bf60f6a1f2f8c1cb748e8fe762d2bf9d3148b5599295b0fc4f"}, @@ -1294,7 +1294,7 @@ version = "1.6.0" description = "plugin and hook calling mechanisms for python" optional = false python-versions = ">=3.9" -groups = ["dev"] +groups = ["main", "dev"] files = [ {file = "pluggy-1.6.0-py3-none-any.whl", hash = "sha256:e920276dd6813095e9377c0bc5566d94c932c33b27a3e3945d8389c374dd4746"}, {file = "pluggy-1.6.0.tar.gz", hash = "sha256:7dcc130b76258d33b90f61b658791dede3486c3e6bfb003ee5c9bfb396dd22f3"}, @@ -1455,7 +1455,7 @@ version = "2.19.2" description = "Pygments is a syntax highlighting package written in Python." optional = false python-versions = ">=3.8" -groups = ["dev"] +groups = ["main", "dev"] files = [ {file = "pygments-2.19.2-py3-none-any.whl", hash = "sha256:86540386c03d588bb81d44bc3928634ff26449851e99741617ecb9037ee5ec0b"}, {file = "pygments-2.19.2.tar.gz", hash = "sha256:636cb2477cec7f8952536970bc533bc43743542f70392ae026374600add5b887"}, @@ -1486,7 +1486,7 @@ version = "8.4.2" description = "pytest: simple powerful testing with Python" optional = false python-versions = ">=3.9" -groups = ["dev"] +groups = ["main", "dev"] files = [ {file = "pytest-8.4.2-py3-none-any.whl", hash = "sha256:872f880de3fc3a5bdc88a11b39c9710c3497a547cfa9320bc3c5e62fbf272e79"}, {file = "pytest-8.4.2.tar.gz", hash = "sha256:86c0d0b93306b961d58d62a4db4879f27fe25513d4b969df351abdddb3c30e01"}, @@ -1582,6 +1582,24 @@ pytest = ">=7.0.0" [package.extras] test = ["black (>=22.1.0)", "flake8 (>=4.0.1)", "pre-commit (>=2.17.0)", "tox (>=3.24.5)"] +[[package]] +name = "pytest-mock" +version = "3.15.1" +description = "Thin-wrapper around the mock package for easier use with pytest" +optional = false +python-versions = ">=3.9" +groups = ["main", "dev"] +files = [ + {file = "pytest_mock-3.15.1-py3-none-any.whl", hash = "sha256:0a25e2eb88fe5168d535041d09a4529a188176ae608a6d249ee65abc0949630d"}, + {file = "pytest_mock-3.15.1.tar.gz", hash = "sha256:1849a238f6f396da19762269de72cb1814ab44416fa73a8686deac10b0d87a0f"}, +] + +[package.dependencies] +pytest = ">=6.2.5" + +[package.extras] +dev = ["pre-commit", "pytest-asyncio", "tox"] + [[package]] name = "pytest-subtests" version = "0.14.2" @@ -2360,4 +2378,4 @@ propcache = ">=0.2.1" [metadata] lock-version = "2.1" python-versions = ">3.13,<4.0.0" -content-hash = "a452bd22e2386a3ff58b4c7a5ac2cb571de9e3d49a4fbc161ffd3aafa2a7bf44" +content-hash = "9646e1adfb86cc4e07b149bc1a93f1e32921f0cd50c57603cdb6fe907092ce7a" diff --git a/gateway-api/pyproject.toml b/gateway-api/pyproject.toml index 748ebd4f..b95d627c 100644 --- a/gateway-api/pyproject.toml +++ b/gateway-api/pyproject.toml @@ -13,6 +13,7 @@ clinical-data-common = { git = "https://github.com/NHSDigital/clinical-data-comm flask = "^3.1.2" types-flask = "^1.1.6" requests = "^2.32.5" +pytest-mock = "^3.15.1" [tool.poetry] packages = [{include = "gateway_api", from = "src"}, @@ -55,6 +56,7 @@ dev = [ "schemathesis>=4.4.1", "types-requests (>=2.32.4.20250913,<3.0.0.0)", "types-pyyaml (>=6.0.12.20250915,<7.0.0.0)", + "pytest-mock (>=3.15.1,<4.0.0)", ] [tool.mypy] diff --git a/gateway-api/src/gateway_api/conftest.py b/gateway-api/src/gateway_api/conftest.py index b9866638..d9db3b59 100644 --- a/gateway-api/src/gateway_api/conftest.py +++ b/gateway-api/src/gateway_api/conftest.py @@ -46,3 +46,11 @@ def valid_simple_response_payload() -> Bundle: } ], } + + +@pytest.fixture +def valid_headers() -> dict[str, str]: + return { + "Ssp-TraceID": "test-trace-id", + "ODS-from": "test-ods", + } diff --git a/gateway-api/src/gateway_api/test_app.py b/gateway-api/src/gateway_api/test_app.py index 97d2f46f..34b5e042 100644 --- a/gateway-api/src/gateway_api/test_app.py +++ b/gateway-api/src/gateway_api/test_app.py @@ -3,17 +3,17 @@ import json import os from collections.abc import Generator +from copy import copy import pytest from fhir.bundle import Bundle from fhir.parameters import Parameters from flask import Flask from flask.testing import FlaskClient +from pytest_mock import MockerFixture from gateway_api.app import app, get_app_host, get_app_port from gateway_api.common.common import FlaskResponse -from gateway_api.controller import Controller -from gateway_api.get_structured_record.request import GetStructuredRecordRequest @pytest.fixture @@ -53,25 +53,19 @@ class TestGetStructuredRecord: def test_get_structured_record_returns_200_with_bundle( self, client: FlaskClient[Flask], - monkeypatch: pytest.MonkeyPatch, + mocker: MockerFixture, valid_simple_request_payload: Parameters, valid_simple_response_payload: Bundle, ) -> None: """Test that successful controller response is returned correctly.""" - def mock_run( - self: Controller, # noqa: ARG001 - request: GetStructuredRecordRequest, # noqa: ARG001 - ) -> FlaskResponse: - return FlaskResponse( - status_code=200, - data=json.dumps(valid_simple_response_payload), - headers={"Content-Type": "application/fhir+json"}, - ) - - monkeypatch.setattr( - "gateway_api.controller.Controller.run", - mock_run, + postive_response = FlaskResponse( + status_code=200, + data=json.dumps(valid_simple_response_payload), + headers={"Content-Type": "application/fhir+json"}, + ) + mocker.patch( + "gateway_api.controller.Controller.run", return_value=postive_response ) response = client.post( @@ -96,73 +90,72 @@ def mock_run( assert data["entry"][0]["resource"]["id"] == "9999999999" assert data["entry"][0]["resource"]["identifier"][0]["value"] == "9999999999" - def test_get_structured_record_handles_exception( + def test_get_structured_record_returns_500_when_an_uncaught_exception_is_raised( self, client: FlaskClient[Flask], - monkeypatch: pytest.MonkeyPatch, + mocker: MockerFixture, valid_simple_request_payload: "Parameters", + valid_headers: dict[str, str], ) -> None: - """ - Test that exceptions during controller execution are caught and return 500. - """ - - # This is mocking the run method of the Controller - # and therefore self is a Controller - def mock_run_with_exception( - self: Controller, # noqa: ARG001 - request: GetStructuredRecordRequest, # noqa: ARG001 - ) -> None: - raise ValueError("Test exception") - - monkeypatch.setattr( - "gateway_api.controller.Controller.run", - mock_run_with_exception, + internal_error = ValueError("Test exception") + mocker.patch( + "gateway_api.controller.Controller.run", side_effect=internal_error ) response = client.post( "/patient/$gpc.getstructuredrecord", json=valid_simple_request_payload, - headers={ - "Ssp-TraceID": "test-trace-id", - "ODS-from": "test-ods", - }, + headers=valid_headers, ) assert response.status_code == 500 - def test_get_structured_record_handles_request_validation_error( + @pytest.mark.parametrize( + ("missing_header_key", "expected_message"), + [ + pytest.param( + "ODS-from", + b'Missing or empty required header "ODS-from"', + id="missing ODS code", + ), + pytest.param( + "Ssp-TraceID", + b'Missing or empty required header "Ssp-TraceID"', + id="missing trace id", + ), + ], + ) + def test_get_structured_record_request_returns_400_when_required_header_missing( self, client: FlaskClient[Flask], valid_simple_request_payload: "Parameters", + valid_headers: dict[str, str], + missing_header_key: str, + expected_message: bytes, ) -> None: """Test that RequestValidationError returns 400 with error message.""" - # Create a request missing the required ODS-from header + invalid_headers = copy(valid_headers) + del invalid_headers[missing_header_key] + response = client.post( "/patient/$gpc.getstructuredrecord", json=valid_simple_request_payload, - headers={ - "Ssp-TraceID": "test-trace-id", - # Missing "ODS-from" header to trigger RequestValidationError - }, + headers=invalid_headers, ) assert response.status_code == 400 assert "text/plain" in response.content_type - assert b'Missing or empty required header "ODS-from"' in response.data + assert expected_message in response.data - def test_get_structured_record_handles_unexpected_exception_during_init( - self, - client: FlaskClient[Flask], + def test_get_structured_record_handles_invalid_json_data( + self, client: FlaskClient[Flask], valid_headers: dict[str, str] ) -> None: """Test that unexpected exceptions during request init return 500.""" - # Send invalid JSON to trigger an exception during request processing + invalid_json = "invalid json data" + response = client.post( "/patient/$gpc.getstructuredrecord", - data="invalid json data", - headers={ - "Ssp-TraceID": "test-trace-id", - "ODS-from": "test-ods", - "Content-Type": "application/fhir+json", - }, + data=invalid_json, + headers=valid_headers, ) assert response.status_code == 500 From dd8c3180c25a0a400e1d0a3898a1bc06ec833163 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 5 Feb 2026 17:06:24 +0000 Subject: [PATCH 05/27] [GPCAPIM-275]: Resolve spurious Sonar issue --- gateway-api/src/gateway_api/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gateway-api/src/gateway_api/conftest.py b/gateway-api/src/gateway_api/conftest.py index d9db3b59..c7a16d40 100644 --- a/gateway-api/src/gateway_api/conftest.py +++ b/gateway-api/src/gateway_api/conftest.py @@ -32,7 +32,7 @@ def valid_simple_response_payload() -> Bundle: "timestamp": datetime.now(timezone.utc).isoformat(), "entry": [ { - "fullUrl": "http://example.com/Patient/9999999999", + "fullUrl": "https://example.com/Patient/9999999999", "resource": { "name": [{"family": "Alice", "given": ["Johnson"], "use": "Ally"}], "gender": "female", From 4f1534da5520886bd1bad5541079c8106040fb25 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 5 Feb 2026 17:57:17 +0000 Subject: [PATCH 06/27] [GPCAPIM-275]: Use single assertion in unit tests --- gateway-api/src/gateway_api/test_app.py | 166 +++++++++++++++--------- 1 file changed, 107 insertions(+), 59 deletions(-) diff --git a/gateway-api/src/gateway_api/test_app.py b/gateway-api/src/gateway_api/test_app.py index 34b5e042..9c4a2dcf 100644 --- a/gateway-api/src/gateway_api/test_app.py +++ b/gateway-api/src/gateway_api/test_app.py @@ -50,64 +50,53 @@ def test_get_app_port_raises_runtime_error_if_port_not_set(self) -> None: class TestGetStructuredRecord: - def test_get_structured_record_returns_200_with_bundle( + @pytest.mark.usefixtures("mock_positive_return_value_from_controller_run") + def test_valid_get_structured_record_request_returns_bundle( self, - client: FlaskClient[Flask], - mocker: MockerFixture, - valid_simple_request_payload: Parameters, - valid_simple_response_payload: Bundle, + get_structured_record_response: Flask, ) -> None: - """Test that successful controller response is returned correctly.""" - - postive_response = FlaskResponse( - status_code=200, - data=json.dumps(valid_simple_response_payload), - headers={"Content-Type": "application/fhir+json"}, - ) - mocker.patch( - "gateway_api.controller.Controller.run", return_value=postive_response - ) - - response = client.post( - "/patient/$gpc.getstructuredrecord", - json=valid_simple_request_payload, - headers={ - "Ssp-TraceID": "test-trace-id", - "ODS-from": "test-ods", - }, - ) - - assert response.status_code == 200 - data = response.get_json() - assert isinstance(data, dict) - assert data.get("resourceType") == "Bundle" - assert data.get("id") == "example-patient-bundle" - assert data.get("type") == "collection" - assert "entry" in data - assert isinstance(data["entry"], list) - assert len(data["entry"]) > 0 - assert data["entry"][0]["resource"]["resourceType"] == "Patient" - assert data["entry"][0]["resource"]["id"] == "9999999999" - assert data["entry"][0]["resource"]["identifier"][0]["value"] == "9999999999" + expected_body_wihtout_timestamp = { + "resourceType": "Bundle", + "id": "example-patient-bundle", + "type": "collection", + "entry": [ + { + "fullUrl": "https://example.com/Patient/9999999999", + "resource": { + "name": [ + {"family": "Alice", "given": ["Johnson"], "use": "Ally"} + ], + "gender": "female", + "birthDate": "1990-05-15", + "resourceType": "Patient", + "id": "9999999999", + "identifier": [ + {"value": "9999999999", "system": "urn:nhs:numbers"} + ], + }, + } + ], + } + + actual_body_without_timestamp = get_structured_record_response.get_json() + del actual_body_without_timestamp["timestamp"] + + assert actual_body_without_timestamp == expected_body_wihtout_timestamp + + @pytest.mark.usefixtures("mock_positive_return_value_from_controller_run") + def test_valid_get_structured_record_request_returns_200( + self, + get_structured_record_response: Flask, + ) -> None: + assert get_structured_record_response.status_code == 200 + @pytest.mark.usefixtures("mock_raise_error_from_controller_run") def test_get_structured_record_returns_500_when_an_uncaught_exception_is_raised( self, - client: FlaskClient[Flask], - mocker: MockerFixture, - valid_simple_request_payload: "Parameters", - valid_headers: dict[str, str], + get_structured_record_response: Flask, ) -> None: - internal_error = ValueError("Test exception") - mocker.patch( - "gateway_api.controller.Controller.run", side_effect=internal_error - ) - - response = client.post( - "/patient/$gpc.getstructuredrecord", - json=valid_simple_request_payload, - headers=valid_headers, - ) - assert response.status_code == 500 + actual_status_code = get_structured_record_response.status_code + assert actual_status_code == 500 @pytest.mark.parametrize( ("missing_header_key", "expected_message"), @@ -132,7 +121,6 @@ def test_get_structured_record_request_returns_400_when_required_header_missing( missing_header_key: str, expected_message: bytes, ) -> None: - """Test that RequestValidationError returns 400 with error message.""" invalid_headers = copy(valid_headers) del invalid_headers[missing_header_key] @@ -146,10 +134,47 @@ def test_get_structured_record_request_returns_400_when_required_header_missing( assert "text/plain" in response.content_type assert expected_message in response.data - def test_get_structured_record_handles_invalid_json_data( - self, client: FlaskClient[Flask], valid_headers: dict[str, str] + def test_get_structured_record_returns_500_when_invalid_json_sent( + self, get_structured_record_response_using_invalid_json_body: Flask + ) -> None: + assert get_structured_record_response_using_invalid_json_body.status_code == 500 + + def test_get_structured_record_returns_content_type_textplain_for_invalid_json_sent( + self, get_structured_record_response_using_invalid_json_body: Flask ) -> None: - """Test that unexpected exceptions during request init return 500.""" + assert ( + "text/plain" + in get_structured_record_response_using_invalid_json_body.content_type + ) + + def test_get_structured_record_returns_intenral_server_error_when_invalid_json_sent( + self, get_structured_record_response_using_invalid_json_body: Flask + ) -> None: + assert ( + b"Internal Server Error:" + in get_structured_record_response_using_invalid_json_body.data + ) + + @staticmethod + @pytest.fixture + def get_structured_record_response( + client: FlaskClient[Flask], + valid_headers: dict[str, str], + valid_simple_request_payload: Parameters, + ) -> Flask: + response = client.post( + "/patient/$gpc.getstructuredrecord", + json=valid_simple_request_payload, + headers=valid_headers, + ) + return response + + @staticmethod + @pytest.fixture + def get_structured_record_response_using_invalid_json_body( + client: FlaskClient[Flask], + valid_headers: dict[str, str], + ) -> Flask: invalid_json = "invalid json data" response = client.post( @@ -157,10 +182,33 @@ def test_get_structured_record_handles_invalid_json_data( data=invalid_json, headers=valid_headers, ) + return response - assert response.status_code == 500 - assert "text/plain" in response.content_type - assert b"Internal Server Error:" in response.data + @staticmethod + @pytest.fixture + def mock_positive_return_value_from_controller_run( + mocker: MockerFixture, + valid_headers: dict[str, str], + valid_simple_response_payload: Bundle, + ) -> None: + postive_response = FlaskResponse( + status_code=200, + data=json.dumps(valid_simple_response_payload), + headers=valid_headers, + ) + mocker.patch( + "gateway_api.controller.Controller.run", return_value=postive_response + ) + + @staticmethod + @pytest.fixture + def mock_raise_error_from_controller_run( + mocker: MockerFixture, + ) -> None: + internal_error = ValueError("Test exception") + mocker.patch( + "gateway_api.controller.Controller.run", side_effect=internal_error + ) class TestHealthCheck: From 3580a2b3565344d78abae5a9645f39d84753554b Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 5 Feb 2026 22:44:50 +0000 Subject: [PATCH 07/27] [GPCAPIM-275]: Content-type header is a required header. --- gateway-api/src/gateway_api/conftest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/gateway-api/src/gateway_api/conftest.py b/gateway-api/src/gateway_api/conftest.py index c7a16d40..b0b24f14 100644 --- a/gateway-api/src/gateway_api/conftest.py +++ b/gateway-api/src/gateway_api/conftest.py @@ -53,4 +53,5 @@ def valid_headers() -> dict[str, str]: return { "Ssp-TraceID": "test-trace-id", "ODS-from": "test-ods", + "Content-type": "application/fhir+json", } From 22adc40e2ceef9fdeae64bc8b3dbf932b33e8a93 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 5 Feb 2026 22:48:13 +0000 Subject: [PATCH 08/27] [GPCAPIM-275]: Use static datetimes for unit tests. --- gateway-api/src/gateway_api/conftest.py | 4 +--- gateway-api/src/gateway_api/test_app.py | 9 ++++----- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/gateway-api/src/gateway_api/conftest.py b/gateway-api/src/gateway_api/conftest.py index b0b24f14..ed88f984 100644 --- a/gateway-api/src/gateway_api/conftest.py +++ b/gateway-api/src/gateway_api/conftest.py @@ -1,7 +1,5 @@ """Pytest configuration and shared fixtures for gateway API tests.""" -from datetime import datetime, timezone - import pytest from fhir.bundle import Bundle from fhir.parameters import Parameters @@ -29,7 +27,7 @@ def valid_simple_response_payload() -> Bundle: "resourceType": "Bundle", "id": "example-patient-bundle", "type": "collection", - "timestamp": datetime.now(timezone.utc).isoformat(), + "timestamp": "2026-02-05T22:45:42.766330+00:00", "entry": [ { "fullUrl": "https://example.com/Patient/9999999999", diff --git a/gateway-api/src/gateway_api/test_app.py b/gateway-api/src/gateway_api/test_app.py index 9c4a2dcf..b766f826 100644 --- a/gateway-api/src/gateway_api/test_app.py +++ b/gateway-api/src/gateway_api/test_app.py @@ -55,10 +55,11 @@ def test_valid_get_structured_record_request_returns_bundle( self, get_structured_record_response: Flask, ) -> None: - expected_body_wihtout_timestamp = { + expected_body = { "resourceType": "Bundle", "id": "example-patient-bundle", "type": "collection", + "timestamp": "2026-02-05T22:45:42.766330+00:00", "entry": [ { "fullUrl": "https://example.com/Patient/9999999999", @@ -78,10 +79,8 @@ def test_valid_get_structured_record_request_returns_bundle( ], } - actual_body_without_timestamp = get_structured_record_response.get_json() - del actual_body_without_timestamp["timestamp"] - - assert actual_body_without_timestamp == expected_body_wihtout_timestamp + actual_body = get_structured_record_response.get_json() + assert actual_body == expected_body @pytest.mark.usefixtures("mock_positive_return_value_from_controller_run") def test_valid_get_structured_record_request_returns_200( From 4b9dbecedfd2538c23ad0d776578de3df759e937 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Mon, 9 Feb 2026 10:35:41 +0000 Subject: [PATCH 09/27] [GPCAPIM-275]: Move towards using a common error class --- gateway-api/src/gateway_api/app.py | 3 ++ gateway-api/src/gateway_api/common/error.py | 41 +++++++++++++++++++ .../get_structured_record/request.py | 8 +++- gateway-api/src/gateway_api/test_app.py | 30 ++++++++++---- 4 files changed, 72 insertions(+), 10 deletions(-) create mode 100644 gateway-api/src/gateway_api/common/error.py diff --git a/gateway-api/src/gateway_api/app.py b/gateway-api/src/gateway_api/app.py index 265601e5..362619ca 100644 --- a/gateway-api/src/gateway_api/app.py +++ b/gateway-api/src/gateway_api/app.py @@ -4,6 +4,7 @@ from flask import Flask, request from flask.wrappers import Response +from gateway_api.common.error import Error from gateway_api.controller import Controller from gateway_api.get_structured_record import ( GetStructuredRecordRequest, @@ -45,6 +46,8 @@ def get_structured_record() -> Response: content_type="text/plain", ) return response + except Error as error: + return error.build_response() except Exception as e: response = Response( response=f"Internal Server Error: {e}", diff --git a/gateway-api/src/gateway_api/common/error.py b/gateway-api/src/gateway_api/common/error.py new file mode 100644 index 00000000..f7b4b5d8 --- /dev/null +++ b/gateway-api/src/gateway_api/common/error.py @@ -0,0 +1,41 @@ +import json +from dataclasses import dataclass +from http.client import BAD_REQUEST +from typing import TYPE_CHECKING + +from flask import Response + +if TYPE_CHECKING: + from fhir.operation_outcome import OperationOutcome + + +@dataclass +class Error(Exception): + message: str = "Internal Server Error" + status_code: int = 500 + severity: str = "error" + fhir_error_code: str = "exception" + + def build_response(self) -> Response: + operation_outcome: OperationOutcome = { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": self.severity, + "code": self.fhir_error_code, + "diagnostics": self.message, + } + ], + } + response = Response( + response=json.dumps(operation_outcome), + status=self.status_code, + content_type="application/fhir+json", + ) + return response + + +class CDGAPIErrors: + INVALID_REQUEST_JSON = Error( + "Invalid JSON body sent in request", status_code=BAD_REQUEST + ) diff --git a/gateway-api/src/gateway_api/get_structured_record/request.py b/gateway-api/src/gateway_api/get_structured_record/request.py index c4279272..b12e040f 100644 --- a/gateway-api/src/gateway_api/get_structured_record/request.py +++ b/gateway-api/src/gateway_api/get_structured_record/request.py @@ -4,8 +4,10 @@ from fhir import OperationOutcome, Parameters from fhir.operation_outcome import OperationOutcomeIssue from flask.wrappers import Request, Response +from werkzeug.exceptions import BadRequest from gateway_api.common.common import FlaskResponse +from gateway_api.common.error import CDGAPIErrors if TYPE_CHECKING: from fhir.bundle import Bundle @@ -23,7 +25,11 @@ class GetStructuredRecordRequest: def __init__(self, request: Request) -> None: self._http_request = request self._headers = request.headers - self._request_body: Parameters = request.get_json() + try: + self._request_body: Parameters = request.get_json() + except BadRequest as error: + raise CDGAPIErrors.INVALID_REQUEST_JSON from error + self._response_body: Bundle | OperationOutcome | None = None self._status_code: int | None = None diff --git a/gateway-api/src/gateway_api/test_app.py b/gateway-api/src/gateway_api/test_app.py index b766f826..8e7d66fa 100644 --- a/gateway-api/src/gateway_api/test_app.py +++ b/gateway-api/src/gateway_api/test_app.py @@ -4,6 +4,7 @@ import os from collections.abc import Generator from copy import copy +from typing import TYPE_CHECKING import pytest from fhir.bundle import Bundle @@ -15,6 +16,9 @@ from gateway_api.app import app, get_app_host, get_app_port from gateway_api.common.common import FlaskResponse +if TYPE_CHECKING: + from fhir.operation_outcome import OperationOutcome + @pytest.fixture def client() -> Generator[FlaskClient[Flask], None, None]: @@ -133,26 +137,34 @@ def test_get_structured_record_request_returns_400_when_required_header_missing( assert "text/plain" in response.content_type assert expected_message in response.data - def test_get_structured_record_returns_500_when_invalid_json_sent( + def test_get_structured_record_returns_400_when_invalid_json_sent( self, get_structured_record_response_using_invalid_json_body: Flask ) -> None: - assert get_structured_record_response_using_invalid_json_body.status_code == 500 + assert get_structured_record_response_using_invalid_json_body.status_code == 400 - def test_get_structured_record_returns_content_type_textplain_for_invalid_json_sent( + def test_get_structured_record_returns_content_type_fhir_json_for_invalid_json_sent( self, get_structured_record_response_using_invalid_json_body: Flask ) -> None: assert ( - "text/plain" + "application/fhir+json" in get_structured_record_response_using_invalid_json_body.content_type ) - def test_get_structured_record_returns_intenral_server_error_when_invalid_json_sent( + def test_get_structured_record_returns_internal_server_error_when_invalid_json_sent( self, get_structured_record_response_using_invalid_json_body: Flask ) -> None: - assert ( - b"Internal Server Error:" - in get_structured_record_response_using_invalid_json_body.data - ) + expected: OperationOutcome = { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "exception", + "diagnostics": "Invalid JSON body sent in request", + } + ], + } + actual = get_structured_record_response_using_invalid_json_body.get_json() + assert actual == expected @staticmethod @pytest.fixture From 80a90eda41b8bee52adf057f7ceb6910d1efa05f Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Mon, 9 Feb 2026 10:58:24 +0000 Subject: [PATCH 10/27] [GPCAPIM-275]: Move towards using a common error class. --- gateway-api/src/gateway_api/common/error.py | 7 +++++++ .../get_structured_record/request.py | 7 ++----- .../get_structured_record/test_request.py | 14 +++++-------- gateway-api/src/gateway_api/test_app.py | 20 ++++++++++++++----- 4 files changed, 29 insertions(+), 19 deletions(-) diff --git a/gateway-api/src/gateway_api/common/error.py b/gateway-api/src/gateway_api/common/error.py index f7b4b5d8..f7288f60 100644 --- a/gateway-api/src/gateway_api/common/error.py +++ b/gateway-api/src/gateway_api/common/error.py @@ -39,3 +39,10 @@ class CDGAPIErrors: INVALID_REQUEST_JSON = Error( "Invalid JSON body sent in request", status_code=BAD_REQUEST ) + + MISSING_TRACE_ID = Error( + 'Missing or empty required header "Ssp-TraceID"', status_code=BAD_REQUEST + ) + MISSING_ODS_CODE = Error( + 'Missing or empty required header "ODS-from"', status_code=BAD_REQUEST + ) diff --git a/gateway-api/src/gateway_api/get_structured_record/request.py b/gateway-api/src/gateway_api/get_structured_record/request.py index b12e040f..a9a1b4e1 100644 --- a/gateway-api/src/gateway_api/get_structured_record/request.py +++ b/gateway-api/src/gateway_api/get_structured_record/request.py @@ -33,7 +33,6 @@ def __init__(self, request: Request) -> None: self._response_body: Bundle | OperationOutcome | None = None self._status_code: int | None = None - # Validate required headers self._validate_headers() @property @@ -62,13 +61,11 @@ def _validate_headers(self) -> None: """ trace_id = self._headers.get("Ssp-TraceID", "").strip() if not trace_id: - raise RequestValidationError( - 'Missing or empty required header "Ssp-TraceID"' - ) + raise CDGAPIErrors.MISSING_TRACE_ID ods_from = self._headers.get("ODS-from", "").strip() if not ods_from: - raise RequestValidationError('Missing or empty required header "ODS-from"') + raise CDGAPIErrors.MISSING_ODS_CODE def build_response(self) -> Response: return Response( diff --git a/gateway-api/src/gateway_api/get_structured_record/test_request.py b/gateway-api/src/gateway_api/get_structured_record/test_request.py index 6fa5f9a2..944c1628 100644 --- a/gateway-api/src/gateway_api/get_structured_record/test_request.py +++ b/gateway-api/src/gateway_api/get_structured_record/test_request.py @@ -7,7 +7,7 @@ from werkzeug.test import EnvironBuilder from gateway_api.common.common import FlaskResponse -from gateway_api.get_structured_record import RequestValidationError +from gateway_api.common.error import Error from gateway_api.get_structured_record.request import GetStructuredRecordRequest if TYPE_CHECKING: @@ -79,9 +79,7 @@ def test_raises_value_error_when_ods_from_header_is_missing( } mock_request = create_mock_request(headers, valid_simple_request_payload) - with pytest.raises( - RequestValidationError, match='Missing or empty required header "ODS-from"' - ): + with pytest.raises(Error, match='Missing or empty required header "ODS-from"'): GetStructuredRecordRequest(request=mock_request) def test_raises_value_error_when_ods_from_header_is_whitespace( @@ -96,9 +94,7 @@ def test_raises_value_error_when_ods_from_header_is_whitespace( } mock_request = create_mock_request(headers, valid_simple_request_payload) - with pytest.raises( - RequestValidationError, match='Missing or empty required header "ODS-from"' - ): + with pytest.raises(Error, match='Missing or empty required header "ODS-from"'): GetStructuredRecordRequest(request=mock_request) def test_raises_value_error_when_trace_id_header_is_missing( @@ -111,7 +107,7 @@ def test_raises_value_error_when_trace_id_header_is_missing( mock_request = create_mock_request(headers, valid_simple_request_payload) with pytest.raises( - RequestValidationError, + Error, match='Missing or empty required header "Ssp-TraceID"', ): GetStructuredRecordRequest(request=mock_request) @@ -129,7 +125,7 @@ def test_raises_value_error_when_trace_id_header_is_whitespace( mock_request = create_mock_request(headers, valid_simple_request_payload) with pytest.raises( - RequestValidationError, + Error, match='Missing or empty required header "Ssp-TraceID"', ): GetStructuredRecordRequest(request=mock_request) diff --git a/gateway-api/src/gateway_api/test_app.py b/gateway-api/src/gateway_api/test_app.py index 8e7d66fa..42c486f9 100644 --- a/gateway-api/src/gateway_api/test_app.py +++ b/gateway-api/src/gateway_api/test_app.py @@ -106,12 +106,12 @@ def test_get_structured_record_returns_500_when_an_uncaught_exception_is_raised( [ pytest.param( "ODS-from", - b'Missing or empty required header "ODS-from"', + 'Missing or empty required header "ODS-from"', id="missing ODS code", ), pytest.param( "Ssp-TraceID", - b'Missing or empty required header "Ssp-TraceID"', + 'Missing or empty required header "Ssp-TraceID"', id="missing trace id", ), ], @@ -122,7 +122,7 @@ def test_get_structured_record_request_returns_400_when_required_header_missing( valid_simple_request_payload: "Parameters", valid_headers: dict[str, str], missing_header_key: str, - expected_message: bytes, + expected_message: str, ) -> None: invalid_headers = copy(valid_headers) del invalid_headers[missing_header_key] @@ -134,8 +134,18 @@ def test_get_structured_record_request_returns_400_when_required_header_missing( ) assert response.status_code == 400 - assert "text/plain" in response.content_type - assert expected_message in response.data + assert "application/fhir+json" in response.content_type + expected_body: OperationOutcome = { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "exception", + "diagnostics": expected_message, + } + ], + } + assert expected_body == response.get_json() def test_get_structured_record_returns_400_when_invalid_json_sent( self, get_structured_record_response_using_invalid_json_body: Flask From 72af28cf4dbf887f86d54cb253fd67a28e1d255e Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Mon, 9 Feb 2026 12:11:04 +0000 Subject: [PATCH 11/27] [GPCAPIM-275]: Rework unit tests to only assert once in a test method --- gateway-api/src/gateway_api/test_app.py | 80 ++++++++++++++++++------- 1 file changed, 60 insertions(+), 20 deletions(-) diff --git a/gateway-api/src/gateway_api/test_app.py b/gateway-api/src/gateway_api/test_app.py index 42c486f9..b3743e6b 100644 --- a/gateway-api/src/gateway_api/test_app.py +++ b/gateway-api/src/gateway_api/test_app.py @@ -101,40 +101,62 @@ def test_get_structured_record_returns_500_when_an_uncaught_exception_is_raised( actual_status_code = get_structured_record_response.status_code assert actual_status_code == 500 + @staticmethod + @pytest.fixture + def missing_headers( + request: pytest.FixtureRequest, valid_headers: dict[str, str] + ) -> dict[str, str]: + invalid_headers = copy(valid_headers) + del invalid_headers[request.param] + return invalid_headers + + @pytest.mark.parametrize( + "missing_headers", + ["ODS-from", "Ssp-TraceID"], + indirect=True, + ) + @pytest.mark.usefixtures("missing_headers") + def test_get_structured_record_returns_400_when_required_header_missing( + self, + get_structured_record_response_from_missing_header: Flask, + ) -> None: + + assert get_structured_record_response_from_missing_header.status_code == 400 + @pytest.mark.parametrize( - ("missing_header_key", "expected_message"), + "missing_headers", + ["ODS-from", "Ssp-TraceID"], + indirect=True, + ) + @pytest.mark.usefixtures("missing_headers") + def test_get_structured_record_returns_fhir_content_when_missing_header( + self, + get_structured_record_response_from_missing_header: Flask, + ) -> None: + assert ( + "application/fhir+json" + in get_structured_record_response_from_missing_header.content_type + ) + + @pytest.mark.parametrize( + ("missing_headers", "expected_message"), [ pytest.param( "ODS-from", 'Missing or empty required header "ODS-from"', - id="missing ODS code", ), pytest.param( "Ssp-TraceID", 'Missing or empty required header "Ssp-TraceID"', - id="missing trace id", ), ], + indirect=["missing_headers"], ) - def test_get_structured_record_request_returns_400_when_required_header_missing( + def test_get_structured_record_returns_operation_outcome_when_missing_header( self, - client: FlaskClient[Flask], - valid_simple_request_payload: "Parameters", - valid_headers: dict[str, str], - missing_header_key: str, + get_structured_record_response_from_missing_header: Flask, expected_message: str, ) -> None: - invalid_headers = copy(valid_headers) - del invalid_headers[missing_header_key] - - response = client.post( - "/patient/$gpc.getstructuredrecord", - json=valid_simple_request_payload, - headers=invalid_headers, - ) - - assert response.status_code == 400 - assert "application/fhir+json" in response.content_type expected_body: OperationOutcome = { "resourceType": "OperationOutcome", "issue": [ @@ -145,7 +167,10 @@ def test_get_structured_record_request_returns_400_when_required_header_missing( } ], } - assert expected_body == response.get_json() + assert ( + expected_body + == get_structured_record_response_from_missing_header.get_json() + ) def test_get_structured_record_returns_400_when_invalid_json_sent( self, get_structured_record_response_using_invalid_json_body: Flask @@ -190,6 +215,21 @@ def get_structured_record_response( ) return response + @staticmethod + @pytest.fixture + def get_structured_record_response_from_missing_header( + client: FlaskClient[Flask], + missing_headers: dict[str, str], + valid_simple_request_payload: Parameters, + ) -> Flask: + + response = client.post( + "/patient/$gpc.getstructuredrecord", + data=json.dumps(valid_simple_request_payload), + headers=missing_headers, + ) + return response + @staticmethod @pytest.fixture def get_structured_record_response_using_invalid_json_body( From a41f24157e86a2ce747fe62db1c7a02489d37e3b Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Mon, 9 Feb 2026 12:16:29 +0000 Subject: [PATCH 12/27] [GPCAPIM-275]: Have a "catch all" error within the common error class --- gateway-api/src/gateway_api/app.py | 18 +++--------------- gateway-api/src/gateway_api/common/error.py | 5 ++++- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/gateway-api/src/gateway_api/app.py b/gateway-api/src/gateway_api/app.py index 362619ca..dc7d748a 100644 --- a/gateway-api/src/gateway_api/app.py +++ b/gateway-api/src/gateway_api/app.py @@ -4,11 +4,10 @@ from flask import Flask, request from flask.wrappers import Response -from gateway_api.common.error import Error +from gateway_api.common.error import CDGAPIErrors, Error from gateway_api.controller import Controller from gateway_api.get_structured_record import ( GetStructuredRecordRequest, - RequestValidationError, ) app = Flask(__name__) @@ -39,21 +38,10 @@ def get_app_port() -> int: def get_structured_record() -> Response: try: get_structured_record_request = GetStructuredRecordRequest(request) - except RequestValidationError as e: - response = Response( - response=str(e), - status=400, - content_type="text/plain", - ) - return response except Error as error: return error.build_response() - except Exception as e: - response = Response( - response=f"Internal Server Error: {e}", - status=500, - content_type="text/plain", - ) + except Exception: + response = CDGAPIErrors.GENERIC_ERROR.build_response() return response try: diff --git a/gateway-api/src/gateway_api/common/error.py b/gateway-api/src/gateway_api/common/error.py index f7288f60..548a1b58 100644 --- a/gateway-api/src/gateway_api/common/error.py +++ b/gateway-api/src/gateway_api/common/error.py @@ -11,7 +11,7 @@ @dataclass class Error(Exception): - message: str = "Internal Server Error" + message: str status_code: int = 500 severity: str = "error" fhir_error_code: str = "exception" @@ -36,6 +36,8 @@ def build_response(self) -> Response: class CDGAPIErrors: + GENERIC_ERROR = Error("Internal Server Error") + INVALID_REQUEST_JSON = Error( "Invalid JSON body sent in request", status_code=BAD_REQUEST ) @@ -43,6 +45,7 @@ class CDGAPIErrors: MISSING_TRACE_ID = Error( 'Missing or empty required header "Ssp-TraceID"', status_code=BAD_REQUEST ) + MISSING_ODS_CODE = Error( 'Missing or empty required header "ODS-from"', status_code=BAD_REQUEST ) From 701038b773fb7d7dd90262e3aacdce88b8b01a6e Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Mon, 9 Feb 2026 12:23:20 +0000 Subject: [PATCH 13/27] [GPCAPIM-275]: Run request handling all in one try-except as exceptions will/should bubble up under the same Error class --- gateway-api/src/gateway_api/app.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/gateway-api/src/gateway_api/app.py b/gateway-api/src/gateway_api/app.py index dc7d748a..ed4f63f6 100644 --- a/gateway-api/src/gateway_api/app.py +++ b/gateway-api/src/gateway_api/app.py @@ -38,19 +38,15 @@ def get_app_port() -> int: def get_structured_record() -> Response: try: get_structured_record_request = GetStructuredRecordRequest(request) + controller = Controller() + flask_response = controller.run(request=get_structured_record_request) + get_structured_record_request.set_response_from_flaskresponse(flask_response) except Error as error: return error.build_response() except Exception: response = CDGAPIErrors.GENERIC_ERROR.build_response() return response - try: - controller = Controller() - flask_response = controller.run(request=get_structured_record_request) - get_structured_record_request.set_response_from_flaskresponse(flask_response) - except Exception as e: - get_structured_record_request.set_negative_response(str(e)) - return get_structured_record_request.build_response() From 5e55b85858cc78ce69e8bc48ea11214e4f3d9a47 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Mon, 9 Feb 2026 12:35:45 +0000 Subject: [PATCH 14/27] [GPCAPIM-275]: Run request handling all in one try-except as exceptions will/should bubble up under the same Error class --- gateway-api/src/gateway_api/app.py | 9 ++++++--- gateway-api/src/gateway_api/common/error.py | 3 +++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/gateway-api/src/gateway_api/app.py b/gateway-api/src/gateway_api/app.py index ed4f63f6..350a5ba0 100644 --- a/gateway-api/src/gateway_api/app.py +++ b/gateway-api/src/gateway_api/app.py @@ -41,10 +41,13 @@ def get_structured_record() -> Response: controller = Controller() flask_response = controller.run(request=get_structured_record_request) get_structured_record_request.set_response_from_flaskresponse(flask_response) - except Error as error: - return error.build_response() + except Error as e: + e.log() + return e.build_response() except Exception: - response = CDGAPIErrors.GENERIC_ERROR.build_response() + error = CDGAPIErrors.GENERIC_ERROR + error.log() + response = error.build_response() return response return get_structured_record_request.build_response() diff --git a/gateway-api/src/gateway_api/common/error.py b/gateway-api/src/gateway_api/common/error.py index 548a1b58..597b62cb 100644 --- a/gateway-api/src/gateway_api/common/error.py +++ b/gateway-api/src/gateway_api/common/error.py @@ -34,6 +34,9 @@ def build_response(self) -> Response: ) return response + def log(self) -> None: + print(self) + class CDGAPIErrors: GENERIC_ERROR = Error("Internal Server Error") From 5d924a8edd5f46393571b8e9efd23c91ff2b0ba7 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Mon, 9 Feb 2026 16:05:13 +0000 Subject: [PATCH 15/27] [GPCAPIM-275]: Enable additional details to be passed to errors. --- gateway-api/src/gateway_api/app.py | 6 +-- gateway-api/src/gateway_api/common/error.py | 43 +++++++++++------- gateway-api/src/gateway_api/controller.py | 7 ++- .../get_structured_record/request.py | 8 ++-- .../get_structured_record/test_request.py | 14 +++--- .../src/gateway_api/test_controller.py | 45 +++++-------------- 6 files changed, 56 insertions(+), 67 deletions(-) diff --git a/gateway-api/src/gateway_api/app.py b/gateway-api/src/gateway_api/app.py index 350a5ba0..c666627e 100644 --- a/gateway-api/src/gateway_api/app.py +++ b/gateway-api/src/gateway_api/app.py @@ -4,7 +4,7 @@ from flask import Flask, request from flask.wrappers import Response -from gateway_api.common.error import CDGAPIErrors, Error +from gateway_api.common.error import BaseError from gateway_api.controller import Controller from gateway_api.get_structured_record import ( GetStructuredRecordRequest, @@ -41,11 +41,11 @@ def get_structured_record() -> Response: controller = Controller() flask_response = controller.run(request=get_structured_record_request) get_structured_record_request.set_response_from_flaskresponse(flask_response) - except Error as e: + except BaseError as e: e.log() return e.build_response() except Exception: - error = CDGAPIErrors.GENERIC_ERROR + error = BaseError() error.log() response = error.build_response() return response diff --git a/gateway-api/src/gateway_api/common/error.py b/gateway-api/src/gateway_api/common/error.py index 597b62cb..2407e2f8 100644 --- a/gateway-api/src/gateway_api/common/error.py +++ b/gateway-api/src/gateway_api/common/error.py @@ -1,6 +1,6 @@ import json from dataclasses import dataclass -from http.client import BAD_REQUEST +from http.client import INTERNAL_SERVER_ERROR from typing import TYPE_CHECKING from flask import Response @@ -10,11 +10,14 @@ @dataclass -class Error(Exception): - message: str - status_code: int = 500 +class BaseError(Exception): + _message = "Internal Server Error" + status_code: int = INTERNAL_SERVER_ERROR severity: str = "error" - fhir_error_code: str = "exception" + error_code: str = "exception" + + def __init__(self, **additional_details: str): + self.additional_details = additional_details def build_response(self) -> Response: operation_outcome: OperationOutcome = { @@ -22,7 +25,7 @@ def build_response(self) -> Response: "issue": [ { "severity": self.severity, - "code": self.fhir_error_code, + "code": self.error_code, "diagnostics": self.message, } ], @@ -37,18 +40,24 @@ def build_response(self) -> Response: def log(self) -> None: print(self) + @property + def message(self) -> str: + return self._message.format(**self.additional_details) + + def __str__(self) -> str: + return self.message + + +class NoPatientFound(BaseError): + _message = "No PDS patient found for NHS number {nhs_number}" + status_code = 400 -class CDGAPIErrors: - GENERIC_ERROR = Error("Internal Server Error") - INVALID_REQUEST_JSON = Error( - "Invalid JSON body sent in request", status_code=BAD_REQUEST - ) +class InvalidRequestJSON(BaseError): + _message = "Invalid JSON body sent in request" + status_code = 400 - MISSING_TRACE_ID = Error( - 'Missing or empty required header "Ssp-TraceID"', status_code=BAD_REQUEST - ) - MISSING_ODS_CODE = Error( - 'Missing or empty required header "ODS-from"', status_code=BAD_REQUEST - ) +class MissingOrEmptyHeader(BaseError): + _message = 'Missing or empty required header "{header}"' + status_code = 400 diff --git a/gateway-api/src/gateway_api/controller.py b/gateway-api/src/gateway_api/controller.py index 4a17d08c..43d2db1c 100644 --- a/gateway-api/src/gateway_api/controller.py +++ b/gateway-api/src/gateway_api/controller.py @@ -7,6 +7,7 @@ import json from typing import TYPE_CHECKING +from gateway_api.common.error import NoPatientFound from gateway_api.provider_request import GpProviderClient if TYPE_CHECKING: @@ -224,10 +225,8 @@ def _get_pds_details( ) if pds_result is None: - raise RequestError( - status_code=404, - message=f"No PDS patient found for NHS number {nhs_number}", - ) + error = NoPatientFound(nhs_number=nhs_number) + raise error if pds_result.gp_ods_code: provider_ods_code = pds_result.gp_ods_code diff --git a/gateway-api/src/gateway_api/get_structured_record/request.py b/gateway-api/src/gateway_api/get_structured_record/request.py index a9a1b4e1..ffd8a302 100644 --- a/gateway-api/src/gateway_api/get_structured_record/request.py +++ b/gateway-api/src/gateway_api/get_structured_record/request.py @@ -7,7 +7,7 @@ from werkzeug.exceptions import BadRequest from gateway_api.common.common import FlaskResponse -from gateway_api.common.error import CDGAPIErrors +from gateway_api.common.error import InvalidRequestJSON, MissingOrEmptyHeader if TYPE_CHECKING: from fhir.bundle import Bundle @@ -28,7 +28,7 @@ def __init__(self, request: Request) -> None: try: self._request_body: Parameters = request.get_json() except BadRequest as error: - raise CDGAPIErrors.INVALID_REQUEST_JSON from error + raise InvalidRequestJSON() from error self._response_body: Bundle | OperationOutcome | None = None self._status_code: int | None = None @@ -61,11 +61,11 @@ def _validate_headers(self) -> None: """ trace_id = self._headers.get("Ssp-TraceID", "").strip() if not trace_id: - raise CDGAPIErrors.MISSING_TRACE_ID + raise MissingOrEmptyHeader(header="Ssp-TraceID") ods_from = self._headers.get("ODS-from", "").strip() if not ods_from: - raise CDGAPIErrors.MISSING_ODS_CODE + raise MissingOrEmptyHeader(header="ODS-from") def build_response(self) -> Response: return Response( diff --git a/gateway-api/src/gateway_api/get_structured_record/test_request.py b/gateway-api/src/gateway_api/get_structured_record/test_request.py index 944c1628..c6565953 100644 --- a/gateway-api/src/gateway_api/get_structured_record/test_request.py +++ b/gateway-api/src/gateway_api/get_structured_record/test_request.py @@ -7,7 +7,7 @@ from werkzeug.test import EnvironBuilder from gateway_api.common.common import FlaskResponse -from gateway_api.common.error import Error +from gateway_api.common.error import BaseError from gateway_api.get_structured_record.request import GetStructuredRecordRequest if TYPE_CHECKING: @@ -79,7 +79,9 @@ def test_raises_value_error_when_ods_from_header_is_missing( } mock_request = create_mock_request(headers, valid_simple_request_payload) - with pytest.raises(Error, match='Missing or empty required header "ODS-from"'): + with pytest.raises( + BaseError, match='Missing or empty required header "ODS-from"' + ): GetStructuredRecordRequest(request=mock_request) def test_raises_value_error_when_ods_from_header_is_whitespace( @@ -94,7 +96,9 @@ def test_raises_value_error_when_ods_from_header_is_whitespace( } mock_request = create_mock_request(headers, valid_simple_request_payload) - with pytest.raises(Error, match='Missing or empty required header "ODS-from"'): + with pytest.raises( + BaseError, match='Missing or empty required header "ODS-from"' + ): GetStructuredRecordRequest(request=mock_request) def test_raises_value_error_when_trace_id_header_is_missing( @@ -107,7 +111,7 @@ def test_raises_value_error_when_trace_id_header_is_missing( mock_request = create_mock_request(headers, valid_simple_request_payload) with pytest.raises( - Error, + BaseError, match='Missing or empty required header "Ssp-TraceID"', ): GetStructuredRecordRequest(request=mock_request) @@ -125,7 +129,7 @@ def test_raises_value_error_when_trace_id_header_is_whitespace( mock_request = create_mock_request(headers, valid_simple_request_payload) with pytest.raises( - Error, + BaseError, match='Missing or empty required header "Ssp-TraceID"', ): GetStructuredRecordRequest(request=mock_request) diff --git a/gateway-api/src/gateway_api/test_controller.py b/gateway-api/src/gateway_api/test_controller.py index 3fc3ded4..568ec656 100644 --- a/gateway-api/src/gateway_api/test_controller.py +++ b/gateway-api/src/gateway_api/test_controller.py @@ -14,6 +14,7 @@ import gateway_api.controller as controller_module from gateway_api.app import app +from gateway_api.common.error import BaseError from gateway_api.controller import ( Controller, SdsSearchResults, @@ -345,7 +346,7 @@ def test_call_gp_provider_returns_200_on_success( [({}, {})], indirect=["get_structured_record_request"], ) -def test_call_gp_provider_returns_404_when_pds_patient_not_found( +def test_controller_run_raises_error_when_request_body_is_empty( patched_deps: Any, # NOQA ARG001 (Fixture patching dependencies) controller: Controller, get_structured_record_request: GetStructuredRecordRequest, @@ -353,11 +354,10 @@ def test_call_gp_provider_returns_404_when_pds_patient_not_found( """ If PDS returns no patient record, the controller should return 404. """ - # FakePdsClient defaults to returning None => RequestError => 404 - r = controller.run(get_structured_record_request) - - assert r.status_code == 404 - assert "No PDS patient found for NHS number" in (r.data or "") + with pytest.raises( + BaseError, match="No PDS patient found for NHS number 9999999999" + ): + _ = controller.run(get_structured_record_request) @pytest.mark.parametrize( @@ -479,35 +479,12 @@ def test_call_gp_provider_returns_502_when_gp_provider_returns_none( assert r.headers is None -@pytest.mark.parametrize( - "get_structured_record_request", - [({"ODS-from": "CONSUMER"}, {})], - indirect=["get_structured_record_request"], -) -def test_call_gp_provider_constructs_pds_client_with_expected_kwargs( - patched_deps: Any, # NOQA ARG001 (Fixture patching dependencies) - controller: Controller, - get_structured_record_request: GetStructuredRecordRequest, -) -> None: - """ - Validate that the controller constructs the PDS client with expected kwargs. - """ - _ = controller.run(get_structured_record_request) # will stop at PDS None => 404 - - assert FakePdsClient.last_init is not None - assert FakePdsClient.last_init["auth_token"] == "PLACEHOLDER_AUTH_TOKEN" # noqa: S105 - assert FakePdsClient.last_init["end_user_org_ods"] == "CONSUMER" - assert FakePdsClient.last_init["base_url"] == "https://pds.example" - assert FakePdsClient.last_init["nhsd_session_urid"] == "session-123" - assert FakePdsClient.last_init["timeout"] == 3 - - @pytest.mark.parametrize( "get_structured_record_request", [({}, {"parameter": [{"valueIdentifier": {"value": "1234567890"}}]})], indirect=["get_structured_record_request"], ) -def test_call_gp_provider_404_message_includes_nhs_number_from_request_body( +def test_controller_run_raises_patient_not_found_error_when_patient_doesnt_exist( patched_deps: Any, # NOQA ARG001 (Fixture patching dependencies) controller: Controller, get_structured_record_request: GetStructuredRecordRequest, @@ -516,10 +493,10 @@ def test_call_gp_provider_404_message_includes_nhs_number_from_request_body( If PDS returns no patient record, error message should include NHS number parsed from the FHIR Parameters request body. """ - r = controller.run(get_structured_record_request) - - assert r.status_code == 404 - assert r.data == "No PDS patient found for NHS number 1234567890" + with pytest.raises( + BaseError, match="No PDS patient found for NHS number 1234567890" + ): + _ = controller.run(get_structured_record_request) @pytest.mark.parametrize( From e5fecaaebcdb5097710162cc474b895291f3b81a Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Mon, 9 Feb 2026 16:30:05 +0000 Subject: [PATCH 16/27] [GPCAPIM-275]: We do not send the ODS code to PDS as an End User Org --- gateway-api/src/gateway_api/controller.py | 10 ++-------- gateway-api/src/gateway_api/pds_search.py | 5 ----- gateway-api/src/gateway_api/test_pds_search.py | 8 -------- 3 files changed, 2 insertions(+), 21 deletions(-) diff --git a/gateway-api/src/gateway_api/controller.py b/gateway-api/src/gateway_api/controller.py index 43d2db1c..2e626ba5 100644 --- a/gateway-api/src/gateway_api/controller.py +++ b/gateway-api/src/gateway_api/controller.py @@ -152,9 +152,7 @@ def run(self, request: GetStructuredRecordRequest) -> FlaskResponse: auth_token = self.get_auth_token() try: - provider_ods = self._get_pds_details( - auth_token, request.ods_from.strip(), request.nhs_number - ) + provider_ods = self._get_pds_details(auth_token, request.nhs_number) except RequestError as err: return FlaskResponse(status_code=err.status_code, data=str(err)) @@ -198,14 +196,11 @@ def get_auth_token(self) -> str: # Placeholder implementation return "PLACEHOLDER_AUTH_TOKEN" - def _get_pds_details( - self, auth_token: str, consumer_ods: str, nhs_number: str - ) -> str: + def _get_pds_details(self, auth_token: str, nhs_number: str) -> str: """ Call PDS to find the provider ODS code (GP ODS code) for a patient. :param auth_token: Authorization token to use for PDS. - :param consumer_ods: Consumer organisation ODS code (from request headers). :param nhs_number: NHS number :returns: Provider ODS code (GP ODS code). :raises RequestError: If the patient cannot be found or has no provider ODS code @@ -213,7 +208,6 @@ def _get_pds_details( # PDS: find patient and extract GP ODS code (provider ODS) pds = PdsClient( auth_token=auth_token, - end_user_org_ods=consumer_ods, base_url=self.pds_base_url, nhsd_session_urid=self.nhsd_session_urid, timeout=self.timeout, diff --git a/gateway-api/src/gateway_api/pds_search.py b/gateway-api/src/gateway_api/pds_search.py index b21b6ecf..7cd3b6fd 100644 --- a/gateway-api/src/gateway_api/pds_search.py +++ b/gateway-api/src/gateway_api/pds_search.py @@ -86,7 +86,6 @@ class PdsClient: pds = PdsClient( auth_token="YOUR_ACCESS_TOKEN", - end_user_org_ods="A12345", base_url="https://sandbox.api.service.nhs.uk/personal-demographics/FHIR/R4", ) @@ -104,7 +103,6 @@ class PdsClient: 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, @@ -114,7 +112,6 @@ def __init__( 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. @@ -123,7 +120,6 @@ def __init__( 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 @@ -151,7 +147,6 @@ def _build_headers( """ headers = { "X-Request-ID": request_id or str(uuid.uuid4()), - "NHSD-End-User-Organisation-ODS": self.end_user_org_ods, "Accept": "application/fhir+json", } diff --git a/gateway-api/src/gateway_api/test_pds_search.py b/gateway-api/src/gateway_api/test_pds_search.py index a433c9a1..b83f4d2a 100644 --- a/gateway-api/src/gateway_api/test_pds_search.py +++ b/gateway-api/src/gateway_api/test_pds_search.py @@ -193,7 +193,6 @@ def test_search_patient_by_nhs_number_get_patient_success( 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", ) @@ -246,7 +245,6 @@ def test_search_patient_by_nhs_number_no_current_gp_returns_gp_ods_code_none( 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", ) @@ -287,7 +285,6 @@ def test_search_patient_by_nhs_number_sends_expected_headers( client = PdsClient( auth_token="test-token", # noqa: S106 - end_user_org_ods="A12345", base_url="https://example.test/personal-demographics/FHIR/R4", ) @@ -303,7 +300,6 @@ def test_search_patient_by_nhs_number_sends_expected_headers( 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 @@ -334,7 +330,6 @@ def test_search_patient_by_nhs_number_generates_request_id( client = PdsClient( auth_token="test-token", # noqa: S106 - end_user_org_ods="A12345", base_url="https://example.test/personal-demographics/FHIR/R4", ) @@ -363,7 +358,6 @@ def test_search_patient_by_nhs_number_not_found_raises_error( """ pds = PdsClient( auth_token="test-token", # noqa: S106 - end_user_org_ods="A12345", base_url="https://example.test/personal-demographics/FHIR/R4", ) @@ -425,7 +419,6 @@ def test_search_patient_by_nhs_number_extracts_current_gp_ods_code( client = PdsClient( auth_token="test-token", # noqa: S106 - end_user_org_ods="A12345", base_url="https://example.test/personal-demographics/FHIR/R4", ) @@ -516,7 +509,6 @@ def test_extract_single_search_result_invalid_body_raises_runtime_error() -> Non """ 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", ) From 8354dafcde342f9bd6e38a9b9ab071b891805464 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Mon, 9 Feb 2026 16:42:01 +0000 Subject: [PATCH 17/27] [GPCAPIM-275]: Given we are accessing PDS as an application, we do not need to handle NHS Session URID --- gateway-api/src/gateway_api/controller.py | 4 ---- gateway-api/src/gateway_api/pds_search.py | 7 ------- gateway-api/src/gateway_api/test_controller.py | 1 - gateway-api/src/gateway_api/test_pds_search.py | 1 - gateway-api/stubs/stubs/stub_pds.py | 2 -- 5 files changed, 15 deletions(-) diff --git a/gateway-api/src/gateway_api/controller.py b/gateway-api/src/gateway_api/controller.py index 2e626ba5..f30916ad 100644 --- a/gateway-api/src/gateway_api/controller.py +++ b/gateway-api/src/gateway_api/controller.py @@ -115,7 +115,6 @@ def __init__( self, pds_base_url: str = PdsClient.SANDBOX_URL, sds_base_url: str = "https://example.invalid/sds", - nhsd_session_urid: str | None = None, timeout: int = 10, ) -> None: """ @@ -123,12 +122,10 @@ def __init__( :param pds_base_url: Base URL for PDS client. :param sds_base_url: Base URL for SDS client. - :param nhsd_session_urid: Session URID for NHS Digital session handling. :param timeout: Timeout in seconds for downstream calls. """ self.pds_base_url = pds_base_url self.sds_base_url = sds_base_url - self.nhsd_session_urid = nhsd_session_urid self.timeout = timeout self.gp_provider_client = None @@ -209,7 +206,6 @@ def _get_pds_details(self, auth_token: str, nhs_number: str) -> str: pds = PdsClient( auth_token=auth_token, base_url=self.pds_base_url, - nhsd_session_urid=self.nhsd_session_urid, timeout=self.timeout, ignore_dates=True, ) diff --git a/gateway-api/src/gateway_api/pds_search.py b/gateway-api/src/gateway_api/pds_search.py index 7cd3b6fd..757c743f 100644 --- a/gateway-api/src/gateway_api/pds_search.py +++ b/gateway-api/src/gateway_api/pds_search.py @@ -104,7 +104,6 @@ def __init__( self, auth_token: str, base_url: str = SANDBOX_URL, - nhsd_session_urid: str | None = None, timeout: int = 10, ignore_dates: bool = False, ) -> None: @@ -114,14 +113,12 @@ def __init__( :param auth_token: OAuth2 bearer token (without the ``"Bearer "`` prefix). :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.base_url = base_url.rstrip("/") - self.nhsd_session_urid = nhsd_session_urid self.timeout = timeout self.ignore_dates = ignore_dates self.stub = PdsFhirApiStub() @@ -154,10 +151,6 @@ def _build_headers( 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: diff --git a/gateway-api/src/gateway_api/test_controller.py b/gateway-api/src/gateway_api/test_controller.py index 568ec656..0b5ae244 100644 --- a/gateway-api/src/gateway_api/test_controller.py +++ b/gateway-api/src/gateway_api/test_controller.py @@ -224,7 +224,6 @@ def controller() -> Controller: return Controller( pds_base_url="https://pds.example", sds_base_url="https://sds.example", - nhsd_session_urid="session-123", timeout=3, ) diff --git a/gateway-api/src/gateway_api/test_pds_search.py b/gateway-api/src/gateway_api/test_pds_search.py index b83f4d2a..c969e091 100644 --- a/gateway-api/src/gateway_api/test_pds_search.py +++ b/gateway-api/src/gateway_api/test_pds_search.py @@ -194,7 +194,6 @@ def test_search_patient_by_nhs_number_get_patient_success( client = PdsClient( auth_token="test-token", # noqa: S106 (test token hardcoded) base_url="https://example.test/personal-demographics/FHIR/R4", - nhsd_session_urid="test-urid", ) result = client.search_patient_by_nhs_number("9000000009") diff --git a/gateway-api/stubs/stubs/stub_pds.py b/gateway-api/stubs/stubs/stub_pds.py index f8249295..2081f896 100644 --- a/gateway-api/stubs/stubs/stub_pds.py +++ b/gateway-api/stubs/stubs/stub_pds.py @@ -275,8 +275,6 @@ def get( request_id = headers.get("X-Request-ID") correlation_id = headers.get("X-Correlation-ID") authorization = headers.get("Authorization") - role_id = headers.get("NHSD-Session-URID") - end_user_org_ods = headers.get("NHSD-End-User-Organisation-ODS") return self.get_patient( nhs_number=nhs_number, From 36d9812a760c8c272d749d7e78ce537e90bdfa82 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Mon, 9 Feb 2026 18:50:56 +0000 Subject: [PATCH 18/27] [GPCAPIM-275]: PDS sandbox can receive auth header. --- gateway-api/src/gateway_api/controller.py | 3 +-- gateway-api/src/gateway_api/pds_search.py | 5 +---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/gateway-api/src/gateway_api/controller.py b/gateway-api/src/gateway_api/controller.py index f30916ad..5a93e342 100644 --- a/gateway-api/src/gateway_api/controller.py +++ b/gateway-api/src/gateway_api/controller.py @@ -190,8 +190,7 @@ def get_auth_token(self) -> str: :returns: Authorization token as a string. """ - # Placeholder implementation - return "PLACEHOLDER_AUTH_TOKEN" + return "AUTH_TOKEN123" def _get_pds_details(self, auth_token: str, nhs_number: str) -> str: """ diff --git a/gateway-api/src/gateway_api/pds_search.py b/gateway-api/src/gateway_api/pds_search.py index 757c743f..230a8309 100644 --- a/gateway-api/src/gateway_api/pds_search.py +++ b/gateway-api/src/gateway_api/pds_search.py @@ -145,12 +145,9 @@ def _build_headers( headers = { "X-Request-ID": request_id or str(uuid.uuid4()), "Accept": "application/fhir+json", + "Authorization": f"Bearer {self.auth_token}", } - # 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}" - # 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: From ddd5b948869ea23438e5a8e05cf16a4e1c60ad63 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Tue, 10 Feb 2026 10:28:27 +0000 Subject: [PATCH 19/27] [GPCAPIM-275]: Move towards common error class --- gateway-api/src/gateway_api/app.py | 3 +- gateway-api/src/gateway_api/common/error.py | 26 +++++++ gateway-api/src/gateway_api/controller.py | 64 +++++------------ .../src/gateway_api/test_controller.py | 72 ++++++++++++------- 4 files changed, 89 insertions(+), 76 deletions(-) diff --git a/gateway-api/src/gateway_api/app.py b/gateway-api/src/gateway_api/app.py index c666627e..881c32fa 100644 --- a/gateway-api/src/gateway_api/app.py +++ b/gateway-api/src/gateway_api/app.py @@ -47,8 +47,7 @@ def get_structured_record() -> Response: except Exception: error = BaseError() error.log() - response = error.build_response() - return response + return error.build_response() return get_structured_record_request.build_response() diff --git a/gateway-api/src/gateway_api/common/error.py b/gateway-api/src/gateway_api/common/error.py index 2407e2f8..d452882b 100644 --- a/gateway-api/src/gateway_api/common/error.py +++ b/gateway-api/src/gateway_api/common/error.py @@ -18,6 +18,7 @@ class BaseError(Exception): def __init__(self, **additional_details: str): self.additional_details = additional_details + super().__init__(self) def build_response(self) -> Response: operation_outcome: OperationOutcome = { @@ -61,3 +62,28 @@ class InvalidRequestJSON(BaseError): class MissingOrEmptyHeader(BaseError): _message = 'Missing or empty required header "{header}"' status_code = 400 + + +class NoCurrentProvider(BaseError): + _message = "PDS patient {nhs_number} did not contain a current provider ODS code" + status_code = 404 + + +class NoOrganisationFound(BaseError): + _message = "No SDS org found for {org_type} ODS code {ods_code}" + status_code = 404 + + +class NoAsidFound(BaseError): + _message = ( + "SDS result for {org_type} ODS code {ods_code} did not contain a current ASID" + ) + status_code = 404 + + +class NoCurrentEndpoint(BaseError): + _message = ( + "SDS result for provider ODS code {provider_ods} did not contain " + "a current endpoint" + ) + status_code = 404 diff --git a/gateway-api/src/gateway_api/controller.py b/gateway-api/src/gateway_api/controller.py index 5a93e342..a8a31c15 100644 --- a/gateway-api/src/gateway_api/controller.py +++ b/gateway-api/src/gateway_api/controller.py @@ -7,7 +7,13 @@ import json from typing import TYPE_CHECKING -from gateway_api.common.error import NoPatientFound +from gateway_api.common.error import ( + NoAsidFound, + NoCurrentEndpoint, + NoCurrentProvider, + NoOrganisationFound, + NoPatientFound, +) from gateway_api.provider_request import GpProviderClient if TYPE_CHECKING: @@ -148,17 +154,11 @@ def run(self, request: GetStructuredRecordRequest) -> FlaskResponse: """ auth_token = self.get_auth_token() - try: - provider_ods = self._get_pds_details(auth_token, request.nhs_number) - except RequestError as err: - return FlaskResponse(status_code=err.status_code, data=str(err)) + provider_ods = self._get_pds_details(auth_token, request.nhs_number) - try: - consumer_asid, provider_asid, provider_endpoint = self._get_sds_details( - auth_token, request.ods_from.strip(), provider_ods - ) - except RequestError as err: - return FlaskResponse(status_code=err.status_code, data=str(err)) + consumer_asid, provider_asid, provider_endpoint = self._get_sds_details( + auth_token, request.ods_from.strip(), provider_ods + ) # Call GP provider with correct parameters self.gp_provider_client = GpProviderClient( @@ -220,13 +220,7 @@ def _get_pds_details(self, auth_token: str, nhs_number: str) -> str: if pds_result.gp_ods_code: provider_ods_code = pds_result.gp_ods_code else: - raise RequestError( - status_code=404, - message=( - f"PDS patient {nhs_number} did not contain a current " - "provider ODS code" - ), - ) + raise NoCurrentProvider(nhs_number=nhs_number) return provider_ods_code @@ -255,47 +249,23 @@ def _get_sds_details( provider_details: SdsSearchResults | None = sds.get_org_details(provider_ods) if provider_details is None: - raise RequestError( - status_code=404, - message=f"No SDS org found for provider ODS code {provider_ods}", - ) + raise NoOrganisationFound(org_type="provider", ods_code=provider_ods) provider_asid = (provider_details.asid or "").strip() if not provider_asid: - raise RequestError( - status_code=404, - message=( - f"SDS result for provider ODS code {provider_ods} did not contain " - "a current ASID" - ), - ) + raise NoAsidFound(org_type="provider", ods_code=provider_ods) provider_endpoint = (provider_details.endpoint or "").strip() if not provider_endpoint: - raise RequestError( - status_code=404, - message=( - f"SDS result for provider ODS code {provider_ods} did not contain " - "a current endpoint" - ), - ) + raise NoCurrentEndpoint(provider_ods=provider_ods) # SDS: Get consumer details (ASID) for consumer ODS consumer_details: SdsSearchResults | None = sds.get_org_details(consumer_ods) if consumer_details is None: - raise RequestError( - status_code=404, - message=f"No SDS org found for consumer ODS code {consumer_ods}", - ) + raise NoOrganisationFound(org_type="consumer", ods_code=consumer_ods) consumer_asid = (consumer_details.asid or "").strip() if not consumer_asid: - raise RequestError( - status_code=404, - message=( - f"SDS result for consumer ODS code {consumer_ods} did not contain " - "a current ASID" - ), - ) + raise NoAsidFound(org_type="consumer", ods_code=consumer_ods) return consumer_asid, provider_asid, provider_endpoint diff --git a/gateway-api/src/gateway_api/test_controller.py b/gateway-api/src/gateway_api/test_controller.py index 0b5ae244..200ef420 100644 --- a/gateway-api/src/gateway_api/test_controller.py +++ b/gateway-api/src/gateway_api/test_controller.py @@ -14,7 +14,13 @@ import gateway_api.controller as controller_module from gateway_api.app import app -from gateway_api.common.error import BaseError +from gateway_api.common.error import ( + NoAsidFound, + NoCurrentEndpoint, + NoCurrentProvider, + NoOrganisationFound, + NoPatientFound, +) from gateway_api.controller import ( Controller, SdsSearchResults, @@ -354,7 +360,7 @@ def test_controller_run_raises_error_when_request_body_is_empty( If PDS returns no patient record, the controller should return 404. """ with pytest.raises( - BaseError, match="No PDS patient found for NHS number 9999999999" + NoPatientFound, match="No PDS patient found for NHS number 9999999999" ): _ = controller.run(get_structured_record_request) @@ -376,10 +382,11 @@ def test_call_gp_provider_returns_404_when_gp_ods_code_missing( pds = pds_factory(ods_code="") monkeypatch.setattr(controller_module, "PdsClient", pds) - r = controller.run(get_structured_record_request) - - assert r.status_code == 404 - assert "did not contain a current provider ODS code" in (r.data or "") + with pytest.raises( + NoCurrentProvider, + match="PDS patient 9999999999 did not contain a current provider ODS code", + ): + _ = controller.run(get_structured_record_request) @pytest.mark.parametrize( @@ -402,10 +409,11 @@ def test_call_gp_provider_returns_404_when_sds_returns_none_for_provider( monkeypatch.setattr(controller_module, "PdsClient", pds) monkeypatch.setattr(controller_module, "SdsClient", sds) - r = controller.run(get_structured_record_request) - - assert r.status_code == 404 - assert r.data == "No SDS org found for provider ODS code PROVIDER" + with pytest.raises( + NoOrganisationFound, + match="No SDS org found for provider ODS code PROVIDER", + ): + _ = controller.run(get_structured_record_request) @pytest.mark.parametrize( @@ -434,10 +442,13 @@ def test_call_gp_provider_returns_404_when_sds_provider_asid_blank( monkeypatch.setattr(controller_module, "PdsClient", pds) monkeypatch.setattr(controller_module, "SdsClient", sds) - r = controller.run(get_structured_record_request) - - assert r.status_code == 404 - assert "did not contain a current ASID" in (r.data or "") + with pytest.raises( + NoAsidFound, + match=( + "SDS result for provider ODS code PROVIDER did not contain a current ASID" + ), + ): + _ = controller.run(get_structured_record_request) @pytest.mark.parametrize( @@ -493,7 +504,7 @@ def test_controller_run_raises_patient_not_found_error_when_patient_doesnt_exist from the FHIR Parameters request body. """ with pytest.raises( - BaseError, match="No PDS patient found for NHS number 1234567890" + NoPatientFound, match="No PDS patient found for NHS number 1234567890" ): _ = controller.run(get_structured_record_request) @@ -522,10 +533,14 @@ def test_call_gp_provider_returns_404_when_sds_provider_endpoint_blank( monkeypatch.setattr(controller_module, "PdsClient", pds) monkeypatch.setattr(controller_module, "SdsClient", sds) - r = controller.run(get_structured_record_request) - - assert r.status_code == 404 - assert "did not contain a current endpoint" in (r.data or "") + with pytest.raises( + NoCurrentEndpoint, + match=( + "SDS result for provider ODS code PROVIDER did not contain " + "a current endpoint" + ), + ): + _ = controller.run(get_structured_record_request) @pytest.mark.parametrize( @@ -554,10 +569,10 @@ def test_call_gp_provider_returns_404_when_sds_returns_none_for_consumer( monkeypatch.setattr(controller_module, "PdsClient", pds) monkeypatch.setattr(controller_module, "SdsClient", sds) - r = controller.run(get_structured_record_request) - - assert r.status_code == 404 - assert r.data == "No SDS org found for consumer ODS code CONSUMER" + with pytest.raises( + NoOrganisationFound, match="No SDS org found for consumer ODS code CONSUMER" + ): + _ = controller.run(get_structured_record_request) @pytest.mark.parametrize( @@ -590,10 +605,13 @@ def test_call_gp_provider_returns_404_when_sds_consumer_asid_blank( monkeypatch.setattr(controller_module, "PdsClient", pds) monkeypatch.setattr(controller_module, "SdsClient", sds) - r = controller.run(get_structured_record_request) - - assert r.status_code == 404 - assert "did not contain a current ASID" in (r.data or "") + with pytest.raises( + NoAsidFound, + match=( + "SDS result for consumer ODS code CONSUMER did not contain a current ASID" + ), + ): + _ = controller.run(get_structured_record_request) @pytest.mark.parametrize( From 750e4d4b1dbbd2770c55bc2115179b2ae1f123ff Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Tue, 10 Feb 2026 11:11:26 +0000 Subject: [PATCH 20/27] [GPCAPIM-275]: Use http client's status code definitions for clarity --- gateway-api/src/gateway_api/common/error.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/gateway-api/src/gateway_api/common/error.py b/gateway-api/src/gateway_api/common/error.py index d452882b..7f733db1 100644 --- a/gateway-api/src/gateway_api/common/error.py +++ b/gateway-api/src/gateway_api/common/error.py @@ -1,6 +1,6 @@ import json from dataclasses import dataclass -from http.client import INTERNAL_SERVER_ERROR +from http.client import BAD_REQUEST, INTERNAL_SERVER_ERROR, NOT_FOUND from typing import TYPE_CHECKING from flask import Response @@ -51,34 +51,34 @@ def __str__(self) -> str: class NoPatientFound(BaseError): _message = "No PDS patient found for NHS number {nhs_number}" - status_code = 400 + status_code = BAD_REQUEST class InvalidRequestJSON(BaseError): _message = "Invalid JSON body sent in request" - status_code = 400 + status_code = BAD_REQUEST class MissingOrEmptyHeader(BaseError): _message = 'Missing or empty required header "{header}"' - status_code = 400 + status_code = BAD_REQUEST class NoCurrentProvider(BaseError): _message = "PDS patient {nhs_number} did not contain a current provider ODS code" - status_code = 404 + status_code = NOT_FOUND class NoOrganisationFound(BaseError): _message = "No SDS org found for {org_type} ODS code {ods_code}" - status_code = 404 + status_code = NOT_FOUND class NoAsidFound(BaseError): _message = ( "SDS result for {org_type} ODS code {ods_code} did not contain a current ASID" ) - status_code = 404 + status_code = NOT_FOUND class NoCurrentEndpoint(BaseError): @@ -86,4 +86,4 @@ class NoCurrentEndpoint(BaseError): "SDS result for provider ODS code {provider_ods} did not contain " "a current endpoint" ) - status_code = 404 + status_code = NOT_FOUND From 7aa9fa24ebc38e2b442d09c8b5796ad2024d4f5f Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Tue, 10 Feb 2026 11:24:58 +0000 Subject: [PATCH 21/27] [GPCAPIM-275]: Move towards common error class --- gateway-api/src/gateway_api/common/error.py | 10 +++++++ gateway-api/src/gateway_api/controller.py | 26 ------------------- .../get_structured_record/__init__.py | 7 ++--- .../get_structured_record/request.py | 9 +------ gateway-api/src/gateway_api/pds_search.py | 19 +++----------- .../src/gateway_api/provider_request.py | 15 +++-------- .../src/gateway_api/test_pds_search.py | 10 ++++--- .../src/gateway_api/test_provider_request.py | 9 ++++--- 8 files changed, 30 insertions(+), 75 deletions(-) diff --git a/gateway-api/src/gateway_api/common/error.py b/gateway-api/src/gateway_api/common/error.py index 7f733db1..8e8133ef 100644 --- a/gateway-api/src/gateway_api/common/error.py +++ b/gateway-api/src/gateway_api/common/error.py @@ -87,3 +87,13 @@ class NoCurrentEndpoint(BaseError): "a current endpoint" ) status_code = NOT_FOUND + + +class PdsRequestFailed(BaseError): + _message = "PDS FHIR API request failed: {error_reason}" + status_code = INTERNAL_SERVER_ERROR + + +class SdsRequestFailed(BaseError): + _message = "SDS FHIR API request failed: {error_reason}" + status_code = INTERNAL_SERVER_ERROR diff --git a/gateway-api/src/gateway_api/controller.py b/gateway-api/src/gateway_api/controller.py index a8a31c15..f485295c 100644 --- a/gateway-api/src/gateway_api/controller.py +++ b/gateway-api/src/gateway_api/controller.py @@ -27,30 +27,6 @@ from gateway_api.pds_search import PdsClient, PdsSearchResults -@dataclass -class RequestError(Exception): - """ - Raised (and handled) when there is a problem with the incoming request. - - Instances of this exception are caught by controller entry points and converted - into an appropriate :class:`FlaskResponse`. - - :param status_code: HTTP status code that should be returned. - :param message: Human-readable error message. - """ - - status_code: int - message: str - - def __str__(self) -> str: - """ - Coercing this exception to a string returns the error message. - - :returns: The error message. - """ - return self.message - - @dataclass class SdsSearchResults: """ @@ -199,7 +175,6 @@ def _get_pds_details(self, auth_token: str, nhs_number: str) -> str: :param auth_token: Authorization token to use for PDS. :param nhs_number: NHS number :returns: Provider ODS code (GP ODS code). - :raises RequestError: If the patient cannot be found or has no provider ODS code """ # PDS: find patient and extract GP ODS code (provider ODS) pds = PdsClient( @@ -238,7 +213,6 @@ def _get_sds_details( :param consumer_ods: Consumer organisation ODS code (from request headers). :param provider_ods: Provider organisation ODS code (from PDS). :returns: Tuple of (consumer_asid, provider_asid, provider_endpoint). - :raises RequestError: If SDS data is missing or incomplete for provider/consumer """ # SDS: Get provider details (ASID + endpoint) for provider ODS sds = SdsClient( diff --git a/gateway-api/src/gateway_api/get_structured_record/__init__.py b/gateway-api/src/gateway_api/get_structured_record/__init__.py index 56dd174d..456f2a4e 100644 --- a/gateway-api/src/gateway_api/get_structured_record/__init__.py +++ b/gateway-api/src/gateway_api/get_structured_record/__init__.py @@ -1,8 +1,5 @@ """Get Structured Record module.""" -from gateway_api.get_structured_record.request import ( - GetStructuredRecordRequest, - RequestValidationError, -) +from gateway_api.get_structured_record.request import GetStructuredRecordRequest -__all__ = ["RequestValidationError", "GetStructuredRecordRequest"] +__all__ = ["GetStructuredRecordRequest"] diff --git a/gateway-api/src/gateway_api/get_structured_record/request.py b/gateway-api/src/gateway_api/get_structured_record/request.py index ffd8a302..8b7edb64 100644 --- a/gateway-api/src/gateway_api/get_structured_record/request.py +++ b/gateway-api/src/gateway_api/get_structured_record/request.py @@ -13,10 +13,6 @@ from fhir.bundle import Bundle -class RequestValidationError(Exception): - """Exception raised for errors in the request validation.""" - - class GetStructuredRecordRequest: INTERACTION_ID: str = "urn:nhs:names:services:gpconnect:gpc.getstructuredrecord-1" RESOURCE: str = "patient" @@ -55,10 +51,7 @@ def request_body(self) -> str: return json.dumps(self._request_body) def _validate_headers(self) -> None: - """Validate required headers are present and non-empty. - - :raises RequestValidationError: If required headers are missing or empty. - """ + """Validate required headers are present and non-empty.""" trace_id = self._headers.get("Ssp-TraceID", "").strip() if not trace_id: raise MissingOrEmptyHeader(header="Ssp-TraceID") diff --git a/gateway-api/src/gateway_api/pds_search.py b/gateway-api/src/gateway_api/pds_search.py index 230a8309..1f0b60ed 100644 --- a/gateway-api/src/gateway_api/pds_search.py +++ b/gateway-api/src/gateway_api/pds_search.py @@ -29,6 +29,8 @@ import requests from stubs.stub_pds import PdsFhirApiStub +from gateway_api.common.error import PdsRequestFailed + # Recursive JSON-like structure typing used for parsed FHIR bodies. type ResultStructure = str | dict[str, "ResultStructure"] | list["ResultStructure"] type ResultStructureDict = dict[str, ResultStructure] @@ -38,16 +40,6 @@ type GetCallable = Callable[..., requests.Response] -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 PdsSearchResults: """ @@ -176,8 +168,6 @@ def search_patient_by_nhs_number( :attr:`timeout` is used. :return: A :class:`PdsSearchResults` 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, @@ -195,12 +185,9 @@ def search_patient_by_nhs_number( ) 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 + raise PdsRequestFailed(error_reason=err.response.reason) from err body = response.json() return self._extract_single_search_result(body) diff --git a/gateway-api/src/gateway_api/provider_request.py b/gateway-api/src/gateway_api/provider_request.py index a628dbcf..c0e3563a 100644 --- a/gateway-api/src/gateway_api/provider_request.py +++ b/gateway-api/src/gateway_api/provider_request.py @@ -28,6 +28,8 @@ from requests import HTTPError, Response, post from stubs.stub_provider import stub_post +from gateway_api.common.error import SdsRequestFailed + ARS_INTERACTION_ID = ( "urn:nhs:names:services:gpconnect:structured" ":fhir:operation:gpc.getstructuredrecord-1" @@ -46,12 +48,6 @@ post: PostCallable = stub_post # type: ignore[no-redef] -class ExternalServiceError(Exception): - """ - Exception raised when the downstream GPProvider FHIR API request fails. - """ - - class GpProviderClient: """ A client for interacting with the GPProvider FHIR GP System. @@ -114,9 +110,6 @@ def access_structured_record( Returns: Response: The response from the GPProvider FHIR API. - - Raises: - ExternalServiceError: If the API request fails with an HTTP error. """ headers = self._build_headers(trace_id) @@ -134,8 +127,6 @@ def access_structured_record( try: response.raise_for_status() except HTTPError as err: - raise ExternalServiceError( - f"GPProvider FHIR API request failed:{err.response.reason}" - ) from err + raise SdsRequestFailed(error_reason=err.response.reason) from err return response diff --git a/gateway-api/src/gateway_api/test_pds_search.py b/gateway-api/src/gateway_api/test_pds_search.py index c969e091..80268ce2 100644 --- a/gateway-api/src/gateway_api/test_pds_search.py +++ b/gateway-api/src/gateway_api/test_pds_search.py @@ -16,8 +16,8 @@ if TYPE_CHECKING: from requests.structures import CaseInsensitiveDict +from gateway_api.common.error import PdsRequestFailed from gateway_api.pds_search import ( - ExternalServiceError, PdsClient, ResultList, ) @@ -345,11 +345,11 @@ def test_search_patient_by_nhs_number_not_found_raises_error( mock_requests_get: dict[str, Any], # NOQA ARG001 (Mock not called directly) ) -> None: """ - Verify that a 404 response results in :class:`ExternalServiceError`. + Verify that a 404 response results in :class:`PDSRequestFailed`. 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`. + into :class:`PDSRequestFailed`. :param stub: Stub backend fixture. :param mock_requests_get: Patched ``requests.get`` fixture. @@ -360,7 +360,9 @@ def test_search_patient_by_nhs_number_not_found_raises_error( base_url="https://example.test/personal-demographics/FHIR/R4", ) - with pytest.raises(ExternalServiceError): + with pytest.raises( + PdsRequestFailed, match="PDS FHIR API request failed: Not Found" + ): pds.search_patient_by_nhs_number("9900000001") diff --git a/gateway-api/src/gateway_api/test_provider_request.py b/gateway-api/src/gateway_api/test_provider_request.py index 6441490a..6af8a0aa 100644 --- a/gateway-api/src/gateway_api/test_provider_request.py +++ b/gateway-api/src/gateway_api/test_provider_request.py @@ -14,7 +14,8 @@ from stubs.stub_provider import GpProviderStub from gateway_api import provider_request -from gateway_api.provider_request import ExternalServiceError, GpProviderClient +from gateway_api.common.error import SdsRequestFailed +from gateway_api.provider_request import GpProviderClient ars_interactionId = ( "urn:nhs:names:services:gpconnect:structured" @@ -199,7 +200,7 @@ def test_access_structured_record_raises_external_service_error( mock_request_post: dict[str, Any], # NOQA ARG001 (Mock not called directly) ) -> None: """ - Test that the `access_structured_record` method raises an `ExternalServiceError` + Test that the `access_structured_record` method raises an `SdsRequestFailed` when the GPProvider FHIR API request fails with an HTTP error. """ provider_asid = "200000001154" @@ -214,7 +215,7 @@ def test_access_structured_record_raises_external_service_error( ) with pytest.raises( - ExternalServiceError, - match="GPProvider FHIR API request failed:Bad Request", + SdsRequestFailed, + match="SDS FHIR API request failed: Bad Request", ): client.access_structured_record(trace_id, "body") From 5af0f0a8523c17a291a5b992218f92f3bf869786 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Tue, 10 Feb 2026 11:34:22 +0000 Subject: [PATCH 22/27] [GPCAPIM-275]: Remove unnecessary imports. --- gateway-api/src/gateway_api/controller.py | 18 ++++-------------- gateway-api/src/gateway_api/pds_search.py | 2 -- gateway-api/src/gateway_api/test_controller.py | 11 +++-------- gateway-api/src/gateway_api/test_pds_search.py | 8 ++------ gateway-api/stubs/stubs/stub_pds.py | 2 -- 5 files changed, 9 insertions(+), 32 deletions(-) diff --git a/gateway-api/src/gateway_api/controller.py b/gateway-api/src/gateway_api/controller.py index f485295c..3e5f938c 100644 --- a/gateway-api/src/gateway_api/controller.py +++ b/gateway-api/src/gateway_api/controller.py @@ -2,11 +2,9 @@ Controller layer for orchestrating calls to external services """ -from __future__ import annotations - -import json -from typing import TYPE_CHECKING +from dataclasses import dataclass +from gateway_api.common.common import FlaskResponse from gateway_api.common.error import ( NoAsidFound, NoCurrentEndpoint, @@ -14,17 +12,9 @@ NoOrganisationFound, NoPatientFound, ) -from gateway_api.provider_request import GpProviderClient - -if TYPE_CHECKING: - from gateway_api.get_structured_record.request import GetStructuredRecordRequest - -__all__ = ["json"] # Make mypy happy in tests - -from dataclasses import dataclass - -from gateway_api.common.common import FlaskResponse +from gateway_api.get_structured_record.request import GetStructuredRecordRequest from gateway_api.pds_search import PdsClient, PdsSearchResults +from gateway_api.provider_request import GpProviderClient @dataclass diff --git a/gateway-api/src/gateway_api/pds_search.py b/gateway-api/src/gateway_api/pds_search.py index 1f0b60ed..71099afa 100644 --- a/gateway-api/src/gateway_api/pds_search.py +++ b/gateway-api/src/gateway_api/pds_search.py @@ -18,8 +18,6 @@ malformed upstream data (or malformed test fixtures) and should be corrected at source. """ -from __future__ import annotations - import uuid from collections.abc import Callable from dataclasses import dataclass diff --git a/gateway-api/src/gateway_api/test_controller.py b/gateway-api/src/gateway_api/test_controller.py index 200ef420..b3a0b5fd 100644 --- a/gateway-api/src/gateway_api/test_controller.py +++ b/gateway-api/src/gateway_api/test_controller.py @@ -2,11 +2,10 @@ Unit tests for :mod:`gateway_api.controller`. """ -from __future__ import annotations - +from collections.abc import Generator from dataclasses import dataclass from types import SimpleNamespace -from typing import TYPE_CHECKING, Any +from typing import Any import pytest from flask import request as flask_request @@ -14,6 +13,7 @@ import gateway_api.controller as controller_module from gateway_api.app import app +from gateway_api.common.common import json_str from gateway_api.common.error import ( NoAsidFound, NoCurrentEndpoint, @@ -27,11 +27,6 @@ ) from gateway_api.get_structured_record.request import GetStructuredRecordRequest -if TYPE_CHECKING: - from collections.abc import Generator - - from gateway_api.common.common import json_str - # ----------------------------- # Fake downstream dependencies diff --git a/gateway-api/src/gateway_api/test_pds_search.py b/gateway-api/src/gateway_api/test_pds_search.py index 80268ce2..e786c242 100644 --- a/gateway-api/src/gateway_api/test_pds_search.py +++ b/gateway-api/src/gateway_api/test_pds_search.py @@ -2,20 +2,16 @@ Unit tests for :mod:`gateway_api.pds_search`. """ -from __future__ import annotations - from dataclasses import dataclass from datetime import date -from typing import TYPE_CHECKING, Any, cast +from typing import Any, cast from uuid import uuid4 import pytest import requests +from requests.structures import CaseInsensitiveDict from stubs.stub_pds import PdsFhirApiStub -if TYPE_CHECKING: - from requests.structures import CaseInsensitiveDict - from gateway_api.common.error import PdsRequestFailed from gateway_api.pds_search import ( PdsClient, diff --git a/gateway-api/stubs/stubs/stub_pds.py b/gateway-api/stubs/stubs/stub_pds.py index 2081f896..6233d7b1 100644 --- a/gateway-api/stubs/stubs/stub_pds.py +++ b/gateway-api/stubs/stubs/stub_pds.py @@ -4,8 +4,6 @@ The stub does **not** implement the full PDS API surface, nor full FHIR validation. """ -from __future__ import annotations - import json import re import uuid From 366f6d45999b1946d5ab104eb9fc50c444f84e7b Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Tue, 10 Feb 2026 11:48:07 +0000 Subject: [PATCH 23/27] [GPCAPIM-275]: Move modules to their own directory --- gateway-api/src/gateway_api/controller.py | 63 +------------------ gateway-api/src/gateway_api/pds/__init__.py | 9 +++ .../{pds_search.py => pds/client.py} | 25 +------- .../src/gateway_api/pds/search_results.py | 26 ++++++++ .../test_client.py} | 6 +- .../src/gateway_api/provider/__init__.py | 7 +++ .../client.py} | 0 .../test_client.py} | 5 +- gateway-api/src/gateway_api/sds/__init__.py | 7 +++ gateway-api/src/gateway_api/sds/client.py | 42 +++++++++++++ .../src/gateway_api/sds/search_results.py | 16 +++++ .../src/gateway_api/test_controller.py | 6 +- 12 files changed, 118 insertions(+), 94 deletions(-) create mode 100644 gateway-api/src/gateway_api/pds/__init__.py rename gateway-api/src/gateway_api/{pds_search.py => pds/client.py} (94%) create mode 100644 gateway-api/src/gateway_api/pds/search_results.py rename gateway-api/src/gateway_api/{test_pds_search.py => pds/test_client.py} (99%) create mode 100644 gateway-api/src/gateway_api/provider/__init__.py rename gateway-api/src/gateway_api/{provider_request.py => provider/client.py} (100%) rename gateway-api/src/gateway_api/{test_provider_request.py => provider/test_client.py} (97%) create mode 100644 gateway-api/src/gateway_api/sds/__init__.py create mode 100644 gateway-api/src/gateway_api/sds/client.py create mode 100644 gateway-api/src/gateway_api/sds/search_results.py diff --git a/gateway-api/src/gateway_api/controller.py b/gateway-api/src/gateway_api/controller.py index 3e5f938c..90e1cc67 100644 --- a/gateway-api/src/gateway_api/controller.py +++ b/gateway-api/src/gateway_api/controller.py @@ -2,8 +2,6 @@ Controller layer for orchestrating calls to external services """ -from dataclasses import dataclass - from gateway_api.common.common import FlaskResponse from gateway_api.common.error import ( NoAsidFound, @@ -13,64 +11,9 @@ NoPatientFound, ) from gateway_api.get_structured_record.request import GetStructuredRecordRequest -from gateway_api.pds_search import PdsClient, PdsSearchResults -from gateway_api.provider_request import GpProviderClient - - -@dataclass -class SdsSearchResults: - """ - Stub SDS search results dataclass. - - Replace this with the real one once it's implemented. - - :param asid: Accredited System ID. - :param endpoint: Endpoint URL associated with the organisation, if applicable. - """ - - asid: str - endpoint: str | None - - -class SdsClient: - """ - Stub SDS client for obtaining ASID from ODS code. - - Replace this with the real one once it's implemented. - """ - - SANDBOX_URL = "https://example.invalid/sds" - - def __init__( - self, - auth_token: str, - base_url: str = SANDBOX_URL, - timeout: int = 10, - ) -> None: - """ - Create an SDS client. - - :param auth_token: Authentication token to present to SDS. - :param base_url: Base URL for SDS. - :param timeout: Timeout in seconds for SDS calls. - """ - self.auth_token = auth_token - self.base_url = base_url - self.timeout = timeout - - def get_org_details(self, ods_code: str) -> SdsSearchResults | None: - """ - Retrieve SDS org details for a given ODS code. - - This is a placeholder implementation that always returns an ASID and endpoint. - - :param ods_code: ODS code to look up. - :returns: SDS search results or ``None`` if not found. - """ - # Placeholder implementation - return SdsSearchResults( - asid=f"asid_{ods_code}", endpoint="https://example-provider.org/endpoint" - ) +from gateway_api.pds import PdsClient, PdsSearchResults +from gateway_api.provider import GpProviderClient +from gateway_api.sds import SdsClient, SdsSearchResults class Controller: diff --git a/gateway-api/src/gateway_api/pds/__init__.py b/gateway-api/src/gateway_api/pds/__init__.py new file mode 100644 index 00000000..7c687699 --- /dev/null +++ b/gateway-api/src/gateway_api/pds/__init__.py @@ -0,0 +1,9 @@ +"""PDS (Personal Demographics Service) client and data structures.""" + +from gateway_api.pds.client import PdsClient +from gateway_api.pds.search_results import PdsSearchResults + +__all__ = [ + "PdsClient", + "PdsSearchResults", +] diff --git a/gateway-api/src/gateway_api/pds_search.py b/gateway-api/src/gateway_api/pds/client.py similarity index 94% rename from gateway-api/src/gateway_api/pds_search.py rename to gateway-api/src/gateway_api/pds/client.py index 71099afa..0cd70660 100644 --- a/gateway-api/src/gateway_api/pds_search.py +++ b/gateway-api/src/gateway_api/pds/client.py @@ -20,7 +20,6 @@ import uuid from collections.abc import Callable -from dataclasses import dataclass from datetime import date, datetime, timezone from typing import cast @@ -28,6 +27,7 @@ from stubs.stub_pds import PdsFhirApiStub from gateway_api.common.error import PdsRequestFailed +from gateway_api.pds.search_results import PdsSearchResults # Recursive JSON-like structure typing used for parsed FHIR bodies. type ResultStructure = str | dict[str, "ResultStructure"] | list["ResultStructure"] @@ -38,29 +38,6 @@ type GetCallable = Callable[..., requests.Response] -@dataclass -class PdsSearchResults: - """ - 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. diff --git a/gateway-api/src/gateway_api/pds/search_results.py b/gateway-api/src/gateway_api/pds/search_results.py new file mode 100644 index 00000000..fc6e929e --- /dev/null +++ b/gateway-api/src/gateway_api/pds/search_results.py @@ -0,0 +1,26 @@ +"""PDS search result data structures.""" + +from dataclasses import dataclass + + +@dataclass +class PdsSearchResults: + """ + 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 diff --git a/gateway-api/src/gateway_api/test_pds_search.py b/gateway-api/src/gateway_api/pds/test_client.py similarity index 99% rename from gateway-api/src/gateway_api/test_pds_search.py rename to gateway-api/src/gateway_api/pds/test_client.py index e786c242..4b94817f 100644 --- a/gateway-api/src/gateway_api/test_pds_search.py +++ b/gateway-api/src/gateway_api/pds/test_client.py @@ -13,9 +13,9 @@ from stubs.stub_pds import PdsFhirApiStub from gateway_api.common.error import PdsRequestFailed -from gateway_api.pds_search import ( +from gateway_api.pds.client import ( PdsClient, - ResultList, + ResultList, # TODO: Use FHIR class here ) @@ -117,7 +117,7 @@ def _capturing_get( stub.get = _capturing_get # type: ignore[method-assign] # Monkeypatch PdsFhirApiStub so PdsClient uses our test stub - import gateway_api.pds_search as pds_module + import gateway_api.pds.client as pds_module monkeypatch.setattr( pds_module, diff --git a/gateway-api/src/gateway_api/provider/__init__.py b/gateway-api/src/gateway_api/provider/__init__.py new file mode 100644 index 00000000..1cc394f9 --- /dev/null +++ b/gateway-api/src/gateway_api/provider/__init__.py @@ -0,0 +1,7 @@ +"""Provider client for fetching structured patient records from GP systems.""" + +from gateway_api.provider.client import GpProviderClient + +__all__ = [ + "GpProviderClient", +] diff --git a/gateway-api/src/gateway_api/provider_request.py b/gateway-api/src/gateway_api/provider/client.py similarity index 100% rename from gateway-api/src/gateway_api/provider_request.py rename to gateway-api/src/gateway_api/provider/client.py diff --git a/gateway-api/src/gateway_api/test_provider_request.py b/gateway-api/src/gateway_api/provider/test_client.py similarity index 97% rename from gateway-api/src/gateway_api/test_provider_request.py rename to gateway-api/src/gateway_api/provider/test_client.py index 6af8a0aa..5b540d4d 100644 --- a/gateway-api/src/gateway_api/test_provider_request.py +++ b/gateway-api/src/gateway_api/provider/test_client.py @@ -13,9 +13,8 @@ from requests.structures import CaseInsensitiveDict from stubs.stub_provider import GpProviderStub -from gateway_api import provider_request from gateway_api.common.error import SdsRequestFailed -from gateway_api.provider_request import GpProviderClient +from gateway_api.provider import GpProviderClient, client ars_interactionId = ( "urn:nhs:names:services:gpconnect:structured" @@ -61,7 +60,7 @@ def _fake_post( trace_id=headers.get("Ssp-TraceID", "dummy-trace-id"), body=data ) - monkeypatch.setattr(provider_request, "post", _fake_post) + monkeypatch.setattr(client, "post", _fake_post) return capture diff --git a/gateway-api/src/gateway_api/sds/__init__.py b/gateway-api/src/gateway_api/sds/__init__.py new file mode 100644 index 00000000..8f6e5ec0 --- /dev/null +++ b/gateway-api/src/gateway_api/sds/__init__.py @@ -0,0 +1,7 @@ +from gateway_api.sds.client import SdsClient +from gateway_api.sds.search_results import SdsSearchResults + +__all__ = [ + "SdsClient", + "SdsSearchResults", +] diff --git a/gateway-api/src/gateway_api/sds/client.py b/gateway-api/src/gateway_api/sds/client.py new file mode 100644 index 00000000..a78ab923 --- /dev/null +++ b/gateway-api/src/gateway_api/sds/client.py @@ -0,0 +1,42 @@ +from gateway_api.sds.search_results import SdsSearchResults + + +class SdsClient: + """ + Stub SDS client for obtaining ASID from ODS code. + + Replace this with the real one once it's implemented. + """ + + SANDBOX_URL = "https://example.invalid/sds" + + def __init__( + self, + auth_token: str, + base_url: str = SANDBOX_URL, + timeout: int = 10, + ) -> None: + """ + Create an SDS client. + + :param auth_token: Authentication token to present to SDS. + :param base_url: Base URL for SDS. + :param timeout: Timeout in seconds for SDS calls. + """ + self.auth_token = auth_token + self.base_url = base_url + self.timeout = timeout + + def get_org_details(self, ods_code: str) -> SdsSearchResults | None: + """ + Retrieve SDS org details for a given ODS code. + + This is a placeholder implementation that always returns an ASID and endpoint. + + :param ods_code: ODS code to look up. + :returns: SDS search results or ``None`` if not found. + """ + # Placeholder implementation + return SdsSearchResults( + asid=f"asid_{ods_code}", endpoint="https://example-provider.org/endpoint" + ) diff --git a/gateway-api/src/gateway_api/sds/search_results.py b/gateway-api/src/gateway_api/sds/search_results.py new file mode 100644 index 00000000..ad956b89 --- /dev/null +++ b/gateway-api/src/gateway_api/sds/search_results.py @@ -0,0 +1,16 @@ +from dataclasses import dataclass + + +@dataclass +class SdsSearchResults: + """ + Stub SDS search results dataclass. + + Replace this with the real one once it's implemented. + + :param asid: Accredited System ID. + :param endpoint: Endpoint URL associated with the organisation, if applicable. + """ + + asid: str + endpoint: str | None diff --git a/gateway-api/src/gateway_api/test_controller.py b/gateway-api/src/gateway_api/test_controller.py index b3a0b5fd..6be83160 100644 --- a/gateway-api/src/gateway_api/test_controller.py +++ b/gateway-api/src/gateway_api/test_controller.py @@ -21,11 +21,9 @@ NoOrganisationFound, NoPatientFound, ) -from gateway_api.controller import ( - Controller, - SdsSearchResults, -) +from gateway_api.controller import Controller from gateway_api.get_structured_record.request import GetStructuredRecordRequest +from gateway_api.sds import SdsSearchResults # ----------------------------- From 416958622e45fb279e1bb699e608935b25d8b3c4 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Tue, 10 Feb 2026 13:09:09 +0000 Subject: [PATCH 24/27] [GPCAPIM-275]: Add example Patient resource - taken from PDS FHIR's sandbox --- gateway-api/src/gateway_api/pds/client.py | 2 +- gateway-api/src/gateway_api/pds/pds_search.py | 354 ++++++++++++++++ .../src/gateway_api/pds/test_client.py | 2 +- .../src/gateway_api/provider/client.py | 2 +- .../src/gateway_api/provider/test_client.py | 2 +- .../src/gateway_api/test_provider_request.py | 220 ++++++++++ gateway-api/stubs/stubs/data/__init__.py | 0 .../stubs/stubs/data/bundles/__init__.py | 20 + .../stubs/stubs/data/patients/__init__.py | 19 + .../data/patients/alice_jones_9999999999.json | 34 ++ .../data/patients/jane_smith_9000000009.json | 24 ++ gateway-api/stubs/stubs/pds/__init__.py | 0 .../stubs/stubs/{stub_pds.py => pds/stub.py} | 62 +-- gateway-api/stubs/stubs/provider/__init__.py | 0 .../{stub_provider.py => provider/stub.py} | 47 +-- .../tests/acceptance/steps/happy_path.py | 4 +- ...GatewayAPIConsumer-GatewayAPIProvider.json | 36 +- .../tests/contract/test_consumer_contract.py | 36 +- .../tests/data/patient/pds_fhir_example.json | 390 ++++++++++++++++++ .../integration/test_get_structured_record.py | 4 +- 20 files changed, 1120 insertions(+), 138 deletions(-) create mode 100644 gateway-api/src/gateway_api/pds/pds_search.py create mode 100644 gateway-api/src/gateway_api/test_provider_request.py create mode 100644 gateway-api/stubs/stubs/data/__init__.py create mode 100644 gateway-api/stubs/stubs/data/bundles/__init__.py create mode 100644 gateway-api/stubs/stubs/data/patients/__init__.py create mode 100644 gateway-api/stubs/stubs/data/patients/alice_jones_9999999999.json create mode 100644 gateway-api/stubs/stubs/data/patients/jane_smith_9000000009.json create mode 100644 gateway-api/stubs/stubs/pds/__init__.py rename gateway-api/stubs/stubs/{stub_pds.py => pds/stub.py} (84%) create mode 100644 gateway-api/stubs/stubs/provider/__init__.py rename gateway-api/stubs/stubs/{stub_provider.py => provider/stub.py} (69%) create mode 100644 gateway-api/tests/data/patient/pds_fhir_example.json diff --git a/gateway-api/src/gateway_api/pds/client.py b/gateway-api/src/gateway_api/pds/client.py index 0cd70660..b8e22a4b 100644 --- a/gateway-api/src/gateway_api/pds/client.py +++ b/gateway-api/src/gateway_api/pds/client.py @@ -24,7 +24,7 @@ from typing import cast import requests -from stubs.stub_pds import PdsFhirApiStub +from stubs.pds.stub import PdsFhirApiStub from gateway_api.common.error import PdsRequestFailed from gateway_api.pds.search_results import PdsSearchResults diff --git a/gateway-api/src/gateway_api/pds/pds_search.py b/gateway-api/src/gateway_api/pds/pds_search.py new file mode 100644 index 00000000..b8e22a4b --- /dev/null +++ b/gateway-api/src/gateway_api/pds/pds_search.py @@ -0,0 +1,354 @@ +""" +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. +""" + +import uuid +from collections.abc import Callable +from datetime import date, datetime, timezone +from typing import cast + +import requests +from stubs.pds.stub import PdsFhirApiStub + +from gateway_api.common.error import PdsRequestFailed +from gateway_api.pds.search_results import PdsSearchResults + +# 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] + +# Type for stub get method +type GetCallable = Callable[..., requests.Response] + + +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:`PdsSearchResults` instance when a patient can be + extracted, otherwise ``None``. + + **Usage example**:: + + pds = PdsClient( + auth_token="YOUR_ACCESS_TOKEN", + 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, + base_url: str = SANDBOX_URL, + timeout: int = 10, + ignore_dates: bool = False, + ) -> None: + """ + Create a PDS client. + + :param auth_token: OAuth2 bearer token (without the ``"Bearer "`` prefix). + :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 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.base_url = base_url.rstrip("/") + self.timeout = timeout + self.ignore_dates = ignore_dates + self.stub = PdsFhirApiStub() + + # TODO: Put this back to using the environment variable + # if os.environ.get("STUB_PDS", None): + self.get_method: GetCallable = self.stub.get + # else: + # self.get_method: GetCallable = requests.get + + 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()), + "Accept": "application/fhir+json", + "Authorization": f"Bearer {self.auth_token}", + } + + # 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: str, + request_id: str | None = None, + correlation_id: str | None = None, + timeout: int | None = None, + ) -> PdsSearchResults | 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:`PdsSearchResults`. + + :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:`PdsSearchResults` instance if a patient can be extracted, + otherwise ``None``. + """ + headers = self._build_headers( + request_id=request_id, + correlation_id=correlation_id, + ) + + url = f"{self.base_url}/Patient/{nhs_number}" + + # This normally calls requests.get, but if STUB_PDS is set it uses the stub. + response = self.get_method( + url, + headers=headers, + params={}, + timeout=timeout or self.timeout, + ) + + try: + response.raise_for_status() + except requests.HTTPError as err: + raise PdsRequestFailed(error_reason=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 + ) -> PdsSearchResults | None: + """ + Extract a single :class:`PdsSearchResults` 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:`PdsSearchResults` 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 PdsSearchResults( + 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/pds/test_client.py b/gateway-api/src/gateway_api/pds/test_client.py index 4b94817f..bc65b46f 100644 --- a/gateway-api/src/gateway_api/pds/test_client.py +++ b/gateway-api/src/gateway_api/pds/test_client.py @@ -10,7 +10,7 @@ import pytest import requests from requests.structures import CaseInsensitiveDict -from stubs.stub_pds import PdsFhirApiStub +from stubs.pds.stub import PdsFhirApiStub from gateway_api.common.error import PdsRequestFailed from gateway_api.pds.client import ( diff --git a/gateway-api/src/gateway_api/provider/client.py b/gateway-api/src/gateway_api/provider/client.py index c0e3563a..cd88e8c1 100644 --- a/gateway-api/src/gateway_api/provider/client.py +++ b/gateway-api/src/gateway_api/provider/client.py @@ -26,7 +26,7 @@ from urllib.parse import urljoin from requests import HTTPError, Response, post -from stubs.stub_provider import stub_post +from stubs.provider.stub import stub_post from gateway_api.common.error import SdsRequestFailed diff --git a/gateway-api/src/gateway_api/provider/test_client.py b/gateway-api/src/gateway_api/provider/test_client.py index 5b540d4d..d9a566ff 100644 --- a/gateway-api/src/gateway_api/provider/test_client.py +++ b/gateway-api/src/gateway_api/provider/test_client.py @@ -11,7 +11,7 @@ import pytest from requests import Response from requests.structures import CaseInsensitiveDict -from stubs.stub_provider import GpProviderStub +from stubs.provider.stub import GpProviderStub from gateway_api.common.error import SdsRequestFailed from gateway_api.provider import GpProviderClient, client diff --git a/gateway-api/src/gateway_api/test_provider_request.py b/gateway-api/src/gateway_api/test_provider_request.py new file mode 100644 index 00000000..d9a566ff --- /dev/null +++ b/gateway-api/src/gateway_api/test_provider_request.py @@ -0,0 +1,220 @@ +""" +Unit tests for :mod:`gateway_api.provider_request`. + +This module contains unit tests for the `GpProviderClient` class, which is responsible +for interacting with the GPProvider FHIR API. + +""" + +from typing import Any + +import pytest +from requests import Response +from requests.structures import CaseInsensitiveDict +from stubs.provider.stub import GpProviderStub + +from gateway_api.common.error import SdsRequestFailed +from gateway_api.provider import GpProviderClient, client + +ars_interactionId = ( + "urn:nhs:names:services:gpconnect:structured" + ":fhir:operation:gpc.getstructuredrecord-1" +) + + +@pytest.fixture +def stub() -> GpProviderStub: + return GpProviderStub() + + +@pytest.fixture +def mock_request_post( + monkeypatch: pytest.MonkeyPatch, stub: GpProviderStub +) -> dict[str, Any]: + """ + Fixture to patch the `requests.post` method for testing. + + This fixture intercepts calls to `requests.post` and routes them to the + stub provider. It also captures the most recent request details, such as + headers, body, and URL, for verification in tests. + + Returns: + dict[str, Any]: A dictionary containing the captured request details. + """ + capture: dict[str, Any] = {} + + def _fake_post( + url: str, + headers: CaseInsensitiveDict[str], + data: str, + timeout: int, # NOQA ARG001 (unused in stub) + ) -> Response: + """A fake requests.post implementation.""" + + capture["headers"] = dict(headers) + capture["data"] = data + capture["url"] = url + + # Provide dummy or captured arguments as required by the stub signature + return stub.access_record_structured( + trace_id=headers.get("Ssp-TraceID", "dummy-trace-id"), body=data + ) + + monkeypatch.setattr(client, "post", _fake_post) + return capture + + +def test_valid_gpprovider_access_structured_record_makes_request_correct_url_post_200( + mock_request_post: dict[str, Any], +) -> None: + """ + Test that the `access_structured_record` method constructs the correct URL + for the GPProvider FHIR API request and receives a 200 OK response. + + This test verifies that the URL includes the correct FHIR base path and + operation for accessing a structured patient record. + """ + provider_asid = "200000001154" + consumer_asid = "200000001152" + provider_endpoint = "https://test.com" + trace_id = "some_uuid_value" + + client = GpProviderClient( + provider_endpoint=provider_endpoint, + provider_asid=provider_asid, + consumer_asid=consumer_asid, + ) + + result = client.access_structured_record(trace_id, "body") + + captured_url = mock_request_post.get("url", provider_endpoint) + + assert ( + captured_url + == provider_endpoint + "/FHIR/STU3/patient/$gpc.getstructuredrecord" + ) + assert result.status_code == 200 + + +def test_valid_gpprovider_access_structured_record_with_correct_headers_post_200( + mock_request_post: dict[str, Any], +) -> None: + """ + Test that the `access_structured_record` method includes the correct headers + in the GPProvider FHIR API request and receives a 200 OK response. + + This test verifies that the headers include: + - Content-Type and Accept headers for FHIR+JSON. + - Ssp-TraceID, Ssp-From, Ssp-To, and Ssp-InteractionID for GPConnect. + """ + provider_asid = "200000001154" + consumer_asid = "200000001152" + provider_endpoint = "https://test.com" + trace_id = "some_uuid_value" + + client = GpProviderClient( + provider_endpoint=provider_endpoint, + provider_asid=provider_asid, + consumer_asid=consumer_asid, + ) + expected_headers = { + "Content-Type": "application/fhir+json", + "Accept": "application/fhir+json", + "Ssp-TraceID": str(trace_id), + "Ssp-From": consumer_asid, + "Ssp-To": provider_asid, + "Ssp-InteractionID": ars_interactionId, + } + + result = client.access_structured_record(trace_id, "body") + + captured_headers = mock_request_post["headers"] + + assert expected_headers == captured_headers + assert result.status_code == 200 + + +def test_valid_gpprovider_access_structured_record_with_correct_body_200( + mock_request_post: dict[str, Any], +) -> None: + """ + Test that the `access_structured_record` method includes the correct body + in the GPProvider FHIR API request and receives a 200 OK response. + + This test verifies that the request body matches the expected FHIR parameters + resource sent to the GPProvider API. + """ + provider_asid = "200000001154" + consumer_asid = "200000001152" + provider_endpoint = "https://test.com" + trace_id = "some_uuid_value" + + request_body = "some_FHIR_request_params" + + client = GpProviderClient( + provider_endpoint=provider_endpoint, + provider_asid=provider_asid, + consumer_asid=consumer_asid, + ) + + result = client.access_structured_record(trace_id, request_body) + + captured_body = mock_request_post["data"] + + assert result.status_code == 200 + assert captured_body == request_body + + +def test_valid_gpprovider_access_structured_record_returns_stub_response_200( + mock_request_post: dict[str, Any], # NOQA ARG001 (Mock not called directly) + stub: GpProviderStub, +) -> None: + """ + Test that the `access_structured_record` method returns the same response + as provided by the stub provider. + + This test verifies that the response from the GPProvider FHIR API matches + the expected response, including the status code and content. + """ + provider_asid = "200000001154" + consumer_asid = "200000001152" + provider_endpoint = "https://test.com" + trace_id = "some_uuid_value" + + client = GpProviderClient( + provider_endpoint=provider_endpoint, + provider_asid=provider_asid, + consumer_asid=consumer_asid, + ) + + expected_response = stub.access_record_structured(trace_id, "body") + + result = client.access_structured_record(trace_id, "body") + + assert result.status_code == 200 + assert result.content == expected_response.content + + +def test_access_structured_record_raises_external_service_error( + mock_request_post: dict[str, Any], # NOQA ARG001 (Mock not called directly) +) -> None: + """ + Test that the `access_structured_record` method raises an `SdsRequestFailed` + when the GPProvider FHIR API request fails with an HTTP error. + """ + provider_asid = "200000001154" + consumer_asid = "200000001152" + provider_endpoint = "https://test.com" + trace_id = "invalid for test" + + client = GpProviderClient( + provider_endpoint=provider_endpoint, + provider_asid=provider_asid, + consumer_asid=consumer_asid, + ) + + with pytest.raises( + SdsRequestFailed, + match="SDS FHIR API request failed: Bad Request", + ): + client.access_structured_record(trace_id, "body") diff --git a/gateway-api/stubs/stubs/data/__init__.py b/gateway-api/stubs/stubs/data/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/gateway-api/stubs/stubs/data/bundles/__init__.py b/gateway-api/stubs/stubs/data/bundles/__init__.py new file mode 100644 index 00000000..d714c29d --- /dev/null +++ b/gateway-api/stubs/stubs/data/bundles/__init__.py @@ -0,0 +1,20 @@ +from typing import Any + +from stubs.data.patients import Patients + + +class Bundles: + @staticmethod + def _wrap_patient_in_bundle(patient: dict[str, Any]) -> dict[str, Any]: + return { + "resourceType": "Bundle", + "type": "collection", + "meta": { + "profile": [ + "https://fhir.nhs.uk/STU3/StructureDefinition/GPConnect-StructuredRecord-Bundle-1" + ] + }, + "entry": [{"resource": patient}], + } + + ALICE_JONES_9999999999 = _wrap_patient_in_bundle(Patients.ALICE_JONES_9999999999) diff --git a/gateway-api/stubs/stubs/data/patients/__init__.py b/gateway-api/stubs/stubs/data/patients/__init__.py new file mode 100644 index 00000000..27cc2751 --- /dev/null +++ b/gateway-api/stubs/stubs/data/patients/__init__.py @@ -0,0 +1,19 @@ +import json +import pathlib +from typing import Any + + +def _path_to_here() -> pathlib.Path: + return pathlib.Path(__file__).parent + + +class Patients: + @staticmethod + def load_patient(filename: str) -> dict[str, Any]: + with open(_path_to_here() / filename, encoding="utf-8") as f: + patient: dict[str, Any] = json.load(f) + return patient + + JANE_SMITH_9000000009 = load_patient("jane_smith_9000000009.json") + + ALICE_JONES_9999999999 = load_patient("alice_jones_9999999999.json") diff --git a/gateway-api/stubs/stubs/data/patients/alice_jones_9999999999.json b/gateway-api/stubs/stubs/data/patients/alice_jones_9999999999.json new file mode 100644 index 00000000..558a4e30 --- /dev/null +++ b/gateway-api/stubs/stubs/data/patients/alice_jones_9999999999.json @@ -0,0 +1,34 @@ +{ + "resourceType": "Patient", + "id": "9999999999", + "meta": { + "versionId": "1", + "lastUpdated": "2020-01-01T00:00:00Z" + }, + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "9999999999" + } + ], + "name": [ + { + "use": "official", + "family": "Jones", + "given": ["Alice"], + "period": {"start": "1900-01-01", "end": "9999-12-31"} + } + ], + "gender": "female", + "birthDate": "1980-01-01", + "generalPractitioner": [ + { + "id": "1", + "type": "Organization", + "identifier": { + "value": "A12345", + "period": {"start": "2020-01-01", "end": "9999-12-31"} + } + } + ] +} diff --git a/gateway-api/stubs/stubs/data/patients/jane_smith_9000000009.json b/gateway-api/stubs/stubs/data/patients/jane_smith_9000000009.json new file mode 100644 index 00000000..81b0ce5f --- /dev/null +++ b/gateway-api/stubs/stubs/data/patients/jane_smith_9000000009.json @@ -0,0 +1,24 @@ +{ + "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" +} diff --git a/gateway-api/stubs/stubs/pds/__init__.py b/gateway-api/stubs/stubs/pds/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/gateway-api/stubs/stubs/stub_pds.py b/gateway-api/stubs/stubs/pds/stub.py similarity index 84% rename from gateway-api/stubs/stubs/stub_pds.py rename to gateway-api/stubs/stubs/pds/stub.py index 6233d7b1..e4b336c0 100644 --- a/gateway-api/stubs/stubs/stub_pds.py +++ b/gateway-api/stubs/stubs/pds/stub.py @@ -14,6 +14,8 @@ from requests import Response from requests.structures import CaseInsensitiveDict +from stubs.data.patients import Patients + def _create_response( status_code: int, @@ -70,69 +72,13 @@ def __init__(self, strict_headers: bool = True) -> None: # 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", - }, + patient=Patients.JANE_SMITH_9000000009, version_id=1, ) self.upsert_patient( nhs_number="9999999999", - patient={ - "resourceType": "Patient", - "id": "9999999999", - "meta": { - "versionId": "1", - "lastUpdated": "2020-01-01T00:00:00Z", - }, - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "9999999999", - } - ], - "name": [ - { - "use": "official", - "family": "Jones", - "given": ["Alice"], - "period": {"start": "1900-01-01", "end": "9999-12-31"}, - } - ], - "gender": "female", - "birthDate": "1980-01-01", - "generalPractitioner": [ - { - "id": "1", - "type": "Organization", - "identifier": { - "value": "A12345", - "period": {"start": "2020-01-01", "end": "9999-12-31"}, - }, - } - ], - }, + patient=Patients.ALICE_JONES_9999999999, version_id=1, ) diff --git a/gateway-api/stubs/stubs/provider/__init__.py b/gateway-api/stubs/stubs/provider/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/gateway-api/stubs/stubs/stub_provider.py b/gateway-api/stubs/stubs/provider/stub.py similarity index 69% rename from gateway-api/stubs/stubs/stub_provider.py rename to gateway-api/stubs/stubs/provider/stub.py index 2d0c96ba..cc723fa3 100644 --- a/gateway-api/stubs/stubs/stub_provider.py +++ b/gateway-api/stubs/stubs/provider/stub.py @@ -28,6 +28,8 @@ from requests import Response from requests.structures import CaseInsensitiveDict +from stubs.data.bundles import Bundles + def _create_response( status_code: int, @@ -64,49 +66,6 @@ class GpProviderStub: # https://simplifier.net/guide/gp-connect-access-record-structured/Home/Examples/Allergy-examples?version=1.6.2 """ - # Example patient resource - patient_bundle = { - "resourceType": "Bundle", - "type": "collection", - "meta": { - "profile": [ - "https://fhir.nhs.uk/STU3/StructureDefinition/GPConnect-StructuredRecord-Bundle-1" - ] - }, - "entry": [ - { - "resource": { - "resourceType": "Patient", - "id": "04603d77-1a4e-4d63-b246-d7504f8bd833", - "meta": { - "versionId": "1469448000000", - "profile": [ - "https://fhir.nhs.uk/STU3/StructureDefinition/CareConnect-GPC-Patient-1" - ], - }, - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "9999999999", - } - ], - "active": True, - "name": [ - { - "use": "official", - "text": "JACKSON Jane (Miss)", - "family": "Jackson", - "given": ["Jane"], - "prefix": ["Miss"], - } - ], - "gender": "female", - "birthDate": "1952-05-31", - } - } - ], - } - def access_record_structured( self, trace_id: str, @@ -122,7 +81,7 @@ def access_record_structured( stub_response = _create_response( status_code=200, headers=CaseInsensitiveDict({"Content-Type": "application/fhir+json"}), - content=json.dumps(self.patient_bundle).encode("utf-8"), + content=json.dumps(Bundles.ALICE_JONES_9999999999).encode("utf-8"), reason="OK", ) diff --git a/gateway-api/tests/acceptance/steps/happy_path.py b/gateway-api/tests/acceptance/steps/happy_path.py index 3485f224..bf777cbe 100644 --- a/gateway-api/tests/acceptance/steps/happy_path.py +++ b/gateway-api/tests/acceptance/steps/happy_path.py @@ -6,7 +6,7 @@ import requests from fhir.parameters import Parameters from pytest_bdd import given, parsers, then, when -from stubs.stub_provider import GpProviderStub +from stubs.data.bundles import Bundles from tests.acceptance.conftest import ResponseContext from tests.conftest import Client @@ -60,6 +60,6 @@ def check_status_code(response_context: ResponseContext, expected_status: int) - @then("the response should contain the patient bundle from the provider") def check_response_matches_provider(response_context: ResponseContext) -> None: assert response_context.response, "Response has not been set." - assert response_context.response.json() == GpProviderStub.patient_bundle, ( + assert response_context.response.json() == Bundles.ALICE_JONES_9999999999, ( "Expected response payload does not match actual response payload." ) diff --git a/gateway-api/tests/contract/pacts/GatewayAPIConsumer-GatewayAPIProvider.json b/gateway-api/tests/contract/pacts/GatewayAPIConsumer-GatewayAPIProvider.json index 12c8a5cf..7c93ed79 100644 --- a/gateway-api/tests/contract/pacts/GatewayAPIConsumer-GatewayAPIProvider.json +++ b/gateway-api/tests/contract/pacts/GatewayAPIConsumer-GatewayAPIProvider.json @@ -55,10 +55,22 @@ "entry": [ { "resource": { - "active": true, - "birthDate": "1952-05-31", + "birthDate": "1980-01-01", "gender": "female", - "id": "04603d77-1a4e-4d63-b246-d7504f8bd833", + "generalPractitioner": [ + { + "id": "1", + "identifier": { + "period": { + "end": "9999-12-31", + "start": "2020-01-01" + }, + "value": "A12345" + }, + "type": "Organization" + } + ], + "id": "9999999999", "identifier": [ { "system": "https://fhir.nhs.uk/Id/nhs-number", @@ -66,21 +78,19 @@ } ], "meta": { - "profile": [ - "https://fhir.nhs.uk/STU3/StructureDefinition/CareConnect-GPC-Patient-1" - ], - "versionId": "1469448000000" + "lastUpdated": "2020-01-01T00:00:00Z", + "versionId": "1" }, "name": [ { - "family": "Jackson", + "family": "Jones", "given": [ - "Jane" - ], - "prefix": [ - "Miss" + "Alice" ], - "text": "JACKSON Jane (Miss)", + "period": { + "end": "9999-12-31", + "start": "1900-01-01" + }, "use": "official" } ], diff --git a/gateway-api/tests/contract/test_consumer_contract.py b/gateway-api/tests/contract/test_consumer_contract.py index cf1998c3..7c9bffee 100644 --- a/gateway-api/tests/contract/test_consumer_contract.py +++ b/gateway-api/tests/contract/test_consumer_contract.py @@ -34,12 +34,10 @@ def test_get_structured_record(self) -> None: { "resource": { "resourceType": "Patient", - "id": "04603d77-1a4e-4d63-b246-d7504f8bd833", + "id": "9999999999", "meta": { - "versionId": "1469448000000", - "profile": [ - "https://fhir.nhs.uk/STU3/StructureDefinition/CareConnect-GPC-Patient-1" - ], + "versionId": "1", + "lastUpdated": "2020-01-01T00:00:00Z", }, "identifier": [ { @@ -47,18 +45,29 @@ def test_get_structured_record(self) -> None: "value": "9999999999", } ], - "active": True, "name": [ { "use": "official", - "text": "JACKSON Jane (Miss)", - "family": "Jackson", - "given": ["Jane"], - "prefix": ["Miss"], + "family": "Jones", + "given": ["Alice"], + "period": {"start": "1900-01-01", "end": "9999-12-31"}, } ], "gender": "female", - "birthDate": "1952-05-31", + "birthDate": "1980-01-01", + "generalPractitioner": [ + { + "id": "1", + "type": "Organization", + "identifier": { + "value": "A12345", + "period": { + "start": "2020-01-01", + "end": "9999-12-31", + }, + }, + } + ], } } ], @@ -128,10 +137,7 @@ def test_get_structured_record(self) -> None: assert body["type"] == "collection" assert len(body["entry"]) == 1 assert body["entry"][0]["resource"]["resourceType"] == "Patient" - assert ( - body["entry"][0]["resource"]["id"] - == "04603d77-1a4e-4d63-b246-d7504f8bd833" - ) + assert body["entry"][0]["resource"]["id"] == "9999999999" assert ( body["entry"][0]["resource"]["identifier"][0]["value"] == "9999999999" ) diff --git a/gateway-api/tests/data/patient/pds_fhir_example.json b/gateway-api/tests/data/patient/pds_fhir_example.json new file mode 100644 index 00000000..2c590256 --- /dev/null +++ b/gateway-api/tests/data/patient/pds_fhir_example.json @@ -0,0 +1,390 @@ +{ + "resourceType": "Patient", + "id": "9000000009", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": "9000000009", + "extension": [ + { + "url": "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-NHSNumberVerificationStatus", + "valueCodeableConcept": { + "coding": [ + { + "system": "https://fhir.hl7.org.uk/CodeSystem/UKCore-NHSNumberVerificationStatus", + "version": "1.0.0", + "code": "01", + "display": "Number present and verified" + } + ] + } + } + ] + } + ], + "meta": { + "versionId": "2", + "security": [ + { + "system": "http://terminology.hl7.org/CodeSystem/v3-Confidentiality", + "code": "U", + "display": "unrestricted" + } + ] + }, + "name": [ + { + "id": "123", + "use": "usual", + "period": { + "start": "2020-01-01", + "end": "2021-12-31" + }, + "given": [ + "Jane" + ], + "family": "Smith", + "prefix": [ + "Mrs" + ] + } + ], + "gender": "female", + "birthDate": "2010-10-22", + "multipleBirthInteger": 1, + "deceasedDateTime": "2010-10-22T00:00:00+00:00", + "generalPractitioner": [ + { + "id": "254406A3", + "type": "Organization", + "identifier": { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "Y12345", + "period": { + "start": "2020-01-01", + "end": "2021-12-31" + } + } + } + ], + "managingOrganization": { + "type": "Organization", + "identifier": { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "Y12345", + "period": { + "start": "2020-01-01", + "end": "2021-12-31" + } + } + }, + "extension": [ + { + "url": "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-NominatedPharmacy", + "valueReference": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "Y12345" + } + } + }, + { + "url": "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-PreferredDispenserOrganization", + "valueReference": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "Y23456" + } + } + }, + { + "url": "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-MedicalApplianceSupplier", + "valueReference": { + "identifier": { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": "Y34567" + } + } + }, + { + "url": "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-DeathNotificationStatus", + "extension": [ + { + "url": "deathNotificationStatus", + "valueCodeableConcept": { + "coding": [ + { + "system": "https://fhir.hl7.org.uk/CodeSystem/UKCore-DeathNotificationStatus", + "version": "1.0.0", + "code": "2", + "display": "Formal - death notice received from Registrar of Deaths" + } + ] + } + }, + { + "url": "systemEffectiveDate", + "valueDateTime": "2010-10-22T00:00:00+00:00" + } + ] + }, + { + "url": "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-NHSCommunication", + "extension": [ + { + "url": "language", + "valueCodeableConcept": { + "coding": [ + { + "system": "https://fhir.hl7.org.uk/CodeSystem/UKCore-HumanLanguage", + "version": "1.0.0", + "code": "fr", + "display": "French" + } + ] + } + }, + { + "url": "interpreterRequired", + "valueBoolean": true + } + ] + }, + { + "url": "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-ContactPreference", + "extension": [ + { + "url": "PreferredWrittenCommunicationFormat", + "valueCodeableConcept": { + "coding": [ + { + "system": "https://fhir.hl7.org.uk/CodeSystem/UKCore-PreferredWrittenCommunicationFormat", + "code": "12", + "display": "Braille" + } + ] + } + }, + { + "url": "PreferredContactMethod", + "valueCodeableConcept": { + "coding": [ + { + "system": "https://fhir.hl7.org.uk/CodeSystem/UKCore-PreferredContactMethod", + "code": "1", + "display": "Letter" + } + ] + } + }, + { + "url": "PreferredContactTimes", + "valueString": "Not after 7pm" + } + ] + }, + { + "url": "http://hl7.org/fhir/StructureDefinition/patient-birthPlace", + "valueAddress": { + "city": "Manchester", + "district": "Greater Manchester", + "country": "GBR" + } + }, + { + "url": "https://fhir.nhs.uk/StructureDefinition/Extension-PDS-RemovalFromRegistration", + "extension": [ + { + "url": "removalFromRegistrationCode", + "valueCodeableConcept": { + "coding": [ + { + "system": "https://fhir.nhs.uk/CodeSystem/PDS-RemovalReasonExitCode", + "code": "SCT", + "display": "Transferred to Scotland" + } + ] + } + }, + { + "url": "effectiveTime", + "valuePeriod": { + "start": "2020-01-01T00:00:00+00:00", + "end": "2021-12-31T00:00:00+00:00" + } + } + ] + } + ], + "telecom": [ + { + "id": "789", + "period": { + "start": "2020-01-01", + "end": "2021-12-31" + }, + "system": "phone", + "value": "01632960587", + "use": "home" + }, + { + "id": "790", + "period": { + "start": "2019-01-01", + "end": "2022-12-31" + }, + "system": "email", + "value": "jane.smith@example.com", + "use": "home" + }, + { + "id": "OC789", + "period": { + "start": "2020-01-01", + "end": "2021-12-31" + }, + "system": "other", + "value": "01632960587", + "use": "home", + "extension": [ + { + "url": "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-OtherContactSystem", + "valueCoding": { + "system": "https://fhir.hl7.org.uk/CodeSystem/UKCore-OtherContactSystem", + "code": "textphone", + "display": "Minicom (Textphone)" + } + } + ] + } + ], + "contact": [ + { + "id": "C123", + "period": { + "start": "2020-01-01", + "end": "2021-12-31" + }, + "relationship": [ + { + "coding": [ + { + "system": "http://terminology.hl7.org/CodeSystem/v2-0131", + "code": "C", + "display": "Emergency Contact" + } + ] + } + ], + "telecom": [ + { + "system": "phone", + "value": "01632960587" + } + ] + } + ], + "address": [ + { + "id": "456", + "period": { + "start": "2020-01-01", + "end": "2021-12-31" + }, + "use": "home", + "line": [ + "1 Trevelyan Square", + "Boar Lane", + "City Centre", + "Leeds", + "West Yorkshire" + ], + "postalCode": "LS1 6AE", + "extension": [ + { + "url": "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-AddressKey", + "extension": [ + { + "url": "type", + "valueCoding": { + "system": "https://fhir.hl7.org.uk/CodeSystem/UKCore-AddressKeyType", + "code": "PAF" + } + }, + { + "url": "value", + "valueString": "12345678" + } + ] + }, + { + "url": "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-AddressKey", + "extension": [ + { + "url": "type", + "valueCoding": { + "system": "https://fhir.hl7.org.uk/CodeSystem/UKCore-AddressKeyType", + "code": "UPRN" + } + }, + { + "url": "value", + "valueString": "123456789012" + } + ] + } + ] + }, + { + "id": "T456", + "period": { + "start": "2020-01-01", + "end": "2021-12-31" + }, + "use": "temp", + "text": "Student Accommodation", + "line": [ + "1 Trevelyan Square", + "Boar Lane", + "City Centre", + "Leeds", + "West Yorkshire" + ], + "postalCode": "LS1 6AE", + "extension": [ + { + "url": "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-AddressKey", + "extension": [ + { + "url": "type", + "valueCoding": { + "system": "https://fhir.hl7.org.uk/CodeSystem/UKCore-AddressKeyType", + "code": "PAF" + } + }, + { + "url": "value", + "valueString": "12345678" + } + ] + }, + { + "url": "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-AddressKey", + "extension": [ + { + "url": "type", + "valueCoding": { + "system": "https://fhir.hl7.org.uk/CodeSystem/UKCore-AddressKeyType", + "code": "UPRN" + } + }, + { + "url": "value", + "valueString": "123456789012" + } + ] + } + ] + } + ] +} diff --git a/gateway-api/tests/integration/test_get_structured_record.py b/gateway-api/tests/integration/test_get_structured_record.py index 32151f2d..a776f0a4 100644 --- a/gateway-api/tests/integration/test_get_structured_record.py +++ b/gateway-api/tests/integration/test_get_structured_record.py @@ -3,7 +3,7 @@ import json from fhir.parameters import Parameters -from stubs.stub_provider import GpProviderStub +from stubs.data.bundles import Bundles from tests.conftest import Client @@ -27,7 +27,7 @@ def test_happy_path_returns_correct_message( response = client.send_to_get_structured_record_endpoint( json.dumps(simple_request_payload) ) - assert response.json() == GpProviderStub.patient_bundle + assert response.json() == Bundles.ALICE_JONES_9999999999 def test_happy_path_content_type( self, client: Client, simple_request_payload: Parameters From be9493701c47ece673d0376c4b1cc85399c6e30d Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Tue, 10 Feb 2026 15:11:55 +0000 Subject: [PATCH 25/27] [GPCAPIM-275]: Move modules to their own directory --- gateway-api/src/gateway_api/pds/pds_search.py | 354 ------------------ 1 file changed, 354 deletions(-) delete mode 100644 gateway-api/src/gateway_api/pds/pds_search.py diff --git a/gateway-api/src/gateway_api/pds/pds_search.py b/gateway-api/src/gateway_api/pds/pds_search.py deleted file mode 100644 index b8e22a4b..00000000 --- a/gateway-api/src/gateway_api/pds/pds_search.py +++ /dev/null @@ -1,354 +0,0 @@ -""" -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. -""" - -import uuid -from collections.abc import Callable -from datetime import date, datetime, timezone -from typing import cast - -import requests -from stubs.pds.stub import PdsFhirApiStub - -from gateway_api.common.error import PdsRequestFailed -from gateway_api.pds.search_results import PdsSearchResults - -# 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] - -# Type for stub get method -type GetCallable = Callable[..., requests.Response] - - -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:`PdsSearchResults` instance when a patient can be - extracted, otherwise ``None``. - - **Usage example**:: - - pds = PdsClient( - auth_token="YOUR_ACCESS_TOKEN", - 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, - base_url: str = SANDBOX_URL, - timeout: int = 10, - ignore_dates: bool = False, - ) -> None: - """ - Create a PDS client. - - :param auth_token: OAuth2 bearer token (without the ``"Bearer "`` prefix). - :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 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.base_url = base_url.rstrip("/") - self.timeout = timeout - self.ignore_dates = ignore_dates - self.stub = PdsFhirApiStub() - - # TODO: Put this back to using the environment variable - # if os.environ.get("STUB_PDS", None): - self.get_method: GetCallable = self.stub.get - # else: - # self.get_method: GetCallable = requests.get - - 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()), - "Accept": "application/fhir+json", - "Authorization": f"Bearer {self.auth_token}", - } - - # 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: str, - request_id: str | None = None, - correlation_id: str | None = None, - timeout: int | None = None, - ) -> PdsSearchResults | 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:`PdsSearchResults`. - - :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:`PdsSearchResults` instance if a patient can be extracted, - otherwise ``None``. - """ - headers = self._build_headers( - request_id=request_id, - correlation_id=correlation_id, - ) - - url = f"{self.base_url}/Patient/{nhs_number}" - - # This normally calls requests.get, but if STUB_PDS is set it uses the stub. - response = self.get_method( - url, - headers=headers, - params={}, - timeout=timeout or self.timeout, - ) - - try: - response.raise_for_status() - except requests.HTTPError as err: - raise PdsRequestFailed(error_reason=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 - ) -> PdsSearchResults | None: - """ - Extract a single :class:`PdsSearchResults` 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:`PdsSearchResults` 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 PdsSearchResults( - 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 From 0c3c8cdddf2f49c4cac7902e90f62d7b97aa8979 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Tue, 10 Feb 2026 15:15:52 +0000 Subject: [PATCH 26/27] [GPCAPIM-275]: Write a test to make number go up --- gateway-api/src/gateway_api/conftest.py | 5 +++++ gateway-api/src/gateway_api/sds/test_client.py | 11 +++++++++++ 2 files changed, 16 insertions(+) create mode 100644 gateway-api/src/gateway_api/sds/test_client.py diff --git a/gateway-api/src/gateway_api/conftest.py b/gateway-api/src/gateway_api/conftest.py index ed88f984..e49ab8b9 100644 --- a/gateway-api/src/gateway_api/conftest.py +++ b/gateway-api/src/gateway_api/conftest.py @@ -53,3 +53,8 @@ def valid_headers() -> dict[str, str]: "ODS-from": "test-ods", "Content-type": "application/fhir+json", } + + +@pytest.fixture +def auth_token() -> str: + return "AUTH_TOKEN123" diff --git a/gateway-api/src/gateway_api/sds/test_client.py b/gateway-api/src/gateway_api/sds/test_client.py new file mode 100644 index 00000000..26105420 --- /dev/null +++ b/gateway-api/src/gateway_api/sds/test_client.py @@ -0,0 +1,11 @@ +from gateway_api.sds.client import SdsClient + + +def test_sds_client(auth_token: str) -> None: + """Test that the SDS client returns the expected ASID and endpoint.""" + sds_client = SdsClient( + auth_token=auth_token, base_url="https://example.invalid/sds" + ) + result = sds_client.get_org_details("test_ods_code") + assert result is not None + assert result.asid == "asid_test_ods_code" From 30fdbaa786565f42add9934df85cac0f8800633b Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Tue, 10 Feb 2026 15:18:22 +0000 Subject: [PATCH 27/27] [GPCAPIM-275]: Behaviour being testted already covered by integration test, TestGetStructuredRecord.test_happy_path* --- .../src/gateway_api/test_controller.py | 59 ------------------- 1 file changed, 59 deletions(-) diff --git a/gateway-api/src/gateway_api/test_controller.py b/gateway-api/src/gateway_api/test_controller.py index 6be83160..2367ba13 100644 --- a/gateway-api/src/gateway_api/test_controller.py +++ b/gateway-api/src/gateway_api/test_controller.py @@ -280,65 +280,6 @@ def get_structured_record_request( # ----------------------------- -@pytest.mark.parametrize( - "get_structured_record_request", - [({}, {})], - indirect=["get_structured_record_request"], -) -def test_call_gp_provider_returns_200_on_success( - patched_deps: Any, # NOQA ARG001 (Fixture patching dependencies) - monkeypatch: pytest.MonkeyPatch, - controller: Controller, - get_structured_record_request: GetStructuredRecordRequest, -) -> None: - """ - On successful end-to-end call, the controller should return 200 with - expected body/headers. - """ - pds = pds_factory(ods_code="PROVIDER") - sds_org1 = SdsSetup( - ods_code="PROVIDER", - search_results=SdsSearchResults( - asid="asid_PROV", endpoint="https://provider.example/ep" - ), - ) - sds_org2 = SdsSetup( - ods_code="CONSUMER", - search_results=SdsSearchResults(asid="asid_CONS", endpoint=None), - ) - sds = sds_factory(org1=sds_org1, org2=sds_org2) - - monkeypatch.setattr(controller_module, "PdsClient", pds) - monkeypatch.setattr(controller_module, "SdsClient", sds) - - FakeGpProviderClient.response_status_code = 200 - FakeGpProviderClient.response_body = b'{"resourceType":"Bundle"}' - FakeGpProviderClient.response_headers = { - "Content-Type": "application/fhir+json", - "X-Downstream": "gp-provider", - } - - r = controller.run(get_structured_record_request) - - # Check that response from GP provider was passed through. - assert r.status_code == 200 - assert r.data == FakeGpProviderClient.response_body.decode("utf-8") - assert r.headers == FakeGpProviderClient.response_headers - - # Check that GP provider was initialised correctly - assert FakeGpProviderClient.last_init == { - "provider_endpoint": "https://provider.example/ep", - "provider_asid": "asid_PROV", - "consumer_asid": "asid_CONS", - } - - # Check that we passed the trace ID and body to the provider - assert FakeGpProviderClient.last_call == { - "trace_id": get_structured_record_request.trace_id, - "body": get_structured_record_request.request_body, - } - - @pytest.mark.parametrize( "get_structured_record_request", [({}, {})],