Sort out docstrings

Author: Vox-Ben

gateway-api/src/gateway_api/common/common.py

@@ -1,30 +1,44 @@
+"""
+Shared lightweight types and helpers used across the gateway API.
+"""
+
 import re
 from dataclasses import dataclass
 
+# This project uses JSON request/response bodies as strings in the controller layer.
+# The alias is used to make intent clearer in function signatures.
 type json_str = str
 
 
 @dataclass
 class FlaskResponse:
+    """
+    Lightweight response container returned by controller entry points.
+
+    This mirrors the minimal set of fields used by the surrounding web framework.
+
+    :param status_code: HTTP status code for the response (e.g., 200, 400, 404).
+    :param data: Response body as text, if any.
+    :param headers: Response headers, if any.
+    """
+
+    # TODO: Un-ai all these docstrings
+
     status_code: int
     data: str | None = None
     headers: dict[str, str] | None = None
 
 
 def validate_nhs_number(value: str | int) -> bool:
-    # TODO: Un-AI all these docstrings
     """
     Validate an NHS number using the NHS modulus-11 check digit algorithm.
 
-    Algorithm summary:
-        - NHS number is 10 digits: d1..d9 + check digit d10
-        - Compute: total = d1*10 + d2*9 + ... + d9*2
-        - remainder = total % 11
-        - check = 11 - remainder
-        - If check == 11 => check digit must be 0
-        - If check == 10 => check digit must be 10 (impossible as digit) => invalid
-        - If remainder == 1 => check would be 10 => invalid
-        - Else check digit must match d10
+    The input may be a string or integer. Any non-digit separators in string
+    inputs (spaces, hyphens, etc.) are ignored.
+
+    :param value: NHS number as a string or integer. Non-digit characters
+        are ignored when a string is provided.
+    :returns: ``True`` if the number is a valid NHS number, otherwise ``False``.
     """
     str_value = str(value)  # Just in case they passed an integer
     digits = re.sub(r"\D", "", str_value or "")

gateway-api/src/gateway_api/common/test_common.py

@@ -1,22 +1,21 @@
-# tests/test_common.py
+"""
+Unit tests for :mod:`gateway_api.common.common`.
+"""
 
 from gateway_api.common import common
 
 
-def test_flask_response_defaults() -> None:
-    r = common.FlaskResponse(status_code=200)
-    assert r.status_code == 200
-    assert r.data is None
-    assert r.headers is None
-
-
 def test_validate_nhs_number_accepts_valid_number_with_separators() -> None:
+    """
+    Validate that separators (spaces, hyphens) are ignored and valid numbers pass.
+    """
     assert common.validate_nhs_number("943 476 5919") is True
     assert common.validate_nhs_number("943-476-5919") is True
     assert common.validate_nhs_number(9434765919) is True
 
 
 def test_validate_nhs_number_rejects_wrong_length_and_bad_check_digit() -> None:
+    """Validate that incorrect lengths and invalid check digits are rejected."""
     assert common.validate_nhs_number("") is False
     assert common.validate_nhs_number("943476591") is False  # 9 digits
     assert common.validate_nhs_number("94347659190") is False  # 11 digits

gateway-api/src/gateway_api/controller.py

@@ -1,3 +1,7 @@
+"""
+Controller layer for orchestrating calls to external services
+"""
+
 from __future__ import annotations
 
 import json
@@ -14,26 +18,39 @@
 from gateway_api.pds_search import PdsClient, PdsSearchResults
 
 
-class DownstreamServiceError(RuntimeError):
-    """Raised when a downstream dependency (PDS/SDS/GP Connect) fails."""
-
-
 @dataclass
 class RequestError(Exception):
-    """Raised (and handled) when there is a problem with the incoming request."""
+    """
+    Raised (and handled) when there is a problem with the incoming request.
+
+    Instances of this exception are caught by controller entry points and converted
+    into an appropriate :class:`FlaskResponse`.
+
+    :param status_code: HTTP status code that should be returned.
+    :param message: Human-readable error message.
+    """
 
     status_code: int
     message: str
 
     def __str__(self) -> str:
+        """
+        Coercing this exception to a string returns the error message.
+
+        :returns: The error message.
+        """
         return self.message
 
 
 @dataclass
 class SdsSearchResults:
     """
     Stub SDS search results dataclass.
+
     Replace this with the real one once it's implemented.
+
+    :param asid: Accredited System ID.
+    :param endpoint: Endpoint URL associated with the organisation, if applicable.
     """
 
     asid: str
@@ -43,22 +60,38 @@ class SdsSearchResults:
 class SdsClient:
     """
     Stub SDS client for obtaining ASID from ODS code.
+
     Replace this with the real one once it's implemented.
     """
 
     SANDBOX_URL = "https://example.invalid/sds"
 
     def __init__(
         self,
-        auth_token: str | None = None,
+        auth_token: str,
         base_url: str = SANDBOX_URL,
         timeout: int = 10,
     ) -> None:
