From 41b1ff255df37a68adc13c561c86c68abbcb336d Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 20 May 2026 22:45:30 +0200 Subject: [PATCH 1/6] Add pagination error surface ExecPlan Draft the pre-implementation plan for backend roadmap item 4.2.2. The plan captures the repository-layer pagination error gap, the hexagonal boundary constraints, validation strategy, and approval gate before any implementation work proceeds. --- ...d-4-2-2-surface-pagination-aware-errors.md | 602 ++++++++++++++++++ 1 file changed, 602 insertions(+) create mode 100644 docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md diff --git a/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md b/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md new file mode 100644 index 00000000..8991523c --- /dev/null +++ b/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md @@ -0,0 +1,602 @@ +# Surface pagination-aware repository errors + +This ExecPlan (execution plan) is a living document. The sections +`Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, +`Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work +proceeds. + +Status: DRAFT; AWAITING APPROVAL + +## Purpose / big picture + +Roadmap task 4.2.2 requires the backend to surface pagination-aware repository +errors, such as malformed cursor tokens and unsupported pagination direction, +as HTTP `400 Bad Request` responses instead of treating them as generic +persistence failures. After this work, a caller of `GET /api/v1/users` can pass +an invalid pagination cursor and receive the existing Wildside error envelope +with a stable invalid-request code, a `details.field` value of `cursor`, and a +machine-readable pagination detail code. The same HTTP behaviour must be +preserved when a repository or query adapter returns a semantic pagination +failure from below the HTTP parser. + +This plan is pre-implementation only. Do not implement it until the user +explicitly approves this ExecPlan. The implementation will be considered +observable when: + +1. `GET /api/v1/users?cursor=not-a-cursor` returns HTTP `400` with an + invalid cursor error envelope. +2. A cursor whose decoded direction is unsupported also returns HTTP `400` + with a pagination-specific detail code. +3. Repository-originated pagination failures map to HTTP `400`, while + repository connection failures remain `503` and ordinary database query + failures remain redacted `500` responses. +4. `make check-fmt`, `make lint`, and `make test` all pass from the repository + root, with command output captured under `/tmp`. + +The key implementation challenge is that the current users HTTP adapter +already decodes cursors before calling the `UsersQuery` port. Malformed raw +cursor text normally fails at the inbound adapter instead of inside the Diesel +repository. The approved implementation must keep the fast HTTP validation +path and also add repository-facing semantic pagination errors, so future +adapters and tests cannot collapse those client failures into generic internal +errors. + +## Constraints + +- This plan must remain a draft until explicitly approved. No code + implementation is authorized by opening the plan pull request. +- Preserve the hexagonal dependency rule from the + `hexagonal-architecture` skill and `docs/wildside-backend-architecture.md`: + domain ports define semantic errors, outbound adapters implement ports, and + inbound HTTP adapters map domain errors to HTTP responses. +- `backend/src/inbound/http/users.rs::list_users` must remain a thin + coordinator. It may parse query parameters, call `state.users`, and build the + response envelope, but it must not import Diesel, bb8, or outbound modules. +- Persistence-specific Diesel logic must stay in + `backend/src/outbound/persistence/diesel_user_repository.rs` and related + outbound modules. +- The cursor transport format remains opaque base64url JSON provided by + `backend/crates/pagination`. Do not introduce a second cursor format. +- Keep the current `/api/v1/users` response envelope from roadmap 4.2.1: + `{ "data": [...], "limit": N, "links": { "self": ..., "next": ..., "prev": + ... } }`. +- Preserve current non-pagination error semantics: unauthenticated requests + return `401`, repository connection failures return `503`, and unexpected + query failures return `500` with the internal message redacted. +- No new external runtime dependency is expected. If a new crate becomes + necessary, stop and ask for approval. +- New or updated documentation must use en-GB Oxford spelling and grammar. +- Each implementation milestone must be committed separately after its relevant + gates pass. Do not commit a failing gate. +- Use `tee` for all gate commands, so full output is available despite terminal + truncation. + +## Tolerances + +- Scope: stop and escalate if the implementation touches more than 18 files or + exceeds roughly 700 net source lines excluding generated OpenAPI output, + Gherkin feature text, and this ExecPlan. +- Public API: stop and escalate before changing the public JSON shape of + successful `GET /api/v1/users` responses or changing the top-level Wildside + error envelope fields. +- Cursor compatibility: stop and escalate if the cursor crate change would make + cursors generated by roadmap 4.1.2 or 4.2.1 fail to decode. +- Dependencies: stop and escalate before adding a new crate, build tool, or + service. +- Testing: if `make test` still fails after three focused attempts, document + the failure in `Surprises & Discoveries` and ask for direction. +- Architecture: stop and escalate if satisfying the task seems to require an + inbound adapter importing `crate::outbound::*` or a domain port importing + Actix types. +- Ambiguity: stop and present options if "repository layer" must be interpreted + as moving all raw cursor decoding into Diesel repositories. That would fight + the current clean split where HTTP parsing produces a decoded + `ListUsersPageRequest`. +- CodeRabbit: if `coderabbit review --agent` reports blocking concerns that + cannot be cleared with a small follow-up commit, stop and ask for direction. + +## Risks + +- Risk: The current HTTP adapter already handles malformed cursors before the + repository is called, so a literal implementation that moves raw cursor + parsing down into Diesel would weaken the boundary. + Severity: high. Likelihood: medium. + Mitigation: model pagination failures as domain port errors and map them to + HTTP `400`; keep inbound raw-query parsing at the HTTP edge. + +- Risk: `pagination::Direction` is currently a closed Rust enum with `Next` and + `Prev`. Unsupported direction appears as a cursor deserialization failure + unless the pagination crate preserves that distinction. + Severity: medium. Likelihood: high. + Mitigation: add explicit cursor-error classification in the pagination crate + if needed, or document that unsupported raw direction maps to + `invalid_cursor` while repository-declared unsupported direction maps to + `unsupported_direction`. + +- Risk: Adding variants to `UserPersistenceError` can break exhaustive matches + in query adapters and tests. + Severity: medium. Likelihood: high. + Mitigation: update `user_persistence_error_mapping.rs`, the Diesel users + query tests, repository tests, and any test doubles in the same milestone. + +- Risk: The implementation could leak database details to clients if a Diesel + error is misclassified as a pagination client error. + Severity: high. Likelihood: low. + Mitigation: only construct pagination-aware variants from explicit + pagination validation paths; keep Diesel `DatabaseError` and + `QueryBuilderError` mapped as they are today. + +- Risk: `docs/users-guide.md` does not currently exist even though the request + asks for it to be updated. + Severity: low. Likelihood: certain. + Mitigation: create the guide only if the implementation introduces + user-visible server behaviour not already documented elsewhere; otherwise + record the absence and explain why a new guide is or is not created. + +- Risk: Embedded PostgreSQL behavioural tests can be slow or transiently fail + in shared environments. + Severity: low. Likelihood: medium. + Mitigation: reuse existing `backend/tests/support/embedded_postgres.rs` + helpers and rerun once before treating setup failures as implementation + failures. + +## Progress + +- [x] 2026-05-20: Loaded `leta`, `execplans`, `hexagonal-architecture`, + `firecrawl`, `commit-message`, `pr-creation`, and + `en-gb-oxendict-style` guidance relevant to this planning task. +- [x] 2026-05-20: Renamed the local branch from `feat/plan-pagination-errors` + to `backend-4-2-2-surface-pagination-aware-errors`. +- [x] 2026-05-20: Reviewed `docs/backend-roadmap.md`, + `docs/keyset-pagination-design.md`, + `docs/wildside-backend-architecture.md`, + `docs/developers-guide.md`, and the prior 4.2.1 ExecPlan. +- [x] 2026-05-20: Created and finalized context pack + `pk_urjsjl5b` for the Wyvern agent team with roadmap, design, architecture, + port, HTTP, and Diesel references. +- [x] 2026-05-20: Used Firecrawl to check prior art. The JSON:API cursor + pagination profile requires HTTP `400` for invalid cursor query parameters, + and RFC 9457 supports machine-readable error details. Wildside will keep its + existing error envelope rather than adopting a new media type. +- [x] 2026-05-20: Received Wyvern planning notes. The main finding is that + external malformed cursors already return `400`, but repository-facing errors + cannot yet distinguish pagination client failures from generic query + failures. +- [ ] M0: Approval received and baseline established. +- [ ] Implementation milestone M1 completed and committed. +- [ ] Implementation milestone M2 completed and committed. +- [ ] Implementation milestone M3 completed and committed. +- [ ] Implementation milestone M4 completed and committed. +- [ ] Implementation milestone M5 completed and committed. +- [ ] M6: Validation and closure completed. +- [ ] Roadmap item 4.2.2 marked done after all gates pass. +- [ ] Draft implementation pull request opened or updated after approval work + lands. + +## Surprises & Discoveries + +- Observation: `docs/users-guide.md` is absent from the repository. + Evidence: `find docs -iname '*user*guide*.md'` lists tool-specific guides + such as `docs/rstest-bdd-users-guide.md` and + `docs/pg-embed-setup-unpriv-users-guide.md`, but not a general + `docs/users-guide.md`. + Impact: The implementation must either create this document for user-visible + server behaviour or record why no user-facing guide update is appropriate. + +- Observation: The current inbound users pagination helper already maps raw + cursor decode failures to `Error::invalid_request("cursor is invalid")`. + Evidence: `backend/src/inbound/http/users_pagination.rs` decodes + `Cursor` in `parse_users_page_params` and calls + `invalid_cursor_error()` on failure. + Impact: The plan must not move HTTP parsing into Diesel just to make invalid + raw cursors "repository layer" errors. + +- Observation: The pagination crate documentation already recommends HTTP + `400` for cursor and page parameter client errors. + Evidence: `backend/crates/pagination/src/lib.rs` documents `CursorError` + and `PageParamsError` mapping guidelines. + Impact: The implementation should align adapter mappings with the existing + crate contract rather than inventing a new policy. + +## Decision Log + +- Decision: Treat "repository layer" as the domain port/query/repository error + surface, not as a requirement that raw HTTP cursor strings be parsed inside + Diesel repositories. + Rationale: Hexagonal architecture keeps transport parsing at the inbound edge + and persistence querying in outbound adapters. The missing piece is a + semantic error variant that can travel from repository/query adapters to HTTP + as `400`. + Date/Author: 2026-05-20 / Codex. + +- Decision: Keep Wildside's existing JSON error envelope instead of adopting + RFC 9457 `application/problem+json`. + Rationale: RFC 9457 is useful prior art for machine-readable details, but the + repository already has a stable `Error` type, `ErrorCode`, `details`, and + Actix `ResponseError` mapping. Introducing a second public error format is + out of scope. + Date/Author: 2026-05-20 / Codex. + +- Decision: Use focused `rstest` unit tests and `rstest-bdd` behavioural tests; + do not introduce Kani or Verus for this item. + Rationale: The change is an error-classification and mapping change, not a + new unbounded ordering invariant or proof obligation. Cursor round-trip and + direction properties were covered by roadmap 4.1.2. + Date/Author: 2026-05-20 / Codex. + +- Decision: The plan pull request is a draft, pre-implementation review PR. + Rationale: The user explicitly required plan approval before implementation. + Date/Author: 2026-05-20 / Codex. + +## Outcomes & Retrospective + +No implementation has been performed yet. The planning phase identified that +the externally visible `400` path mostly exists for malformed users cursors, +but the repository-facing error taxonomy is too coarse. The future +implementation should therefore be small and careful: add semantic pagination +errors to the port/query mapping, prove the HTTP response remains stable, and +document the contract. + +## Context and orientation + +The repository is a Rust backend and TypeScript frontend monorepo. The backend +uses a hexagonal modular monolith: domain modules and ports live under +`backend/src/domain`, inbound HTTP adapters live under +`backend/src/inbound/http`, and outbound persistence adapters live under +`backend/src/outbound/persistence`. + +Pagination is implemented by a shared workspace crate at +`backend/crates/pagination`. The crate provides: + +- `Cursor`: an opaque base64url JSON token containing a key and a + `Direction`. +- `Direction`: currently `Next` or `Prev`. +- `PageParams`: normalized `cursor` and `limit` query parameters. +- `Paginated` and `PaginationLinks`: the response envelope and navigation + links. + +The users endpoint is the first endpoint that adopted the shared pagination +crate. Current relevant files are: + +- `backend/src/domain/ports/user_repository.rs`: defines + `ListUsersPageRequest`, the `UserRepository` trait, and + `UserPersistenceError`. +- `backend/src/domain/ports/users_query.rs`: defines the driving `UsersQuery` + port and `FixtureUsersQuery` test implementation. +- `backend/src/inbound/http/users.rs`: defines the `GET /api/v1/users` + handler. +- `backend/src/inbound/http/users_pagination.rs`: parses raw HTTP pagination + query parameters, decodes users cursors, and builds `Paginated` + responses. +- `backend/src/outbound/persistence/diesel_user_repository.rs`: implements + `UserRepository::list_page` using `(created_at, id)` keyset predicates. +- `backend/src/outbound/persistence/diesel_users_query.rs`: adapts + `UserRepository::list_page` into `UsersQuery::list_users_page` and trims the + overflow row. +- `backend/src/outbound/persistence/user_persistence_error_mapping.rs`: maps + `UserPersistenceError` into the HTTP-safe domain `Error`. +- `backend/tests/features/users_list_pagination.feature` and + `backend/tests/users_list_pagination_bdd.rs`: cover externally observable + users pagination behaviour. + +Current `UserPersistenceError` has only `Connection` and `Query` variants. +`Connection` maps to service unavailable. `Query` maps to an internal error. +That is correct for database failures, but wrong for semantic pagination +failures that are caused by client input. Roadmap item 4.2.2 closes that gap. + +## Plan of work + +M0 is approval and baseline capture. Re-read this plan, confirm the branch is +`backend-4-2-2-surface-pagination-aware-errors`, and do not proceed until the +user approves. Run a baseline status check and record the result in +`Progress`. If the worktree contains unrelated changes, leave them untouched +and record them. + +M1 adds the domain-level pagination error vocabulary. In +`backend/src/domain/ports/user_repository.rs`, add a semantic error surface for +pagination failures. The preferred shape is either a small +`UserPaginationError` enum nested beside the port, or explicit +`UserPersistenceError` variants such as `InvalidCursorFormat` and +`UnsupportedDirection`. The exact shape should make callers able to distinguish +client pagination failures from connection and database query failures without +using string matching. Add Rustdoc examples for any public constructor. + +M1 must also update test doubles that construct `UserPersistenceError`, +including stubs in `backend/src/outbound/persistence/diesel_login_service.rs`, +`backend/src/outbound/persistence/diesel_users_query.rs`, and any adapter +guardrail doubles if the compiler requires it. End M1 with focused unit tests +for the new constructors and with `make check-fmt` if only source formatting is +affected. + +M2 updates error mapping. In +`backend/src/outbound/persistence/user_persistence_error_mapping.rs`, map +pagination-aware user repository errors to `Error::invalid_request(...)` with +details similar to: + +```json +{ + "field": "cursor", + "code": "invalid_cursor" +} +``` + +For unsupported directions, prefer: + +```json +{ + "field": "cursor", + "code": "unsupported_direction" +} +``` + +Keep connection failures mapped to `Error::service_unavailable` and ordinary +query failures mapped to `Error::internal`. Add or extend `rstest` unit tests +in `backend/src/outbound/persistence/diesel_users_query.rs` or a dedicated +mapping test module so a stub `UserRepository` returning the new pagination +error yields `ErrorCode::InvalidRequest`. + +M3 tightens inbound cursor error classification. In +`backend/src/inbound/http/users_pagination.rs`, keep +`parse_users_page_params` responsible for raw query parsing. If the pagination +crate can distinguish `CursorError::UnsupportedDirection`, map that case to +the `unsupported_direction` detail code. If the crate cannot distinguish it +without brittle string matching, add focused support in +`backend/crates/pagination/src/cursor.rs` so invalid base64, invalid JSON, and +unsupported `dir` values are separately testable. Do not change successful +cursor encoding. Add `rstest` cases for malformed base64, structurally invalid +JSON, legacy cursors without `dir`, supported `Next` and `Prev`, and an +unsupported direction string. + +M4 adds externally observable behavioural coverage. Extend +`backend/tests/features/users_list_pagination.feature` and +`backend/tests/users_list_pagination_bdd.rs` with unhappy-path scenarios for +malformed cursor and unsupported direction. The unsupported-direction scenario +can construct a base64url JSON cursor containing the valid users key shape and +an unsupported `dir` value, then request `GET /api/v1/users?cursor=`. +Assert HTTP `400` and the stable error detail code. Keep existing happy path +traversal scenarios unchanged. + +M5 updates documentation. Update `docs/keyset-pagination-design.md` to record +that pagination decode and direction failures are semantic client errors and +must be surfaced as HTTP `400` through adapter mapping. Update +`docs/wildside-backend-architecture.md` near the shared pagination foundation +section to state that repository/query adapters may return pagination-aware +port errors, and HTTP adapters map them to `400` without leaking persistence +details. Update `docs/developers-guide.md` if a new convention is introduced +for pagination error mapping or BDD cursor fixtures. If user-visible behaviour +has changed and no better user-facing document exists, create +`docs/users-guide.md` with a concise API behaviour section for pagination +errors; otherwise record why no new guide was created. + +M6 validates, reviews, and closes. Run the focused tests first, then the full +gates. Run `coderabbit review --agent` after the main implementation milestone, +and again after documentation/roadmap closure if it reports concerns. Clear +all actionable concerns before moving on. Mark `docs/backend-roadmap.md` item +4.2.2 as done only after the implementation, tests, docs, CodeRabbit review, +and full gates are green. Commit each milestone separately. + +## Concrete steps + +All commands run from the repository root. Confirm the working directory before +starting: + +```bash +pwd +``` + +Before implementation approval, only planning commands are allowed: + +```bash +git branch --show-current +git status --short +``` + +Expected branch: + +```plaintext +backend-4-2-2-surface-pagination-aware-errors +``` + +For M1 and M2 focused checks, use targeted Rust tests before the full gates: + +```bash +set -o pipefail +branch=backend-4-2-2-surface-pagination-aware-errors +cargo test -p backend user_persistence_error -- --nocapture \ + | tee "/tmp/unit-user-persistence-error-${branch}.out" +cargo test -p backend diesel_users_query -- --nocapture \ + | tee "/tmp/unit-diesel-users-query-${branch}.out" +``` + +If M3 changes the pagination crate, run: + +```bash +set -o pipefail +branch=backend-4-2-2-surface-pagination-aware-errors +cargo test -p pagination cursor -- --nocapture \ + | tee "/tmp/unit-pagination-cursor-${branch}.out" +cargo test -p pagination -- --nocapture \ + | tee "/tmp/test-pagination-${branch}.out" +``` + +For M4 BDD coverage, run the focused BDD test binary: + +```bash +set -o pipefail +branch=backend-4-2-2-surface-pagination-aware-errors +cargo test -p backend --test users_list_pagination_bdd -- --nocapture \ + | tee "/tmp/bdd-users-list-pagination-${branch}.out" +``` + +After each major milestone, run CodeRabbit and address concerns before +continuing: + +```bash +coderabbit review --agent +``` + +For commit gates, run the full commands sequentially: + +```bash +set -o pipefail +make check-fmt \ + | tee /tmp/check-fmt-backend-4-2-2-surface-pagination-aware-errors.out +make lint \ + | tee /tmp/lint-backend-4-2-2-surface-pagination-aware-errors.out +make test \ + | tee /tmp/test-backend-4-2-2-surface-pagination-aware-errors.out +``` + +For Markdown-only plan or documentation edits, run: + +```bash +set -o pipefail +make fmt \ + | tee /tmp/fmt-backend-4-2-2-surface-pagination-aware-errors.out +make markdownlint \ + | tee /tmp/markdownlint-backend-4-2-2-surface-pagination-aware-errors.out +``` + +When committing, follow the `commit-message` skill: inspect the diff, write the +commit message to a file in `mktemp -d`, and commit with `git commit -F`. +Never use `git commit -m`. + +When the implementation is complete and approved for review, push: + +```bash +git push -u origin backend-4-2-2-surface-pagination-aware-errors +``` + +Then create or update a draft pull request using the `pr-creation` skill. The +title must include `(backend-4.2.2)`, and the description must link this +ExecPlan and include a `## References` section with this Lody session link: + +```plaintext +https://lody.ai/leynos/sessions/0568ecd7-a8ee-4b84-b013-e806f14fd7bf +``` + +## Validation and acceptance + +This change is accepted when all the following are true: + +- Unit tests demonstrate the new pagination-aware repository error variants and + their mapping to `ErrorCode::InvalidRequest`. +- Behavioural tests demonstrate that malformed cursors and unsupported cursor + directions on `GET /api/v1/users` return HTTP `400` with stable error + details. +- Existing users pagination happy paths still pass, including first page, + forward traversal, reverse traversal, final page, and unauthenticated access. +- Documentation explains the error mapping convention in the design and + architecture documents, and user-visible behaviour is documented or the + absence of a general user guide is explicitly addressed. +- `coderabbit review --agent` has no unresolved actionable concerns. +- `make check-fmt`, `make lint`, and `make test` pass in sequence. +- `docs/backend-roadmap.md` marks item 4.2.2 done only after all preceding + criteria are satisfied. + +Expected HTTP response shape for a malformed users cursor: + +```json +{ + "code": "invalid_request", + "message": "cursor is invalid", + "details": { + "field": "cursor", + "code": "invalid_cursor" + } +} +``` + +Expected HTTP response shape for a repository-originated unsupported direction: + +```json +{ + "code": "invalid_request", + "message": "cursor direction is unsupported", + "details": { + "field": "cursor", + "code": "unsupported_direction" + } +} +``` + +Exact `traceId` handling follows the existing domain `Error` type and may be +present when a trace identifier is in scope. + +## Idempotence and recovery + +Most implementation steps are additive and can be retried safely. If a focused +test fails, fix the smallest relevant unit and rerun that focused test before +rerunning full gates. If a full gate fails because of a transient embedded +PostgreSQL setup problem, inspect the `/tmp` log, verify no unrelated service +was killed, and rerun once before treating it as a code failure. + +Do not use destructive git commands such as `git reset --hard` or +`git checkout --`. If an implementation milestone needs rollback, use normal +revert commits or ask for direction. Leave unrelated worktree changes intact. + +## Artifacts and notes + +Firecrawl prior art used during planning: + +- JSON:API cursor pagination profile: + `https://jsonapi.org/profiles/ethanresnick/cursor-pagination/`. Relevant + point: invalid cursor and invalid pagination query parameters are modelled as + HTTP `400` client errors that identify the problematic parameter. +- RFC 9457: + `https://www.rfc-editor.org/rfc/rfc9457.html`. Relevant point: + machine-readable error details are useful for HTTP APIs, but Wildside already + has a stable JSON error envelope, so this work should enrich `details` + instead of changing media type or top-level shape. + +Wyvern agent findings used during planning: + +- Current malformed cursor behaviour already returns HTTP `400` at the inbound + users pagination parser. +- Current `UserPersistenceError` cannot represent pagination-specific client + failures, so repository-originated cursor failures would currently map as + generic internal errors. +- `docs/users-guide.md` is missing, and the implementation should address that + explicitly. + +## Interfaces and dependencies + +At the end of M1/M2, the domain port layer must expose a semantic way to +describe pagination client failures. One acceptable shape is: + +```rust +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum UserPaginationError { + InvalidCursorFormat { message: String }, + UnsupportedDirection { direction: String }, +} +``` + +and either: + +```rust +pub enum UserPersistenceError { + Connection { message: String }, + Query { message: String }, + Pagination { error: UserPaginationError }, +} +``` + +or equivalent explicit variants. The exact implementation may differ, but it +must satisfy these constraints: + +- callers can match pagination client failures without parsing messages; +- connection and database query failures remain distinguishable; +- `map_user_persistence_error` can map pagination failures to + `Error::invalid_request(...)`; +- details include a stable `field: "cursor"` and a stable detail code; +- no Actix, Diesel, or HTTP status types appear in the domain port error. + +If M3 changes the pagination crate, the public `CursorError` type should remain +backward-compatible for successful cursor tokens and should expose enough +information for adapters to classify unsupported direction without brittle +message matching. + +Revision note: initial draft created on 2026-05-20 from roadmap item 4.2.2, +repository inspection, Wyvern planning notes, and Firecrawl prior-art review. +The plan is not approved for implementation. From c9b82d763d394cf93ff3c09bc2db7048c5ae576b Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 26 May 2026 12:33:12 +0200 Subject: [PATCH 2/6] Record pagination plan implementation start Mark the ExecPlan as in progress after explicit implementation approval. Capture the clean branch baseline so later milestones can distinguish planned work from unrelated local state. --- ...kend-4-2-2-surface-pagination-aware-errors.md | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md b/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md index 8991523c..b25f5d53 100644 --- a/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md +++ b/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md @@ -5,7 +5,7 @@ This ExecPlan (execution plan) is a living document. The sections `Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work proceeds. -Status: DRAFT; AWAITING APPROVAL +Status: IN PROGRESS ## Purpose / big picture @@ -162,7 +162,10 @@ errors. external malformed cursors already return `400`, but repository-facing errors cannot yet distinguish pagination client failures from generic query failures. -- [ ] M0: Approval received and baseline established. +- [x] 2026-05-26: Implementation approved by the user. Baseline confirmed on + branch `backend-4-2-2-surface-pagination-aware-errors` with a clean + worktree before code changes. +- [x] M0: Approval received and baseline established. - [ ] Implementation milestone M1 completed and committed. - [ ] Implementation milestone M2 completed and committed. - [ ] Implementation milestone M3 completed and committed. @@ -228,6 +231,13 @@ errors. Rationale: The user explicitly required plan approval before implementation. Date/Author: 2026-05-20 / Codex. +- Decision: Start implementation on the existing plan branch and keep the + planning pull request as the review vehicle. + Rationale: The user explicitly approved implementation on 2026-05-26, and + the branch already tracks + `origin/backend-4-2-2-surface-pagination-aware-errors`. + Date/Author: 2026-05-26 / Codex. + ## Outcomes & Retrospective No implementation has been performed yet. The planning phase identified that @@ -599,4 +609,4 @@ message matching. Revision note: initial draft created on 2026-05-20 from roadmap item 4.2.2, repository inspection, Wyvern planning notes, and Firecrawl prior-art review. -The plan is not approved for implementation. +Implementation was approved on 2026-05-26. From 40a85ed5f1390ac2b1fa8a06d5fb2eec6bc0d0e7 Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 26 May 2026 13:01:00 +0200 Subject: [PATCH 3/6] Surface pagination-aware errors Add a user repository pagination error surface so invalid cursor and unsupported direction failures can travel through the port layer without being collapsed into generic persistence query failures. Classify unsupported cursor directions in the pagination crate, map both repository-originated and inbound cursor failures to HTTP 400 details, and cover the externally visible users-list behaviour with rstest-bdd. --- backend/crates/pagination/src/cursor.rs | 399 ------------------ backend/crates/pagination/src/cursor/mod.rs | 247 +++++++++++ backend/crates/pagination/src/cursor/tests.rs | 175 ++++++++ backend/crates/pagination/src/lib.rs | 1 + backend/src/domain/ports/mod.rs | 4 +- backend/src/domain/ports/user_repository.rs | 48 +++ backend/src/inbound/http/users_pagination.rs | 21 +- .../persistence/diesel_users_query.rs | 12 +- .../user_persistence_error_mapping.rs | 53 ++- .../features/users_list_pagination.feature | 5 + backend/tests/users_list_pagination_bdd.rs | 26 ++ ...d-4-2-2-surface-pagination-aware-errors.md | 81 +++- 12 files changed, 664 insertions(+), 408 deletions(-) delete mode 100644 backend/crates/pagination/src/cursor.rs create mode 100644 backend/crates/pagination/src/cursor/mod.rs create mode 100644 backend/crates/pagination/src/cursor/tests.rs diff --git a/backend/crates/pagination/src/cursor.rs b/backend/crates/pagination/src/cursor.rs deleted file mode 100644 index 6030801e..00000000 --- a/backend/crates/pagination/src/cursor.rs +++ /dev/null @@ -1,399 +0,0 @@ -//! Opaque cursor encoding and decoding helpers. -//! -//! This module provides the [`Cursor`] type for encoding pagination -//! positions as opaque base64url-encoded JSON tokens, and the [`Direction`] -//! enum for bidirectional navigation. Cursors are transport-neutral and can be -//! used with any HTTP framework or serialization format. -//! -//! The base64url JSON encoding format ensures cursors are URL-safe and do not -//! require additional escaping in query parameters. The `dir` field is optional -//! in the JSON representation for backward compatibility with clients that omit -//! it (the default direction is `Next`). -//! -//! **Security consideration**: cursors are opaque but not signed or encrypted. -//! They encode the ordering key only, not any access control information. -//! Consumers must validate that the requesting user has permission to access -//! the underlying data. - -use base64::{ - Engine as _, - engine::general_purpose::{URL_SAFE, URL_SAFE_NO_PAD}, -}; -use serde::{Deserialize, Serialize, de::DeserializeOwned}; -use thiserror::Error; - -/// Direction of pagination relative to the cursor. -/// -/// Indicates whether the cursor represents a position for fetching -/// the next page (forward in sort order) or the previous page -/// (backward in sort order). -/// -/// # Examples -/// -/// ``` -/// use pagination::Direction; -/// -/// let forward = Direction::Next; -/// let backward = Direction::Prev; -/// -/// assert_ne!(forward, backward); -/// ``` -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] -pub enum Direction { - /// Forward in the sort order (e.g., newer items if sorting ascending). - #[default] - Next, - /// Backward in the sort order (e.g., older items). - Prev, -} - -/// Cursor wrapper for an ordered boundary key with direction. -/// -/// The encoded representation is base64url JSON and must be treated as opaque -/// by clients. The direction indicates whether this cursor is meant for -/// fetching the next page (forward) or previous page (backward). -/// -/// # Example -/// -/// ``` -/// use pagination::{Cursor, Direction}; -/// use serde::{Deserialize, Serialize}; -/// -/// #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -/// struct UserKey { -/// created_at: String, -/// id: String, -/// } -/// -/// let cursor = Cursor::new(UserKey { -/// created_at: "2026-03-22T10:30:00Z".to_owned(), -/// id: "8b116c56-0a58-4c55-b7d7-06ee6bbddb8c".to_owned(), -/// }); -/// let encoded = cursor.encode().expect("cursor encoding succeeds"); -/// let decoded = Cursor::::decode(&encoded).expect("cursor decoding succeeds"); -/// -/// assert_eq!(decoded.key(), cursor.key()); -/// assert_eq!(decoded.direction(), Direction::Next); -/// ``` -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct Cursor { - key: Key, - #[serde(default)] - dir: Direction, -} - -/// Errors raised while encoding or decoding opaque cursors. -#[derive(Debug, Error, Clone, PartialEq, Eq)] -pub enum CursorError { - /// The cursor key could not be serialized into JSON. - #[error("cursor JSON serialization failed: {message}")] - Serialize { - /// Human-readable serialization failure details. - message: String, - }, - /// The encoded token was not valid base64url. - #[error("cursor is not valid base64url: {message}")] - InvalidBase64 { - /// Human-readable base64 decoding failure details. - message: String, - }, - /// The decoded JSON payload did not match the expected key shape. - #[error("cursor JSON deserialization failed: {message}")] - Deserialize { - /// Human-readable deserialization failure details. - message: String, - }, -} - -impl Cursor { - /// Construct a cursor from one ordering key with the default direction (`Next`). - /// - /// # Examples - /// - /// ``` - /// use pagination::{Cursor, Direction}; - /// - /// let cursor = Cursor::new("my-key"); - /// assert_eq!(cursor.direction(), Direction::Next); - /// ``` - #[must_use] - pub const fn new(key: Key) -> Self { - Self { - key, - dir: Direction::Next, - } - } - - /// Construct a cursor from one ordering key with an explicit direction. - /// - /// # Examples - /// - /// ``` - /// use pagination::{Cursor, Direction}; - /// - /// let cursor = Cursor::with_direction("my-key", Direction::Prev); - /// assert_eq!(cursor.direction(), Direction::Prev); - /// ``` - #[must_use] - pub const fn with_direction(key: Key, dir: Direction) -> Self { - Self { key, dir } - } - - /// Borrow the cursor key. - #[must_use] - pub const fn key(&self) -> &Key { - &self.key - } - - /// Access the pagination direction. - #[must_use] - pub const fn direction(&self) -> Direction { - self.dir - } - - /// Consume the cursor and return the inner key. - #[must_use] - pub fn into_inner(self) -> Key { - self.key - } - - /// Decompose the cursor into its constituent parts (key and direction). - /// - /// # Examples - /// - /// ``` - /// use pagination::{Cursor, Direction}; - /// - /// let cursor = Cursor::with_direction("my-key", Direction::Prev); - /// let (key, dir) = cursor.into_parts(); - /// assert_eq!(key, "my-key"); - /// assert_eq!(dir, Direction::Prev); - /// ``` - #[must_use] - pub fn into_parts(self) -> (Key, Direction) { - (self.key, self.dir) - } -} - -impl Cursor -where - Key: Serialize, -{ - /// Encode the cursor to an opaque base64url JSON token. - /// - /// # Errors - /// - /// Returns [`CursorError::Serialize`] when the cursor key cannot be - /// serialized into JSON. - pub fn encode(&self) -> Result { - let payload = serde_json::to_vec(self).map_err(|error| CursorError::Serialize { - message: error.to_string(), - })?; - Ok(URL_SAFE_NO_PAD.encode(payload)) - } -} - -impl Cursor -where - Key: DeserializeOwned, -{ - /// Decode a cursor from an opaque base64url JSON token. - /// - /// # Errors - /// - /// Returns [`CursorError::InvalidBase64`] when `value` is not valid - /// base64url and [`CursorError::Deserialize`] when the decoded JSON does - /// not match the expected cursor shape. - pub fn decode(value: &str) -> Result { - let payload = URL_SAFE_NO_PAD - .decode(value) - .or_else(|_| URL_SAFE.decode(value)) - .map_err(|error| CursorError::InvalidBase64 { - message: error.to_string(), - })?; - serde_json::from_slice(&payload).map_err(|error| CursorError::Deserialize { - message: error.to_string(), - }) - } -} - -#[cfg(test)] -mod tests { - //! Unit tests for opaque cursor encoding and decoding. - - use base64::Engine as _; - use rstest::rstest; - use serde::{Deserialize, Serialize}; - - use super::{Cursor, CursorError, Direction}; - - #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] - struct FixtureKey { - created_at: String, - id: String, - } - - // Verify Cursor constructors work in const contexts - const _CONST_CURSOR: Cursor<&str> = Cursor::new("compile-time-test"); - const _CONST_DIRECTIONAL_CURSOR: Cursor<&str> = - Cursor::with_direction("compile-time-test", Direction::Prev); - - #[test] - fn cursor_round_trips_through_opaque_token() { - let cursor = Cursor::new(FixtureKey { - created_at: "2026-03-22T10:30:00Z".to_owned(), - id: "8b116c56-0a58-4c55-b7d7-06ee6bbddb8c".to_owned(), - }); - - let encoded = cursor.encode().expect("cursor encoding should succeed"); - let decoded = - Cursor::::decode(&encoded).expect("cursor decoding should succeed"); - - assert_eq!(decoded, cursor); - } - - #[test] - fn invalid_base64_cursor_fails_decode() { - let result = Cursor::::decode("!!!"); - - assert!(matches!(result, Err(CursorError::InvalidBase64 { .. }))); - } - - #[test] - fn padded_base64_cursor_decodes_successfully() { - let cursor = Cursor::new(FixtureKey { - created_at: "2026-03-22T10:30:00Z".to_owned(), - id: "8b116c56-0a58-4c55-b7d7-06ee6bbddb8c".to_owned(), - }); - let payload = serde_json::to_vec(&cursor).expect("cursor should serialize"); - let encoded = base64::engine::general_purpose::URL_SAFE.encode(payload); - - let decoded = - Cursor::::decode(&encoded).expect("padded cursor decoding should succeed"); - - assert_eq!(decoded, cursor); - } - - #[test] - fn structurally_invalid_json_cursor_fails_decode() { - let invalid_payload = - base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(br#"{"unexpected":true}"#); - - let result = Cursor::::decode(&invalid_payload); - - assert!(matches!(result, Err(CursorError::Deserialize { .. }))); - } - - #[rstest] - #[case(Direction::Next)] - #[case(Direction::Prev)] - fn direction_round_trips_through_encoding(#[case] direction: Direction) { - let cursor = Cursor::with_direction( - FixtureKey { - created_at: "2026-03-22T10:30:00Z".to_owned(), - id: "test-id".to_owned(), - }, - direction, - ); - let encoded = cursor.encode().expect("encoding succeeds"); - let decoded = Cursor::::decode(&encoded).expect("decoding succeeds"); - - assert_eq!(decoded.direction(), direction); - assert_eq!(decoded.key(), cursor.key()); - } - - #[test] - fn cursor_without_direction_defaults_to_next() { - // Simulate an old cursor (pre-4.1.2) without the `dir` field - let old_cursor_json = r#"{"key":{"created_at":"2026-03-22T10:30:00Z","id":"test-id"}}"#; - let encoded = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(old_cursor_json); - - let decoded = Cursor::::decode(&encoded).expect("decoding succeeds"); - - assert_eq!(decoded.direction(), Direction::Next); - } - - #[rstest] - #[case(Direction::Next, "Next")] - #[case(Direction::Prev, "Prev")] - fn new_cursor_includes_direction_in_json(#[case] direction: Direction, #[case] expected: &str) { - let cursor = Cursor::with_direction( - FixtureKey { - created_at: "2026-03-22T10:30:00Z".to_owned(), - id: "test-id".to_owned(), - }, - direction, - ); - let encoded = cursor.encode().expect("encoding succeeds"); - let decoded_bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD - .decode(&encoded) - .expect("base64 decoding succeeds"); - let json_value: serde_json::Value = - serde_json::from_slice(&decoded_bytes).expect("valid JSON"); - - // Verify the direction field exists and has the expected value - let dir_value = json_value - .get("dir") - .and_then(|v| v.as_str()) - .expect("dir field should exist and be a string"); - assert_eq!(dir_value, expected); - } - - #[test] - fn invalid_direction_value_returns_deserialize_error() { - // Create a cursor JSON with an invalid "dir" value - let invalid_cursor_json = - r#"{"key":{"created_at":"2026-03-22T10:30:00Z","id":"test-id"},"dir":"Sideways"}"#; - let encoded = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(invalid_cursor_json); - - let result = Cursor::::decode(&encoded); - - assert!(matches!(result, Err(CursorError::Deserialize { .. }))); - } - - #[rstest] - #[case(Direction::Next)] - #[case(Direction::Prev)] - fn into_parts_returns_key_and_direction(#[case] direction: Direction) { - let key = FixtureKey { - created_at: "2026-03-22T10:30:00Z".to_owned(), - id: "test-id".to_owned(), - }; - let cursor = Cursor::with_direction(key.clone(), direction); - - let (returned_key, returned_dir) = cursor.into_parts(); - - assert_eq!(returned_key, key); - assert_eq!(returned_dir, direction); - } - - #[test] - fn cursor_new_uses_next_direction() { - let cursor = Cursor::new(FixtureKey { - created_at: "2026-03-22T10:30:00Z".to_owned(), - id: "test-id".to_owned(), - }); - - assert_eq!(cursor.direction(), Direction::Next); - } - - #[test] - fn encode_returns_serialize_error_when_key_cannot_be_serialized() { - use std::collections::HashMap; - #[derive(Hash, PartialEq, Eq)] - struct FailingKey; - impl Serialize for FailingKey { - fn serialize(&self, _: S) -> Result { - Err(serde::ser::Error::custom("fail")) - } - } - let cursor = Cursor { - key: HashMap::from([(FailingKey, String::new())]), - dir: Direction::Next, - }; - let Err(CursorError::Serialize { message }) = cursor.encode() else { - panic!("expected Serialize error") - }; - assert!(message.contains("fail")); - } -} diff --git a/backend/crates/pagination/src/cursor/mod.rs b/backend/crates/pagination/src/cursor/mod.rs new file mode 100644 index 00000000..2d806c75 --- /dev/null +++ b/backend/crates/pagination/src/cursor/mod.rs @@ -0,0 +1,247 @@ +//! Opaque cursor encoding and decoding helpers. +//! +//! This module provides the [`Cursor`] type for encoding pagination +//! positions as opaque base64url-encoded JSON tokens, and the [`Direction`] +//! enum for bidirectional navigation. Cursors are transport-neutral and can be +//! used with any HTTP framework or serialization format. +//! +//! The base64url JSON encoding format ensures cursors are URL-safe and do not +//! require additional escaping in query parameters. The `dir` field is optional +//! in the JSON representation for backward compatibility with clients that omit +//! it (the default direction is `Next`). +//! +//! **Security consideration**: cursors are opaque but not signed or encrypted. +//! They encode the ordering key only, not any access control information. +//! Consumers must validate that the requesting user has permission to access +//! the underlying data. + +use base64::{ + Engine as _, + engine::general_purpose::{URL_SAFE, URL_SAFE_NO_PAD}, +}; +use serde::{Deserialize, Serialize, de::DeserializeOwned}; +use thiserror::Error; + +/// Direction of pagination relative to the cursor. +/// +/// Indicates whether the cursor represents a position for fetching +/// the next page (forward in sort order) or the previous page +/// (backward in sort order). +/// +/// # Examples +/// +/// ``` +/// use pagination::Direction; +/// +/// let forward = Direction::Next; +/// let backward = Direction::Prev; +/// +/// assert_ne!(forward, backward); +/// ``` +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub enum Direction { + /// Forward in the sort order (e.g., newer items if sorting ascending). + #[default] + Next, + /// Backward in the sort order (e.g., older items). + Prev, +} + +/// Cursor wrapper for an ordered boundary key with direction. +/// +/// The encoded representation is base64url JSON and must be treated as opaque +/// by clients. The direction indicates whether this cursor is meant for +/// fetching the next page (forward) or previous page (backward). +/// +/// # Example +/// +/// ``` +/// use pagination::{Cursor, Direction}; +/// use serde::{Deserialize, Serialize}; +/// +/// #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +/// struct UserKey { +/// created_at: String, +/// id: String, +/// } +/// +/// let cursor = Cursor::new(UserKey { +/// created_at: "2026-03-22T10:30:00Z".to_owned(), +/// id: "8b116c56-0a58-4c55-b7d7-06ee6bbddb8c".to_owned(), +/// }); +/// let encoded = cursor.encode().expect("cursor encoding succeeds"); +/// let decoded = Cursor::::decode(&encoded).expect("cursor decoding succeeds"); +/// +/// assert_eq!(decoded.key(), cursor.key()); +/// assert_eq!(decoded.direction(), Direction::Next); +/// ``` +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct Cursor { + key: Key, + #[serde(default)] + dir: Direction, +} + +/// Errors raised while encoding or decoding opaque cursors. +#[derive(Debug, Error, Clone, PartialEq, Eq)] +pub enum CursorError { + /// The cursor key could not be serialized into JSON. + #[error("cursor JSON serialization failed: {message}")] + Serialize { + /// Human-readable serialization failure details. + message: String, + }, + /// The encoded token was not valid base64url. + #[error("cursor is not valid base64url: {message}")] + InvalidBase64 { + /// Human-readable base64 decoding failure details. + message: String, + }, + /// The decoded JSON payload did not match the expected key shape. + #[error("cursor JSON deserialization failed: {message}")] + Deserialize { + /// Human-readable deserialization failure details. + message: String, + }, + /// The decoded JSON payload used a direction this crate cannot navigate. + #[error("cursor direction is unsupported: {direction}")] + UnsupportedDirection { + /// Direction value found in the cursor payload. + direction: String, + }, +} + +impl Cursor { + /// Construct a cursor from one ordering key with the default direction (`Next`). + /// + /// # Examples + /// + /// ``` + /// use pagination::{Cursor, Direction}; + /// + /// let cursor = Cursor::new("my-key"); + /// assert_eq!(cursor.direction(), Direction::Next); + /// ``` + #[must_use] + pub const fn new(key: Key) -> Self { + Self { + key, + dir: Direction::Next, + } + } + + /// Construct a cursor from one ordering key with an explicit direction. + /// + /// # Examples + /// + /// ``` + /// use pagination::{Cursor, Direction}; + /// + /// let cursor = Cursor::with_direction("my-key", Direction::Prev); + /// assert_eq!(cursor.direction(), Direction::Prev); + /// ``` + #[must_use] + pub const fn with_direction(key: Key, dir: Direction) -> Self { + Self { key, dir } + } + + /// Borrow the cursor key. + #[must_use] + pub const fn key(&self) -> &Key { + &self.key + } + + /// Access the pagination direction. + #[must_use] + pub const fn direction(&self) -> Direction { + self.dir + } + + /// Consume the cursor and return the inner key. + #[must_use] + pub fn into_inner(self) -> Key { + self.key + } + + /// Decompose the cursor into its constituent parts (key and direction). + /// + /// # Examples + /// + /// ``` + /// use pagination::{Cursor, Direction}; + /// + /// let cursor = Cursor::with_direction("my-key", Direction::Prev); + /// let (key, dir) = cursor.into_parts(); + /// assert_eq!(key, "my-key"); + /// assert_eq!(dir, Direction::Prev); + /// ``` + #[must_use] + pub fn into_parts(self) -> (Key, Direction) { + (self.key, self.dir) + } +} + +impl Cursor +where + Key: Serialize, +{ + /// Encode the cursor to an opaque base64url JSON token. + /// + /// # Errors + /// + /// Returns [`CursorError::Serialize`] when the cursor key cannot be + /// serialized into JSON. + pub fn encode(&self) -> Result { + let payload = serde_json::to_vec(self).map_err(|error| CursorError::Serialize { + message: error.to_string(), + })?; + Ok(URL_SAFE_NO_PAD.encode(payload)) + } +} + +impl Cursor +where + Key: DeserializeOwned, +{ + /// Decode a cursor from an opaque base64url JSON token. + /// + /// # Errors + /// + /// Returns [`CursorError::InvalidBase64`] when `value` is not valid + /// base64url, [`CursorError::UnsupportedDirection`] when `dir` is present + /// but not one of the supported directions, and [`CursorError::Deserialize`] + /// when the decoded JSON does not match the expected cursor shape. + pub fn decode(value: &str) -> Result { + let payload = URL_SAFE_NO_PAD + .decode(value) + .or_else(|_| URL_SAFE.decode(value)) + .map_err(|error| CursorError::InvalidBase64 { + message: error.to_string(), + })?; + let cursor_json: serde_json::Value = + serde_json::from_slice(&payload).map_err(|error| CursorError::Deserialize { + message: error.to_string(), + })?; + reject_unsupported_direction(&cursor_json)?; + serde_json::from_value(cursor_json).map_err(|error| CursorError::Deserialize { + message: error.to_string(), + }) + } +} + +fn reject_unsupported_direction(cursor_json: &serde_json::Value) -> Result<(), CursorError> { + let Some(direction) = cursor_json.get("dir") else { + return Ok(()); + }; + if matches!(direction.as_str(), Some("Next" | "Prev")) { + return Ok(()); + } + Err(CursorError::UnsupportedDirection { + direction: direction + .as_str() + .map_or_else(|| direction.to_string(), str::to_owned), + }) +} + +#[cfg(test)] +mod tests; diff --git a/backend/crates/pagination/src/cursor/tests.rs b/backend/crates/pagination/src/cursor/tests.rs new file mode 100644 index 00000000..64ee2025 --- /dev/null +++ b/backend/crates/pagination/src/cursor/tests.rs @@ -0,0 +1,175 @@ +//! Unit tests for opaque cursor encoding and decoding. + +use base64::Engine as _; +use rstest::rstest; +use serde::{Deserialize, Serialize}; + +use super::{Cursor, CursorError, Direction}; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +struct FixtureKey { + created_at: String, + id: String, +} + +// Verify Cursor constructors work in const contexts. +const _CONST_CURSOR: Cursor<&str> = Cursor::new("compile-time-test"); +const _CONST_DIRECTIONAL_CURSOR: Cursor<&str> = + Cursor::with_direction("compile-time-test", Direction::Prev); + +#[test] +fn cursor_round_trips_through_opaque_token() { + let cursor = Cursor::new(FixtureKey { + created_at: "2026-03-22T10:30:00Z".to_owned(), + id: "8b116c56-0a58-4c55-b7d7-06ee6bbddb8c".to_owned(), + }); + + let encoded = cursor.encode().expect("cursor encoding should succeed"); + let decoded = Cursor::::decode(&encoded).expect("cursor decoding should succeed"); + + assert_eq!(decoded, cursor); +} + +#[test] +fn invalid_base64_cursor_fails_decode() { + let result = Cursor::::decode("!!!"); + + assert!(matches!(result, Err(CursorError::InvalidBase64 { .. }))); +} + +#[test] +fn padded_base64_cursor_decodes_successfully() { + let cursor = Cursor::new(FixtureKey { + created_at: "2026-03-22T10:30:00Z".to_owned(), + id: "8b116c56-0a58-4c55-b7d7-06ee6bbddb8c".to_owned(), + }); + let payload = serde_json::to_vec(&cursor).expect("cursor should serialize"); + let encoded = base64::engine::general_purpose::URL_SAFE.encode(payload); + + let decoded = + Cursor::::decode(&encoded).expect("padded cursor decoding should succeed"); + + assert_eq!(decoded, cursor); +} + +#[test] +fn structurally_invalid_json_cursor_fails_decode() { + let invalid_payload = + base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(br#"{"unexpected":true}"#); + + let result = Cursor::::decode(&invalid_payload); + + assert!(matches!(result, Err(CursorError::Deserialize { .. }))); +} + +#[rstest] +#[case(Direction::Next)] +#[case(Direction::Prev)] +fn direction_round_trips_through_encoding(#[case] direction: Direction) { + let cursor = Cursor::with_direction( + FixtureKey { + created_at: "2026-03-22T10:30:00Z".to_owned(), + id: "test-id".to_owned(), + }, + direction, + ); + let encoded = cursor.encode().expect("encoding succeeds"); + let decoded = Cursor::::decode(&encoded).expect("decoding succeeds"); + + assert_eq!(decoded.direction(), direction); + assert_eq!(decoded.key(), cursor.key()); +} + +#[test] +fn cursor_without_direction_defaults_to_next() { + let old_cursor_json = r#"{"key":{"created_at":"2026-03-22T10:30:00Z","id":"test-id"}}"#; + let encoded = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(old_cursor_json); + + let decoded = Cursor::::decode(&encoded).expect("decoding succeeds"); + + assert_eq!(decoded.direction(), Direction::Next); +} + +#[rstest] +#[case(Direction::Next, "Next")] +#[case(Direction::Prev, "Prev")] +fn new_cursor_includes_direction_in_json(#[case] direction: Direction, #[case] expected: &str) { + let cursor = Cursor::with_direction( + FixtureKey { + created_at: "2026-03-22T10:30:00Z".to_owned(), + id: "test-id".to_owned(), + }, + direction, + ); + let encoded = cursor.encode().expect("encoding succeeds"); + let decoded_bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD + .decode(&encoded) + .expect("base64 decoding succeeds"); + let json_value: serde_json::Value = serde_json::from_slice(&decoded_bytes).expect("valid JSON"); + + let dir_value = json_value + .get("dir") + .and_then(|v| v.as_str()) + .expect("dir field should exist and be a string"); + assert_eq!(dir_value, expected); +} + +#[test] +fn invalid_direction_value_returns_unsupported_direction_error() { + let invalid_cursor_json = + r#"{"key":{"created_at":"2026-03-22T10:30:00Z","id":"test-id"},"dir":"Sideways"}"#; + let encoded = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(invalid_cursor_json); + + let result = Cursor::::decode(&encoded); + + assert!(matches!( + result, + Err(CursorError::UnsupportedDirection { direction }) if direction == "Sideways" + )); +} + +#[rstest] +#[case(Direction::Next)] +#[case(Direction::Prev)] +fn into_parts_returns_key_and_direction(#[case] direction: Direction) { + let key = FixtureKey { + created_at: "2026-03-22T10:30:00Z".to_owned(), + id: "test-id".to_owned(), + }; + let cursor = Cursor::with_direction(key.clone(), direction); + + let (returned_key, returned_dir) = cursor.into_parts(); + + assert_eq!(returned_key, key); + assert_eq!(returned_dir, direction); +} + +#[test] +fn cursor_new_uses_next_direction() { + let cursor = Cursor::new(FixtureKey { + created_at: "2026-03-22T10:30:00Z".to_owned(), + id: "test-id".to_owned(), + }); + + assert_eq!(cursor.direction(), Direction::Next); +} + +#[test] +fn encode_returns_serialize_error_when_key_cannot_be_serialized() { + use std::collections::HashMap; + #[derive(Hash, PartialEq, Eq)] + struct FailingKey; + impl Serialize for FailingKey { + fn serialize(&self, _: S) -> Result { + Err(serde::ser::Error::custom("fail")) + } + } + let cursor = Cursor { + key: HashMap::from([(FailingKey, String::new())]), + dir: Direction::Next, + }; + let Err(CursorError::Serialize { message }) = cursor.encode() else { + panic!("expected Serialize error") + }; + assert!(message.contains("fail")); +} diff --git a/backend/crates/pagination/src/lib.rs b/backend/crates/pagination/src/lib.rs index 043019b4..45bc7643 100644 --- a/backend/crates/pagination/src/lib.rs +++ b/backend/crates/pagination/src/lib.rs @@ -128,6 +128,7 @@ //! |---------------------|------------------|-----------------|-------------------------| //! | [`CursorError`] | `InvalidBase64` | 400 Bad Request | `invalid_cursor` | //! | [`CursorError`] | `Deserialize` | 400 Bad Request | `invalid_cursor` | +//! | [`CursorError`] | `UnsupportedDirection` | 400 Bad Request | `unsupported_direction` | //! | [`CursorError`] | `Serialize` | 500 Internal | `internal_error` | //! | [`PageParamsError`] | `InvalidLimit` | 400 Bad Request | `invalid_page_params` | //! diff --git a/backend/src/domain/ports/mod.rs b/backend/src/domain/ports/mod.rs index d69c3580..9feced4c 100644 --- a/backend/src/domain/ports/mod.rs +++ b/backend/src/domain/ports/mod.rs @@ -194,7 +194,9 @@ pub use user_preferences_repository::{ FixtureUserPreferencesRepository, UserPreferencesRepository, UserPreferencesRepositoryError, }; pub use user_profile_query::{FixtureUserProfileQuery, UserProfileQuery}; -pub use user_repository::{ListUsersPageRequest, UserPersistenceError, UserRepository}; +pub use user_repository::{ + ListUsersPageRequest, UserPaginationError, UserPersistenceError, UserRepository, +}; pub use users_query::{FixtureUsersQuery, UsersPage, UsersQuery}; #[cfg(test)] pub use walk_session_command::MockWalkSessionCommand; diff --git a/backend/src/domain/ports/user_repository.rs b/backend/src/domain/ports/user_repository.rs index 684043f3..c92d3b77 100644 --- a/backend/src/domain/ports/user_repository.rs +++ b/backend/src/domain/ports/user_repository.rs @@ -8,6 +8,16 @@ use crate::domain::{User, UserCursorKey, UserId}; use super::define_port_error; +define_port_error! { + /// Pagination errors caused by caller-provided page boundaries. + pub enum UserPaginationError { + /// Cursor text could not be decoded into the expected users cursor. + InvalidCursorFormat { message: String } => "invalid users cursor: {message}", + /// Cursor direction is not supported by the repository query. + UnsupportedDirection { direction: String } => "unsupported users cursor direction: {direction}", + } +} + define_port_error! { /// Persistence errors raised by user repository adapters. pub enum UserPersistenceError { @@ -15,6 +25,8 @@ define_port_error! { Connection { message: String } => "user repository connection failed: {message}", /// Query or mutation failed during execution. Query { message: String } => "user repository query failed: {message}", + /// Pagination request failed because the caller supplied an invalid page boundary. + Pagination { error: UserPaginationError } => "user repository pagination failed: {error}", } } @@ -86,3 +98,39 @@ pub trait UserRepository: Send + Sync { )) } } + +#[cfg(test)] +mod tests { + //! Regression coverage for user repository port error constructors. + + use super::{UserPaginationError, UserPersistenceError}; + + #[test] + fn pagination_error_constructors_preserve_client_failure_context() { + let invalid = UserPaginationError::invalid_cursor_format("base64 decode failed"); + assert_eq!( + invalid.to_string(), + "invalid users cursor: base64 decode failed" + ); + + let unsupported = UserPaginationError::unsupported_direction("sideways"); + assert_eq!( + unsupported.to_string(), + "unsupported users cursor direction: sideways" + ); + } + + #[test] + fn persistence_error_can_wrap_pagination_failures() { + let error = UserPersistenceError::pagination(UserPaginationError::unsupported_direction( + "sideways", + )); + + assert!(matches!( + error, + UserPersistenceError::Pagination { + error: UserPaginationError::UnsupportedDirection { .. } + } + )); + } +} diff --git a/backend/src/inbound/http/users_pagination.rs b/backend/src/inbound/http/users_pagination.rs index d05ba1e0..730e0480 100644 --- a/backend/src/inbound/http/users_pagination.rs +++ b/backend/src/inbound/http/users_pagination.rs @@ -7,7 +7,9 @@ use std::num::NonZeroUsize; use actix_web::HttpRequest; -use pagination::{Cursor, Direction, MAX_LIMIT, PageParams, Paginated, PaginationLinks}; +use pagination::{ + Cursor, CursorError, Direction, MAX_LIMIT, PageParams, Paginated, PaginationLinks, +}; use serde::Deserialize; use serde_json::json; use url::Url; @@ -131,7 +133,7 @@ pub fn parse_users_page_params( .as_deref() .map(Cursor::::decode) .transpose() - .map_err(|_| invalid_cursor_error())?; + .map_err(map_cursor_error)?; let direction = cursor .as_ref() .map_or(UsersPageDirection::First, cursor_direction); @@ -275,6 +277,21 @@ fn invalid_cursor_error() -> Error { .with_details(json!({ "field": "cursor", "code": "invalid_cursor" })) } +fn unsupported_direction_error() -> Error { + Error::invalid_request("cursor direction is unsupported") + .with_details(json!({ "field": "cursor", "code": "unsupported_direction" })) +} + +fn map_cursor_error(error: CursorError) -> Error { + match error { + CursorError::UnsupportedDirection { .. } => unsupported_direction_error(), + CursorError::InvalidBase64 { .. } | CursorError::Deserialize { .. } => { + invalid_cursor_error() + } + CursorError::Serialize { .. } => Error::internal("failed to decode users cursor"), + } +} + fn invalid_limit_error(value: Option<&str>) -> Error { Error::invalid_request(format!("limit must be between 1 and {MAX_LIMIT}")).with_details(json!({ "field": "limit", diff --git a/backend/src/outbound/persistence/diesel_users_query.rs b/backend/src/outbound/persistence/diesel_users_query.rs index d2db2d8a..87632628 100644 --- a/backend/src/outbound/persistence/diesel_users_query.rs +++ b/backend/src/outbound/persistence/diesel_users_query.rs @@ -14,7 +14,7 @@ use crate::domain::{Error, User, UserCursorKey, UserId}; use super::diesel_user_repository::DieselUserRepository; use super::user_persistence_error_mapping::map_user_persistence_error; #[cfg(test)] -use crate::domain::ports::UserPersistenceError; +use crate::domain::ports::{UserPaginationError, UserPersistenceError}; /// Diesel-backed `UsersQuery` implementation backed by user repository reads. #[derive(Clone)] @@ -107,6 +107,8 @@ mod tests { enum StubFailure { Connection, Query, + InvalidCursor, + UnsupportedDirection, } impl StubFailure { @@ -114,6 +116,12 @@ mod tests { match self { Self::Connection => UserPersistenceError::connection("database unavailable"), Self::Query => UserPersistenceError::query("database query failed"), + Self::InvalidCursor => UserPersistenceError::pagination( + UserPaginationError::invalid_cursor_format("bad token"), + ), + Self::UnsupportedDirection => UserPersistenceError::pagination( + UserPaginationError::unsupported_direction("sideways"), + ), } } } @@ -327,6 +335,8 @@ mod tests { #[rstest] #[case(StubFailure::Connection, ErrorCode::ServiceUnavailable)] #[case(StubFailure::Query, ErrorCode::InternalError)] + #[case(StubFailure::InvalidCursor, ErrorCode::InvalidRequest)] + #[case(StubFailure::UnsupportedDirection, ErrorCode::InvalidRequest)] #[tokio::test] async fn list_users_page_maps_persistence_failures( #[case] failure: StubFailure, diff --git a/backend/src/outbound/persistence/user_persistence_error_mapping.rs b/backend/src/outbound/persistence/user_persistence_error_mapping.rs index a02d3037..9d28ed2d 100644 --- a/backend/src/outbound/persistence/user_persistence_error_mapping.rs +++ b/backend/src/outbound/persistence/user_persistence_error_mapping.rs @@ -1,11 +1,62 @@ //! Shared mapping from user persistence failures to domain HTTP-safe errors. use crate::domain::Error; -use crate::domain::ports::UserPersistenceError; +use crate::domain::ports::{UserPaginationError, UserPersistenceError}; +use serde_json::json; pub(super) fn map_user_persistence_error(error: UserPersistenceError) -> Error { match error { UserPersistenceError::Connection { message } => Error::service_unavailable(message), UserPersistenceError::Query { message } => Error::internal(message), + UserPersistenceError::Pagination { error } => map_user_pagination_error(error), + } +} + +fn map_user_pagination_error(error: UserPaginationError) -> Error { + match error { + UserPaginationError::InvalidCursorFormat { .. } => { + Error::invalid_request("cursor is invalid") + .with_details(json!({ "field": "cursor", "code": "invalid_cursor" })) + } + UserPaginationError::UnsupportedDirection { .. } => { + Error::invalid_request("cursor direction is unsupported") + .with_details(json!({ "field": "cursor", "code": "unsupported_direction" })) + } + } +} + +#[cfg(test)] +mod tests { + //! Regression coverage for HTTP-safe user persistence error mapping. + + use super::*; + use crate::domain::ErrorCode; + use rstest::rstest; + use serde_json::json; + + #[rstest] + #[case( + UserPersistenceError::pagination(UserPaginationError::invalid_cursor_format("bad token")), + "cursor is invalid", + "invalid_cursor" + )] + #[case( + UserPersistenceError::pagination(UserPaginationError::unsupported_direction("sideways")), + "cursor direction is unsupported", + "unsupported_direction" + )] + fn pagination_errors_map_to_invalid_request( + #[case] source: UserPersistenceError, + #[case] expected_message: &str, + #[case] expected_detail_code: &str, + ) { + let error = map_user_persistence_error(source); + + assert_eq!(error.code(), ErrorCode::InvalidRequest); + assert_eq!(error.message(), expected_message); + assert_eq!( + error.details(), + Some(&json!({ "field": "cursor", "code": expected_detail_code })) + ); } } diff --git a/backend/tests/features/users_list_pagination.feature b/backend/tests/features/users_list_pagination.feature index e0da1e97..ff8e2c38 100644 --- a/backend/tests/features/users_list_pagination.feature +++ b/backend/tests/features/users_list_pagination.feature @@ -30,6 +30,11 @@ Feature: Users list pagination When the client requests the users list with an invalid cursor Then the users response is bad request with invalid_cursor details + Scenario: Unsupported users cursor direction is rejected + Given db-present startup mode with five ordered users + When the client requests the users list with an unsupported cursor direction + Then the users response is bad request with unsupported_direction details + Scenario: Users list requires a session Given db-present startup mode with five ordered users When the client requests the users list without a session diff --git a/backend/tests/users_list_pagination_bdd.rs b/backend/tests/users_list_pagination_bdd.rs index 47c946fa..9e350c89 100644 --- a/backend/tests/users_list_pagination_bdd.rs +++ b/backend/tests/users_list_pagination_bdd.rs @@ -58,6 +58,19 @@ fn the_client_requests_the_users_list_with_an_invalid_cursor(world: &mut World) run_authenticated_request(world, "/api/v1/users?cursor=not-a-cursor"); } +#[when("the client requests the users list with an unsupported cursor direction")] +fn the_client_requests_the_users_list_with_an_unsupported_cursor_direction(world: &mut World) { + run_authenticated_request( + world, + concat!( + "/api/v1/users?cursor=", + "eyJrZXkiOnsiY3JlYXRlZF9hdCI6IjIwMjYtMDEtMDFUMDA6MDA6MDBaIiwiaWQi", + "OiIxMTExMTExMS0xMTExLTExMTEtMTExMS0xMTExMTExMTExMTEifSwiZGlyIjoi", + "U2lkZXdheXMifQ" + ), + ); +} + #[when("the client requests the users list without a session")] fn the_client_requests_the_users_list_without_a_session(world: &mut World) { run_unauthenticated_request(world); @@ -108,6 +121,11 @@ fn the_users_response_is_bad_request_with_invalid_cursor_details(world: &mut Wor assert_error(world, 400, "invalid_cursor"); } +#[then("the users response is bad request with unsupported_direction details")] +fn the_users_response_is_bad_request_with_unsupported_direction_details(world: &mut World) { + assert_error(world, 400, "unsupported_direction"); +} + #[then("the users response is unauthorised")] fn the_users_response_is_unauthorised(world: &mut World) { assert_status(world, 401); @@ -158,6 +176,14 @@ fn invalid_users_cursor_is_rejected(world: World) { drop(world); } +#[scenario( + path = "tests/features/users_list_pagination.feature", + name = "Unsupported users cursor direction is rejected" +)] +fn unsupported_users_cursor_direction_is_rejected(world: World) { + drop(world); +} + #[scenario( path = "tests/features/users_list_pagination.feature", name = "Users list requires a session" diff --git a/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md b/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md index b25f5d53..212f4dc3 100644 --- a/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md +++ b/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md @@ -165,11 +165,37 @@ errors. - [x] 2026-05-26: Implementation approved by the user. Baseline confirmed on branch `backend-4-2-2-surface-pagination-aware-errors` with a clean worktree before code changes. +- [x] 2026-05-26: Added `UserPaginationError` and the + `UserPersistenceError::Pagination` wrapper so user repository ports can + distinguish client pagination failures from connection and query failures. +- [x] 2026-05-26: Mapped repository-originated invalid cursor and unsupported + direction failures to HTTP-safe invalid-request errors with stable + `details.code` values of `invalid_cursor` and `unsupported_direction`. +- [x] 2026-05-26: Extended the pagination crate with + `CursorError::UnsupportedDirection` so decoded cursor payloads with an + unsupported `dir` value are no longer collapsed into generic deserialization + failures. +- [x] 2026-05-26: Added `rstest-bdd` coverage proving that + `GET /api/v1/users` returns HTTP `400` with `unsupported_direction` details + for an opaque cursor whose decoded `dir` is unsupported. A duplicate + Actix-level unit test was removed after `make lint` showed it pushed the + legacy users test module over the 400-line module limit. +- [x] 2026-05-26: Focused checks passed: + `cargo test -p pagination -- --nocapture`, + `cargo test -p backend user_persistence_error -- --nocapture`, + `cargo test -p backend diesel_users_query -- --nocapture`, + and + `cargo test -p backend --test users_list_pagination_bdd + unsupported_users_cursor_direction_is_rejected -- --nocapture`. +- [x] 2026-05-26: Full deterministic gates passed after lint-driven + refactoring: `make check-fmt`, `make lint`, and `make test`. The full test + gate ran 1227 Rust tests with 1227 passing and 4 skipped, followed by the + frontend/workspace test commands. - [x] M0: Approval received and baseline established. -- [ ] Implementation milestone M1 completed and committed. -- [ ] Implementation milestone M2 completed and committed. -- [ ] Implementation milestone M3 completed and committed. -- [ ] Implementation milestone M4 completed and committed. +- [x] Implementation milestone M1 completed. +- [x] Implementation milestone M2 completed. +- [x] Implementation milestone M3 completed. +- [x] Implementation milestone M4 completed. - [ ] Implementation milestone M5 completed and committed. - [ ] M6: Validation and closure completed. - [ ] Roadmap item 4.2.2 marked done after all gates pass. @@ -201,6 +227,34 @@ errors. Impact: The implementation should align adapter mappings with the existing crate contract rather than inventing a new policy. +- Observation: Unsupported direction can be classified before deserializing a + typed `Cursor` by first parsing the decoded payload as + `serde_json::Value` and inspecting the optional `dir` field. + Evidence: `backend/crates/pagination/src/cursor.rs::decode` now rejects + unsupported `dir` values before calling `serde_json::from_value`. + Impact: Legacy cursors with no `dir`, and supported `Next`/`Prev` cursors, + keep their existing behaviour while unsupported direction receives a stable + client-facing error code. + +- Observation: The users pagination BDD support requires request paths with a + static lifetime. + Evidence: `run_authenticated_request` rejected a formatted temporary path + for the unsupported-direction cursor; using `concat!` for the full opaque + cursor URL satisfies the helper contract. + Impact: Future BDD cursor fixtures should prefer static opaque tokens when + the request helper does not need runtime data. + +- Observation: `make lint` enforces module size and self-named-module rules + across test code. + Evidence: The first lint run rejected `user_repository.rs` for items after a + test module and `cursor.rs` for exceeding 400 lines; the second lint run + rejected `cursor.rs` plus `cursor/tests.rs` as a self-named module layout + and rejected the legacy users Actix test module at 401 lines. + Impact: Cursor code now lives under `cursor/mod.rs` with tests in + `cursor/tests.rs`, user repository tests sit at the end of the file, and the + unsupported-direction HTTP contract is covered by BDD rather than another + Actix unit test. + ## Decision Log - Decision: Treat "repository layer" as the domain port/query/repository error @@ -238,6 +292,25 @@ errors. `origin/backend-4-2-2-surface-pagination-aware-errors`. Date/Author: 2026-05-26 / Codex. +- Decision: Use a dedicated `UserPaginationError` enum wrapped by + `UserPersistenceError::Pagination`, rather than adding multiple pagination + variants directly to `UserPersistenceError`. + Rationale: The wrapper keeps pagination policy grouped while preserving the + existing connection/query split for persistence failures. + Date/Author: 2026-05-26 / Codex. + +- Decision: Add `CursorError::UnsupportedDirection` to the shared pagination + crate. + Rationale: HTTP adapters should not string-match serde errors to decide + whether a cursor failure is an invalid token or an unsupported direction. + Date/Author: 2026-05-26 / Codex. + +- Decision: Commit M1-M4 together after the full source gates pass. + Rationale: The port variant, HTTP-safe mapping, cursor classification, and + externally visible behaviour are tightly coupled and were developed as one + coherent error-surfacing change. + Date/Author: 2026-05-26 / Codex. + ## Outcomes & Retrospective No implementation has been performed yet. The planning phase identified that From 69d6463c385493198bbdbb99101cb218147d4606 Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 26 May 2026 13:09:30 +0200 Subject: [PATCH 4/6] Document pagination-aware errors Record the user-facing and internal pagination error contract. Mark backend roadmap item 4.2.2 done after the implementation and validation gates passed. --- docs/backend-roadmap.md | 2 +- docs/contents.md | 3 ++ docs/developers-guide.md | 10 +++++ ...d-4-2-2-surface-pagination-aware-errors.md | 14 +++++- docs/keyset-pagination-design.md | 30 +++++++++++++ docs/users-guide.md | 44 +++++++++++++++++++ docs/wildside-backend-architecture.md | 16 +++++++ 7 files changed, 116 insertions(+), 3 deletions(-) create mode 100644 docs/users-guide.md diff --git a/docs/backend-roadmap.md b/docs/backend-roadmap.md index 13646733..069e418e 100644 --- a/docs/backend-roadmap.md +++ b/docs/backend-roadmap.md @@ -232,7 +232,7 @@ see `docs/keyset-pagination-design.md` for the detailed crate design. - [x] 4.2.1. Replace offset pagination in `GET /api/v1/users` with the new crate, including Diesel filters that respect `(created_at, id)` ordering and bb8 connection pooling. -- [ ] 4.2.2. Update the repository layer to surface pagination-aware errors +- [x] 4.2.2. Update the repository layer to surface pagination-aware errors (for example, invalid cursor format and unsupported direction) with HTTP 400 responses. - [ ] 4.2.3. Ensure pagination telemetry records page size, cursor direction, diff --git a/docs/contents.md b/docs/contents.md index 6f2984a9..beba9657 100644 --- a/docs/contents.md +++ b/docs/contents.md @@ -96,6 +96,9 @@ ## Operational runbooks +- [Wildside server users guide](users-guide.md) – user-visible backend API + behaviour, including users list pagination errors. _Audience: API consumers + and backend developers._ - [OSM ingestion end-to-end runbook](runbooks/osm-ingestion-e2e.md) – operator procedure for executing and verifying `ingest-osm` runs, including deterministic reruns. _Audience: backend operators and developers._ diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 8d9f4dad..e323cc9b 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -358,6 +358,16 @@ transport-agnostic: - **Error mapping** is performed by inbound adapters, not by the shared crate. The crate documents recommended HTTP status codes and envelope `code` values, but the adapter layer owns the final mapping. +- **Pagination-aware repository errors** are modelled as semantic port errors + rather than opaque query strings. For users pagination, + `UserPersistenceError::Pagination` wraps `UserPaginationError`, allowing + repository-originated invalid cursor and unsupported direction failures to + map to HTTP `400` while connection failures remain `503` and unexpected + query failures remain redacted `500` responses. +- **BDD cursor fixtures** that exercise invalid opaque cursor payloads should + use static base64url tokens via `concat!` when the request helper expects a + `'static` path. This keeps the behaviour explicit without adding test-only + dependencies to an adapter crate. ### Documentation invariant testing diff --git a/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md b/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md index 212f4dc3..b127af33 100644 --- a/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md +++ b/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md @@ -191,14 +191,24 @@ errors. refactoring: `make check-fmt`, `make lint`, and `make test`. The full test gate ran 1227 Rust tests with 1227 passing and 4 skipped, followed by the frontend/workspace test commands. +- [x] 2026-05-26: Committed M1-M4 as `40a85ed` after deterministic gates + passed. +- [x] 2026-05-26: CodeRabbit reviewed the implementation milestone and + reported zero findings. +- [x] 2026-05-26: Updated the pagination design, backend architecture, + developers guide, new server users guide, documentation index, and roadmap + entry to document the pagination-aware error contract. +- [x] 2026-05-26: Documentation closure gates passed after the user guide + table alignment fix: `make fmt`, `make check-fmt`, `make markdownlint`, + `make lint`, and `make test`. - [x] M0: Approval received and baseline established. - [x] Implementation milestone M1 completed. - [x] Implementation milestone M2 completed. - [x] Implementation milestone M3 completed. - [x] Implementation milestone M4 completed. -- [ ] Implementation milestone M5 completed and committed. +- [x] Implementation milestone M5 completed. - [ ] M6: Validation and closure completed. -- [ ] Roadmap item 4.2.2 marked done after all gates pass. +- [x] Roadmap item 4.2.2 marked done after all gates pass. - [ ] Draft implementation pull request opened or updated after approval work lands. diff --git a/docs/keyset-pagination-design.md b/docs/keyset-pagination-design.md index aab98691..49776e04 100644 --- a/docs/keyset-pagination-design.md +++ b/docs/keyset-pagination-design.md @@ -769,6 +769,36 @@ We also update any Postman collections or documentation pages to reflect that the response is now an object with `data` and `links`, etc., and that clients should use the provided `next/prev` URLs rather than manipulating offsets. +## Pagination error mapping + +Pagination failures caused by caller input are semantic client errors. The +shared pagination crate classifies malformed cursor tokens as +`CursorError::InvalidBase64` or `CursorError::Deserialize`, and decoded cursor +payloads whose `dir` field is not supported as +`CursorError::UnsupportedDirection`. Inbound adapters map these cases to HTTP +`400 Bad Request` using the existing Wildside error envelope. + +The repository layer also exposes pagination-aware port errors, so a query or +repository adapter can return the same client-error semantics when it detects +an invalid page boundary below the HTTP parser. The user repository port wraps +these failures in `UserPersistenceError::Pagination`, keeping them distinct +from connection failures and ordinary database query failures. + +Adapters must preserve the following mapping: + +- Malformed cursor tokens or structurally invalid cursor JSON return + `400 Bad Request` with `details.code = "invalid_cursor"`. +- Unsupported cursor directions return `400 Bad Request` with + `details.code = "unsupported_direction"`. +- Repository connection failures return the existing `503 Service Unavailable` + envelope. +- Unexpected database query failures return the existing redacted + `500 Internal Server Error` envelope. + +This keeps raw cursor parsing at the inbound HTTP boundary while allowing +repository-facing pagination failures to remain inspectable without string +matching or persistence-detail leakage. + ## Future Enhancements and Conclusion This pagination crate lays the groundwork for a consistent, efficient paging diff --git a/docs/users-guide.md b/docs/users-guide.md new file mode 100644 index 00000000..9a08d8eb --- /dev/null +++ b/docs/users-guide.md @@ -0,0 +1,44 @@ +# Wildside server users guide + +This guide records user-visible server behaviour for Wildside API consumers. +It focuses on contracts that clients can rely on when calling the backend. + +## Users list pagination + +`GET /api/v1/users` returns a paginated users response. Clients should follow +the `links.next` and `links.prev` URLs returned by the server instead of +building cursor values themselves. + +The endpoint accepts: + +- `cursor`: an opaque base64url cursor returned by a previous users list + response. +- `limit`: page size. The shared pagination default is 20 and the maximum is + 100. + +Successful responses include the existing paginated envelope: + +```json +{ + "data": [], + "limit": 20, + "links": { + "self": "/api/v1/users", + "next": null, + "prev": null + } +} +``` + +Pagination input errors use the standard Wildside error envelope and return +HTTP `400 Bad Request`: + +| Condition | Message | `details.field` | `details.code` | +|--------------------------------------------|-------------------------------------|-----------------|-------------------------| +| Cursor text is not a valid users cursor | `cursor is invalid` | `cursor` | `invalid_cursor` | +| Cursor direction is not supported | `cursor direction is unsupported` | `cursor` | `unsupported_direction` | + +Authentication and infrastructure errors keep their existing meanings. +Unauthenticated requests return `401`, repository availability failures return +`503`, and unexpected persistence query failures return a redacted `500` +response. diff --git a/docs/wildside-backend-architecture.md b/docs/wildside-backend-architecture.md index b9ae3829..dd52994f 100644 --- a/docs/wildside-backend-architecture.md +++ b/docs/wildside-backend-architecture.md @@ -2019,6 +2019,22 @@ JSON), backward-compatibility behaviour of the `dir` field, link construction query parameter preservation, and automatic limit normalization via the Deserialize implementation. +Roadmap 4.2.2 adds the pagination-aware error contract for endpoint adoption. +The pagination crate now distinguishes unsupported decoded cursor directions +from generic deserialization failures with `CursorError::UnsupportedDirection`. +Inbound HTTP adapters map malformed cursor tokens to `invalid_cursor` and +unsupported directions to `unsupported_direction`, both as HTTP `400 Bad +Request` responses in the existing Wildside error envelope. + +Repository and query ports may also surface pagination-aware errors when an +adapter detects an invalid page boundary below the HTTP parser. For the users +listing path, `UserPersistenceError::Pagination` carries +`UserPaginationError`, and the outbound query adapter maps those failures to +HTTP-safe invalid-request errors. Connection failures remain `503`, and +ordinary database query failures remain redacted `500` responses. Inbound +adapters must not import Diesel or outbound modules to make this mapping; the +semantic error travels through the port surface. + ##### 3.4.3 Pagination Compatibility Requirements (Phase 4, Future Work) Opaque pagination remains deferred future work. The current public contract for From 85462f881ac32519725a9b4dc06ee9e9dc090021 Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 26 May 2026 13:16:46 +0200 Subject: [PATCH 5/6] Record final pagination review Update the execution plan with the final CodeRabbit result and closure status for backend roadmap item 4.2.2. --- .../backend-4-2-2-surface-pagination-aware-errors.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md b/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md index b127af33..84c7bdad 100644 --- a/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md +++ b/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md @@ -201,13 +201,15 @@ errors. - [x] 2026-05-26: Documentation closure gates passed after the user guide table alignment fix: `make fmt`, `make check-fmt`, `make markdownlint`, `make lint`, and `make test`. +- [x] 2026-05-26: CodeRabbit reviewed the documentation and roadmap closure + milestone and reported zero findings. - [x] M0: Approval received and baseline established. - [x] Implementation milestone M1 completed. - [x] Implementation milestone M2 completed. - [x] Implementation milestone M3 completed. - [x] Implementation milestone M4 completed. - [x] Implementation milestone M5 completed. -- [ ] M6: Validation and closure completed. +- [x] M6: Validation and closure completed. - [x] Roadmap item 4.2.2 marked done after all gates pass. - [ ] Draft implementation pull request opened or updated after approval work lands. From ca56a3fef0c475ed339f2013136b0ec475f7edbe Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 26 May 2026 13:17:41 +0200 Subject: [PATCH 6/6] Record pagination PR handoff Update the execution plan with the draft PR handoff after pushing the completed backend 4.2.2 work. --- .../backend-4-2-2-surface-pagination-aware-errors.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md b/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md index 84c7bdad..9e68a6a2 100644 --- a/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md +++ b/docs/execplans/backend-4-2-2-surface-pagination-aware-errors.md @@ -203,6 +203,8 @@ errors. `make lint`, and `make test`. - [x] 2026-05-26: CodeRabbit reviewed the documentation and roadmap closure milestone and reported zero findings. +- [x] 2026-05-26: Draft PR #356 was updated after implementation with the + ExecPlan link, validation record, and Lody session reference. - [x] M0: Approval received and baseline established. - [x] Implementation milestone M1 completed. - [x] Implementation milestone M2 completed. @@ -211,7 +213,7 @@ errors. - [x] Implementation milestone M5 completed. - [x] M6: Validation and closure completed. - [x] Roadmap item 4.2.2 marked done after all gates pass. -- [ ] Draft implementation pull request opened or updated after approval work +- [x] Draft implementation pull request opened or updated after approval work lands. ## Surprises & Discoveries