Skip to content
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

Note relative URL-reference resolution ambiguity #4130

Merged
merged 2 commits into from
Oct 14, 2024

Conversation

handrews
Copy link
Member

This explicitly lists out the cases and notes which ones were (as far as we can recall) intended to behave like $ref, but could be reasonably read to behave like API URLs.

See:

This explicitly lists out the cases and notes which ones were
(as far as we can recall) intended to behave like `$ref`, but
could be reasonably read to behave like API URLs.
@handrews handrews added the re-use: ref/id resolution how $ref, operationId, or anything else is resolved label Oct 10, 2024
@handrews handrews added this to the v3.0.4 milestone Oct 10, 2024
@handrews handrews requested a review from a team as a code owner October 10, 2024 19:17
ralfhandl
ralfhandl previously approved these changes Oct 10, 2024
mikekistler
mikekistler previously approved these changes Oct 10, 2024
Copy link
Contributor

@mikekistler mikekistler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 👍

I think the wording as is conveys the key information, but I left an alternate wording that I think might read better in the spec. I'll leave it up to you if you prefer the original text.

Comment on lines 240 to 243
The wording above is intended to distinguish between API URLs (which use the Server Object to determine the base URI or the prefix for the URL path template), and OAD URLs (which use the current document's base URI per RFC3986).
However, only `$ref` was mentioned explicitly in versions 3.0.0 through 3.0.3 of this specification, leaving the behavior of the following fields ambiguous: the `operationRef` field of the [Link Object](#link-object), the URI form of the `mapping` field of the [Discriminator Object](#discriminator-object), the `externalValue` field of the [Example Object](#example-object), and the `url` fields of the [External Documentation](#external-documentation-object), [Contact](#contact-object), and [License](#license-object) Objects.
Therefore, which of the above two rules is used for each of these fields is _implementation-defined_, although it is RECOMMENDED to use the `$ref` process for all of them for compatibility with future versions of this specification.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this slightly more concise wording:

Suggested change
The wording above is intended to distinguish between API URLs (which use the Server Object to determine the base URI or the prefix for the URL path template), and OAD URLs (which use the current document's base URI per RFC3986).
However, only `$ref` was mentioned explicitly in versions 3.0.0 through 3.0.3 of this specification, leaving the behavior of the following fields ambiguous: the `operationRef` field of the [Link Object](#link-object), the URI form of the `mapping` field of the [Discriminator Object](#discriminator-object), the `externalValue` field of the [Example Object](#example-object), and the `url` fields of the [External Documentation](#external-documentation-object), [Contact](#contact-object), and [License](#license-object) Objects.
Therefore, which of the above two rules is used for each of these fields is _implementation-defined_, although it is RECOMMENDED to use the `$ref` process for all of them for compatibility with future versions of this specification.
Resolution of relative references in the `operationRef` field of the [Link Object](#link-object), the URI form of the `mapping` field of the [Discriminator Object](#discriminator-object), the `externalValue` field of the [Example Object](#example-object), and the `url` fields of the [External Documentation](#external-documentation-object), [Contact](#contact-object), and [License](#license-object) Objects is _implementation-defined_. For compatibility with future versions of this specification, It is RECOMMENDED to use the same process as $ref, but MAY instead use the URLs defined in the [Server Object](#server-object) as a Base URI, as either interpretation was possible in previous versions of this specification.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikekistler I like most of the wording change but want to tweak it a bit further- will update shortly.

@ralfhandl ralfhandl self-requested a review October 11, 2024 08:02
@ralfhandl ralfhandl dismissed stale reviews from mikekistler and themself October 11, 2024 08:03

Waiting for @handrews' improvements

Co-authored-by: Mike Kistler <mikekistler@microsoft.com>
@handrews
Copy link
Member Author

@mikekistler see how you like this latest change. I kept most of your deletions, but decided to entirely drop mention of previous drafts, and kept wording that highlights two important points:

  • It is only implementation-defined which of the two alternatives is used; it is not implementation-defined without constraint
  • The each / all wording indicates that tools don't have to choose the same process for every keyword in the list. For example, @darrelmiller (in our conversation after the TDC call) drew the line in a different place, which would put some in the list using Server Objects and some using the $ref process, and we don't want to invalidate that either

@handrews handrews requested a review from a team October 12, 2024 14:50
Copy link
Contributor

@mikekistler mikekistler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 👍

@handrews
Copy link
Member Author

Two TSC approvals, so I'm hitting the merge button.

@handrews handrews merged commit c86f197 into OAI:v3.0.4-dev Oct 14, 2024
2 checks passed
@handrews handrews deleted the resolve-304 branch October 14, 2024 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
re-use: ref/id resolution how $ref, operationId, or anything else is resolved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants