Ignore actual default value instead of 0/false #3252
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Opened PR to add protocol tests in Smithy for this: smithy-lang/smithy#2070 |
milesziemer
force-pushed
the
expand-ifnotdefault
branch
from
71e37cd
to
afd247d
Compare
Previously, there was logic used to ignore the default 0/false values for numbers/booleans when serializing members if they weren't boxed. This was to fix issues that occured when upstream models didn't properly box shapes that were meant to be optional, and for the most part worked because services would just fill in the default if it wasn't passed. However, with Smithy 2.0, models may have defaults != 0/false, but the codegenerator still ignores the zero value. This commit makes it so that the actual default value for the member is ignored for booleans and numbers, instead of just 0/false. It also updates serialization for http bindings so that headers and query parameters with 0/false values aren't ignored if they are optional parameters.
milesziemer
force-pushed
the
expand-ifnotdefault
branch
2 times, most recently
from
6863ec9
to
279b3bd
Compare
milesziemer
force-pushed
the
expand-ifnotdefault
branch
from
279b3bd
to
ce062cd
Compare
jdisanti
approved these changes
Dec 15, 2023
milesziemer
added a commit
to smithy-lang/smithy
that referenced
this pull request
Jan 24, 2024
Previously we didn't have protocol tests specifically for 0/false in query params, which allowed for situations like smithy-lang/smithy-rs#3252 to occur.
hpmellema
pushed a commit
to hpmellema/smithy
that referenced
this pull request
Jan 25, 2024
Previously we didn't have protocol tests specifically for 0/false in query params, which allowed for situations like smithy-lang/smithy-rs#3252 to occur.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
Previously, there was logic used to ignore the default 0/false values for numbers/booleans when serializing members if they weren't boxed. This was to fix issues that occured when upstream models didn't properly box shapes that were meant to be optional, and for the most part worked because services would just fill in the default if it wasn't passed. However, with Smithy 2.0, models may have defaults != 0/false, but the codegenerator still ignores the zero value.
Description
This commit makes it so that the actual default value for the member is ignored for booleans and numbers, instead of just 0/false. It also updates serialization for http bindings so that headers and query parameters with 0/false values aren't ignored if they are optional parameters.
Testing
if let Some() = ...blocks, and ignoring default value instead of just 0 (only seems to effect nimble).Checklist
CHANGELOG.next.tomlif I made changes to the smithy-rs codegen or runtime cratesCHANGELOG.next.tomlif I made changes to the AWS SDK, generated SDK code, or SDK runtime cratesBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.