diff --git a/Justfile b/Justfile index 0853917..493f3b3 100644 --- a/Justfile +++ b/Justfile @@ -23,6 +23,8 @@ test-branch: @just test --cov-branch publish: + @test -n "${GITHUB_REF_NAME:-}" || (echo "GITHUB_REF_NAME is required; refusing to run outside CI" >&2; exit 1) + @test -n "${PYPI_TOKEN:-}" || (echo "PYPI_TOKEN is required; refusing to run outside CI" >&2; exit 1) rm -rf dist uv version $GITHUB_REF_NAME uv build diff --git a/planning/deferred-work.md b/planning/deferred-work.md index c605912..0a74b00 100644 --- a/planning/deferred-work.md +++ b/planning/deferred-work.md @@ -4,15 +4,12 @@ Items raised in reviews that are real but not actionable now. ## Deferred from: retrospective review of stories 1-1 through 1-5 (2026-05-31) -- **`PydanticDecoder.decode` `TypeError` fallback is unreachable through normal usage** — the `except TypeError` branch only triggers when `_get_adapter(model)` raises during `lru_cache` lookup, which requires `model` to be unhashable; every concrete `type[T]` is hashable. The branch is now deliberately exercised via mock at `tests/test_decoders_pydantic.py:141-156`, so the contract is pinned even though no production caller hits it. (`src/httpware/decoders/pydantic.py:22-26`) - **`_get_adapter` `lru_cache` is module-global, not per-decoder instance** — keyed by `model` only; two `PydanticDecoder()` instances with different configurations (none today) would share adapters, and the cache survives across tests unless explicitly cleared. Revisit if/when a configurable `PydanticDecoder(mode=..., strict=...)` lands. (`src/httpware/decoders/pydantic.py:12-14`) - **`extensions=dict(request.extensions)` forwards opaque payloads to httpx2 verbatim** — `httpx2` interprets specific keys (e.g. `timeout`, `sni_hostname`); a typo or unknown key silently bypasses our timeout/limits config. The seam now has a real user: `AsyncClient._build_request` writes `extensions["timeout"]` (`src/httpware/client.py:140-142`). Epic 3 timeout middleware will own the extensions contract; introducing an allowlist now risks blocking legitimate forward-compat uses. (`src/httpware/transports/httpx2.py:121`) -- **`Response.json()` raises raw `JSONDecodeError` and ignores declared charset** — `json.loads(self.content)` propagates `json.JSONDecodeError` to callers and ignores any declared charset (`text` honors it); inconsistent with `_try_decode_json` in the transport which never raises. `AsyncClient` doesn't call `.json()` (it goes through `decoder.decode`), but end users do. Two-line fix; bundle with the next response-API touch. (`src/httpware/response.py:50-52`) ## Deferred from: code review of story-1-5 (2026-05-14) - **Empty/malformed payload tests** — `b""`, `b"null"`, `b"{}"`, invalid UTF-8: current pydantic-core behavior is correct but unpinned; a future pydantic upgrade could change error types undetected. (`tests/test_decoders_pydantic.py`) -- **`PLR2004` per-file-ignores** — `# noqa: PLR2004` repeated 5× in this test file; idiomatic fix is `tool.ruff.lint.per-file-ignores` for `tests/*`. Project-wide lint-config tidy. (`tests/test_decoders_pydantic.py:63,67,83,107,153`) ## Deferred from: code review of story-1-4 (2026-05-14) @@ -41,7 +38,5 @@ Items raised in reviews that are real but not actionable now. ## Deferred from: code review of story-1-1 (2026-05-13) -- **`just publish` lacks env-var validation** — recipe assumes `GITHUB_REF_NAME` and `PYPI_TOKEN` are set; running locally could corrupt the version. Add `test -n "$GITHUB_REF_NAME"` guard before release work. (`Justfile:25-29`) -- **`uv_build>=0.11,<0.12` narrow window** — single-minor band will expire as soon as uv_build 0.12 ships; bump when that happens. (`pyproject.toml:49`) - **Unpinned `ruff`/`ty` with `select=["ALL"]`** — any new ruff release adds rules and can break CI overnight. Pin major versions or pin specific rules when a regression occurs. (`pyproject.toml` `[dependency-groups] lint`, `[tool.ruff.lint] select`) -- **No `[test]` extra; CI installs all extras** — `just install` runs `uv sync --all-extras --group lint`, so every CI run pulls msgspec/otel/niquests even though most tests don't need them. Declare a `test` extra (or move test-only deps into a dedicated dependency-group) and switch CI to the narrower install. Mild YAGNI today; revisit when extras grow heavier. (`pyproject.toml` `[project.optional-dependencies]`, `Justfile:install`) \ No newline at end of file +- **No `[test]` extra; CI installs all extras** — `just install` runs `uv sync --all-extras --group lint`, so every CI run pulls msgspec/otel/niquests even though most tests don't need them. Declare a `test` extra (or move test-only deps into a dedicated dependency-group) and switch CI to the narrower install. Mild YAGNI today; revisit when extras grow heavier. (`pyproject.toml` `[project.optional-dependencies]`, `Justfile:install`) diff --git a/pyproject.toml b/pyproject.toml index 2283552..1d0b979 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -46,7 +46,7 @@ repository = "https://github.com/modern-python/httpware" docs = "https://httpware.readthedocs.io" [build-system] -requires = ["uv_build>=0.11,<0.12"] +requires = ["uv_build>=0.11,<1.0"] build-backend = "uv_build" [tool.uv.build-backend] diff --git a/src/httpware/response.py b/src/httpware/response.py index 64ce2e0..6c9069b 100644 --- a/src/httpware/response.py +++ b/src/httpware/response.py @@ -48,8 +48,13 @@ def text(self) -> str: return self.content.decode("utf-8") def json(self) -> Any: # noqa: ANN401 - """Parse `content` as JSON.""" - return json.loads(self.content) + """Parse `content` as JSON using the declared charset (default UTF-8). + + Raises: + json.JSONDecodeError: if the body is not valid JSON. + + """ + return json.loads(self.text) def with_headers(self, headers: Mapping[str, str]) -> Self: """Return a copy with the given headers merged in (incoming keys override existing).""" diff --git a/src/httpware/transports/httpx2.py b/src/httpware/transports/httpx2.py index 1ab32a0..e272880 100644 --- a/src/httpware/transports/httpx2.py +++ b/src/httpware/transports/httpx2.py @@ -10,6 +10,7 @@ import json import time from contextlib import AbstractAsyncContextManager +from http import HTTPStatus from typing import Any import httpx2 @@ -141,10 +142,10 @@ async def __call__(self, request: Request) -> Response: # to the last value — see class docstring; widens with the # multi-valued header contract in a later story. headers = dict(resp.headers) - if 400 <= status < 600: # noqa: PLR2004 + if HTTPStatus.BAD_REQUEST <= status < 600: # noqa: PLR2004 — 600 is the synthetic 5xx upper bound exc_class = STATUS_TO_EXCEPTION.get( status, - ClientStatusError if status < 500 else ServerStatusError, # noqa: PLR2004 + ClientStatusError if status < HTTPStatus.INTERNAL_SERVER_ERROR else ServerStatusError, ) raise exc_class( status=status, diff --git a/tests/test_middleware.py b/tests/test_middleware.py index b005f28..bda6234 100644 --- a/tests/test_middleware.py +++ b/tests/test_middleware.py @@ -3,6 +3,7 @@ import asyncio from collections.abc import Awaitable, Callable from contextlib import AbstractAsyncContextManager +from http import HTTPStatus from typing import get_type_hints import pytest @@ -64,7 +65,7 @@ async def test_empty_list_composes_to_transport_call() -> None: request = _make_request() response = await dispatch(request) - assert response.status == 200 # noqa: PLR2004 + assert response.status == HTTPStatus.OK assert response.content == b"transport" assert response.headers["x-from"] == "transport" @@ -192,7 +193,7 @@ async def __call__(self, request: Request, next: Next) -> Response: # noqa: A00 response = await compose([ShortCircuit(), NeverReached()], CountingTransport())(_make_request()) - assert response.status == 418 # noqa: PLR2004 + assert response.status == HTTPStatus.IM_A_TEAPOT assert response.content == b"teapot" assert transport_calls == 0 @@ -265,7 +266,7 @@ async def __call__(self, request: Request, next: Next) -> Response: # noqa: A00 for _ in range(3): response = await dispatch(_make_request()) - assert response.status == 200 # noqa: PLR2004 + assert response.status == HTTPStatus.OK assert count == 3 # noqa: PLR2004 @@ -332,7 +333,7 @@ async def recover(request: Request, exc: Exception) -> Response | None: # noqa: transport = RecordedTransport(default=RuntimeError("boom")) response = await compose([recover], transport)(_make_request()) - assert response.status == 503 # noqa: PLR2004 + assert response.status == HTTPStatus.SERVICE_UNAVAILABLE assert response.headers["x-recovered"] == "true" assert response.content == b"recovered" diff --git a/tests/test_response.py b/tests/test_response.py index 50ee60a..192a711 100644 --- a/tests/test_response.py +++ b/tests/test_response.py @@ -1,6 +1,7 @@ """Unit tests for httpware.response.Response.""" from dataclasses import FrozenInstanceError +from http import HTTPStatus import pytest @@ -83,6 +84,18 @@ def test_response_json_parses_body() -> None: assert resp.json() == {"a": 1, "b": [2, 3]} +def test_response_json_uses_declared_charset() -> None: + body = '{"name": "café"}'.encode("iso-8859-1") + resp = Response( + status=HTTPStatus.OK, + headers={"content-type": "application/json; charset=iso-8859-1"}, + content=body, + url="/", + elapsed=0.0, + ) + assert resp.json() == {"name": "café"} + + def test_response_equality_on_identical_fields() -> None: r1 = Response(status=200, headers={"a": "1"}, content=b"x", url="/", elapsed=0.5) r2 = Response(status=200, headers={"a": "1"}, content=b"x", url="/", elapsed=0.5) @@ -108,12 +121,12 @@ def test_response_with_headers_overrides_existing_key() -> None: def test_response_with_status_replaces_status() -> None: resp = Response(status=200, headers={"a": "1"}, content=b"body", url="/x", elapsed=0.5) new = resp.with_status(503) - assert new.status == 503 # noqa: PLR2004 + assert new.status == HTTPStatus.SERVICE_UNAVAILABLE assert new.headers == {"a": "1"} assert new.content == b"body" assert new.url == "/x" assert new.elapsed == 0.5 # noqa: PLR2004 - assert resp.status == 200 # noqa: PLR2004 + assert resp.status == HTTPStatus.OK def test_response_with_status_accepts_arbitrary_int() -> None: diff --git a/tests/test_transports_httpx2.py b/tests/test_transports_httpx2.py index faacfc7..a25f0cb 100644 --- a/tests/test_transports_httpx2.py +++ b/tests/test_transports_httpx2.py @@ -2,6 +2,7 @@ import asyncio from collections.abc import Callable +from http import HTTPStatus import httpx2 import pytest @@ -69,7 +70,7 @@ async def test_success_path_returns_response() -> None: await transport.aclose() assert isinstance(resp, Response) - assert resp.status == 200 # noqa: PLR2004 + assert resp.status == HTTPStatus.OK assert resp.content == b"hello" assert resp.url == "http://example.com/x" # lowercase ASCII keys per AC11 @@ -100,7 +101,7 @@ async def test_success_status_200_returns_response_not_raises() -> None: resp = await transport(Request(method="GET", url="http://example.com/")) finally: await transport.aclose() - assert resp.status == 200 # noqa: PLR2004 + assert resp.status == HTTPStatus.OK @pytest.mark.parametrize(("code", "exc_cls"), _STATUS_LEAVES) @@ -132,7 +133,7 @@ async def test_unknown_4xx_falls_back_to_client_status_error() -> None: finally: await transport.aclose() assert type(info.value) is ClientStatusError - assert info.value.status == 418 # noqa: PLR2004 + assert info.value.status == HTTPStatus.IM_A_TEAPOT async def test_unknown_5xx_falls_back_to_server_status_error() -> None: @@ -143,7 +144,7 @@ async def test_unknown_5xx_falls_back_to_server_status_error() -> None: finally: await transport.aclose() assert type(info.value) is ServerStatusError - assert info.value.status == 504 # noqa: PLR2004 + assert info.value.status == HTTPStatus.GATEWAY_TIMEOUT # ----- (e) _try_decode_json branches ----------------------------------------