-
Notifications
You must be signed in to change notification settings - Fork 9.1k
updated to support alternate link URLs #797
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
@OAI/tdc please review for discussion |
links: | ||
UserRepositories: | ||
operationId: getRepositoriesByOwner | ||
url: $responseHeaders.link |
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 that should be $responseHeader.link
? (At least the table with the variable substitutions has it in singular.)
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, the examples in the table below have the variables in braces (i.e. {$responseHeader.link}
, here they are raw. I think we should always use the braces, or at least define it clearly when they can be omitted.
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, this is a minor doc change, we'll address that while starting a sweep of the docs next week
type: object | ||
``` | ||
|
||
will extract the URL from the response header named `link`. If an absolute URL is provided, it will be used instead of the server / path provided in the target operation. Parameters _may_ be provided as well. If a `path` parameter is provided, it shall be ignored unless the `url` value contains a matching location for substitution. If query parameters are provided in the `url` value, they will be interpreted per the operation definition, and the absence of a required query parameter will result in an invalid request. |
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 "absolute"/"relative" URL distinction happens after replacement of response payload variables, right?
Same for the path parameter values? (I.e. the response can include a URI template, which will then be filled using the parameters
settings with parameter values provided by the client.)
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 intent is that you have /foo/bar/{username}/baz
as a relative URL, we will use the document's host, scheme, and the supplied url of /foo/bar/{username}/baz
. If a parameter is extracted from the payload, it too would be used for the substitution
All in all, thanks, this is a big step to supporting HATEOAS. 👍 |
I just wrote a 1000 words about why I think this change is a really bad idea and moments before clicking the "comment" button I convinced myself I'm wrong. However, I still see two issues with this proposal:
This change significantly increases the scope of what we were trying to achieve with the original static description of relations between operations. That was my initial objection, but I do think I see a glimmer of light at the end of the tunnel that this might actually enable useful description of dynamic hypermedia APIs. If we are going to open the door to allowing dynamic characteristics I think we need acknowledge a couple of extra realities.
.... so much for, oh, I'll just go add a thumbs up to that PR.... :-) |
I'm trying to understand the need for this change, but am having a hard time following #783. I'm looking at @lcrabb's examples for I don't fully understand the need for a dynamic scheme/host/basepath when linking an operation. I understand there's no way to override it right now in the link definition, just can't understand the use case for it and would appreciate a sample (realistic) use case to fully grasp the need. Without it, it's hard for me to vote in a change that has no concrete need (and I'm not doubting there is one). Part of the concern is for regularly defined operations, we do not allow the scheme/host/basepath to be defined dynamically. You can override the top level definition, but at the operation level itself, this is fixed. We're ending up providing extra functionality here. This will not be the case if we end up supporting host and basepath templating, but then we won't really need this PR at all. Details-wise, the proposal itself looks good, I just have a few questions/clarifications.
The above example should also be included in some form in the spec itself - it explains the use in a clearer way than the proposed text only.
It sounds like it MUST be overridden.
How is a required query parameter defined? Or once it's in the URL it's required? I'm waiting with my vote on this, hoping to get some clarifications before. |
Looks like we will not be supporting this in 3.0 but consider for 3.x example: response header:
link: https://na2.foo.bar.com/users/tony/sensitiveData?token=abcd1234 target operation: server:
url: http://foo.bar.com/v2
paths:
/{user}/sensitiveData:
get:
server:
url: {scheme}://{host}/{basePath}
parameters:
- in: path
name: user
#... |
@darrelmiller I'd be interested in seeing your original 1000 word counter-argument. I'd also be interested in seeing some specific examples where such a thing would be useful. I am very much interested in seeing how we can support navigation through the data in the body. I'm less clear on the value of supporting navigation through headers that are detached from the data... that does not seem like "hateos"y to me. I could see header-born links being useful for certain meta-service references. |
@cfineman From an architectural perspective, whether the links are in the header or body it really doesn't make a difference. However, the whole notion of discovering resources via URLs in responses is counter to the idea of describing resources in a description document. It is possible there is an effective hybrid that can exist but I fear we don't have time to get it right for 3.0 so we're punting for the moment. |
@fehguy Based on your label edits, I've updated the "Milestone" on this to v3.Next so it's off the radar for OAI v3.0.0. If you think this should be on the radar for v3.0.0, please update the milestone. |
@darrelmiller Is this change being considered for 3.1? |
One reason that I think we have to reconsider this issue in V3.1 is to provide support linking a long running operation to it's status monitor resource. The status monitor is pointed to by the Location header and as there is no standardized definition for status monitor, being able to describe this would be really useful. I have seen some pretty ugly hacks to get around this problem. Being able to use the URL in a response to drive a linked operation would solve this problem. I definitely think this should be reconsidered for 3.1 |
Fixes #783
For links, I propose adding the
url
property to theLink Object
. When supplied, the url will be applied to the operation, effectively overriding the{scheme}://{host}/{basePath}
value.If the target operation defines any parameters, the following rules will be applied:
url
valueurl
value. Tooling will effectively extract these parameters to satisfy the requirements of parameter definitionsExample definition:
Example request:
Following the response link would honor the definition provided in the
getRepositoriesByOwner
operationId.Additional considerations:
When a
url
is provided, the single operation can target an alternate host and path combination. Subsequent links from that targeted operation will be considered against the host as defined in the document, not the overridden host. Thus if secondary links are required to reside on the alternate host, they must be supplied in the response from the alternate host.The
url
field may be a relative url, allowing the existing scheme/host/basePath scheme to be used.