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

Serialize zero values #3243

Closed

Conversation

milesziemer
Copy link
Contributor

@milesziemer milesziemer commented Nov 21, 2023

Motivation and Context

Generated clients currently don't serialize the value 0 and false for members that have a default value. It seems like this logic was originally added to fix issues with S3 which are technically modeling issues and have since been fixed. Not serializing zero values has caused issues in the past, for example awslabs/aws-sdk-rust#630, and in other SDKs that do it like Go v2: aws/aws-sdk-go-v2#2162. Also the Smithy spec says clients should serialize default values, which generated clients currently do not do if that default is 0/false.

Description

Removes logic from RustWriter that was used to not serialize zero values, ie. 0 for numbers and false for booleans, if that was also the default value for the member. With this change, the only time the default value for a member is not serialized is if the member is also @clientOptional per Smithy spec.

The Blob type was also updated to implement std::Default, since SymbolVisitor treats "" as a RustDefault.

Important: This change impacts all the protocols and will cause many members to now be sent over the wire that previously weren't.

Testing

  • Generated the AWS SDK and inspected the diff, looks like the expected changes, removing checks for default values in serializers.
  • TODO: Testing with individual clients: This is a much larger task since there are so many clients that have been changed, will likely use a subset.

Checklist

  • I have updated CHANGELOG.next.toml if I made changes to the smithy-rs codegen or runtime crates
  • I have updated CHANGELOG.next.toml if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates

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

@milesziemer milesziemer requested review from a team as code owners November 21, 2023 00:16
Removes logic from RustWriter that was used to not serialize
zero values, ie. 0 for numbers and false for booleans, if
that was also the default value for the member. With this change,
the only time the default value for a member is not serialized is
if the member is also `@clientOptional`.

The Blob type was also updated to implement std::Default, since
SymbolVisitor treats `""` as a RustDefault.

It _seems_ like this logic was originally added to fix issues with
[S3](smithy-lang#102) which
are technically modeling issues and have since been fixed.
However, this change impacts all the protocols and will cause
many members to now be sent over the wire that previously weren't.
@milesziemer
Copy link
Contributor Author

Closing in favor of #3252, until we better test the impact of default value serialization.

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.

1 participant