mirror of
https://github.com/zap-proto/zip.git
synced 2026-10-19 15:38:57 +00:00
A repeated argument rides a URL, and the document names what the binder fills
Two halves of one rule, and the second is what made the first safe to rely on. A URL carries one value per name, so a repeated argument is written as one value with commas — style: form, explode: false, the spelling the document already publishes for it. The binder had no case for a slice, so an op taking a list of ids answered 200 about an empty list. Each element is written by setScalar, so a list of ids says nothing about ids a second time, and a []byte is not a list here: encoding/json spells it as one base64 string. urlFields had the kind switch written a SECOND time, and the second one drifted the moment setText taught the binder to read an id. The binder filled a parameter the document did not name, which is the same disagreement as before with the sides swapped — a caller reading the document had no way to know the argument existed. Both now read urlSchema, so what is published and what is bound are one answer. A type that reads itself from text is a string there, whatever it is made of: describing an id by its 32 bytes published an array of integers for a value nothing spells that way.
This commit is contained in:
+12
-10
@@ -120,14 +120,16 @@ func TestBodyless_ExampleSurvivesAsParameterExamples(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A bodyless op's flags are what the URL can actually carry. `--tags` on a
|
||||
// DELETE used to marshal fine, go out as a query value and be dropped by the
|
||||
// binder: a flag the CLI offered and the wire ignored.
|
||||
// A bodyless op's flags are what the URL can actually carry. `--filters` on a
|
||||
// DELETE marshals fine, goes out as a query value and is dropped by the binder:
|
||||
// a flag the CLI offers and the wire ignores. A repeated field is not that case
|
||||
// — it rides as one comma-joined value — but a map has no URL spelling at all.
|
||||
func TestBodyless_FlagsAreWhatTheURLCanCarry(t *testing.T) {
|
||||
type wideIn struct {
|
||||
ID string `json:"id"`
|
||||
Note string `json:"note"`
|
||||
Tags []string `json:"tags"`
|
||||
ID string `json:"id"`
|
||||
Note string `json:"note"`
|
||||
Tags []string `json:"tags"`
|
||||
Filters map[string]string `json:"filters"`
|
||||
}
|
||||
a := zip.New(zip.Config{AppName: "wide", DisableStartupMessage: true})
|
||||
zip.Delete(a, "/v1/wide/things/:id", func(_ context.Context, in *wideIn) (*wideIn, error) { return in, nil })
|
||||
@@ -139,11 +141,11 @@ func TestBodyless_FlagsAreWhatTheURLCanCarry(t *testing.T) {
|
||||
byName[c.Name] = append(byName[c.Name], f.Name)
|
||||
}
|
||||
}
|
||||
if got := byName["things-delete"]; len(got) != 1 || got[0] != "note" {
|
||||
t.Errorf("DELETE flags = %v, want just --note — a slice cannot ride a URL", got)
|
||||
if got := byName["things-delete"]; len(got) != 2 || got[0] != "note" || got[1] != "tags" {
|
||||
t.Errorf("DELETE flags = %v, want --note and --tags — a map cannot ride a URL", got)
|
||||
}
|
||||
if got := byName["things-create"]; len(got) != 3 {
|
||||
t.Errorf("POST flags = %v, want all three — a body carries anything", got)
|
||||
if got := byName["things-create"]; len(got) != 4 {
|
||||
t.Errorf("POST flags = %v, want all four — a body carries anything", got)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+57
-25
@@ -461,12 +461,51 @@ type urlField struct {
|
||||
required bool
|
||||
}
|
||||
|
||||
// urlFields lists the top-level scalar fields of an op's input — the exact set
|
||||
// bindURL can fill, whether the value arrives as a path segment or a query key.
|
||||
// urlSchema is the schema of what a URL carries into a field of type t, and nil
|
||||
// when it carries nothing.
|
||||
//
|
||||
// It is [setScalar]'s rule stated for the document, and the ONE place the two
|
||||
// are held together: the binder fills exactly what this names, so a parameter
|
||||
// that does nothing is never published and one that works is never left out.
|
||||
// The same question was answered by a second kind switch here, and the second
|
||||
// one drifted — an id bound by [setText] was described by neither.
|
||||
//
|
||||
// A type that reads itself from text is a STRING here whatever it is made of.
|
||||
// An id is [32]byte and a time is a struct, and each is one word in the URL and
|
||||
// the same word in the JSON body beside it; schemaOf describes what a value is
|
||||
// MADE of, which for these is not what it looks like.
|
||||
func urlSchema(t reflect.Type) map[string]any {
|
||||
for t.Kind() == reflect.Pointer {
|
||||
t = t.Elem()
|
||||
}
|
||||
if readsText(t) {
|
||||
return map[string]any{"type": "string"}
|
||||
}
|
||||
switch t.Kind() {
|
||||
case reflect.String, reflect.Bool,
|
||||
reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
|
||||
reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64,
|
||||
reflect.Float32, reflect.Float64:
|
||||
return schemaOf(t, nil, nil)
|
||||
case reflect.Slice:
|
||||
// A []byte with no written form of its own is one base64 string, which
|
||||
// the binder does not split and cannot read — so nothing is published.
|
||||
if t.Elem().Kind() == reflect.Uint8 {
|
||||
return nil
|
||||
}
|
||||
if item := urlSchema(t.Elem()); item != nil {
|
||||
return map[string]any{"type": "array", "items": item}
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// urlFields lists the fields of an op's input a URL fills — the exact set
|
||||
// bindURL fills, whether the value arrives as a path segment or a query key.
|
||||
// ONE list serves both because it is ONE binder: a path param and a query param
|
||||
// are the same kind of value, so the document describes them from the same
|
||||
// place. Non-scalars are omitted because the binder cannot fill them either, so
|
||||
// naming them would promise a parameter that silently does nothing.
|
||||
// place. What the binder cannot fill is omitted, because naming it would
|
||||
// promise a parameter that silently does nothing.
|
||||
func urlFields(t reflect.Type) urlFieldList {
|
||||
if t == nil {
|
||||
return nil
|
||||
@@ -483,28 +522,21 @@ func urlFields(t reflect.Type) urlFieldList {
|
||||
if name == "-" {
|
||||
continue // on the wire, but not in the URL.
|
||||
}
|
||||
ft := f.Type
|
||||
for ft.Kind() == reflect.Pointer {
|
||||
ft = ft.Elem()
|
||||
}
|
||||
switch ft.Kind() {
|
||||
case reflect.String, reflect.Bool,
|
||||
reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
|
||||
reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64,
|
||||
reflect.Float32, reflect.Float64:
|
||||
out = append(out, urlField{
|
||||
name: name,
|
||||
field: jsonFieldName(f),
|
||||
schema: schemaOf(ft, nil, nil),
|
||||
// The same `validate:"required"` that makes a body field
|
||||
// required in its schema makes a URL-borne one required in its
|
||||
// parameter. The handler refuses the request either way; a
|
||||
// document that called it optional was describing a call that
|
||||
// cannot succeed, and every generated client made the argument
|
||||
// optional to match.
|
||||
required: strings.Contains(f.Tag.Get("validate"), "required"),
|
||||
})
|
||||
schema := urlSchema(f.Type)
|
||||
if schema == nil {
|
||||
continue // on the wire, but not something a URL carries.
|
||||
}
|
||||
out = append(out, urlField{
|
||||
name: name,
|
||||
field: jsonFieldName(f),
|
||||
schema: schema,
|
||||
// The same `validate:"required"` that makes a body field required in
|
||||
// its schema makes a URL-borne one required in its parameter. The
|
||||
// handler refuses the request either way; a document that called it
|
||||
// optional was describing a call that cannot succeed, and every
|
||||
// generated client made the argument optional to match.
|
||||
required: strings.Contains(f.Tag.Get("validate"), "required"),
|
||||
})
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
+36
-21
@@ -21,29 +21,31 @@ func stringReader(s string) io.Reader { return strings.NewReader(s) }
|
||||
// searchIn exercises every kind the URL binder converts, plus a body-only
|
||||
// field, plus a path param that shares its name with a query key.
|
||||
type searchIn struct {
|
||||
Owner string `json:"owner"` // path param on the member route
|
||||
Q string `json:"q"`
|
||||
Limit int `json:"limit"`
|
||||
Offset uint32 `json:"offset"`
|
||||
Score float64 `json:"score"`
|
||||
Debug bool `json:"debug"`
|
||||
Ignored []string `json:"ignored"` // not a scalar: never URL-bound
|
||||
Owner string `json:"owner"` // path param on the member route
|
||||
Q string `json:"q"`
|
||||
Limit int `json:"limit"`
|
||||
Offset uint32 `json:"offset"`
|
||||
Score float64 `json:"score"`
|
||||
Debug bool `json:"debug"`
|
||||
Tags []string `json:"tags"` // one value, comma-joined
|
||||
Filters map[string]string `json:"filters"` // no URL spelling at all
|
||||
}
|
||||
|
||||
type searchOut struct {
|
||||
Owner string `json:"owner"`
|
||||
Q string `json:"q"`
|
||||
Limit int `json:"limit"`
|
||||
Offset uint32 `json:"offset"`
|
||||
Score float64 `json:"score"`
|
||||
Debug bool `json:"debug"`
|
||||
Ignored []string `json:"ignored"`
|
||||
Owner string `json:"owner"`
|
||||
Q string `json:"q"`
|
||||
Limit int `json:"limit"`
|
||||
Offset uint32 `json:"offset"`
|
||||
Score float64 `json:"score"`
|
||||
Debug bool `json:"debug"`
|
||||
Tags []string `json:"tags"`
|
||||
Filters map[string]string `json:"filters"`
|
||||
}
|
||||
|
||||
func echoSearch(_ context.Context, in *searchIn) (*searchOut, error) {
|
||||
return &searchOut{
|
||||
Owner: in.Owner, Q: in.Q, Limit: in.Limit, Offset: in.Offset,
|
||||
Score: in.Score, Debug: in.Debug, Ignored: in.Ignored,
|
||||
Score: in.Score, Debug: in.Debug, Tags: in.Tags, Filters: in.Filters,
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -148,11 +150,23 @@ func TestTypedGetIgnoresUnknownQueryKeys(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// Non-scalar fields are not URL-bindable and are left alone.
|
||||
func TestTypedGetLeavesNonScalarsUnbound(t *testing.T) {
|
||||
// A repeated field rides as one comma-joined value — `style: form, explode:
|
||||
// false`, the spelling the document publishes for it and the only one a binder
|
||||
// given one value per name can read.
|
||||
func TestTypedGetBindsAListFromOneValue(t *testing.T) {
|
||||
a := searchApp(t)
|
||||
if _, out := getSearch(t, a, "GET", "/v1/t/search?ignored=a,b", ""); out.Ignored != nil {
|
||||
t.Errorf("ignored = %v, want nil (a slice is not URL-bindable)", out.Ignored)
|
||||
_, out := getSearch(t, a, "GET", "/v1/t/search?tags=a,b", "")
|
||||
if len(out.Tags) != 2 || out.Tags[0] != "a" || out.Tags[1] != "b" {
|
||||
t.Errorf("tags = %v, want [a b]", out.Tags)
|
||||
}
|
||||
}
|
||||
|
||||
// A map has no URL spelling and is left alone — and the document does not name
|
||||
// it either, so no client is offered an argument the wire ignores.
|
||||
func TestTypedGetLeavesAMapUnbound(t *testing.T) {
|
||||
a := searchApp(t)
|
||||
if _, out := getSearch(t, a, "GET", "/v1/t/search?filters=a:b", ""); out.Filters != nil {
|
||||
t.Errorf("filters = %v, want nil (a map is not URL-bindable)", out.Filters)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -214,6 +228,7 @@ func TestOpenAPIDeclaresQueryParams(t *testing.T) {
|
||||
for name, wantType := range map[string]string{
|
||||
"q": "string", "limit": "integer", "offset": "integer",
|
||||
"score": "number", "debug": "boolean", "owner": "string",
|
||||
"tags": "array",
|
||||
} {
|
||||
p, ok := got[name]
|
||||
if !ok {
|
||||
@@ -228,8 +243,8 @@ func TestOpenAPIDeclaresQueryParams(t *testing.T) {
|
||||
t.Errorf("parameter %q type = %v, want %s", name, schema["type"], wantType)
|
||||
}
|
||||
}
|
||||
// A non-scalar is not declared, because the binder cannot fill it.
|
||||
if _, ok := got["ignored"]; ok {
|
||||
// What the binder cannot fill is not declared.
|
||||
if _, ok := got["filters"]; ok {
|
||||
t.Errorf("declared a query parameter the binder cannot bind")
|
||||
}
|
||||
// A POST declares a requestBody instead of query parameters — the two are
|
||||
|
||||
@@ -419,11 +419,38 @@ func setScalar(fv reflect.Value, val string) {
|
||||
if f, err := strconv.ParseFloat(val, fv.Type().Bits()); err == nil {
|
||||
fv.SetFloat(f)
|
||||
}
|
||||
case reflect.Slice:
|
||||
setList(fv, val)
|
||||
default:
|
||||
setText(fv, val)
|
||||
}
|
||||
}
|
||||
|
||||
// setList writes one wire string into a repeated field, splitting it on commas.
|
||||
//
|
||||
// That is `style: form, explode: false`, the spelling OpenAPI already publishes
|
||||
// for a repeated query parameter and the one a URL can carry in a single value
|
||||
// — which is what the binder is given, one value per name. Each element is
|
||||
// written by [setScalar], so a list of ids says nothing about ids a second
|
||||
// time, and a list of lists is refused the same way a bare one would be.
|
||||
//
|
||||
// A []byte is not a list here. encoding/json spells it as one base64 string, so
|
||||
// splitting it on commas would invent a shape nothing sends; it goes to
|
||||
// [setText] instead, where a named byte slice with its own written form is read
|
||||
// by that form.
|
||||
func setList(fv reflect.Value, val string) {
|
||||
if fv.Type().Elem().Kind() == reflect.Uint8 {
|
||||
setText(fv, val)
|
||||
return
|
||||
}
|
||||
parts := strings.Split(val, ",")
|
||||
out := reflect.MakeSlice(fv.Type(), len(parts), len(parts))
|
||||
for i, p := range parts {
|
||||
setScalar(out.Index(i), strings.TrimSpace(p))
|
||||
}
|
||||
fv.Set(out)
|
||||
}
|
||||
|
||||
// setText writes one wire string into a field that states how to read itself
|
||||
// from text.
|
||||
//
|
||||
@@ -460,6 +487,12 @@ func setText(fv reflect.Value, val string) {
|
||||
fv.Set(held.Elem())
|
||||
}
|
||||
|
||||
// readsText is [setText]'s question asked of the TYPE, which is what the
|
||||
// document has to ask: describing a parameter there is no value to address.
|
||||
func readsText(t reflect.Type) bool { return reflect.PointerTo(t).Implements(textReader) }
|
||||
|
||||
var textReader = reflect.TypeOf((*encoding.TextUnmarshaler)(nil)).Elem()
|
||||
|
||||
func registerTyped[In, Out any](on OpTarget, method, path string, fn TypedHandler[In, Out], opts ...OpOption) {
|
||||
scope := on.OpScope()
|
||||
app := scope.App
|
||||
|
||||
+125
@@ -0,0 +1,125 @@
|
||||
package zip_test
|
||||
|
||||
// A URL carries one value per name, so a REPEATED argument is written as one
|
||||
// value with commas — `style: form, explode: false`, which is what the document
|
||||
// publishes for it. Until the binder read that spelling, an op taking a list of
|
||||
// ids answered 200 about an empty list. These pin the spelling, and pin the
|
||||
// document to naming exactly what the binder fills.
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/hex"
|
||||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
"github.com/zap-proto/zip"
|
||||
)
|
||||
|
||||
type listIn struct {
|
||||
IDs []hash `json:"ids"`
|
||||
Names []string `json:"names"`
|
||||
Sizes []int `json:"sizes"`
|
||||
By map[string]string `json:"by"`
|
||||
}
|
||||
|
||||
func echoList(_ context.Context, in *listIn) (*listIn, error) { return in, nil }
|
||||
|
||||
func listApp() *zip.App {
|
||||
a := zip.New(zip.Config{AppName: "list", DisableStartupMessage: true})
|
||||
zip.Get(a, "/v1/list/things", echoList)
|
||||
return a
|
||||
}
|
||||
|
||||
func askList(t *testing.T, query string) listIn {
|
||||
t.Helper()
|
||||
code, body := call2(t, listApp(), "GET", "/v1/list/things?"+query, "")
|
||||
if code != 200 {
|
||||
t.Fatalf("status = %d %s", code, body)
|
||||
}
|
||||
var got listIn
|
||||
if err := json.Unmarshal([]byte(body), &got); err != nil {
|
||||
t.Fatalf("body %s: %v", body, err)
|
||||
}
|
||||
return got
|
||||
}
|
||||
|
||||
// A repeated field arrives, and its elements are read the same way a bare one
|
||||
// would be — so a list of ids says nothing about ids a second time.
|
||||
func TestAURLCarriesARepeatedValue(t *testing.T) {
|
||||
one, two := hash{0x11}, hash{0x22}
|
||||
got := askList(t, "ids="+hex.EncodeToString(one[:])+","+hex.EncodeToString(two[:])+
|
||||
"&names=a,b&sizes=1,2,3")
|
||||
|
||||
if len(got.IDs) != 2 || got.IDs[0] != one || got.IDs[1] != two {
|
||||
t.Errorf("ids = %x, want the two named", got.IDs)
|
||||
}
|
||||
if len(got.Names) != 2 || got.Names[0] != "a" || got.Names[1] != "b" {
|
||||
t.Errorf("names = %v", got.Names)
|
||||
}
|
||||
if len(got.Sizes) != 3 || got.Sizes[2] != 3 {
|
||||
t.Errorf("sizes = %v", got.Sizes)
|
||||
}
|
||||
}
|
||||
|
||||
// One element is still a list. A caller asking about a single id sends one
|
||||
// value, and an op that took a list only when given two would be a trap.
|
||||
func TestOneValueIsAListOfOne(t *testing.T) {
|
||||
if got := askList(t, "names=solo"); len(got.Names) != 1 || got.Names[0] != "solo" {
|
||||
t.Errorf("names = %v, want [solo]", got.Names)
|
||||
}
|
||||
}
|
||||
|
||||
// A map is not a list with different punctuation. It has no URL spelling, so
|
||||
// the binder leaves it alone and the document does not name it.
|
||||
func TestAMapDoesNotRideAURL(t *testing.T) {
|
||||
if got := askList(t, "by=k:v"); got.By != nil {
|
||||
t.Errorf("by = %v, want nil", got.By)
|
||||
}
|
||||
}
|
||||
|
||||
// The other half of the same rule: the document names exactly what the binder
|
||||
// fills, so a generated client's arguments and the server's binding cannot
|
||||
// disagree. An id is a STRING there — one word in the URL and the same word in
|
||||
// the JSON beside it, whatever it is made of — and a list of ids is an array of
|
||||
// that string. Describing it by what it is MADE of published an array of
|
||||
// integers for a value nothing spells that way.
|
||||
func TestTheDocumentNamesWhatTheBinderFills(t *testing.T) {
|
||||
type oneIn struct {
|
||||
ID hash `json:"id"`
|
||||
}
|
||||
a := listApp()
|
||||
zip.Get(a, "/v1/list/one", func(_ context.Context, in *oneIn) (*oneIn, error) { return in, nil })
|
||||
|
||||
says := func(path string) map[string]string {
|
||||
t.Helper()
|
||||
out := map[string]string{}
|
||||
for _, raw := range paramsOf2(t, a, path, "get") {
|
||||
p, _ := raw.(map[string]any)
|
||||
schema, _ := p["schema"].(map[string]any)
|
||||
kind, _ := schema["type"].(string)
|
||||
if kind == "array" {
|
||||
items, _ := schema["items"].(map[string]any)
|
||||
kind += " of " + items["type"].(string)
|
||||
}
|
||||
out[p["name"].(string)] = kind
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
list := says("/v1/list/things")
|
||||
for name, want := range map[string]string{
|
||||
"ids": "array of string",
|
||||
"names": "array of string",
|
||||
"sizes": "array of integer",
|
||||
} {
|
||||
if list[name] != want {
|
||||
t.Errorf("%s: document says %q, want %q", name, list[name], want)
|
||||
}
|
||||
}
|
||||
if _, named := list["by"]; named {
|
||||
t.Errorf("the document names `by`, which the binder cannot fill")
|
||||
}
|
||||
if one := says("/v1/list/one"); one["id"] != "string" {
|
||||
t.Errorf("id: document says %q, want a string", one["id"])
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user