+        """
+        Create an SDS client.
+
+        :param auth_token: Authentication token to present to SDS.
+        :param base_url: Base URL for SDS.
+        :param timeout: Timeout in seconds for SDS calls.
+        """
         self.auth_token = auth_token
         self.base_url = base_url
         self.timeout = timeout
 
     def get_org_details(self, ods_code: str) -> SdsSearchResults | None:
+        """
+        Retrieve SDS org details for a given ODS code.
+
+        This is a placeholder implementation that always returns an ASID and endpoint.
+
+        :param ods_code: ODS code to look up.
+        :returns: SDS search results or ``None`` if not found.
+        """
         # Placeholder implementation
         return SdsSearchResults(
             asid=f"asid_{ods_code}", endpoint="https://example-provider.org/endpoint"
@@ -68,6 +101,7 @@ def get_org_details(self, ods_code: str) -> SdsSearchResults | None:
 class GpConnectClient:
     """
     Stub GP Connect client for obtaining patient records.
+
     Replace this with the real one once it's implemented.
     """
 
@@ -79,6 +113,13 @@ def __init__(
         provider_asid: str,
         consumer_asid: str,
     ) -> None:
+        """
+        Create a GP Connect client.
+
+        :param provider_endpoint: Provider endpoint obtained from SDS.
+        :param provider_asid: Provider ASID obtained from SDS.
+        :param consumer_asid: Consumer ASID obtained from SDS.
+        """
         self.provider_endpoint = provider_endpoint
         self.provider_asid = provider_asid
         self.consumer_asid = consumer_asid
@@ -89,6 +130,16 @@ def access_structured_record(
         body: json_str,  # NOSONAR S1172 (ignore in stub)
         nhsnumber: str,  # NOSONAR S1172 (ignore in stub)
     ) -> requests.Response | None:
+        """
+        Retrieve a patient's structured record from GP Connect.
+
+        This stub just returns None, the real thing will be more interesting!
+
+        :param trace_id: Correlation/trace identifier for request tracking.
+        :param body: Original request body.
+        :param nhsnumber: NHS number as a string.
+        :returns: A ``requests.Response`` if the call was made, otherwise ``None``.
+        """
         # Placeholder implementation
         return None
 
@@ -98,11 +149,9 @@ class Controller:
     Orchestrates calls to PDS -> SDS -> GP Connect.
 
     Entry point:
-        - call_gp_connect(request_body_json, headers, auth_token) -> requests.Response
+        - ``call_gp_connect(request_body_json, headers, auth_token) -> FlaskResponse``
     """
 
-    # TODO: Un-AI the docstrings and comments
-
     gp_connect_client: GpConnectClient | None
 
     def __init__(
@@ -112,16 +161,30 @@ def __init__(
         nhsd_session_urid: str | None = None,
         timeout: int = 10,
     ) -> None:
+        """
+        Create a controller instance.
+
+        :param pds_base_url: Base URL for PDS client.
+        :param sds_base_url: Base URL for SDS client.
+        :param nhsd_session_urid: Session URID for NHS Digital session handling.
+        :param timeout: Timeout in seconds for downstream calls.
+        """
         self.pds_base_url = pds_base_url
         self.sds_base_url = sds_base_url
         self.nhsd_session_urid = nhsd_session_urid
         self.timeout = timeout
-
-        self.sds_client = SdsClient(base_url=sds_base_url, timeout=timeout)
         self.gp_connect_client = None
 
     def _get_details_from_body(self, request_body: json_str) -> int:
-        # --- Extract NHS number from request body ---
+        """
+        Parse request JSON and extract the NHS number as an integer.
+
+        :param request_body: JSON request body containing an ``"nhs-number"`` field.
+        :returns: NHS number as an integer.
+        :raises RequestError: If the request body is invalid, missing fields, or
+            contains an invalid NHS number.
+        """
+        # Extract NHS number from request body
         try:
             body: Any = json.loads(request_body)
         except (TypeError, json.JSONDecodeError):
@@ -130,6 +193,7 @@ def _get_details_from_body(self, request_body: json_str) -> int:
                 message='Request body must be valid JSON with an "nhs-number" field',
             ) from None
 
+        # Guard: require "dict-like" semantics without relying on isinstance checks.
         if not (
             hasattr(body, "__getitem__") and hasattr(body, "get")
         ):  # Must be a dict-like object
@@ -160,7 +224,16 @@ def _get_details_from_body(self, request_body: json_str) -> int:
     def _get_pds_details(
         self, auth_token: str, consumer_ods: str, nhs_number: int
     ) -> str:
-        # --- PDS: find patient and extract GP ODS code (provider ODS) ---
+        """
+        Call PDS to find the provider ODS code (GP ODS code) for a patient.
+
+        :param auth_token: Authorization token to use for PDS.
+        :param consumer_ods: Consumer organisation ODS code (from request headers).
+        :param nhs_number: NHS number (already coerced to an integer).
+        :returns: Provider ODS code (GP ODS code).
+        :raises RequestError: If the patient cannot be found or has no provider ODS code
+        """
+        # PDS: find patient and extract GP ODS code (provider ODS)
         pds = PdsClient(
             auth_token=auth_token,
             end_user_org_ods=consumer_ods,
@@ -195,7 +268,20 @@ def _get_pds_details(
     def _get_sds_details(
         self, auth_token: str, consumer_ods: str, provider_ods: str
     ) -> tuple[str, str, str]:
-        # --- SDS: Get provider details (ASID + endpoint) for provider ODS ---
+        """
+        Call SDS to obtain consumer ASID, provider ASID, and provider endpoint.
+
+        This method performs two SDS lookups:
+        - provider details (ASID + endpoint)
+        - consumer details (ASID)
+
+        :param auth_token: Authorization token to use for SDS.
+        :param consumer_ods: Consumer organisation ODS code (from request headers).
+        :param provider_ods: Provider organisation ODS code (from PDS).
+        :returns: Tuple of (consumer_asid, provider_asid, provider_endpoint).
+        :raises RequestError: If SDS data is missing or incomplete for provider/consumer
+        """
+        # SDS: Get provider details (ASID + endpoint) for provider ODS
         sds = SdsClient(
             auth_token=auth_token,
             base_url=self.sds_base_url,
@@ -229,7 +315,7 @@ def _get_sds_details(
                 ),
             )
 
-        # --- SDS: Get consumer details (ASID) for consumer ODS ---
+        # SDS: Get consumer details (ASID) for consumer ODS
         consumer_details: SdsSearchResults | None = sds.get_org_details(consumer_ods)
         if consumer_details is None:
             raise RequestError(
@@ -256,15 +342,25 @@ def call_gp_connect(
         auth_token: str,
     ) -> FlaskResponse:
         """
-        Expects a JSON request body containing an "nhs-number" field.
-        Also expects HTTP headers (from Flask) and extracts "Ods-from" as consumer_ods.
+        Controller entry point
+
+        Expects a JSON request body containing an ``"nhs-number"`` field.
+        Also expects HTTP headers (from Flask) and extracts:
+        - ``Ods-from`` as the consumer organisation ODS code
+        - ``X-Request-ID`` as the trace/correlation ID
 
+        Orchestration steps:
         1) Call PDS to obtain the patient's GP (provider) ODS code.
         2) Call SDS using provider ODS to obtain provider ASID + provider endpoint.
         3) Call SDS using consumer ODS to obtain consumer ASID.
