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.
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
Fix description vs document terminology #4100
base: v3.0.4-dev
Are you sure you want to change the base?
Fix description vs document terminology #4100
Changes from 3 commits
e46084e
ce21969
dfacec5
abddcf0
d447096
c06c4fa
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 another one where it's about the document. "parent Object within the OAD" is ambiguous- do you mean the literal parent in the JSON/YAML structure or the logical parent which might be a reference depending on how you reached it? This section was intended to clarify that distinction and the problems it can cause.
Here we are talking about the JSON/YAML structure parent.
While the presence of the next bullet point implies that this one has to be about something other than referencing, I would prefer to emphasize the document-oriented nature of the parent/child context.
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'm in favour of "OpenAPI Description" here
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 still "OpenAPI Document" for the same reasons I stated previously- it is literally about the parent-child JSON/YAML relationship within the Document, and never about any other possible relationship within the Description. That is the entire purpose of this statement, and without that, the distinction between this bullet point and the next doesn't make any sense.
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 example we discussed in the TDC meeting yesterday that I think swayed the group to conclude that "Description" is correct here is this:
entryDoc.json
otherDoc.json
In this example, the parent object of "param1" in otherDoc.json is just a Json object -- it implies nothing about how to interpret param1. Rather, the interpretation of "param1" comes from it being referenced within the parameters array of entryDoc.json -- it's parent object in the OpenAPI Description.
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.
@mikekistler There are three bullet points here, which I will list with numbers even though they are not numbered in the text:
Your example is point 3. Point 2 is also important - it's how parsing works before you resolve references. All three points are critical to properly handling OADs.
If the reference crosses to a different document, then, depending on how you wrote your parser (e.g. how Swagger's 3.0 parsing works vs how @darrelmiller says Microsoft's 3.0 parsing works), you can parse the same JSON/YAML object by both context 2 (which Microsoft uses even with 3.0 according to Darrel) and context 3 (which all implementations must use for proper reference resolution).
That is why the following paragraph exists:
Please stop trying to change context 2 to be the same as context 3. The entire point of the "Structural Interoperability" section is to point out that different implementations use these contexts and they don't always say the same thing.
Your change completely guts this section. Your example is about context 3. But context 2 is also critically important and cannot be removed. You literally cannot parse a document without it. How do you know that the child of the
info
field is an Info Object? Context 2 is what tells you that. There is no context 3 for that field.Whenever someone writes a
$ref
to something that is also part of a well-formed OpenAPI Document, context 2 and 3 both apply and they can be different.Your example only uses context 3. But that does not invalidate context 2.
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.
Hmmm. Okay ... I see your point about the overlap between 2 and 3.
But I still think there is a problem with using "OpenAPI Document" in bullet 2, and I think it is illustrated by the same example as given above.
What is the context for the value of the "schema" field within otherDoc.json? If bullet 2 is about "OpenAPI Document", then it doesn't apply because otherDoc.json is not an "OpenAPI Document" (by the definition we have now added), and bullet 3 also does not apply since this is not a reference target.
If we make bullet 2 be only about OpenAPI documents, then we'll need to add another bullet for documents that are not OpenAPI documents.
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.
Due to the way the document flow was broken by the comment I had missed the third bullet point. This contributed to me misunderstanding what the second point was trying to say.
In the tooling my team builds, when parsing an OpenAPI document that has been referenced, we don't use the context of the reference (context 3) to determine the type of the thing beingback to the probele referenced. If someone attempts to reference a schema in a location that expects a parameter, we give a type mismatch error. We don't attempt to parse the schema as if it was a parameter. We would only do that when pointing to a document that is not an OpenAPI Document.
But as Henry states, this is implementation dependent. Tooling that is building an OpenAPI document editing experience would always use the structural context (context 2) to determine the type of something. Whereas a a tool that is just consuming an OpenAPI document could easily choose to use context 3 for determining the type of a reference target.
Going back to the debated terminology, the phrase "OpenAPI Description" is problematic because you could argue that the parent object in context 2 is not actually part of the OpenAPI Description. However, if you say "OpenAPI Document" you a) know explicitly which parent you are talking about and b) know that tooling can parse the OpenAPI document and know the type of the parent object.
So, apologies to everyone involved but I am going to reverse my opinion from what I said on Thursday and say this needs to say 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.
Okay, I can see the argument for this. But then which of bullet do you say explains the context used for parsing/interpreting the value of "schema" in otherDoc.json?
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 think this might also be document. It doesn't matter for Server Objects under the OpenAPI Object's
servers
field, and if there are no such Server Objects, then we're basically "pretending" that there is one with aurl
based on the entry document's location.But this gets weird when you have a Path Item Object in a referenced document that defines its own
servers
, or that has Operation Objects that define their ownservers
(or a Link Object that definesserver
). I think those ought to be relevant to the location of the document in which they appear.In "Resolving Implicit Connections", I hand-waved the Servers problem on the grounds that only the entry document's Paths Object is relevant. But with a shared Path Item Object under the Components Object of a components-only "syntactically complete" document, do you look up the default by current document or by entry document? That might actually need clarification.
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.
It would seem logical to me then the lookup happens on the entry 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.
This ought to follow RFC3986 which would go:
Do we state anywhere how to locate Server Objects from referenced (non-entry) documents that are OpenAPI Documents and therefore might have an OpenAPI Object (and
servers
field) in the document root that is not the OAD root (which is always the entry 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 this mean that
operationRef
can only point to a resource/document that is already part of the OAD due to the action of another keyword? This is relevant to the text early in the spec that states what keywords are allowed to connect parts into an OAD. (but see next comment in the Reference Object)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 read it that way. Though I do find the terminology "A relative or absolute URI reference" a bit strange. I think that URI references are either relative refs or URIs, and URIs may be absolute or not, but I don't think "an absolute URI reference" is a thing. This is the only place this phrase occurs in the spec. I think "URI reference" is clearer and more accurate, so I'll make that change. Also open to additional changes if you think they are needed.
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.
"URI reference" in RFC3986 is the most generic term. Anything vaguely URI-ish (relative, absolute, full) is technically A URI reference (often written "URI-reference").
But you are right that this needs to be changed for a different reason:
https://example.com/foo#/components/pathItems/bar/get
is a URI-reference that is neither relative (because it has a scheme) nor absolute (because it has a fragment).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 suggests that "in the OAD" means "in the OAD, including documents that are only part of the OAD because of this Reference Object", which would mean that
operationRef
above, with similar wording, can also pull in otherwise-unconnected documents. Is this how this wording is intended?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'm not sure what you mean by "otherwise-unconnected documents" -- I think $ref is a thing that connects documents. But perhaps to directly answer your question -- I think it is intended for $ref to be able to reference any document, whether or not it was somehow "connected" through some other means.
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.
Yes,
$ref
has to be able to do that. I'm pointing out that I find that the language suggests to me that the document has to somehow already be part of the OAD, but the only way we define to connect things is$ref
/operationRef
/mapping
. For$ref
it's (to us) self-evident that there's no other way, but foroperationRef
andmapping
this wording could be read to say that you can only pointoperationRef
andmapping
to a document that has already been$ref
'd. So this comment is really more aboutoperationRef
andmapping
.