Skip to content
1 change: 1 addition & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,5 @@
"python.analysis.extraPaths": [
"./gateway-api/stubs"
],
"python-envs.defaultEnvManager": "ms-python.python:pyenv",
}
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,11 @@ Environment variables control whether stubs are used in place of the real PDS, S
| Variable | Description |
| --- | --- |
| `PDS_URL` | The URL for the PDS FHIR API; set as `stub` to use development stub. |
| `PDS_API_TOKEN`| Leave unset in development environment. |
| `PDS_API_SECRET`| Leave unset in development environment. |
| `PDS_API_KID`| Leave unset in development environment. |
| `PDS_API_TOKEN` | Leave unset in development environment. |
| `PDS_API_SECRET` | Leave unset in development environment. |
| `PDS_API_KID` | Leave unset in development environment. |
| `SDS_URL` | The URL for the SDS FHIR API; set as `stub` to use development stub. |
| `SDS_API_TOKEN`| Leave unset in development environment. |
| `SDS_API_TOKEN` | Leave unset in development environment. |
| `PROVIDER_URL` | The URL for the GP Provider; set as `stub` to use development stub. |
| `CDG_DEBUG` | `true`, return additional debug information when the call to the GP provider returns an error. |

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
name: localInt
variables:
- name: base_url
value: http://localhost:5000
- name: nhs_number
value: "9658218865"
- name: from_ods
value: A20047
5 changes: 4 additions & 1 deletion gateway-api/src/gateway_api/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ def configure_app(app: Flask) -> None:
"FLASK_PORT": get_env_var("FLASK_PORT", int),
"PDS_URL": get_env_var("PDS_URL", str),
"SDS_URL": get_env_var("SDS_URL", str),
"SDS_API_TOKEN": get_env_var("SDS_API_TOKEN", str),
}
app.config.update(config)

Expand Down Expand Up @@ -108,7 +109,9 @@ def get_structured_record() -> Response:
try:
get_structured_record_request = GetStructuredRecordRequest(request)
controller = Controller(
pds_base_url=app.config["PDS_URL"], sds_base_url=app.config["SDS_URL"]
pds_base_url=app.config["PDS_URL"],
sds_base_url=app.config["SDS_URL"],
sds_api_key=app.config["SDS_API_TOKEN"],
)
provider_response = controller.run(request=get_structured_record_request)
response.add_provider_response(provider_response)
Expand Down
3 changes: 3 additions & 0 deletions gateway-api/src/gateway_api/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,15 @@ def __init__(
self,
pds_base_url: str,
sds_base_url: str,
sds_api_key: str,
timeout: int = 10,
) -> None:
"""
Create a controller instance.
"""
self.pds_base_url = pds_base_url
self.sds_base_url = sds_base_url
self.sds_api_key = sds_api_key
self.timeout = timeout
self.gp_provider_client = None

