fix(compilers/openapi): keep undeclared keys on nested objects - #378
Open
OmarAlJarrah wants to merge 3 commits into
Open
fix(compilers/openapi): keep undeclared keys on nested objects#378OmarAlJarrah wants to merge 3 commits into
OmarAlJarrah wants to merge 3 commits into
Conversation
A key the OpenAPI model names no field for was kept at the objects that lower to a node with an Unmodeled map of their own, and dropped in silence at the objects nested inside one: an example, an encoding, the oauth flows and each flow, a schema's xml, discriminator and externalDocs, an operation's and a tag's externalDocs, and the components object. Those had no Unmodeled map to land on until the carriers went in; this reads the census at each of them and keys the entries under the same scheme the extensions there already use, so an entry says which object wrote it and two objects reaching one map cannot collide. The components object was classified as a map with nothing to census. It is not: only the map under each of its keys is the document's to name, while its own key set is the fixed list of component kinds, which the library models as named fields and takes a census over. `paths`, `responses` and a callback are the real maps of that kind, and each is confirmed to fold an unrecognized key into itself rather than report it. Two objects are deliberately left out, both recorded at the code: - A path item. The library folds an unrecognized key into the item's embedded operations map, so there is no census to read; recovering the value needs a method vocabulary wider than the one this compiler owns, which is what #293 is about. Unlike every object above, the key is not lost in silence — folding it reports a type mismatch at error severity naming the key at its own pointer. Filed as #377. - A Link Object, following the same decision made for its extensions. This compiler lowers no Link Object anywhere: a response's links survive only as a verbatim node, and an unreferenced components link is dropped whole. A schema's xml, discriminator and externalDocs are censused but stay out of the corpus fixture. The OpenAPI dialect meta-schema closes all three to anything but an x- key, so an undeclared key there draws a library validation error, and an error diagnostic stops harness.Check before the oracles that fixture exists to reach.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A key the OpenAPI model names no field for was kept at the objects that lower to
a node with an
Unmodeledmap of their own, and dropped in silence at theobjects nested inside one. Those had nowhere to land until the carriers went in.
This reads the census at each of them:
openapi:components/definitionsopenapi:tags/0/externalDocs/titleopenapi:externalDocs/titleir.Exampleopenapi:nameopenapi:encoding/part/contentEncodingopenapi:flows/applicationir.OAuthFlowopenapi:scopeopenapi:xml/attribute2openapi:discriminator/mapping2openapi:externalDocs/titleEntries are keyed under the same scheme the extensions at those objects already
use — unscoped where the object has a map of its own, and scoped by the source
path from the carrier down where it rides on one. That is what lets a document
write
titleon its root, its tag's and its operation'sexternalDocsand getthree entries rather than one whose survivor depends on which lowering ran last;
the fixture writes exactly that, so the scoping is asserted rather than assumed.
The three schema sub-objects are graded as an OpenAPI object's keys (warning)
rather than as schema keywords (info), even though they hang off a schema: the
JSON Schema rule that an unrecognized keyword is legal governs the schema, and
xml,discriminatorandexternalDocsare OpenAPI objects the schemavocabulary says nothing about.
One object was misclassified
componentshad been grouped withpaths,responsesand callbacks as a mapwhere every key is a valid entry and a census has nothing to say. It is not one:
only the map under each of its keys is the document's to name, while its own
key set is the fixed list of component kinds, which the library models as named
fields and takes a census over. Probing all four confirmed the split — the other
three fold an unrecognized key into themselves and report a type mismatch, and
their census is empty;
componentsreports[definitions].Two objects are deliberately left out
Both are stated at the code as well as here.
A path item (
applyPathItem). The library folds a key it does not recognizeinto the item's embedded operations map rather than recording it as undeclared,
so
GetUnknownPropertiesreturns nothing and there is no census to read —confirmed at
speakeasy-api/openapi v1.24.0, the version ingo.mod, by readingthe census off the parsed model directly. Two things decide against the raw-node
read that would recover it:
Folding it produces
validation-type-mismatchat error severity naming thekey at its own pointer, and that validation is unconditional. What is lost is
the key's value, not the fact that it was written.
vocabulary, and the only one this compiler owns is
httpMethods— eightmethods against the library's nine. It has no
query, the method OpenAPI 3.2adds, and a 3.2
queryoperation sits in the same map asget, so the obviousreading would report a valid operation as an undeclared key. That vocabulary is
what openapi: 3.2 additionalOperations are dropped entirely and silently #293 is about, so widening it here would settle that issue as a side
effect of this one.
Filed as #377 with the reproduction and the options.
A Link Object, following the decision #345 records for its extensions. This
compiler lowers no Link Object anywhere: a response's
linkssurvive only as theverbatim node kept on the response, and an unreferenced
components/linksentryis dropped whole (verified: nothing of it appears in the compiled document). An
entry at one of the two positions would be the only trace of a construct the IR
does not model, while duplicating a value the node beside it already carries.
Test plan
testdata/openapi/unknown_keys.yamlgains one undeclared key per object above,each valued with the object it was written on and each a mistake real documents
make — a Swagger 2.0 name (
definitionsforschemas, a flow namedapplication), a neighbouring field (titleon anexternalDocs,contentEncodingon an encoding), or a field with the number wrong (scope).Living under
testdata/is what runs it through the harness oracles ratherthan through assertions alone; the sweep was confirmed to reach this file by
breaking it and watching the sweep name it.
TestUnknownKeys_KeptAtEveryObjectgains a row per object, each naming theUnmodeledmap the entry must land on by the path the value-graph walk reachesit at, so an entry written to the wrong carrier fails rather than passing
because the assertion looked only where it expected. The operation-externalDocs
row names its carrier down to the operation, since the document writes that
exact key too and the row is only evidence of the operation's if it cannot
match the document's.
TestUnknownKeys_SchemaSubObjectscovers the three schema sub-objects, whichthe fixture deliberately omits: the OpenAPI dialect meta-schema closes all three
to anything but an
x-key, so an undeclared key there draws a libraryvalidation error, and an error diagnostic stops
harness.Checkbefore theoracles that fixture exists to reach. Confirmed at both 3.0 and 3.1. The keys
are kept and announced all the same, which is what the test pins.
time and the suite re-run; all ten went red, each with the failing row naming
that object: components, tag externalDocs, operation externalDocs, example,
encoding, oauth flows, oauth flow, and — separately, one keyword at a time,
since they share a reader — schema xml, discriminator and externalDocs.
assertrather thanrequireon the lookup, soa keyword missing from the map reports itself and leaves the other two still
checked. With
requirethe first failure aborted the rest, and a row that neverruns is no evidence about the keyword it names.
-update: nothing moved, includingunwitnessed.golden.txt.gofmt,go vet,golangci-lint(0 issues),go build, andcoverage at 100%.
Closes #357