-        4) Call GP Connect to obtain patient records
-        """
+        4) Call GP Connect to obtain patient records.
 
+        :param request_body: Raw JSON request body.
+        :param headers: HTTP headers from the request.
+        :param auth_token: Authorization token used for downstream services.
+        :returns: A :class:`~gateway_api.common.common.FlaskResponse` representing the
+            outcome.
+        """
         try:
             nhs_number = self._get_details_from_body(request_body)
         except RequestError as err:
@@ -273,7 +369,7 @@ def call_gp_connect(
                 data=str(err),
             )
 
-        # --- Extract consumer ODS from headers ---
+        # Extract consumer ODS from headers
         consumer_ods = headers.get("Ods-from", "").strip()
         if not consumer_ods:
             return FlaskResponse(
@@ -299,8 +395,7 @@ def call_gp_connect(
         except RequestError as err:
             return FlaskResponse(status_code=err.status_code, data=str(err))
 
-        # --- Call GP Connect with correct parameters ---
-        # (If these are dynamic per-request, reinitialise the client accordingly.)
+        # Call GP Connect with correct parameters
         self.gp_connect_client = GpConnectClient(
             provider_endpoint=provider_endpoint,
             provider_asid=provider_asid,
@@ -313,6 +408,9 @@ def call_gp_connect(
             nhsnumber=str(nhs_number),
         )
 
+        # If we get a None from GP Connect, that means that either the service did not
+        # respond or we didn't make the request to the service in the first place.
+        # Therefore a None is a 502, any real response just pass straight back.
         return FlaskResponse(
             status_code=response.status_code if response is not None else 502,
             data=response.text if response is not None else "GP Connect service error",
@@ -322,9 +420,16 @@ def call_gp_connect(
 
 def _coerce_nhs_number_to_int(value: str | int) -> int:
     """
-    Coerce NHS number to int with basic validation.
-    NHS numbers are 10 digits, but leading zeros are not typically used.
-    Adjust validation as needed for your domain rules.
+    Coerce an NHS number to an integer with basic validation.
+
+    Notes:
+    - NHS numbers are 10 digits.
+    - Input may include whitespace (e.g., ``"943 476 5919"``).
+
+    :param value: NHS number value, as a string or integer.
+    :returns: The coerced NHS number as an integer.
+    :raises ValueError: If the NHS number is non-numeric, the wrong length, or fails
+        validation.
     """
     try:
         stripped = cast("str", value).strip().replace(" ", "")