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

What is the server behavior for deserializing httpQueryParams when multiple values appear for the same key? #1071

Closed
david-perez opened this issue Jan 31, 2022 · 4 comments

Comments

@david-perez
Copy link
Contributor

The httpQueryParams trait can be applied to structure members that target a map of string, or a map of list/set of string.

When it targets a map of strings, what should a server implementation deserialize to when encountering multiple values for the same query key param? Should it pick the first value or the last one in the query string? The docs don't specify a behavior.

There are also no protocol tests.

@adamthom-amzn
Copy link
Contributor

There's no defined behavior. The current server implementation is, I believe, as follows:

  • for @httpQuery scalar members, the first value is taken
  • for @httpQueryParams with scalar values, the last value is taken

adamthom-amzn added a commit to adamthom-amzn/smithy that referenced this issue Feb 4, 2022
Existing implementations of restJson1 have diverging behavior for query
parameters with multiple values depending on whether @httpQuery or
@httpQueryParams is used. This updates the docs to use SHOULD to describe
the preferred, but not mandatory, behavior.

Fixes smithy-lang#1071
@adamthom-amzn
Copy link
Contributor

As we don't have a way to add protocol tests for SHOULDs yet (though this made us think we, uh, should), right now all I can offer is a documentation update.

@david-perez
Copy link
Contributor Author

As we don't have a way to add protocol tests for SHOULDs yet

@adamthom-amzn What mechanism did you have in mind? I was thinking that having separate test suites under smithy-aws-protocol-tests/model/ for SHOULDs should suffice.

@adamthom-amzn
Copy link
Contributor

As we don't have a way to add protocol tests for SHOULDs yet

@adamthom-amzn What mechanism did you have in mind? I was thinking that having separate test suites under smithy-aws-protocol-tests/model/ for SHOULDs should suffice.

I was thinking that we would have at least a flag for SHOULD tests, so code generators would have the option to try to make the tests run but not fail the build, if it could be managed. That way we could keep the tests grouped by functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants