OAI / OpenAPI-Specification

The OpenAPI Specification Repository
https://openapis.org
Apache License 2.0
28.82k stars 9.07k forks source link

style: form + object looses the parameter name? #1006

Closed ePaul closed 4 months ago

ePaul commented 7 years ago

Hi, while preparing a presentation of the new features of OpenAPI 3.0, I stumbled over this line in the parameter style examples table:

style explode ... object
form true ... R=100&G=200&B=150

So if I have a parameter color with an object { "R": 100, "G": 200, "B": 150 } to be passed as a query parameter (which has default style form, and default explode value true), I get a query of ?R=100&G=200&B=150 – no trace of the parameter name color in here anymore.

If I have two such parameters (e.g. "foregroundColor": { "R": 100, "G": 50, "B": 150 } and "backgroundColor": { "R": 200, "G": 250, "B": 200 }), they would passed as ?R=100&G=50&B=150&R=200&G=250&B=200, from which no server could reliably derive the actual parameters.

Is it really meant to be that way? I guess this is how RFC 6570 defines "form style query expansion", but it will work only if there is only one of those parameters with object schema at all, or if they have non-overlapping property name sets (though having to implement that seems ugly).

webron commented 7 years ago

Good catch. The answer, to me, seems to be 'yes'. Let's face it, there's no elegant way to solve this. We're trying to mix and match cases here, and some may not make sense. How would you solve that?

Like you said, the RFC defines the behavior just like that, so let's leave it as it is. Tooling will have to deal with it, and I'd suggest just clobbering values. It's really up to the user to define things that make sense, within the defined behavior.

Authoring tools might be able to provide warnings to users in such a case.

ePaul commented 7 years ago

My original idea to solve this was to make deepObject the default when the parameter is an object, but defining this seems to be even more messy.

By the way, is there a reason we couldn't have deepObject work for arrays too?color[0]=blue&color[1]=black&color[2]=brown seems a fine way to serialize that, and would work even for nested things.

darrelmiller commented 7 years ago

@ePaul Supporting arrays as you describe was my intent. I was supposed to find some canonical implementation to use as a guideline for the behavior, but didn't get around to it.

webron commented 7 years ago

Not sure the name deepObject would work for that though. Ironically, the name matrix could be have been useful for both. :wink:

If we end up supporting the exploded array notation, it needs to be clear that the first index is 0 (or 1, or -1, or whatever).

kduis commented 6 years ago

In 2.0, I could individually enter "in:filter" parameters, and they'd show up as "...&filter=param1:val1,param2:val2". How do you do the same in OpenAPI 3?

vvanpo commented 5 years ago

Good catch. The answer, to me, seems to be 'yes'. Let's face it, there's no elegant way to solve this. We're trying to mix and match cases here, and some may not make sense. How would you solve that?

Like you said, the RFC defines the behavior just like that, so let's leave it as it is. Tooling will have to deal with it, and I'd suggest just clobbering values. It's really up to the user to define things that make sense, within the defined behavior.

Authoring tools might be able to provide warnings to users in such a case.

The solution would be to not allow mixing and matching of styles. URI templates were never intended to be used this way. Having to build out a URL with separate templates for each query parameter based on its style and explode field (and forget about using a form-style template combined with allowReserved) completely negates any of the benefits of using templates in the first place.

I don't believe it should be up to the user. The user shouldn't have to understand the minutiae of the particular edge-cases when their query argument will and won't work. The standard should be such that a user can safely assume their serialized parameter will be parsed as intended by the receiving service. Any less than that is a failure of the standard, in my opinion.

darrelmiller commented 5 years ago

@vvanpo We had a few choices, we could invent our own rules for parameter serialization into a URL, or we could adopt an existing standard that has existing tooling. We chose the later because it has proven to be an effective standard for describing URLs and it is quite feasible to construct a standard URI Template from the OpenAPI description and then use standard tooling to resolve the template.

It just so happens that RFC6570 doesn't support every scenario. URI Templates define serialization rules. We just happen to have a different syntax for defining URI Templates. I don't see why they were not intended to be used this way.

Assuming the API provider has designed a URL that can be represented in a URI Template, and created an accurate description of the parameters, the consumer of the API should have no concern about the details of the URL construction. They should simply be able to pass the parameter value and the URL will be constructed as intended by the API Provider. There is a burden on the API provider to understand the limitations of URI Templates, but I don't see how inventing a new URL construction mechanism is going to solve that. I certainly don't have the confidence to think I could do a better job than the authors of RFC6570.

vvanpo commented 5 years ago

@darrelmiller I can appreciate wanting to lean on existing standards and tooling, I would have approached it the same way. But precisely because we can't rely on RFC6570 to solve for every scenario that we would like to support, we also cannot use existing tooling for every case. That might not be a big problem if you're manually writing a client against a particular specification, but becomes a big issue if you're trying to write a generic client that can parse any spec and dynamically generate serializers. Perhaps the most popular example of such a client would be swagger-js, and that project does not use any URI template tooling from what I can tell.

So if you're trying to solve for the general case, what you end up doing is looping over every argument and either using a single-argument URI template (which might now require post-processing, like to remove the ? from an {?arg}-style template) or some custom serialization function for cases URI templates don't support, and finally concatenating everything with the correct separators after all serialization is complete. You can't build a single template string and run a URI template function over it, because the parameters that aren't supported by RFC6570 will be double-encoded, etc. This is what I meant when I said URI templates weren't intended to be used this way. URI templates are great if you can build a single template string out of all your parameter definitions, but if you're forced to split it up for many of your parameters you've lost all the simplicity that URI templates otherwise provides.

To me, the benefit of a standard like OpenAPI is not just for consistent API documentation, but also to allow for an ecosystem of tools around it. If serialization rules are ambiguous, or require manual care to ensure definitions are consistent (which is what you allude to when you say the burden is on the provider to understand RFC6570's limitations), then you can no longer build effective tools to apply said definitions.

vvanpo commented 5 years ago

Further affirming my point is that you can't (or at least shouldn't) use URI templates for header or cookie parameters. RFC 6570 expects to only be used for URIs and therefore specifies percent-encoding for simple and form-style templates, but headers are not URIs and shouldn't be encoded that way. Nor should cookies be exploded using an ampersand delimiter, because RFC6265 requires a semi-colon. And if you combine the original post's issue about parameter name erasure with the default for cookies being explode: true, it's clear that URI templates are not a reasonable serialization strategy for cookies.

So if you take out headers, cookies, deepObject, allowReserved, pipeDelimited, spaceDelimited, and explode: true form objects, you now have very few parameter definitions left that URI template tooling is actually appropriate for.

handrews commented 4 months ago

I'm deeming @webron's comment:

Like you said, the RFC defines the behavior just like that, so let's leave it as it is. Tooling will have to deal with it, and I'd suggest just clobbering values. It's really up to the user to define things that make sense, within the defined behavior.

to be the resolution of this issue's actual question. It's not clear to me that any of the subsequent discussion calls for action. If I missed something, please comment, or better yet file a new issue with a clear request.