Skip to content

Commit c2e4052

Browse files
committed
[GPCAPIM-395]: Refactor PDS client to use session-based GET requests
- Introduced a new session_get_impl function to handle GET requests with session support - Updated PdsClient to use the new session_get_impl for making requests - Modified tests to replace calls to the deprecated _make_get_request with _make_session_get_request - Added SessionGetStub to the PdsFhirApiStub for handling session-based GET requests
1 parent 06a516e commit c2e4052

5 files changed

Lines changed: 85 additions & 30 deletions

File tree

gateway-api/src/gateway_api/pds/client.py

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,34 @@
3737
_pds_url = os.getenv("PDS_URL", "stub")
3838
STUB_PDS = _pds_url.strip().lower() == "stub"
3939

40-
if not STUB_PDS:
41-
from requests import get
42-
else:
40+
if STUB_PDS:
4341
from stubs.pds.stub import PdsFhirApiStub
4442

45-
pds = PdsFhirApiStub()
46-
get = pds.get # type: ignore
43+
pds_stub = PdsFhirApiStub()
44+
45+
46+
def session_get_impl(
47+
session: requests.Session,
48+
url: str,
49+
headers: dict[str, str],
50+
timeout: int,
51+
) -> requests.Response:
52+
if STUB_PDS:
53+
return pds_stub.session_get(
54+
session=session,
55+
url=url,
56+
headers=headers,
57+
params={},
58+
timeout=timeout,
59+
)
60+
61+
return session.get(
62+
url=url,
63+
headers=headers,
64+
params={},
65+
timeout=timeout,
66+
)
67+
4768

4869
_logger = logging.getLogger(__name__)
4970

@@ -131,18 +152,13 @@ def search_patient_by_nhs_number(
131152
}
132153
_logger.info(log_details)
133154

