diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index 17ae7d91..40359acb 100644 --- a/compilers/openapi/conformance_test.go +++ b/compilers/openapi/conformance_test.go @@ -8,6 +8,7 @@ import ( "encoding/json" "os" "path/filepath" + "reflect" "slices" "strings" "testing" @@ -1677,6 +1678,12 @@ func assertFormPartStyle(t *testing.T, doc *ir.Document) { assert.True(t, *pe.Explode) assert.True(t, pe.Multi, "the structural flag still comes from the part's own schema") } + // allowReserved has no PartEncoding field, and PartEncoding has no Unmodeled + // map, so it is kept on the content keyed by the part it governs. Two + // documents differing only in it used to compile to one IR (GitHub #291). + entry := unmodeledEntry(t, op.Request.Contents[0].Unmodeled, "openapi:encoding/ids/allowReserved") + assert.Equal(t, ir.ReasonNoIRHome, entry.Reason) + assert.JSONEq(t, "true", string(entry.Value)) } // assertFileBody pins a binary body: the payload's type degrades to bytes and the @@ -2013,21 +2020,111 @@ func assertExtensionsX(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { assert.JSONEq(t, "100", string(raw.Value)) assert.Equal(t, ir.ReasonVendorExtension, raw.Reason) - // The same rule applies at every object that admits an extension, so the - // document root and an operation each keep their own. - root, ok := doc.Unmodeled["openapi:x-audience"] - require.True(t, ok, "a root extension lands on the document; got %v", doc.Unmodeled) - assert.Equal(t, ir.ReasonVendorExtension, root.Reason) - assert.JSONEq(t, `"public"`, string(root.Value)) + assertEveryObjectKeepsItsExtensions(t, doc) + assertRawPreservedBinary(t, m) +} - op, ok := opByName(doc, "listWidgets") - require.True(t, ok) - entry, ok := op.Unmodeled["openapi:x-internal"] - require.True(t, ok, "an operation extension lands on the operation; got %v", op.Unmodeled) - assert.Equal(t, ir.ReasonVendorExtension, entry.Reason) - assert.JSONEq(t, `true`, string(entry.Value)) +// assertEveryObjectKeepsItsExtensions holds the whole rule rather than the +// positions that happened to be noticed: every OpenAPI object that admits an +// x-* keeps it. The fixture writes `x-mark` on each, with a value naming the +// object, so a row that stops arriving names exactly which lowering stopped +// reading — and a lowering that never read one fails here before it ships. +// +// Carriers are derived from the value graph, not named: the row says which +// Unmodeled map the entry must land on by the path the walk reaches it at, so a +// carrier that moves still matches and an entry written to the wrong one does +// not. +func assertEveryObjectKeepsItsExtensions(t *testing.T, doc *ir.Document) { + t.Helper() + sites := unmodeledSites(doc) + for _, tc := range []struct{ object, key, want, carrier string }{ + {"openapi root", "openapi:x-audience", `"public"`, "doc.Unmodeled"}, + {"info", "openapi:info/x-mark", `"XINFO"`, "doc.Unmodeled"}, + {"contact", "openapi:info/contact/x-mark", `"XCONTACT"`, "doc.Unmodeled"}, + {"license", "openapi:info/license/x-mark", `"XLICENSE"`, "doc.Unmodeled"}, + {"externalDocs", "openapi:externalDocs/x-mark", `"XEXTERNALDOCS"`, "doc.Unmodeled"}, + {"components", "openapi:components/x-mark", `"XCOMPONENTS"`, "doc.Unmodeled"}, + {"tag", "openapi:tags/0/x-mark", `"XTAG"`, "doc.Unmodeled"}, + {"tag externalDocs", "openapi:tags/0/externalDocs/x-mark", `"XTAGEXTERNALDOCS"`, "doc.Unmodeled"}, + {"server", "openapi:x-mark", `"XSERVER"`, "doc.Servers[0].Unmodeled"}, + {"server variable", "openapi:x-mark", `"XSERVERVARIABLE"`, "doc.Servers[0].Variables[0].Unmodeled"}, + {"paths", "openapi:paths/x-mark", `"XPATHS"`, "doc.Services[0].Unmodeled"}, + {"path item", "openapi:pathItem/x-mark", `"XPATHITEM"`, ".Unmodeled"}, + {"operation", "openapi:x-internal", `true`, ".Unmodeled"}, + {"operation externalDocs", "openapi:externalDocs/x-mark", `"XOPERATIONEXTERNALDOCS"`, ".Unmodeled"}, + {"responses", "openapi:responses/x-mark", `"XRESPONSES"`, ".Unmodeled"}, + {"parameter", "openapi:x-mark", `"XPARAMETER"`, ".Params[0].Unmodeled"}, + {"request body", "openapi:x-mark", `"XREQUESTBODY"`, ".Request.Unmodeled"}, + {"media type", "openapi:x-mark", `"XMEDIATYPE"`, ".Contents[0].Unmodeled"}, + {"encoding", "openapi:encoding/f/x-mark", `"XENCODING"`, ".Contents[0].Unmodeled"}, + {"example", "openapi:x-mark", `"XEXAMPLE"`, ".Examples[0].Unmodeled"}, + {"response", "openapi:x-mark", `"XRESPONSE"`, ".Responses[0].Unmodeled"}, + {"error response", "openapi:x-mark", `"XERRORRESPONSE"`, ".Errors[0].Unmodeled"}, + {"header", "openapi:x-mark", `"XHEADER"`, ".Headers[0].Unmodeled"}, + {"callback", "openapi:callbacks/onEvent/x-mark", `"XCALLBACK"`, ".Bindings.HTTP[0].Unmodeled"}, + {"schema xml", "openapi:xml/x-mark", `"XXML"`, "doc.Types[t/openapi/components/schemas/S].Unmodeled"}, + {"schema externalDocs", "openapi:externalDocs/x-mark", `"XSCHEMAEXTERNALDOCS"`, + "doc.Types[t/openapi/components/schemas/S].Unmodeled"}, + {"discriminator", "openapi:discriminator/x-mark", `"XDISCRIMINATOR"`, + "doc.Types[t/openapi/components/schemas/D].Unmodeled"}, + {"security scheme", "openapi:x-mark", `"XSECURITYSCHEME"`, + "doc.Auth[auth/openapi/components/securitySchemes/k].Unmodeled"}, + {"oauth flows", "openapi:flows/x-mark", `"XOAUTHFLOWS"`, + "doc.Auth[auth/openapi/components/securitySchemes/o].Unmodeled"}, + {"oauth flow", "openapi:x-mark", `"XOAUTHFLOW"`, + "doc.Auth[auth/openapi/components/securitySchemes/o].Flows[0].Unmodeled"}, + } { + site, found := findUnmodeled(sites, tc.key, tc.want) + if !assert.True(t, found, "%s extension is dropped: no %s = %s anywhere in the document", + tc.object, tc.key, tc.want) { + continue + } + assert.Equal(t, ir.ReasonVendorExtension, site.entry.Reason, "%s extension reason", tc.object) + assert.Contains(t, site.path, tc.carrier, "%s extension lands on the wrong carrier", tc.object) + } +} - assertRawPreservedBinary(t, m) +// unmodeledSite is one Unmodeled entry paired with the walk path of the map +// holding it. +type unmodeledSite struct { + key string + path string + entry ir.UnmodeledEntry +} + +// unmodeledSites returns every Unmodeled entry the document holds. It walks the +// value graph rather than naming carriers so that a test asserting "this is kept +// somewhere sensible" cannot pass by looking only where it expected to. +func unmodeledSites(doc *ir.Document) []unmodeledSite { + unmodeledType := reflect.TypeOf(ir.Unmodeled(nil)) + var out []unmodeledSite + ir.WalkValues(doc, ir.DocumentPath, func(v reflect.Value, path string) bool { + if v.Type() != unmodeledType || !v.CanInterface() { + return true + } + u, ok := v.Interface().(ir.Unmodeled) + if !ok { + return true + } + for key, entry := range u { + out = append(out, unmodeledSite{key: key, path: path, entry: entry}) + } + return true + }) + return out +} + +// findUnmodeled returns the site holding key with the given JSON value. The +// value is part of the match because one key spelling occurs at many carriers — +// "openapi:x-mark" is written on a dozen objects in this fixture — so matching +// on the key alone would find a different object's entry and call it a pass. +func findUnmodeled(sites []unmodeledSite, key, wantJSON string) (unmodeledSite, bool) { + for _, site := range sites { + if site.key == key && string(site.entry.Value) == wantJSON { + return site, true + } + } + return unmodeledSite{}, false } // assertRawPreservedBinary pins what a !!binary extension keeps: the base64 the diff --git a/compilers/openapi/conformance_unmodeled_test.go b/compilers/openapi/conformance_unmodeled_test.go index 7244a8c5..f475d1f2 100644 --- a/compilers/openapi/conformance_unmodeled_test.go +++ b/compilers/openapi/conformance_unmodeled_test.go @@ -387,9 +387,15 @@ func assertResidue(t *testing.T, p ir.Unmodeled, want map[string]string) { } } -// assertResponseLinks pins a response's links: ir.Response has no field for the -// link objects OpenAPI declares there, so they are kept verbatim on the response -// rather than dropped while the operation they name lowers normally. +// assertResponseLinks pins a response's links: neither ir.Response nor +// ir.ErrorCase has a field for the link objects OpenAPI declares there, so they +// are kept verbatim rather than dropped while the operation they name lowers +// normally. +// +// Both status ranges, because only the success one used to keep them: the same +// declaration survived on a 2xx and vanished on a 4xx, with no diagnostic either +// way, purely because the error branch had no links rule of its own +// (GitHub #275). func assertResponseLinks(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { op, ok := opByName(doc, "createOrder") require.True(t, ok) @@ -399,6 +405,15 @@ func assertResponseLinks(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { assert.JSONEq(t, `{"GetOrder":{"operationId":"getOrder","parameters":{"orderId":"$response.body#/id"}}}`, string(entry.Value)) + + require.Len(t, op.Errors, 1) + errEntry := unmodeledEntry(t, op.Errors[0].Unmodeled, "openapi:links") + assert.Equal(t, ir.ReasonNoIRHome, errEntry.Reason) + assert.JSONEq(t, + `{"GetConflicting":{"operationId":"getOrder","parameters":{"orderId":"$response.body#/existingId"}}}`, + string(errEntry.Value), + "an error response keeps its links by the same rule the success one does") + _, ok = opByName(doc, "getOrder") assert.True(t, ok, "the operation a link names is an ordinary operation") } diff --git a/compilers/openapi/internal/annotation/annotation.go b/compilers/openapi/internal/annotation/annotation.go index 886b8f58..a2125148 100644 --- a/compilers/openapi/internal/annotation/annotation.go +++ b/compilers/openapi/internal/annotation/annotation.go @@ -175,9 +175,31 @@ func XMLHints(x *oas3.XML) *ir.XMLHints { // key beneath it and marked ReasonVendorExtension, since the format assigns an // x-* key no semantics at all. func ExtensionsFrom(ext *extensions.Extensions, srcIndex int, owner string) (ir.Unmodeled, []ir.Diagnostic) { + return ExtensionsUnder(ext, srcIndex, owner, "") +} + +// ExtensionsUnder is ExtensionsFrom with every entry keyed beneath scope, for +// the objects whose extensions have no Unmodeled map of their own to land on. +// +// Most OpenAPI objects lower to an IR node that carries one, and those pass an +// empty scope. The rest ride on the nearest node that does — an info object's on +// the document, an encoding's on the content, a path item's on each of its +// operations — and several of them can reach the same map, where "openapi:x-id" +// from two objects is one key and the surviving entry would depend on which +// lowering ran last. scope names which object wrote them: the source path from +// the carrier down to it, or the object's own keyword where it is not beneath +// the carrier at all. +// +// A scoped key cannot collide with an unscoped one on the same map, since only +// an x-* key reaches here and no scope begins with "x-". +func ExtensionsUnder(ext *extensions.Extensions, srcIndex int, owner, scope string) (ir.Unmodeled, []ir.Diagnostic) { if ext == nil || ext.Len() == 0 { return nil, nil } + prefix := "openapi:" + if scope != "" { + prefix += scope + "/" + } out := ir.Unmodeled{} var diags []ir.Diagnostic for name, node := range ext.All() { @@ -188,7 +210,7 @@ func ExtensionsFrom(ext *extensions.Extensions, srcIndex int, owner string) (ir. "extension %q could not be serialized", name)) continue } - out["openapi:"+name] = ir.UnmodeledEntry{ + out[prefix+name] = ir.UnmodeledEntry{ Reason: ir.ReasonVendorExtension, Value: raw, Provenance: ir.Provenance{Source: srcIndex, Pointer: owner + ids.Ptr(name)}, @@ -200,6 +222,30 @@ func ExtensionsFrom(ext *extensions.Extensions, srcIndex int, owner string) (ir. return out, diags } +// ExtensionSite is one object's x-* map paired with where it was written: Owner +// is the object's own source pointer, and Scope is what its entries key under on +// the carrier that ends up holding them (see ExtensionsUnder). +type ExtensionSite struct { + Scope string + Owner string + Ext *extensions.Extensions +} + +// ExtensionsAt folds every site into one Unmodeled map, for the carriers that +// hold more than one object's extensions. Sites are applied in the order given, +// which is source order at every caller; distinct scopes cannot collide, so the +// order decides nothing but is fixed anyway. +func ExtensionsAt(srcIndex int, sites ...ExtensionSite) (ir.Unmodeled, []ir.Diagnostic) { + var out ir.Unmodeled + var diags []ir.Diagnostic + for _, site := range sites { + ext, extDiags := ExtensionsUnder(site.Ext, srcIndex, site.Owner, site.Scope) + out = MergeUnmodeled(out, ext) + diags = append(diags, extDiags...) + } + return out, diags +} + // MergeUnmodeled overlays src onto dst, allocating dst on first write. func MergeUnmodeled(dst, src ir.Unmodeled) ir.Unmodeled { if len(src) == 0 { @@ -498,14 +544,51 @@ func Read(st Site, pointer string, srcIndex int) (Set, []ir.Diagnostic) { out.Examples = examples ext, extDiags := ExtensionsFrom(st.Node.GetExtensions(), srcIndex, pointer) + sub, subDiags := subObjectKeys(st.Node, pointer, srcIndex) kept, keptDiags := unmodeledAt(st.Node, pointer, srcIndex) - diags := make([]ir.Diagnostic, 0, len(exDiags)+len(extDiags)+len(keptDiags)) + diags := make([]ir.Diagnostic, 0, len(exDiags)+len(extDiags)+len(subDiags)+len(keptDiags)) diags = append(diags, exDiags...) diags = append(diags, extDiags...) + diags = append(diags, subDiags...) diags = append(diags, keptDiags...) - out.Unmodeled = MergeUnmodeled(ext, kept) + out.Unmodeled = MergeUnmodeled(MergeUnmodeled(ext, sub), kept) + return out, diags +} + +// subObjectKeys collects what the sub-objects of a schema declare that reaches +// no IR field — the x-* they carry and the keys the specification defines for +// none of them — over its xml, its discriminator and its externalDocs. +// +// Each is an OpenAPI object with its own closed key set, and none of +// ir.XMLHints, ir.Discriminator or ir.Link holds an Unmodeled map, so the +// entries ride on the node the schema itself lowers to; the keyword each was +// written under is what keeps three objects' entries apart on that one map. +// +// The census is graded as an OpenAPI object's rather than as a schema keyword's, +// even though these hang off a schema: the JSON Schema rule that an unrecognized +// keyword is legal governs the schema itself, and these three are OpenAPI +// objects that the schema vocabulary says nothing about. +func subObjectKeys(s *oas3.Schema, pointer string, srcIndex int) (ir.Unmodeled, []ir.Diagnostic) { + subs := []struct { + keyword string + obj any + ext *extensions.Extensions + }{ + {"xml", s.GetXML(), s.GetXML().GetExtensions()}, + {"discriminator", s.GetDiscriminator(), s.GetDiscriminator().GetExtensions()}, + {"externalDocs", s.GetExternalDocs(), s.GetExternalDocs().GetExtensions()}, + } + var out ir.Unmodeled + var diags []ir.Diagnostic + for _, sub := range subs { + owner := pointer + ids.Ptr(sub.keyword) + ext, extDiags := ExtensionsUnder(sub.ext, srcIndex, owner, sub.keyword) + out = MergeUnmodeled(out, ext) + diags = append(diags, extDiags...) + diags = append(diags, UnknownKeysUnder(&out, sub.obj, srcIndex, owner, sub.keyword)...) + } return out, diags } diff --git a/compilers/openapi/internal/annotation/readers_internal_test.go b/compilers/openapi/internal/annotation/readers_internal_test.go index 4ffaf5f1..30bb7bc1 100644 --- a/compilers/openapi/internal/annotation/readers_internal_test.go +++ b/compilers/openapi/internal/annotation/readers_internal_test.go @@ -303,6 +303,58 @@ func TestExtensionsFrom_UnserializableIsWarnedNotKept(t *testing.T) { assert.Contains(t, diags[0].Message, `"x-bad"`) } +// TestExtensionsUnder_KeysBeneathTheScope pins the scoped spelling: an object +// with no Unmodeled map of its own keys its entries under the path that says +// which object wrote them, while the entry's provenance still points at the +// extension itself. +func TestExtensionsUnder_KeysBeneathTheScope(t *testing.T) { + t.Parallel() + s := schemaFromYAML(t, "type: string\nx-a: 1\n") + + got, diags := ExtensionsUnder(s.GetExtensions(), 2, "/info/contact", "info/contact") + + assert.Empty(t, diags) + require.Len(t, got, 1) + require.Contains(t, got, "openapi:info/contact/x-a") + assert.Equal(t, ir.Provenance{Source: 2, Pointer: "/info/contact/x-a"}, + got["openapi:info/contact/x-a"].Provenance) +} + +// TestExtensionsAt_FoldsEverySiteWithoutCollision is the reason the scope +// exists: two objects writing the same x-* key onto one carrier must both +// survive, which one unscoped key cannot do. +func TestExtensionsAt_FoldsEverySiteWithoutCollision(t *testing.T) { + t.Parallel() + info := schemaFromYAML(t, "type: string\nx-a: 1\n") + license := schemaFromYAML(t, "type: string\nx-a: 2\n") + + got, diags := ExtensionsAt(0, + ExtensionSite{Scope: "info", Owner: "/info", Ext: info.GetExtensions()}, + ExtensionSite{Scope: "info/license", Owner: "/info/license", Ext: license.GetExtensions()}, + ExtensionSite{Scope: "components", Owner: "/components", Ext: nil}, + ) + + assert.Empty(t, diags) + require.Len(t, got, 2, "one key spelling, two objects, two entries") + assert.Equal(t, ir.RawValue("1"), got["openapi:info/x-a"].Value) + assert.Equal(t, ir.RawValue("2"), got["openapi:info/license/x-a"].Value) +} + +// TestExtensionsAt_ReportsEverySiteThatFailed holds the diagnostics to the same +// completeness as the entries: a site whose extension cannot be serialized keeps +// nothing, and the fold must still carry its warning out (GitHub #144's rule, +// applied to the multi-site form). +func TestExtensionsAt_ReportsEverySiteThatFailed(t *testing.T) { + t.Parallel() + bad := schemaFromYAML(t, "type: string\nx-bad: .nan\n") + + got, diags := ExtensionsAt(0, ExtensionSite{Scope: "info", Owner: "/info", Ext: bad.GetExtensions()}) + + assert.Nil(t, got, "nothing was kept, so there is no map to emit") + require.Len(t, diags, 1) + assert.Equal(t, ir.SeverityWarning, diags[0].Severity) +} + // TestMergeUnmodeled_AllocatesOnlyOnFirstWrite pins the overlay: merging nothing // leaves the destination exactly as it was — including nil, which must not // become an empty map. diff --git a/compilers/openapi/internal/annotation/unknown.go b/compilers/openapi/internal/annotation/unknown.go new file mode 100644 index 00000000..bab5accd --- /dev/null +++ b/compilers/openapi/internal/annotation/unknown.go @@ -0,0 +1,207 @@ +package annotation + +import ( + "reflect" + "slices" + + oas3 "github.com/speakeasy-api/openapi/jsonschema/oas3" + yaml "gopkg.in/yaml.v3" + + "github.com/dexpace/morphic/compilers/openapi/internal/diag" + "github.com/dexpace/morphic/compilers/openapi/internal/ids" + "github.com/dexpace/morphic/ir" +) + +// MaxUnknownKeys bounds how many keys one object contributes to the IR. +// +// The key set is the document's to choose the size of, and every collection in +// this compiler is bounded, so this one is too. It sits far above what a +// document writes by accident, so an object reaching it is generated or hostile +// rather than merely sloppy, and what it discards is announced under +// diag.UnknownKeyBudget rather than dropped in silence. +const MaxUnknownKeys = 64 + +// DecidedKeywords are the JSON Schema keywords the library's schema model names +// no field for and this compiler has already decided about, so the census must +// not claim them as unread. Each decision is recorded where it was made, and the +// schema walk's 2020-12 vocabulary test fails if one starts being carried: +// +// - $comment — 2020-12 §8.3 forbids presenting it to end users, so no SDK +// emitter may see it. Dropped on purpose. +// - $dynamicAnchor — read by the anchor index as a reference target, which is +// what lets a $dynamicRef expand; declaring one says nothing about the shape. +// - $dynamicRef — carried by the dynamic-reference lowering, which either +// expands it into the position's type or keeps it under a reason of its own. +// An entry beside an expanded one would tell a consumer the compiler ignored +// a reference it had in fact resolved. +// +// The other 2020-12 keywords with no field of their own — $vocabulary and +// dependentRequired — need no entry here. Their readers write to the same map, +// so the census finds them already recorded and leaves them alone. +var DecidedKeywords = []string{"$comment", "$dynamicAnchor", "$dynamicRef"} + +// UnknownKeywordsIn records on p the keywords s writes that no field of the JSON +// Schema model names, and announces each. +// +// OpenAPI 3.1 schemas are JSON Schema 2020-12, where an unrecognized keyword is +// legal input: the specification requires an implementation to ignore what it +// does not recognize and allows such a keyword to carry meaning for other +// tooling. So this reports a decision rather than a fault, and is graded +// accordingly — see diag.UnknownSchemaKeyword. +// +// It keeps only what no other reader kept, which is why it runs after all of +// them: `$vocabulary` and `dependentRequired` have no field in the model either +// and are read straight off the raw node by readers with more to say about them, +// so the census finds those already recorded and leaves them alone. A keyword no +// reader leaves a trace of needs naming in DecidedKeywords instead. +func UnknownKeywordsIn(p *ir.Unmodeled, s *oas3.Schema, pointer string, srcIndex int) []ir.Diagnostic { + return census(p, s, srcIndex, pointer, "", keyClass{ + code: diag.UnknownSchemaKeyword, + severity: ir.SeverityInfo, + skip: DecidedKeywords, + message: "keyword %q has no field in the schema model this compiler lowers and no IR " + + "position of its own; kept verbatim under Unmodeled", + }) +} + +// UnknownKeysIn records on p the keys an OpenAPI object writes that the +// specification neither defines nor admits as an extension, for an object +// lowering to a node with an Unmodeled map of its own. owner is the object's own +// source pointer. +// +// Unlike its schema neighbour this reports a fault: OpenAPI gives each of its +// objects a closed key set and requires every extension to be prefixed x-, so a +// key that is neither is nothing the document is permitted to write — in +// practice a misspelling of the field beside it. It is kept all the same, +// because invariant 2 does not bend for invalid input, and a misspelt key is the +// one a reader most needs to find. +func UnknownKeysIn(p *ir.Unmodeled, model any, srcIndex int, owner string) []ir.Diagnostic { + return UnknownKeysUnder(p, model, srcIndex, owner, "") +} + +// UnknownKeysUnder is UnknownKeysIn with every entry keyed beneath scope, for +// the objects with no Unmodeled map of their own, whose keys ride on the nearest +// node that has one — an info object's on the document, a tag's on the document. +// +// scope says which object wrote them: the source path from the carrier down to +// the object. Several objects reach one map, where "openapi:status" from two of +// them would be a single key and the entry that survived would depend on which +// lowering ran last. +func UnknownKeysUnder(p *ir.Unmodeled, model any, srcIndex int, owner, scope string) []ir.Diagnostic { + return census(p, model, srcIndex, owner, scope, keyClass{ + code: diag.UnknownObjectKey, + severity: ir.SeverityWarning, + message: "key %q is not defined by the OpenAPI object it is written on and is not an " + + "x- extension; kept verbatim under Unmodeled", + }) +} + +// keyClass is how a key the model does not name is graded: which diagnostic +// announces it, and at what severity. +// +// The reason is not part of it. Both classes carry ReasonOutOfScope, because +// that is a property of the construct rather than of the document: no IR node is +// coming for a key the format does not define, nor for one a schema dialect +// defines and this compiler does not model, so an emitter policy layer is the +// only consumer either has. Which of the two a key is says something about the +// source, and the diagnostic channel is where this compiler says that. +type keyClass struct { + code string + severity ir.Severity + skip []string // keywords already decided about; see DecidedKeywords + message string // one %q, filled with the key +} + +// census records on p every key model's source object wrote that its own model +// names no field for, each under its own key beneath scope. +// +// A key p already holds is left alone and not announced: the census is the +// complement of everything the compiler read, not only of what the model names, +// and a reader with a reason of its own for a keyword has already said it +// better. +func census(p *ir.Unmodeled, model any, srcIndex int, owner, scope string, cl keyClass) []ir.Diagnostic { + keys, root := undeclaredKeys(model) + if len(keys) == 0 { + return nil + } + var diags []ir.Diagnostic + if len(keys) > MaxUnknownKeys { + diags = append(diags, budgetDiag(len(keys), owner, srcIndex)) + keys = keys[:MaxUnknownKeys] + } + for _, key := range keys { + entry := "openapi:" + scoped(scope, key) + if _, recorded := (*p)[entry]; recorded || slices.Contains(cl.skip, key) { + continue + } + at := owner + ids.Ptr(key) + kept, keptDiags := PreserveNodeInto(p, entry, RawChildNode(root, key), + ir.ReasonOutOfScope, at, srcIndex) + diags = append(diags, keptDiags...) + if !kept { + continue + } + diags = append(diags, diag.Newf(cl.severity, cl.code, + ir.Provenance{Source: srcIndex, Pointer: at}, cl.message, key)) + } + return diags +} + +// scoped spells one entry's key on the carrier holding it. +func scoped(scope, key string) string { + if scope == "" { + return key + } + return scope + "/" + key +} + +// budgetDiag reports the keys past MaxUnknownKeys, which reach the IR in no form. +func budgetDiag(total int, owner string, srcIndex int) ir.Diagnostic { + return diag.Newf(ir.SeverityWarning, diag.UnknownKeyBudget, + ir.Provenance{Source: srcIndex, Pointer: owner}, + "object writes %d keys its model names no field for, past the %d this compiler keeps; "+ + "the rest are represented in the IR in no form at all", total, MaxUnknownKeys) +} + +// parsedObject is the part of a parsed model the census reads: its core, which +// holds the census the unmarshaller took, and the mapping node the keys were +// written on. +// +// Declared here rather than taken from the library, so this package depends on +// the shape it uses rather than on the marshaller package, and so a test can +// drive the branches below with a model of its own. +type parsedObject interface { + GetCoreAny() any + GetRootNode() *yaml.Node +} + +// unknownReporter is a core model's own record of the keys it did not name. +type unknownReporter interface{ GetUnknownProperties() []string } + +// undeclaredKeys returns, sorted, the keys model's source object wrote that its +// model names no field for, and the mapping node they were written on. +// +// Sorted, and on a copy: the library fills that list from a parallel walk of the +// mapping under a mutex, so its order is neither source order nor stable, and +// the slice it hands back is the model's own. An unsorted read would order this +// compiler's diagnostics by something the source does not decide, which +// invariant 7 forbids. +// +// A model reporting no census yields nothing rather than panicking. The receiver +// may be a typed nil — an absent object is what the getters return for one the +// document omitted — and a promoted method on one of those dereferences it. +func undeclaredKeys(model any) ([]string, *yaml.Node) { + v := reflect.ValueOf(model) + if v.Kind() == reflect.Pointer && v.IsNil() { + return nil, nil + } + obj, ok := model.(parsedObject) + if !ok { + return nil, nil + } + core, ok := obj.GetCoreAny().(unknownReporter) + if !ok { + return nil, nil + } + return slices.Sorted(slices.Values(core.GetUnknownProperties())), obj.GetRootNode() +} diff --git a/compilers/openapi/internal/annotation/unknown_internal_test.go b/compilers/openapi/internal/annotation/unknown_internal_test.go new file mode 100644 index 00000000..764a402b --- /dev/null +++ b/compilers/openapi/internal/annotation/unknown_internal_test.go @@ -0,0 +1,212 @@ +package annotation + +import ( + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + yaml "gopkg.in/yaml.v3" + + "github.com/dexpace/morphic/ir" +) + +// TestUnknownKeywordsIn_KeepsWhatTheModelDoesNotName is the census at its own +// subject: a keyword with no field in the schema model reaches the IR under its +// own key, located at itself, and is announced at info. +func TestUnknownKeywordsIn_KeepsWhatTheModelDoesNotName(t *testing.T) { + t.Parallel() + s := schemaFromYAML(t, "type: string\nx-ray: kept\nnotAKeyword: 7\n") + + var got ir.Unmodeled + diags := UnknownKeywordsIn(&got, s, "/components/schemas/A", 3) + + require.Len(t, got, 1, "the x-* is the extension reader's, not the census's; got %v", got) + entry := got["openapi:notAKeyword"] + assert.Equal(t, ir.ReasonOutOfScope, entry.Reason) + assert.Equal(t, ir.RawValue("7"), entry.Value) + assert.Equal(t, ir.Provenance{Source: 3, Pointer: "/components/schemas/A/notAKeyword"}, entry.Provenance) + + require.Len(t, diags, 1) + assert.Equal(t, ir.SeverityInfo, diags[0].Severity) + assert.Equal(t, "openapi/unknown-schema-keyword", diags[0].Code) + assert.Contains(t, diags[0].Message, `"notAKeyword"`) +} + +// TestUnknownKeywordsIn_DecidedKeywordsAreLeftAlone holds the census to the +// decisions already recorded elsewhere. Each of these has no field in the schema +// model, so the census would otherwise claim all three and overrule a +// deliberate drop — or, for an expanded $dynamicRef, say the compiler ignored a +// reference it resolved. +func TestUnknownKeywordsIn_DecidedKeywordsAreLeftAlone(t *testing.T) { + t.Parallel() + assert.Equal(t, []string{"$comment", "$dynamicAnchor", "$dynamicRef"}, DecidedKeywords, + "a keyword joining or leaving the exclusion must be decided here too") + + var body strings.Builder + body.WriteString("type: string\n") + for _, keyword := range DecidedKeywords { + body.WriteString(keyword + ": v\n") + } + s := schemaFromYAML(t, body.String()) + + var got ir.Unmodeled + diags := UnknownKeywordsIn(&got, s, "/components/schemas/A", 0) + + assert.Empty(t, got, "each is decided about elsewhere, so the census keeps none of them") + assert.Empty(t, diags) +} + +// TestUnknownKeywordsIn_AlreadyRecordedKeyIsLeftAlone is why the census runs +// last. dependentRequired has no field in the model either and is kept by the +// validation-only reader under a reason that says what it is; a census that +// overwrote it would replace that with a weaker one and announce the keyword +// twice. +func TestUnknownKeywordsIn_AlreadyRecordedKeyIsLeftAlone(t *testing.T) { + t.Parallel() + s := schemaFromYAML(t, "type: object\ndependentRequired: {a: [b]}\n") + already := ir.UnmodeledEntry{ + Reason: ir.ReasonValidationOnly, + Value: ir.RawValue(`{"a":["b"]}`), + Provenance: ir.Provenance{Pointer: "/A/dependentRequired"}, + } + got := ir.Unmodeled{"openapi:dependentRequired": already} + + diags := UnknownKeywordsIn(&got, s, "/A", 0) + + assert.Equal(t, ir.Unmodeled{"openapi:dependentRequired": already}, got) + assert.Empty(t, diags, "a keyword another reader already announced is not announced twice") +} + +// TestUnknownKeywordsIn_UnpreservableValueIsReportedNotKept holds the census to +// GitHub #144's rule: a value that cannot be rendered as JSON keeps nothing, and +// says so, rather than announcing a preservation that did not happen. +func TestUnknownKeywordsIn_UnpreservableValueIsReportedNotKept(t *testing.T) { + t.Parallel() + s := schemaFromYAML(t, "type: string\nnotAKeyword: .nan\n") + + var got ir.Unmodeled + diags := UnknownKeywordsIn(&got, s, "/A", 0) + + assert.Empty(t, got) + require.Len(t, diags, 1) + assert.Equal(t, ir.SeverityError, diags[0].Severity) + assert.Equal(t, "openapi/unpreservable-construct", diags[0].Code) +} + +// TestUnknownKeysUnder_KeysBeneathTheScopeAndSorted pins the OpenAPI-object +// class: entries key under the path that says which object wrote them, the +// grading is a warning because the format admits no such key, and both the +// entries and the findings come out in key order. +// +// Sorted matters on its own. The library builds its census from a parallel walk +// of the mapping, so the order it hands back is neither source order nor stable, +// and diagnostics ordered by it would make the document non-deterministic +// (invariant 7). +func TestUnknownKeysUnder_KeysBeneathTheScopeAndSorted(t *testing.T) { + t.Parallel() + reported := []string{"zeta", "alpha"} + obj := fakeObject{core: &fakeCore{keys: reported}, root: parsedMapping(t, "zeta: 1\nalpha: 2\n")} + + var got ir.Unmodeled + diags := UnknownKeysUnder(&got, obj, 1, "/info/contact", "info/contact") + + assert.Equal(t, []string{"zeta", "alpha"}, reported, "the model's own slice is not reordered") + require.Len(t, got, 2) + assert.Equal(t, ir.RawValue("2"), got["openapi:info/contact/alpha"].Value) + assert.Equal(t, ir.Provenance{Source: 1, Pointer: "/info/contact/zeta"}, + got["openapi:info/contact/zeta"].Provenance) + + require.Len(t, diags, 2) + assert.Equal(t, ir.SeverityWarning, diags[0].Severity) + assert.Equal(t, "openapi/unknown-object-key", diags[0].Code) + assert.Contains(t, diags[0].Message, `"alpha"`, "the findings follow the sorted keys") + assert.Contains(t, diags[1].Message, `"zeta"`) +} + +// TestUnknownKeysIn_BudgetBoundsWhatOneObjectContributes exercises the bound. An +// object writing more undeclared keys than MaxUnknownKeys keeps exactly that +// many and reports the remainder, which is what stops the discarded tail from +// being the silent loss this census exists to end. +func TestUnknownKeysIn_BudgetBoundsWhatOneObjectContributes(t *testing.T) { + t.Parallel() + const over = MaxUnknownKeys + 3 + keys := make([]string, 0, over) + var body strings.Builder + for i := range over { + // Zero-padded, so sorting the census is sorting the source order too and the + // key that survives the truncation is a predictable one. + key := "k" + strconv.Itoa(1000+i) + keys = append(keys, key) + body.WriteString(key + ": " + strconv.Itoa(i) + "\n") + } + obj := fakeObject{core: &fakeCore{keys: keys}, root: parsedMapping(t, body.String())} + + var got ir.Unmodeled + diags := UnknownKeysIn(&got, obj, 0, "/x") + + assert.Len(t, got, MaxUnknownKeys) + assert.NotContains(t, got, "openapi:k"+strconv.Itoa(1000+over-1), "the tail past the bound is dropped") + require.NotEmpty(t, diags) + assert.Equal(t, "openapi/unknown-key-budget", diags[0].Code) + assert.Equal(t, ir.SeverityWarning, diags[0].Severity) + assert.Equal(t, ir.Provenance{Pointer: "/x"}, diags[0].Provenance) +} + +// TestUnknownKeysIn_ModelWithNoCensusRecordsNothing covers the shapes the reader +// must survive rather than panic on. The absent object is the one that occurs: +// the getters hand back a typed nil for an object the document omitted, and a +// promoted method on one of those dereferences it. +func TestUnknownKeysIn_ModelWithNoCensusRecordsNothing(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + model any + }{ + {"an object the document omitted", (*fakeObjectPtr)(nil)}, + {"an untyped nil", nil}, + {"a value that is no parsed model", 42}, + {"a model whose core keeps no census", fakeObject{core: "not a core"}}, + {"a model with an empty census", fakeObject{core: &fakeCore{}}}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + var got ir.Unmodeled + + diags := UnknownKeysIn(&got, tc.model, 0, "/x") + + assert.Nil(t, got) + assert.Empty(t, diags) + }) + } +} + +// fakeObject is a parsed model standing in for the library's, so the census can +// be driven at shapes no real document produces — an unsorted census, one past +// the bound, and a core that keeps none. +type fakeObject struct { + core any + root *yaml.Node +} + +func (f fakeObject) GetCoreAny() any { return f.core } +func (f fakeObject) GetRootNode() *yaml.Node { return f.root } + +// fakeObjectPtr is fakeObject's pointer-receiver twin, for the typed-nil case: +// a promoted method on a nil pointer is what the guard exists for, and a +// value-receiver method on a nil pointer would not reach it. +type fakeObjectPtr struct{ fakeObject } + +// fakeCore is a core model's census, reported exactly as given. +type fakeCore struct{ keys []string } + +func (c *fakeCore) GetUnknownProperties() []string { return c.keys } + +// parsedMapping parses body into the mapping node the census reads values from. +func parsedMapping(t *testing.T, body string) *yaml.Node { + t.Helper() + var doc yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(body), &doc)) + return &doc +} diff --git a/compilers/openapi/internal/auth/auth.go b/compilers/openapi/internal/auth/auth.go index b04dbe96..fdfc82af 100644 --- a/compilers/openapi/internal/auth/auth.go +++ b/compilers/openapi/internal/auth/auth.go @@ -132,9 +132,54 @@ func lowerSecurityScheme(c lowering.Ctx, name string, ss *soa.SecurityScheme, return ir.AuthScheme{}, false, []ir.Diagnostic{mechanismRefusalDiag(c, name, missing, entry)} } diags = preserveUnreadFields(c, &scheme, ss, decl) - ext, extDiags := annotation.ExtensionsFrom(ss.GetExtensions(), c.SrcIndex, decl) + diags = append(diags, applySchemeAnnotations(c, &scheme, ss, decl)...) + // Distinct from preserveUnreadFields above it: that keeps the fields OpenAPI + // defines for a securityScheme which this entry's own mechanism gives no + // meaning to, while this keeps the keys OpenAPI defines for no securityScheme + // at all. + return scheme, true, append(diags, + annotation.UnknownKeysIn(&scheme.Unmodeled, ss, c.SrcIndex, decl)...) +} + +// applySchemeAnnotations keeps what the securitySchemes entry and, for an oauth2 +// scheme, the flows object and each flow inside it declare that reaches no IR +// field: their x-*, and the keys OpenAPI defines for none of them. ir.OAuthFlow +// carries an Unmodeled map of its own; the flows object does not lower to a node +// at all, so its entries are kept on the scheme under the keyword. +// +// Only oauth2 reads inside `flows`: on any other type the whole node is kept +// verbatim by preserveUnreadFields, extensions and all, and no ir.OAuthFlow was +// lowered for a flow's own to land on. +func applySchemeAnnotations(c lowering.Ctx, scheme *ir.AuthScheme, ss *soa.SecurityScheme, decl string) []ir.Diagnostic { + ext, diags := annotation.ExtensionsFrom(ss.GetExtensions(), c.SrcIndex, decl) scheme.Unmodeled = annotation.MergeUnmodeled(scheme.Unmodeled, ext) - return scheme, true, append(diags, extDiags...) + if scheme.Kind != ir.AuthKindOAuth2 { + return diags + } + flows := ss.GetFlows() + flowsPtr := decl + ids.Ptr("flows") + flowsExt, flowsDiags := annotation.ExtensionsUnder(flows.GetExtensions(), c.SrcIndex, flowsPtr, "flows") + scheme.Unmodeled = annotation.MergeUnmodeled(scheme.Unmodeled, flowsExt) + diags = append(diags, flowsDiags...) + diags = append(diags, annotation.UnknownKeysUnder(&scheme.Unmodeled, flows, c.SrcIndex, flowsPtr, "flows")...) + return append(diags, applyFlowAnnotations(c, scheme.Flows, flows, flowsPtr)...) +} + +// applyFlowAnnotations writes each declared flow's own x-* and undeclared keys +// onto the ir.OAuthFlow it lowered to. Both lists are one ordered reading of the +// same object — scheme.Flows came from oauthFlows, which walks presentFlows — so +// the i-th lowered flow is the i-th declared one and the two cannot differ in +// length. +func applyFlowAnnotations(c lowering.Ctx, lowered []ir.OAuthFlow, flows *soa.OAuthFlows, flowsPtr string) []ir.Diagnostic { + var diags []ir.Diagnostic + for i, f := range presentFlows(flows) { + fptr := flowsPtr + ids.Ptr(f.keyword) + ext, extDiags := annotation.ExtensionsFrom(f.src.GetExtensions(), c.SrcIndex, fptr) + lowered[i].Unmodeled = annotation.MergeUnmodeled(lowered[i].Unmodeled, ext) + diags = append(diags, extDiags...) + diags = append(diags, annotation.UnknownKeysIn(&lowered[i].Unmodeled, f.src, c.SrcIndex, fptr)...) + } + return diags } // mechanismRefusalDiag reports a securitySchemes entry that declares a scheme @@ -312,28 +357,47 @@ func preserveUnreadFields(c lowering.Ctx, scheme *ir.AuthScheme, ss *soa.Securit return diags } +// sourceFlow is one declared OAuth2 flow: the IR kind it lowers to, the keyword +// it is written under, and the object itself. +type sourceFlow struct { + kind string + keyword string + src *soa.OAuthFlow +} + +// presentFlows returns the flows an OAuthFlows object declares, in a fixed, +// deterministic order. It is the single ordered reading of that object: the +// lowering and the extension reader both walk it, which is what lets the i-th +// ir.OAuthFlow be matched back to the i-th declaration without a second, and +// possibly divergent, enumeration. +func presentFlows(flows *soa.OAuthFlows) []sourceFlow { + candidates := []sourceFlow{ + {"authorization_code", "authorizationCode", flows.GetAuthorizationCode()}, + {"client_credentials", "clientCredentials", flows.GetClientCredentials()}, + {"implicit", "implicit", flows.GetImplicit()}, + {"password", "password", flows.GetPassword()}, + {"device", "deviceAuthorization", flows.GetDeviceAuthorization()}, + } + var out []sourceFlow + for _, cand := range candidates { + if cand.src != nil { + out = append(out, cand) + } + } + return out +} + // oauthFlows lowers each present OAuth2 flow in a fixed, deterministic order. // The device flow's deviceAuthorizationUrl rides OAuthFlow.AuthorizationURL // (ir-design §9). func oauthFlows(flows *soa.OAuthFlows) []ir.OAuthFlow { - if flows == nil { - return nil - } var out []ir.OAuthFlow - if f := flows.GetAuthorizationCode(); f != nil { - out = append(out, oauthFlow("authorization_code", f)) - } - if f := flows.GetClientCredentials(); f != nil { - out = append(out, oauthFlow("client_credentials", f)) - } - if f := flows.GetImplicit(); f != nil { - out = append(out, oauthFlow("implicit", f)) - } - if f := flows.GetPassword(); f != nil { - out = append(out, oauthFlow("password", f)) - } - if f := flows.GetDeviceAuthorization(); f != nil { - out = append(out, deviceFlow(f)) + for _, f := range presentFlows(flows) { + if f.kind == "device" { + out = append(out, deviceFlow(f.src)) + continue + } + out = append(out, oauthFlow(f.kind, f.src)) } return out } diff --git a/compilers/openapi/internal/diag/diag.go b/compilers/openapi/internal/diag/diag.go index 59027084..5a9a12b8 100644 --- a/compilers/openapi/internal/diag/diag.go +++ b/compilers/openapi/internal/diag/diag.go @@ -191,6 +191,40 @@ const ( // DegradedConstruct's constructs survive in a weaker shape, and these survive // in none (GitHub #144). UnpreservableConstruct = "openapi/unpreservable-construct" + // UnknownSchemaKeyword reports a JSON Schema keyword no field of the schema + // model names, kept verbatim under Unmodeled. + // + // Info, because the document did nothing wrong: JSON Schema requires an + // implementation to ignore a keyword it does not recognize, and says such a + // keyword may carry meaning for other tooling, so an unrecognized keyword is + // legal input rather than a defect. What is recorded is this compiler's own + // decision — that it read no meaning from the keyword and kept the text — which + // is the same thing ValidationOnlyKeyword records beside it. + UnknownSchemaKeyword = "openapi/unknown-schema-keyword" + // UnknownObjectKey reports a key on an OpenAPI object that the specification + // neither defines nor admits as an extension, kept verbatim under Unmodeled. + // + // Warning rather than info, because unlike its schema neighbour this one is a + // defect: OpenAPI gives its objects a closed key set and requires every + // extension to be prefixed x-, so a key that is neither is a document error — + // in practice a misspelling of the field beside it, which is precisely the + // class of mistake that survives when the compiler swallows the key in silence. + // + // Warning rather than error for the reason ReservedHeaderName is one: the + // document still lowers, everything the key was written beside is unaffected, + // and harness.Check stops at the first error diagnostic, which would hide every + // later finding in the same spec and make any fixture carrying a stray key + // unable to reach the invariant checks. + UnknownObjectKey = "openapi/unknown-object-key" + // UnknownKeyBudget reports an object declaring more keys the model does not + // name than the compiler keeps, so the ones past the bound reached the IR in no + // form at all. + // + // Every collection here is bounded, and this one is over a key set the document + // chooses the size of. The bound is far above what any document writes by + // accident, so tripping it is either a generated file or a hostile one; the + // diagnostic is what keeps the discarded remainder from being a silent loss. + UnknownKeyBudget = "openapi/unknown-key-budget" ) // Newf builds an ir.Diagnostic with a formatted message. It is the single diff --git a/compilers/openapi/internal/diag/diag_test.go b/compilers/openapi/internal/diag/diag_test.go index 26e351ac..8b172081 100644 --- a/compilers/openapi/internal/diag/diag_test.go +++ b/compilers/openapi/internal/diag/diag_test.go @@ -138,6 +138,7 @@ func codes() []string { diag.AliasAmplification, diag.UnattachableRequired, diag.InternalInvariant, diag.DuplicateOperationID, diag.IncompleteSecurityScheme, diag.ReservedHeaderName, diag.UnpreservableConstruct, + diag.UnknownSchemaKeyword, diag.UnknownObjectKey, diag.UnknownKeyBudget, } } diff --git a/compilers/openapi/internal/operation/content.go b/compilers/openapi/internal/operation/content.go index c202731a..677f1109 100644 --- a/compilers/openapi/internal/operation/content.go +++ b/compilers/openapi/internal/operation/content.go @@ -76,11 +76,12 @@ func lowerContent(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex content.File = &ir.FileInfo{IsText: false, ContentTypes: []string{mt}} content.Type = ts.PrimRef(ir.PrimBytes) case isFormContent(mt): - enc, encDiags := partEncodings(c, ts, anchors, media, mediaPtr, content.Type.Target) + enc, encUnmodeled, encDiags := partEncodings(c, ts, anchors, media, mediaPtr, content.Type.Target) diags = append(diags, encDiags...) if len(enc) > 0 { content.Encoding = enc } + content.Unmodeled = annotation.MergeUnmodeled(content.Unmodeled, encUnmodeled) } diags = append(diags, fillSequential(c, ts, anchors, &content, media, mediaPtr, hint)...) ext, extDiags := schema.ExtensionsOf(c, media.GetExtensions(), mediaPtr) @@ -88,7 +89,8 @@ func lowerContent(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex if len(ext) > 0 { content.Unmodeled = annotation.MergeUnmodeled(content.Unmodeled, ext) } - return content, diags + return content, append(diags, + annotation.UnknownKeysIn(&content.Unmodeled, media, c.SrcIndex, mediaPtr)...) } // fillSequential lowers 3.2 sequential-media fields: itemSchema becomes the @@ -113,9 +115,10 @@ func fillSequential(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd if enc == nil { return diags } - pe, encDiags := encodingConfig(c, ts, anchors, enc, mediaPtr+ids.Ptr("itemEncoding")) + pe, unmodeled, encDiags := encodingConfig(c, ts, anchors, enc, mediaPtr+ids.Ptr("itemEncoding"), "itemEncoding") pe.Multi = true content.ItemEncoding = &pe + content.Unmodeled = annotation.MergeUnmodeled(content.Unmodeled, unmodeled) return append(diags, encDiags...) } @@ -154,10 +157,10 @@ func positionalEncoding(c lowering.Ctx, content *ir.Content, media *soa.MediaTyp // body-model property's PropID. A part is included when it carries an explicit // encoding entry or is itself a repeated (array) or file (binary) part. body is // the TypeID the content's own schema position lowered to. -func partEncodings(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, media *soa.MediaType, mediaPtr string, body ir.TypeID) (map[ir.PropID]ir.PartEncoding, []ir.Diagnostic) { +func partEncodings(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, media *soa.MediaType, mediaPtr string, body ir.TypeID) (map[ir.PropID]ir.PartEncoding, ir.Unmodeled, []ir.Diagnostic) { parts := bodyParts(media.GetSchema(), 0) if len(parts) == 0 { - return nil, nil + return nil, nil, nil } // Key by the pointer the body model was interned at, not by mediaPtr, so the // keys align with that model's property IDs (invariant 2/3). Asking the IR is @@ -168,20 +171,25 @@ func partEncodings(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde schemaPtr = bodySchemaPointer(c, media.GetSchema(), mediaPtr+ids.Ptr("schema")) } var diags []ir.Diagnostic + var unmodeled ir.Unmodeled encMap := media.GetEncoding() out := map[ir.PropID]ir.PartEncoding{} for _, part := range parts { - pe, partDiags := buildPartEncoding(c, ts, anchors, part.name, part.schema, encMap, mediaPtr) + pe, partUnmodeled, partDiags := buildPartEncoding(c, ts, anchors, part.name, part.schema, encMap, mediaPtr) diags = append(diags, partDiags...) + // Before the emptiness check, not after it: an entry declaring only + // allowReserved or only x-* lowers to an empty PartEncoding that is left out + // of the map, and what it did declare would go with it. + unmodeled = annotation.MergeUnmodeled(unmodeled, partUnmodeled) if partEncodingEmpty(pe) { continue } out[partPropID(ts, body, part.name, schemaPtr)] = pe } if len(out) == 0 { - return nil, diags + return nil, unmodeled, diags } - return out, diags + return out, unmodeled, diags } // bodyPart is one multipart part: the name keying its encoding entry, and the @@ -304,28 +312,35 @@ func propIDInComposition(ts *compile.Types, m *ir.Model, wire string, depth int) // buildPartEncoding assembles one part's PartEncoding: explicit encoding config // (content types, headers, style, explode) merged with the structural flags Multi // (array part) and Filename (binary/file part). -func buildPartEncoding(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, name string, pjs *oas3.JSONSchema[oas3.Referenceable], encMap *sequencedmap.Map[string, *soa.Encoding], mediaPtr string) (ir.PartEncoding, []ir.Diagnostic) { +func buildPartEncoding(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, name string, pjs *oas3.JSONSchema[oas3.Referenceable], encMap *sequencedmap.Map[string, *soa.Encoding], mediaPtr string) (ir.PartEncoding, ir.Unmodeled, []ir.Diagnostic) { pe := ir.PartEncoding{} + var unmodeled ir.Unmodeled var diags []ir.Diagnostic if encMap != nil { if enc, ok := encMap.Get(name); ok { - pe, diags = encodingConfig(c, ts, anchors, enc, mediaPtr+ids.Ptr("encoding", name)) + pe, unmodeled, diags = encodingConfig(c, ts, anchors, enc, + mediaPtr+ids.Ptr("encoding", name), "encoding/"+name) } } if part := schemaOf(pjs); part != nil { pe.Multi = schemaIsArray(part) pe.Filename = schemaIsFilePart(part) } - return pe, diags + return pe, unmodeled, diags } // encodingConfig lowers one Encoding object's declared wire config: content // types, per-part headers, and form-style serialization. The structural flags // (Multi, Filename) come from the part's own schema, not from here. -func encodingConfig(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, enc *soa.Encoding, encPtr string) (ir.PartEncoding, []ir.Diagnostic) { +// +// It returns what the object declared that PartEncoding has no field for as a +// second value rather than writing it, because PartEncoding carries no +// Unmodeled map of its own; scope is where that belongs on the owning Content +// (see encodingUnmodeled). +func encodingConfig(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, enc *soa.Encoding, encPtr, scope string) (ir.PartEncoding, ir.Unmodeled, []ir.Diagnostic) { pe := ir.PartEncoding{} if enc == nil { - return pe, nil + return pe, nil, nil } pe.ContentTypes = splitContentTypes(enc.GetContentTypeValue()) headers, diags := lowerHeaders(c, ts, anchors, enc.GetHeaders(), encPtr) @@ -334,7 +349,38 @@ func encodingConfig(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd pe.Style = string(*enc.Style) } pe.Explode = enc.Explode - return pe, diags + unmodeled, encDiags := encodingUnmodeled(c, enc, encPtr, scope) + return pe, unmodeled, append(diags, encDiags...) +} + +// encodingUnmodeled keeps what an Encoding Object declares that nothing in the +// IR holds: `allowReserved`, which ir.PartEncoding has no field for even though +// its neighbours style and explode do, the object's own x-*, and the keys the +// specification defines for no encoding at all. Neither allowReserved nor the +// extensions had reached an IR field, an Unmodeled entry or a diagnostic, so two +// documents differing only in them compiled to one IR (GitHub #291). +// +// Both ride on the owning ir.Content, since PartEncoding carries no Unmodeled +// map, keyed under scope — "encoding/" or "itemEncoding". One content can +// hold an entry per multipart part plus a sequential item's, and they reach the +// same map, so the part is what tells them apart. +// +// allowReserved carries ReasonNoIRHome and announces itself only when something +// was written, the shape preserveHeaderSerialization already uses for the pair +// beside it. +func encodingUnmodeled(c lowering.Ctx, enc *soa.Encoding, encPtr, scope string) (ir.Unmodeled, []ir.Diagnostic) { + var out ir.Unmodeled + at := encPtr + ids.Ptr("allowReserved") + kept, diags := schema.PreserveNode(c, &out, "openapi:"+scope+"/allowReserved", + annotation.RawChildNode(enc.GetRootNode(), "allowReserved"), ir.ReasonNoIRHome, at) + if kept { + diags = append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, at, + "encoding allowReserved has no ir.PartEncoding home; kept verbatim under Unmodeled")) + } + ext, extDiags := schema.ExtensionsIn(c, enc.GetExtensions(), encPtr, scope) + out = annotation.MergeUnmodeled(out, ext) + diags = append(diags, extDiags...) + return out, append(diags, annotation.UnknownKeysUnder(&out, enc, c.SrcIndex, encPtr, scope)...) } // lowerHeaders lowers a header map into Properties in source order. Each @@ -512,7 +558,7 @@ func applyHeaderAnnotations(c lowering.Ctx, p *ir.Property, h *soa.Header, hdecl hExt, extDiags := schema.ExtensionsOf(c, h.GetExtensions(), hdecl) diags = append(diags, extDiags...) p.Unmodeled = annotation.MergeUnmodeled(p.Unmodeled, hExt) - return diags + return append(diags, annotation.UnknownKeysIn(&p.Unmodeled, h, c.SrcIndex, hdecl)...) } // exampleList lowers a single example node and a plural example map into value @@ -547,16 +593,34 @@ func exampleList(c lowering.Ctx, single *yaml.Node, plural *sequencedmap.Map[str // is de-referenced: an enclosing $ref'd response or parameter is already // flattened into pointer. func appendPluralExample(c lowering.Ctx, out []ir.Example, re *soa.ReferencedExample, pointer, name string) ([]ir.Example, []ir.Diagnostic) { - ex := resolve.Object[soa.Example](re) + // The declaration pointer, not the entry's, is where an Example Object's own + // keywords are written: a $ref entry holds none of them. ir.Example carries an + // Unmodeled map, so neither the object's x-* nor its undeclared keys need a + // scope. + ex, decl := resolve.ObjectAt[soa.Example](c.RefScope(), re, pointer+ids.Ptr("examples", name)) if ex == nil { return out, nil } + ext, diags := schema.ExtensionsOf(c, ex.GetExtensions(), decl) + diags = append(diags, annotation.UnknownKeysIn(&ext, ex, c.SrcIndex, decl)...) proto := ir.Example{ Name: name, Summary: ex.GetSummary(), Description: ex.GetDescription(), ExternalURL: ex.GetExternalValue(), + Unmodeled: ext, } + out, exDiags := appendExampleValue(c, out, proto, ex, re, pointer, name) + return out, append(diags, exDiags...) +} + +// appendExampleValue appends the entry's value under the annotations proto +// already carries, stamping the failure pointer where the value is written: at +// the reference site for a $ref entry, which holds no `value` node of its own, +// and at its own `value` for an inline one. +func appendExampleValue(c lowering.Ctx, out []ir.Example, proto ir.Example, ex *soa.Example, + re *soa.ReferencedExample, pointer, name string, +) ([]ir.Example, []ir.Diagnostic) { node := ex.GetValue() if node == nil { return appendValuelessExample(c, out, proto, pointer, name) @@ -604,6 +668,16 @@ func lowerRequestBody(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorI diags = append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, bodyPtr, "request body is not required; optionality kept under Unmodeled")) } + // soa.RequestBody exposes no GetExtensions at this library version, so the + // field is read directly — as XMLHints already reads its own. Both reads sit + // after the payload guard because ir.Payload is the body's only carrier: a + // request body declaring no content lowers to nothing to hang them on, and + // OpenAPI makes content REQUIRED there, so such a body is a defect in the + // document rather than a shape this compiler has to place. + bodyExt, bodyExtDiags := schema.ExtensionsOf(c, rb.Extensions, bodyPtr) + payload.Unmodeled = annotation.MergeUnmodeled(payload.Unmodeled, bodyExt) + diags = append(diags, bodyExtDiags...) + diags = append(diags, annotation.UnknownKeysIn(&payload.Unmodeled, rb, c.SrcIndex, bodyPtr)...) op.Request = payload hb.RequestContentTypes = contentTypeKeys(rb.GetContent()) return diags diff --git a/compilers/openapi/internal/operation/content_internal_test.go b/compilers/openapi/internal/operation/content_internal_test.go index 966050a7..ceeebc5f 100644 --- a/compilers/openapi/internal/operation/content_internal_test.go +++ b/compilers/openapi/internal/operation/content_internal_test.go @@ -102,8 +102,9 @@ func TestFillSequential_EmptyItemEncoding(t *testing.T) { func TestEncodingConfig_NilEncoding(t *testing.T) { t.Parallel() l := newRawLowerer(&soa.OpenAPI{}) - pe, diags := encodingConfig(l.ctx, l.types, &l.anchors, nil, "/mp") + pe, unmodeled, diags := encodingConfig(l.ctx, l.types, &l.anchors, nil, "/mp", "itemEncoding") assert.Equal(t, ir.PartEncoding{}, pe) + assert.Nil(t, unmodeled) assert.Empty(t, diags) } diff --git a/compilers/openapi/internal/operation/content_test.go b/compilers/openapi/internal/operation/content_test.go index 4273d5d7..1c5eb9d9 100644 --- a/compilers/openapi/internal/operation/content_test.go +++ b/compilers/openapi/internal/operation/content_test.go @@ -386,7 +386,7 @@ paths: "200": description: ok content: - application/jsonl: + multipart/mixed: itemSchema: {type: object, properties: {a: {type: string}}} itemEncoding: {contentType: application/json} x-note: streamy @@ -474,6 +474,105 @@ func TestContent_SequentialAndEmptyBody(t *testing.T) { assert.Nil(t, empty.Request) } +// encodingSpec declares one form part per position an Encoding Object reaches: +// a multipart part, and a 3.2 sequential itemEncoding. Each writes the two +// things ir.PartEncoding has no field for, so both positions are exercised by +// the one fixture. +const encodingSpec = ` /form: + post: + operationId: postForm + requestBody: + required: true + content: + application/x-www-form-urlencoded: + schema: {type: object, properties: {q: {type: string}}} + encoding: + q: {style: form, allowReserved: true, x-vendor: vvv} + responses: {"200": {description: ok}} +` + +// TestEncoding_AllowReservedAndExtensionsKeptOnTheContent pins both halves of +// GitHub #291. Neither allowReserved nor an encoding's x-* reached an IR field, +// an Unmodeled entry or a diagnostic, so two documents differing only in them +// compiled to the same IR. PartEncoding carries no Unmodeled map, so both land +// on the owning Content keyed by the part they govern. +func TestEncoding_AllowReservedAndExtensionsKeptOnTheContent(t *testing.T) { + t.Parallel() + _, svc, diags := lowerServiceSpec(t, pathsSpecVer("3.1.0", encodingSpec)) + requireNoErrorDiags(t, diags) + op := firstOp(t, svc) + require.NotNil(t, op.Request) + require.Len(t, op.Request.Contents, 1) + kept := op.Request.Contents[0].Unmodeled + + at := "/paths/~1form/post/requestBody/content/application~1x-www-form-urlencoded/encoding/q" + entry, ok := kept["openapi:encoding/q/allowReserved"] + require.True(t, ok, "allowReserved is kept; got %v", kept) + assert.Equal(t, ir.ReasonNoIRHome, entry.Reason) + assert.JSONEq(t, "true", string(entry.Value)) + assert.Equal(t, at+"/allowReserved", entry.Provenance.Pointer) + assertInfoDiagAt(t, diags, at+"/allowReserved") + + ext, ok := kept["openapi:encoding/q/x-vendor"] + require.True(t, ok, "the encoding's own x-* is kept; got %v", kept) + assert.Equal(t, ir.ReasonVendorExtension, ext.Reason) + assert.Equal(t, at+"/x-vendor", ext.Provenance.Pointer) +} + +// TestEncoding_AbsentAllowReservedRecordsNothing is the control: preservation +// keys off what the encoding declares, so an entry that omits allowReserved +// records neither an entry nor a diagnostic — rather than recording the OpenAPI +// default the accessor would hand back and calling it a source fact. +func TestEncoding_AbsentAllowReservedRecordsNothing(t *testing.T) { + t.Parallel() + _, svc, diags := lowerServiceSpec(t, pathsSpecVer("3.1.0", ` /form: + post: + operationId: postForm + requestBody: + required: true + content: + application/x-www-form-urlencoded: + schema: {type: object, properties: {q: {type: string}}} + encoding: + q: {style: form} + responses: {"200": {description: ok}} +`)) + requireNoErrorDiags(t, diags) + op := firstOp(t, svc) + require.NotNil(t, op.Request) + require.Len(t, op.Request.Contents, 1) + assert.Empty(t, op.Request.Contents[0].Unmodeled, "nothing was declared, so nothing is kept") + assert.Empty(t, diags) +} + +// TestEncoding_ItemEncodingKeepsItsOwnUnderItsOwnKey is the other position the +// same reader serves. A content can hold both a per-part encoding map and a +// sequential itemEncoding, and both reach one Unmodeled map, so the key has to +// tell them apart. +func TestEncoding_ItemEncodingKeepsItsOwnUnderItsOwnKey(t *testing.T) { + t.Parallel() + _, svc, diags := lowerServiceSpec(t, pathsSpecVer("3.2.0", ` /stream: + get: + operationId: streamEvents + responses: + "200": + description: ok + content: + multipart/mixed: + itemSchema: {type: object, properties: {n: {type: string}}} + itemEncoding: {contentType: application/json, allowReserved: true, x-vendor: vvv} +`)) + requireNoErrorDiags(t, diags) + op := firstOp(t, svc) + require.NotEmpty(t, op.Responses) + require.NotNil(t, op.Responses[0].Payload) + require.Len(t, op.Responses[0].Payload.Contents, 1) + kept := op.Responses[0].Payload.Contents[0].Unmodeled + + assert.Contains(t, kept, "openapi:itemEncoding/allowReserved") + assert.Contains(t, kept, "openapi:itemEncoding/x-vendor") +} + // multipartEncoding returns the part-encoding map of an operation's request. func multipartEncoding(t *testing.T, op ir.Operation) map[ir.PropID]ir.PartEncoding { t.Helper() diff --git a/compilers/openapi/internal/operation/operations.go b/compilers/openapi/internal/operation/operations.go index 351fb057..583b964b 100644 --- a/compilers/openapi/internal/operation/operations.go +++ b/compilers/openapi/internal/operation/operations.go @@ -59,6 +59,13 @@ func LowerService(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex } svcAuth, diags := auth.LowerSecurityRequirements(c, c.Doc.GetSecurity(), "") svc.Auth = svcAuth + // The Paths Object admits x-* of its own, distinct from any path item's, and + // lowers to no node — its entries become operations. The service is the + // nearest node holding an Unmodeled map, so they are kept there under the + // keyword they were written at. + pathsExt, pathsDiags := schema.ExtensionsIn(c, c.Doc.GetPaths().GetExtensions(), ids.Ptr("paths"), "paths") + svc.Unmodeled = annotation.MergeUnmodeled(svc.Unmodeled, pathsExt) + diags = append(diags, pathsDiags...) groups := newServiceGroups() diags = append(diags, lowerPaths(c, ts, anchors, operationIDs, groups)...) diags = append(diags, lowerWebhooks(c, ts, anchors, operationIDs, groups)...) @@ -135,7 +142,7 @@ func lowerPathItem(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde } op, extra, opDiags := lowerOperation(c, ts, anchors, operationIDs, src, opCtx) diags = append(diags, opDiags...) - diags = append(diags, applyPathServers(c, &op, pi, declPtr)...) + diags = append(diags, applyPathItem(c, &op, pi, declPtr)...) grp := groups.group(key, func() ir.OperationGroup { return ir.OperationGroup{Name: name, Docs: docs} }) grp.Operations = append(grp.Operations, op) grp.Operations = append(grp.Operations, extra...) @@ -173,7 +180,7 @@ func lowerWebhooks(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde } op, extra, opDiags := lowerOperation(c, ts, anchors, operationIDs, src, opCtx) diags = append(diags, opDiags...) - diags = append(diags, applyPathServers(c, &op, pi, declPtr)...) + diags = append(diags, applyPathItem(c, &op, pi, declPtr)...) grp := groups.group("webhook", func() ir.OperationGroup { // A hint, not a source name: no document declares this group. The // compiler synthesizes it to hold webhook operations, exactly as it @@ -282,21 +289,47 @@ func lowerOperation(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd diags = append(diags, lowerRequestBody(c, ts, anchors, &op, &hb, src, decl)...) var extra []ir.Operation if opCtx.withCallbacks { + var cbExt ir.Unmodeled var cbDiags []ir.Diagnostic - hb.Callbacks, extra, cbDiags = lowerCallbacks(c, ts, anchors, operationIDs, src, opCtx.ptrs, opCtx.inferred) + hb.Callbacks, extra, cbExt, cbDiags = lowerCallbacks(c, ts, anchors, operationIDs, src, opCtx.ptrs, opCtx.inferred) + hb.Unmodeled = annotation.MergeUnmodeled(hb.Unmodeled, cbExt) diags = append(diags, cbDiags...) } op.Bindings = ir.OpBindings{HTTP: []ir.HTTPBinding{hb}} - ext, extDiags := schema.ExtensionsOf(c, src.GetExtensions(), decl) - diags = append(diags, extDiags...) - if len(ext) > 0 { - op.Unmodeled = ext - } - // After the extensions assignment, which would otherwise overwrite the map. + diags = append(diags, applyOperationAnnotations(c, &op, src, decl)...) diags = append(diags, applyOperationServers(c, &op, src, decl)...) return op, extra, append(diags, checkOperationIDUnique(c, operationIDs, op, mount)...) } +// applyOperationAnnotations keeps the operation's own x-* and undeclared keys, +// and those of the two objects beneath it that lower to no node of their own: +// its externalDocs, since ir.Link holds no Unmodeled map, and its Responses +// Object, whose extensions are the map's own rather than any one response's. +// +// The Responses Object contributes extensions but no census. Its key set is the +// status codes the document chooses, which the library models as a map, so an +// undeclared key there is read as one more response rather than reported as +// unknown — there is nothing for a census to say about it. +// +// It merges rather than assigns. The operation's map already carries whatever +// its parameters or callbacks wrote by the time this runs, and an assignment +// here would drop them — which is why the servers preservation used to have to +// run after it. +func applyOperationAnnotations(c lowering.Ctx, op *ir.Operation, src *soa.Operation, decl string) []ir.Diagnostic { + docsPtr := decl + ids.Ptr("externalDocs") + ext, diags := annotation.ExtensionsAt(c.SrcIndex, + annotation.ExtensionSite{Owner: decl, Ext: src.GetExtensions()}, + annotation.ExtensionSite{Scope: "externalDocs", Owner: docsPtr, + Ext: src.GetExternalDocs().GetExtensions()}, + annotation.ExtensionSite{Scope: "responses", Owner: decl + ids.Ptr("responses"), + Ext: src.GetResponses().GetExtensions()}, + ) + op.Unmodeled = annotation.MergeUnmodeled(op.Unmodeled, ext) + diags = append(diags, annotation.UnknownKeysIn(&op.Unmodeled, src, c.SrcIndex, decl)...) + return append(diags, annotation.UnknownKeysUnder(&op.Unmodeled, + src.GetExternalDocs(), c.SrcIndex, docsPtr, "externalDocs")...) +} + // applyOperationServers preserves an operation's own `servers` verbatim under // Unmodeled, for the same reason applyPathServers preserves the path item's: // §10 scopes servers by index list at service and channel, and ir.Operation has @@ -376,6 +409,42 @@ func fillOperationDocs(d *ir.Docs, src *soa.Operation) { } } +// applyPathItem keeps what a path item declares that its operations have no +// home for: its servers and its own x-* extensions. Both belong to the path +// item rather than to any one operation on it, so both are written onto every +// operation the item declares. +// +// The two are applied together, through this one entry point, because every +// route that lowers a path item — a path, a webhook, a callback expression — +// must reach both, and a second call beside the first is a second chance to +// forget one on a route added later. That is exactly how the servers half came +// to be missing on two of its three routes (GitHub #39). +// +// A path item takes no census, unlike every other object the compiler reads +// extensions from, and deliberately so. The library folds a key it does not +// recognize into the item's embedded operations map rather than recording it as +// undeclared, so GetUnknownProperties reports nothing and there is no census to +// read (speakeasy-api/openapi v1.24.0). Two consequences decide it: +// +// - The key is not lost in silence. Folding it produces a +// validation-type-mismatch at error severity naming the key at its own +// pointer, which is the losslessness property the census exists for; what is +// lost is the key's value, not the fact that it was written. +// - Recovering the value means reading the raw node against a path item's key +// vocabulary, and the only vocabulary this compiler owns is httpMethods, +// which is narrower than the library's — it has no `query`, the method +// OpenAPI 3.2 adds. A census over what httpMethods does not name would +// therefore report a valid 3.2 `query` operation as an undeclared key. +// +// That vocabulary is what GitHub #293 is about, so widening it here would settle +// that issue as a side effect of this one. Tracked separately in GitHub #377. +func applyPathItem(c lowering.Ctx, op *ir.Operation, pi *soa.PathItem, declPtr string) []ir.Diagnostic { + diags := applyPathServers(c, op, pi, declPtr) + ext, extDiags := schema.ExtensionsIn(c, pi.GetExtensions(), declPtr, "pathItem") + op.Unmodeled = annotation.MergeUnmodeled(op.Unmodeled, ext) + return append(diags, extDiags...) +} + // applyPathServers preserves path-item-level servers verbatim under Unmodeled // on the operation. §10 models servers as Document.Servers with per-scope index // lists (Service.Servers, Channel.Servers); ir.Operation just has no such list @@ -463,9 +532,37 @@ func lowerResponse(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde Headers: headers, } resp.Docs.Description = r.GetDescription() - _, linkDiags := schema.PreserveNode(c, &resp.Unmodeled, "openapi:links", + return resp, append(diags, preserveResponseExtras(c, &resp.Unmodeled, r, rptr)...) +} + +// preserveResponseExtras keeps what a Response Object declares that has no home +// on the node it lowered to: its links map, its own x-* extensions, and the keys +// the specification does not define at all. +// +// One helper for both branches on purpose. ir.Response and ir.ErrorCase are two +// lowerings of the same source object, and each construct kept on only one of +// them makes a declaration survive or vanish on nothing but its status code: +// links were kept on a 2xx and dropped on a 4xx, and extensions were read at +// neither (GitHub #275). Adding a construct here reaches both by construction. +// +// The links entry carries ReasonNoIRHome and no diagnostic, as it always has: +// nothing is degraded, the map is in the document, and the gap is one the IR can +// close by growing a links field. +// +// A Link Object inside that map gets no entry and no census of its own, which is +// the decision already recorded for its extensions. This compiler lowers no Link +// Object anywhere: a response's links survive only as the verbatim node above, +// and a components/links entry nothing references is dropped whole, so a keyed +// entry at one of the two positions would be the only trace of a construct the +// IR does not model — while duplicating, for the response position alone, a +// value the node above already carries. +func preserveResponseExtras(c lowering.Ctx, p *ir.Unmodeled, r *soa.Response, rptr string) []ir.Diagnostic { + _, diags := schema.PreserveNode(c, p, "openapi:links", annotation.RawChildNode(r.GetRootNode(), "links"), ir.ReasonNoIRHome, rptr+ids.Ptr("links")) - return resp, append(diags, linkDiags...) + ext, extDiags := schema.ExtensionsOf(c, r.GetExtensions(), rptr) + *p = annotation.MergeUnmodeled(*p, ext) + diags = append(diags, extDiags...) + return append(diags, annotation.UnknownKeysIn(p, r, c.SrcIndex, rptr)...) } // responseName builds a success response's neutral naming. OpenAPI names no @@ -497,7 +594,8 @@ func lowerErrorCase(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd } ec.Docs.Description = r.GetDescription() diags := fillErrorType(c, ts, anchors, &ec, r, rptr) - return ec, append(diags, preserveErrorHeaders(c, &ec, r, rptr)...) + diags = append(diags, preserveErrorHeaders(c, &ec, r, rptr)...) + return ec, append(diags, preserveResponseExtras(c, &ec.Unmodeled, r, rptr)...) } // preserveErrorHeaders keeps an error response's headers from being dropped: @@ -578,19 +676,27 @@ func errorContentMessage(n int) string { // parent.mount roots callback operation identity, so two parents sharing one // $ref'd callback keep distinct callback operations; parent.decl is the base a // $ref'd callback or path item resolves against (issue #107). -func lowerCallbacks(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, operationIDs map[string]string, src *soa.Operation, parent opPointers, inferred string) ([]ir.Callback, []ir.Operation, []ir.Diagnostic) { +func lowerCallbacks(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, operationIDs map[string]string, src *soa.Operation, parent opPointers, inferred string) ([]ir.Callback, []ir.Operation, ir.Unmodeled, []ir.Diagnostic) { cbMap := src.GetCallbacks() if cbMap == nil || cbMap.Len() == 0 { - return nil, nil, nil + return nil, nil, nil, nil } var callbacks []ir.Callback var ops []ir.Operation + var ext ir.Unmodeled var diags []ir.Diagnostic for cbName, rcb := range cbMap.All() { cb, cbDecl := resolve.ObjectAt[soa.Callback](c.RefScope(), rcb, parent.decl+ids.Ptr("callbacks", cbName)) if cb == nil { continue } + // A Callback Object's own x-* describe the callback rather than any + // expression's path item, and ir.Callback holds no Unmodeled map. The HTTP + // binding does, and is where the callbacks themselves live, so they are kept + // there under the name the callback is mapped by. + cbExt, cbExtDiags := schema.ExtensionsIn(c, cb.GetExtensions(), cbDecl, "callbacks/"+cbName) + ext = annotation.MergeUnmodeled(ext, cbExt) + diags = append(diags, cbExtDiags...) for expr, rp := range cb.All() { exprStr := string(expr) pi, piDecl := resolve.ObjectAt[soa.PathItem](c.RefScope(), rp, cbDecl+ids.Ptr(exprStr)) @@ -604,7 +710,7 @@ func lowerCallbacks(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd ops = append(ops, cbOps...) } } - return callbacks, ops, diags + return callbacks, ops, ext, diags } // lowerCallbackOps lowers a callback expression's path-item operations. Callback @@ -632,7 +738,7 @@ func lowerCallbackOps(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorI } op, _, opDiags := lowerOperation(c, ts, anchors, operationIDs, src, opCtx) diags = append(diags, opDiags...) - diags = append(diags, applyPathServers(c, &op, pi, cb.decl)...) + diags = append(diags, applyPathItem(c, &op, pi, cb.decl)...) opIDs = append(opIDs, op.ID) ops = append(ops, op) } diff --git a/compilers/openapi/internal/operation/params.go b/compilers/openapi/internal/operation/params.go index e0875ee3..c7bcc3a6 100644 --- a/compilers/openapi/internal/operation/params.go +++ b/compilers/openapi/internal/operation/params.go @@ -201,7 +201,10 @@ func fillParamSchemaAnnotations(c lowering.Ctx, ts *compile.Types, param *ir.Par diags = append(diags, preserveParamXML(c, param, s, pointer)...) } param.Unmodeled = annotation.MergeUnmodeled(param.Unmodeled, a.Unmodeled) - return diags + // Last, per schema.PreserveUnknownKeywords: it keeps only the keywords no + // reader above it kept, and everything annotation.Read recorded is already on + // the parameter by this line. + return append(diags, schema.PreserveUnknownKeywords(c, ¶m.Unmodeled, s, pointer)...) } // preserveParamXML keeps a parameter schema's xml hints instead of dropping @@ -274,6 +277,7 @@ func fillParamDetail(c lowering.Ctx, param *ir.Parameter, p *soa.Parameter, pptr pExt, extDiags := schema.ExtensionsOf(c, p.GetExtensions(), pptr) diags = append(diags, extDiags...) param.Unmodeled = annotation.MergeUnmodeled(param.Unmodeled, pExt) + diags = append(diags, annotation.UnknownKeysIn(¶m.Unmodeled, p, c.SrcIndex, pptr)...) return append(diags, preserveAllowEmptyValue(c, param, p, pptr)...) } diff --git a/compilers/openapi/internal/schema/accumulate.go b/compilers/openapi/internal/schema/accumulate.go index bf2a5fac..ea056307 100644 --- a/compilers/openapi/internal/schema/accumulate.go +++ b/compilers/openapi/internal/schema/accumulate.go @@ -73,6 +73,20 @@ func PreserveSchemaKeyword(c lowering.Ctx, p *ir.Unmodeled, s *oas3.Schema, keyw return PreserveNode(c, p, "openapi:"+keyword, annotation.RawPropertyNode(s, keyword), reason, pointer) } +// PreserveUnknownKeywords records every keyword s writes that no field of the +// schema model names and no reader above it already kept (GitHub #297). +// +// It is the last thing an attachment does, which is the contract +// annotation.UnknownKeywordsIn states and the reason it is called here rather +// than from inside annotation.Read with the other readers: $dynamicRef has no +// field in the schema model either and is decided by recordUnexpandedDynamicRef, +// which runs after Read and deliberately keeps nothing once the reference has +// expanded. A census running before it could not tell that from an unread +// keyword. +func PreserveUnknownKeywords(c lowering.Ctx, p *ir.Unmodeled, s *oas3.Schema, pointer string) []ir.Diagnostic { + return annotation.UnknownKeywordsIn(p, s, pointer, c.SrcIndex) +} + // preserveKeyword records a validation-only keyword's raw payload under key in // p and returns the one info diagnostic naming it at declPtr, the schema that // wrote it. An absent or unconvertible payload records nothing and returns @@ -123,3 +137,10 @@ func lowerArray(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth i func ExtensionsOf(c lowering.Ctx, ext *extensions.Extensions, owner string) (ir.Unmodeled, []ir.Diagnostic) { return annotation.ExtensionsFrom(ext, c.SrcIndex, owner) } + +// ExtensionsIn is ExtensionsOf for an object with no Unmodeled map of its own, +// whose entries ride on an enclosing node's under scope — see +// annotation.ExtensionsUnder for what scope names and why it is needed. +func ExtensionsIn(c lowering.Ctx, ext *extensions.Extensions, owner, scope string) (ir.Unmodeled, []ir.Diagnostic) { + return annotation.ExtensionsUnder(ext, c.SrcIndex, owner, scope) +} diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index d06d1eb9..b5cc509f 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -894,7 +894,8 @@ func fillPropertyAnnotations(c lowering.Ctx, ts *compile.Types, anchors *AnchorI // nil node: this arm runs only when the schema lowered to no node of its own, // so nothing here can be carrying an Encoding. diags = append(diags, recordUnplacedContent(c, &p.Unmodeled, ref, nil, pointer)...) - return append(diags, recordUnexpandedDynamicRef(c, anchors, &p.Unmodeled, ref, pointer)...) + diags = append(diags, recordUnexpandedDynamicRef(c, anchors, &p.Unmodeled, ref, pointer)...) + return append(diags, PreserveUnknownKeywords(c, &p.Unmodeled, ref, pointer)...) } // LoweredToOwnNode reports whether the declaration at pointer lowered to a type @@ -978,7 +979,8 @@ func attachDeclaredAnnotations(c lowering.Ctx, ts *compile.Types, anchors *Ancho common.Examples = a.Examples } diags = append(diags, recordUnplacedContent(c, &common.Unmodeled, s, td, pointer)...) - return append(diags, recordUnexpandedDynamicRef(c, anchors, &common.Unmodeled, s, pointer)...) + diags = append(diags, recordUnexpandedDynamicRef(c, anchors, &common.Unmodeled, s, pointer)...) + return append(diags, PreserveUnknownKeywords(c, &common.Unmodeled, s, pointer)...) } // fillAdditional lowers additionalProperties, patternProperties, and diff --git a/compilers/openapi/meta.go b/compilers/openapi/meta.go index 23191d28..ae6aafdb 100644 --- a/compilers/openapi/meta.go +++ b/compilers/openapi/meta.go @@ -1,10 +1,13 @@ package openapi import ( + "strconv" + soa "github.com/speakeasy-api/openapi/openapi" "github.com/dexpace/morphic/compilers/compile" "github.com/dexpace/morphic/compilers/openapi/internal/annotation" + "github.com/dexpace/morphic/compilers/openapi/internal/ids" "github.com/dexpace/morphic/compilers/openapi/internal/lowering" "github.com/dexpace/morphic/ir" ) @@ -35,14 +38,143 @@ type docMeta struct { } // lowerMeta lowers the document-level metadata that is not part of the type or -// service graph: info, servers, and top-level extensions (ir-design §10, §12). +// service graph: info, servers, and the extensions of every object around them +// that lowers to no node of its own (ir-design §10, §12). func lowerMeta(c lowering.Ctx) (docMeta, []ir.Diagnostic) { m := lowerInfo(c) - m.Servers = lowerServers(c) - - ext, diags := annotation.ExtensionsFrom(c.Doc.GetExtensions(), c.SrcIndex, "") + ext, diags := documentExtensions(c) m.Unmodeled = ext - return m, diags + + servers, serverDiags := lowerServers(c) + m.Servers = servers + diags = append(diags, serverDiags...) + // After the extensions assignment, which would otherwise overwrite the map. + return m, append(diags, documentUnknownKeys(c, &m.Unmodeled)...) +} + +// documentExtensions collects the x-* of every object that lowers to no IR node +// of its own: the document root, the info block and the contact and license +// inside it, the root externalDocs, the components object, and each declared +// tag with its own externalDocs. ir.Document is the nearest node with an +// Unmodeled map for all of them, so each object's entries are keyed under the +// source path it was written at — see annotation.ExtensionsUnder for why one +// unscoped key for all of them would not do. +func documentExtensions(c lowering.Ctx) (ir.Unmodeled, []ir.Diagnostic) { + return annotation.ExtensionsAt(c.SrcIndex, append(rootExtensions(c), tagExtensions(c)...)...) +} + +// rootExtensions returns the extension sites a document has exactly one of. The +// root's own take no scope, since ir.Document stands for the OpenAPI Object +// itself; the rest are keyed by the path from it down to the object that wrote +// them. +func rootExtensions(c lowering.Ctx) []annotation.ExtensionSite { + info := c.Doc.GetInfo() + infoPtr := ids.Ptr("info") + return []annotation.ExtensionSite{ + {Scope: "", Owner: "", Ext: c.Doc.GetExtensions()}, + {Scope: "info", Owner: infoPtr, Ext: info.GetExtensions()}, + {Scope: "info/contact", Owner: infoPtr + ids.Ptr("contact"), Ext: info.GetContact().GetExtensions()}, + {Scope: "info/license", Owner: infoPtr + ids.Ptr("license"), Ext: info.GetLicense().GetExtensions()}, + {Scope: "externalDocs", Owner: ids.Ptr("externalDocs"), Ext: c.Doc.GetExternalDocs().GetExtensions()}, + {Scope: "components", Owner: ids.Ptr("components"), Ext: c.Doc.GetComponents().GetExtensions()}, + } +} + +// tagExtensions returns the extension sites each declared tag contributes: its +// own, and its externalDocs object's. ir.TagDef holds no Unmodeled map and +// neither does ir.Link, so both ride on the document. +// +// Keyed by the tag's index rather than its name, which is the pointer the tag +// is written at. A name would read better and is what TagDefs are found by, but +// OpenAPI's requirement that tag names be unique is the document's to keep, not +// this compiler's to rely on: two tags spelled alike would silently leave one +// entry. +func tagExtensions(c lowering.Ctx) []annotation.ExtensionSite { + tags := c.Doc.GetTags() + out := make([]annotation.ExtensionSite, 0, 2*len(tags)) + for i, t := range tags { + if t == nil { + continue + } + scope := "tags/" + strconv.Itoa(i) + ptr := ids.Ptr("tags", strconv.Itoa(i)) + out = append(out, + annotation.ExtensionSite{Scope: scope, Owner: ptr, Ext: t.GetExtensions()}, + annotation.ExtensionSite{Scope: scope + "/externalDocs", Owner: ptr + ids.Ptr("externalDocs"), + Ext: t.GetExternalDocs().GetExtensions()}) + } + return out +} + +// documentUnknownKeys collects the keys the OpenAPI model names no field for +// from every object around the document metadata that lowers to no node of its +// own: the document root, the info block and the contact and license inside it, +// the root externalDocs, the components object, and each declared tag with its +// own externalDocs. +// +// ir.Document is the nearest node with an Unmodeled map for all of them, so each +// object's keys are scoped by the source path they were written at. One unscoped +// "openapi:status" would be a single key for six objects, and the entry that +// survived would be whichever site ran last. +func documentUnknownKeys(c lowering.Ctx, p *ir.Unmodeled) []ir.Diagnostic { + sites := append(rootUnknownSites(c), tagUnknownSites(c)...) + diags := make([]ir.Diagnostic, 0, len(sites)) + for _, site := range sites { + diags = append(diags, + annotation.UnknownKeysUnder(p, site.model, c.SrcIndex, site.owner, site.scope)...) + } + return diags +} + +// unknownSite is one object's census: what it keys under on the carrier holding +// it, the object's own source pointer, and the parsed object itself. +type unknownSite struct { + scope string + owner string + model any +} + +// rootUnknownSites returns the census sites a document has exactly one of. The +// root's keys take no scope, since ir.Document stands for the OpenAPI Object +// itself; the rest are keyed by the path from it down to the object that wrote +// them. +// +// The components object is one of them rather than a map with nothing to say: +// its own key set is the fixed list of component kinds, which the library models +// as named fields and takes a census over, and only the map *under* each of +// those keys is the document's to name. +func rootUnknownSites(c lowering.Ctx) []unknownSite { + info := c.Doc.GetInfo() + infoPtr := ids.Ptr("info") + return []unknownSite{ + {"", "", c.Doc}, + {"info", infoPtr, info}, + {"info/contact", infoPtr + ids.Ptr("contact"), info.GetContact()}, + {"info/license", infoPtr + ids.Ptr("license"), info.GetLicense()}, + {"externalDocs", ids.Ptr("externalDocs"), c.Doc.GetExternalDocs()}, + {"components", ids.Ptr("components"), c.Doc.GetComponents()}, + } +} + +// tagUnknownSites returns the census sites each declared tag contributes: its +// own, and its externalDocs object's. Neither ir.TagDef nor ir.Link holds an +// Unmodeled map, so both ride on the document. +// +// Scoped by index rather than name, which is the pointer a tag is written at. A +// name would read better, but OpenAPI's requirement that tag names be unique is +// the document's to keep and not this compiler's to rely on: two tags spelled +// alike would silently leave one entry. +func tagUnknownSites(c lowering.Ctx) []unknownSite { + tags := c.Doc.GetTags() + out := make([]unknownSite, 0, 2*len(tags)) + for i, t := range tags { + index := strconv.Itoa(i) + ptr := ids.Ptr("tags", index) + out = append(out, + unknownSite{"tags/" + index, ptr, t}, + unknownSite{"tags/" + index + "/externalDocs", ptr + ids.Ptr("externalDocs"), t.GetExternalDocs()}) + } + return out } // lowerInfo maps info onto the document identity, docs, contact, and license. @@ -79,31 +211,40 @@ func infoDocs(c lowering.Ctx, info *soa.Info) ir.Docs { // template, description, and templated variables (ir-design §10). It returns nil // rather than an empty slice when every entry was skipped, so a document // declaring no usable server leaves the field unset. -func lowerServers(c lowering.Ctx) []ir.Server { +func lowerServers(c lowering.Ctx) ([]ir.Server, []ir.Diagnostic) { // GetServers never returns an empty slice — it injects a default "/" server // when none are declared — so the loop always runs at least once. servers := c.Doc.GetServers() out := make([]ir.Server, 0, len(servers)) - for _, s := range servers { + var diags []ir.Diagnostic + for i, s := range servers { if s == nil { continue } - out = append(out, lowerServer(s)) + one, serverDiags := lowerServer(c, s, ids.Ptr("servers", strconv.Itoa(i))) + diags = append(diags, serverDiags...) + out = append(out, one) } if len(out) == 0 { - return nil + return nil, diags } - return out + return out, diags } -// lowerServer lowers one server, named by serverName. -func lowerServer(s *soa.Server) ir.Server { - return ir.Server{ +// lowerServer lowers one server, named by serverName, keeping its x-* on the +// ir.Server itself; sptr is the server's own pointer in the servers list. +func lowerServer(c lowering.Ctx, s *soa.Server, sptr string) (ir.Server, []ir.Diagnostic) { + vars, diags := serverVariables(c, s, sptr) + ext, extDiags := annotation.ExtensionsFrom(s.GetExtensions(), c.SrcIndex, sptr) + out := ir.Server{ Name: serverName(s), URLTemplate: s.GetURL(), Description: ir.Docs{Description: s.GetDescription()}, - Variables: serverVariables(s), + Variables: vars, + Unmodeled: ext, } + diags = append(diags, extDiags...) + return out, append(diags, annotation.UnknownKeysIn(&out.Unmodeled, s, c.SrcIndex, sptr)...) } // serverName builds a server's neutral naming: the declared name when the source @@ -134,22 +275,31 @@ func serverName(s *soa.Server) ir.Naming { // serverVariables lowers a server's URL template variables in source order, or // nil when it declares none. -func serverVariables(s *soa.Server) []ir.ServerVariable { +func serverVariables(c lowering.Ctx, s *soa.Server, sptr string) ([]ir.ServerVariable, []ir.Diagnostic) { vars := s.GetVariables() if vars == nil || vars.Len() == 0 { - return nil + return nil, nil } out := make([]ir.ServerVariable, 0, vars.Len()) + var diags []ir.Diagnostic for name, v := range vars.All() { if v == nil { continue } - out = append(out, ir.ServerVariable{ - Name: name, - Default: v.GetDefault(), - Enum: v.GetEnum(), - Docs: ir.Docs{Description: v.GetDescription()}, - }) + vptr := sptr + ids.Ptr("variables", name) + // ServerVariable exposes no GetExtensions at this library version, so the + // field is read directly — as XMLHints already reads its own. + ext, extDiags := annotation.ExtensionsFrom(v.Extensions, c.SrcIndex, vptr) + diags = append(diags, extDiags...) + one := ir.ServerVariable{ + Name: name, + Default: v.GetDefault(), + Enum: v.GetEnum(), + Docs: ir.Docs{Description: v.GetDescription()}, + Unmodeled: ext, + } + diags = append(diags, annotation.UnknownKeysIn(&one.Unmodeled, v, c.SrcIndex, vptr)...) + out = append(out, one) } - return out + return out, diags } diff --git a/compilers/openapi/meta_test.go b/compilers/openapi/meta_test.go index 528f6243..b37c2d84 100644 --- a/compilers/openapi/meta_test.go +++ b/compilers/openapi/meta_test.go @@ -36,9 +36,9 @@ paths: // TestMeta_UnserializableExtensionStillWarns pins the document-level twin of // TestAuth_UnserializableExtensionStillWarns: when every top-level x-* extension -// fails to serialize, lowerMeta's "if len(ext) > 0" guard is false, so the -// warning must already have been recorded by the extension reader rather than appended -// inside the guard — the shape that used to drop it silently. +// fails to serialize, lowerMeta ends up with an empty Unmodeled map, so the +// warning must come from the extension reader rather than from a branch guarded +// on what was kept — the shape that used to drop it silently. func TestMeta_UnserializableExtensionStillWarns(t *testing.T) { t.Parallel() spec := `openapi: 3.1.0 @@ -70,6 +70,20 @@ func TestMeta_FullDocumentMetadata(t *testing.T) { assert.Equal(t, []string{"us", "eu"}, doc.Servers[0].Variables[0].Enum) } +// TestTagExtensions_NilEntrySkipped pins the same guard lowerTagDefs keeps on +// the other side of the tag list: a nil entry contributes no extension site, so +// nothing dereferences it and no site is keyed at an index holding no tag. +func TestTagExtensions_NilEntrySkipped(t *testing.T) { + t.Parallel() + doc := &soa.OpenAPI{Tags: []*soa.Tag{nil, {Name: "kept"}}} + + got := tagExtensions(lowering.Ctx{Doc: doc}) + + require.Len(t, got, 2, "one surviving tag contributes its own site and its externalDocs one") + assert.Equal(t, "tags/1", got[0].Scope, "the site is keyed at the tag's own index, not its position") + assert.Equal(t, "/tags/1", got[0].Owner) +} + func TestMeta_NoInfoNoServers(t *testing.T) { t.Parallel() // With no info block the title is empty; with no servers the library injects @@ -103,7 +117,7 @@ func TestServerName_DerivedFromURLWhenUnnamed(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { t.Parallel() - assert.Equal(t, tc.want, lowerServer(&soa.Server{URL: tc.url}).Name) + assert.Equal(t, tc.want, serverName(&soa.Server{URL: tc.url})) }) } } @@ -114,9 +128,9 @@ func TestServerName_DerivedFromURLWhenUnnamed(t *testing.T) { // both servers the same. func TestServerName_DistinguishesServersDifferingOnlyInPath(t *testing.T) { t.Parallel() - v1 := lowerServer(&soa.Server{URL: "https://api.example.com/v1"}) - v2 := lowerServer(&soa.Server{URL: "https://api.example.com/v2"}) - assert.NotEqual(t, v1.Name.Hint, v2.Name.Hint, "two distinct servers get two distinct hints") + v1 := serverName(&soa.Server{URL: "https://api.example.com/v1"}) + v2 := serverName(&soa.Server{URL: "https://api.example.com/v2"}) + assert.NotEqual(t, v1.Hint, v2.Hint, "two distinct servers get two distinct hints") } // TestServerName_CollidesOnPunctuationAlone bounds that claim, which must not be @@ -126,17 +140,18 @@ func TestServerName_DistinguishesServersDifferingOnlyInPath(t *testing.T) { // known bound rather than a later discovery. func TestServerName_CollidesOnPunctuationAlone(t *testing.T) { t.Parallel() - dotted := lowerServer(&soa.Server{URL: "https://api.example.com/v1"}) - dashed := lowerServer(&soa.Server{URL: "https://api.example.com/v-1"}) - assert.Equal(t, dotted.Name.Hint, dashed.Name.Hint, + dotted := serverName(&soa.Server{URL: "https://api.example.com/v1"}) + dashed := serverName(&soa.Server{URL: "https://api.example.com/v-1"}) + assert.Equal(t, dotted.Hint, dashed.Hint, "neutral words carry no punctuation, so these two collide") } func TestLowerServers_NilEntrySkipped(t *testing.T) { t.Parallel() doc := &soa.OpenAPI{Servers: []*soa.Server{nil, {URL: "https://x.example.com"}}} - got := lowerServers(lowering.Ctx{Doc: doc}) + got, diags := lowerServers(lowering.Ctx{Doc: doc}) + assert.Empty(t, diags) require.Len(t, got, 1, "nil server entry skipped, valid one lowered") assert.Equal(t, "https://x.example.com", got[0].URLTemplate) } @@ -147,7 +162,8 @@ func TestServerVariables_NilEntrySkipped(t *testing.T) { sequencedmap.NewElem("skip", (*soa.ServerVariable)(nil)), sequencedmap.NewElem("keep", &soa.ServerVariable{}), ) - srv := lowerServer(&soa.Server{URL: "https://x", Variables: vars}) + srv, diags := lowerServer(lowering.Ctx{}, &soa.Server{URL: "https://x", Variables: vars}, "/servers/0") + assert.Empty(t, diags) require.Len(t, srv.Variables, 1, "nil variable entry skipped") assert.Equal(t, "keep", srv.Variables[0].Name) } @@ -159,5 +175,7 @@ func TestLowerServers_EveryEntrySkippedIsNil(t *testing.T) { t.Parallel() doc := &soa.OpenAPI{Servers: []*soa.Server{nil, nil}} - assert.Nil(t, lowerServers(lowering.Ctx{Doc: doc})) + got, diags := lowerServers(lowering.Ctx{Doc: doc}) + assert.Nil(t, got) + assert.Empty(t, diags) } diff --git a/compilers/openapi/unknownkeys_test.go b/compilers/openapi/unknownkeys_test.go new file mode 100644 index 00000000..34f5269c --- /dev/null +++ b/compilers/openapi/unknownkeys_test.go @@ -0,0 +1,227 @@ +// This file covers one property: a key the OpenAPI model names no field for +// reaches the IR rather than vanishing between the parser and the lowering. It +// is separate from annotations_test.go because the subject is the complement of +// what the model names, not any one annotation. +package openapi_test // external test package — exercises only the public API + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/dexpace/morphic/ir" +) + +// unknownKeysSpec reads the census fixture the corpus sweeps also drive, so the +// assertions below and the six oracles run over the same bytes: putting it under +// testdata is what gets it compiled in both declaration orders, round-tripped and +// verified, none of which a spec written inline reaches. +func unknownKeysSpec(t *testing.T) string { + t.Helper() + data, err := os.ReadFile(filepath.Join("..", "..", "testdata", "openapi", "unknown_keys.yaml")) + require.NoError(t, err) + return string(data) +} + +// TestUnknownKeys_KeptAtEveryObject holds the whole rule rather than the +// positions that happened to be noticed. A key the model does not name reached +// no IR field, no Unmodeled entry and no diagnostic at every one of these +// objects, so two documents differing only in it compiled to the same IR +// (GitHub #297). +// +// Carriers are derived from the value graph rather than named: each row says +// which Unmodeled map the entry must land on by the path the walk reaches it at, +// so an entry written to the wrong carrier fails here rather than passing +// because the assertion looked only where it expected. +func TestUnknownKeys_KeptAtEveryObject(t *testing.T) { + t.Parallel() + doc, diags := compileAnnotationSpec(t, "unknown-keys", unknownKeysSpec(t)) + requireNoErrorDiagnostics(t, diags) + sites := unmodeledSites(doc) + + for _, tc := range []struct { + object string + key string + want string + carrier string + }{ + {"openapi root", "openapi:basePath", `"ROOT"`, "doc.Unmodeled"}, + {"info", "openapi:info/contactEmail", `"INFO"`, "doc.Unmodeled"}, + {"contact", "openapi:info/contact/slack", `"CONTACT"`, "doc.Unmodeled"}, + {"license", "openapi:info/license/spdx", `"LICENSE"`, "doc.Unmodeled"}, + {"externalDocs", "openapi:externalDocs/title", `"EXTERNALDOCS"`, "doc.Unmodeled"}, + {"server", "openapi:host", `"SERVER"`, "doc.Servers[0].Unmodeled"}, + {"server variable", "openapi:example", `"SERVERVARIABLE"`, "doc.Servers[0].Variables[0].Unmodeled"}, + {"tag", "openapi:tags/0/color", `"TAG"`, "doc.Unmodeled"}, + {"components", "openapi:components/definitions", `"COMPONENTS"`, "doc.Unmodeled"}, + {"tag externalDocs", "openapi:tags/0/externalDocs/title", `"TAGEXTERNALDOCS"`, "doc.Unmodeled"}, + {"operation", "openapi:operationid", `"OPERATION"`, ".Unmodeled"}, + // The carrier is named down to the operation rather than left at + // ".Unmodeled": the document writes this exact key too, and the row is only + // evidence of the operation's if it cannot match the document's. + {"operation externalDocs", "openapi:externalDocs/title", `"OPERATIONEXTERNALDOCS"`, + "Operations[0].Unmodeled"}, + {"parameter", "openapi:collectionFormat", `"PARAMETER"`, ".Params[0].Unmodeled"}, + {"example", "openapi:name", `"EXAMPLE"`, ".Params[0].Examples[0].Unmodeled"}, + {"request body", "openapi:schema", `"REQUESTBODY"`, ".Request.Unmodeled"}, + {"media type", "openapi:format", `"MEDIATYPE"`, ".Contents[0].Unmodeled"}, + // On the content rather than on the part: ir.PartEncoding holds no + // Unmodeled map, and one content can carry an entry per part, so the part + // name is what tells two encodings' keys apart on that one map. + {"encoding", "openapi:encoding/part/contentEncoding", `"ENCODING"`, ".Contents[1].Unmodeled"}, + {"response", "openapi:status", `"RESPONSE"`, ".Responses[0].Unmodeled"}, + {"error response", "openapi:status", `"ERRORRESPONSE"`, ".Errors[0].Unmodeled"}, + {"header", "openapi:in", `"HEADER"`, ".Headers[0].Unmodeled"}, + {"security scheme", "openapi:tokenUrl", `"SECURITYSCHEME"`, + "doc.Auth[auth/openapi/components/securitySchemes/k].Unmodeled"}, + {"oauth flows", "openapi:flows/application", `"OAUTHFLOWS"`, + "doc.Auth[auth/openapi/components/securitySchemes/o].Unmodeled"}, + {"oauth flow", "openapi:scope", `"OAUTHFLOW"`, + "doc.Auth[auth/openapi/components/securitySchemes/o].Flows[0].Unmodeled"}, + {"schema", "openapi:additionalItems", `"SCHEMA"`, + "doc.Types[t/openapi/components/schemas/S].Unmodeled"}, + // The property's schema reduced to a shared primitive, so it owns no node + // and its keywords stay on the declaring property — the carrier rule + // fillPropertyAnnotations already applies to every other annotation. + {"property schema", "openapi:divisibleBy", `"PROPERTYSCHEMA"`, + "doc.Types[t/openapi/components/schemas/S].Properties[0].Unmodeled"}, + } { + site, found := findUnmodeled(sites, tc.key, tc.want) + if !assert.True(t, found, "%s drops %s = %s: it is nowhere in the document", + tc.object, tc.key, tc.want) { + continue + } + assert.Contains(t, site.path, tc.carrier, "%s key lands on the wrong carrier", tc.object) + } +} + +// TestUnknownKeys_SchemaAndObjectAreGradedApart pins the one distinction the +// census turns on, at the two keys the fixture picks for it: a draft-07 +// additionalItems in a schema, and an operationId with the case wrong on an +// operation. +// +// JSON Schema states that an implementation must ignore a keyword it does not +// recognize, so an unrecognized keyword in a schema is legal input and may carry +// meaning for other tooling; OpenAPI states that an extension key must be +// prefixed x-, so an undefined key on one of its objects is not an extension but +// a defect in the document. +// +// Both are kept — invariant 2 does not bend for invalid input — and both carry +// the same reason, because "no IR node is coming" is true of each. What differs +// is what the document did, which is the diagnostic channel's subject: an +// unrecognized keyword records a decision at info, and an undefined key reports +// a fault at warning. +func TestUnknownKeys_SchemaAndObjectAreGradedApart(t *testing.T) { + t.Parallel() + doc, diags := compileAnnotationSpec(t, "unknown-keys", unknownKeysSpec(t)) + requireNoErrorDiagnostics(t, diags) + + schema, ok := doc.Types[ir.TypeID("t/openapi/components/schemas/S")] + require.True(t, ok) + keyword := unmodeledEntry(t, schema.Common().Unmodeled, "openapi:additionalItems") + assert.Equal(t, ir.ReasonOutOfScope, keyword.Reason, + "no IR node is coming for a keyword this compiler does not model") + assert.Equal(t, "/components/schemas/S/additionalItems", keyword.Provenance.Pointer) + assert.Equal(t, []ir.Severity{ir.SeverityInfo}, + diagsAt(diags, "openapi/unknown-schema-keyword", "/components/schemas/S/additionalItems")) + + op, ok := opByName(doc, "listWidgets") + require.True(t, ok) + key := unmodeledEntry(t, op.Unmodeled, "openapi:operationid") + assert.Equal(t, ir.ReasonOutOfScope, key.Reason, + "OpenAPI defines no such key, so no IR node is coming for it either") + assert.Equal(t, "/paths/~1widgets/get/operationid", key.Provenance.Pointer) + assert.Equal(t, []ir.Severity{ir.SeverityWarning}, + diagsAt(diags, "openapi/unknown-object-key", "/paths/~1widgets/get/operationid")) +} + +// TestUnknownKeys_WellFormedDocumentRecordsNothing is the control the census +// needs: a document writing only what the model names keeps no entry and reports +// no finding, so the rows above are evidence of the keys they name rather than +// of a sweep that fires on everything. +func TestUnknownKeys_WellFormedDocumentRecordsNothing(t *testing.T) { + t.Parallel() + doc, diags := compileAnnotationSpec(t, "clean", `openapi: 3.1.0 +info: {title: T, version: "1"} +paths: + /widgets: + get: + operationId: listWidgets + responses: {"200": {description: ok}} +components: + schemas: + S: {type: object, properties: {a: {type: string}}} +`) + requireNoErrorDiagnostics(t, diags) + + assert.Empty(t, unmodeledSites(doc), "nothing undeclared, so nothing kept") + for _, d := range diags { + assert.NotContains(t, d.Code, "unknown-", "a well-formed document reports no unknown key: %+v", d) + } +} + +// TestUnknownKeys_SchemaSubObjects covers the three objects that hang off a +// schema — its xml, its discriminator and its externalDocs — which the fixture +// above deliberately leaves out. +// +// It leaves them out because the OpenAPI dialect meta-schema closes all three to +// anything but an x- key, so the library reports a validation error on each and +// harness.Check returns at the first one, before the oracles that fixture exists +// to reach. The keys are kept and announced all the same, which is what this +// asserts: an invalid document is still not a document whose keys may vanish. +// +// Graded as an OpenAPI object's keys rather than as schema keywords, at warning +// rather than info: JSON Schema's rule that an unrecognized keyword is legal +// governs the schema, and these three are OpenAPI objects the schema vocabulary +// says nothing about. All three ride on the schema's own map, since none of +// ir.XMLHints, ir.Discriminator or ir.Link holds one, and the keyword each was +// written under is what keeps them apart there. +func TestUnknownKeys_SchemaSubObjects(t *testing.T) { + t.Parallel() + doc, diags := compileAnnotationSpec(t, "schema-sub-objects", `openapi: 3.1.0 +info: {title: T, version: "1"} +paths: {} +components: + schemas: + S: + type: object + xml: {name: s, attribute2: XML} + discriminator: {propertyName: k, mapping2: DISCRIMINATOR} + externalDocs: {url: 'https://d.example', title: SCHEMAEXTERNALDOCS} + properties: {k: {type: string}} +`) + schema, ok := doc.Types[ir.TypeID("t/openapi/components/schemas/S")] + require.True(t, ok) + + // assert rather than require on the lookup, so a keyword missing from the map + // reports itself and leaves the other two still checked. The three share one + // reader, and a row that never runs is no evidence about the keyword it names. + for _, tc := range []struct{ keyword, key, want string }{ + {"xml", "openapi:xml/attribute2", `"XML"`}, + {"discriminator", "openapi:discriminator/mapping2", `"DISCRIMINATOR"`}, + {"externalDocs", "openapi:externalDocs/title", `"SCHEMAEXTERNALDOCS"`}, + } { + entry, found := schema.Common().Unmodeled[tc.key] + if !assert.True(t, found, "schema %s drops %s: it is nowhere on the schema", tc.keyword, tc.key) { + continue + } + assert.Equal(t, tc.want, string(entry.Value), "%s key keeps what the source wrote", tc.keyword) + assert.Equal(t, ir.ReasonOutOfScope, entry.Reason, "%s key reason", tc.keyword) + at := "/components/schemas/S/" + tc.keyword + "/" + tc.key[len("openapi:"+tc.keyword+"/"):] + assert.Equal(t, at, entry.Provenance.Pointer, "%s key provenance", tc.keyword) + assert.Equal(t, []ir.Severity{ir.SeverityWarning}, + diagsAt(diags, "openapi/unknown-object-key", at), + "%s key is announced as an object's, not as a schema keyword's", tc.keyword) + } +} + +// requireNoErrorDiagnostics fails the test on the first error-severity +// diagnostic, naming it. +func requireNoErrorDiagnostics(t *testing.T, diags []ir.Diagnostic) { + t.Helper() + d, ok := ir.FirstError(diags) + require.False(t, ok, "unexpected error diagnostic: %+v", d) +} diff --git a/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json b/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json index c43e9937..3218cb4f 100644 --- a/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json +++ b/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json @@ -44,7 +44,7 @@ }, "anonymous": true, "docs": { - "description": "named by position" + "description": "named by position, not by target" }, "sensitive": false, "provenance": { @@ -366,7 +366,7 @@ "eventPayload": false, "secret": false, "docs": { - "description": "named by position" + "description": "named by position, not by target" }, "provenance": { "source": 0, @@ -548,7 +548,7 @@ { "format": "openapi@3.1", "path": "allof-oneof-cooccurrence.yaml", - "hash": "cb7fb6bc613222b0211fc363b9028e7739b44a1d14b04f74cf709b8d603655f2" + "hash": "580c2ffd45b8b24e175b4c5b7f1af6c237a920de7d1942d84b10a00ad2dc1e33" } ] } diff --git a/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml b/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml index f085da90..042727df 100644 --- a/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml +++ b/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml @@ -56,5 +56,5 @@ components: branch: {$ref: '#/components/schemas/InlineHost/oneOf/0'} InlineHost: oneOf: - - {type: integer, description: named by position, not by target} + - {type: integer, description: 'named by position, not by target'} - {type: string} diff --git a/testdata/conformance/openapi/extensions-x.golden.json b/testdata/conformance/openapi/extensions-x.golden.json index ce499e59..c67df892 100644 --- a/testdata/conformance/openapi/extensions-x.golden.json +++ b/testdata/conformance/openapi/extensions-x.golden.json @@ -2,7 +2,19 @@ "irVersion": "0.3.0", "name": "ExtensionsX", "version": "1.0.0", - "docs": {}, + "docs": { + "externalDocs": [ + { + "url": "https://d.example" + } + ] + }, + "contact": { + "name": "n" + }, + "license": { + "name": "MIT" + }, "services": [ { "id": "s/openapi/0", @@ -14,9 +26,16 @@ "groups": [ { "name": { - "hint": "default" + "source": "t1", + "canonical": "t_1" + }, + "docs": { + "externalDocs": [ + { + "url": "https://t.example" + } + ] }, - "docs": {}, "operations": [ { "id": "op/openapi/paths/~1widgets/get", @@ -24,7 +43,37 @@ "source": "listWidgets", "canonical": "list_widgets" }, - "docs": {}, + "docs": { + "externalDocs": [ + { + "url": "https://o.example" + } + ] + }, + "params": [ + { + "name": { + "source": "shape", + "canonical": "shape" + }, + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "docs": {}, + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XPARAMETER", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/parameters/0/x-mark" + } + } + } + } + ], "responses": [ { "name": { @@ -38,26 +87,184 @@ } ] }, + "headers": [ + { + "id": "p/openapi/paths/~1widgets/get/responses/200/headers/X-Rate", + "name": { + "source": "X-Rate", + "canonical": "x_rate" + }, + "wireName": "X-Rate", + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XHEADER", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/responses/200/headers/X-Rate/x-mark" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/responses/200/headers/X-Rate" + } + } + ], "docs": { "description": "ok" + }, + "unmodeled": { + "openapi:links": { + "reason": "no_ir_home", + "value": { + "self": { + "operationId": "listWidgets" + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/responses/200/links" + } + }, + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XRESPONSE", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/responses/200/x-mark" + } + } + } + } + ], + "errors": [ + { + "type": { + "target": "", + "nullable": false + }, + "conditions": { + "statusCodes": [ + { + "from": 404, + "to": 404 + } + ] + }, + "fault": "client", + "docs": { + "description": "gone" + }, + "unmodeled": { + "openapi:links": { + "reason": "no_ir_home", + "value": { + "retry": { + "operationId": "listWidgets" + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/responses/404/links" + } + }, + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XERRORRESPONSE", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/responses/404/x-mark" + } + } } } ], "oneWay": false, "idempotency": {}, "auth": null, + "tags": [ + "t1" + ], "bindings": { "http": [ { "method": "GET", "uriTemplate": "/widgets", "sharedRoute": false, + "paramBindings": [ + { + "param": "shape", + "location": "query", + "wireName": "shape", + "style": "form", + "explode": true, + "allowReserved": false + } + ], "checksumRequired": false, - "isWebhook": false + "isWebhook": false, + "callbacks": [ + { + "expression": "{$request.body#/u}", + "operations": [ + "op/openapi/paths/~1widgets/get/callbacks/onEvent/{$request.body#~1u}/post" + ] + } + ], + "unmodeled": { + "openapi:callbacks/onEvent/x-mark": { + "reason": "vendor_extension", + "value": "XCALLBACK", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/callbacks/onEvent/x-mark" + } + } + } } ] }, "unmodeled": { + "openapi:externalDocs/x-mark": { + "reason": "vendor_extension", + "value": "XOPERATIONEXTERNALDOCS", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/externalDocs/x-mark" + } + }, + "openapi:pathItem/x-mark": { + "reason": "vendor_extension", + "value": "XPATHITEM", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/x-mark" + } + }, + "openapi:responses/x-mark": { + "reason": "vendor_extension", + "value": "XRESPONSES", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/responses/x-mark" + } + }, "openapi:x-internal": { "reason": "vendor_extension", "value": true, @@ -71,17 +278,384 @@ "source": 0, "pointer": "/paths/~1widgets/get" } + }, + { + "id": "op/openapi/paths/~1widgets/get/callbacks/onEvent/{$request.body#~1u}/post", + "name": { + "source": "onEventPost", + "canonical": "on_event_post" + }, + "docs": {}, + "responses": [ + { + "name": { + "hint": "200" + }, + "conditions": { + "statusCodes": [ + { + "from": 200, + "to": 200 + } + ] + }, + "docs": { + "description": "ok" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "POST", + "uriTemplate": "{$request.body#/u}", + "sharedRoute": false, + "checksumRequired": false, + "isWebhook": false + } + ] + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/callbacks/onEvent/{$request.body#~1u}/post" + } + } + ] + }, + { + "name": { + "hint": "default" + }, + "docs": {}, + "operations": [ + { + "id": "op/openapi/paths/~1widgets/post", + "name": { + "source": "createWidget", + "canonical": "create_widget" + }, + "docs": {}, + "params": [ + { + "name": { + "source": "shape", + "canonical": "shape" + }, + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "docs": {}, + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XPARAMETER", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/parameters/0/x-mark" + } + } + } + } + ], + "request": { + "contents": [ + { + "mediaType": "multipart/form-data", + "type": { + "target": "t/anon/paths/~1widgets/post/requestBody/content/multipart~1form-data/schema", + "nullable": false + }, + "encoding": { + "p/openapi/paths/~1widgets/post/requestBody/content/multipart~1form-data/schema/properties/f": { + "contentTypes": [ + "text/plain" + ], + "multi": false, + "filename": false + } + }, + "examples": [ + { + "name": "e1", + "value": { + "kind": "string", + "str": "v", + "bytes": null, + "list": null, + "object": null + }, + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XEXAMPLE", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post/requestBody/content/multipart~1form-data/examples/e1/x-mark" + } + } + } + } + ], + "unmodeled": { + "openapi:encoding/f/allowReserved": { + "reason": "no_ir_home", + "value": true, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post/requestBody/content/multipart~1form-data/encoding/f/allowReserved" + } + }, + "openapi:encoding/f/x-mark": { + "reason": "vendor_extension", + "value": "XENCODING", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post/requestBody/content/multipart~1form-data/encoding/f/x-mark" + } + }, + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XMEDIATYPE", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post/requestBody/content/multipart~1form-data/x-mark" + } + } + } + } + ], + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XREQUESTBODY", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post/requestBody/x-mark" + } + } + } + }, + "responses": [ + { + "name": { + "hint": "201" + }, + "conditions": { + "statusCodes": [ + { + "from": 201, + "to": 201 + } + ] + }, + "docs": { + "description": "created" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "POST", + "uriTemplate": "/widgets", + "sharedRoute": false, + "paramBindings": [ + { + "param": "shape", + "location": "query", + "wireName": "shape", + "style": "form", + "explode": true, + "allowReserved": false + } + ], + "requestContentTypes": [ + "multipart/form-data" + ], + "checksumRequired": false, + "isWebhook": false + } + ] + }, + "unmodeled": { + "openapi:pathItem/x-mark": { + "reason": "vendor_extension", + "value": "XPATHITEM", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/x-mark" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post" + } } ] } ], "auth": null, + "unmodeled": { + "openapi:paths/x-mark": { + "reason": "vendor_extension", + "value": "XPATHS", + "provenance": { + "source": 0, + "pointer": "/paths/x-mark" + } + } + }, "provenance": { "source": 0 } } ], "types": { + "t/anon/paths/~1widgets/post/requestBody/content/multipart~1form-data/schema": { + "kind": "model", + "id": "t/anon/paths/~1widgets/post/requestBody/content/multipart~1form-data/schema", + "name": { + "hint": "createWidget_request" + }, + "anonymous": true, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post/requestBody/content/multipart~1form-data/schema" + }, + "properties": [ + { + "id": "p/openapi/paths/~1widgets/post/requestBody/content/multipart~1form-data/schema/properties/f", + "name": { + "source": "f", + "canonical": "f" + }, + "wireName": "f", + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post/requestBody/content/multipart~1form-data/schema/properties/f" + } + } + ], + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/openapi/components/schemas/A": { + "kind": "model", + "id": "t/openapi/components/schemas/A", + "name": { + "source": "A", + "canonical": "a" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/A" + }, + "properties": [ + { + "id": "p/openapi/components/schemas/A/properties/k", + "name": { + "source": "k", + "canonical": "k" + }, + "wireName": "k", + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/components/schemas/A/properties/k" + } + } + ], + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/openapi/components/schemas/D": { + "kind": "union", + "id": "t/openapi/components/schemas/D", + "name": { + "source": "D", + "canonical": "d" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "unmodeled": { + "openapi:discriminator/x-mark": { + "reason": "vendor_extension", + "value": "XDISCRIMINATOR", + "provenance": { + "source": 0, + "pointer": "/components/schemas/D/discriminator/x-mark" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/D" + }, + "variants": [ + { + "name": { + "hint": "A" + }, + "type": { + "target": "t/openapi/components/schemas/A", + "nullable": false + }, + "docs": {} + } + ], + "exclusive": true, + "wireTagged": false, + "discriminator": { + "propertyName": "k", + "inferred": false + } + }, "t/openapi/components/schemas/S": { "kind": "model", "id": "t/openapi/components/schemas/S", @@ -90,9 +664,27 @@ "canonical": "s" }, "anonymous": false, - "docs": {}, + "docs": { + "externalDocs": [ + { + "url": "https://s.example" + } + ] + }, "sensitive": false, + "xml": { + "name": "s", + "wrapped": false + }, "unmodeled": { + "openapi:externalDocs/x-mark": { + "reason": "vendor_extension", + "value": "XSCHEMAEXTERNALDOCS", + "provenance": { + "source": 0, + "pointer": "/components/schemas/S/externalDocs/x-mark" + } + }, "openapi:x-blob": { "reason": "vendor_extension", "value": "aGVsbG8=", @@ -124,6 +716,14 @@ "source": 0, "pointer": "/components/schemas/S/x-wrapped" } + }, + "openapi:xml/x-mark": { + "reason": "vendor_extension", + "value": "XXML", + "provenance": { + "source": 0, + "pointer": "/components/schemas/S/xml/x-mark" + } } }, "provenance": { @@ -176,17 +776,181 @@ "prim": "string" } }, + "auth": { + "auth/openapi/components/securitySchemes/k": { + "id": "auth/openapi/components/securitySchemes/k", + "name": { + "source": "k", + "canonical": "k" + }, + "kind": "apiKey", + "docs": {}, + "in": "header", + "keyName": "K", + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XSECURITYSCHEME", + "provenance": { + "source": 0, + "pointer": "/components/securitySchemes/k/x-mark" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/securitySchemes/k" + } + }, + "auth/openapi/components/securitySchemes/o": { + "id": "auth/openapi/components/securitySchemes/o", + "name": { + "source": "o", + "canonical": "o" + }, + "kind": "oauth2", + "docs": {}, + "flows": [ + { + "kind": "implicit", + "authorizationURL": "https://a.example", + "scopes": { + "read": "r" + }, + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XOAUTHFLOW", + "provenance": { + "source": 0, + "pointer": "/components/securitySchemes/o/flows/implicit/x-mark" + } + } + } + } + ], + "unmodeled": { + "openapi:flows/x-mark": { + "reason": "vendor_extension", + "value": "XOAUTHFLOWS", + "provenance": { + "source": 0, + "pointer": "/components/securitySchemes/o/flows/x-mark" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/securitySchemes/o" + } + } + }, "servers": [ { "name": { - "hint": "server" + "hint": "https_e_example_v" }, - "urlTemplate": "/", + "urlTemplate": "https://e.example/{v}", "description": {}, - "auth": null + "variables": [ + { + "name": "v", + "default": "1", + "docs": {}, + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XSERVERVARIABLE", + "provenance": { + "source": 0, + "pointer": "/servers/0/variables/v/x-mark" + } + } + } + } + ], + "auth": null, + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XSERVER", + "provenance": { + "source": 0, + "pointer": "/servers/0/x-mark" + } + } + } + } + ], + "tagDefs": [ + { + "name": "t1", + "docs": { + "externalDocs": [ + { + "url": "https://t.example" + } + ] + } } ], "unmodeled": { + "openapi:components/x-mark": { + "reason": "vendor_extension", + "value": "XCOMPONENTS", + "provenance": { + "source": 0, + "pointer": "/components/x-mark" + } + }, + "openapi:externalDocs/x-mark": { + "reason": "vendor_extension", + "value": "XEXTERNALDOCS", + "provenance": { + "source": 0, + "pointer": "/externalDocs/x-mark" + } + }, + "openapi:info/contact/x-mark": { + "reason": "vendor_extension", + "value": "XCONTACT", + "provenance": { + "source": 0, + "pointer": "/info/contact/x-mark" + } + }, + "openapi:info/license/x-mark": { + "reason": "vendor_extension", + "value": "XLICENSE", + "provenance": { + "source": 0, + "pointer": "/info/license/x-mark" + } + }, + "openapi:info/x-mark": { + "reason": "vendor_extension", + "value": "XINFO", + "provenance": { + "source": 0, + "pointer": "/info/x-mark" + } + }, + "openapi:tags/0/externalDocs/x-mark": { + "reason": "vendor_extension", + "value": "XTAGEXTERNALDOCS", + "provenance": { + "source": 0, + "pointer": "/tags/0/externalDocs/x-mark" + } + }, + "openapi:tags/0/x-mark": { + "reason": "vendor_extension", + "value": "XTAG", + "provenance": { + "source": 0, + "pointer": "/tags/0/x-mark" + } + }, "openapi:x-audience": { "reason": "vendor_extension", "value": "public", @@ -196,11 +960,22 @@ } } }, + "diagnostics": [ + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "encoding allowReserved has no ir.PartEncoding home; kept verbatim under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post/requestBody/content/multipart~1form-data/encoding/f/allowReserved" + } + } + ], "sources": [ { "format": "openapi@3.1", "path": "extensions-x.yaml", - "hash": "bdd4e78c821cd4ff1e418d0036d0f44c67120aa29ecd201a8d9a2cbc5a825ae9" + "hash": "50a3f85f94845ca391a312360b2c49c55938271c370d5ccc682ded1ab6f35f17" } ] } diff --git a/testdata/conformance/openapi/extensions-x.yaml b/testdata/conformance/openapi/extensions-x.yaml index dfbcee4a..7304f57b 100644 --- a/testdata/conformance/openapi/extensions-x.yaml +++ b/testdata/conformance/openapi/extensions-x.yaml @@ -1,15 +1,65 @@ openapi: 3.1.0 -info: {title: ExtensionsX, version: "1.0.0"} x-audience: public +info: + title: ExtensionsX + version: "1.0.0" + x-mark: XINFO + contact: {name: n, x-mark: XCONTACT} + license: {name: MIT, x-mark: XLICENSE} +externalDocs: {url: https://d.example, x-mark: XEXTERNALDOCS} +servers: + - url: https://e.example/{v} + x-mark: XSERVER + variables: {v: {default: "1", x-mark: XSERVERVARIABLE}} +tags: + - name: t1 + x-mark: XTAG + externalDocs: {url: https://t.example, x-mark: XTAGEXTERNALDOCS} paths: + x-mark: XPATHS /widgets: + x-mark: XPATHITEM + parameters: [{name: shape, in: query, schema: {type: string}, x-mark: XPARAMETER}] get: operationId: listWidgets + tags: [t1] x-internal: true + externalDocs: {url: https://o.example, x-mark: XOPERATIONEXTERNALDOCS} responses: + x-mark: XRESPONSES "200": description: ok + x-mark: XRESPONSE + headers: {X-Rate: {schema: {type: string}, x-mark: XHEADER}} + links: {self: {operationId: listWidgets}} + "404": + description: gone + x-mark: XERRORRESPONSE + links: {retry: {operationId: listWidgets}} + callbacks: + onEvent: + x-mark: XCALLBACK + "{$request.body#/u}": + post: + operationId: onEventPost + responses: {"200": {description: ok}} + post: + operationId: createWidget + requestBody: + x-mark: XREQUESTBODY + required: true + content: + multipart/form-data: + x-mark: XMEDIATYPE + schema: {type: object, properties: {f: {type: string}}} + encoding: + f: {contentType: text/plain, allowReserved: true, x-mark: XENCODING} + examples: + e1: {value: v, x-mark: XEXAMPLE} + responses: + "201": {description: created} components: + x-mark: XCOMPONENTS schemas: S: type: object @@ -24,5 +74,23 @@ components: x-wrapped: !!binary | aGVs bG8= + xml: {name: s, x-mark: XXML} + externalDocs: {url: https://s.example, x-mark: XSCHEMAEXTERNALDOCS} properties: a: {type: string} + D: + oneOf: [{$ref: '#/components/schemas/A'}] + discriminator: {propertyName: k, x-mark: XDISCRIMINATOR} + A: + type: object + properties: {k: {type: string}} + securitySchemes: + k: {type: apiKey, name: K, in: header, x-mark: XSECURITYSCHEME} + o: + type: oauth2 + flows: + x-mark: XOAUTHFLOWS + implicit: + authorizationUrl: https://a.example + scopes: {read: r} + x-mark: XOAUTHFLOW diff --git a/testdata/conformance/openapi/multipart-encoding.golden.json b/testdata/conformance/openapi/multipart-encoding.golden.json index fde8c823..c424e100 100644 --- a/testdata/conformance/openapi/multipart-encoding.golden.json +++ b/testdata/conformance/openapi/multipart-encoding.golden.json @@ -269,6 +269,16 @@ "style": "form", "explode": true } + }, + "unmodeled": { + "openapi:encoding/ids/allowReserved": { + "reason": "no_ir_home", + "value": true, + "provenance": { + "source": 0, + "pointer": "/paths/~1submit/post/requestBody/content/application~1x-www-form-urlencoded/encoding/ids/allowReserved" + } + } } } ], @@ -721,6 +731,15 @@ "pointer": "/paths/~1upload-composed/post/requestBody" } }, + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "encoding allowReserved has no ir.PartEncoding home; kept verbatim under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/paths/~1submit/post/requestBody/content/application~1x-www-form-urlencoded/encoding/ids/allowReserved" + } + }, { "severity": "info", "code": "openapi/degraded-construct", @@ -735,7 +754,7 @@ { "format": "openapi@3.1", "path": "multipart-encoding.yaml", - "hash": "3fc8c047afb3b7a847d3875de98e50a9b09052a495631f6f884ae197e2a17423" + "hash": "b15abb2581ba82770d20ef9459dd63c4871f3ab3e2f7a84f83dc146f26314e1e" } ] } diff --git a/testdata/conformance/openapi/multipart-encoding.yaml b/testdata/conformance/openapi/multipart-encoding.yaml index 3f5508f3..fabfa657 100644 --- a/testdata/conformance/openapi/multipart-encoding.yaml +++ b/testdata/conformance/openapi/multipart-encoding.yaml @@ -62,7 +62,10 @@ paths: type: array items: {type: string} encoding: - ids: {style: form, explode: true} + # allowReserved sits beside style and explode in the source and has + # no ir.PartEncoding field, where those two do; it is kept verbatim + # on the content instead of vanishing (GitHub #291). + ids: {style: form, explode: true, allowReserved: true} responses: "200": description: ok diff --git a/testdata/conformance/openapi/response-links.golden.json b/testdata/conformance/openapi/response-links.golden.json index 5478dd7a..57bcfa4e 100644 --- a/testdata/conformance/openapi/response-links.golden.json +++ b/testdata/conformance/openapi/response-links.golden.json @@ -60,6 +60,43 @@ } } ], + "errors": [ + { + "type": { + "target": "", + "nullable": false + }, + "conditions": { + "statusCodes": [ + { + "from": 409, + "to": 409 + } + ] + }, + "fault": "client", + "docs": { + "description": "conflict" + }, + "unmodeled": { + "openapi:links": { + "reason": "no_ir_home", + "value": { + "GetConflicting": { + "operationId": "getOrder", + "parameters": { + "orderId": "$response.body#/existingId" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1orders/post/responses/409/links" + } + } + } + } + ], "oneWay": false, "idempotency": {}, "auth": null, @@ -184,7 +221,7 @@ { "format": "openapi@3.1", "path": "response-links.yaml", - "hash": "76f06e7ecfec2ebe4bbd3c5ca6e02f781042b76afc35be07e1d4be8a53b0f85c" + "hash": "15ba7e8058c2731f060d6d93c0a9ec22aa2731add81e5a1a19372d16f72b169f" } ] } diff --git a/testdata/conformance/openapi/response-links.yaml b/testdata/conformance/openapi/response-links.yaml index fc6f9a90..eb9425e1 100644 --- a/testdata/conformance/openapi/response-links.yaml +++ b/testdata/conformance/openapi/response-links.yaml @@ -5,6 +5,16 @@ paths: post: operationId: createOrder responses: + # The error case first, so the fixture is declared in the order that used + # to be wrong: links were kept on a success response and dropped on an + # error one, and a fixture whose 4xx trails a 2xx that already works + # cannot show the difference. + "409": + description: conflict + links: + GetConflicting: + operationId: getOrder + parameters: {orderId: '$response.body#/existingId'} "201": description: created links: diff --git a/testdata/conformance/openapi/unwitnessed.golden.txt b/testdata/conformance/openapi/unwitnessed.golden.txt index 0ef3569a..21893c70 100644 --- a/testdata/conformance/openapi/unwitnessed.golden.txt +++ b/testdata/conformance/openapi/unwitnessed.golden.txt @@ -51,7 +51,6 @@ Example.Error Example.Headers Example.Input Example.Output -Example.Unmodeled External.Identity External.MinVersion External.Package @@ -71,7 +70,6 @@ HTTPBinding.PatchImplicitOptionality HTTPBinding.ResponseBodyPath HTTPBinding.SharedRoute HTTPBinding.SuccessStatus -HTTPBinding.Unmodeled HTTPParamBinding.BodyPath HTTPParamBinding.ParamPath HTTPParamBinding.PathPattern @@ -113,7 +111,6 @@ Model.Implements Model.InputOnly Model.Positional Naming.Aliases -OAuthFlow.Unmodeled OTPBinding.Behaviour OTPBinding.Kind OTPBinding.Process @@ -195,8 +192,6 @@ Server.Bindings Server.Protocol Server.ProtocolVersion Server.Tags -Server.Unmodeled -ServerVariable.Unmodeled Service.CommonErrors Service.Extends Service.Namespace @@ -204,7 +199,6 @@ Service.Protocols Service.Provenance Service.Renames Service.Servers -Service.Unmodeled Service.Version StreamDetail.Events StreamDetail.Initial diff --git a/testdata/openapi/unknown_keys.yaml b/testdata/openapi/unknown_keys.yaml new file mode 100644 index 00000000..ba1668b1 --- /dev/null +++ b/testdata/openapi/unknown_keys.yaml @@ -0,0 +1,93 @@ +# One key the OpenAPI model names no field for at every object the compiler takes +# a census from, each valued with the object it was written on so an entry found +# at the wrong carrier cannot pass for the right one. +# +# Every key here is one real documents carry rather than invented nonsense: a +# Swagger 2.0 field with no OpenAPI 3 equivalent (basePath, host, definitions, a +# body parameter's schema, a flow named `application`), a keyword from an older +# JSON Schema draft (additionalItems, divisibleBy), a field belonging to a +# neighbouring object (a flow's tokenUrl on the scheme, a parameter's `in` on a +# header, a documentation page's `title` on an externalDocs), or a field with the +# case or the number wrong (operationid, scope). None of them reached an IR field, +# an Unmodeled entry or a diagnostic. +# +# `title` is written on all three externalDocs objects and `scope` on the one +# flow, so the entries a document can write more than once are here in more than +# one copy: an unscoped key would leave a single entry and the survivor would +# depend on which lowering ran last. +# +# A schema's own xml, discriminator and externalDocs are censused too but are not +# here. The OpenAPI dialect meta-schema closes those three to undeclared keys, so +# one draws a library validation error, and an error diagnostic stops +# harness.Check before the oracles this fixture exists to reach — see +# TestUnknownKeys_SchemaSubObjects for where they are asserted instead. +openapi: 3.1.0 +basePath: ROOT +info: + title: T + version: "1" + contactEmail: INFO + contact: {name: n, slack: CONTACT} + license: {name: MIT, spdx: LICENSE} +externalDocs: {url: 'https://d.example', title: EXTERNALDOCS} +servers: + - url: 'https://a.example/{region}' + host: SERVER + variables: + region: {default: us, example: SERVERVARIABLE} +tags: + - name: t1 + color: TAG + externalDocs: {url: 'https://t.example', title: TAGEXTERNALDOCS} +paths: + /widgets: + get: + operationId: listWidgets + tags: [t1] + operationid: OPERATION + externalDocs: {url: 'https://o.example', title: OPERATIONEXTERNALDOCS} + parameters: + - name: shape + in: query + schema: {type: string} + collectionFormat: PARAMETER + examples: + round: {value: circle, name: EXAMPLE} + requestBody: + required: true + schema: REQUESTBODY + content: + application/json: + schema: {$ref: '#/components/schemas/S'} + format: MEDIATYPE + multipart/form-data: + schema: {type: object, properties: {part: {type: string}}} + encoding: + part: {contentType: text/plain, contentEncoding: ENCODING} + responses: + "200": + description: ok + status: RESPONSE + headers: + X-Trace: {schema: {type: string}, in: HEADER} + "404": + description: gone + status: ERRORRESPONSE +components: + definitions: COMPONENTS + securitySchemes: + k: {type: apiKey, in: header, name: X-Key, tokenUrl: SECURITYSCHEME} + o: + type: oauth2 + flows: + application: OAUTHFLOWS + implicit: + authorizationUrl: 'https://auth.example' + scopes: {} + scope: OAUTHFLOW + schemas: + S: + type: object + additionalItems: SCHEMA + properties: + a: {type: string, divisibleBy: PROPERTYSCHEMA}