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

Fix query parameters in SSDK #449

Conversation

adamthom-amzn
Copy link
Contributor

Description of changes:
Query parameters in protocol tests were being generated in a way that did not match how our APIGateway integration code would present them to our generated code. Query parameters are actually broken right now, but protocol tests do not detect this. These commits align our protocol test generation to how APIGateway treats query parameter values, and fixes code generation to handle them appropriately.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

We use APIGateway multi-value query parameters which will always use an array
value for query parameters, even if they're specified only once, and even if
that single specification does not have a value. Our translation logic from
APIG preserves this structure, and since all query parameters can have multiple
values, this isn't necessarily wrong. Our handling of query parameters is
actually broken in main, but protocol tests don't uncover this due to the logic
that generates non-array values for single query parameters. Streamlining our
generated requests in protocol tests uncovers this bug, and aligns more closely
with how our code runs in the wild.
We use APIGateway multi-value query parameters which will always use an array
value for query parameters, even if they're specified only once, and even if
that single specification does not have a value. This changes our logic to
not reject an array with a single value when parsing the value for a
scalar-bound query parameter, which fixes our handling of query parameters.
@adamthom-amzn adamthom-amzn merged commit 766a9bd into smithy-lang:main Oct 13, 2021
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

Successfully merging this pull request may close these issues.

None yet

3 participants