Skip to content

Commit

Permalink
Make updates to default value behavior (#3064)
Browse files Browse the repository at this point in the history
## Motivation and Context
Several bugs in our default implementation where uncovered

## Description
- Documents and Timestamps both don't match the 0-value semantics, so in
those cases, also use an explicit value

## Testing
Unit test updated


----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
  • Loading branch information
rcoh authored Oct 12, 2023
1 parent 5a2dbd4 commit 98dadc0
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,14 @@ open class SymbolVisitor(
override fun memberShape(shape: MemberShape): Symbol {
val target = model.expectShape(shape.target)
val defaultValue = shape.getMemberTrait(model, DefaultTrait::class.java).orNull()?.let { trait ->
when (val value = trait.toNode()) {
Node.from(""), Node.from(0), Node.from(false), Node.arrayNode(), Node.objectNode() -> Default.RustDefault
Node.nullNode() -> Default.NoDefault
else -> Default.NonZeroDefault(value)
if (target.isDocumentShape || target.isTimestampShape) {
Default.NonZeroDefault(trait.toNode())
} else {
when (val value = trait.toNode()) {
Node.from(""), Node.from(0), Node.from(false), Node.arrayNode(), Node.objectNode() -> Default.RustDefault
Node.nullNode() -> Default.NoDefault
else -> Default.NonZeroDefault(value)
}
}
} ?: Default.NoDefault
// Handle boxing first, so we end up with Option<Box<_>>, not Box<Option<_>>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,26 @@ internal class BuilderGeneratorTest {
@default(true)
@required
defaultDocument: Document
@default([])
@required
listDocument: Document
@default(0)
zero_timestamp: Timestamp
@default(1)
one_timestamp: Timestamp
@default("abc")
blob_abc: Blob
}
list StringList {
member: String
}
@default(1)
integer OneDefault
""".asSmithyModel(smithyVersion = "2.0")

val provider = testSymbolProvider(
Expand All @@ -207,6 +221,10 @@ internal class BuilderGeneratorTest {
assert_eq!(s.an_actually_required_field(), 5);
assert_eq!(s.no_default(), None);
assert_eq!(s.default_document().as_bool().unwrap(), true);
assert_eq!(s.list_document().as_array().expect("empty list"), &[]);
assert_eq!(std::str::from_utf8(s.blob_abc().as_ref()).expect("invalid blob"), "abc");
assert_eq!(s.zero_timestamp().secs(), 0);
assert_eq!(s.one_timestamp().secs(), 1);
""",
"Struct" to provider.toSymbol(shape),
)
Expand Down

0 comments on commit 98dadc0

Please sign in to comment.