diff --git a/docs/doc-site/shaping-the-output/field-types-and-formats/file-uploads-and-streams.md b/docs/doc-site/shaping-the-output/field-types-and-formats/file-uploads-and-streams.md new file mode 100644 index 00000000..532f12cf --- /dev/null +++ b/docs/doc-site/shaping-the-output/field-types-and-formats/file-uploads-and-streams.md @@ -0,0 +1,93 @@ +--- +title: File uploads and byte streams +weight: 15 +description: | + How io.Reader, multipart.File and the other stream types render — type: file + on a formData parameter, base64 bytes everywhere else — and how to say what + the bytes actually are. +--- + +A Go type like `io.Reader` says that bytes will flow. It says nothing about +*what* they are, how they are framed, or how long they run. codescan recognizes +these types and answers with the only two things Swagger 2.0 lets it say about +opaque bytes — picked by **where the field sits**, not by anything in the +declaration. + +## The two answers + +| Position | Rendering | +|---|---| +| `in: formData` parameter | `type: file` | +| model field, body, response body, header, other parameters | `{type: string, format: byte}` | + +`type: file` is the canonical upload shape, and formData is the only location +Swagger 2.0 permits it in. Everywhere else the bytes travel inside a JSON +document, which cannot carry raw octets — so they render as `format: byte`, the +base64-encoded string the specification defines for exactly this. + +## Uploading a file + +Put the stream in a `formData` parameter and consume `multipart/form-data`: + +{{< example go="shaping/streams/streams.go" goregion="params" golabel="parameters" + json="shaping/streams/testdata/upload_params.json" jsonlabel="parameters" >}} + +`upload` becomes `type: file`; the sibling `caption` is an ordinary form field. +`multipart.File` and `io.Reader` are interchangeable here — both are recognized. + +## Streams in a model or a body + +Anywhere that is not a formData parameter, the same types render as base64 +bytes: + +{{< example go="shaping/streams/streams.go" goregion="model" golabel="model" + json="shaping/streams/testdata/attachment.json" jsonlabel="#/definitions/Attachment" >}} + +`content` and `thumbnail` carry `{string, byte}`. `checksum` carries +`swagger:strfmt base64`, and the annotation wins — which is the point of the +next section. + +## Say what the bytes are + +The default is deliberately uninformative, because a stream *is* +uninformative. When you know more, say so and codescan will step aside: + +- [`swagger:strfmt`]({{% relref "forcing-a-format" %}}) — name the format + (`base64`, `binary`, a custom one); +- [`swagger:type`]({{% relref "/maintainers/annotations/swagger-type" %}}) — + override the type outright; +- [`swagger:file`]({{% relref "/maintainers/annotations/swagger-file" %}}) — + force the file shape where you want it and the position allows it. + +## What is recognized + +| Package | Types | +|---|---| +| `io` | `Reader`, `ReadCloser`, `ReadSeeker`, `ReadSeekCloser`, `ReadWriter`, `ReaderAt`, `ReaderFrom`, `LimitedReader`, `ByteReader`, `ByteScanner` | +| `mime/multipart` | `File` | +| `github.com/go-openapi/runtime` | `NamedReadCloser` | + +Recognition is by **identity** — the exact named type — never by shape. An +interface of your own that happens to have a `Read` method is *your* type and is +documented as you declared it. + +Because both renderings erase *which* stream it was — every type in the table +produces the same schema — each one also carries an +[`x-go-type`]({{% relref "vendor-extensions" %}}) extension naming the Go type it +came from, so a consumer can tell an `io.Reader` field from a `multipart.File` +one. `SkipExtensions` suppresses it along with the rest of the `x-go-*` family. + +{{% notice style="note" %}} +**`io.Writer` is not recognized**, nor are the write-only closers. A sink the +caller writes into is not something that travels on the wire, so codescan does +not assume what you meant by putting one in an API type — it documents the type +structurally, and you override it if you had something in mind. +{{% /notice %}} + +## What's next + +- [Forcing a conformant format]({{% relref "forcing-a-format" %}}) — the + field-level `swagger:strfmt` used above. +- [Type discovery]({{% relref "/shaping-the-output/scope-and-discovery/type-discovery" %}}) — + how codescan decides what a Go type becomes when no recognizer applies. +- [`swagger:file` reference]({{% relref "/maintainers/annotations/swagger-file" %}}). diff --git a/docs/doc-site/shaping-the-output/names-and-refs/composing-embeds-with-allof.md b/docs/doc-site/shaping-the-output/names-and-refs/composing-embeds-with-allof.md index c1691907..415acf57 100644 --- a/docs/doc-site/shaping-the-output/names-and-refs/composing-embeds-with-allof.md +++ b/docs/doc-site/shaping-the-output/names-and-refs/composing-embeds-with-allof.md @@ -71,6 +71,69 @@ embeds should compose; reach for the option when composition is your house style for every plain embed. {{% /notice %}} +## Composition needs a marshaller you write + +An `allOf` says the JSON document satisfies every member at once — one flat object carrying all +their properties. Go's **default** marshaller only produces that shape by coincidence, and the +coincidence holds for exactly one case: a plain struct embed with no marshaller of its own, whose +fields Go promotes. + +Step outside that case and the default rendering stops matching the spec: + +- a member that is **not a struct** — a map, a slice, a named basic — promotes nothing, so Go emits + it as one key named after the type instead of merging it; +- a member with **its own `MarshalJSON`/`MarshalText`** is promoted into your type's method set, and + `json.Marshal` then consults it *before* reading any field — rendering the whole struct as whatever + that method returns. + +This is why go-swagger's generated models never rely on the default. A model with `allOf` embeds its +members **and** carries a hand-written pair that flattens them, reading every member from the same +raw document: + +```go +// swagger:model WithAllOf +type WithAllOf struct { + Notable // an allOf member + + AO1 map[string]int32 `json:"-"` // a map member — json:"-" keeps the default out of the way + + WithAllOfAO2P2 // another member + + Body string `json:"body,omitempty"` // the model's own fields + Title string `json:"title,omitempty"` +} + +// UnmarshalJSON reads every member from the SAME document — that is what allOf means. +func (m *WithAllOf) UnmarshalJSON(raw []byte) error { + var aO0 Notable + if err := jsonutils.ReadJSON(raw, &aO0); err != nil { + return err + } + m.Notable = aO0 + + var aO1 map[string]int32 + if err := jsonutils.ReadJSON(raw, &aO1); err != nil { + return err + } + m.AO1 = aO1 + + // … one block per member, then the model's own fields +} +``` + +{{% notice style="warning" %}} +If you hand-write the Go types that codescan scans, `swagger:allOf` describes your **intent**; it +does not make `encoding/json` produce that document. Write the marshaller, or generate the model +from the spec and let go-swagger write it for you. codescan reads declarations — it cannot tell +whether the marshalling you need exists, so it will not warn you. +{{% /notice %}} + +Because of this, codescan reads an embed as *composition* and never as an instruction about the +default marshaller. In particular, a promoted `MarshalText`/`MarshalJSON` on an embedded type is +**not** treated as a claim that the whole model is a scalar — see +[Forcing a conformant format]({{% relref "forcing-a-format" %}}) if you want a type rendered as +one. + ## Annotate the embedded type, not the embed A classifier annotation in an **embedded field's** doc comment does nothing. diff --git a/docs/examples/shaping/streams/streams.go b/docs/examples/shaping/streams/streams.go new file mode 100644 index 00000000..d901504d --- /dev/null +++ b/docs/examples/shaping/streams/streams.go @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Package streams holds the annotated declarations for the "File uploads and byte +// streams" how-to. streams_test.go scans it and writes the goldens the guide +// renders. +package streams + +import ( + "io" + "mime/multipart" +) + +// snippet:model + +// Attachment carries opaque byte streams as model fields. +// +// A stream says nothing about its own framing, so codescan does not invent one: +// each field renders as `{string, format: byte}` — the base64-encoded string +// Swagger 2.0 uses for arbitrary bytes. +// +// swagger:model +type Attachment struct { + // Content is the attachment payload. + Content io.Reader `json:"content"` + + // Thumbnail is a closeable stream; the same answer applies. + Thumbnail io.ReadCloser `json:"thumbnail"` + + // Checksum says what its bytes are, so the annotation wins over the default. + // + // swagger:strfmt base64 + Checksum io.Reader `json:"checksum"` +} + +// endsnippet:model + +// snippet:params + +// UploadParams uploads a file and its metadata. +// +// swagger:parameters uploadAttachment +type UploadParams struct { + // Upload is the file to store. + // + // in: formData + Upload multipart.File `json:"upload"` + + // Caption describes the upload. + // + // in: formData + Caption string `json:"caption"` +} + +// endsnippet:params + +// swagger:route POST /attachments attachments uploadAttachment +// +// Uploads an attachment. +// +// Consumes: +// - multipart/form-data +// +// Responses: +// +// 200: attachmentResponse + +// AttachmentResponse returns the stored attachment. +// +// swagger:response attachmentResponse +type AttachmentResponse struct { + // in: body + Body Attachment +} diff --git a/docs/examples/shaping/streams/streams_test.go b/docs/examples/shaping/streams/streams_test.go new file mode 100644 index 00000000..076a5a85 --- /dev/null +++ b/docs/examples/shaping/streams/streams_test.go @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: Apache-2.0 + +package streams + +import ( + "encoding/json" + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/go-openapi/codescan" + "github.com/go-openapi/spec" + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +func examplesRoot(t *testing.T) string { + t.Helper() + _, thisFile, _, ok := runtime.Caller(0) + require.True(t, ok) + return filepath.Clean(filepath.Join(filepath.Dir(thisFile), "..", "..")) +} + +func scanStreams(t *testing.T) *spec.Swagger { + t.Helper() + doc, err := codescan.Run(&codescan.Options{ + WorkDir: examplesRoot(t), + Packages: []string{"./shaping/streams"}, + ScanModels: true, + }) + require.NoError(t, err) + require.NotNil(t, doc) + return doc +} + +// TestStreams emits testdata/attachment.json and testdata/upload_params.json — the +// two shapes the "File uploads and byte streams" guide renders — and asserts the +// position-dependent split: `file` on a formData parameter, `{string, byte}` +// everywhere else, with an explicit annotation still winning. +// +// Regenerate with: UPDATE_GOLDEN=1 go test ./... +func TestStreams(t *testing.T) { + doc := scanStreams(t) + + a, ok := doc.Definitions["Attachment"] + require.True(t, ok, "Attachment definition missing") + + for _, name := range []string{"content", "thumbnail"} { + p := a.Properties[name] + assert.Equal(t, "string", p.Type[0], "%s renders as a string", name) + assert.Equal(t, "byte", p.Format, "%s carries the base64 format", name) + } + + checksum := a.Properties["checksum"] + assert.Equal(t, "base64", checksum.Format, "an explicit swagger:strfmt wins over the default") + + assert.NotContains(t, doc.Definitions, "Reader", "a recognized stream type is never published") + assert.NotContains(t, doc.Definitions, "ReadCloser", "a recognized stream type is never published") + + require.NotNil(t, doc.Paths) + item, ok := doc.Paths.Paths["/attachments"] + require.True(t, ok, "/attachments missing") + require.NotNil(t, item.Post) + + params := make(map[string]spec.Parameter, len(item.Post.Parameters)) + for _, p := range item.Post.Parameters { + params[p.Name] = p + } + + upload, ok := params["upload"] + require.True(t, ok, "upload parameter missing") + assert.Equal(t, "formData", upload.In) + assert.Equal(t, "file", upload.Type, "a stream in formData is the canonical upload shape") + + writeGolden(t, filepath.Join("testdata", "attachment.json"), a) + writeGolden(t, filepath.Join("testdata", "upload_params.json"), item.Post.Parameters) +} + +func writeGolden(t *testing.T, path string, v any) { + t.Helper() + + got, err := json.MarshalIndent(v, "", " ") + require.NoError(t, err) + got = append(got, '\n') + + if os.Getenv("UPDATE_GOLDEN") != "" { + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o700)) + require.NoError(t, os.WriteFile(path, got, 0o600)) + } + want, err := os.ReadFile(path) + require.NoError(t, err) + assert.JSONEq(t, string(want), string(got)) +} diff --git a/docs/examples/shaping/streams/testdata/attachment.json b/docs/examples/shaping/streams/testdata/attachment.json new file mode 100644 index 00000000..1819710e --- /dev/null +++ b/docs/examples/shaping/streams/testdata/attachment.json @@ -0,0 +1,29 @@ +{ + "description": "A stream says nothing about its own framing, so codescan does not invent one:\neach field renders as `{string, format: byte}` — the base64-encoded string\nSwagger 2.0 uses for arbitrary bytes.", + "type": "object", + "title": "Attachment carries opaque byte streams as model fields.", + "properties": { + "checksum": { + "description": "Checksum says what its bytes are, so the annotation wins over the default.", + "type": "string", + "format": "base64", + "x-go-name": "Checksum", + "x-go-type": "io.Reader" + }, + "content": { + "description": "Content is the attachment payload.", + "type": "string", + "format": "byte", + "x-go-name": "Content", + "x-go-type": "io.Reader" + }, + "thumbnail": { + "description": "Thumbnail is a closeable stream; the same answer applies.", + "type": "string", + "format": "byte", + "x-go-name": "Thumbnail", + "x-go-type": "io.ReadCloser" + } + }, + "x-go-package": "github.com/go-openapi/codescan/docs/examples/shaping/streams" +} diff --git a/docs/examples/shaping/streams/testdata/upload_params.json b/docs/examples/shaping/streams/testdata/upload_params.json new file mode 100644 index 00000000..6f9a7448 --- /dev/null +++ b/docs/examples/shaping/streams/testdata/upload_params.json @@ -0,0 +1,17 @@ +[ + { + "type": "file", + "x-go-name": "Upload", + "x-go-type": "mime/multipart.File", + "description": "Upload is the file to store.", + "name": "upload", + "in": "formData" + }, + { + "type": "string", + "x-go-name": "Caption", + "description": "Caption describes the upload.", + "name": "caption", + "in": "formData" + } +] diff --git a/fixtures/enhancements/annotation-noise/types.go b/fixtures/enhancements/annotation-noise/types.go index d05d07a3..c7cdc056 100644 --- a/fixtures/enhancements/annotation-noise/types.go +++ b/fixtures/enhancements/annotation-noise/types.go @@ -45,6 +45,19 @@ type IneffectiveOnPlain struct { Note string `json:"note"` } +// EffectiveOnNamedEmbed gives the embed a json name, which makes it a single +// named property rather than a promotion — so the classifier IS consulted, +// exactly as on a regular field. Reporting it as ineffective was a false alarm. +// +// swagger:model EffectiveOnNamedEmbed +type EffectiveOnNamedEmbed struct { + // swagger:strfmt uuid + Target `json:"nested"` + + // Note is the embedding struct's own field. + Note string `json:"note"` +} + // EffectiveOnField is the control: the same annotations on regular fields, where // both are honoured. // diff --git a/fixtures/enhancements/embed-basic-underlying/types.go b/fixtures/enhancements/embed-basic-underlying/types.go new file mode 100644 index 00000000..a84a481c --- /dev/null +++ b/fixtures/enhancements/embed-basic-underlying/types.go @@ -0,0 +1,155 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +// Package embed_basic_underlying exercises the embed of a named type whose UNDERLYING is +// neither a struct nor an interface. +// +// Such an embed promotes no field — there is none to promote — so Go keeps the embedded value as +// an ordinary member keyed by the TYPE NAME. `buildNamedEmbedded` had arms for struct and +// interface only, so every shape here fell to a warn-and-skip default and the member vanished +// from the schema. +// +// This is the one embed shape where the embed's own json tag is meaningful again: it names an +// ordinary property rather than steering a promotion. +// +// See [§embedded](../../../internal/builders/schema/README.md#embedded). +package embed_basic_underlying + +// Count is a named type over a primitive, unannotated. +type Count int + +// FmtBasic is a named type over a primitive, carrying a format, so the witness shows that the +// member is built from the embedded type — classifiers included — and not merely declared. +// +// swagger:strfmt duration +type FmtBasic int + +// Codes is a named type over a slice. +type Codes []string + +// Grid is a named type over an array. +type Grid [4]int32 + +// Token is a named type over an array that also implements encoding.TextMarshaler. +// +// Under encoding/json the promoted MarshalText makes the WHOLE embedding struct render as a bare +// string. codescan deliberately does not model that: an embed means composition, and a composed +// model round-trips through a custom marshaller rather than the default one. The member is built +// like any other instead. +type Token [16]byte + +// MarshalText renders the token as text. +func (t Token) MarshalText() ([]byte, error) { return []byte("tok"), nil } + +// UnmarshalText parses the token from text. +func (t *Token) UnmarshalText([]byte) error { return nil } + +// BasicHost embeds a plain primitive-underlying named type. +// +// swagger:model BasicHost +type BasicHost struct { + Count + + // Label is the embedding struct's own field. + Label string `json:"label"` +} + +// FmtHost embeds a primitive-underlying named type that carries a format. +// +// swagger:model FmtHost +type FmtHost struct { + FmtBasic + + // Label is the embedding struct's own field. + Label string `json:"label"` +} + +// SliceHost embeds a slice-underlying named type. +// +// swagger:model SliceHost +type SliceHost struct { + Codes + + // Label is the embedding struct's own field. + Label string `json:"label"` +} + +// ArrayHost embeds an array-underlying named type. +// +// swagger:model ArrayHost +type ArrayHost struct { + Grid + + // Label is the embedding struct's own field. + Label string `json:"label"` +} + +// TaggedHost names the embed with a json tag, which here renames an ordinary property. +// +// swagger:model TaggedHost +type TaggedHost struct { + Count `json:"count"` + + // Label is the embedding struct's own field. + Label string `json:"label"` +} + +// OmittedHost drops the embed with `json:"-"`, exactly as it would drop a regular field. +// +// swagger:model OmittedHost +type OmittedHost struct { + Count `json:"-"` + + // Label is the embedding struct's own field. + Label string `json:"label"` +} + +// PtrHost embeds a pointer to a primitive-underlying named type. +// +// swagger:model PtrHost +type PtrHost struct { + *Count + + // Label is the embedding struct's own field. + Label string `json:"label"` +} + +// MarshalHost embeds a text-marshalable array-underlying named type. +// +// swagger:model MarshalHost +type MarshalHost struct { + Token + + // Label is the embedding struct's own field. + Label string `json:"label"` +} + +// MapHost embeds a map-underlying named type, the remaining non-struct underlying. +// +// swagger:model MapHost +type MapHost struct { + Registry + + // Label is the embedding struct's own field. + Label string `json:"label"` +} + +// Registry is a named type over a map. +type Registry map[string]int32 + +// Control declares every embedded type above as an ORDINARY field. +// +// It is the calibration for what "built from the embedded type" has to mean: the member an embed +// contributes must be the same schema a plain field of that type already produces. Without it the +// expected shape would be invented here rather than derived from the builder's existing behaviour. +// +// swagger:model Control +type Control struct { + CountField Count `json:"countField"` + FmtField FmtBasic `json:"fmtField"` + CodesField Codes `json:"codesField"` + GridField Grid `json:"gridField"` + TokenField Token `json:"tokenField"` + RegistryField Registry `json:"registryField"` + PtrField *Count `json:"ptrField"` +} diff --git a/fixtures/enhancements/embed-basic-underlying/wire.golden.json b/fixtures/enhancements/embed-basic-underlying/wire.golden.json new file mode 100644 index 00000000..018976ec --- /dev/null +++ b/fixtures/enhancements/embed-basic-underlying/wire.golden.json @@ -0,0 +1,44 @@ +{ + "ArrayHost": { + "Grid": [ + 1, + 2, + 3, + 4 + ], + "label": "l" + }, + "BasicHost": { + "Count": 7, + "label": "l" + }, + "FmtHost": { + "FmtBasic": 7, + "label": "l" + }, + "MapHost": { + "Registry": { + "k": 1 + }, + "label": "l" + }, + "MarshalHost": "tok", + "OmittedHost": { + "label": "l" + }, + "PtrHost": { + "Count": 7, + "label": "l" + }, + "SliceHost": { + "Codes": [ + "a", + "b" + ], + "label": "l" + }, + "TaggedHost": { + "count": 7, + "label": "l" + } +} diff --git a/fixtures/enhancements/embed-basic-underlying/wire_test.go b/fixtures/enhancements/embed-basic-underlying/wire_test.go new file mode 100644 index 00000000..11106908 --- /dev/null +++ b/fixtures/enhancements/embed-basic-underlying/wire_test.go @@ -0,0 +1,75 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package embed_basic_underlying + +import ( + "encoding/json" + "os" + "path/filepath" + "reflect" + "testing" +) + +// The second oracle in the fixtures module, for the same reason as the first (see +// enhancements/json-tag-fidelity/wire_test.go): the types live here, so only this package can +// marshal them, and the integration test cannot import across the module boundary. +// +// It records the RAW marshalled document rather than a key set, because one subject here does not +// marshal to an object at all — MarshalHost's promoted MarshalText renders the whole struct as a +// bare string. That divergence is the point: it is the case codescan deliberately does NOT model, +// and an oracle that could only describe objects would not be able to state it. +// +// Regenerate with UPDATE_GOLDEN=1, like every other golden in the repo. +const wireGolden = "wire.golden.json" + +func TestWireShapes(t *testing.T) { + count := Count(7) + + // Non-zero values throughout, so `omitempty` never hides a key. + subjects := map[string]any{ + "BasicHost": BasicHost{Count: 7, Label: "l"}, + "FmtHost": FmtHost{FmtBasic: 7, Label: "l"}, + "SliceHost": SliceHost{Codes: Codes{"a", "b"}, Label: "l"}, + "ArrayHost": ArrayHost{Grid: Grid{1, 2, 3, 4}, Label: "l"}, + "TaggedHost": TaggedHost{Count: 7, Label: "l"}, + "OmittedHost": OmittedHost{Count: 7, Label: "l"}, + "PtrHost": PtrHost{Count: &count, Label: "l"}, + "MarshalHost": MarshalHost{Token: Token{}, Label: "l"}, + "MapHost": MapHost{Registry: Registry{"k": 1}, Label: "l"}, + } + + got := make(map[string]json.RawMessage, len(subjects)) + for name, v := range subjects { + raw, err := json.Marshal(v) + if err != nil { + t.Fatalf("marshal %s: %v", name, err) + } + got[name] = raw + } + + data, err := json.MarshalIndent(got, "", " ") + if err != nil { + t.Fatal(err) + } + data = append(data, '\n') + + path := filepath.Join(".", wireGolden) + if os.Getenv("UPDATE_GOLDEN") == "1" { + const filePerm = 0o600 + if err := os.WriteFile(path, data, filePerm); err != nil { + t.Fatal(err) + } + t.Logf("wrote %s", wireGolden) + + return + } + + want, err := os.ReadFile(path) + if err != nil { + t.Fatalf("missing %s — run with UPDATE_GOLDEN=1 to create: %v", wireGolden, err) + } + if !reflect.DeepEqual(string(want), string(data)) { + t.Errorf("wire shapes drifted.\nwant:\n%s\ngot:\n%s", want, data) + } +} diff --git a/fixtures/enhancements/opaque-streams/api.go b/fixtures/enhancements/opaque-streams/api.go new file mode 100644 index 00000000..c814ec86 --- /dev/null +++ b/fixtures/enhancements/opaque-streams/api.go @@ -0,0 +1,16 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package opaque_streams + +// swagger:route POST /streams streams uploadStream +// +// Uploads a stream. +// +// Consumes: +// - multipart/form-data +// +// Responses: +// +// 200: streamResponse +func uploadStream() {} diff --git a/fixtures/enhancements/opaque-streams/types.go b/fixtures/enhancements/opaque-streams/types.go new file mode 100644 index 00000000..45e72c22 --- /dev/null +++ b/fixtures/enhancements/opaque-streams/types.go @@ -0,0 +1,115 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +// Package opaque_streams witnesses the stdlib stream types across every position that can carry +// one. +// +// None of them had a recognizer, so they fell through to ordinary structural drilling — and an +// interface is the shape drilling handles worst. `io.Reader` as a formData parameter emitted no +// `type` at all (invalid: SimpleSchema requires one), and as a model field it published `io`'s own +// interfaces as definitions carrying io's godoc, inventing a `close` property of type string out +// of `Close() error`. +// +// A stream is opaque by construction: nothing in the declaration says what the bytes are. The +// recognizers therefore do not try to guess — they say "opaque bytes" in whichever way the +// position allows, and the author's `swagger:file` / `swagger:type` override still wins. +// +// See [§opaque-streams](../../../internal/builders/schema/README.md#opaque-streams). +package opaque_streams + +import ( + "io" + "mime/multipart" + + "github.com/go-openapi/runtime" +) + +// StreamModel carries every recognized stream type as a model field. +// +// Every field is deliberately named UNLIKE its type. The emitted `x-go-name` is the Go FIELD name +// and `x-go-type` is the Go type; naming a field after its type would make the two extensions +// indistinguishable in the golden and hide a regression in either. +// +// swagger:model StreamModel +type StreamModel struct { + Payload io.Reader `json:"payload"` + Envelope io.ReadCloser `json:"envelope"` + Rewindable io.ReadSeeker `json:"rewindable"` + Archive io.ReadSeekCloser `json:"archive"` + Duplex io.ReadWriter `json:"duplex"` + Chunk io.ReaderAt `json:"chunk"` + Sink io.ReaderFrom `json:"sink"` + Excerpt io.LimitedReader `json:"excerpt"` + Nibble io.ByteReader `json:"nibble"` + Peekable io.ByteScanner `json:"peekable"` + Attachment runtime.NamedReadCloser `json:"attachment"` + Upload multipart.File `json:"upload"` +} + +// WriterModel holds the type deliberately left OUT of the recognized set. +// +// An API payload does not plausibly contain an `io.Writer` — a sink the caller writes into is not +// something that goes on the wire. Recognizing it would be inventing an intent, so it keeps +// whatever the structural walk makes of it and the author overrides if they meant something. +// +// swagger:model WriterModel +type WriterModel struct { + Writer io.Writer `json:"writer"` +} + +// OverriddenModel is the control: an explicit annotation still wins over the recognizer. +// +// swagger:model OverriddenModel +type OverriddenModel struct { + // Blob says what the bytes are, so the recognizer must not overrule it. + // + // swagger:strfmt base64 + Blob io.Reader `json:"blob"` + + // Handle overrides the TYPE rather than the format. The two overrides differ in what they do to + // the recognizer's x-go-type stamp: a format override adjusts the format and the stamp survives, + // whereas a type override replaces the schema outright and the stamp goes with it. Witnessed + // rather than asserted, because the difference follows from how the two branches are written. + // + // swagger:type string + Handle io.ReadCloser `json:"handle"` +} + +// UploadParams reaches the stream types from each parameter location. +// +// swagger:parameters uploadStream +type UploadParams struct { + // Upload is the canonical file-upload shape. + // + // in: formData + Upload io.Reader `json:"upload"` + + // Doc is the same shape spelled with multipart.File. + // + // in: formData + Doc multipart.File `json:"doc"` + + // Body carries the stream as the request body. + // + // in: body + Body io.ReadCloser `json:"body"` + + // Marker is a stream in a location where it makes little sense, kept so the witness records + // what such a declaration produces rather than leaving it undefined. + // + // in: query + Marker io.Reader `json:"marker"` +} + +// StreamResponse returns a stream as the response body. +// +// swagger:response streamResponse +type StreamResponse struct { + // in: body + Body io.ReadCloser + + // XChecksum is a stream reached through a response header. + // + // in: header + XChecksum io.Reader +} diff --git a/fixtures/integration/golden/enhancements_annotation_noise.json b/fixtures/integration/golden/enhancements_annotation_noise.json index 0b9725ec..1499c289 100644 --- a/fixtures/integration/golden/enhancements_annotation_noise.json +++ b/fixtures/integration/golden/enhancements_annotation_noise.json @@ -20,6 +20,23 @@ }, "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/annotation-noise" }, + "EffectiveOnNamedEmbed": { + "description": "EffectiveOnNamedEmbed gives the embed a json name, which makes it a single\nnamed property rather than a promotion — so the classifier IS consulted,\nexactly as on a regular field. Reporting it as ineffective was a false alarm.", + "type": "object", + "properties": { + "nested": { + "type": "string", + "format": "uuid", + "x-go-name": "Target" + }, + "note": { + "description": "Note is the embedding struct's own field.", + "type": "string", + "x-go-name": "Note" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/annotation-noise" + }, "IneffectiveOnAllOf": { "title": "IneffectiveOnAllOf annotates an allOf embed with classifiers the arm ignores.", "allOf": [ diff --git a/fixtures/integration/golden/enhancements_embed_basic_underlying.json b/fixtures/integration/golden/enhancements_embed_basic_underlying.json new file mode 100644 index 00000000..a89cfdba --- /dev/null +++ b/fixtures/integration/golden/enhancements_embed_basic_underlying.json @@ -0,0 +1,205 @@ +{ + "swagger": "2.0", + "paths": {}, + "definitions": { + "ArrayHost": { + "type": "object", + "title": "ArrayHost embeds an array-underlying named type.", + "properties": { + "Grid": { + "$ref": "#/definitions/Grid" + }, + "label": { + "description": "Label is the embedding struct's own field.", + "type": "string", + "x-go-name": "Label" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/embed-basic-underlying" + }, + "BasicHost": { + "type": "object", + "title": "BasicHost embeds a plain primitive-underlying named type.", + "properties": { + "Count": { + "$ref": "#/definitions/Count" + }, + "label": { + "description": "Label is the embedding struct's own field.", + "type": "string", + "x-go-name": "Label" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/embed-basic-underlying" + }, + "Codes": { + "type": "array", + "title": "Codes is a named type over a slice.", + "items": { + "type": "string" + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/embed-basic-underlying" + }, + "Control": { + "description": "It is the calibration for what \"built from the embedded type\" has to mean: the member an embed\ncontributes must be the same schema a plain field of that type already produces. Without it the\nexpected shape would be invented here rather than derived from the builder's existing behaviour.", + "type": "object", + "title": "Control declares every embedded type above as an ORDINARY field.", + "properties": { + "codesField": { + "$ref": "#/definitions/Codes" + }, + "countField": { + "$ref": "#/definitions/Count" + }, + "fmtField": { + "type": "string", + "format": "duration", + "x-go-name": "FmtField" + }, + "gridField": { + "$ref": "#/definitions/Grid" + }, + "ptrField": { + "$ref": "#/definitions/Count" + }, + "registryField": { + "$ref": "#/definitions/Registry" + }, + "tokenField": { + "type": "string", + "x-go-name": "TokenField", + "x-go-type": "github.com/go-openapi/codescan/fixtures/enhancements/embed-basic-underlying.Token" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/embed-basic-underlying" + }, + "Count": { + "type": "integer", + "format": "int64", + "title": "Count is a named type over a primitive, unannotated.", + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/embed-basic-underlying" + }, + "FmtHost": { + "type": "object", + "title": "FmtHost embeds a primitive-underlying named type that carries a format.", + "properties": { + "FmtBasic": { + "type": "string", + "format": "duration" + }, + "label": { + "description": "Label is the embedding struct's own field.", + "type": "string", + "x-go-name": "Label" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/embed-basic-underlying" + }, + "Grid": { + "type": "array", + "title": "Grid is a named type over an array.", + "items": { + "type": "integer", + "format": "int32" + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/embed-basic-underlying" + }, + "MapHost": { + "type": "object", + "title": "MapHost embeds a map-underlying named type, the remaining non-struct underlying.", + "properties": { + "Registry": { + "$ref": "#/definitions/Registry" + }, + "label": { + "description": "Label is the embedding struct's own field.", + "type": "string", + "x-go-name": "Label" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/embed-basic-underlying" + }, + "MarshalHost": { + "type": "object", + "title": "MarshalHost embeds a text-marshalable array-underlying named type.", + "properties": { + "Token": { + "type": "string", + "x-go-type": "github.com/go-openapi/codescan/fixtures/enhancements/embed-basic-underlying.Token" + }, + "label": { + "description": "Label is the embedding struct's own field.", + "type": "string", + "x-go-name": "Label" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/embed-basic-underlying" + }, + "OmittedHost": { + "type": "object", + "title": "OmittedHost drops the embed with `json:\"-\"`, exactly as it would drop a regular field.", + "properties": { + "label": { + "description": "Label is the embedding struct's own field.", + "type": "string", + "x-go-name": "Label" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/embed-basic-underlying" + }, + "PtrHost": { + "type": "object", + "title": "PtrHost embeds a pointer to a primitive-underlying named type.", + "properties": { + "Count": { + "$ref": "#/definitions/Count" + }, + "label": { + "description": "Label is the embedding struct's own field.", + "type": "string", + "x-go-name": "Label" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/embed-basic-underlying" + }, + "Registry": { + "type": "object", + "title": "Registry is a named type over a map.", + "additionalProperties": { + "type": "integer", + "format": "int32" + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/embed-basic-underlying" + }, + "SliceHost": { + "type": "object", + "title": "SliceHost embeds a slice-underlying named type.", + "properties": { + "Codes": { + "$ref": "#/definitions/Codes" + }, + "label": { + "description": "Label is the embedding struct's own field.", + "type": "string", + "x-go-name": "Label" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/embed-basic-underlying" + }, + "TaggedHost": { + "type": "object", + "title": "TaggedHost names the embed with a json tag, which here renames an ordinary property.", + "properties": { + "count": { + "$ref": "#/definitions/Count" + }, + "label": { + "description": "Label is the embedding struct's own field.", + "type": "string", + "x-go-name": "Label" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/embed-basic-underlying" + } + } +} \ No newline at end of file diff --git a/fixtures/integration/golden/enhancements_in_case_insensitive.json b/fixtures/integration/golden/enhancements_in_case_insensitive.json index 582b0129..04c0466f 100644 --- a/fixtures/integration/golden/enhancements_in_case_insensitive.json +++ b/fixtures/integration/golden/enhancements_in_case_insensitive.json @@ -37,7 +37,9 @@ "in": "header" }, { + "type": "file", "x-go-name": "Upload", + "x-go-type": "io.Reader", "name": "upload", "in": "formData" } diff --git a/fixtures/integration/golden/enhancements_opaque_streams.json b/fixtures/integration/golden/enhancements_opaque_streams.json new file mode 100644 index 00000000..2ea90dc4 --- /dev/null +++ b/fixtures/integration/golden/enhancements_opaque_streams.json @@ -0,0 +1,196 @@ +{ + "swagger": "2.0", + "paths": { + "/streams": { + "post": { + "consumes": [ + "multipart/form-data" + ], + "tags": [ + "streams" + ], + "summary": "Uploads a stream.", + "operationId": "uploadStream", + "parameters": [ + { + "type": "file", + "x-go-name": "Upload", + "x-go-type": "io.Reader", + "description": "Upload is the canonical file-upload shape.", + "name": "upload", + "in": "formData" + }, + { + "type": "file", + "x-go-name": "Doc", + "x-go-type": "mime/multipart.File", + "description": "Doc is the same shape spelled with multipart.File.", + "name": "doc", + "in": "formData" + }, + { + "x-go-name": "Body", + "description": "Body carries the stream as the request body.", + "name": "body", + "in": "body", + "schema": { + "type": "string", + "format": "byte", + "x-go-type": "io.ReadCloser" + } + }, + { + "type": "string", + "format": "byte", + "x-go-name": "Marker", + "x-go-type": "io.Reader", + "description": "Marker is a stream in a location where it makes little sense, kept so the witness records\nwhat such a declaration produces rather than leaving it undefined.", + "name": "marker", + "in": "query" + } + ], + "responses": { + "200": { + "$ref": "#/responses/streamResponse" + } + } + } + } + }, + "definitions": { + "OverriddenModel": { + "type": "object", + "title": "OverriddenModel is the control: an explicit annotation still wins over the recognizer.", + "properties": { + "blob": { + "description": "Blob says what the bytes are, so the recognizer must not overrule it.", + "type": "string", + "format": "base64", + "x-go-name": "Blob", + "x-go-type": "io.Reader" + }, + "handle": { + "description": "Handle overrides the TYPE rather than the format. The two overrides differ in what they do to\nthe recognizer's x-go-type stamp: a format override adjusts the format and the stamp survives,\nwhereas a type override replaces the schema outright and the stamp goes with it. Witnessed\nrather than asserted, because the difference follows from how the two branches are written.", + "type": "string", + "x-go-name": "Handle" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/opaque-streams" + }, + "StreamModel": { + "description": "Every field is deliberately named UNLIKE its type. The emitted `x-go-name` is the Go FIELD name\nand `x-go-type` is the Go type; naming a field after its type would make the two extensions\nindistinguishable in the golden and hide a regression in either.", + "type": "object", + "title": "StreamModel carries every recognized stream type as a model field.", + "properties": { + "archive": { + "type": "string", + "format": "byte", + "x-go-name": "Archive", + "x-go-type": "io.ReadSeekCloser" + }, + "attachment": { + "type": "string", + "format": "byte", + "x-go-name": "Attachment", + "x-go-type": "github.com/go-openapi/runtime.NamedReadCloser" + }, + "chunk": { + "type": "string", + "format": "byte", + "x-go-name": "Chunk", + "x-go-type": "io.ReaderAt" + }, + "duplex": { + "type": "string", + "format": "byte", + "x-go-name": "Duplex", + "x-go-type": "io.ReadWriter" + }, + "envelope": { + "type": "string", + "format": "byte", + "x-go-name": "Envelope", + "x-go-type": "io.ReadCloser" + }, + "excerpt": { + "type": "string", + "format": "byte", + "x-go-name": "Excerpt", + "x-go-type": "io.LimitedReader" + }, + "nibble": { + "type": "string", + "format": "byte", + "x-go-name": "Nibble", + "x-go-type": "io.ByteReader" + }, + "payload": { + "type": "string", + "format": "byte", + "x-go-name": "Payload", + "x-go-type": "io.Reader" + }, + "peekable": { + "type": "string", + "format": "byte", + "x-go-name": "Peekable", + "x-go-type": "io.ByteScanner" + }, + "rewindable": { + "type": "string", + "format": "byte", + "x-go-name": "Rewindable", + "x-go-type": "io.ReadSeeker" + }, + "sink": { + "type": "string", + "format": "byte", + "x-go-name": "Sink", + "x-go-type": "io.ReaderFrom" + }, + "upload": { + "type": "string", + "format": "byte", + "x-go-name": "Upload", + "x-go-type": "mime/multipart.File" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/opaque-streams" + }, + "Writer": { + "description": "Write writes len(p) bytes from p to the underlying data stream.\nIt returns the number of bytes written from p (0 \u003c= n \u003c= len(p))\nand any error encountered that caused the write to stop early.\nWrite must return a non-nil error if it returns n \u003c len(p).\nWrite must not modify the slice data, even temporarily.\n\nImplementations must not retain p.", + "type": "object", + "title": "Writer is the interface that wraps the basic Write method.", + "x-go-package": "io" + }, + "WriterModel": { + "description": "An API payload does not plausibly contain an `io.Writer` — a sink the caller writes into is not\nsomething that goes on the wire. Recognizing it would be inventing an intent, so it keeps\nwhatever the structural walk makes of it and the author overrides if they meant something.", + "type": "object", + "title": "WriterModel holds the type deliberately left OUT of the recognized set.", + "properties": { + "writer": { + "$ref": "#/definitions/Writer" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/opaque-streams" + } + }, + "responses": { + "streamResponse": { + "description": "StreamResponse returns a stream as the response body.", + "schema": { + "type": "string", + "format": "byte", + "x-go-type": "io.ReadCloser" + }, + "headers": { + "XChecksum": { + "type": "string", + "format": "byte", + "description": "XChecksum is a stream reached through a response header.", + "x-go-type": "io.Reader" + } + } + } + } +} \ No newline at end of file diff --git a/fixtures/integration/golden/go123_aliased_spec.json b/fixtures/integration/golden/go123_aliased_spec.json index a1c05a68..32fb040b 100644 --- a/fixtures/integration/golden/go123_aliased_spec.json +++ b/fixtures/integration/golden/go123_aliased_spec.json @@ -72,6 +72,10 @@ "properties": { "EvenMore": {}, "StillMore": {}, + "UUID": { + "type": "integer", + "format": "int64" + }, "more": { "type": "string", "x-go-name": "More" diff --git a/fixtures/integration/golden/strfmt_symmetry_composition_default.json b/fixtures/integration/golden/strfmt_symmetry_composition_default.json index e606c16c..874811cb 100644 --- a/fixtures/integration/golden/strfmt_symmetry_composition_default.json +++ b/fixtures/integration/golden/strfmt_symmetry_composition_default.json @@ -86,6 +86,10 @@ "type": "object", "title": "EmbedBasicAlias plainly embeds the basic pair's alias half.", "properties": { + "FmtBasicAlias": { + "type": "string", + "format": "isbn" + }, "label": { "description": "Label is the embedding struct's own field.", "type": "string", @@ -98,6 +102,10 @@ "type": "object", "title": "EmbedBasicNamed plainly embeds the basic pair's named half.", "properties": { + "FmtBasicNamed": { + "type": "string", + "format": "isbn" + }, "label": { "description": "Label is the embedding struct's own field.", "type": "string", diff --git a/fixtures/integration/golden/strfmt_symmetry_composition_refaliases.json b/fixtures/integration/golden/strfmt_symmetry_composition_refaliases.json index e606c16c..874811cb 100644 --- a/fixtures/integration/golden/strfmt_symmetry_composition_refaliases.json +++ b/fixtures/integration/golden/strfmt_symmetry_composition_refaliases.json @@ -86,6 +86,10 @@ "type": "object", "title": "EmbedBasicAlias plainly embeds the basic pair's alias half.", "properties": { + "FmtBasicAlias": { + "type": "string", + "format": "isbn" + }, "label": { "description": "Label is the embedding struct's own field.", "type": "string", @@ -98,6 +102,10 @@ "type": "object", "title": "EmbedBasicNamed plainly embeds the basic pair's named half.", "properties": { + "FmtBasicNamed": { + "type": "string", + "format": "isbn" + }, "label": { "description": "Label is the embedding struct's own field.", "type": "string", diff --git a/fixtures/integration/golden/strfmt_symmetry_composition_transparentaliases.json b/fixtures/integration/golden/strfmt_symmetry_composition_transparentaliases.json index e606c16c..874811cb 100644 --- a/fixtures/integration/golden/strfmt_symmetry_composition_transparentaliases.json +++ b/fixtures/integration/golden/strfmt_symmetry_composition_transparentaliases.json @@ -86,6 +86,10 @@ "type": "object", "title": "EmbedBasicAlias plainly embeds the basic pair's alias half.", "properties": { + "FmtBasicAlias": { + "type": "string", + "format": "isbn" + }, "label": { "description": "Label is the embedding struct's own field.", "type": "string", @@ -98,6 +102,10 @@ "type": "object", "title": "EmbedBasicNamed plainly embeds the basic pair's named half.", "properties": { + "FmtBasicNamed": { + "type": "string", + "format": "isbn" + }, "label": { "description": "Label is the embedding struct's own field.", "type": "string", diff --git a/internal/builders/resolvers/assertions.go b/internal/builders/resolvers/assertions.go index a6a7926d..63c19851 100644 --- a/internal/builders/resolvers/assertions.go +++ b/internal/builders/resolvers/assertions.go @@ -179,6 +179,58 @@ func IsStdErrorType(t types.Type) bool { return ok && IsStdError(named.Obj()) } +// opaqueStreamTypes are the named types that mean "a stream of bytes whose framing the declaration +// does not state". +// +// Keyed by package path, then type name — identity, never structure. A structural rule ("anything +// with a Read method") would swallow any user interface that happens to expose one, which is +// exactly the over-reach that makes guessing dangerous here. This list is closed: a type joins it +// because it is a known stream carrier, not because its method set resembles one. +// +// `io.Writer` and the write-only closers are deliberately absent. A sink the caller writes into is +// not something that travels on the wire, so an API type containing one is unknown territory; it +// keeps whatever the structural walk makes of it, and the author says what they meant with +// `swagger:file` or `swagger:type`. +var opaqueStreamTypes = map[string]map[string]struct{}{ //nolint:gochecknoglobals // immutable lookup table, built once + "io": { + "Reader": {}, + "ReadCloser": {}, + "ReadSeeker": {}, + "ReadSeekCloser": {}, + "ReadWriter": {}, + "ReaderAt": {}, + "ReaderFrom": {}, + "LimitedReader": {}, + "ByteReader": {}, + "ByteScanner": {}, + }, + "mime/multipart": { + "File": {}, + }, + "github.com/go-openapi/runtime": { + "NamedReadCloser": {}, + }, +} + +// IsOpaqueStream reports whether o is one of the known byte-stream carriers. +// +// Identity-based, so it answers from the object alone and can run ahead of any declaration lookup — +// which matters, because the drilling these types used to reach is what invented a `close` property +// of type string out of `Close() error`. +func IsOpaqueStream(o *types.TypeName) bool { + if o == nil || o.Pkg() == nil { + return false + } + + names, found := opaqueStreamTypes[o.Pkg().Path()] + if !found { + return false + } + _, found = names[o.Name()] + + return found +} + func IsStdJSONRawMessage(o *types.TypeName) bool { return o.Pkg() != nil && o.Pkg().Path() == "encoding/json" && o.Name() == "RawMessage" } diff --git a/internal/builders/schema/README.md b/internal/builders/schema/README.md index bbb4b135..7820b8e1 100644 --- a/internal/builders/schema/README.md +++ b/internal/builders/schema/README.md @@ -20,6 +20,7 @@ trade-offs, and known quirks live here. - [§allof](#allof) — `buildAllOf`, `buildNamedAllOf`, `scanEmbeddedFields` - [§embedded](#embedded) — embed routing, struct/interface specials asymmetry - [§embed-depth](#embed-depth) — ambiguous-embed diagnostic mechanism +- [§opaque-streams](#opaque-streams) — stream types, and the two answers a stream can take - [§omit](#omit) — `swagger:omit` — the author's pre-filter on promoted fields - [§json-dash](#json-dash) — what `json:"-"` does, and the two shapes it is confused with - [§method-mangler](#method-mangler) — interface-method JSON-name derivation @@ -162,9 +163,9 @@ Three layers, all in `special_types.go`: - **`applyStdlibSpecials(obj, target)`** — the canonical safe set `{recognizeAny, recognizeTime, recognizeError, recognizeRawMessage, - recognizeStdUUID}`. All five are identity-based and cannot misfire - on user types, so this helper is **called uniformly at every site** - that handles a `*types.TypeName`. + recognizeStdUUID, recognizeOpaqueStream}`. All six are + identity-based and cannot misfire on user types, so this helper is + **called uniformly at every site** that handles a `*types.TypeName`. The two UUID recognizers are a **certain/guessed pair**, and the distinction is the whole reason both exist: @@ -639,6 +640,58 @@ the text, the change is golden-neutral. --- +## §opaque-streams — stream types, and the two answers a stream can take + +`resolvers.IsOpaqueStream` recognizes the named types that carry a stream of bytes: + +| Package | Types | +|---------|-------| +| `io` | `Reader`, `ReadCloser`, `ReadSeeker`, `ReadSeekCloser`, `ReadWriter`, `ReaderAt`, `ReaderFrom`, `LimitedReader`, `ByteReader`, `ByteScanner` | +| `mime/multipart` | `File` | +| `github.com/go-openapi/runtime` | `NamedReadCloser` | + +`recognizeOpaqueStream` sits in the canonical `ApplyStdlibSpecials` set, so it fires at every site +that handles a `*types.TypeName` — model field, parameter, response body, response header — and it +runs **before** any declaration lookup, which is the point: these types used to reach structural +drilling, and an interface is the shape drilling handles worst. + +**Two answers, chosen by position rather than by the declaration:** + +- `In() == "formData"` → **`type: file`**. The only place OAS 2.0 permits `file`, and the canonical + upload shape. It also repairs a spec that was outright invalid: a formData parameter typed + `io.Reader` emitted **no `type` at all**, and SimpleSchema requires one. +- everywhere else → **`{type: string, format: byte}`**. In a JSON body a raw octet sequence has no + representation; `byte` is the base64-encoded string OAS 2.0 defines for exactly that. `binary` + would claim a framing the position cannot carry. + +**`x-go-type` carries what neither answer can.** `byte` says base64 bytes and `file` says an upload; +both erase *which* stream this was, and all twelve recognized types collapse onto the same schema — +an `io.Reader` field and a `multipart.File` field become indistinguishable. So the recognizer stamps +`x-go-type: .`, under `skipExt` like every other extension. This is the +`recognizeError` criterion ("the rendering erases the type"), not the `time.Time` one ("the format +*is* the type") — see [§traceability](#traceability). + +**Identity, never structure.** A rule like "anything with a `Read([]byte) (int, error)` method" +would swallow any user interface that happens to expose one. The table is closed: a type joins it +because it is a known stream carrier, not because its method set resembles one. + +**`io.Writer` is deliberately absent**, with its write-only closers. A sink the caller writes into +does not travel on the wire, so an API type containing one is unknown territory — recognizing it +would be inventing an intent. It keeps whatever the structural walk makes of it, and the author says +what they meant. + +**This is not a guess about content.** Before the recognizers codescan already answered, and +answered worse: `io`'s own interfaces became definitions carrying io's godoc as title/description, +and `ReadCloser` grew a **`close` property of type `string`** out of `Close() error` (interface-method +promotion applied to a method that is not an accessor). `{string, byte}` is the *less* presumptuous +answer — the standard way to say "opaque bytes, framing unstated". + +An explicit `swagger:strfmt` / `swagger:type` / `swagger:file` still wins; the classifier runs first. + +Related: this closes part of [§quirks](#quirks)' degraded-graph concern for these types — a +recognizer answers from the object alone, so a truncated package graph no longer means a hard +`unable to find package and source file for: io.Reader`. + ## §embedded — embed routing, struct/interface specials asymmetry `buildEmbedded` is the entry point for a struct's embedded fields @@ -649,6 +702,57 @@ peel (recurse), `*types.Named` descends into `buildNamedEmbedded`, `*types.Alias` goes through `buildAlias` (so alias-resolution honours `TransparentAliases` / `RefAliases`). +### An embed that promotes nothing is an ordinary property + +`buildEmbedded` is only reached for an embed that actually **promotes**. Go promotes struct fields +and interface methods; a named type over a basic, slice, array or map has no member to promote, so +Go keeps the value as an ordinary key named after the **type**: + +```go +type Count int +type Host struct { + Count // → {"Count": 0, "label": "…"} + Label string `json:"label"` +} +``` + +`embedPromotes` (allof.go) makes that call, and `buildPlainEmbed` routes the false branch down the +same path as a **json-named** embed — because it is the same thing: a single named property built +from the embedded type, classifiers included. The name is the Go field name (which for an embed *is* +the type name), and the embed's own json tag renames or drops it exactly as on a regular field. + +This shape used to reach `buildNamedEmbedded`, whose switch has struct and interface arms only, and +fall to a `default` that warned `unsupported Go type` and skipped — wording that describes a type +codescan cannot model rather than one it silently drops. The default arm survives as a defensive +guard; nothing on the struct path reaches it now. + +### Why a promoted marshaller is not modelled + +A type reaching that false branch may implement `encoding.TextMarshaler`. Go promotes `MarshalText` +to the embedding struct, and under the **default** marshaller that makes the whole struct render as +a bare scalar — siblings and all: + +```go +type Token [16]byte +func (t Token) MarshalText() ([]byte, error) { return []byte("tok"), nil } + +type Embedder struct { Token; Name string `json:"name"` } +json.Marshal(Embedder{}) // → "tok" — "name" never reaches the wire +``` + +codescan deliberately does not model this. In the convention it describes, an embed means +**composition**, and a composed model round-trips through a hand-written +`MarshalJSON`/`UnmarshalJSON` rather than the default one — go-swagger's generated models embed +their `allOf` members *and* generate the marshaller that flattens them. A promoted marshaller in +hand-written source is therefore not evidence about the wire. + +Detecting it would also require answering a question a declaration cannot answer: a **pointer** +-receiver marshaller squashes for `&v` and not for `v`, and codescan reads the type, not the use +site. + +The author who does want the scalar says so on the embedded type's own declaration, with +`swagger:strfmt` or `swagger:type` — the escape hatch that already works. + ### `buildNamedEmbedded` — the two-arm specials asymmetry The interface arm runs `applyStdlibSpecials(o, target, skipExt)` @@ -1039,11 +1143,13 @@ filter protects against. `applySpecialType` and `applyStdlibSpecials` take a `skipExt bool` parameter that gates any vendor-extension writes the recognizers -would otherwise emit. Currently only `recognizeError` writes one -(`x-go-type: error`); the other recognizers +would otherwise emit. Two write one: `recognizeError` +(`x-go-type: error`) and `recognizeOpaqueStream` +(`x-go-type: .`). The others (`recognizeTime`, `recognizeAny`, `recognizeRawMessage`, `recognizeUUID`) are purely type / format mutations and don't -consult `skipExt`. All eight schema-internal call sites pass +consult `skipExt` — see [§traceability](#traceability) for why the +split falls where it does. All eight schema-internal call sites pass `s.skipExtensions` so the recognizer subsystem honours the same `SkipExtensions` flag as the rest of the builder. @@ -1105,8 +1211,13 @@ All three pass through `resolvers.AddExtension(..., s.skipExtensions)`, so `SkipExtensions` suppresses the whole family. `x-go-type` predates the option as a narrow type-rendering signal: the -generic `PkgForType` fallback (`special_types.go`) and `recognizeError` -stamp it deliberately to record an otherwise-unmodellable type. The +generic `PkgForType` fallback (`special_types.go`), `recognizeError` and +`recognizeOpaqueStream` stamp it deliberately to record an +otherwise-unmodellable type. The criterion is whether the emitted schema +still identifies the Go type: `time.Time` → `{string, date-time}` and +`uuid.UUID` → `{string, uuid}` do, so those recognizers stay silent; +`error` → `{string, ""}` and every stream → `{string, byte}` / `file` do +not — the latter collapses twelve distinct types onto one schema. The `annotateSchema` stamp is **presence-guarded** (`if _, exists := schema.Extensions["x-go-type"]; !exists`) so it never clobbers a value a recognizer already chose — for ordinary types the recognizer leaves it diff --git a/internal/builders/schema/allof.go b/internal/builders/schema/allof.go index 872a7cf5..022d9d83 100644 --- a/internal/builders/schema/allof.go +++ b/internal/builders/schema/allof.go @@ -48,8 +48,6 @@ func (s *Builder) scanEmbeddedFields( if fd.Ignored { continue } - s.warnIneffectiveEmbedAnnotations(afld, fd) - _, ignore, isString, omitEmpty, err := resolvers.ParseFieldTag(afld, fld.Name(), s.Ctx.NameFromTags()) if err != nil { return nil, false, err @@ -86,6 +84,7 @@ func (s *Builder) scanEmbeddedFields( } hasAllOf = true + s.warnIneffectiveEmbedAnnotations(afld, fd) if target == nil { target = &oaispec.Schema{} } @@ -156,6 +155,13 @@ func (s *Builder) buildPlainEmbed( } nestName := embedNestName(afld, fd) + if nestName == "" && !embedPromotes(fld.Type()) { + // An embed that promotes nothing is not a promotion at all: Go keeps the value as an ordinary + // member keyed by the TYPE name, so that is what the schema says. It takes the same path as a + // json-named embed because it IS the same thing — a single named property built from the + // embedded type, classifiers included. + nestName = fld.Name() + } if nestName != "" { err := s.applyFieldCarrier(fieldCarrier{ name: nestName, @@ -169,6 +175,10 @@ func (s *Builder) buildPlainEmbed( return target, err } + // Past this point the embed genuinely promotes, and no arm of the promotion walk consults the + // embed's own comment — so a classifier written there is dropped and must be reported. + s.warnIneffectiveEmbedAnnotations(afld, fd) + // A `required:` annotation on the embed applies to the properties it promotes (go-swagger#2701). // Thread it through the recursion, restoring afterwards so sibling fields are unaffected. saved := s.embedInherited @@ -191,6 +201,40 @@ func embedNestName(afld *ast.Field, fd fieldDoc) string { return resolvers.ExplicitJSONName(afld) } +// embedPromotes reports whether embedding tpe contributes PROMOTED members — struct fields or +// interface methods — rather than a single member named after the type. +// +// Anything else (a named type over a basic, slice, array or map) has no member to promote, so Go +// marshals it as an ordinary key named after the type. This used to reach `buildNamedEmbedded`, +// whose switch had struct and interface arms only, and fall to a warn-and-skip default that dropped +// the member from the schema entirely. +// +// # Why a promoted marshaller does not enter into it +// +// A type reaching the false branch may implement encoding.TextMarshaler, which Go promotes to the +// embedding struct and which makes the WHOLE struct marshal as a bare scalar under the DEFAULT +// marshaller — siblings and all. codescan deliberately does not model that: in the convention it +// describes, an embed means composition, and a composed model round-trips through a hand-written +// MarshalJSON/UnmarshalJSON (as go-swagger's generated models do) rather than the default one. A +// promoted marshaller in the source is therefore not evidence about the wire. Detecting it would +// also require deciding a case that is not decidable from a declaration: a POINTER-receiver +// marshaller squashes for `&v` and not for `v`, and codescan reads the type, not the use site. +// +// See [§embedded](./README.md#embedded). +func embedPromotes(tpe types.Type) bool { + unaliased := types.Unalias(tpe) + if ptr, ok := unaliased.(*types.Pointer); ok { + unaliased = types.Unalias(ptr.Elem()) + } + + switch unaliased.Underlying().(type) { + case *types.Struct, *types.Interface: + return true + default: + return false + } +} + // buildAllOf builds the schema for one allOf compound member. // // Peels pointers and routes named types and aliases to their dedicated helpers. diff --git a/internal/builders/schema/embedded.go b/internal/builders/schema/embedded.go index 75f36b6c..ec66cea6 100644 --- a/internal/builders/schema/embedded.go +++ b/internal/builders/schema/embedded.go @@ -67,6 +67,10 @@ func (s *Builder) buildEmbedded(tpe types.Type, schema *oaispec.Schema, nameByJS // The interface arm runs `ApplyStdlibSpecials` so `error` etc. recognize cleanly; the struct arm // does not — the asymmetry is intentional, see README §embedded. // +// Only an embed that PROMOTES reaches here: `embedPromotes` diverts a named type over a basic, +// slice, array or map to the named-property path, since Go has no member to promote for it. The +// default arm below is therefore a defensive guard rather than a live path. +// // # Details // // See [§embedded](./README.md#embedded) — `AddDiscoveredModel` pairing, struct-vs-interface diff --git a/internal/builders/schema/special_types.go b/internal/builders/schema/special_types.go index 5c107d49..564ab8b1 100644 --- a/internal/builders/schema/special_types.go +++ b/internal/builders/schema/special_types.go @@ -72,6 +72,12 @@ const ( // Caller-gated — opt in only where the type is guaranteed to render as text. // See [§special-types](./README.md#special-types). recognizeUUID + // recognizeOpaqueStream is an identity match on the known byte-stream carriers (io.Reader and + // friends, multipart.File, runtime.NamedReadCloser). + // + // Safe everywhere, so it lives in the canonical set applied by [ApplyStdlibSpecials]. + // See [§opaque-streams](./README.md#opaque-streams). + recognizeOpaqueStream ) // ApplyStdlibSpecials runs the canonical safe set of identity-based recognizers (any / time.Time / @@ -91,7 +97,8 @@ const ( // See [§special-types](./README.md#special-types). func ApplyStdlibSpecials(obj *types.TypeName, target ifaces.SwaggerTypable, skipExt bool) bool { return applySpecialType(obj, target, skipExt, - recognizeAny, recognizeTime, recognizeError, recognizeRawMessage, recognizeStdUUID) + recognizeAny, recognizeTime, recognizeError, recognizeRawMessage, recognizeStdUUID, + recognizeOpaqueStream) } // applySpecialType iterates wanted recognizers in order and applies the first match to target, @@ -142,6 +149,28 @@ func applySpecialType(obj *types.TypeName, target ifaces.SwaggerTypable, skipExt return true } + case recognizeOpaqueStream: // identity — see [§opaque-streams](./README.md#opaque-streams). + if resolvers.IsOpaqueStream(obj) { + // x-go-type records which stream this was, because neither answer below can: `byte` says + // base64 bytes and `file` says an upload, and every recognized type collapses onto the + // same schema either way. That is the `recognizeError` criterion — stamp when the + // rendering erases the type — and not the `time.Time` / uuid one, where the format IS + // the type. See [§traceability](./README.md#traceability). + if !skipExt { + target.AddExtension("x-go-type", obj.Pkg().Path()+"."+obj.Name()) + } + // The only two answers a stream can honestly take, chosen by what the position permits + // rather than by anything in the declaration. `file` is legal on a formData parameter and + // nowhere else in OAS 2.0, and it is the canonical upload shape; everywhere else the + // stream is base64 in a string, which is how OAS 2.0 spells "opaque bytes". + if target.In() == inFormData { + target.Typed("file", "") + return true + } + target.Typed("string", "byte") + return true + } + case recognizeUUID: // fuzzy — see [§special-types](./README.md#special-types). if obj != nil && strings.ToLower(obj.Name()) == "uuid" { target.Typed("string", "uuid") diff --git a/internal/integration/annotation_noise_test.go b/internal/integration/annotation_noise_test.go index 6a8cb946..c14d25e8 100644 --- a/internal/integration/annotation_noise_test.go +++ b/internal/integration/annotation_noise_test.go @@ -61,7 +61,17 @@ func TestAnnotationNoise(t *testing.T) { n++ } } - assert.Equal(t, 2, n, "one report per annotated embed — the allOf one and the plain one") + // Three embeds in the fixture carry a classifier; only the two that discard it are reported. + assert.Equal(t, 2, n, "one report per DISCARDING embed — the allOf one and the promoting one") + }) + + t.Run("a json-named embed is honoured, so it is not reported", func(t *testing.T) { + // Naming an embed with a json tag makes it a single named property instead of a promotion, and + // that path DOES consult the classifier. Reporting it was a false alarm: the author was told an + // annotation had been dropped while it was being applied. The count above is what pins it. + props := doc.Definitions["EffectiveOnNamedEmbed"].Properties + assert.Equal(t, "string/uuid", schemaSignature(props["nested"], doc.Definitions, 0), + "the classifier must reach a json-named embed") }) t.Run("the annotations are still ignored, not applied", func(t *testing.T) { diff --git a/internal/integration/embed_basic_underlying_test.go b/internal/integration/embed_basic_underlying_test.go new file mode 100644 index 00000000..5c938b24 --- /dev/null +++ b/internal/integration/embed_basic_underlying_test.go @@ -0,0 +1,113 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package integration_test + +import ( + "encoding/json" + "os" + "path/filepath" + "sort" + "testing" + + "github.com/go-openapi/codescan" + "github.com/go-openapi/codescan/internal/scantest" + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +// Differential test for embedding a named type whose underlying is neither a struct nor an +// interface: the emitted property set must equal the key set `encoding/json` puts on the wire. +// +// Such an embed promotes nothing — there is no field to promote — so Go keeps the value as an +// ordinary member named after the TYPE. `buildNamedEmbedded` had arms for struct and interface +// only, so every one of these fell to a warn-and-skip default and the member disappeared. The +// warning read "unsupported Go type", which describes a type codescan cannot model rather than one +// it silently drops. +// +// Like json-tag-fidelity this corpus has an ORACLE, so no expectation is written here: the fixture +// module marshals its own types and commits the raw documents as `wire.golden.json`. +// +// # The one deliberate divergence +// +// MarshalHost embeds a type implementing encoding.TextMarshaler, which promotes MarshalText and +// makes the whole struct marshal as a bare string under the DEFAULT marshaller. codescan does not +// model that, by decision: an embed means composition, and a composed model round-trips through a +// hand-written marshaller (as go-swagger's generated models do) rather than the default one. The +// oracle records the divergence rather than hiding it — that is why it stores raw documents rather +// than key sets, since this one is not an object at all. +func TestEmbedBasicUnderlying(t *testing.T) { + wire := loadEmbedWireGolden(t) + + doc, err := codescan.Run(&codescan.Options{ + Packages: []string{"./enhancements/embed-basic-underlying/..."}, + WorkDir: scantest.FixturesDir(), + ScanModels: true, + }) + require.NoError(t, err) + require.NotNil(t, doc) + + for name, raw := range wire { + t.Run(name, func(t *testing.T) { + def, ok := doc.Definitions[name] + require.True(t, ok, "missing definition %s", name) + + got := make([]string, 0, len(def.Properties)) + for k := range def.Properties { + got = append(got, k) + } + sort.Strings(got) + + var obj map[string]json.RawMessage + if err := json.Unmarshal(raw, &obj); err != nil { + // Not an object on the wire: the promoted-marshaller case. + require.Equal(t, "MarshalHost", name, + "only the promoted-marshaller subject may diverge from the oracle; %s marshalled to %s", name, raw) + assert.Equal(t, []string{"Token", "label"}, got, + "the member is built like any other embed of a named type — the promoted marshaller is not modelled") + + return + } + + want := make([]string, 0, len(obj)) + for k := range obj { + want = append(want, k) + } + sort.Strings(want) + + assert.Equal(t, want, got, + "the emitted property set must match what encoding/json marshals") + }) + } + + // The member is BUILT from the embedded type, not merely declared: a classifier on that type + // reaches it exactly as it would reach any named-type property. + t.Run("the embedded type's classifiers apply to the member", func(t *testing.T) { + props := doc.Definitions["FmtHost"].Properties + assert.Equal(t, "string/duration", schemaSignature(props["FmtBasic"], doc.Definitions, 0)) + }) + + t.Run("the embed's json tag names the property", func(t *testing.T) { + // The one embed shape where the json tag is meaningful again: it names an ordinary property + // instead of steering a promotion. + assert.Contains(t, doc.Definitions["TaggedHost"].Properties, "count") + assert.NotContains(t, doc.Definitions["OmittedHost"].Properties, "Count") + }) + + scantest.CompareOrDumpJSON(t, doc, "enhancements_embed_basic_underlying.json") +} + +// loadEmbedWireGolden reads the raw documents the fixture module captured from encoding/json. +func loadEmbedWireGolden(t *testing.T) map[string]json.RawMessage { + t.Helper() + + path := filepath.Join(scantest.FixturesDir(), "enhancements", "embed-basic-underlying", "wire.golden.json") + data, err := os.ReadFile(path) + require.NoError(t, err, "wire oracle missing — regenerate with UPDATE_GOLDEN=1 in the fixtures module") + + var wire map[string]json.RawMessage + require.NoError(t, json.Unmarshal(data, &wire)) + require.NotEmpty(t, wire) + + return wire +} diff --git a/internal/integration/opaque_streams_test.go b/internal/integration/opaque_streams_test.go new file mode 100644 index 00000000..b782066a --- /dev/null +++ b/internal/integration/opaque_streams_test.go @@ -0,0 +1,163 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package integration_test + +import ( + "testing" + + "github.com/go-openapi/codescan" + "github.com/go-openapi/codescan/internal/scantest" + oaispec "github.com/go-openapi/spec" + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +// The stdlib stream types are recognized by identity, so they never reach structural drilling. +// +// Two answers, decided by what the position allows rather than by anything in the declaration: +// `type: file` for a formData parameter — the only place OAS 2.0 permits it, and the canonical +// file-upload shape — and `{type: string, format: byte}` everywhere else. +// +// `byte` rather than `binary`: in a JSON body a raw octet sequence has no representation, and +// `byte` is the base64-encoded string OAS 2.0 defines for exactly that. It is not a claim about +// the content — a stream is opaque, and this is the standard way of saying so. +func TestOpaqueStreams(t *testing.T) { + doc, err := codescan.Run(&codescan.Options{ + Packages: []string{"./enhancements/opaque-streams/..."}, + WorkDir: scantest.FixturesDir(), + ScanModels: true, + }) + require.NoError(t, err) + require.NotNil(t, doc) + + t.Run("every recognized type is opaque bytes on a model field", func(t *testing.T) { + props := doc.Definitions["StreamModel"].Properties + require.NotEmpty(t, props) + + for name := range props { + assert.Equal(t, "string/byte", schemaSignature(props[name], doc.Definitions, 0), + "%s must be opaque bytes, not a drilled structure", name) + } + }) + + t.Run("x-go-type records which stream it was", func(t *testing.T) { + // Neither answer can carry it: `byte` says base64 bytes, `file` says an upload, and all twelve + // types collapse onto the same schema. Without the extension a consumer cannot tell an + // io.Reader field from a multipart.File one — the `recognizeError` criterion exactly. + props := doc.Definitions["StreamModel"].Properties + + for field, want := range map[string]string{ + "payload": "io.Reader", + "envelope": "io.ReadCloser", + "excerpt": "io.LimitedReader", + "upload": "mime/multipart.File", + "attachment": "github.com/go-openapi/runtime.NamedReadCloser", + } { + assert.Equal(t, want, props[field].Extensions["x-go-type"], + "%s must record its Go type", field) + } + + // x-go-name stays the FIELD name — the fixture names every field unlike its type so the two + // extensions can never be confused for one another. + assert.Equal(t, "Payload", props["payload"].Extensions["x-go-name"]) + }) + + t.Run("SkipExtensions suppresses the x-go-type stamp", func(t *testing.T) { + bare, err := codescan.Run(&codescan.Options{ + Packages: []string{"./enhancements/opaque-streams/..."}, + WorkDir: scantest.FixturesDir(), + ScanModels: true, + SkipExtensions: true, + }) + require.NoError(t, err) + + p := bare.Definitions["StreamModel"].Properties["payload"] + assert.NotContains(t, p.Extensions, "x-go-type") + assert.Equal(t, "string/byte", schemaSignature(p, bare.Definitions, 0), + "suppressing the extension must not change the type") + }) + + t.Run("no stdlib interface is published as a definition", func(t *testing.T) { + // The defect that made this visible: `io`'s own interfaces became definitions carrying io's + // godoc, and ReadCloser grew a `close` property of type string out of `Close() error`. + for _, leaked := range []string{"Reader", "ReadCloser", "ReadSeeker", "LimitedReader", "File", "NamedReadCloser"} { + assert.NotContains(t, doc.Definitions, leaked, + "a recognized stream type must not reach model discovery") + } + }) + + t.Run("an explicit annotation still wins", func(t *testing.T) { + props := doc.Definitions["OverriddenModel"].Properties + assert.Equal(t, "string/base64", schemaSignature(props["blob"], doc.Definitions, 0)) + assert.Equal(t, "string/", schemaSignature(props["handle"], doc.Definitions, 0)) + + // The two overrides treat the recognizer's stamp differently, and the difference follows from + // what each one means: swagger:strfmt adjusts the format, leaving the Go type it came from + // intact and recorded; swagger:type replaces the schema outright, so the record goes too. + assert.Equal(t, "io.Reader", props["blob"].Extensions["x-go-type"], + "a format override leaves the Go type recorded") + assert.NotContains(t, props["handle"].Extensions, "x-go-type", + "a type override replaces the schema, stamp included") + }) + + t.Run("a formData parameter is a file", func(t *testing.T) { + params := postParamsByName(t, doc, "/streams", "uploadStream") + + for _, name := range []string{"upload", "doc"} { + p, ok := params[name] + require.True(t, ok, "missing parameter %s", name) + assert.Equal(t, "formData", p.In) + assert.Equal(t, "file", p.Type, + "the canonical upload shape is type: file — and SimpleSchema requires SOME type here") + } + }) + + t.Run("a body parameter is opaque bytes", func(t *testing.T) { + params := postParamsByName(t, doc, "/streams", "uploadStream") + + p, ok := params["body"] + require.True(t, ok) + require.NotNil(t, p.Schema) + assert.Equal(t, "string/byte", schemaSignature(*p.Schema, doc.Definitions, 0), + "`file` is not legal on a body parameter") + }) + + t.Run("a non-body, non-formData parameter is opaque bytes", func(t *testing.T) { + params := postParamsByName(t, doc, "/streams", "uploadStream") + + p, ok := params["marker"] + require.True(t, ok) + assert.Equal(t, simpleSignature("string", "byte", nil), simpleSignature(p.Type, p.Format, p.Items)) + }) + + t.Run("a response body and a response header are opaque bytes", func(t *testing.T) { + resp, ok := doc.Responses["streamResponse"] + require.True(t, ok) + require.NotNil(t, resp.Schema) + assert.Equal(t, "string/byte", schemaSignature(*resp.Schema, doc.Definitions, 0)) + + h, ok := resp.Headers["XChecksum"] + require.True(t, ok, "missing response header; got %v", resp.Headers) + assert.Equal(t, simpleSignature("string", "byte", nil), simpleSignature(h.Type, h.Format, h.Items)) + }) + + scantest.CompareOrDumpJSON(t, doc, "enhancements_opaque_streams.json") +} + +// postParamsByName indexes a POST operation's parameters by name. +func postParamsByName(t *testing.T, doc *oaispec.Swagger, path, opID string) map[string]oaispec.Parameter { + t.Helper() + + item, ok := doc.Paths.Paths[path] + require.True(t, ok, "missing path %s", path) + require.NotNil(t, item.Post, "missing POST %s", path) + require.Equal(t, opID, item.Post.ID) + + params := make(map[string]oaispec.Parameter, len(item.Post.Parameters)) + for _, p := range item.Post.Parameters { + params[p.Name] = p + } + + return params +} diff --git a/internal/integration/strfmt_symmetry_composition_test.go b/internal/integration/strfmt_symmetry_composition_test.go index 990fd900..2b80f7e9 100644 --- a/internal/integration/strfmt_symmetry_composition_test.go +++ b/internal/integration/strfmt_symmetry_composition_test.go @@ -16,18 +16,23 @@ func TestStrfmtSymmetryComposition(t *testing.T) { pkg: "enhancements/strfmt-symmetry-composition", goldenPrefix: "strfmt_symmetry_composition", cells: []symmetryCell{ - // Plain embed: SYMMETRIC, and both halves are wrong the same way. buildNamedEmbedded switches - // on the member's underlying shape and never consults its comments, so the format is dropped on - // both sides — a basic member vanishes entirely, a struct member promotes its properties. Not a - // Q32 asymmetry; the same shared gap Q33 describes for TextMarshaler embeds. Left unasserted - // because what an embed of a formatted type SHOULD produce is an open design question. + // Plain embed of a BASIC-underlying type: the member no longer vanishes. Such an embed + // promotes nothing, so it is an ordinary property keyed by the Go field name and built from + // the embedded type — the format included. The two halves therefore differ by the only thing + // that legitimately differs between them, the identifier being embedded, which is why this is + // an exception rather than a symmetry failure. That the format lands is asserted where the + // signature can show it, in TestEmbedBasicUnderlying. { namedProp: "EmbedBasicNamed", aliasProp: "EmbedBasicAlias", - note: "SHARED GAP: both halves drop the member entirely (buildNamedEmbedded reads no comments) — see Q33", + wantNamed: "object{FmtBasicNamed,label}", }, + // Plain embed of a STRUCT-underlying type: still symmetric, still a shared gap. This one + // really does promote, and no arm of the promotion walk consults the embedded type's format — + // left unasserted because what a formatted type SHOULD contribute when its properties are + // promoted is an open question, not a bug with one answer. { namedProp: "EmbedStructNamed", aliasProp: "EmbedStructAlias", - note: "SHARED GAP: both halves promote left/right and drop the format — see Q33", + note: "SHARED GAP: both halves promote left/right and drop the format", }, // allOf member: the money row. The named arm runs classifierAliasTargetStrfmt (allof.go:205); @@ -42,7 +47,14 @@ func TestStrfmtSymmetryComposition(t *testing.T) { }, }, - exceptions: map[string]string{}, + // A promotes-nothing embed is keyed by the embedded IDENTIFIER, and the two halves of a pair + // are two different identifiers by construction. The difference is the fixture's, not the + // builder's. + exceptions: map[string]string{ + "default/EmbedBasic": "the property is named after the embedded identifier", + "refaliases/EmbedBasic": "the property is named after the embedded identifier", + "transparentaliases/EmbedBasic": "the property is named after the embedded identifier", + }, // The allOf alias arm now reads the member's declaration before dissolving, matching the // classifierAliasTargetStrfmt its named counterpart runs. knownBroken: map[string]string{},