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

Specification for deserialising @httpQueryParams #956

Closed
lunaris opened this issue Oct 22, 2021 · 4 comments
Closed

Specification for deserialising @httpQueryParams #956

lunaris opened this issue Oct 22, 2021 · 4 comments
Labels
documentation This is a problem with documentation.

Comments

@lunaris
Copy link
Contributor

lunaris commented Oct 22, 2021

The specification states that @httpQueryParams can only be used on operation inputs, and gives rules about how such inputs should be serialised when making and sending requests. It doesn't (I believe), however, talk about how to deserialise/parse query parameters (e.g. in [generated] server-side code) when receiving requests. Specifically, it's not clear how to handle query string parameters that do not have values.

For instance, given these Smithy definitions:

operation MyOperation {
  input: MyInput
}

structure MyInput {
  @httpQueryParams
  myQueryParams: MyMap
}

map MyMap {
  key: String,
  value: String
}

Serialising a request is specified and makes sense -- supposing we have a map with entries ("first", "1") and ("second", "true") we must produce ?first=1&second=true. However, suppose we receive a request with the query string ?first=1&second&third=false. What should we do? Options I can think of:

  1. Discard all keys for which there is no value (here second), yielding ("first", "1"), ("third", "false")
  2. Treat the MyMap map shape as though it were @sparse, permitting us to have optional values in it, and store all keys, yielding ("first", "1"), ("second", null), ("third", "false")
  3. Discard all keys, yielding an empty map
  4. Throw an error (e.g. 400 Bad Request) and require that all keys must have values

Thoughts welcome and apologies if I have missed something that clears this up in the spec.

@JordonPhillips
Copy link
Contributor

That's a great question, we actually ran into that recently and added a server test that incorporates it, but failed to boil that up to narrative documentation. The answer is that a key without a value (or a key with an empty value like first=&second=2) is treated as an empty string.

@lunaris
Copy link
Contributor Author

lunaris commented Oct 22, 2021

This makes sense; thanks. A similar issue crops up for @httpResponseCode in the documentation -- what to do with an optional field there? I'm guessing that null means falling back to the "default" for the enclosing operation (e.g. as specified by the code field of the @http trait)? If so I will see about raising a PR to update the documentation in this vein.

@JordonPhillips
Copy link
Contributor

Ah, we really should have forced that to target a required member. For a client, you always just take whatever is on the request. For a server, you would just serialize the default code as you said

@adamthom-amzn
Copy link
Contributor

Closing this as I think the associated pull request (thanks for the contribution!) covered what you were looking for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation This is a problem with documentation.
Projects
None yet
Development

No branches or pull requests

3 participants