Ignore actual default value instead of 0/false

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"

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
+    }
+}

codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/http/RequestBindingGenerator.kt

@@ -29,6 +29,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.generators.http.HttpBindi
 import software.amazon.smithy.rust.codegen.core.smithy.generators.operationBuildError
 import software.amazon.smithy.rust.codegen.core.smithy.isOptional
 import software.amazon.smithy.rust.codegen.core.smithy.protocols.Protocol
+import software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize.SerializerUtil
 import software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize.ValueExpression
 import software.amazon.smithy.rust.codegen.core.util.dq
 import software.amazon.smithy.rust.codegen.core.util.expectMember
@@ -74,6 +75,7 @@ class RequestBindingGenerator(
         HttpBindingGenerator(protocol, codegenContext, codegenContext.symbolProvider, operationShape)
     private val index = HttpBindingIndex.of(model)
     private val encoder = RuntimeType.smithyTypes(runtimeConfig).resolve("primitive::Encoder")
+    private val util = SerializerUtil(model, symbolProvider)
 
     private val codegenScope =
         arrayOf(
@@ -246,9 +248,15 @@ class RequestBindingGenerator(
 
                     paramList(target, derefName, param, writer, memberShape)
                 } else {
-                    ifSet(target, memberSymbol, ValueExpression.Reference("&_input.$memberName")) { field ->
-                        // if `param` is a list, generate another level of iteration
-                        paramList(target, field.name, param, writer, memberShape)
+                    // If we have an Option<T>, there won't be a default so nothing to ignore. If it's a primitive
+                    // boolean or number, we ignore the default.
+                    ifSome(memberSymbol, ValueExpression.Reference("&_input.$memberName")) { field ->
+                        with(util) {
+                            ignoreDefaultsForNumbersAndBools(memberShape, field) {
+                                // if `param` is a list, generate another level of iteration
+                                paramList(target, field.name, param, writer, memberShape)
+                            }
+                        }
                     }
                 }
             }

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
+}

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
@@ -729,33 +732,48 @@ 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.
+         * 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 ifNotDefault(
+        fun ifNotNumberOrBoolDefault(
             shape: Shape,
+            memberSymbol: Symbol,
             variable: ValueExpression,
             block: RustWriter.(field: ValueExpression) -> Unit,
         ) {
             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, 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)
+                                }
+                        }
+                    } else if (memberSymbol.defaultValue() is Default.NonZeroDefault) {
+                        val default = Node.printJson((memberSymbol.defaultValue() as Default.NonZeroDefault).value)
+                        rustBlock("if ${variable.asValue()} != $default") {
+                            block(variable)
+                        }
+                    } else {
+                        rustBlock("") {
+                            block(variable)
+                        }
                     }
-
+                }
                 else ->
                     rustBlock("") {
-                        this.block(variable)
+                        block(variable)
                     }
             }
         }
