From a778227fc25c3ca8b02eac3efff265be4477819c Mon Sep 17 00:00:00 2001 From: Rene Nochebuena Guerrero Date: Thu, 13 Aug 2026 23:11:23 -0600 Subject: [PATCH] feat(httputil): add Bind/BindEmpty request binding from path and query; v1.6.0 Bind and BindEmpty fill Req from path/query/json struct tags and validate once, extending the typed decode->validate->call->encode pipeline to routes with identifiers and filters. Conversion via builtins + encoding.TextUnmarshaler (uuid.UUID, time.Time); malformed value -> 400 naming the parameter; default: applies only when absent; repeated query -> slice; mis-tagged struct panics at wiring. Purely additive; existing adapters unchanged. Coordinated lockstep v1.6.0. --- CHANGELOG.md | 31 +++ README.md | 67 +++++- docs/adr/ADR-001-request-binding.md | 109 +++++++++ docs/adr/index.md | 7 + go.mod | 4 +- go.sum | 8 +- httputil/bind.go | 325 ++++++++++++++++++++++++++ httputil/bind_test.go | 343 ++++++++++++++++++++++++++++ httputil/doc.go | 32 ++- httputil/handler_func.go | 8 +- 10 files changed, 920 insertions(+), 14 deletions(-) create mode 100644 docs/adr/ADR-001-request-binding.md create mode 100644 httputil/bind.go create mode 100644 httputil/bind_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index cf05c91..723b2a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,37 @@ This module adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html --- +## [1.6.0] — 2026-08-13 + +Minor — request binding from path and query, not only the JSON body. + +### Added + +- **`httputil.Bind[Req, Res]`** and **`httputil.BindEmpty[Req]`** — a fourth adapter family + that fills `Req` from the path, the query string **and** the body, each field declaring its + source with a struct tag (`path:` / `query:` / `json:`), then validates the assembled struct + once with the same `valid.Validator`. The handler signature is identical to `Handle` / + `HandleEmpty`; `WithStatus` and the full error-mapping pipeline are reused unchanged. + - Conversion covers `string`, the sized integer/unsigned/float types, `bool`, and any type + whose pointer implements `encoding.TextUnmarshaler` — so `uuid.UUID` and `time.Time` bind + with no special-casing and no new dependency in `web`. + - A conversion failure is `ErrInvalidInput` naming the parameter (**400, never 500**). + - `default:` applies only when a parameter is **absent** (a present-but-empty `?q=` is left + as the zero value). Repeated query parameters bind to a slice; a comma inside a single value + is not split. A bodiless `GET`/`DELETE` is not an error — `BindEmpty` retires the + `HandleEmpty` empty-body (`io.EOF`) trap for routes keyed only by a path parameter. + - The struct is reflected over **once per type and cached**. A field with more than one source + tag, an unsupported field type, or a `default:` that is not a valid value for its field all + **panic at wiring** — a mis-tagged struct fails the service at boot, not on a request. + +### Changed + +- `HandlerFunc`'s doc comment no longer advertises itself for path/query parameters — those go + through `Bind` now; it remains the escape hatch for genuinely custom responses (streaming, + file downloads, non-JSON). `Handle`, `HandleNoBody`, `HandleEmpty` and `HandlerFunc` are + behaviourally unchanged. +- Bumped `contracts`, `core` to v1.6.0. + ## [1.5.0] — 2026-08-09 Minor — configurable success status on the httputil handler adapters. diff --git a/README.md b/README.md index 278d4c2..c731877 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # einherjar/web -[![version](https://img.shields.io/badge/version-v1.5.0-5C4EE5?style=flat-square)](https://code.nochebuena.dev/einherjar/web) +[![version](https://img.shields.io/badge/version-v1.6.0-5C4EE5?style=flat-square)](https://code.nochebuena.dev/einherjar/web) [![license](https://img.shields.io/badge/license-AGPL--3.0-22863A?style=flat-square)](LICENSE) [![go](https://img.shields.io/badge/Go-1.26+-00ADD8?style=flat-square&logo=go&logoColor=white)](https://go.dev) @@ -182,6 +182,71 @@ values are mapped to their canonical HTTP status codes (full 16-code table below --- +### Request binding (`Bind` / `BindEmpty`) + +`Handle` and friends fill `Req` from the **JSON body only**. A route with an +identifier or a filter needs more, and hand-rolling the parse via `HandlerFunc` is +the one path that reaches production with **no validation**. `Bind` closes that gap: +each field declares its source with a struct tag — `path:`, `query:` or `json:` — +and the assembled struct is validated once by the same `valid.Validator`. + +```go +type listRolesReq struct { + Page int `query:"page" default:"1" validate:"min=1"` + PerPage int `query:"per_page" default:"50" validate:"min=1,max=200"` + Q string `query:"q" validate:"omitempty,max=100"` + Kind []string `query:"kind" validate:"omitempty,dive,oneof=POS KDS"` +} + +// GET /roles?page=2&per_page=50&q=turno&kind=POS&kind=KDS → all validated, 200. +srv.Get("/roles", httputil.Bind(v, logger, func(ctx context.Context, req listRolesReq) (ListRes, error) { + return roleService.List(ctx, req) +})) + +type updateRoleReq struct { + RoleID uuid.UUID `path:"roleID" validate:"required"` // from the path, already typed + Name string `json:"name" validate:"omitempty,max=200"` // from the body +} + +// PATCH /roles/{roleID} — path + body in one struct; WithStatus still applies. +srv.Patch("/roles/{roleID}", httputil.Bind(v, logger, func(ctx context.Context, req updateRoleReq) (RoleRes, error) { + return roleService.Update(ctx, req) +})) + +type deleteRoleReq struct { + RoleID uuid.UUID `path:"roleID" validate:"required"` +} + +// DELETE /roles/{roleID} — path only, no body. BindEmpty writes 204 and does not +// fail on the empty body the way HandleEmpty would. +srv.Delete("/roles/{roleID}", httputil.BindEmpty(v, logger, func(ctx context.Context, req deleteRoleReq) error { + return roleService.Delete(ctx, req.RoleID) +})) +``` + +Binding rules: + +- **One source per field.** A field carries at most one of `path:` / `query:` / + `json:`; declaring two fails at **wiring** (the service does not boot). +- **Conversion** covers `string`, the sized integer/unsigned/float types, `bool`, + and any type whose pointer implements `encoding.TextUnmarshaler` — so `uuid.UUID` + and `time.Time` bind with no special-casing and no new import in `web`. +- **A malformed value is a 400** naming the parameter (`ErrInvalidInput`), never a 500. +- **`default:` applies only when the parameter is absent** — a present-but-empty + `?q=` is the caller clearing a filter and is left as the zero value. Pair `default:` + with a plain `min=1` (not `omitempty,min=1`): the default guarantees presence, and + `omitempty` would let an explicit `?page=0` skip the bound. +- **Repeated query parameters bind to a slice** (`?kind=POS&kind=KDS` → `[]string{…}`); + a comma inside a single value is **not** split. +- **No body is not an error** — a bodiless `GET`/`DELETE` binds path/query directly. +- The struct is reflected over **once per type and cached**; per-request work does + not re-parse tags. + +`HandlerFunc` remains for genuinely custom responses (streaming, file downloads, +non-JSON) — not for parameters. + +--- + ### Health endpoint ```go diff --git a/docs/adr/ADR-001-request-binding.md b/docs/adr/ADR-001-request-binding.md new file mode 100644 index 0000000..239410d --- /dev/null +++ b/docs/adr/ADR-001-request-binding.md @@ -0,0 +1,109 @@ +# ADR-001 — `httputil` request binding: `Bind` and `BindEmpty` + +**Status:** Accepted +**Date:** 2026-08-13 +**Module:** `web` (`httputil`) +**Shipped:** v1.6.0 + +## Context + +The typed adapters (`Handle`, `HandleNoBody`, `HandleEmpty`) keep +`http.ResponseWriter` and `*http.Request` out of business handlers so that decode, +validation, encoding, status selection and error mapping happen once inside the +framework. But they define a handler's input as **the JSON body and nothing else**. +An HTTP request carries four input channels; only the body was reachable from a +typed handler: + +| Channel | Before v1.6.0 | +|---|---| +| JSON body | decoded into `Req`, validated | +| Path parameter | only via `chi.URLParamFromCtx(ctx, …)` — untyped `string`, unvalidated, off the handler signature | +| Query parameter | unreachable — the adapters never pass `r.URL` through | +| Header | out of scope by design (middleware's concern) | + +So the two most ordinary REST shapes — `GET /roles/{id}` and +`GET /roles?page=2&per_page=50` — had to abandon the typed adapters for +`HandlerFunc` and hand-write the decode, the validation call, the encoding and the +status. This is a **correctness** problem, not only ergonomics: `HandlerFunc` is the +single path by which a handler reaches production without `v.Struct(req)` ever +running. Two failure modes followed, both observed in a consumer (`kch-core-svc`): + +1. **Unvalidated bounds** — a list endpoint that forgets to clamp answers + `?per_page=99999`; the validator that would refuse it is not in the code path. +2. **Client mistakes reported as server faults** — a hand-written + `strconv.Atoi(...)` whose error is wrapped as internal answers **500** for a + plain **400**, misclassifying a client error as an outage. + +A third, smaller trap: `HandleEmpty` decodes a body unconditionally, so a bodiless +`DELETE` fails on `io.EOF` before the handler runs. + +## Decision + +Add a fourth adapter family, `Bind` and `BindEmpty`, that fills `Req` from path, +query **and** body — each field declaring its source with a struct tag — and +validates the assembled struct once with the `valid.Validator` already in scope. +**The handler signature does not change**; only what `Req` may be filled from does. +`JSON`, `NoContent`, `Error` and `WithStatus` are reused unmodified. + +### Binding rules + +1. **One source per field.** A field carries at most one of `path:`/`query:`/`json:`. + Two source tags is a programming error, detected when the type is first reflected + over and reported as a **wiring failure at startup**, not per request. +2. **No body is not an error.** An empty body (`GET`, `DELETE`, `Content-Length: 0`) + decodes to `io.EOF`, which is treated as "no body" — retiring the `HandleEmpty` + bodiless trap. +3. **Conversion** covers `string`, the sized integer/unsigned/float types, `bool`, + and anything whose pointer implements `encoding.TextUnmarshaler`. That one + interface is the whole extensibility story: `uuid.UUID` and `time.Time` bind with + no special-casing and no new dependency in `web`. +4. **A conversion failure is `ErrInvalidInput`, naming the parameter** — never + `ErrInternal`. This turns failure mode 2 from a 500 into the 400 it always was. +5. **`default:` applies only when a parameter is absent** — never when present and + empty, because `?q=` is a caller deliberately clearing a filter. (Pair `default:` + with a plain `min=1`, not `omitempty,min=1`: the default guarantees presence, and + `omitempty` would let an explicit `?page=0` skip the bound.) +6. **Repeated query parameters bind to a slice.** A comma inside a single value is + **not** split — one syntax, so a value legitimately containing a comma survives. +7. **Metadata is parsed once per type and cached**, as `encoding/json` does. A + startup benchmark keeps the per-request cost flat in the number of tagged fields. + +### Out of scope: header binding + +There is no `header:` tag, in this version or a later one. Headers are middleware's +concern (authentication, request identity, tenancy). A `header:` tag would make one +specific mistake ergonomic — filling a tenant/actor identifier from a value the +client fully controls — which `kch-core-svc`'s own ADR-007 forbids. Reducing that +mistake to one word in a struct tag would make it likely rather than merely possible. + +## Options considered + +- **Fourth adapter family** *(chosen)* — additive, one concept, existing call sites + untouched. +- **Extend the existing three** — smallest diff, but `HandleNoBody` would need a + `Req` type parameter it does not have: a breaking signature change to the + most-used adapter, for the benefit of routes that could equally call something new. +- **One adapter per channel combination** (`HandleQuery`, `HandlePathBody`, …) — + eight exported functions expressing one idea; the caller must pick correctly each + time. +- **An `Option`** (`Handle(v, logger, fn, WithBinding())`) — leaves two ways to + express one thing, and `Option` currently means "adjust the response", not "change + how the request is read". +- **Leave it to `HandlerFunc`** — the status quo, and the only route to production + without validation. REST resources with identifiers are not an edge case. + +## Consequences + +- **Purely additive.** `Handle`, `HandleNoBody`, `HandleEmpty` and `HandlerFunc` are + behaviourally identical to v1.5.0; adoption is per route and per service. +- **`HandlerFunc`'s doc comment is amended** — it stops advertising itself for path + parameters and remains the answer for genuinely custom responses (streaming, file + downloads, non-JSON content types). +- **Routing coupling is acknowledged, not abstracted.** Path binding asks chi for a + named parameter, so `httputil` imports `chi/v5` directly (it was already a `web` + module dependency). `web/server` is chi and does not pretend to be swappable; an + indirection layer nothing else uses would cost more than it buys. +- **Reflection enters `httputil`** (a package that previously did none), mitigated by + the per-type cache and kept honest by the benchmark. +- **A new failure mode at startup, by design** — a mis-tagged struct fails the + service at boot rather than on the first request that exercises it. diff --git a/docs/adr/index.md b/docs/adr/index.md index bfe93f1..d9d725e 100644 --- a/docs/adr/index.md +++ b/docs/adr/index.md @@ -9,6 +9,12 @@ No module-level ADRs for v1.0.0 — all design decisions were consistent with existing framework principles (ADR-001 through ADR-003 from `core`, framework ADRs 001–004 from `docs`). No contested choices required a record. +Module ADRs: + +| ADR | Title | Shipped | +|---|---|---| +| [ADR-001](ADR-001-request-binding.md) | `httputil` request binding — `Bind` / `BindEmpty` | v1.6.0 | + Decisions worth noting (not ADR-worthy individually): | Decision | Outcome | Rationale | @@ -21,3 +27,4 @@ Decisions worth noting (not ADR-worthy individually): | Background goroutine for in-memory eviction | `time.Ticker` goroutine | Avoids `worker` module dependency; in-memory store is self-contained | | `mw.RequestIDFrom` resolver sees the request (v1.4.0) | New entry point takes `func(*http.Request) string`; `RequestID` becomes a request-ignoring wrapper over it | An inbound correlation id must be able to survive this boundary, but acceptability is per-service — a typed audit column rejects what an opaque log accepts. The framework provides plumbing only (context + header, once per request); the app owns policy (which header, validation, generation fallback). An empty resolver result attaches nothing rather than a silently-empty value | | `httputil` success status is configurable (v1.5.0) | `Handle`/`HandleNoBody`/`HandleEmpty` take `opts ...Option`; `WithStatus(code)` overrides the default (200 / 200 / 204) | 201 Created / 202 Accepted are common and were only reachable by hand-rolling the handler (losing decode+validate+error-mapping). Variadic options are non-breaking and future-extensible (headers, etc.). `WithStatus` is success-only (2xx) and panics at wiring on a non-2xx code — a wrong status is a routing mistake that should fail to boot, not surface at runtime; error status stays separate, resolved from the xerror by `Error` | +| `httputil` request binding (v1.6.0) | `Bind`/`BindEmpty` fill `Req` from `path:`/`query:`/`json:` tags and validate once; see [ADR-001](ADR-001-request-binding.md) | Path/query were only reachable via `HandlerFunc`, the one route to production with no validation (unvalidated bounds; client mistakes as 500s). `encoding.TextUnmarshaler` is the whole extensibility story (uuid/time, no new dep). Mis-tagged struct panics at wiring. Purely additive | diff --git a/go.mod b/go.mod index 911bcf1..91bd82e 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,8 @@ module code.nochebuena.dev/einherjar/web go 1.26 require ( - code.nochebuena.dev/einherjar/contracts v1.5.0 - code.nochebuena.dev/einherjar/core v1.5.0 + code.nochebuena.dev/einherjar/contracts v1.6.0 + code.nochebuena.dev/einherjar/core v1.6.0 github.com/go-chi/chi/v5 v5.3.1 github.com/google/uuid v1.6.0 golang.org/x/time v0.15.0 diff --git a/go.sum b/go.sum index daab723..a41516d 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,7 @@ -code.nochebuena.dev/einherjar/contracts v1.5.0 h1:vDlpLXtVZ4Q4l3AR02qLQhnKnEDq2vE8+mydwg85hUU= -code.nochebuena.dev/einherjar/contracts v1.5.0/go.mod h1:ccltUtrFb5+MEJdkx2VVEUL+xC5pupVlVVsMM8AlCWI= -code.nochebuena.dev/einherjar/core v1.5.0 h1:LXVyHaite+NHL8LIGj/NvCVqgCrkpVMfm2VaIZ9aAU8= -code.nochebuena.dev/einherjar/core v1.5.0/go.mod h1:lxiRdCVLl1/XnbCsHbeZJIFNXZ29QaSl2lMGZm3CQjM= +code.nochebuena.dev/einherjar/contracts v1.6.0 h1:Y+8B+m4kQR5l/6lMY7SYdvIbwhFOnliCXDv9oBCOUP4= +code.nochebuena.dev/einherjar/contracts v1.6.0/go.mod h1:ccltUtrFb5+MEJdkx2VVEUL+xC5pupVlVVsMM8AlCWI= +code.nochebuena.dev/einherjar/core v1.6.0 h1:6cQIYZliw0hcuk7Cy44swleLC1Ch8WFxTkkHsGxkAFk= +code.nochebuena.dev/einherjar/core v1.6.0/go.mod h1:azRKvJBtMWGp8jhveG1fZc9jlPTwXu8clvBdXIjTB9k= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/gabriel-vasile/mimetype v1.4.15 h1:05iP/CYtZ/w455R/KZM6rZ5ieAdh99UPtd+d3YzLmaI= diff --git a/httputil/bind.go b/httputil/bind.go new file mode 100644 index 0000000..6dfa14d --- /dev/null +++ b/httputil/bind.go @@ -0,0 +1,325 @@ +package httputil + +import ( + "context" + "encoding" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "reflect" + "strconv" + "sync" + + "github.com/go-chi/chi/v5" + + "code.nochebuena.dev/einherjar/contracts/logging" + "code.nochebuena.dev/einherjar/core/valid" + "code.nochebuena.dev/einherjar/core/xerrors" +) + +// Bind adapts a typed business function whose request is assembled from more than +// the JSON body. Each field of Req declares its source with a struct tag: +// +// type updateRoleRequest struct { +// RoleID uuid.UUID `path:"roleID" validate:"required"` +// Page int `query:"page" default:"1" validate:"min=1"` +// Name string `json:"name" validate:"omitempty,max=200"` +// } +// +// - path: fills from a chi route parameter (r.Context()). +// - query: fills from the URL query string; a repeated parameter binds to a slice. +// - json: fills from the JSON body (standard encoding/json). +// +// It then validates the assembled struct once with v and calls fn — the handler +// signature is identical to [Handle]. On success Res is encoded as JSON (200 by +// default, or the [WithStatus] code). On error it flows through [Error]. +// +// Conversion covers string, the sized integer/unsigned/float types, bool, and any +// type whose pointer implements [encoding.TextUnmarshaler] (so uuid.UUID and +// time.Time bind with no special-casing). A value that fails to convert is +// reported as [xerrors.ErrInvalidInput] naming the parameter — a 400, never a 500. +// +// The struct is reflected over once per type at wiring time and the result cached. +// A field declaring more than one source tag, an unsupported field type, or a +// default: that is not a valid value for its field all panic at wiring, so a +// mis-tagged struct fails the service at boot rather than on a request. +func Bind[Req, Res any](v valid.Validator, logger logging.Logger, fn func(ctx context.Context, req Req) (Res, error), opts ...Option) http.HandlerFunc { + status := resolveStatus(http.StatusOK, opts) + plan := planFor(reflect.TypeOf((*Req)(nil)).Elem()) + return func(w http.ResponseWriter, r *http.Request) { + var req Req + if err := bindRequest(plan, v, r, &req); err != nil { + Error(logger, w, r, err) + return + } + res, err := fn(r.Context(), req) + if err != nil { + Error(logger, w, r, err) + return + } + JSON(w, status, res) + } +} + +// BindEmpty is [Bind] for a function that returns no response body. Req is filled +// from path, query and body exactly as in Bind; on success a body-less status is +// written (204 by default, or the [WithStatus] code). +// +// Unlike [HandleEmpty] it does not require a request body: a bodiless request +// (GET, DELETE, Content-Length: 0) is not an error, so a DELETE /resource/{id} +// with a path: tag binds directly instead of failing on io.EOF. +func BindEmpty[Req any](v valid.Validator, logger logging.Logger, fn func(ctx context.Context, req Req) error, opts ...Option) http.HandlerFunc { + status := resolveStatus(http.StatusNoContent, opts) + plan := planFor(reflect.TypeOf((*Req)(nil)).Elem()) + return func(w http.ResponseWriter, r *http.Request) { + var req Req + if err := bindRequest(plan, v, r, &req); err != nil { + Error(logger, w, r, err) + return + } + if err := fn(r.Context(), req); err != nil { + Error(logger, w, r, err) + return + } + w.WriteHeader(status) + } +} + +// bindRequest decodes the body (when present), overlays path/query fields, and +// validates the assembled struct once. dst must be a pointer to Req. +func bindRequest(plan *bindPlan, v valid.Validator, r *http.Request, dst any) error { + // A body is optional: an empty body decodes to io.EOF, which we treat as + // "no body" rather than an error (retires the HandleEmpty bodiless trap). + if r.Body != nil { + if err := json.NewDecoder(r.Body).Decode(dst); err != nil && !errors.Is(err, io.EOF) { + return xerrors.New(xerrors.ErrInvalidInput, "invalid JSON: "+err.Error()) + } + } + if err := plan.apply(r, reflect.ValueOf(dst).Elem()); err != nil { + return err + } + if err := v.Struct(reflect.ValueOf(dst).Elem().Interface()); err != nil { + return err + } + return nil +} + +// --- binding plan (reflected once per type, cached) --- + +type sourceKind uint8 + +const ( + sourcePath sourceKind = iota + sourceQuery +) + +// fieldBind describes how one path/query field is filled. json/untagged fields +// are handled by the body decoder and never appear here. +type fieldBind struct { + index int + name string + source sourceKind + isSlice bool + hasDefault bool + defaultVal string +} + +type bindPlan struct { + fields []fieldBind +} + +var planCache sync.Map // reflect.Type -> *bindPlan + +// planFor returns the cached plan for t, building (and validating) it once. It +// panics on a mis-tagged struct, so callers reach it at wiring time and the +// service fails to boot rather than at request time. +func planFor(t reflect.Type) *bindPlan { + if cached, ok := planCache.Load(t); ok { + return cached.(*bindPlan) + } + p := buildPlan(t) + actual, _ := planCache.LoadOrStore(t, p) + return actual.(*bindPlan) +} + +func buildPlan(t reflect.Type) *bindPlan { + if t.Kind() != reflect.Struct { + panic(fmt.Sprintf("httputil.Bind: Req must be a struct, got %s", t)) + } + p := &bindPlan{} + for i := 0; i < t.NumField(); i++ { + f := t.Field(i) + pathTag, hasPath := f.Tag.Lookup("path") + queryTag, hasQuery := f.Tag.Lookup("query") + _, hasJSON := f.Tag.Lookup("json") + + n := 0 + for _, ok := range []bool{hasPath, hasQuery, hasJSON} { + if ok { + n++ + } + } + if n > 1 { + panic(fmt.Sprintf("httputil.Bind: field %s.%s declares more than one source tag (path/query/json); a field binds from exactly one channel", t.Name(), f.Name)) + } + if !hasPath && !hasQuery { + continue // json or untagged — the body decoder owns it + } + + fb := fieldBind{index: i} + if hasPath { + fb.source, fb.name = sourcePath, pathTag + } else { + fb.source, fb.name = sourceQuery, queryTag + } + + ft := f.Type + fb.isSlice = ft.Kind() == reflect.Slice && !implementsTextUnmarshaler(ft) + if fb.isSlice && fb.source == sourcePath { + panic(fmt.Sprintf("httputil.Bind: field %s.%s is a path parameter and cannot be a slice", t.Name(), f.Name)) + } + + elem := ft + if fb.isSlice { + elem = ft.Elem() + } + if !convertible(elem) { + panic(fmt.Sprintf("httputil.Bind: field %s.%s has unsupported type %s (want string, integer, float, bool, or encoding.TextUnmarshaler)", t.Name(), f.Name, ft)) + } + + if dv, ok := f.Tag.Lookup("default"); ok { + fb.hasDefault, fb.defaultVal = true, dv + // A default that cannot convert is a wiring mistake — fail at boot. + if err := setScalar(reflect.New(elem).Elem(), dv); err != nil { + panic(fmt.Sprintf("httputil.Bind: field %s.%s default %q is not a valid %s: %v", t.Name(), f.Name, dv, elem, err)) + } + } + + p.fields = append(p.fields, fb) + } + return p +} + +// apply overlays the path/query fields onto an already body-decoded struct value. +func (p *bindPlan) apply(r *http.Request, sv reflect.Value) error { + var query map[string][]string + for _, fb := range p.fields { + var raw []string + present := false + + switch fb.source { + case sourcePath: + if v := chi.URLParamFromCtx(r.Context(), fb.name); v != "" { + raw, present = []string{v}, true + } + case sourceQuery: + if query == nil { + query = r.URL.Query() + } + if vs, ok := query[fb.name]; ok { + raw, present = vs, true + } + } + + field := sv.Field(fb.index) + if !present { + // Absent: apply the default if declared, else leave the zero value. + // A present-but-empty value (?q=) is *not* absent and skips this. + if fb.hasDefault { + if err := setScalar(field, fb.defaultVal); err != nil { + return xerrors.New(xerrors.ErrInvalidInput, fmt.Sprintf("invalid %s: %v", fb.name, err)) + } + } + continue + } + + if fb.isSlice { + slice := reflect.MakeSlice(field.Type(), len(raw), len(raw)) + for i, s := range raw { + if err := setScalar(slice.Index(i), s); err != nil { + return xerrors.New(xerrors.ErrInvalidInput, fmt.Sprintf("invalid %s: %v", fb.name, err)) + } + } + field.Set(slice) + continue + } + + if err := setScalar(field, raw[0]); err != nil { + return xerrors.New(xerrors.ErrInvalidInput, fmt.Sprintf("invalid %s: %v", fb.name, err)) + } + } + return nil +} + +// --- conversion --- + +var textUnmarshalerType = reflect.TypeOf((*encoding.TextUnmarshaler)(nil)).Elem() + +func implementsTextUnmarshaler(t reflect.Type) bool { + return reflect.PointerTo(t).Implements(textUnmarshalerType) +} + +// convertible reports whether a single value of type t can be set from a string. +func convertible(t reflect.Type) bool { + if implementsTextUnmarshaler(t) { + return true + } + switch t.Kind() { + case reflect.String, + reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, + reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, + reflect.Float32, reflect.Float64, + reflect.Bool: + return true + default: + return false + } +} + +// setScalar sets one addressable value from its string form. TextUnmarshaler is +// preferred so uuid.UUID / time.Time bind through their own parsing. +func setScalar(field reflect.Value, s string) error { + if field.CanAddr() { + if u, ok := field.Addr().Interface().(encoding.TextUnmarshaler); ok { + return u.UnmarshalText([]byte(s)) + } + } + switch field.Kind() { + case reflect.String: + field.SetString(s) + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + n, err := strconv.ParseInt(s, 10, field.Type().Bits()) + if err != nil { + return errNumeric(s, "integer") + } + field.SetInt(n) + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + n, err := strconv.ParseUint(s, 10, field.Type().Bits()) + if err != nil { + return errNumeric(s, "unsigned integer") + } + field.SetUint(n) + case reflect.Float32, reflect.Float64: + n, err := strconv.ParseFloat(s, field.Type().Bits()) + if err != nil { + return errNumeric(s, "number") + } + field.SetFloat(n) + case reflect.Bool: + b, err := strconv.ParseBool(s) + if err != nil { + return fmt.Errorf("%q is not a boolean", s) + } + field.SetBool(b) + default: + // Unreachable: buildPlan rejects unsupported types at wiring time. + return fmt.Errorf("unsupported type %s", field.Type()) + } + return nil +} + +func errNumeric(s, kind string) error { + return fmt.Errorf("%q is not a valid %s", s, kind) +} diff --git a/httputil/bind_test.go b/httputil/bind_test.go new file mode 100644 index 0000000..a321435 --- /dev/null +++ b/httputil/bind_test.go @@ -0,0 +1,343 @@ +package httputil + +import ( + "context" + "net/http" + "net/http/httptest" + "reflect" + "strings" + "testing" + "time" + + "github.com/go-chi/chi/v5" + "github.com/google/uuid" + + "code.nochebuena.dev/einherjar/core/valid" +) + +// withPath attaches a chi route context carrying the given key/value path params, +// mirroring what the router injects before a handler runs. +func withPath(r *http.Request, kv ...string) *http.Request { + rctx := chi.NewRouteContext() + for i := 0; i+1 < len(kv); i += 2 { + rctx.URLParams.Add(kv[i], kv[i+1]) + } + return r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx)) +} + +// AC1 — Bind fills path, query and json fields on one struct. +func TestBind_FillsAllThreeSources(t *testing.T) { + type req struct { + RoleID string `path:"roleID"` + Page int `query:"page"` + Name string `json:"name"` + } + var got req + h := Bind(valid.New(), discardLogger(), func(_ context.Context, r req) (tRes, error) { + got = r + return tRes{ID: r.RoleID}, nil + }) + rec := httptest.NewRecorder() + r := withPath(httptest.NewRequest(http.MethodPatch, "/roles/abc?page=7", strings.NewReader(`{"name":"turno"}`)), "roleID", "abc") + h(rec, r) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body: %s)", rec.Code, rec.Body.String()) + } + if got.RoleID != "abc" || got.Page != 7 || got.Name != "turno" { + t.Fatalf("bound = %+v, want {abc 7 turno}", got) + } +} + +// AC1 + AC3 — BindEmpty writes a body-less success and needs no request body. +func TestBindEmpty_PathOnly_NoBody(t *testing.T) { + type req struct { + RoleID string `path:"roleID"` + } + called := "" + h := BindEmpty(valid.New(), discardLogger(), func(_ context.Context, r req) error { + called = r.RoleID + return nil + }) + rec := httptest.NewRecorder() + // DELETE with a nil body — the HandleEmpty io.EOF trap must not fire. + h(rec, withPath(httptest.NewRequest(http.MethodDelete, "/roles/xyz", nil), "roleID", "xyz")) + + if rec.Code != http.StatusNoContent { + t.Fatalf("status = %d, want 204", rec.Code) + } + if rec.Body.Len() != 0 { + t.Errorf("expected empty body, got %q", rec.Body.String()) + } + if called != "xyz" { + t.Errorf("path not bound: got %q", called) + } +} + +// AC3 — a bodiless GET succeeds through Bind (query only, no io.EOF). +func TestBind_NoBody_QueryOnly(t *testing.T) { + type req struct { + Q string `query:"q"` + } + h := Bind(valid.New(), discardLogger(), func(_ context.Context, r req) (tRes, error) { + return tRes{ID: r.Q}, nil + }) + rec := httptest.NewRecorder() + h(rec, httptest.NewRequest(http.MethodGet, "/roles?q=hola", nil)) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body: %s)", rec.Code, rec.Body.String()) + } + if got := strings.TrimSpace(rec.Body.String()); got != `{"id":"hola"}` { + t.Errorf("body = %q", got) + } +} + +// AC2 — a field with two source tags fails at wiring (Bind panics at registration). +func TestBind_TwoSourceTags_PanicsAtWiring(t *testing.T) { + type req struct { + Bad string `path:"id" query:"id"` + } + defer func() { + if recover() == nil { + t.Fatal("Bind did not panic on a two-source-tag field") + } + }() + _ = Bind(valid.New(), discardLogger(), func(_ context.Context, _ req) (tRes, error) { + return tRes{}, nil + }) +} + +// AC2 (companion) — an unsupported field type and a bad default also fail at wiring. +func TestBind_UnsupportedType_PanicsAtWiring(t *testing.T) { + type req struct { + Ch chan int `query:"ch"` + } + defer func() { + if recover() == nil { + t.Fatal("Bind did not panic on an unsupported field type") + } + }() + _ = Bind(valid.New(), discardLogger(), func(_ context.Context, _ req) (tRes, error) { return tRes{}, nil }) +} + +func TestBind_BadDefault_PanicsAtWiring(t *testing.T) { + type req struct { + Page int `query:"page" default:"not-a-number"` + } + defer func() { + if recover() == nil { + t.Fatal("Bind did not panic on an invalid default tag") + } + }() + _ = Bind(valid.New(), discardLogger(), func(_ context.Context, _ req) (tRes, error) { return tRes{}, nil }) +} + +// AC4 — uuid.UUID and time.Time bind from path and query via TextUnmarshaler. +func TestBind_TextUnmarshaler_UUIDAndTime(t *testing.T) { + type req struct { + ID uuid.UUID `path:"id"` + From time.Time `query:"from"` + } + id := uuid.New() + var got req + h := Bind(valid.New(), discardLogger(), func(_ context.Context, r req) (tRes, error) { + got = r + return tRes{ID: r.ID.String()}, nil + }) + rec := httptest.NewRecorder() + r := withPath(httptest.NewRequest(http.MethodGet, "/x/"+id.String()+"?from=2026-01-02T03:04:05Z", nil), "id", id.String()) + h(rec, r) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body: %s)", rec.Code, rec.Body.String()) + } + if got.ID != id { + t.Errorf("uuid = %s, want %s", got.ID, id) + } + if !got.From.Equal(time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC)) { + t.Errorf("time = %s, want 2026-01-02T03:04:05Z", got.From) + } +} + +// AC5 — a malformed value answers 400 and names the parameter, never 500. +func TestBind_MalformedParam_400WithName(t *testing.T) { + type req struct { + Page int `query:"page"` + } + h := Bind(valid.New(), discardLogger(), func(_ context.Context, _ req) (tRes, error) { + return tRes{}, nil + }) + rec := httptest.NewRecorder() + h(rec, httptest.NewRequest(http.MethodGet, "/roles?page=abc", nil)) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rec.Code) + } + if !strings.Contains(rec.Body.String(), "page") { + t.Errorf("error body %q does not name the parameter", rec.Body.String()) + } +} + +// AC4/AC5 — a malformed uuid path parameter is also a 400, not a 500. +func TestBind_MalformedUUID_400(t *testing.T) { + type req struct { + ID uuid.UUID `path:"id"` + } + h := Bind(valid.New(), discardLogger(), func(_ context.Context, _ req) (tRes, error) { + return tRes{}, nil + }) + rec := httptest.NewRecorder() + h(rec, withPath(httptest.NewRequest(http.MethodGet, "/x/nope", nil), "id", "nope")) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rec.Code) + } +} + +// AC6 — default applies when the parameter is absent, and NOT when present-and-empty. +func TestBind_Default_AbsentOnly(t *testing.T) { + type req struct { + Page int `query:"page" default:"1"` + Q string `query:"q" default:"all"` + } + + // Absent → defaults applied. + var absent req + h := Bind(valid.New(), discardLogger(), func(_ context.Context, r req) (tRes, error) { + absent = r + return tRes{}, nil + }) + h(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/roles", nil)) + if absent.Page != 1 || absent.Q != "all" { + t.Fatalf("absent defaults = %+v, want {1 all}", absent) + } + + // Present-but-empty (?q=) → the caller is clearing the filter; default must NOT win. + var present req + h2 := Bind(valid.New(), discardLogger(), func(_ context.Context, r req) (tRes, error) { + present = r + return tRes{}, nil + }) + rec := httptest.NewRecorder() + h2(rec, httptest.NewRequest(http.MethodGet, "/roles?q=", nil)) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body: %s)", rec.Code, rec.Body.String()) + } + if present.Q != "" { + t.Errorf("present-empty q = %q, want \"\" (default must not override)", present.Q) + } +} + +// AC7 — repeated query parameters bind to a slice; a comma inside a scalar survives. +func TestBind_RepeatedQuery_Slice(t *testing.T) { + type req struct { + Kind []string `query:"kind"` + Q string `query:"q"` + } + var got req + h := Bind(valid.New(), discardLogger(), func(_ context.Context, r req) (tRes, error) { + got = r + return tRes{}, nil + }) + rec := httptest.NewRecorder() + h(rec, httptest.NewRequest(http.MethodGet, "/x?kind=POS&kind=KDS&q=a,b,c", nil)) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body: %s)", rec.Code, rec.Body.String()) + } + if len(got.Kind) != 2 || got.Kind[0] != "POS" || got.Kind[1] != "KDS" { + t.Errorf("kind = %v, want [POS KDS]", got.Kind) + } + if got.Q != "a,b,c" { + t.Errorf("q = %q, want verbatim a,b,c (no comma splitting)", got.Q) + } +} + +// AC7 (companion) — a typed slice ([]uuid.UUID) binds each repeated value. +func TestBind_RepeatedQuery_TypedSlice(t *testing.T) { + type req struct { + IDs []uuid.UUID `query:"id"` + } + a, b := uuid.New(), uuid.New() + var got req + h := Bind(valid.New(), discardLogger(), func(_ context.Context, r req) (tRes, error) { + got = r + return tRes{}, nil + }) + rec := httptest.NewRecorder() + h(rec, httptest.NewRequest(http.MethodGet, "/x?id="+a.String()+"&id="+b.String(), nil)) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body: %s)", rec.Code, rec.Body.String()) + } + if len(got.IDs) != 2 || got.IDs[0] != a || got.IDs[1] != b { + t.Errorf("ids = %v, want [%s %s]", got.IDs, a, b) + } +} + +// WithStatus composes with Bind exactly as with Handle (201 on create). +func TestBind_WithStatus(t *testing.T) { + type req struct { + Name string `json:"name" validate:"required"` + } + h := Bind(valid.New(), discardLogger(), func(_ context.Context, r req) (tRes, error) { + return tRes{ID: r.Name}, nil + }, WithStatus(http.StatusCreated)) + rec := httptest.NewRecorder() + h(rec, httptest.NewRequest(http.MethodPost, "/roles", strings.NewReader(`{"name":"cajero"}`))) + if rec.Code != http.StatusCreated { + t.Fatalf("status = %d, want 201", rec.Code) + } +} + +// Validation runs on the assembled struct — a query bound value is validated too. +func TestBind_ValidatesAssembledStruct(t *testing.T) { + type req struct { + PerPage int `query:"per_page" default:"50" validate:"min=1,max=200"` + } + h := Bind(valid.New(), discardLogger(), func(_ context.Context, _ req) (tRes, error) { + return tRes{}, nil + }) + rec := httptest.NewRecorder() + h(rec, httptest.NewRequest(http.MethodGet, "/roles?per_page=99999", nil)) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (per_page over max should fail validation)", rec.Code) + } +} + +// AC9 — the per-type plan is reflected once and cached (same pointer across calls). +func TestBind_PlanCachedPerType(t *testing.T) { + type req struct { + A string `query:"a"` + B int `query:"b"` + } + rt := reflect.TypeOf((*req)(nil)).Elem() + if planFor(rt) != planFor(rt) { + t.Fatal("planFor returned a different plan for the same type — cache not effective") + } +} + +// AC9 — per-request work does not re-reflect the type; the plan metadata is +// parsed once and reused. Run with -benchmem to see allocations stay flat +// regardless of how many tagged fields the struct declares. +func BenchmarkBind_ManyFields(b *testing.B) { + type req struct { + ID uuid.UUID `path:"id"` + Page int `query:"page" default:"1"` + PerPage int `query:"per_page" default:"50"` + Q string `query:"q"` + Sort string `query:"sort" default:"name"` + Order string `query:"order" default:"asc"` + Kind []string `query:"kind"` + } + id := uuid.New() + h := Bind(valid.New(), discardLogger(), func(_ context.Context, _ req) (tRes, error) { + return tRes{}, nil + }) + target := "/x/" + id.String() + "?page=2&per_page=25&q=turno&sort=name&order=desc&kind=POS&kind=KDS" + + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + rec := httptest.NewRecorder() + h(rec, withPath(httptest.NewRequest(http.MethodGet, target, nil), "id", id.String())) + } +} diff --git a/httputil/doc.go b/httputil/doc.go index 9d1b54d..c55ceaf 100644 --- a/httputil/doc.go +++ b/httputil/doc.go @@ -22,6 +22,26 @@ // return CreateUserRes{ID: u.ID}, nil // })) // +// # Request binding +// +// [Bind] and [BindEmpty] extend the same decode → validate → call → encode pipeline +// to requests that carry more than a JSON body. Each field declares its source with +// a struct tag — path:, query: or json: — and the assembled struct is validated once: +// +// type getRoleReq struct { +// RoleID uuid.UUID `path:"roleID" validate:"required"` +// Expand []string `query:"expand"` +// } +// +// r.Get("/roles/{roleID}", httputil.Bind(v, logger, func(ctx context.Context, req getRoleReq) (RoleRes, error) { +// return svc.GetRole(ctx, req.RoleID) +// })) +// +// A malformed value is a 400 naming the parameter (never a 500), uuid.UUID and +// time.Time bind via [encoding.TextUnmarshaler], and a mis-tagged struct fails at +// wiring rather than on a request. Use these instead of [HandlerFunc] for any route +// with an identifier or a filter. +// // # Centralized error handler // // [Error] is the single point of error processing for all handlers: @@ -29,16 +49,16 @@ // - 4xx → Warn level (client mistake — not a server failure) // - 499 → Info level (client cancelled the request intentionally) // -// Call it directly from [HandlerFunc] when you need path parameters or custom logic: +// Call it directly from [HandlerFunc] for genuinely custom responses — streaming, +// file downloads, non-JSON content types: // -// r.Get("/users/{id}", httputil.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { -// id := chi.URLParam(r, "id") -// u, err := svc.GetUser(r.Context(), id) +// r.Get("/reports/{id}.csv", httputil.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { +// rows, err := svc.Export(r.Context(), chi.URLParam(r, "id")) // if err != nil { // httputil.Error(logger, w, r, err) // return nil // } -// httputil.JSON(w, http.StatusOK, u) -// return nil +// w.Header().Set("Content-Type", "text/csv") +// return csv.NewWriter(w).WriteAll(rows) // }).ServeHTTP) package httputil diff --git a/httputil/handler_func.go b/httputil/handler_func.go index b0d665a..ca8fa7a 100644 --- a/httputil/handler_func.go +++ b/httputil/handler_func.go @@ -6,7 +6,13 @@ var _ http.Handler = HandlerFunc(nil) // HandlerFunc is an http.Handler that returns an error. // On non-nil error the error is mapped to the appropriate HTTP response via [Error]. -// Use for manual handlers that need path parameters or custom status codes. +// +// Use it for genuinely custom responses — streaming, file downloads, non-JSON +// content types — where the typed adapters do not fit. It is no longer the answer +// for path or query parameters: [Bind] and [BindEmpty] fill those from struct tags +// with the same decode → validate → encode guarantees, and a custom success status +// is set with [WithStatus]. Reaching for HandlerFunc to read a parameter is the one +// path by which a handler reaches production without validation running. type HandlerFunc func(w http.ResponseWriter, r *http.Request) error // ServeHTTP implements http.Handler.