134-
# This normally calls requests.get, but if PDS_URL is set it uses the stub.
135-
# response = get(
136-
# url,
137-
# headers=headers,
138-
# params={},
139-
# timeout=timeout or self.timeout,
140-
# )
155+
# This uses an auth-wrapped session. The underlying GET implementation
156+
# is selected at import time based on PDS_URL.
141157
set_correlation_id(
142158
full_id=correlation_id or "no-correlation-id",
143159
short_id=correlation_id or "no-correlation-id",
144160
)
145-
response = _make_get_request(
161+
response = _make_session_get_request(
146162
url,
147163
headers=headers,
148164
timeout=timeout or self.timeout,
@@ -171,16 +187,11 @@ def search_patient_by_nhs_number(
171187

172188
# TODO [GPCAPIM-359]: consider moving this to a nested method
173189
@environment.apim_authenticator().auth
174-
def _make_get_request(
190+
def _make_session_get_request(
175191
session: requests.Session,
176192
url: str,
177193
headers: dict[str, str],
178194
timeout: int,
179195
) -> Any:
180-
response = session.get(
181-
url=url,
182-
headers=headers,
183-
params={},
184-
timeout=timeout,
185-
)
196+
response = session_get_impl(session, url, headers, timeout)
186197
return response

gateway-api/src/gateway_api/pds/test_client.py

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ def test_search_patient_by_nhs_number_happy_path(
2222
status_code=200, headers={}, _json=happy_path_pds_response_body
2323
)
2424
mocker.patch(
25-
"gateway_api.pds.client._make_get_request", return_value=happy_path_response
25+
"gateway_api.pds.client._make_session_get_request",
26+
return_value=happy_path_response,
2627
)
2728

2829
client = PdsClient(base_url="https://test.com")
@@ -43,7 +44,8 @@ def test_search_patient_by_nhs_number_has_no_gp_returns_gp_ods_code_none(
4344
status_code=200, headers={}, _json=gp_less_response_body
4445
)
4546
mocker.patch(
46-
"gateway_api.pds.client._make_get_request", return_value=gp_less_response
47+
"gateway_api.pds.client._make_session_get_request",
48+
return_value=gp_less_response,
4749
)
4850

4951
client = PdsClient(base_url="https://test.com")
@@ -62,7 +64,8 @@ def test_search_patient_by_nhs_number_sends_expected_headers(
6264
status_code=200, headers={}, _json=happy_path_pds_response_body
6365
)
6466
mocked_get = mocker.patch(
65-
"gateway_api.pds.client._make_get_request", return_value=happy_path_response
67+
"gateway_api.pds.client._make_session_get_request",
68+
return_value=happy_path_response,
6669
)
6770

6871
request_id = str(uuid4())
@@ -93,7 +96,8 @@ def test_search_patient_by_nhs_number_generates_request_id(
9396
status_code=200, headers={}, _json=happy_path_pds_response_body
9497
)
9598
mocked_get = mocker.patch(
96-
"gateway_api.pds.client._make_get_request", return_value=happy_path_response
99+
"gateway_api.pds.client._make_session_get_request",
100+
return_value=happy_path_response,
97101
)
98102

99103
client = PdsClient(base_url="https://test.com")
@@ -116,7 +120,8 @@ def test_search_patient_by_nhs_number_not_found_raises_error(
116120
reason="Not Found",
117121
)
118122
mocker.patch(
119-
"gateway_api.pds.client._make_get_request", return_value=not_found_response
123+
"gateway_api.pds.client._make_session_get_request",
124+
return_value=not_found_response,
120125
)
121126
pds = PdsClient(base_url="https://test.com")
122127

@@ -138,7 +143,9 @@ def test_search_patient_by_nhs_number_missing_nhs_number_raises_error(
138143
headers={},
139144
_json=response_body_missing_nhs_number,
140145
)
141-
mocker.patch("gateway_api.pds.client._make_get_request", return_value=response)
146+
mocker.patch(
147+
"gateway_api.pds.client._make_session_get_request", return_value=response
148+
)
142149

143150
client = PdsClient(base_url="https://test.com")
144151

@@ -157,7 +164,8 @@ def test_search_patient_respects_url(
157164
status_code=200, headers={}, _json=happy_path_pds_response_body
158165
)
159166
mocked_get = mocker.patch(
160-
"gateway_api.pds.client._make_get_request", return_value=happy_path_response
167+
"gateway_api.pds.client._make_session_get_request",
168+
return_value=happy_path_response,
161169
)
162170

163171
client = PdsClient(base_url="https://a.different.url/base")

gateway-api/src/gateway_api/test_controller.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ def test_controller_respects_pds_url(
366366
Test that the controller uses the PDS URL provided in the constructor.
367367
"""
368368
mocked_get_pds = mocker.patch(
369-
"gateway_api.pds.client._make_get_request",
369+
"gateway_api.pds.client._make_session_get_request",
370370
return_value=FakeResponse(
371371
status_code=200, headers={}, _json=happy_path_pds_response_body
372372
),

gateway-api/stubs/stubs/base_stub.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,3 +145,18 @@ def post_session(self) -> Session:
145145
"""
146146
Last Session stub.session_post was called with. None if not called yet.
147147
"""
148+
149+
150+
class SessionGetStub(Protocol):
151+
@abstractmethod
152+
def session_get(
153+
self,
154+
session: Session,
155+
url: str,
156+
headers: dict[str, str],
157+
params: dict[str, Any],
158+
timeout: int,
159+
) -> Response:
160+
"""
161+
Handle HTTP GET requests for the stub, using a provided Session.
162+
"""

gateway-api/stubs/stubs/pds/stub.py

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,14 @@
99
from datetime import UTC, datetime
1010
from typing import Any
1111

12+
import requests
1213
from requests import Response
1314

14-
from stubs.base_stub import StubBase
15+
from stubs.base_stub import SessionGetStub, StubBase
1516
from stubs.data.patients import Patients
1617

1718

18-
class PdsFhirApiStub(StubBase):
19+
class PdsFhirApiStub(StubBase, SessionGetStub):
1920
"""
2021
Minimal in-memory stub for the PDS FHIR API, implementing only ``GET /Patient/{id}``
2122
@@ -174,6 +175,7 @@ def get_patient(
174175
status_code=200, json_data=patient, additional_headers=headers_out
175176
)
176177

178+
# TODO [GPCAPIM-395]: remove depricated, and updated tests
177179
def get(
178180
self,
179181
url: str,
@@ -210,6 +212,25 @@ def post(
210212
"""
211213
raise NotImplementedError("POST requests are not supported by PdsFhirApiStub")
212214

215+
def session_get(
216+
self,
217+
session: requests.Session, # noqa: ARG002 - maintain signature for stub
218+
url: str,
219+
headers: dict[str, str],
220+
params: dict[str, Any], # noqa: ARG002 - maintain signature for stub
221+
timeout: int, # noqa: ARG002 - maintain signature for stub
222+
) -> Response:
223+
nhs_number = url.split("/")[-1]
224+
225+
request_id = headers.get("X-Request-ID") if headers else None
226+
correlation_id = headers.get("X-Correlation-ID") if headers else None
227+
228+
return self.get_patient(
229+
nhs_number=nhs_number,
230+
request_id=request_id,
231+
correlation_id=correlation_id,
232+
)
233+
213234
# ---------------------------
214235
# Internal helpers
215236
# ---------------------------

0 commit comments

Comments
 (0)