Upgrade Smithy to 1.16.1 (#1053)

CHANGELOG.next.toml

@@ -22,3 +22,9 @@ message = "Add support for SSO credentials"
 references = ["smithy-rs#1051", "aws-sdk-rust#4"]
 meta = { "breaking" = false, "tada" = true, "bug" = false }
 author = "rcoh"
+
+[[smithy-rs]]
+message = "Upgraded Smithy to 1.16.1"
+references = ["smithy-rs#1053"]
+meta = { "breaking" = false, "tada" = false, "bug" = false }
+author = "jdisanti"

aws/sdk-codegen-test/build.gradle.kts

@@ -14,7 +14,6 @@ plugins {
 
 val smithyVersion: String by project
 
-
 dependencies {
     implementation(project(":aws:sdk-codegen"))
     implementation("software.amazon.smithy:smithy-aws-protocol-tests:$smithyVersion")
@@ -55,15 +54,13 @@ fun generateSmithyBuild(tests: List<CodegenTest>): String {
     """
 }
 
-
 task("generateSmithyBuild") {
     description = "generate smithy-build.json"
     doFirst {
         projectDir.resolve("smithy-build.json").writeText(generateSmithyBuild(CodegenTests))
     }
 }
 
-
 fun generateCargoWorkspace(tests: List<CodegenTest>): String {
     return """
     [workspace]
@@ -82,7 +79,6 @@ task("generateCargoWorkspace") {
 tasks["smithyBuildJar"].dependsOn("generateSmithyBuild")
 tasks["assemble"].finalizedBy("generateCargoWorkspace")
 
-
 tasks.register<Exec>("cargoCheck") {
     workingDir("build/smithyprojections/sdk-codegen-test/")
     // disallow warnings

build.gradle.kts

@@ -36,7 +36,7 @@ dependencies {
 }
 
 val lintPaths = listOf(
-        "codegen/src/**/*.kt"
+    "codegen/src/**/*.kt"
 )
 
 tasks.register<JavaExec>("ktlint") {

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt

@@ -142,20 +142,22 @@ class ServerProtocolTestGenerator(
     // This function applies a "fix function" to each broken test before we synthesize it.
     // Broken tests are those whose definitions in the `awslabs/smithy` repository are wrong, usually because they have
     // not been written with a server-side perspective in mind.
-    private fun List<TestCase>.fixBroken(): List<TestCase> = this.map { when (it) {
-        is TestCase.RequestTest -> {
-            val howToFixIt = BrokenRequestTests[Pair(codegenContext.serviceShape.id.toString(), it.testCase.id)]
-            if (howToFixIt == null) {
+    private fun List<TestCase>.fixBroken(): List<TestCase> = this.map {
+        when (it) {
+            is TestCase.RequestTest -> {
+                val howToFixIt = BrokenRequestTests[Pair(codegenContext.serviceShape.id.toString(), it.testCase.id)]
+                if (howToFixIt == null) {
+                    it
+                } else {
+                    val fixed = howToFixIt(it.testCase)
+                    TestCase.RequestTest(fixed, it.targetShape)
+                }
+            }
+            is TestCase.ResponseTest -> {
                 it
-            } else {
-                val fixed = howToFixIt(it.testCase)
-                TestCase.RequestTest(fixed, it.targetShape)
             }
         }
-        is TestCase.ResponseTest -> {
-            it
-        }
-    } }
+    }
 
     private fun renderTestCaseBlock(
         testCase: HttpMessageTestCase,
@@ -431,6 +433,11 @@ class ServerProtocolTestGenerator(
         private val AwsQuery = "aws.protocoltests.query#AwsQuery"
         private val Ec2Query = "aws.protocoltests.ec2#AwsEc2"
         private val ExpectFail = setOf<FailingTest>(
+            FailingTest(RestJson, "RestJsonInputAndOutputWithQuotedStringHeaders", Action.Request),
+            FailingTest(RestJson, "RestJsonInputAndOutputWithQuotedStringHeaders", Action.Response),
+            FailingTest(RestJson, "RestJsonOutputUnionWithUnitMember", Action.Response),
+            FailingTest(RestJson, "RestJsonUnitInputAllowsAccept", Action.Request),
+            FailingTest(RestJson, "RestJsonUnitInputAndOutputNoOutput", Action.Response),
             FailingTest(RestJson, "RestJsonAllQueryStringTypes", Action.Request),
             FailingTest(RestJson, "RestJsonQueryStringEscaping", Action.Request),
             FailingTest(RestJson, "RestJsonSupportsNaNFloatQueryValues", Action.Request),
@@ -572,38 +579,50 @@ class ServerProtocolTestGenerator(
             // to any "expected" value.
             // Reference: https://doc.rust-lang.org/std/primitive.f32.html
             // Request for guidance about this test to Smithy team: https://github.com/awslabs/smithy/pull/1040#discussion_r780418707
-            val params = Node.parse("""{
-                "queryFloat": "NaN",
-                "queryDouble": "NaN",
-                "queryParamsMapOfStringList": {
-                    "Float": ["NaN"],
-                    "Double": ["NaN"]
+            val params = Node.parse(
+                """
+                {
+                    "queryFloat": "NaN",
+                    "queryDouble": "NaN",
+                    "queryParamsMapOfStringList": {
+                        "Float": ["NaN"],
+                        "Double": ["NaN"]
+                    }
                 }
-            }""".trimIndent()).asObjectNode().get()
+                """.trimIndent()
+            ).asObjectNode().get()
 
             return testCase.toBuilder().params(params).build()
         }
         private fun fixRestJsonSupportsInfinityFloatQueryValues(testCase: HttpRequestTestCase): HttpRequestTestCase =
             testCase.toBuilder().params(
-               Node.parse("""{
-                   "queryFloat": "Infinity",
-                   "queryDouble": "Infinity",
-                   "queryParamsMapOfStringList": {
-                       "Float": ["Infinity"],
-                       "Double": ["Infinity"]
-                   }
-               }""".trimMargin()).asObjectNode().get()
+                Node.parse(
+                    """
+                    {
+                        "queryFloat": "Infinity",
+                        "queryDouble": "Infinity",
+                        "queryParamsMapOfStringList": {
+                            "Float": ["Infinity"],
+                            "Double": ["Infinity"]
+                        }
+                    }
+                    """.trimMargin()
+                ).asObjectNode().get()
             ).build()
         private fun fixRestJsonSupportsNegativeInfinityFloatQueryValues(testCase: HttpRequestTestCase): HttpRequestTestCase =
             testCase.toBuilder().params(
-                Node.parse("""{
-                   "queryFloat": "-Infinity",
-                   "queryDouble": "-Infinity",
-                   "queryParamsMapOfStringList": {
-                       "Float": ["-Infinity"],
-                       "Double": ["-Infinity"]
-                   }
-               }""".trimMargin()).asObjectNode().get()
+                Node.parse(
+                    """
+                    {
+                        "queryFloat": "-Infinity",
+                        "queryDouble": "-Infinity",
+                        "queryParamsMapOfStringList": {
+                            "Float": ["-Infinity"],
+                            "Double": ["-Infinity"]
+                        }
+                    }
+                    """.trimMargin()
+                ).asObjectNode().get()
             ).build()
 
         // These are tests whose definitions in the `awslabs/smithy` repository are wrong.

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerRestJson.kt

@@ -22,6 +22,7 @@ import software.amazon.smithy.rust.codegen.smithy.protocols.ProtocolContentTypes
 import software.amazon.smithy.rust.codegen.smithy.protocols.ProtocolGeneratorFactory
 import software.amazon.smithy.rust.codegen.smithy.protocols.parse.JsonParserGenerator
 import software.amazon.smithy.rust.codegen.smithy.protocols.parse.StructuredDataParserGenerator
+import software.amazon.smithy.rust.codegen.smithy.protocols.restJsonFieldName
 import software.amazon.smithy.rust.codegen.smithy.protocols.serialize.JsonSerializerGenerator
 import software.amazon.smithy.rust.codegen.smithy.protocols.serialize.StructuredDataSerializerGenerator
 
@@ -75,11 +76,11 @@ class ServerRestJson(private val codegenContext: CodegenContext) : Protocol {
     override val defaultTimestampFormat: TimestampFormatTrait.Format = TimestampFormatTrait.Format.EPOCH_SECONDS
 
     override fun structuredDataParser(operationShape: OperationShape): StructuredDataParserGenerator {
-        return JsonParserGenerator(codegenContext, httpBindingResolver)
+        return JsonParserGenerator(codegenContext, httpBindingResolver, ::restJsonFieldName)
     }
 
     override fun structuredDataSerializer(operationShape: OperationShape): StructuredDataSerializerGenerator {
-        return JsonSerializerGenerator(codegenContext, httpBindingResolver)
+        return JsonSerializerGenerator(codegenContext, httpBindingResolver, ::restJsonFieldName)
     }
 
     // NOTE: this method is only needed for the little part of client-codegen we use in tests.

codegen-test/model/rest-json-extras.smithy

@@ -7,6 +7,46 @@ use aws.api#service
 use smithy.test#httpRequestTests
 use smithy.test#httpResponseTests
 
+// TODO(https://github.com/awslabs/smithy/pull/1049): Remove this once the test case in Smithy is fixed
+apply InputAndOutputWithHeaders @httpResponseTests([
+    {
+        id: "FIXED_RestJsonInputAndOutputWithQuotedStringHeaders",
+        documentation: "Tests responses with string list header bindings that require quoting",
+        protocol: restJson1,
+        code: 200,
+        headers: {
+            "X-StringList": "\"b,c\", \"\\\"def\\\"\", a"
+        },
+        params: {
+            headerStringList: ["b,c", "\"def\"", "a"]
+        }
+    }
+])
+
+// TODO(https://github.com/awslabs/smithy/pull/1042): Remove this once the test case in Smithy is fixed
+apply PostPlayerAction @httpRequestTests([
+    {
+        id: "FIXED_RestJsonInputUnionWithUnitMember",
+        documentation: "Unit types in unions are serialized like normal structures in requests.",
+        protocol: restJson1,
+        method: "POST",
+        "uri": "/PostPlayerInput",
+        body: """
+            {
+                "action": {
+                    "quit": {}
+                }
+            }""",
+        bodyMediaType: "application/json",
+        headers: {"Content-Type": "application/json"},
+        params: {
+            action: {
+                quit: {}
+            }
+        }
+    }
+])
+
 apply QueryPrecedence @httpRequestTests([
     {
         id: "UrlParamsKeyEncoding",
@@ -64,6 +104,8 @@ service RestJsonExtras {
         NullInNonSparse,
         CaseInsensitiveErrorOperation,
         EmptyStructWithContentOnWireOp,
+        // TODO(https://github.com/awslabs/smithy/pull/1042): Remove this once the test case in Smithy is fixed
+        PostPlayerAction
     ],
     errors: [ExtraError]
 }

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

@@ -77,6 +77,7 @@ class RequestBindingGenerator(
 ) {
     private val index = HttpBindingIndex.of(model)
     private val Encoder = CargoDependency.SmithyTypes(runtimeConfig).asType().member("primitive::Encoder")
+    private val headerUtil = CargoDependency.SmithyHttp(runtimeConfig).asType().member("header")
 
     private val codegenScope = arrayOf(
         "BuildError" to runtimeConfig.operationBuildError(),
@@ -175,6 +176,7 @@ class RequestBindingGenerator(
             else -> UNREACHABLE("unexpected member for prefix headers: $memberType")
         }
         ifSet(memberType, memberSymbol, "&_input.$memberName") { field ->
+            val listHeader = memberType is CollectionShape
             rustTemplate(
                 """
                 for (k, v) in $field {
@@ -183,8 +185,8 @@ class RequestBindingGenerator(
                         #{build_error}::InvalidField { field: ${memberName.dq()}, details: format!("`{}` cannot be used as a header name: {}", k, err)}
                     })?;
                     use std::convert::TryFrom;
-                    let header_value = ${headerFmtFun(this, target, memberShape, "v")};
-                    let header_value = http::header::HeaderValue::try_from(header_value).map_err(|err| {
+                    let header_value = ${headerFmtFun(this, target, memberShape, "v", listHeader)};
+                    let header_value = http::header::HeaderValue::try_from(&*header_value).map_err(|err| {
                         #{build_error}::InvalidField {
                             field: ${memberName.dq()},
                             details: format!("`{}` cannot be used as a header value: {}", ${
@@ -210,12 +212,13 @@ class RequestBindingGenerator(
         val memberSymbol = symbolProvider.toSymbol(memberShape)
         val memberName = symbolProvider.toMemberName(memberShape)
         ifSet(memberType, memberSymbol, "&_input.$memberName") { field ->
+            val isListHeader = memberType is CollectionShape
             listForEach(memberType, field) { innerField, targetId ->
                 val innerMemberType = model.expectShape(targetId)
                 if (innerMemberType.isPrimitive()) {
                     rust("let mut encoder = #T::from(${autoDeref(innerField)});", Encoder)
                 }
-                val formatted = headerFmtFun(this, innerMemberType, memberShape, innerField)
+                val formatted = headerFmtFun(this, innerMemberType, memberShape, innerField, isListHeader)
                 val safeName = safeName("formatted")
                 write("let $safeName = $formatted;")
                 rustBlock("if !$safeName.is_empty()") {
@@ -244,21 +247,30 @@ class RequestBindingGenerator(
     /**
      * Format [member] in the when used as an HTTP header
      */
-    private fun headerFmtFun(writer: RustWriter, target: Shape, member: MemberShape, targetName: String): String {
+    private fun headerFmtFun(writer: RustWriter, target: Shape, member: MemberShape, targetName: String, isListHeader: Boolean): String {
+        fun quoteValue(value: String): String {
+            // Timestamp shapes are not quoted in header lists
+            return if (isListHeader && !target.isTimestampShape) {
+                val quoteFn = writer.format(headerUtil.member("quote_header_value"))
+                "$quoteFn($value)"
+            } else {
+                value
+            }
+        }
         return when {
             target.isStringShape -> {
                 if (target.hasTrait<MediaTypeTrait>()) {
                     val func = writer.format(RuntimeType.Base64Encode(runtimeConfig))
                     "$func(&$targetName)"
                 } else {
-                    "AsRef::<str>::as_ref($targetName)"
+                    quoteValue("AsRef::<str>::as_ref($targetName)")
                 }
             }
             target.isTimestampShape -> {
                 val timestampFormat =
                     index.determineTimestampFormat(member, HttpBinding.Location.HEADER, defaultTimestampFormat)
                 val timestampFormatType = RuntimeType.TimestampFormat(runtimeConfig, timestampFormat)
-                "$targetName.fmt(${writer.format(timestampFormatType)})?"
+                quoteValue("$targetName.fmt(${writer.format(timestampFormatType)})?")
             }
             target.isListShape || target.isMemberShape -> {
                 throw IllegalArgumentException("lists should be handled at a higher level")

codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/protocol/ProtocolTestGenerator.kt

@@ -308,9 +308,9 @@ class ProtocolTestGenerator(
                     rust(
                         """
                         assert_eq!(
-                                        parsed.$memberName.collect().await.unwrap().into_bytes(),
-                                        expected_output.$memberName.collect().await.unwrap().into_bytes()
-                                    );
+                            parsed.$memberName.collect().await.unwrap().into_bytes(),
+                            expected_output.$memberName.collect().await.unwrap().into_bytes()
+                        );
                         """
                     )
                 } else {
@@ -367,7 +367,7 @@ class ProtocolTestGenerator(
         }
         val variableName = "expected_headers"
         rustWriter.withBlock("let $variableName = [", "];") {
-            write(
+            writeWithNoFormatting(
                 headers.entries.joinToString(",") {
                     "(${it.key.dq()}, ${it.value.dq()})"
                 }
@@ -450,7 +450,13 @@ class ProtocolTestGenerator(
         private val RestXml = "aws.protocoltests.restxml#RestXml"
         private val AwsQuery = "aws.protocoltests.query#AwsQuery"
         private val Ec2Query = "aws.protocoltests.ec2#AwsEc2"
-        private val ExpectFail = setOf<FailingTest>()
+        private val ExpectFail = setOf<FailingTest>(
+            // TODO(https://github.com/awslabs/smithy/pull/1049): Remove this once the test case in Smithy is fixed
+            FailingTest(RestJson, "RestJsonInputAndOutputWithQuotedStringHeaders", Action.Response),
+            // TODO(https://github.com/awslabs/smithy/pull/1042): Remove this once the test case in Smithy is fixed
+            FailingTest(RestJson, "RestJsonInputUnionWithUnitMember", Action.Request),
+            FailingTest("${RestJson}Extras", "RestJsonInputUnionWithUnitMember", Action.Request),
+        )
         private val RunOnly: Set<String>? = null
 
         // These tests are not even attempted to be generated, either because they will not compile