-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Clarify interoperable parsing expectations (3.0.4 port of #3732) #3772
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
Conversation
I've asked a few people who were around for 2.0 and 3.0 to weigh in on the wording here, so even though this has two approvals, let's not merge it just yet please (but thank you to the approvers so 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.
Looks good. 👍
I had to read this three times before I finally understood it though.
@@ -133,10 +133,22 @@ In order to preserve the ability to round-trip between YAML and JSON formats, YA | |||
|
|||
### <a name="documentStructure"></a>OpenAPI Description Structure | |||
|
|||
An OpenAPI Description MAY be made up of a single document or be divided into multiple, connected parts at the discretion of the author. In the latter case, `$ref` fields MUST be used in the specification to reference those parts as follows from the [JSON Schema](https://json-schema.org) definitions. In a multi-document description, the document containing the [OpenAPI Object](#oasObject) is known as the **entry OpenAPI document.** | |||
An OpenAPI Description (OAD) MAY be made up of a single document or be divided into multiple, connected parts at the discretion of the author. In the latter case, [Reference Object](#referenceObject) and [Path Item Object](#pathItemObject) `$ref` keywords, as well as the [Link Object](#linkObject) `operationRef` keyword, are used. In a multi-document description, the document containing the [OpenAPI Object](#oasObject) is known as the **entry OpenAPI document.** |
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.
be divided into multiple, connected parts (called a "multi-document description")
(to unambiguously define a term that is used later)
When parsing an OAD, JSON or YAML objects are parsed into specific Objects (such as [Operation Objects](#operationObject), [Response Objects](#responseObject), [Reference Objects](#referenceObject), etc.) based on the parsing context. Depending on how references are arranged, a given JSON or YAML object can be parsed in multiple different contexts: | ||
|
||
* As a full OpenAPI Description document (an [OpenAPI Object](#oasObject) taking up an entire document) | ||
* As the Object type implied by its parent Object within the document |
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.
does item 2 allow for a $ref
to refer to a JSON schema document (or a schema within a JSON schema, such as to schema a
in a schema bundle containing a schema named a
and perhaps other schemas that a
references and perhaps other unrelated schemas)?
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.
JSON Schema bundles aren't supported in 3.0, because id
/$id
is not supported (nor are any other keywords that would require scanning a whole document - see #3758 for how that is handled in 3.1).
These bullet points are not intended to allow anything as much as they are to describe what implementations already do. Otherwise I'd have probably thrown in a MAY.
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.
@DavidBiesack let me know if I did not answer your question fully - I'm not sure if you were just talking about formal bundling or something else.
I have converted this to a draft just to make sure it does not get merged as (despite having several TSC approvals) there is still discussion going on. |
versions/3.0.4.md
Outdated
* As the Object type implied by its parent Object within the document | ||
* As a reference target, with the Object type matching the reference source's context | ||
|
||
In version 2.0 of this specification, this section specified that the OAD is a "single file", which (together with the treatment of `$ref` as a URL resolved separately each time it is encountered), resulted in the common interpretation that if the same object is parsed multiple times in contexts that require it to be parsed as _different_ Object types, then as long as the reference target is valid for each Object type, then there is no error. An example would be parsing an empty JSON object as a reference target twice: Once as a Path Item Object and once as a Schema Object. Since both Object types allow empty objects, this is syntactically valid for both Objects. |
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.
Extra space
In version 2.0 of this specification, this section specified that the OAD is a "single file", which (together with the treatment of `$ref` as a URL resolved separately each time it is encountered), resulted in the common interpretation that if the same object is parsed multiple times in contexts that require it to be parsed as _different_ Object types, then as long as the reference target is valid for each Object type, then there is no error. An example would be parsing an empty JSON object as a reference target twice: Once as a Path Item Object and once as a Schema Object. Since both Object types allow empty objects, this is syntactically valid for both Objects. | |
In version 2.0 of this specification, this section specified that the OAD is a "single file", which (together with the treatment of `$ref` as a URL resolved separately each time it is encountered), resulted in the common interpretation that if the same object is parsed multiple times in contexts that require it to be parsed as _different_ Object types, then as long as the reference target is valid for each Object type, then there is no error. An example would be parsing an empty JSON object as a reference target twice: Once as a Path Item Object and once as a Schema Object. Since both Object types allow empty objects, this is syntactically valid for both Objects. |
@RomanHotsiy I'm not sure I know of specific misinterpretations, more like assumptions from those coming from JSON Schema and adding OpenAPI support. They (we) tend to assume referencing works like it's intended to in 3.1, because there's nothing obvious that says otherwise in 3.0. @ralfhandl 's reading of the JSON Reference spec is valid, but that'a level of textual analysis most people don't do. I'm probably being overly paranoid from some past debates. If no one else shows up with more context in tomorrow morning's call, I'll probably incorporate the feedback, take a shot at better wording, and move ahead. |
As discovered through the OASComply project, certain referencing scenarios are ambiguous, with different authorities holding contradictory interpretations regarding whether and how they are to be supported. As a result, it is impossible to define compliance, as all of the interpretations can be argued to be "correct" in some sense. This change excludes some particularly challenging scenarios from compliance testing by making their behavior explicitly implementation-defined. This has several benefits: * No current implementation is rendered non-compliant * No currently usable OAD is rendered invalid * New implementers need not put effort into handling these scenarios * User expectations are set to _not_ expect consistent behavior * Linters can write a rule to match these expectations * Everyone is guided towards straightforwad best practices Includes substantially better wording from ralfhandl from review comments for the 3.1.1 version of this change. Co-authored-by: Ralf Handl <[email protected]>
Based on today's TDC call, the intention regarding
Because of this, the 3.1.1 wording was deemed appropriate, with one change (the 3.0 Schema Object does not have its own |
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 new text is slightly too restrictive, can't do that in a patch version.
Which text do you mean? If it's the text I did not change, then it's already in and that is not a reason to block this PR (although it is a good reason to open a new PR to address the separate problem). |
@ralfhandl I have filed PR #3820 (replacing 3819 which had errors and a misnamed branch) for the "entry document" concern. Let's please get this PR merged and I will resolve conflicts in that one and we can finish that discussion separately. |
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.
This is a direct port of what we already agreed for 3.1, I'm happy with it but @ralfhandl it would be good to have you circle back to your "request changes" review and see if we're ready to move forward or not.
[EDIT: After much discussion, based on today's TDC call this PR is now a direct port of #3732, as the intended model had shifted from 2.0's "copy the JSON/YAML and then apply OAS Semantics" to "the reference target has OAS Semantics, but the implications of this were not entirely worked out."]
This is sort-of a port of #3732, but modified because the scenario involved was mostly assumed to be valid based on the more direct text of OAS 2.0. So instead of making the scenario implementation-defined, I opted to acknowledge that it's the common assumption (there is probably better wording for this, please feel free to suggest - I fear that my current wording is too skeptical or judgemental), but also that the text does not cleary require it in 3.0. And then recommend against doing it because it is not required to be supported in 3.1. See the commit message below for more details of how this is different from the 3.1.1 PR.
As discovered through the OASComply project, certain referencing scenarios are ambiguous, with different authorities holding contradictory interpretations regarding whether and how they are to be supported.
This particular scenario, where the same JSON/YAML object is parsed in different semantic contexts, is well-defined if the requirements of 2.0 are assumed to apply despite no longer appearing in the text. However, the scenario is not well-defined in 3.1, so we should advise against it.
While the 3.1 version of this change made the scenario explicitly implementation-defined, it seems better to acknowledge the assumption here while still noting that it is not clearly required.
This also removes the confusing reference to JSON Schema for "$ref" as the Reference Object explicitly states that it is governed by JSON Reference and not JSON Schema. It also enumerates more of the referencing keywords like we do in 3.1.