@@ -792,7 +810,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(

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt

@@ -55,6 +55,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.protocols.HttpLocation
 import software.amazon.smithy.rust.codegen.core.smithy.protocols.Protocol
 import software.amazon.smithy.rust.codegen.core.smithy.protocols.ProtocolFunctions
 import software.amazon.smithy.rust.codegen.core.smithy.protocols.parse.EventStreamUnmarshallerGenerator
+import software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize.SerializerUtil
 import software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize.ValueExpression
 import software.amazon.smithy.rust.codegen.core.smithy.rustType
 import software.amazon.smithy.rust.codegen.core.util.UNREACHABLE
@@ -130,6 +131,7 @@ class HttpBindingGenerator(
     private val headerUtil = RuntimeType.smithyHttp(runtimeConfig).resolve("header")
     private val defaultTimestampFormat = TimestampFormatTrait.Format.EPOCH_SECONDS
     private val protocolFunctions = ProtocolFunctions(codegenContext)
+    private val serializerUtil = SerializerUtil(model, symbolProvider)
 
     /**
      * Generate a function to deserialize [binding] from HTTP headers.
@@ -578,7 +580,6 @@ class HttpBindingGenerator(
         // default value for that primitive type (e.g. `Some(false)` for an `Option<bool>` header).
         // If a header is multivalued, we always want to serialize its primitive members, regardless of their
         // values.
-        val serializePrimitiveValuesIfDefault = memberSymbol.isOptional() || (targetShape is CollectionShape)
         ifSome(memberSymbol, ValueExpression.Reference("&input.$memberName")) { variableName ->
             if (targetShape is CollectionShape) {
                 renderMultiValuedHeader(
@@ -597,7 +598,8 @@ class HttpBindingGenerator(
                     false,
                     timestampFormat,
                     renderErrorMessage,
-                    serializePrimitiveValuesIfDefault,
+                    serializeIfDefault = memberSymbol.isOptional(),
+                    memberShape,
                 )
             }
         }
@@ -628,6 +630,7 @@ class HttpBindingGenerator(
                 timestampFormat,
                 renderErrorMessage,
                 serializeIfDefault = true,
+                shape.member,
             )
         }
     }
@@ -647,6 +650,7 @@ class HttpBindingGenerator(
         timestampFormat: TimestampFormatTrait.Format,
         renderErrorMessage: (String) -> Writable,
         serializeIfDefault: Boolean,
+        memberShape: MemberShape,
     ) {
         val context = HeaderValueSerializationContext(value, shape)
         for (customization in customizations) {
@@ -687,7 +691,11 @@ class HttpBindingGenerator(
         if (serializeIfDefault) {
             block(context.valueExpression)
         } else {
-            ifNotDefault(context.shape, context.valueExpression, block)
+            with(serializerUtil) {
+                ignoreDefaultsForNumbersAndBools(memberShape, context.valueExpression) {
+                    block(context.valueExpression)
+                }
+            }
         }
     }
 

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt

@@ -188,7 +188,7 @@ class JsonSerializerGenerator(
             "JsonValueWriter" to RuntimeType.smithyJson(runtimeConfig).resolve("serialize::JsonValueWriter"),
             "ByteSlab" to RuntimeType.ByteSlab,
         )
-    private val serializerUtil = SerializerUtil(model)
+    private val serializerUtil = SerializerUtil(model, symbolProvider)
 
     /**
      * Reusable structure serializer implementation that can be used to generate serializing code for
@@ -394,7 +394,7 @@ class JsonSerializerGenerator(
             }
 
             with(serializerUtil) {
-                ignoreZeroValues(context.shape, context.valueExpression) {
+                ignoreDefaultsForNumbersAndBools(context.shape, context.valueExpression) {
                     serializeMemberValue(context, targetShape)
                 }
             }

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/QuerySerializerGenerator.kt

@@ -5,7 +5,6 @@
 
 package software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize
 
-import software.amazon.smithy.model.knowledge.NullableIndex
 import software.amazon.smithy.model.shapes.BlobShape
 import software.amazon.smithy.model.shapes.BooleanShape
 import software.amazon.smithy.model.shapes.CollectionShape
@@ -90,13 +89,12 @@ abstract class QuerySerializerGenerator(private val codegenContext: CodegenConte
     protected val model = codegenContext.model
     protected val symbolProvider = codegenContext.symbolProvider
     protected val runtimeConfig = codegenContext.runtimeConfig
-    private val nullableIndex = NullableIndex(model)
     private val target = codegenContext.target
     private val serviceShape = codegenContext.serviceShape
     private val serializerError = runtimeConfig.serializationError()
     private val smithyTypes = RuntimeType.smithyTypes(runtimeConfig)
     private val smithyQuery = RuntimeType.smithyQuery(runtimeConfig)
-    private val serdeUtil = SerializerUtil(model)
+    private val serdeUtil = SerializerUtil(model, symbolProvider)
     private val protocolFunctions = ProtocolFunctions(codegenContext)
     private val codegenScope =
         arrayOf(
@@ -213,7 +211,7 @@ abstract class QuerySerializerGenerator(private val codegenContext: CodegenConte
             }
         } else {
             with(serdeUtil) {
-                ignoreZeroValues(context.shape, context.valueExpression) {
+                ignoreDefaultsForNumbersAndBools(context.shape, context.valueExpression) {
                     serializeMemberValue(context, targetShape)
                 }
             }

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/SerializerUtil.kt

@@ -5,31 +5,42 @@
 
 package software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize
 
+import software.amazon.smithy.codegen.core.SymbolProvider
 import software.amazon.smithy.model.Model
 import software.amazon.smithy.model.shapes.MemberShape
 import software.amazon.smithy.model.shapes.StructureShape
+import software.amazon.smithy.model.traits.ClientOptionalTrait
+import software.amazon.smithy.model.traits.InputTrait
 import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
 import software.amazon.smithy.rust.codegen.core.rustlang.Writable
 import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock
+import software.amazon.smithy.rust.codegen.core.util.hasTrait
 
-class SerializerUtil(private val model: Model) {
-    fun RustWriter.ignoreZeroValues(
+class SerializerUtil(private val model: Model, private val symbolProvider: SymbolProvider) {
+    fun RustWriter.ignoreDefaultsForNumbersAndBools(
         shape: MemberShape,
         value: ValueExpression,
         inner: Writable,
     ) {
-        // Required shapes should always be serialized
+        // @required shapes should always be serialized, and members with @clientOptional or part of @input structures
+        // should ignore default values. If we have an Option<T>, it won't have a default anyway, so we don't need to
+        // ignore it.
         // See https://github.com/smithy-lang/smithy-rs/issues/230 and https://github.com/aws/aws-sdk-go-v2/pull/1129
+        val container = model.expectShape(shape.container)
         if (
             shape.isRequired ||
+            shape.hasTrait<ClientOptionalTrait>() ||
             // Zero values are always serialized in lists and collections, this only applies to structures
-            model.expectShape(shape.container) !is StructureShape
+            container !is StructureShape ||
+            container.hasTrait<InputTrait>()
         ) {
             rustBlock("") {
                 inner(this)
             }
         } else {
-            this.ifNotDefault(model.expectShape(shape.target), value) { inner(this) }
+            this.ifNotNumberOrBoolDefault(model.expectShape(shape.target), symbolProvider.toSymbol(shape), value) {
+                inner(this)
+            }
         }
     }
 }