diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 737eecb..8cce9d4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,7 +42,7 @@ jobs: cache-dependency-glob: "**/pyproject.toml" - run: uv python install ${{ matrix.python-version }} - run: just install - - run: just test --cov=src/httpware --cov-report xml + - run: just test --cov-report xml - uses: codecov/codecov-action@v4.0.1 env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/CHANGELOG.md b/CHANGELOG.md index de50c1d..052b3e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,11 +10,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), - Initial project scaffold: `src/httpware/` package, `py.typed` marker, `pyproject.toml` with `uv_build` backend. - Org conventions ported from `modern-python/modern-di`: `Justfile`, `.github/workflows/ci.yml`, `[tool.ruff]` config, `[tool.pytest.ini_options]`, dev and lint dep groups. -- Declared dependencies: `httpx2>=2.0.0b1,<3.0`, `pydantic>=2.0,<3.0`. +- Declared dependencies: `httpx2>=2.0.0,<3.0`, `pydantic>=2.0,<3.0`. - Declared install extras: `[msgspec]`, `[otel]`, `[niquests]`, `[all]`. - `SECURITY.md` with 90-day private-disclosure window. - `CONTRIBUTING.md` with development workflow. - `CLAUDE.md` with AI-agent guidance. - Core data types: `Request`, `Response`, `Limits`, `Timeout`, `ClientConfig` — frozen+slotted dataclasses with `with_*` immutability helpers on `Request` and computed `text`/`json()` accessors on `Response` (Story 1.2). -[Unreleased]: https://github.com/modern-python/httpware/compare/HEAD...HEAD +[Unreleased]: https://github.com/modern-python/httpware/commits/main diff --git a/README.md b/README.md index d934922..172e8ef 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ Optional extras: ```bash pip install httpware[msgspec] # msgspec ResponseDecoder pip install httpware[otel] # OpenTelemetry instrumentation +pip install httpware[niquests] # niquests transport pip install httpware[all] # all of the above ``` diff --git a/docs/deferred-work.md b/docs/deferred-work.md new file mode 100644 index 0000000..62f6009 --- /dev/null +++ b/docs/deferred-work.md @@ -0,0 +1,25 @@ +# Deferred Work + +Items raised in reviews that are real but not actionable now. + +## Deferred from: code review of story-1-2 (2026-05-13) + +- **Charset parser robustness** — quoted whitespace, mismatched quotes, multi-`charset=` directives, substring false-positives (e.g. `boundary` containing `charset=`). (`src/httpware/response.py:21-26`) +- **Header name/value validation** — `with_header` accepts CR/LF (injection), `None`, empty string. Lands with header-handling story (2.3 or later). (`src/httpware/request.py:21-23`) +- **URL validation** — `with_url("")` accepts empty; `base_url` has no trailing-slash normalization. (`src/httpware/request.py:25-27`, `src/httpware/config.py:27-33`) +- **`with_query(None)` handling** — currently accepted and breaks downstream iteration. (`src/httpware/request.py:33-35`) +- **`Timeout` / `Limits` negative-value validation** — no `__post_init__` guard; nonsensical values silently accepted. (`src/httpware/config.py:10-22`) +- **Multi-valued query params** — `Mapping[str, str]` cannot express `?tag=a&tag=b`. Type widening needed. (`src/httpware/request.py:8`) +- **Streaming / async-iterable request bodies** — `body: bytes | None` only. Revisit in transport stories. (`src/httpware/request.py:11`) +- **`with_headers` / `with_cookie` / `with_extension` merge helpers** — only `with_header` (single) and `with_query` (replace) exist. Story 2.3 will fill this in. (`src/httpware/request.py:20-35`) +- **`Response.json()` honor declared charset** — `json.loads(bytes)` auto-detects only UTF-8/16/32. Real APIs vary. (`src/httpware/response.py:44-45`) +- **`@final` to prevent subclassing** — frozen+slots subclassing is fragile. No current subclasser; defer until needed. (`src/httpware/request.py`, `response.py`, `config.py`) + +## Deferred from: code review of story-1-1 (2026-05-13) + +- **Codecov upload fails on fork PRs** — fork PRs cannot access `CODECOV_TOKEN`; matches modern-di pattern, accepted tradeoff. (`.github/workflows/ci.yml:46-52`) +- **`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:54`) +- **Python 3.14 wheel availability risk** — `httpx2` / `pydantic` / `uv_build` may not have 3.14 wheels yet, breaking the matrix entry. Watch CI red on 3.14. (`.github/workflows/ci.yml:30-33`) +- **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:70-72, 84-85`) +- **No `[test]` extra; CI uses `--all-extras`** — future heavy extras will be installed in every CI run. Declare a `test` extra and switch CI to `--extra test`. (`pyproject.toml:35-47`) diff --git a/docs/stories/1-1-project-scaffold-and-tooling.md b/docs/stories/1-1-project-scaffold-and-tooling.md index d31307d..14ddc4d 100644 --- a/docs/stories/1-1-project-scaffold-and-tooling.md +++ b/docs/stories/1-1-project-scaffold-and-tooling.md @@ -3,7 +3,7 @@ story_key: 1-1-project-scaffold-and-tooling epic: 1 story: 1 title: Project scaffold and tooling -status: review +status: done created: 2026-05-12 completed: 2026-05-13 input_documents: @@ -168,7 +168,7 @@ input_documents: ## File List -Files added (15 total): +Files added (14 total): - `.github/workflows/ci.yml` — CI workflow (ruff/ty/pytest, Python 3.11-3.14 matrix) - `.gitignore` — modern-di convention + project-specific ignores @@ -201,4 +201,25 @@ Generated/transient (not committed): ## Status -`review` +`done` + +### Review Findings + +_Code review run: 2026-05-13. Reviewers: Blind Hunter, Edge Case Hunter, Acceptance Auditor. 6 patches applied, 3 dismissed by maintainer (not errors), 6 deferred, 20 dismissed as noise (2 decision-needed items were resolved → dismissed: `.gitignore plan.md` blacklist is intentional convention; AC6 lint-on-single-version matches modern-di canon and is accepted)._ + +- [x] [Review][Patch] CHANGELOG declared `httpx2>=2.0.0b1,<3.0` while pyproject shipped `>=2.0.0,<3.0`. [`CHANGELOG.md:13`] — applied +- [x] [Review][Patch] CHANGELOG `[Unreleased]` link was `compare/HEAD...HEAD` — replaced with `commits/main` until first tag. [`CHANGELOG.md:20`] — applied +- [x] [Review][Dismissed] `version = "0"` placeholder. [`pyproject.toml:29`] — dismissed by maintainer, not an error +- [x] [Review][Patch] `[all]` extra refactored to self-reference siblings: `all = ["httpware[msgspec,otel,niquests]"]`. [`pyproject.toml:42-47`] — applied +- [x] [Review][Patch] README "Optional extras" snippet — added `pip install httpware[niquests]`. [`README.md:22-26`] — applied +- [x] [Review][Dismissed] CI `timeout-minutes`. [`.github/workflows/ci.yml:14, 26`] — dismissed by maintainer, not an error +- [x] [Review][Dismissed] CI explicit `permissions:` block. [`.github/workflows/ci.yml:1-12`] — dismissed by maintainer, not an error +- [x] [Review][Patch] Duplicate `--cov` flag — dropped `--cov=src/httpware` from CI invocation; `addopts` is now the single source of `--cov` source. [`.github/workflows/ci.yml:45`] — applied +- [x] [Review][Patch] File List header `15 total` → `14 total`. [`docs/stories/1-1-project-scaffold-and-tooling.md:171`] — applied + +- [x] [Review][Defer] Codecov upload fails on fork PRs without `CODECOV_TOKEN`; matches modern-di pattern, accepted tradeoff. [`.github/workflows/ci.yml:46-52`] — deferred, pre-existing +- [x] [Review][Defer] `just publish` does not validate `GITHUB_REF_NAME` / `PYPI_TOKEN`; local invocation could corrupt the version. [`Justfile:25-29`] — deferred, release-flow hygiene +- [x] [Review][Defer] `uv_build>=0.11,<0.12` is a one-minor-version window that will expire fast. [`pyproject.toml:54`] — deferred, will bump on release +- [x] [Review][Defer] Python 3.14 in CI matrix; httpx2 / pydantic / uv_build wheels may not yet exist on 3.14, causing the matrix entry to fail. [`.github/workflows/ci.yml:30-33`] — deferred, wait-and-see +- [x] [Review][Defer] `[tool.ruff.lint] select = ["ALL"]` paired with unpinned `ruff`/`ty` — any new ruff release adds rules and breaks CI overnight. [`pyproject.toml:70-72, 84-85`] — deferred, matches modern-di +- [x] [Review][Defer] No `[test]` extra declared; CI relies on `--all-extras`, so any future heavy extra is pulled into every CI run. [`pyproject.toml:35-47`] — deferred, scope creep concern diff --git a/docs/stories/1-2-core-data-types.md b/docs/stories/1-2-core-data-types.md index d082e1a..cfa6839 100644 --- a/docs/stories/1-2-core-data-types.md +++ b/docs/stories/1-2-core-data-types.md @@ -3,7 +3,7 @@ story_key: 1-2-core-data-types epic: 1 story: 2 title: Core data types -status: ready-for-dev +status: done created: 2026-05-13 input_documents: - docs/prd.md @@ -197,7 +197,7 @@ Followed TDD per module: write failing tests → confirm collection failure (RED - **AC5** ✓ `Timeout`, `Limits`, `ClientConfig` in `src/httpware/config.py` are all `@dataclass(frozen=True, slots=True)`. Defaults exactly match the spec. `ClientConfig.timeout` and `.limits` use `field(default_factory=Timeout)` / `field(default_factory=Limits)` — never bare instances. `Redactor` is intentionally not present (Story 5.3). - **AC6** ✓ `src/httpware/__init__.py` re-exports all five via explicit absolute imports; `__all__` is the sorted list `["ClientConfig", "Limits", "Request", "Response", "Timeout"]`. Smoke import succeeds. - **AC7** ✓ `just lint-ci` reports zero diagnostics across `eof-fixer`, `ruff format --check`, `ruff check --no-fix`, `ty check`. -- **AC8** ✓ 24 tests cover every documented case: frozen on all 4 dataclasses, `with_*` immutability + new-instance identity + replace-semantics on `with_header`, equality, independent default mappings, charset case-insensitivity (parametrized over three casings), UTF-8 / latin-1 / missing-charset paths, JSON round-trip, default constructor returns documented defaults. `pytest` exits 0; coverage 100%. +- **AC8** ✓ 24 tests cover every documented case: frozen on all 5 dataclasses, `with_*` immutability + new-instance identity + replace-semantics on `with_header`, equality, independent default mappings, charset case-insensitivity (parametrized over three casings), UTF-8 / latin-1 / missing-charset paths, JSON round-trip, default constructor returns documented defaults. `pytest` exits 0; coverage 100%. **Deviations from the story spec (worth flagging for reviewer):** @@ -229,4 +229,26 @@ Followed TDD per module: write failing tests → confirm collection failure (RED ## Status -`review` +`done` + +### Review Findings + +_Code review run: 2026-05-13. Reviewers: Blind Hunter, Edge Case Hunter, Acceptance Auditor. Acceptance Auditor reported **no AC violations**. 5 patches applied, 11 deferred, 36 dismissed as noise. Post-patch verification: `just lint-ci` clean, 27 tests pass (+3 new), 100% coverage retained._ + +- [x] [Review][Patch] `Response.text` now wraps `bytes.decode` in `try/except LookupError` and falls back to UTF-8 when the declared charset is unknown. [`src/httpware/response.py:42-46`] — applied +- [x] [Review][Patch] `Response` class docstring now documents `elapsed` as wall-clock seconds from request send to response receipt. [`src/httpware/response.py:29-32`] — applied +- [x] [Review][Patch] Added `test_response_text_strips_quotes_around_charset` (parametrized over `"latin-1"` and `'latin-1'`) covering the quote-stripping branch. [`tests/test_response.py`] — applied +- [x] [Review][Patch] Equality tests now also assert `!=` against per-field variants (Request and Response). [`tests/test_request.py`, `tests/test_response.py`] — applied +- [x] [Review][Patch] Completion Notes AC8 — "frozen on all 4 dataclasses" → "all 5". [`docs/stories/1-2-core-data-types.md:200`] — applied + +- [x] [Review][Defer] Charset parser robustness — whitespace inside quotes, mismatched quotes, multiple `charset=` directives, `charset=` substring inside a quoted boundary param. [`src/httpware/response.py:21-26`] — deferred, pragmatic v0; revisit when transport tests reveal real-world breakage +- [x] [Review][Defer] Header name/value validation (CR/LF injection, `None`, empty string) on `Request.with_header`. [`src/httpware/request.py:21-23`] — deferred to header-handling story (2.3 or later) +- [x] [Review][Defer] URL validation — `with_url("")` accepts empty, `base_url` has no trailing-slash normalization. [`src/httpware/request.py:25-27`, `src/httpware/config.py:27-33`] — deferred, lands when transport composes URLs +- [x] [Review][Defer] `with_query(None)` is currently accepted and breaks downstream iteration. [`src/httpware/request.py:33-35`] — deferred, type system already says `Mapping[str, str]` +- [x] [Review][Defer] `Timeout` / `Limits` accept negative or zero values silently (no `__post_init__`). [`src/httpware/config.py:10-22`] — deferred, validation lands with transport integration +- [x] [Review][Defer] `params: Mapping[str, str]` cannot express multi-valued query strings (`?tag=a&tag=b`). [`src/httpware/request.py:8`] — deferred, type widening tracked separately +- [x] [Review][Defer] `body: bytes | None` precludes streaming / async-iterable bodies. [`src/httpware/request.py:11`] — deferred, intentional minimal scope; revisit in transport stories +- [x] [Review][Defer] No `with_headers` / `with_cookie` / `with_extension` merge helpers; only `with_header` (single) and `with_query` (replace). [`src/httpware/request.py:20-35`] — deferred to Story 2.3 (merge/case-insensitive helpers) +- [x] [Review][Defer] `Response.json()` ignores declared charset; `json.loads(bytes)` auto-detects only UTF-8/16/32 by BOM. [`src/httpware/response.py:44-45`] — deferred, most APIs are UTF-8; revisit if a real backend breaks +- [x] [Review][Defer] `Request.body` / `Response.content` (bytes) render verbatim in the default dataclass `__repr__`, leaking large payloads / secrets into logs. [`src/httpware/request.py`, `src/httpware/response.py`] — deferred to Story 5.3 (`Redactor`) +- [x] [Review][Defer] No `@final` decorator — frozen+slots subclassing is fragile and the spec doesn't bless it; an explicit `@final` would prevent the footgun. [`src/httpware/request.py`, `src/httpware/response.py`, `src/httpware/config.py`] — deferred, no current subclasser diff --git a/pyproject.toml b/pyproject.toml index df08b48..162bb0d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,12 +39,7 @@ otel = [ "opentelemetry-sdk>=1.20", ] niquests = ["niquests>=3.18"] -all = [ - "msgspec>=0.18", - "opentelemetry-api>=1.20", - "opentelemetry-sdk>=1.20", - "niquests>=3.18", -] +all = ["httpware[msgspec,otel,niquests]"] [project.urls] repository = "https://github.com/modern-python/httpware" diff --git a/src/httpware/response.py b/src/httpware/response.py index 9e34483..637f774 100644 --- a/src/httpware/response.py +++ b/src/httpware/response.py @@ -26,7 +26,10 @@ def _parse_charset(content_type: str) -> str | None: @dataclass(frozen=True, slots=True) class Response: - """Immutable HTTP response value type.""" + """Immutable HTTP response value type. + + `elapsed` is wall-clock seconds from request send to response receipt. + """ status: int headers: Mapping[str, str] @@ -38,7 +41,10 @@ class Response: def text(self) -> str: """Decode `content` using the response's declared charset (default UTF-8).""" charset = _parse_charset(_get_content_type(self.headers)) or "utf-8" - return self.content.decode(charset) + try: + return self.content.decode(charset) + except LookupError: + return self.content.decode("utf-8") def json(self) -> Any: # noqa: ANN401 """Parse `content` as JSON.""" diff --git a/tests/test_request.py b/tests/test_request.py index 096bcc3..1af85d4 100644 --- a/tests/test_request.py +++ b/tests/test_request.py @@ -28,6 +28,9 @@ def test_request_equality_on_identical_fields() -> None: r1 = Request(method="GET", url="/x", headers={"a": "1"}) r2 = Request(method="GET", url="/x", headers={"a": "1"}) assert r1 == r2 + assert r1 != Request(method="POST", url="/x", headers={"a": "1"}) + assert r1 != Request(method="GET", url="/y", headers={"a": "1"}) + assert r1 != Request(method="GET", url="/x", headers={"a": "2"}) def test_with_header_adds_when_absent() -> None: diff --git a/tests/test_response.py b/tests/test_response.py index 5df460e..fa5eb17 100644 --- a/tests/test_response.py +++ b/tests/test_response.py @@ -48,6 +48,36 @@ def test_response_text_falls_back_to_utf8_on_missing_charset() -> None: assert resp.text == '{"x": 1}' +@pytest.mark.parametrize( + "content_type", + [ + 'text/plain; charset="latin-1"', + "text/plain; charset='latin-1'", + ], +) +def test_response_text_strips_quotes_around_charset(content_type: str) -> None: + body = "café".encode("latin-1") + resp = Response( + status=200, + headers={"content-type": content_type}, + content=body, + url="/", + elapsed=0.0, + ) + assert resp.text == "café" + + +def test_response_text_falls_back_to_utf8_on_unknown_charset() -> None: + resp = Response( + status=200, + headers={"content-type": "text/plain; charset=not-a-real-codec"}, + content=b"hello", + url="/", + elapsed=0.0, + ) + assert resp.text == "hello" + + def test_response_json_parses_body() -> None: resp = Response(status=200, headers={}, content=b'{"a": 1, "b": [2, 3]}', url="/", elapsed=0.0) assert resp.json() == {"a": 1, "b": [2, 3]} @@ -57,3 +87,5 @@ 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) assert r1 == r2 + assert r1 != Response(status=200, headers={"a": "1"}, content=b"x", url="/", elapsed=0.6) + assert r1 != Response(status=201, headers={"a": "1"}, content=b"x", url="/", elapsed=0.5)