Commit 40b240d2 authored by Dimitrios Gogos's avatar Dimitrios Gogos
Browse files

fix: scope SRM 422 translation to sync queries and key it on RFC 7807 type



Four defects in one fifteen-line block; they are one change because each edit
sits inside the previous one.

Scope. The 422 branch lived in the shared _request, so a location-retrieval
rule applied to every SRM call. An SRM 422 during a zone read became a CAMARA
422 SERVICE_NOT_APPLICABLE where it had previously been a 503 -- and neither
the EAM nor the QoD vendored spec documents a 422 at all, so OEG could emit a
status its own contract excludes. _request now translates only when a caller
passes a `problems` table; retrieve_location is the only one that does.

Signal. The mapping keyed on problem["code"]. SRM answers RFC 7807
(REQ-SRM-10), which defines type/title/status/detail/instance and no `code`,
and the vocabulary it matched appears nowhere in the architecture bundle. Every
422 therefore fell through to the fallback, making the four specific CAMARA
codes unreachable in production. Now keyed on `type`, RFC 7807's
machine-readable problem identifier -- no extension member needed -- against
the vocabulary in srm/interface-contract.md E.2. An unknown type still
degrades to SERVICE_NOT_APPLICABLE, so SRM can add one without waiting on OEG.

Trust boundary. The outward `message` was SRM's `detail`, which is internal
diagnostic prose ("adapter oai-nef-01 timed out on subscriber 262011234567890")
and crosses to the app provider. Messages now come from the vendored CAMARA
spec; `detail` is logged instead.

Robustness. response.json() sat inside a try that catches only httpx errors, so
a 422 carrying an HTML error page, an empty body, or a JSON array escaped as
JSONDecodeError/AttributeError and surfaced as 500 -- blaming OEG for a
downstream fault. An unreadable body now falls through to the >= 400 branch and
becomes 503. That is deliberately distinct from an unrecognized type: there SRM
refused, here we cannot confirm the response even came from SRM.

test_eam_contract's _request doubles gain the new parameter. The unit suite
passed without that; mypy caught it.

Co-Authored-By: default avatarClaude Opus 5 <noreply@anthropic.com>
parent 31552279
Loading
Loading
Loading
Loading
+46 −15
Original line number Diff line number Diff line
@@ -25,14 +25,36 @@ from open_exposure_gateway.domain.quality_on_demand import SRMNetworkCapability

logger = structlog.get_logger(__name__)