Expand Down Expand Up @@ -194,6 +196,7 @@ def _get_sds_details(
# SDS: Get provider details (ASID + endpoint) for provider ODS
sds = SdsClient(
base_url=self.sds_base_url,
api_key=self.sds_api_key,
timeout=self.timeout,
)

Expand Down
7 changes: 2 additions & 5 deletions gateway-api/src/gateway_api/sds/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
from gateway_api.sds.client import SdsClient
from gateway_api.sds.client import SdsClient, get
from gateway_api.sds.search_results import SdsSearchResults

__all__ = [
"SdsClient",
"SdsSearchResults",
]
__all__ = ["SdsClient", "SdsSearchResults", "get"]
44 changes: 19 additions & 25 deletions gateway-api/src/gateway_api/sds/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
from fhir import Resource
from fhir.constants import FHIRSystem
from fhir.r4 import Bundle, Device, Endpoint
from requests import HTTPError
from requests import HTTPError, Response
from requests import get as external_sds_get
from stubs import SdsFhirApiStub

from gateway_api.common.error import SdsRequestFailedError
from gateway_api.get_structured_record import (
Expand All @@ -25,17 +27,21 @@
)
from gateway_api.sds.search_results import SdsSearchResults

# TODO [GPCAPIM-359]: Once stub servers/containers made for PDS, SDS and provider
# we should remove the SDS_URL environment variable and just
# use the stub client
STUB_SDS = os.environ["SDS_URL"].lower() == "stub"
if not STUB_SDS:
from requests import get
else:
from stubs import SdsFhirApiStub

sds = SdsFhirApiStub()
get = sds.get # type: ignore
def get(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm interested in what other people think about this pattern, but I think it's nicer than the conditional imports. We're not planning on moving to separately deployment mocks any time soon, so I think a little wrapper function is useful. Not technically part of the scope of this ticket though, so I can revert and discuss separately if we want to.

url: str,
headers: dict[str, str],
params: dict[str, str],
timeout: int,
) -> Response:
STUB_SDS = os.environ["SDS_URL"].lower() == "stub"
if not STUB_SDS:
return external_sds_get(url, headers=headers, params=params, timeout=timeout)
else:
return SdsFhirApiStub().get(
url, headers=headers, params=params, timeout=timeout
)


_logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -83,12 +89,13 @@ class SdsClient:
def __init__(
self,
base_url: str,
api_key: str,
timeout: int = 10,
service_interaction_id: str | None = None,
) -> None:
self.base_url = base_url.rstrip("/")
self.timeout = timeout
self.api_key = self._get_api_key()
self.api_key = api_key
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This fits better with the env var pattern established in the previous ticket. If we try to read the config from the "app" variable in app.py we get circular imports


if service_interaction_id is not None:
self.service_interaction_id = service_interaction_id
Expand Down Expand Up @@ -169,19 +176,6 @@ def get_org_details(

return SdsSearchResults(asid=asid, endpoint=endpoint_url)

@staticmethod
def _get_api_key() -> str:
"""
Retrieve the API key to use for SDS requests.

This is a placeholder at present because we don't have a real API key.
Ultimately it will probably obtain the key from AWS secrets
"""

# TODO [GPCAPIM-366]: Obtain key from AWS secrets
# DO NOT PUT A REAL KEY HERE, IT WILL BE VISIBLE ON GITHUB
return "test_api_key_DO_NOT_REPLACE_HERE"

def _query_sds(
self,
ods_code: str,
Expand Down
91 changes: 50 additions & 41 deletions gateway-api/src/gateway_api/sds/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,21 @@
Unit tests for :mod:`gateway_api.sds_search`.
"""

from unittest.mock import Mock, patch

import pytest
from fhir.constants import FHIRSystem
from fhir.r4.resources.bundle import Bundle
from pytest_mock import MockerFixture
from stubs.sds.stub import SdsFhirApiStub

from gateway_api.common.error import SdsRequestFailedError
from gateway_api.conftest import FakeResponse
from gateway_api.conftest import FakeResponse, ScopedEnvVars
from gateway_api.get_structured_record import (
ACCESS_RECORD_STRUCTURED_INTERACTION_ID,
SDS_SANDBOX_INTERACTION_ID,
)
from gateway_api.sds import (
SdsClient,
SdsSearchResults,
)
from gateway_api.sds import SdsClient, SdsSearchResults, get


@pytest.fixture
Expand All @@ -32,15 +31,14 @@ def stub(monkeypatch: pytest.MonkeyPatch) -> SdsFhirApiStub:
return stub


def test_sds_client_get_org_details_success(
stub: SdsFhirApiStub,
) -> None:
def test_sds_client_get_org_details_success(stub: SdsFhirApiStub) -> None:
"""
Test SdsClient can successfully look up organization details.

:param stub: SDS stub fixture.
:param mock_flask: Mock Flask app fixture.
"""
client = SdsClient(base_url="https://test.com")
client = SdsClient(base_url="https://test.com", api_key="example_api_key")

result = client.get_org_details(ods_code="PROVIDER")

Expand All @@ -56,9 +54,7 @@ def test_sds_client_get_org_details_success(
)


def test_sds_client_get_org_details_with_endpoint(
stub: SdsFhirApiStub,
) -> None:
def test_sds_client_get_org_details_with_endpoint(stub: SdsFhirApiStub) -> None:
"""
Test SdsClient retrieves endpoint when available.

Expand Down Expand Up @@ -111,56 +107,49 @@ def test_sds_client_get_org_details_with_endpoint(
},
)

client = SdsClient(base_url="https://test.com")
client = SdsClient(base_url="https://test.com", api_key="example_api_key")
result = client.get_org_details(ods_code="TESTORG")

assert result is not None
assert result.asid == "999999999999"
assert result.endpoint == "https://testorg.example.com/fhir"


def test_sds_client_sends_correct_headers(
stub: SdsFhirApiStub,
) -> None:
def test_sds_client_sends_correct_headers(stub: SdsFhirApiStub) -> None:
"""
Test that SdsClient sends X-Correlation-Id and apikey headers when provided.

:param stub: SDS stub fixture.
:param mock_requests_get: Capture fixture for request details.
"""
client = SdsClient(base_url="https://test.com")
client = SdsClient(base_url="https://test.com", api_key="example_api_key")

correlation_id = "test-correlation-123"
client.get_org_details(ods_code="PROVIDER", correlation_id=correlation_id)

# Check that the headers were
assert stub.get_headers["X-Correlation-Id"] == correlation_id

# In future when _get_api_key calls AWS secrets, this will break.
# That's a good thing, because we'll want to mock that call.
assert stub.get_headers["apikey"] == "test_api_key_DO_NOT_REPLACE_HERE"
assert stub.get_headers["apikey"] == "example_api_key"


def test_sds_client_timeout_parameter(
stub: SdsFhirApiStub,
) -> None:
def test_sds_client_timeout_parameter(stub: SdsFhirApiStub) -> None:
"""
Test that SdsClient passes timeout parameter to requests.

:param stub: SDS stub fixture.
:param mock_requests_get: Capture fixture for request details.
"""
client = SdsClient(base_url="https://test.com", timeout=30)
client = SdsClient(
base_url="https://test.com", api_key="example_api_key", timeout=30
)

client.get_org_details(ods_code="PROVIDER", timeout=60)

# Check that the custom timeout was passed
assert stub.get_timeout == 60


def test_sds_client_custom_service_interaction_id(
stub: SdsFhirApiStub,
) -> None:
def test_sds_client_custom_service_interaction_id(stub: SdsFhirApiStub) -> None:
"""
Test that SdsClient uses custom interaction ID when provided.

Expand Down Expand Up @@ -194,6 +183,7 @@ def test_sds_client_custom_service_interaction_id(
client = SdsClient(
base_url="https://test.com",
service_interaction_id=custom_interaction,
api_key="example_api_key",
)

result = client.get_org_details(ods_code="CUSTOMINT", get_endpoint=False)
Expand All @@ -209,16 +199,14 @@ def test_sds_client_custom_service_interaction_id(
assert result.asid == "777777777777"


def test_sds_client_builds_correct_device_query_params(
stub: SdsFhirApiStub,
) -> None:
def test_sds_client_builds_correct_device_query_params(stub: SdsFhirApiStub) -> None:
"""
Test that SdsClient builds Device query parameters correctly.

:param stub: SDS stub fixture.
:param mock_requests_get: Capture fixture for request details.
"""
client = SdsClient(base_url="https://test.com")
client = SdsClient(base_url="https://test.com", api_key="example_api_key")

client.get_org_details(ods_code="PROVIDER")

Expand Down Expand Up @@ -268,7 +256,8 @@ def test_sds_client_uses_sandbox_interaction_id_for_sandbox_url(
)

client = SdsClient(
base_url="https://sandbox.api.service.nhs.uk/spine-directory/FHIR/R4"
base_url="https://sandbox.api.service.nhs.uk/spine-directory/FHIR/R4",
api_key="example_api_key",
)
result = client.get_org_details(ods_code="SANDBOX_ORG", get_endpoint=False)

Expand Down Expand Up @@ -313,7 +302,7 @@ def get_without_apikey(

monkeypatch.setattr("gateway_api.sds.client.get", get_without_apikey)

client = SdsClient(base_url="https://test.com")
client = SdsClient(base_url="https://test.com", api_key="example_api_key")

with pytest.raises(SdsRequestFailedError, match="SDS FHIR API request failed"):
client.get_org_details(ods_code="PROVIDER")
Expand Down Expand Up @@ -353,7 +342,7 @@ def test_sds_client_endpoint_entry_without_address_returns_none(
},
)

client = SdsClient(base_url="https://test.com")
client = SdsClient(base_url="https://test.com", api_key="example_api_key")
result = client.get_org_details(ods_code="NOADDR")

assert result.asid == "111111111111"
Expand All @@ -369,7 +358,7 @@ def test_sds_client_empty_device_bundle_returns_none_asid() -> None:

:param stub: SDS stub fixture.
"""
client = SdsClient(base_url="https://test.com")
client = SdsClient(base_url="https://test.com", api_key="example_api_key")
# "UNKNOWN_ORG" has no seeded devices, so the bundle entry list will be empty
result = client.get_org_details(ods_code="UNKNOWN_ORG", get_endpoint=False)

Expand Down Expand Up @@ -398,24 +387,44 @@ def test_sds_client_no_endpoint_bundle_entries_returns_none_endpoint(
)
# Deliberately do not seed any endpoint for NOENDPOINT

client = SdsClient(base_url="https://test.com")
client = SdsClient(base_url="https://test.com", api_key="example_api_key")
result = client.get_org_details(ods_code="NOENDPOINT")

assert result.asid == "222222222222"
assert result.endpoint is None


def test_sds_client_respects_url(
mocker: MockerFixture,
) -> None:
def test_sds_client_respects_url(mocker: MockerFixture) -> None:
empty_bundle = Bundle.empty("searchset").model_dump()
mocked_get = mocker.patch(
"gateway_api.sds.client.get",
return_value=FakeResponse(status_code=200, headers={}, _json=empty_bundle),
)

client = SdsClient(base_url="https://a.different.url/base")
client = SdsClient(
base_url="https://a.different.url/base", api_key="example_api_key"
)
_ = client.get_org_details(ods_code="A12345", get_endpoint=False)

actual_url = mocked_get.call_args.args[0]
actual_headers = mocked_get.call_args.kwargs["headers"]
assert actual_url == "https://a.different.url/base/Device"
assert actual_headers["apikey"] == "example_api_key"


@patch("gateway_api.sds.client.SdsFhirApiStub")
@patch("gateway_api.sds.client.external_sds_get")
def test_get_with_stub(mock_external_get: Mock, mock_stub: Mock) -> None:
with ScopedEnvVars({"SDS_URL": "stub"}):
get("https://example.com/", headers={}, params={}, timeout=10)
assert mock_stub.return_value.get.called
assert not mock_external_get.called


@patch("gateway_api.sds.client.SdsFhirApiStub")
@patch("gateway_api.sds.client.external_sds_get")
def test_get_without_stub(mock_external_get: Mock, mock_stub: Mock) -> None:
with ScopedEnvVars({"SDS_URL": "https://www.example.com/"}):
get("https://example.com/", headers={}, params={}, timeout=10)
assert mock_external_get.called
assert not mock_stub.return_value.get.called
Loading
Loading