diff --git a/gateway-api/src/gateway_api/pds_search.py b/gateway-api/src/gateway_api/pds_search.py index b21b6ecf..3ac1decd 100644 --- a/gateway-api/src/gateway_api/pds_search.py +++ b/gateway-api/src/gateway_api/pds_search.py @@ -138,19 +138,16 @@ def __init__( 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()), + "X-Request-ID": str(uuid.uuid4()), "NHSD-End-User-Organisation-ODS": self.end_user_org_ods, "Accept": "application/fhir+json", } @@ -173,7 +170,6 @@ def _build_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: @@ -182,10 +178,9 @@ def search_patient_by_nhs_number( Calls ``GET /Patient/{nhs_number}``, which returns a single FHIR Patient resource on success, then extracts a single :class:`PdsSearchResults`. + A fresh UUID is generated for the X-Request-ID header with each call. :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. @@ -195,7 +190,6 @@ def search_patient_by_nhs_number( ``raise_for_status()`` raises :class:`requests.HTTPError`. """ headers = self._build_headers( - request_id=request_id, correlation_id=correlation_id, ) diff --git a/gateway-api/src/gateway_api/test_controller.py b/gateway-api/src/gateway_api/test_controller.py index 3fc3ded4..f4d3c7bf 100644 --- a/gateway-api/src/gateway_api/test_controller.py +++ b/gateway-api/src/gateway_api/test_controller.py @@ -14,11 +14,9 @@ import gateway_api.controller as controller_module from gateway_api.app import app -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_search import SdsSearchResults if TYPE_CHECKING: from collections.abc import Generator @@ -61,6 +59,8 @@ def set_patient_details(self, value: Any) -> None: def search_patient_by_nhs_number( self, nhs_number: int, # noqa: ARG002 (unused in fake) + correlation_id: str | None = None, # noqa: ARG002 (unused in fake) + timeout: int | None = None, # noqa: ARG002 (unused in fake) ) -> Any | None: return self._patient_details @@ -77,18 +77,21 @@ class FakeSdsClient: def __init__( self, - auth_token: str | None = None, + api_key: str, base_url: str = "test_url", timeout: int = 10, + service_interaction_id: str | None = None, ) -> None: FakeSdsClient.last_init = { - "auth_token": auth_token, + "api_key": api_key, "base_url": base_url, "timeout": timeout, + "service_interaction_id": service_interaction_id, } - self.auth_token = auth_token + self.api_key = api_key self.base_url = base_url self.timeout = timeout + self.service_interaction_id = service_interaction_id self._org_details_by_ods: dict[str, SdsSearchResults | None] = {} def set_org_details( diff --git a/gateway-api/src/gateway_api/test_pds_search.py b/gateway-api/src/gateway_api/test_pds_search.py index a433c9a1..003125d6 100644 --- a/gateway-api/src/gateway_api/test_pds_search.py +++ b/gateway-api/src/gateway_api/test_pds_search.py @@ -7,7 +7,6 @@ from dataclasses import dataclass from datetime import date from typing import TYPE_CHECKING, Any, cast -from uuid import uuid4 import pytest import requests @@ -270,7 +269,8 @@ def test_search_patient_by_nhs_number_sends_expected_headers( * Authorization header * NHSD-End-User-Organisation-ODS header * Accept header - * caller-provided X-Request-ID and X-Correlation-ID headers + * X-Request-ID header (generated as UUID) + * caller-provided X-Correlation-ID header :param stub: Stub backend fixture. :param mock_requests_get: Patched ``requests.get`` fixture capturing outbound @@ -291,12 +291,10 @@ def test_search_patient_by_nhs_number_sends_expected_headers( base_url="https://example.test/personal-demographics/FHIR/R4", ) - req_id = str(uuid4()) corr_id = "corr-123" result = client.search_patient_by_nhs_number( "9000000009", - request_id=req_id, correlation_id=corr_id, ) assert result is not None @@ -305,7 +303,9 @@ def test_search_patient_by_nhs_number_sends_expected_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 "X-Request-ID" in headers + assert isinstance(headers["X-Request-ID"], str) + assert len(headers["X-Request-ID"]) >= 32 # UUID length check assert headers["X-Correlation-ID"] == corr_id @@ -314,10 +314,10 @@ def test_search_patient_by_nhs_number_generates_request_id( mock_requests_get: dict[str, Any], ) -> None: """ - Verify that the client generates an X-Request-ID when not provided. + Verify that the client generates a valid UUID for X-Request-ID. The stub is in strict mode, so a missing or invalid X-Request-ID would cause a 400. - This test confirms a request ID is present and looks UUID-like. + This test confirms a request ID is present and is a valid UUID. :param stub: Stub backend fixture. :param mock_requests_get: Patched ``requests.get`` fixture capturing outbound @@ -343,8 +343,66 @@ def test_search_patient_by_nhs_number_generates_request_id( headers = mock_requests_get["headers"] assert "X-Request-ID" in headers - assert isinstance(headers["X-Request-ID"], str) - assert len(headers["X-Request-ID"]) >= 32 + request_id = headers["X-Request-ID"] + assert isinstance(request_id, str) + # Verify it's a valid UUID by parsing it + import uuid + + try: + uuid.UUID(request_id) + except ValueError: + pytest.fail(f"X-Request-ID '{request_id}' is not a valid UUID") + + +def test_search_patient_by_nhs_number_generates_fresh_request_id_each_call( + stub: PdsFhirApiStub, + mock_requests_get: dict[str, Any], +) -> None: + """ + Verify that a fresh UUID is generated for X-Request-ID on each call. + + This test makes two calls to search_patient_by_nhs_number and confirms that + different UUIDs are generated for each request. + + :param stub: Stub backend fixture. + :param mock_requests_get: Patched ``requests.get`` fixture capturing outbound + headers. + :return: ``None``. + """ + _insert_basic_patient( + stub=stub, + nhs_number="9000000009", + family="Smith", + given=["Jane"], + general_practitioner=[], + ) + + client = PdsClient( + auth_token="test-token", # noqa: S106 + end_user_org_ods="A12345", + base_url="https://example.test/personal-demographics/FHIR/R4", + ) + + # First call + result1 = client.search_patient_by_nhs_number("9000000009") + assert result1 is not None + request_id_1 = mock_requests_get["headers"]["X-Request-ID"] + + # Second call + result2 = client.search_patient_by_nhs_number("9000000009") + assert result2 is not None + request_id_2 = mock_requests_get["headers"]["X-Request-ID"] + + # Verify both are valid UUIDs + import uuid + + uuid.UUID(request_id_1) + uuid.UUID(request_id_2) + + # Verify they are different + assert request_id_1 != request_id_2, ( + "X-Request-ID should be a fresh UUID for each call" + ) def test_search_patient_by_nhs_number_not_found_raises_error(