_QUERY_PROBLEM_CODES: dict[str, ErrorCode] = {
    "unable_to_locate": ErrorCode.LOCATION_RETRIEVAL_UNABLE_TO_LOCATE,
    "unable_to_fulfill_max_age": ErrorCode.LOCATION_RETRIEVAL_UNABLE_TO_FULFILL_MAX_AGE,
    "unable_to_fulfill_max_surface": (ErrorCode.LOCATION_RETRIEVAL_UNABLE_TO_FULFILL_MAX_SURFACE),
    "unsupported_identifier": ErrorCode.UNSUPPORTED_IDENTIFIER,
    "service_not_applicable": ErrorCode.SERVICE_NOT_APPLICABLE,
# SRM's RFC 7807 problem `type` -> the CAMARA code and the message the vendored spec pairs
# with it (srm/interface-contract.md §E.2).
# The message is fixed here rather than passed through from `detail` because this body crosses
# the trust boundary to the app provider; `detail` is logged instead.
_PROBLEM_NS = "https://etsi.org/sdg/oop/problems/"
_QUERY_PROBLEMS: dict[str, tuple[ErrorCode, str]] = {
    f"{_PROBLEM_NS}unable-to-locate": (
        ErrorCode.LOCATION_RETRIEVAL_UNABLE_TO_LOCATE,
        "The network is unable to locate the device",
    ),
    f"{_PROBLEM_NS}unable-to-fulfill-max-age": (
        ErrorCode.LOCATION_RETRIEVAL_UNABLE_TO_FULFILL_MAX_AGE,
        "Unable to provide expected freshness for location",
    ),
    f"{_PROBLEM_NS}unable-to-fulfill-max-surface": (
        ErrorCode.LOCATION_RETRIEVAL_UNABLE_TO_FULFILL_MAX_SURFACE,
        "Unable to provide accurate acceptable surface for location",
    ),
    f"{_PROBLEM_NS}unsupported-identifier": (
        ErrorCode.UNSUPPORTED_IDENTIFIER,
        "The identifier provided is not supported.",
    ),
    f"{_PROBLEM_NS}service-not-applicable": (
        ErrorCode.SERVICE_NOT_APPLICABLE,
        "The service is not available for the provided identifier.",
    ),
}

_UNKNOWN_QUERY_PROBLEM = _QUERY_PROBLEMS[f"{_PROBLEM_NS}service-not-applicable"]


class SRMClient:
    def __init__(self) -> None:
@@ -47,6 +69,7 @@ class SRMClient:
        json: dict[str, Any] | None = None,
        params: dict[str, Any] | None = None,
        headers: dict[str, str] | None = None,
        problems: dict[str, tuple[ErrorCode, str]] | None = None,
    ) -> Any:
        url = f"{self.base_url}{path}"
        log = logger.bind(
@@ -69,15 +92,22 @@ class SRMClient:
                log.warning("SRM resource not found", url=url)
                raise NotFoundException(message="Resource not found")

            if response.status_code == 422:
            if response.status_code == 422 and problems is not None:
                try:
                    problem = response.json()
                code = problem.get("code")
                error_code = _QUERY_PROBLEM_CODES.get(code, ErrorCode.SERVICE_NOT_APPLICABLE)
                log.warning("SRM could not fulfil query", code=code, mapped_code=error_code)
                raise UnprocessableEntityException(
                    error_code=error_code,
                    message=problem.get("detail") or "Unable to fulfil the request",
                except ValueError:  # JSONDecodeError -- HTML error page, empty body, truncation
                    problem = None
                if isinstance(problem, dict):
                    problem_type = problem.get("type")
                    key = problem_type if isinstance(problem_type, str) else ""
                    error_code, message = problems.get(key, _UNKNOWN_QUERY_PROBLEM)
                    log.warning(
                        "SRM could not fulfil query",
                        problem_type=problem_type,
                        mapped_code=error_code,
                        detail=problem.get("detail"),
                    )
                    raise UnprocessableEntityException(error_code=error_code, message=message)

            if response.status_code >= 400:
                log.error("SRM returned error", status=response.status_code, body=response.text)
@@ -218,6 +248,7 @@ class SRMClient:
                "/internal/network-queries/location",
                json=query.model_dump(mode="json", exclude_none=True),
                headers=headers,
                problems=_QUERY_PROBLEMS,
            )
        except NotFoundException as exc:
            raise NotFoundException(
+3 −0
Original line number Diff line number Diff line
@@ -292,6 +292,7 @@ class TestInternalHttpPaths:
            json: dict[str, Any] | None = None,
            params: dict[str, Any] | None = None,
            headers: dict[str, str] | None = None,
            problems: Any = None,
        ) -> Any:
            calls.append((method, path))
            return []
@@ -316,6 +317,7 @@ class TestInternalHttpPaths:
            json: dict[str, Any] | None = None,
            params: dict[str, Any] | None = None,
            headers: dict[str, str] | None = None,
            problems: Any = None,
        ) -> Any:
            nonlocal recorded_params
            recorded_params = params
@@ -375,6 +377,7 @@ class TestInternalHttpPaths:
            json: dict[str, Any] | None = None,
            params: dict[str, Any] | None = None,
            headers: dict[str, str] | None = None,
            problems: Any = None,
        ) -> Any:
            return srm_response

+113 −15
Original line number Diff line number Diff line
@@ -12,7 +12,7 @@ from typing import Any
import httpx
import pytest

