From 279b3bdebef08c72dbf4a35b725b7ed81b765c5b Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Wed, 13 Dec 2023 10:06:09 -0500 Subject: [PATCH] Add protocol tests for 0/false query params --- CHANGELOG.next.toml | 12 ++++ .../model/rest-xml-extras.smithy | 31 +++++++++ .../rest-json-extras.smithy | 32 +++++++++ .../rust/codegen/core/rustlang/RustWriter.kt | 66 +++++++++++-------- 4 files changed, 115 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 42e5b69a620..2a7daf59430 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -109,3 +109,15 @@ message = "Add support for constructing [`SdkBody`] and [`ByteStream`] from `htt references = ["smithy-rs#3300", "aws-sdk-rust#977"] meta = { "breaking" = false, "tada" = true, "bug" = false } author = "rcoh" + +[[smithy-rs]] +message = "Serialize 0/false in query parameters, and ignore actual default value during serialization instead of just 0/false." +references = ["smithy-rs#3252", "smithy-rs#3312"] +meta = { "breaking" = true, "tada" = false, "bug" = true, "target" = "client" } +author = "milesziemer" + +[[aws-sdk-rust]] +message = "Serialize 0/false in query parameters, and ignore actual default value during serialization instead of just 0/false." +references = ["smithy-rs#3252", "smithy-rs#3312"] +meta = { "breaking" = true, "tada" = false, "bug" = true } +author = "milesziemer" diff --git a/codegen-client-test/model/rest-xml-extras.smithy b/codegen-client-test/model/rest-xml-extras.smithy index b518ad09c69..2c18a35e920 100644 --- a/codegen-client-test/model/rest-xml-extras.smithy +++ b/codegen-client-test/model/rest-xml-extras.smithy @@ -21,6 +21,8 @@ service RestXmlExtras { StringHeader, CreateFoo, RequiredMember, + // TODO(https://github.com/smithy-lang/smithy-rs/issues/3315) + ZeroAndFalseQueryParams, ] } @@ -254,3 +256,32 @@ structure RequiredMemberInputOutput { @required requiredString: String } + +@httpRequestTests([ + { + id: "RestXmlZeroAndFalseQueryParamsAreSerialized" + protocol: restXml + code: 200 + method: "GET" + uri: "/ZeroAndFalseQueryParams" + body: "" + queryParams: [ + "Zero=0", + "False=false" + ] + params: { + zeroValue: 0 + falseValue: false + } + } +]) +@http(uri: "/ZeroAndFalseQueryParams", method: "GET") +operation ZeroAndFalseQueryParams { + input := { + @httpQuery("Zero") + zeroValue: Integer + + @httpQuery("False") + falseValue: Boolean + } +} diff --git a/codegen-core/common-test-models/rest-json-extras.smithy b/codegen-core/common-test-models/rest-json-extras.smithy index 2633fe00a83..a4ff54af966 100644 --- a/codegen-core/common-test-models/rest-json-extras.smithy +++ b/codegen-core/common-test-models/rest-json-extras.smithy @@ -68,6 +68,8 @@ service RestJsonExtras { // TODO(https://github.com/smithy-lang/smithy-rs/issues/2968): Remove the following once these tests are included in Smithy // They're being added in https://github.com/smithy-lang/smithy/pull/1908 HttpPayloadWithUnion, + // TODO(https://github.com/smithy-lang/smithy-rs/issues/3315) + ZeroAndFalseQueryParams, ], errors: [ExtraError] } @@ -351,3 +353,33 @@ structure EmptyStructWithContentOnWireOpOutput { operation EmptyStructWithContentOnWireOp { output: EmptyStructWithContentOnWireOpOutput, } +@http(uri: "/zero-and-false-query-params", method: "GET") +@httpRequestTests([ + { + id: "RestJsonZeroAndFalseQueryParamsAreSerialized", + protocol: restJson1, + code: 200, + method: "GET", + uri: "/zero-and-false-query-params", + body: "", + queryParams: [ + "Zero=0", + "False=false" + ], + params: { + zeroValue: 0, + falseValue: false + } + } +]) +operation ZeroAndFalseQueryParams { + input: ZeroAndFalseQueryParamsInput +} + +structure ZeroAndFalseQueryParamsInput { + @httpQuery("Zero") + zeroValue: Integer + + @httpQuery("False") + falseValue: Boolean +} diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriter.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriter.kt index 05504e54c73..709e1b6b34e 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriter.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriter.kt @@ -14,6 +14,7 @@ import software.amazon.smithy.codegen.core.SymbolDependencyContainer import software.amazon.smithy.codegen.core.SymbolWriter import software.amazon.smithy.codegen.core.SymbolWriter.Factory import software.amazon.smithy.model.Model +import software.amazon.smithy.model.node.Node import software.amazon.smithy.model.shapes.BooleanShape import software.amazon.smithy.model.shapes.CollectionShape import software.amazon.smithy.model.shapes.DoubleShape @@ -24,9 +25,11 @@ import software.amazon.smithy.model.shapes.ShapeId import software.amazon.smithy.model.traits.DeprecatedTrait import software.amazon.smithy.model.traits.DocumentationTrait import software.amazon.smithy.rust.codegen.core.rustlang.Attribute.Companion.deprecated +import software.amazon.smithy.rust.codegen.core.smithy.Default import software.amazon.smithy.rust.codegen.core.smithy.ModuleDocProvider import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope +import software.amazon.smithy.rust.codegen.core.smithy.defaultValue import software.amazon.smithy.rust.codegen.core.smithy.isOptional import software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize.ValueExpression import software.amazon.smithy.rust.codegen.core.smithy.rustType @@ -727,38 +730,49 @@ class RustWriter private constructor( } } - /** - * Generate a wrapping if statement around a primitive field. - * The specified block will only be called if the field is not set to its default value - `0` for - * numbers, `false` for booleans. - */ - fun ifNotDefault( - shape: Shape, - variable: ValueExpression, - block: RustWriter.(field: ValueExpression) -> Unit, - ) { - when (shape) { - is FloatShape, is DoubleShape -> - rustBlock("if ${variable.asValue()} != 0.0") { - block(variable) + /** + * Generate a wrapping if statement around a primitive field. + * If the field is a number or boolean, the specified block is only called if the field is not equal to the + * member's default value. + */ + fun ifNotNumberOrBoolDefault( + shape: Shape, + memberSymbol: Symbol, + variable: ValueExpression, + block: RustWriter.(field: ValueExpression) -> Unit, + ) { + when (shape) { + is NumberShape, is BooleanShape -> { + if (memberSymbol.defaultValue() is Default.RustDefault) { + when (shape) { + is FloatShape, is DoubleShape -> rustBlock("if ${variable.asValue()} != 0.0") { + block(variable) + } + + is NumberShape -> rustBlock("if ${variable.asValue()} != 0") { + block(variable) + } + + is BooleanShape -> rustBlock("if ${variable.asValue()}") { + block(variable) + } } - - is NumberShape -> - rustBlock("if ${variable.asValue()} != 0") { + } else if (memberSymbol.defaultValue() is Default.NonZeroDefault) { + val default = Node.printJson((memberSymbol.defaultValue() as Default.NonZeroDefault).value) + rustBlock("if ${variable.asValue()} != $default") { block(variable) } - - is BooleanShape -> - rustBlock("if ${variable.asValue()}") { - block(variable) - } - - else -> + } else { rustBlock("") { - this.block(variable) + block(variable) } + } + } + else -> rustBlock("") { + block(variable) } } + } /** * Generate a wrapping if statement around a field. @@ -792,7 +806,7 @@ class RustWriter private constructor( variable: ValueExpression, block: RustWriter.(field: ValueExpression) -> Unit, ) { - ifSome(member, variable) { inner -> ifNotDefault(shape, inner, block) } + ifSome(member, variable) { inner -> ifNotNumberOrBoolDefault(shape, member, inner, block) } } fun listForEach(