Skip to content

[GPCAPIM-265] Generate X-Request-Id header #85

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Vox-Ben wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[GPCAPIM-265] Generate X-Request-Id header #85

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions gateway-api/src/gateway_api/pds_search.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,19 +138,18 @@ 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.
A new UUID is generated for ``X-Request-ID`` with each call.

: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",
}
Expand All @@ -173,7 +172,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:
Expand All @@ -183,9 +181,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 new UUID is generated for the ``X-Request-ID`` header with each call.
Copy link
Contributor

@davidhamill1-nhs davidhamill1-nhs Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've stated this twice. I'm not sure it's need once.


: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.
Expand All @@ -195,7 +193,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,
)

Expand Down
19 changes: 12 additions & 7 deletions gateway-api/src/gateway_api/test_pds_search.py
View file
Open in desktop
Copy link
Contributor

@davidhamill1-nhs davidhamill1-nhs Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would parameterise the header test so that only a single assertion is made per test.

You could also patch the uuid module to return a yest value and assert for that.

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
* auto-generated X-Request-ID (UUID format)
* caller-provided X-Correlation-ID header

:param stub: Stub backend fixture.
:param mock_requests_get: Patched ``requests.get`` fixture capturing outbound
Expand All @@ -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
Expand All @@ -305,7 +303,14 @@ 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
# X-Request-ID should be auto-generated as a UUID
assert "X-Request-ID" in headers
assert isinstance(headers["X-Request-ID"], str)
assert len(headers["X-Request-ID"]) >= 32
# Verify it's a valid UUID by trying to parse it
Comment on lines +306 to +310
Copy link
Contributor

@davidhamill1-nhs davidhamill1-nhs Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, these are unnecessary. The final assertion, UUID(headers["X-Request-ID"]) will check for these.

from uuid import UUID
Copy link
Contributor

@davidhamill1-nhs davidhamill1-nhs Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move to import block at the top of the file.


UUID(headers["X-Request-ID"]) # Should not raise
Copy link
Contributor

@davidhamill1-nhs davidhamill1-nhs Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To produce nicer error messages if/when the test fails:

Suggested change
UUID(headers["X-Request-ID"]) # Should not raise
try:
UUID(headers["X-Request-ID"])
except:
pytest.fail("Should not raise an error if this is genuinely a uuid")

assert headers["X-Correlation-ID"] == corr_id


Expand All @@ -314,10 +319,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 new X-Request-ID for each request.

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
Expand Down