Add marker trait for numeric shapes that should be serialized even when equal to 0 #1830
Conversation
…h the SerializeZeroValues trait fix: bug that occurs when serializing a "com.amazonaws.s3#ScanRange" starting at zero add: SerializeZeroValues smithy trait update: unify names of serializerUtil vars in serializer codegen update: CHANGELOG.next.toml with release notes
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
add: missing copyright header format: run ktlint
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
I've recently looked at this part of the codebase to fix #1831. Why are we not serializing zero values? I was able to trace the introduction of this behavior to #439. We're enforcing it via tests in And These tests are part of our "extras" suites; I can't find mention of this behavior in the official AWS specs. To me this seems really odd. The server is also reusing the serializers, so it's also not serializing zero values in responses. |
| private val serializeZeroValuesAllowList = setOf( | ||
| // Omitting this when zero would change the meaning of the range. | ||
| // https://docs.aws.amazon.com/AmazonS3/latest/API/API_ScanRange.html | ||
| ShapeId.from("com.amazonaws.s3#Start"), |
There was a problem hiding this comment.
I believe End should also accept zero.
https://docs.aws.amazon.com/AmazonS3/latest/API/API_ScanRange.html#AmazonS3-Type-ScanRange-End
[~] aws s3api select-object-content \
--bucket ${BUCKET_NAME_OMMITED} \
--key data-1m.json \
--expression 'SELECT * FROM s3object s' \
--expression-type SQL \
--input-serialization '{"JSON": {"Type": "LINES"}, "CompressionType": "NONE"}' \
--output-serialization '{"JSON": {}}' \
--scan-range Start=0,End=0 \
0.json
[~] echo $?
0
[~] cat 0.json
{"a":"foo","b":1,"c":"aaa"}
|
I briefly looked into the I think this is the structural problem, and, in the long term, |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
I think the larger fix that needs to happen is input tracking. Any value provided by a customer must be serialized and sent, regardless if it's zero value or not. It's hard to say why omitting the zero value was implemented since the commit doesn't provide any reasoning. I also didn't see any information about it in the Smithy specs. @david-perez - What's the expected behavior for server side serialization? Is it compatible with input tracking (output tracking in the server case)? |
|
No problems server-side on implementing input/output tracking. Omitting serialization of values (even if they're the default for a type) is even called out as something we should not do in the server in the spec:
|
|
I spoke with @rcoh about this and he thinks it should be solved by boxing primitive fields for S3 models. I can look into this after I complete my current task |
|
Closing in favor of an alternate solution |
Motivation and Context
aws-sdk-rust#630
Description
update: serializer generation to serialize zero values for shapes with the
SerializeZeroValuestraitfix: bug that occurs when serializing a
"com.amazonaws.s3#ScanRange"starting at zeroadd:
SerializeZeroValuessmithy traitupdate: unify names of
serializerUtilvars in serializer codegenupdate: CHANGELOG.next.toml with release notes
Testing
I verified that this change fixes the bug and included an integration test.
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.