from open_exposure_gateway.adapters.http.srm_client import SRMClient
from open_exposure_gateway.adapters.http.srm_client import _PROBLEM_NS, SRMClient
from open_exposure_gateway.core.exceptions import (
    DownstreamServiceException,
    ErrorCode,
@@ -144,27 +144,54 @@ def _location_query() -> SRMLocationQuery:


@pytest.mark.parametrize(
    ("problem_code", "expected"),
    ("problem_type", "expected", "expected_message"),
    [
        ("unable_to_locate", ErrorCode.LOCATION_RETRIEVAL_UNABLE_TO_LOCATE),
        ("unable_to_fulfill_max_age", ErrorCode.LOCATION_RETRIEVAL_UNABLE_TO_FULFILL_MAX_AGE),
        (
            "unable_to_fulfill_max_surface",
            "unable-to-locate",
            ErrorCode.LOCATION_RETRIEVAL_UNABLE_TO_LOCATE,
            "The network is unable to locate the device",
        ),
        (
            "unable-to-fulfill-max-age",
            ErrorCode.LOCATION_RETRIEVAL_UNABLE_TO_FULFILL_MAX_AGE,
            "Unable to provide expected freshness for location",
        ),
        (
            "unable-to-fulfill-max-surface",
            ErrorCode.LOCATION_RETRIEVAL_UNABLE_TO_FULFILL_MAX_SURFACE,
            "Unable to provide accurate acceptable surface for location",
        ),
        (
            "unsupported-identifier",
            ErrorCode.UNSUPPORTED_IDENTIFIER,
            "The identifier provided is not supported.",
        ),
        (
            "service-not-applicable",
            ErrorCode.SERVICE_NOT_APPLICABLE,
            "The service is not available for the provided identifier.",
        ),
        ("unsupported_identifier", ErrorCode.UNSUPPORTED_IDENTIFIER),
        ("service_not_applicable", ErrorCode.SERVICE_NOT_APPLICABLE),
    ],
)
async def test_422_problem_code_maps_to_camara_code(
    monkeypatch: pytest.MonkeyPatch, problem_code: str, expected: ErrorCode
async def test_422_problem_type_maps_to_camara_code(
    monkeypatch: pytest.MonkeyPatch,
    problem_type: str,
    expected: ErrorCode,
    expected_message: str,
) -> None:
    """SRM's neutral RFC 7807 code is the only signal for conditions only the network knows;
    this is the table that turns it into what the CAMARA consumer branches on."""
    """RFC 7807's `type` is the only machine-readable signal for conditions only the network
    knows (srm/interface-contract.md §E.2); this is the table that turns it into what the
    CAMARA consumer branches on. The message comes from the vendored CAMARA spec, not from
    SRM's `detail` -- see the next test."""
    client = _client(
        monkeypatch,
        lambda request: httpx.Response(
            422, json={"status": 422, "code": problem_code, "detail": "nope"}
            422,
            json={
                "status": 422,
                "type": f"{_PROBLEM_NS}{problem_type}",
                "detail": "internal detail",
            },
        ),
    )

@@ -172,21 +199,92 @@ async def test_422_problem_code_maps_to_camara_code(
        await client.retrieve_location(_location_query())

    assert exc_info.value.error_code == expected
    assert exc_info.value.message == "nope"
    assert exc_info.value.message == expected_message


async def test_srm_problem_detail_is_not_echoed_to_the_consumer(
    monkeypatch: pytest.MonkeyPatch,
) -> None:
    """`detail` is SRM's internal diagnostic text. It is logged, but the body OEG returns
    crosses the trust boundary to the app provider, so it carries the spec's wording."""
    client = _client(
        monkeypatch,
        lambda request: httpx.Response(
            422,
            json={
                "status": 422,
                "type": f"{_PROBLEM_NS}unable-to-locate",
                "detail": "adapter oai-nef-01 timed out on subscriber 262011234567890",
            },
        ),
    )

    with pytest.raises(UnprocessableEntityException) as exc_info:
        await client.retrieve_location(_location_query())

    assert "oai-nef-01" not in exc_info.value.message
    assert exc_info.value.message == "The network is unable to locate the device"


@pytest.mark.parametrize("body", [{"status": 422, "code": "something_new"}, {"status": 422}])
async def test_422_on_other_endpoints_stays_a_downstream_failure(
    monkeypatch: pytest.MonkeyPatch,
) -> None:
    """The 422 vocabulary belongs to the sync query paths. Neither the EAM nor the QoD
    vendored spec documents a 422 at all, so SRM answering 422 on a zone read must not
    become a CAMARA 422 with a location-retrieval code."""
    client = _client(
        monkeypatch,
        lambda request: httpx.Response(
            422, json={"status": 422, "title": "Validation failed", "detail": "zone filter invalid"}
        ),
    )

    with pytest.raises(DownstreamServiceException):
        await client.get_zones(x_correlator=None)


@pytest.mark.parametrize(
    "body",
    [
        {"status": 422, "type": "https://etsi.org/sdg/oop/problems/something-new"},
        {"status": 422, "type": "about:blank"},
        {"status": 422},
    ],
)
async def test_unrecognized_422_falls_back_to_service_not_applicable(
    monkeypatch: pytest.MonkeyPatch, body: dict[str, Any]
) -> None:
    """An SRM build newer than this OEG must not produce a 500. SERVICE_NOT_APPLICABLE is a
    documented CAMARA code and an honest answer."""
    documented CAMARA code and an honest answer, so adding a problem type on SRM's side stays
    backward-compatible."""
    client = _client(monkeypatch, lambda request: httpx.Response(422, json=body))

    with pytest.raises(UnprocessableEntityException) as exc_info:
        await client.retrieve_location(_location_query())

    assert exc_info.value.error_code == ErrorCode.SERVICE_NOT_APPLICABLE
    assert exc_info.value.message == "The service is not available for the provided identifier."


@pytest.mark.parametrize(
    ("name", "response_kwargs"),
    [
        ("html error page", {"content": b"<html>502 Bad Gateway</html>"}),
        ("empty body", {"content": b""}),
        ("json but not an object", {"json": ["something"]}),
    ],
)
async def test_unreadable_422_body_is_a_downstream_failure(
    monkeypatch: pytest.MonkeyPatch, name: str, response_kwargs: dict[str, Any]
) -> None:
    """A 422 whose body is not a problem document must not surface as a 500 -- that reads as
    an OEG bug when the fault is downstream. It is also distinct from an unrecognized problem
    type: there we know SRM refused, here we cannot confirm the response even came from SRM
    rather than a proxy in between, so it stays retryable."""
    client = _client(monkeypatch, lambda request: httpx.Response(422, **response_kwargs))

    with pytest.raises(DownstreamServiceException):
        await client.retrieve_location(_location_query())


async def test_location_404_is_identifier_not_found(monkeypatch: pytest.MonkeyPatch) -> None: