-
Notifications
You must be signed in to change notification settings - Fork 9.1k
v3.2: Guidance on searching and evaluating schemas #4743
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v3.2-dev
Are you sure you want to change the base?
Conversation
Some OAS features casually state that they depend on the type of data being examined, or implicitly carry ambiguity about how to determine how to parse the data. This section attempts to provide some guidance and limits, requiring only that implementations follow the unambiguous, statically deterministic keywords `$ref` and `allOf`. It also provides for just validating the data (when possible) and using the actual in-memory type when a schema is too complex to analyze statically. One use of this is breaking apart schemas to use them with mixed binary and JSON-compatible data, and a new section has been added to address that. Finally, a typo in a related section was fixed.
src/oas.md
Outdated
|
||
When the data is in a non-JSON format, particularly one such as XML or various form media types where data is stored as strings without type information, it can be necessary to find this information through the relevant Schema Object to determine how to parse the format into a structure that can be validated by the schema. | ||
As schema organization can become very complex, implementations are not expected to handle every possible schema layout. | ||
However, given a known starting point schema (usually the value of the nearest `schema` field), implementations MUST search the following for the relevant keywords (e.g. `type`, `format`, `contentMediaType`, etc.): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this suggesting that if contentMediaType
is present at the top level of the schema that the entire data instance should be decoded with that media type before passing to validation? That would conflict with the expectation by the schema itself that the data instance is a string and that contentSchema
should be used for validation of the decoded instance -- if the instance is already decoded to an object, contentSchema
will do nothing. I think we should recommend that an explicit media type should be specified instead, which is available for all places that a schema is used (parameters as well as message bodies).
Additionally, we could specify that schema keywords that require a specific type (e.g. uniqueItems
, properties
) can be used to infer a specific data type for the instance. I also wouldn't mind if we didn't do this, and instead required the user to be explicit with a type
keyword, so an implementation doesn't have to enumerate all of the type-specific keywords present in JSON Schema (it's version specific, e.g. an OAD that happened to use the a schema dialect under draft2019-09 would need to include additionalItems
-- and asking the OAD parser to also have to parse the $schema
keyword is going a little far).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, is this suggesting that data deeper within the instance (for example items in an array) need type inference as well, or is this intended only as a mechanism for determining the type of the overall data instance?
If the latter, there is no need to perform schema inspection for an XML document, as surely the media type would be indicating that. The only application I've found in my implementation for needing type inference is for the parameter style and explode features, which require an array or object to operate on and do not use media types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this suggesting that if contentMediaType is present at the top level of the schema that the entire data instance should be decoded with that media type before passing to validation?
No. I'll have to go back over this in detail to try to figure out how it seems to be implying that and improve it. I'm trying to cover several scenarios here and I probably muddled them together a bit.
Additionally, we could specify that schema keywords that require a specific type
I spent too much effort ripping a rule somewhat like that out of 3.x (type: array
required items
in 3.0) to be willing to do such a thing now (and I appreciate that you would rather not do that either). More seriously, the goal here is to strike a balance of allowing some authoring flexibility in arranging schemas without requiring implementors to do too much. Inferring types from other keywords, which can quickly get contradictory when dealing with dynamically typed structures, is definitely over the line into "too much" for me.
and instead required the user to be explicit with a
type
keyword
I don't want to outright require one in any specific place either. I'd like to make it clear where things definitely MUST work, and make it clear that there are a lot of things beyond that that won't be expected to work. But there might be unexpected configurations that will work fine in practice, or that specific tools support (perhaps already implemented when deciding how to handle the ambiguity in the first place).
Also, is this suggesting that data deeper within the instance (for example items in an array) need type inference as well, or is this intended only as a mechanism for determining the type of the overall data instance?
It's for whatever requirements in the spec need this kind of things. Correlating Encoding Objects with Schema Objects is probably the most obvious and complicated. And then once you've done that, figuring out the default contentType
. Other use cases are figuring out the destination type of ambiguous strings (e.g. query parameters).
If the latter, there is no need to perform schema inspection for an XML document, as surely the media type would be indicating that.
No this is the "where data is stored as strings without type information" use case, where it might be unclear how to parse <foo attr="true">42</foo>
. Are those the boolean true
and the number 42
or the strings "true"
and "42"
or some combination? That's distinct from the multipart
part use case or other embedded media type use case where you might need to figure out a media type that does not have a Media Type Object with parent key- see PR #4744 for this specific use case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@karenetheridge to make this part more clear:
But there might be unexpected configurations that will work fine in practice
For example, if you set the Encoding Object's contentType
explicitly, there's no need to locate any schema information to determine its default. But also I don't want complex rules around "if you don't set contentType
explicitly, your schema MUST look like...." Rather, if it is unclear whether tooling can detect the default behavior, authors should heed the limitations in the spec and decide to set contentType
explicitly themselves.
Similarly, when parsing query parameters and trying to figure out the type, maybe tools just try parsing them in that type order I give, on the grounds that the precedence is probably right. Basically treating it as an "any" type and guessing. As long as they validate it afterwards, that should be pretty reliable (I say "should" because somewhere there's some schema that could work out either of two ways and was intended to go the other way, but I can't figure out what that would look like off the top of my head).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want to outright require one in any specific place either. I'd like to make it clear where things definitely MUST work, and make it clear that there are a lot of things beyond that that won't be expected to work.
One scenario here where a type
keyword is required is for a header parameter -- an incoming value of "blue,black,brown" could be legitimately parsed as a string or an array, and we'd need to look at the schema to know which is expected. If we see "type":"array"
at the top level of the schema then we can confidently parse into the array ["blue", "black", "brown"]
and at least have a chance of validating against the schema as a whole (whereas leaving it as a string will definitely be invalid).
In my implementation I don't do any of the guessing that you describe (try parsing into one type, see if it's valid, then parse it a different way and try again). The schema gets one crack at trying to validate the data, and if it failed to specify what type was expected, too bad!
It's nice that this is getting into the spec in clear language. When I implemented parameter parsing I was following https://swagger.io/docs/specification/v3_0/serialization/ and tried to reverse-engineer what they were assuming, since these docs are all written from the client-side perspective (constructing an HTTP message from raw data) as opposed to the parsing and deserializing perspective.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The schema gets one crack at trying to validate the data, and if it failed to specify what type was expected, too bad!
There are too many scenarios where it's reasonable not to have a type
right there in the inline schema. If nothing else, when there is a $ref
. I wrestled a bit with how much to require, but following $ref
and allOf
seems straightforward enough. But I am open to other requirements. I have just seen plenty of implementations (including one I worked on - not publicly available sadly) that search those and roll up examples or flatten allOf
s to make the documentation rendering better. It seemed like a reasonable amount of searching.
I also considered allowing implementations to put some sort of step limit on it, like not following more than X number of $ref
s, but it's not like that gets any harder the more you do, and figuring out how to count "steps" with both $ref
and allOf
hurt my brain :-)
One scenario here where a type keyword is required is for a header parameter -- an incoming value of "blue,black,brown" could be legitimately parsed as a string or an array, and we'd need to look at the schema to know which is expected.
This is a really good point as "type": ["array", "string"]
would be a problem for that parameter. I think the rules I wrote would end up with it treated as an array, as string is always the type of last resort. But do we want that? Unlike the use cases I was thinking of, this truly could go either way, so we're picking a winner here based on rules I made up as I wrote them.
The one multi-value type
use case that I think we MUST (heh) require support for is adding "null"
to another type (e.g. "type": ["number", "null"]
). If we think the multi-type rules are too complex or too likely to have unexpected behavior, we could just special-case it for one non-"null"
type plus "null"
. I'm really uncertain as to what is best here and would love to hear more opinions.
@karenetheridge while I have your attention, do you think this is fine where it is or should it go under the Schema Object somewhere? I really could not decide. |
I'm putting this in draft because based on @karenetheridge's feedback I'm going to rework it fairly substantially, but it's still of use when understanding how it fits with the other related PRs. The effect of the rewrite should be the same, but I think the wording and organization will be significantly different. It's clear that the different use cases here need to be separated out and clarified. I think this ended up being a bit oddly abstract because of how I tried to split things up into PRs that don't conflict. |
Move things under the Schema Object, organize by use case and by the point in the process at which things occur, and link directly from more parts of the spec so that the parts in the Schema Object section can stay more focused.
I have added a commit that almost totally rewrites this- you probably just want to review the whole thing and not look at the per-commit diff as it will be a mess. The new version:
I do not think that has changed anything substantial, but it's essentially a new PR now. |
@karenetheridge I'm going to mark various threads as resolved since the text is now so different that they are confusing- please do not take that to mean I'm dismissing open questions, please just re-start whatever is needed with comments on the new text, or as new top-level comments. Apologies for the inconvenience. |
NOTE 1: This is intended to clarify requirements that already exist but have never been well-defined, both by making certain things required and stating clearly that other things are not. It is particularly relevant in light of the Encoding Object changes, although the vaguely-defined behavior predates the new features.
NOTE 2: I wasn't sure whether to put this here or under the Schema Object, or some other arrangment- suggestions welcome!
Some OAS features casually state that they depend on the type of data being examined, or implicitly carry ambiguity about how to determine how to parse the data.
This section attempts to provide some guidance and limits, requiring only that implementations follow the unambiguous, statically deterministic keywords
$ref
andallOf
.It also provides for just validating the data (when possible) and using the actual in-memory type when a schema is too complex to analyze statically.
One use of this is breaking apart schemas to use them with mixed binary and JSON-compatible data, and a new section has been added to address that.
Finally, a typo in a related section was fixed.