-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
Support deep objects for query parameters with deepObject style #1706
Comments
Are there any other examples of these nested deepObjects outside of Express? The more widespread a pattern, the more likely it is to be considered. Myself, I have an aversion to passing such a complicated object in the query string. Any insight into why Express even landed on this pattern? |
We were a bit stuck when allowing the If we can get some confidence that the |
What about the URL max length limits? I think one of the reasons why a standard for object serialization in the URL is hard to materialize is because of the URL max length problem, the URL just wasn't intended to pass data in this way. Depending on the browser and/or server software being used the URL max length varies but in general it is small when compared to how much data can be transmitted with other methods like POST or PUT. It certainly would work for small objects but people tend to inadvertently abuse these kinds of APIs by passing bigger than allowed payloads. |
@rmunix regarding URL max length problems, I was very happy to see the HTTP SEARCH method draft RFC revived last month: https://tools.ietf.org/html/draft-snell-search-method-01 |
http://esbenp.github.io/2016/04/15/modern-rest-api-laravel-part-2/ I use a modified version of https://github.com/esbenp/bruno referenced in the article above in a few apis, it's extremely useful for including related data and search filtering. I'm not really sure how to define those in a spec. I do appreciate that the url strings could get silly long to the point of failing if abused, but without this sort of thing searches and such would have to be actioned as POST requests or limited to basic GET request params. It seems to me no ones really come up with a holy grail API solution for complex search queries so it's a bit of a free for all at the moment. |
Ruby on Rails Seems to use the same approach. Reference: https://edgeapi.rubyonrails.org/classes/Hash.html#method-i-to_query Example code: require "activesupport"
data = {
"name" => "David",
"nationality" => "Danish",
"address" => {
"street" => "12 High Street",
"city" => "London",
},
"location" => [10, 20],
}
print data.to_query("person") Produces the following query string, I have urldecoded and reformatted it for better readability:
Notice that array items are using an empty index, i.e. |
Python 2.7 AFAICT, Python does not support nested objects in query parameters. Example code: from urllib import urlencode
from urlparse import parse_qs
data = {
'person': {
'name': 'David',
'nationality': 'Danish',
'address': {
'street': '12 High Street',
'city': 'London',
},
'location': [10, 20],
}
}
print urlencode(data) Produces the following query string, I have urldecoded and reformatted it for better readability:
Interestingly enough, the roundtrip does not preserve the original data. print parse_qs(urlencode(data)) Outcome:
Another example: print parse_qs('foo[bar]=1&foo[baz]=2')
# {'foo[baz]': ['2'], 'foo[bar]': ['1']} |
Not API specific, but jQuery can generate nested array url params from objects. |
JSONAPI sparse fieldsets require this: https://jsonapi.org/format/#fetching-sparse-fieldsets |
Hey @earth2marsh and @darrelmiller ... We (Directus team) have been trying to use OpenAPI 3.0 for a while now, but the lack of support for nested deepObjects has kept us from using this spec. We have a dynamic API that allows for relatively complex filtering, for example:
Our API Reference for this filtering Is there any hope for this being supported in the future or should we "move on"? |
@bajtos: On October 11, 2018 you wrote "I'll try to find some time to fix swagger-js in the next few weeks." What is your status on this? |
Eh, I didn't even started 😢 Feel free to contribute the fix yourself. |
@benhaynes Sorry if this comes across as snarky, it's not intended to, it's just I'm in a rush and I don't know how to ask this in a friendly/sincere way. What would you like us to do? Pick a winner from the many options? Design our own format? If we pick a format that is incompatible with what you currently use, would you switch? Should we find a way of supporting many different formats? |
Hey @darrelmiller — not snarky at all, I sincerely appreciate the response as it maintains some momentum in the discussion! We're certainly not trying to force the spec to follow our format, and understand your position of not wanting to blaze ahead without a standard to follow. To answer your question honestly, if the option you choose is incompatible with our method, then we wouldn't be able to use OpenAPI since we can't introduce a breaking change into our platform's filtering. Still, we'd support your team's decision if they think a different direction is a better solution. I'm not sure how "extensible" your spec/codebase is, but support for multiple (even optional) serialization formats seems the most promising. Perhaps leaving these as unofficial until a "winner" eventually surfaces. In our experience, industry-leading frameworks offering solutions is the most efficient way for a de facto standard to emerge. Our proposal is to support a deeply nested structure, such as: Thanks again. I'd love to hear your (or anyone else's) thoughts on this approach! |
Hey @darrelmiller, I would like to understand the OpenAPI position on this. Is the reason to not support the outcome Also, in the technical side, will this bring a complex/breaking change and it requires much more time? In term of where else this format is supported I would like to add another example. PHP Example: <?php
$data = [
'person' => [
'name' => 'David',
'nationality' => 'Danish',
'address' => [
'street' => '12 High Street',
'city' => 'London',
],
'location' => [10,20],
]
];
echo http_build_query($data); Result:
The result is urldecoded. Also PHP automatically parse these parameters into an array. Passing the result above into query string will result in the original array. |
I think I'm running into this same issue. I have a simple C# type that needs two integers. I can define an API endpoint like this just fine: and it works but if I use my binding model type with those same two integers as properties: then my /swagger endpoint wants to generate JSON+Patch stuff and has no idea how to generate a URL with the appropriate querystring values to bind to that model. Will complex / deep object support help me in this (very simple) case? If not is there a known good way to support it currently? |
Arrrgh, hit that wall, too.... great example of a not so obvious limitation that might kill your whole project dev setup and workflow. |
Ran into the same problem (using Rails), also with a parameter allowing for dynamic filtering. The funny thing is, that swagger-editor does generate deeper nested objects as placeholder text for the parameters text area when clicking "try it out":
results in
which will be silently ignored. I know that this is actually a bug in swagger-editor, but it shows that it would be more consistent to allow deeper nested objects. |
What needs to be done to move on with this @earth2marsh? You asked for a couple examples outside of Express, which I think have been provided in the messages above. I can help out writing some of the needed documents for this in a PR if that helps. |
This gem allows us to generate an [OpenAPI schema](https://www.openapis.org/) of an [Apia API](https://github.com/krystal/apia). ## Why are we using v3.0.0 when the latest is v.3.1.0 ? The [OpenAPI generator](https://openapi-generator.tech) does not support 3.1.0 (at least for Ruby yet). So the specification is for version 3.0.0. Annoyingly in v3.0.0, having a request body against a DELETE is deemed to be an error. And this shows up in [swagger-editor](https://editor.swagger.io/). However, after [community pressure](OAI/OpenAPI-Specification#1801), this decision was reversed and in [version 3.1.0 DELETE requests are now allowed to have a request body](OAI/OpenAPI-Specification#1937). I have successfully used the Ruby client library to use a DELETE request with a v3.0.0 schema, so I don't think it's a big deal. We can bump to 3.1.0 when the tooling is ready. ## What is implemented? - All endpoints are described by the spec. - ArgumentSet lookups with multiple methods of supplying params are handled - All the various "non-standard" Apia data types are mapped to OpenAPI ones (e.g. decimal, unix) - If `include` is declared on a field for partial object properties, then the endpoint response will accurately reflect that - Array params for get requests work in the "rails way". e.g. `user_ids[]=1,user_ids[]=2` - [swagger-editor](https://editor.swagger.io/) works, so we can use the "try it out" feature (including bearer auth) - Routes that exclude themselves from the Apia schema are excluded from the OpenAPI output - Endpoints are converted into "nice" names so that the generated client code is more pleasant to use - Apia types (enums, objects, argument sets, polymorphs) are implemented as re-usable component schemas - The spec is good enough to generate [client libraries in various programming languages](https://github.com/krystal/katapult-openapi-clients) ## What isn't implemented? - Only the "happy path" response is included in the spec, we need to add error responses - There are places in the spec where we can explicitly mark things as "required" and this has not been implemented everywhere. - Perhaps we can improve how ArgumentSet lookups are declared – currently [swagger-editor](https://editor.swagger.io/) allows both params (e.g. id and permalink) to be sent in the request which triggers an error. - We can improve the accuracy of the [data types](https://swagger.io/docs/specification/data-models/data-types/#numbers) by declaring the `format`. This is not implemented. - There's one specification test that simply asserts against a static json file generated from the example app. Perhaps we could try actually validating it with something like https://github.com/kevindew/openapi3_parser - Might be nice to dynamically determine the API version - The example app needs expanding to ensure all code-paths are triggered in the generation of the schema ## Any other known issues? - We can't have deeply nested objects in GET request query params. This just isn't defined by the OpenAPI spec. [There's GitHub issue about it](OAI/OpenAPI-Specification#1706). I don't believe we can do much here and probably we don't need to. - File uploads are not implemented, but I don't think we have a need for that. - We do not try to be too 'clever' when the endpoint field uses include to customize the properties returned. e.g. `include: '*,target[id,name]'` in this case we could actually use a `$ref` for the object referenced by the `*`. But instead, if a field uses `include` we just declare an inline schema for that field for that endpoint. - The example API has been copied and expanded from the apia repo. Some of the additional arguments and ways the objects have been expanded is nonsense, but they're there just to ensure we execute all the code paths in the schema generation. Maybe we could come up with a better example API or perhaps we just don't worry about it.
I think the simplest way to allow folks to experiment with different approaches would be to implement #1502 (Support for arbitrary query strings). That would let folks define entirely custom serialization, perhaps based on media types as @darrelmiller suggests, without having to work around the spec's mechanisms for supporting more common serialization forms. |
https://github.com/abbasudo/laravel-purity This is extremely necessary! I can't convert to format query parameter filters[is_publish][$contains]=true Terrible. An outrageous flaw. |
@POMXARK let's keep it respectful, the maintainers don't owe you anything. You're welcome to submit a PR or make and share a third-party package if this is so important to you, we're all just doing our best here. FWIW I'm not a JS developer but I would still strongly vote for standardizing around "qs style" as it's very intuitive and close enough to other serialization methods to work with in different languages. It should be recognized though that there is really no way to do nested query objects in a GET query string that will be straightforward to document. |
@dafeder thanks for your comment!
Yeah, this is why I'm pushing for #1502 in version 3.2 of the spec. It's not ideal, but it gets the OpenAPI spec details out of the way and lets folks do their own serialization for the entire query string, including by just setting the whole thing to |
@handrews yes that makes a lot of sense, the problem is really more on the SwaggerUI side. As a spec I can see this being the right way to do it (and clarifying this will probably help Swagger handle it better). |
Any updates on this? |
@DustinCai see my comment above about 3.2. We're getting 3.0.4 and 3.1.1 out the door right now, then 3.2 should be a relatively quick release as we want to keep it small (and then do a 3.3 if 3.2 is well-received). I'd hoped to get 3.2 done by the end of the year, but 3.0.4 and 3.1.1 have taken a bit longer so maybe January or (hopefully) at worst February. I know that solution (#1502) might not be what you're looking for, but there are too many possible different formats and too many competing demands. Allowing people to define their own query string formats (media types) for the whole query string will unblock experimentation, particularly if combined with a media type registry per #3771. |
EDIT: no need to read, I was making an implementation mistake. Is there actually a situation where someone wants the query parameter specified as a deepObject to be keys with url encoded values that are themselves JSON? I feel like I'm misunderstanding the issue, because I've never heard of that as being something that anyone actually wants. The other flaw with that is that it's ambiguous. For instance, our filters support If the default was I think I must be understanding because I can't really conceive of how thousands of other people haven't hit on this same issue up to now. Is there a solution I'm not aware of? EDIT: nevermind. I was missing |
@zachdaniel it's mostly that In any event, please take a look at the just-released-yesterday 3.0.4 (release notes) and 3.1.1 (release notes) releases, as they both contain a substantial rewrite and dramatic expansion (with multiple new appendices added to the spec) of the parameter and form serialization rules. |
Will check it out. I also may have been wrong about the form style. I think I still haven't figured out how to get the format I'm looking for yet. |
I still don't see something hinting at what, to me, would be the most common option. What is the way to tell it that a query parameter with nested schemas should be encoded as |
@zachdaniel That's because In 3.0.4 and 3.1.1 we added the following clarification to
Unsatisfying, I know, but I dug through the history and there was no sign of any intention to specify how to handle these things. While certain frameworks have "standards" they don't all agree (you can look through other issues in this repo to find conflicting requests). In 3.2, I'm hoping to fix #1502 which would allow people to define their own alternative query string formats and not try to contort |
I get it from the spec perspective, but ultimately as someone maintaining a framework offering automatic swagger UI generation, this one singular sticking point issue is causing users to decide not to use swagger UI at all. It has (for them) a sort of all-or-nothing effect because they want to expose this to API consumers. I may be able to figure something out along the lines of detecting a request coming through swagger UI's "try it out" feature (maybe some way to put in a header to all of those requests for example), which would allow me to then do special query parameter decoding. I think I'm primarily taking umbrage with SwaggerUI at this point, not the spec. I can't imagine a world where anyone wants a query parameter to be URI encoded JSON. I get it that it's unspecified, but thats a weird choice. Not much left to do here on this issue, and looking forward to a resolution to #1502 as potentially opening a path here. EDIT: nevermind, that won't work either because I still can't tell the difference between the value being a string of valid JSON and a query parameter encoded by swagger UI 😢 |
Yeah we can't help you with that, I'm afraid!
Oh, I've seen it. I can't say I'd recommend it, but I've definitely seen it. I've also seen JSON documents embedded in JSON strings. People do some astonishing things...
I'm not quite sure where the JSON complaint comes in, though. Is SwaggerUI treating |
As far as I can tell, it's when I have a deepObject where a value is an |
@zachdaniel huh. Well, the spec does not define behavior for that case, so on the one hand, we can't say that's wrong. But also if they've chosen to do that, that's thier own decision and they don't need anything from us if they want to make a different decision. As far as the spec is concerned, the behavior of |
@zachdaniel although if this is an |
Background
Many applications expect deeply nested objects in input parameters, see the discussion in swagger-ui starting from this comment: swagger-api/swagger-ui#4064 (comment) In LoopBack, we are running into this problem too, see loopbackio/loopback-next#1679.
Consider a
filter
parameter defined as follows:Let's say the user wants to provide the following value, for example by entering a JSON into the text area rendered by swagger-ui:
At the moment, the OpenAPI Specification v3 does not describe how to encode such value in a query string. As a result, OpenAPI clients like swagger-ui don't know how to handle this input - see the discussion in swagger-api/swagger-js#1385 for an example.
Proposal
The following query string should be created by swagger-js client for the input value shown above.
The proposed serialization style is supported by https://www.npmjs.com/package/qs, which is used by http://expressjs.com as the default query parser, which means that a vast number of Node.js API servers are already expecting this serialization style.
I am not sure if there is any existing formal specification of this format, I am happy to look into that once my proposal gets considered as acceptable in principle.
Additional information
Existing issues that are touching a similar topic:
Two older comments from swagger-js that may be relevant:
swagger-api/swagger-js#1140
swagger-api/swagger-js#1140 (comment)
The text was updated successfully, but these errors were encountered: