Add protocol tests for 0/false query params

smithy-lang · Dec 15, 2023 · 279b3bd · 279b3bd
1 parent fb5e318
commit 279b3bd
Show file tree

Hide file tree

Showing 4 changed files with 115 additions and 26 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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(