-
Notifications
You must be signed in to change notification settings - Fork 218
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
Clarify parsing of @httpQueryParams
members
#957
Conversation
When deserializing HTTP request query string parameters into members with the | ||
``httpQueryParams`` trait, servers MUST treat all values as strings and produce | ||
empty string values for keys which do not have values specified. For example, | ||
given the following model: |
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 isn't quite right.
When deserializing HTTP request query string parameters into members with the | |
``httpQueryParams`` trait, servers MUST treat all values as strings and produce | |
empty string values for keys which do not have values specified. For example, | |
given the following model: | |
When deserializing HTTP request query string parameters into members with the | |
``httpQueryParams`` trait, servers MUST treat parameters without values as | |
empty strings. For example, given the following model: |
In summary:
?queryParam
=>{"queryParam": ""}
?queryParam=
=>{"queryParam": ""}
?
=>{"queryParam": null}
Though.... that does kinda imply you can't have a sparse list, which I don't think we enforce.... I'll need to investigate that more Nevermind a sparse list would just omit that index I really need to talk to some of the other folks when they wake up
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.
To your point, are you referring to the fact that values could be split on commas to be treated as sets/lists of strings? Anything else is forbidden by the trait's selector, right? If so, it might be worth adding this to the documentation too (I'm happy to do it here), e.g. is splitting on commas correct/standardised, etc.
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.
And in your third example, would ?
not simply yield {}
? Or in the example given, tags: {}
?
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.
To your point, are you referring to the fact that values could be split on commas to be treated as sets/lists of strings?
No, query string lists duplicate the value. (Except in some aws protocols that embed the index as part of the key, which is causing some of my confusion). I really need to talk to the rest of the team about this when they're awake
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.
Jordon's correct, you get list values from repeating the key, like a=x&a=y&a=z
yielding a: ["x", "y", "z"]
. There's no way to introduce an explicit null value -- it's either not defined, or it's a non-null string.
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.
OK, based on this discussion, I've added a last example of the k=
case as well. I am happy to accept the above suggestion but feel the original is better because it clarifies (or at least, it was my intent to clarify) that there should be no parsing/treatment of values as anything other than strings. For instance, if I parse k1=true&k2=24
into @httpQueryParams
, the fact that it is a map { key: String, value: String }
shape means I get { "k1": "true", "k2": "24" }
and not e.g. { "k1": true, "k2": 24 }
(note the parsing of the boolean/number). Thoughts? Apologies if I've missed the point.
This commit documents how members with the `@httpQueryParams` trait applied should be parsed in the presence of query string parameters that do and do not have values specified.
Issue #, if available: #956
Description of changes:
This commit documents how members with the
@httpQueryParams
traitapplied should be parsed in the presence of query string parameters that
do and do not have values specified.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.