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/18] [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/18] [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/18] [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/18] [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/18] [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/18] [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/18] [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/18] [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/18] [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/18] [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/18] [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/18] [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/18] [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/18] [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/18] [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/18] [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/18] [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/18] [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: