Merge branch 'main' into feature/GPCAPIM-427-remove-common

Author: ian-robinson-35

.vscode/settings.json

@@ -107,4 +107,5 @@
   "python.analysis.extraPaths": [
     "./gateway-api/stubs"
   ],
+  "python-envs.defaultEnvManager": "ms-python.python:pyenv",
 }

bruno/gateway-api/collections/Steel_Thread/environments/localInt.yml

@@ -0,0 +1,8 @@
+name: localInt
+variables:
+  - name: base_url
+    value: http://localhost:5000
+  - name: nhs_number
+    value: "9692140466"
+  - name: from_ods
+    value: A20047

gateway-api/src/gateway_api/app.py

@@ -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)
 
@@ -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)

gateway-api/src/gateway_api/controller.py

@@ -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
 
@@ -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,
         )
 

gateway-api/src/gateway_api/sds/__init__.py

@@ -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"]

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

@@ -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 (
@@ -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(
+    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__)
 
@@ -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
 
         if service_interaction_id is not None:
             self.service_interaction_id = service_interaction_id
@@ -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,

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

@@ -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
@@ -32,15 +31,13 @@ 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.
     """
-    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")
 
@@ -56,9 +53,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.
 
@@ -111,56 +106,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.
 
@@ -194,6 +182,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)
@@ -209,16 +198,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")
 
@@ -268,7 +255,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)
 
@@ -313,7 +301,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")
@@ -353,7 +341,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"
@@ -369,7 +357,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)
 
@@ -398,24 +386,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