-
Notifications
You must be signed in to change notification settings - Fork 1
Feature/gpcapim 251 #46
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
19453b9
4e048c7
66c97ed
ed23fdf
8527927
851153e
f0d3d3f
c954a25
7db6a1a
6140cdd
3075176
6c65af4
89fa638
70c06cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| """ | ||
| Module: gateway_api.provider_request | ||
|
|
||
| This module contains the GpProviderClient class, which provides a | ||
| simple client for interacting with the GPProvider FHIR GP System. | ||
|
|
||
| The GpProviderClient class includes methods to fetch structured patient | ||
| records from a GPProvider FHIR API endpoint. | ||
|
|
||
| Usage: | ||
| Instantiate a GpProviderClient with: | ||
| - provider_endpoint: The FHIR API endpoint for the provider. | ||
| - provider_asid: The ASID for the provider. | ||
| - consumer_asid: The ASID for the consumer. | ||
|
|
||
| Use the `access_structured_record` method to fetch a structured patient record: | ||
| Parameters: | ||
| - trace_id (str): A unique identifier for the request. | ||
| - body (str): The request body in FHIR format. | ||
|
|
||
| Returns: | ||
| The response from the provider FHIR API. | ||
| """ | ||
|
|
||
| # imports | ||
|
|
||
| import requests | ||
| from requests import Response | ||
|
|
||
| # definitions | ||
| ars_interactionId = "urn:nhs:names:services:gpconnect:structured:fhir:operation:gpc.getstructuredrecord-1" # noqa: E501 this is standard InteractionID for accessRecordStructured | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can break this over two lines, it doesn't need to bust the line length limit. |
||
| ars_fhir_base = "/FHIR/STU3" | ||
| ars_fhir_operation = "$gpc.getstructuredrecord" | ||
| timeout = None # None used for quicker dev, adjust as needed | ||
|
|
||
|
|
||
| class ExternalServiceError(Exception): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we could move this into a common library. I've started one in my current ticket anyway. |
||
| """ | ||
| Exception raised when the downstream GPProvider FHIR API request fails. | ||
|
|
||
| This exception wraps :class:`requests.HTTPError` thrown by | ||
| ``response.raise_for_status()`` and re-raises it as ``ExternalServiceError`` | ||
| to decouple callers from the underlying ``requests`` library exception types. | ||
|
Comment on lines
+41
to
+43
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand this comment. The exception as written isn't related to |
||
| """ | ||
|
|
||
|
|
||
| class GpProviderClient: | ||
| """ | ||
| A client for interacting with the GPProvider FHIR GP System. | ||
|
|
||
| This class provides methods to interact with the GPProvider FHIR API, | ||
| including fetching structured patient records. | ||
|
|
||
| Attributes: | ||
| provider_endpoint (str): The FHIR API endpoint for the provider. | ||
| provider_asid (str): The ASID for the provider. | ||
| consumer_asid (str): The ASID for the consumer. | ||
|
|
||
| Methods: | ||
| access_structured_record(trace_id: str, body: str) -> Response: | ||
| Fetch a structured patient record from the GPProvider FHIR API. | ||
| """ | ||
|
|
||
| def __init__( | ||
| self, | ||
| provider_endpoint: str, | ||
| provider_asid: str, | ||
| consumer_asid: str, | ||
| ) -> None: | ||
| """ | ||
| Create a GPProviderClient instance. | ||
|
|
||
| Args: | ||
| provider_endpoint (str): The FHIR API endpoint for the provider. | ||
| provider_asid (str): The ASID for the provider. | ||
| consumer_asid (str): The ASID for the consumer. | ||
|
Comment on lines
+73
to
+76
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest we write docstrings with Sphinx-style argument listing. Then we can auto-generate our documentation (though that said I don't know if the likes of ChatGPT are going to obsolete the likes of Sphinx). Still, it's standard and it gives us the option, and it's easy enough to just ask the AI to do. Don't know if @davidhamill1-nhs and @nhsd-jack-wainwright agree? |
||
|
|
||
| methods: | ||
| access_structured_record: fetch structured patient record | ||
| from GPProvider FHIR API. | ||
| """ | ||
| self.provider_endpoint = provider_endpoint | ||
| self.provider_asid = provider_asid | ||
| self.consumer_asid = consumer_asid | ||
|
|
||
| def _build_headers(self, trace_id: str) -> dict[str, str]: | ||
| """ | ||
| Build the headers required for the GPProvider FHIR API request. | ||
|
|
||
| Args: | ||
| trace_id (str): A unique identifier for the request. | ||
|
|
||
| Returns: | ||
| dict[str, str]: A dictionary containing the headers for the request, | ||
| including content type, interaction ID, and ASIDs for the provider | ||
| and consumer. | ||
| """ | ||
| return { | ||
| "Content-Type": "application/fhir+json", | ||
| "Accept": "application/fhir+json", | ||
| "Ssp-InteractionID": ars_interactionId, | ||
| "Ssp-To": self.provider_asid, | ||
| "Ssp-From": self.consumer_asid, | ||
| "Ssp-TraceID": trace_id, | ||
| } | ||
|
|
||
| def access_structured_record( | ||
| self, | ||
| trace_id: str, | ||
| body: str, | ||
| ) -> Response: | ||
| """ | ||
| Fetch a structured patient record from the GPProvider FHIR API. | ||
|
|
||
| Args: | ||
| trace_id (str): A unique identifier for the request, passed in the headers. | ||
| body (str): The request body in FHIR format. | ||
|
|
||
| Returns: | ||
| Response: The response from the GPProvider FHIR API. | ||
|
|
||
| Raises: | ||
| ExternalServiceError: If the API request fails with an HTTP error. | ||
| """ | ||
|
|
||
| headers = self._build_headers(trace_id) | ||
|
|
||
| response = requests.post( | ||
| self.provider_endpoint + ars_fhir_base + "/patient/" + ars_fhir_operation, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was convinced that Python had a built-in function for constructing URLs from arbitrary components, but it turns out that it doesn't. And you can't use pathlib either because if any component has a leading slash it drops all the previous components. I wonder if it's worth writing one ourselves. |
||
| headers=headers, | ||
| data=body, | ||
| timeout=timeout, | ||
| ) | ||
|
|
||
| try: | ||
| response.raise_for_status() | ||
| except requests.HTTPError as err: | ||
| raise ExternalServiceError( | ||
| f"GPProvider FHIR API request failed:{err.response.reason}" | ||
| ) from err | ||
|
|
||
| return response | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,240 @@ | ||
| """ | ||
| Unit tests for :mod:`gateway_api.provider_request`. | ||
|
|
||
| This module contains unit tests for the `GpProviderClient` class, which is responsible | ||
| for interacting with the GPProvider FHIR API. | ||
|
|
||
| Fixtures: | ||
| - `stub`: Provides an instance of the `GpProviderStub` for simulating the GPProvider | ||
| - `mock_request_post`: Patches the `requests.post` method to intercept API calls and | ||
| route them to the stub provider. Captures request details for verification. | ||
|
Comment on lines
+7
to
+10
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure we really need to list the fixtures in the module docstring. Easy enough to find them by looking through the file, and the header will go out of date because nobody will update it. |
||
|
|
||
| """ | ||
|
|
||
| from typing import Any | ||
|
|
||
| import pytest | ||
| import requests | ||
| from requests import Response | ||
| from requests.structures import CaseInsensitiveDict | ||
| from stubs.stub_provider import GpProviderStub | ||
|
|
||
| from gateway_api.provider_request import ExternalServiceError, GpProviderClient | ||
|
|
||
| ars_interactionId = "urn:nhs:names:services:gpconnect:structured:fhir:operation:gpc.getstructuredrecord-1" # noqa: E501 this is standard InteractionID for accessRecordStructured | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above don't need to #noqa this, just break it across two lines. |
||
|
|
||
|
|
||
| @pytest.fixture | ||
| def stub() -> GpProviderStub: | ||
| return GpProviderStub() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def mock_request_post( | ||
| monkeypatch: pytest.MonkeyPatch, stub: GpProviderStub | ||
| ) -> dict[str, Any]: | ||
| """ | ||
| Fixture to patch the `requests.post` method for testing. | ||
|
|
||
| This fixture intercepts calls to `requests.post` and routes them to the | ||
| stub provider. It also captures the most recent request details, such as | ||
| headers, body, and URL, for verification in tests. | ||
|
|
||
| Returns: | ||
| dict[str, Any]: A dictionary containing the captured request details. | ||
| """ | ||
| capture: dict[str, Any] = {} | ||
|
|
||
| def _fake_post( | ||
| url: str, | ||
| headers: CaseInsensitiveDict[str], | ||
| data: str, | ||
| timeout: int, | ||
| ) -> Response: | ||
| """A fake requests.post implementation.""" | ||
|
|
||
| capture["headers"] = dict(headers) | ||
| capture["data"] = data | ||
| capture["url"] = url | ||
|
|
||
| return stub.access_record_structured() | ||
|
|
||
| monkeypatch.setattr(requests, "post", _fake_post) | ||
| return capture | ||
|
|
||
|
|
||
| def test_valid_gpprovider_access_structured_record_makes_request_correct_url_post_200( | ||
| mock_request_post: dict[str, Any], | ||
| stub: GpProviderStub, | ||
| ) -> None: | ||
| """ | ||
| Test that the `access_structured_record` method constructs the correct URL | ||
| for the GPProvider FHIR API request and receives a 200 OK response. | ||
|
|
||
| This test verifies that the URL includes the correct FHIR base path and | ||
| operation for accessing a structured patient record. | ||
| """ | ||
| provider_asid = "200000001154" | ||
| consumer_asid = "200000001152" | ||
| provider_endpoint = "https://invalid.com" | ||
| trace_id = "some_uuid_value" | ||
|
|
||
| client = GpProviderClient( | ||
| provider_endpoint=provider_endpoint, | ||
| provider_asid=provider_asid, | ||
| consumer_asid=consumer_asid, | ||
| ) | ||
|
|
||
| result = client.access_structured_record(trace_id, "body") | ||
|
|
||
| captured_url = mock_request_post.get("url", provider_endpoint) | ||
|
|
||
| assert ( | ||
| captured_url | ||
| == provider_endpoint + "/FHIR/STU3/patient/$gpc.getstructuredrecord" | ||
| ) | ||
| assert result.status_code == 200 | ||
|
|
||
|
|
||
| def test_valid_gpprovider_access_structured_record_with_correct_headers_post_200( | ||
| mock_request_post: dict[str, Any], | ||
| stub: GpProviderStub, | ||
| ) -> None: | ||
| """ | ||
| Test that the `access_structured_record` method includes the correct headers | ||
| in the GPProvider FHIR API request and receives a 200 OK response. | ||
|
|
||
| This test verifies that the headers include: | ||
| - Content-Type and Accept headers for FHIR+JSON. | ||
| - Ssp-TraceID, Ssp-From, Ssp-To, and Ssp-InteractionID for GPConnect. | ||
| """ | ||
| provider_asid = "200000001154" | ||
| consumer_asid = "200000001152" | ||
| provider_endpoint = "https://invalid.com" | ||
| trace_id = "some_uuid_value" | ||
|
|
||
| client = GpProviderClient( | ||
| provider_endpoint=provider_endpoint, | ||
| provider_asid=provider_asid, | ||
| consumer_asid=consumer_asid, | ||
| ) | ||
| expected_headers = { | ||
| "Content-Type": "application/fhir+json", | ||
| "Accept": "application/fhir+json", | ||
| "Ssp-TraceID": str(trace_id), | ||
| "Ssp-From": consumer_asid, | ||
| "Ssp-To": provider_asid, | ||
| "Ssp-InteractionID": ars_interactionId, | ||
| } | ||
|
|
||
| result = client.access_structured_record(trace_id, "body") | ||
|
|
||
| captured_headers = mock_request_post["headers"] | ||
|
|
||
| for key, value in expected_headers.items(): | ||
| assert captured_headers.get(key) == value | ||
| assert result.status_code == 200 | ||
|
|
||
|
|
||
| def test_valid_gpprovider_access_structured_record_with_correct_body_200( | ||
| mock_request_post: dict[str, Any], | ||
| stub: GpProviderStub, | ||
| ) -> None: | ||
| """ | ||
| Test that the `access_structured_record` method includes the correct body | ||
| in the GPProvider FHIR API request and receives a 200 OK response. | ||
|
|
||
| This test verifies that the request body matches the expected FHIR parameters | ||
| resource sent to the GPProvider API. | ||
| """ | ||
| provider_asid = "200000001154" | ||
| consumer_asid = "200000001152" | ||
| provider_endpoint = "https://invalid.com" | ||
| trace_id = "some_uuid_value" | ||
|
|
||
| request_body = "some_FHIR_request_params" | ||
|
|
||
| client = GpProviderClient( | ||
| provider_endpoint=provider_endpoint, | ||
| provider_asid=provider_asid, | ||
| consumer_asid=consumer_asid, | ||
| ) | ||
|
|
||
| result = client.access_structured_record(trace_id, request_body) | ||
|
|
||
| captured_body = mock_request_post["data"] | ||
|
|
||
| assert result.status_code == 200 | ||
| assert captured_body == request_body | ||
|
|
||
|
|
||
| def test_valid_gpprovider_access_structured_record_returns_stub_response_200( | ||
| mock_request_post: dict[str, Any], | ||
| stub: GpProviderStub, | ||
| ) -> None: | ||
| """ | ||
| Test that the `access_structured_record` method returns the same response | ||
| as provided by the stub provider. | ||
|
|
||
| This test verifies that the response from the GPProvider FHIR API matches | ||
| the expected response, including the status code and content. | ||
| """ | ||
| provider_asid = "200000001154" | ||
| consumer_asid = "200000001152" | ||
| provider_endpoint = "https://invalid.com" | ||
| trace_id = "some_uuid_value" | ||
|
|
||
| client = GpProviderClient( | ||
| provider_endpoint=provider_endpoint, | ||
| provider_asid=provider_asid, | ||
| consumer_asid=consumer_asid, | ||
| ) | ||
|
|
||
| expected_response = stub.access_record_structured() | ||
|
|
||
| result = client.access_structured_record(trace_id, "body") | ||
|
|
||
| assert result.status_code == expected_response.status_code | ||
| assert result.content == expected_response.content | ||
|
|
||
|
|
||
| def test_access_structured_record_raises_external_service_error( | ||
| mock_request_post: dict[str, Any], | ||
| stub: GpProviderStub, | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| ) -> None: | ||
| """ | ||
| Test that the `access_structured_record` method raises an `ExternalServiceError` | ||
| when the GPProvider FHIR API request fails with an HTTP error. | ||
| """ | ||
| provider_asid = "200000001154" | ||
| consumer_asid = "200000001152" | ||
| provider_endpoint = "https://invalid.com" | ||
| trace_id = "some_uuid_value" | ||
|
|
||
| client = GpProviderClient( | ||
| provider_endpoint=provider_endpoint, | ||
| provider_asid=provider_asid, | ||
| consumer_asid=consumer_asid, | ||
| ) | ||
|
|
||
| # Simulate an error response from the stub | ||
| def _fake_post_error( | ||
| url: str, | ||
| headers: CaseInsensitiveDict[str], | ||
| data: str, | ||
| timeout: int, | ||
| ) -> Response: | ||
| response = Response() | ||
| response.status_code = 500 | ||
| response._content = b"Internal Server Error" # noqa: SLF001 TODO: push this back into the stub? | ||
| response.reason = "Internal Server Error" | ||
| return response | ||
|
|
||
| monkeypatch.setattr(requests, "post", _fake_post_error) | ||
|
|
||
| with pytest.raises( | ||
| ExternalServiceError, | ||
| match="GPProvider FHIR API request failed:Internal Server Error", | ||
| ): | ||
| client.access_structured_record(trace_id, "body") | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need to do this? You're basically importing from the same file twice (which AI seems to like doing and I'm not sure why). IMO better to just do
from requests import Response, <whatever else you're using>.