diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 42e5b69a62..dcc8089443 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. See [changelog discussion](https://github.com/smithy-lang/smithy-rs/discussions/3312) for details." +references = ["smithy-rs#3252", "smithy-rs#3312"] +meta = { "breaking" = false, "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. See [changelog discussion](https://github.com/smithy-lang/smithy-rs/discussions/3312) for details." +references = ["smithy-rs#3252", "smithy-rs#3312"] +meta = { "breaking" = false, "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 b518ad09c6..2c18a35e92 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-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/http/RequestBindingGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/http/RequestBindingGenerator.kt index 24aa79d6cd..62b1d66d03 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/http/RequestBindingGenerator.kt +++ b/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, 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) + } + } } } } diff --git a/codegen-core/common-test-models/rest-json-extras.smithy b/codegen-core/common-test-models/rest-json-extras.smithy index 2633fe00a8..a4ff54af96 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 05504e54c7..302b2ba917 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 @@ -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, 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 NumberShape -> + rustBlock("if ${variable.asValue()} != 0") { + block(variable) + } - is BooleanShape -> - rustBlock("if ${variable.asValue()}") { - 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( diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt index 68cdaadd1f..9687bb2492 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt +++ b/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` 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) + } + } } } diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt index a351b25b65..0a95ab2652 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt +++ b/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) } } diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/QuerySerializerGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/QuerySerializerGenerator.kt index 8d6454160b..1686950a09 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/QuerySerializerGenerator.kt +++ b/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) } } diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/SerializerUtil.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/SerializerUtil.kt index 3617fa8ae9..55b65063b5 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/SerializerUtil.kt +++ b/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, 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() || // 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() ) { 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) + } } } } diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/XmlBindingTraitSerializerGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/XmlBindingTraitSerializerGenerator.kt index f29e867e1e..ebb5f6aa67 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/XmlBindingTraitSerializerGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/XmlBindingTraitSerializerGenerator.kt @@ -73,7 +73,7 @@ class XmlBindingTraitSerializerGenerator( private val xmlIndex = XmlNameIndex.of(model) private val rootNamespace = codegenContext.serviceShape.getTrait() - private val util = SerializerUtil(model) + private val util = SerializerUtil(model, symbolProvider) sealed class Ctx { abstract val input: String @@ -563,7 +563,7 @@ class XmlBindingTraitSerializerGenerator( } else { ValueExpression.Value(ctx.input) } - ignoreZeroValues(member, valueExpression) { + ignoreDefaultsForNumbersAndBools(member, valueExpression) { inner(ctx) } }