Smithy 1.9/1.10 Upgrade (#618)

* smithy 1.9.1 upgrade & primitive encode/decode This upgrades to Smithy 1.10, but the major change is a complete overhaul of how primitives are formatted and parsed. Primitive serialization was migrated and unified into Smithy Types with the end requirement of dealing with special float serialization semantics. * Switch to Smithy Core S3 Customization Trait Smithy 1.9.1 brings S3UnwrappedXmlOutput as a vended trait. This commit pulls in the new model & uses that trait. * Fix clippy warnings * Fix doc links * fix kotlin formatting * Fix s3 customization to use the operation shape * Ensure that numbers in string don't parse as numbers * remove unused itoa * Apply suggestions from code review Co-authored-by: John DiSanti <[email protected]> * Fix tests, CR feedback * rename parse to parse_smithy_primitive * Fix some more clippy errors * Update changelog Co-authored-by: John DiSanti <[email protected]>
smithy-lang · Jul 30, 2021 · f1a726c · f1a726c
1 parent 9fef09a
commit f1a726c
Show file tree

Hide file tree

Showing 54 changed files with 708 additions and 229 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 **New This Week**
 - :bug: Correctly encode HTTP Checksums using base64 instead of hex. Fixes aws-sdk-rust#164. (#615)
 - (When complete) Add profile file provider for region (#594, #xyz)
+- Overhaul serialization/deserialization of numeric/boolean types. This resolves issues around serialization of NaN/Infinity and should also reduce the number of allocations required during serialization. (#618)
 
 ## v0.18.1 (July 27th 2021)
 * Remove timestreamwrite and timestreamquery from the generated services (#613)

diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/s3/S3Decorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/s3/S3Decorator.kt
@@ -6,12 +6,8 @@
 package software.amazon.smithy.rustsdk.customize.s3
 
 import software.amazon.smithy.aws.traits.protocols.RestXmlTrait
-import software.amazon.smithy.model.Model
 import software.amazon.smithy.model.shapes.OperationShape
-import software.amazon.smithy.model.shapes.ServiceShape
 import software.amazon.smithy.model.shapes.ShapeId
-import software.amazon.smithy.model.shapes.StructureShape
-import software.amazon.smithy.model.transform.ModelTransformer
 import software.amazon.smithy.rust.codegen.rustlang.CargoDependency
 import software.amazon.smithy.rust.codegen.rustlang.Writable
 import software.amazon.smithy.rust.codegen.rustlang.asType
@@ -28,7 +24,6 @@ import software.amazon.smithy.rust.codegen.smithy.letIf
 import software.amazon.smithy.rust.codegen.smithy.protocols.ProtocolMap
 import software.amazon.smithy.rust.codegen.smithy.protocols.RestXml
 import software.amazon.smithy.rust.codegen.smithy.protocols.RestXmlFactory
-import software.amazon.smithy.rust.codegen.smithy.traits.S3UnwrappedXmlOutputTrait
 import software.amazon.smithy.rustsdk.AwsRuntimeType
 
 /**
@@ -59,20 +54,6 @@ class S3Decorator : RustCodegenDecorator {
             it + S3PubUse()
         }
     }
-
-    override fun transformModel(service: ServiceShape, model: Model): Model {
-        return model.letIf(applies(service.id)) {
-            ModelTransformer.create().mapShapes(model) { shape ->
-                // Apply the S3UnwrappedXmlOutput customization to GetBucketLocation (more
-                // details on the S3UnwrappedXmlOutputTrait)
-                if (shape is StructureShape && shape.id == ShapeId.from("com.amazonaws.s3#GetBucketLocationOutput")) {
-                    shape.toBuilder().addTrait(S3UnwrappedXmlOutputTrait()).build()
-                } else {
-                    shape
-                }
-            }
-        }
-    }
 }
 
 class S3(protocolConfig: ProtocolConfig) : RestXml(protocolConfig) {

diff --git a/aws/sdk/aws-models/accessanalyzer.json b/aws/sdk/aws-models/accessanalyzer.json
@@ -4823,4 +4823,4 @@
             }
         }
     }
-}
+}
diff --git a/aws/sdk/aws-models/amp.json b/aws/sdk/aws-models/amp.json
@@ -935,4 +935,4 @@
             }
         }
     }
-}
+}
diff --git a/aws/sdk/aws-models/appmesh.json b/aws/sdk/aws-models/appmesh.json
@@ -8671,4 +8671,4 @@
             }
         }
     }
-}
+}
diff --git a/aws/sdk/aws-models/braket.json b/aws/sdk/aws-models/braket.json
@@ -1404,4 +1404,4 @@
             }
         }
     }
-}
+}
diff --git a/aws/sdk/aws-models/codeguruprofiler.json b/aws/sdk/aws-models/codeguruprofiler.json
@@ -3174,4 +3174,4 @@
             }
         }
     }
-}
+}
diff --git a/aws/sdk/aws-models/groundstation.json b/aws/sdk/aws-models/groundstation.json
@@ -3544,4 +3544,4 @@
             }
         }
     }
-}
+}
diff --git a/aws/sdk/aws-models/location.json b/aws/sdk/aws-models/location.json
@@ -5937,4 +5937,4 @@
             }
         }
     }
-}
+}
diff --git a/aws/sdk/aws-models/mgn.json b/aws/sdk/aws-models/mgn.json
@@ -3944,4 +3944,4 @@
             }
         }
     }
-}
+}
diff --git a/aws/sdk/aws-models/mwaa.json b/aws/sdk/aws-models/mwaa.json
@@ -1938,4 +1938,4 @@
             }
         }
     }
-}
+}
diff --git a/aws/sdk/aws-models/proton.json b/aws/sdk/aws-models/proton.json
@@ -6482,4 +6482,4 @@
             }
         }
     }
-}
+}
diff --git a/aws/sdk/aws-models/rds-data.json b/aws/sdk/aws-models/rds-data.json
@@ -1336,4 +1336,4 @@
             }
         }
     }
-}
+}
diff --git a/aws/sdk/aws-models/redshift-data.json b/aws/sdk/aws-models/redshift-data.json
@@ -1423,4 +1423,4 @@
             "type": "boolean"
         }
     }
-}
+}
diff --git a/aws/sdk/aws-models/s3.json b/aws/sdk/aws-models/s3.json
@@ -4073,6 +4073,7 @@
                 "target": "com.amazonaws.s3#GetBucketLocationOutput"
             },
             "traits": {
+                "aws.customizations#s3UnwrappedXmlOutput": {},
                 "smithy.api#documentation": "<p>Returns the Region the bucket resides in. You set the bucket's Region using the\n            <code>LocationConstraint</code> request parameter in a <code>CreateBucket</code>\n         request. For more information, see <a href=\"https://docs.aws.amazon.com/AmazonS3/latest/API/API_CreateBucket.html\">CreateBucket</a>.</p>\n\n         <p> To use this implementation of the operation, you must be the bucket owner.</p>\n\n         <p>The following operations are related to <code>GetBucketLocation</code>:</p>\n         <ul>\n            <li>\n               <p>\n                  <a href=\"https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObject.html\">GetObject</a>\n               </p>\n            </li>\n            <li>\n               <p>\n                  <a href=\"https://docs.aws.amazon.com/AmazonS3/latest/API/API_CreateBucket.html\">CreateBucket</a>\n               </p>\n            </li>\n         </ul>",
                 "smithy.api#http": {
                     "method": "GET",

diff --git a/aws/sdk/aws-models/ssm-incidents.json b/aws/sdk/aws-models/ssm-incidents.json
@@ -4041,4 +4041,4 @@
             }
         }
     }
-}
+}
diff --git a/aws/sdk/examples/batch/Cargo.toml b/aws/sdk/examples/batch/Cargo.toml
@@ -11,4 +11,4 @@ batch = { package = "aws-sdk-batch", path = "../../build/aws-sdk/batch" }
 aws-types = { path = "../../build/aws-sdk/aws-types" }
 tokio = { version = "1", features = ["full"] }
 structopt = { version = "0.3", default-features = false }
-tracing-subscriber = "0.2.18"
+tracing-subscriber = "0.2.18"
diff --git a/aws/sdk/examples/ebs/Cargo.toml b/aws/sdk/examples/ebs/Cargo.toml
@@ -14,4 +14,4 @@ tokio = { version = "1", features = ["full"]}
 base64 = "0.13.0"
 sha2 = "0.9.5"
 structopt = { version = "0.3", default-features = false }
-tracing-subscriber = "0.2.19"
+tracing-subscriber = "0.2.19"
diff --git a/aws/sdk/examples/kinesis/Cargo.toml b/aws/sdk/examples/kinesis/Cargo.toml
@@ -11,4 +11,4 @@ kinesis = { package = "aws-sdk-kinesis", path = "../../build/aws-sdk/kinesis" }
 aws-types = { path = "../../build/aws-sdk/aws-types" }
 tokio = { version = "1", features = ["full"] }
 structopt = { version = "0.3", default-features = false }
-tracing-subscriber = { version = "0.2.16", features = ["fmt"] }
+tracing-subscriber = { version = "0.2.16", features = ["fmt"] }
diff --git a/aws/sdk/examples/medialive/Cargo.toml b/aws/sdk/examples/medialive/Cargo.toml
@@ -11,4 +11,4 @@ medialive = { package = "aws-sdk-medialive", path = "../../build/aws-sdk/mediali
 aws-types = { path = "../../build/aws-sdk/aws-types" }
 tokio = { version = "1", features = ["full"] }
 structopt = { version = "0.3", default-features = false }
-tracing-subscriber = { version = "0.2.16", features = ["fmt"] }
+tracing-subscriber = { version = "0.2.16", features = ["fmt"] }
diff --git a/aws/sdk/examples/mediapackage/Cargo.toml b/aws/sdk/examples/mediapackage/Cargo.toml
@@ -11,4 +11,4 @@ mediapackage = { package = "aws-sdk-mediapackage", path = "../../build/aws-sdk/m
 aws-types = { path = "../../build/aws-sdk/aws-types" }
 tokio = { version = "1", features = ["full"] }
 structopt = { version = "0.3", default-features = false }
-tracing-subscriber = { version = "0.2.16", features = ["fmt"] }
+tracing-subscriber = { version = "0.2.16", features = ["fmt"] }
diff --git a/aws/sdk/examples/polly/Cargo.toml b/aws/sdk/examples/polly/Cargo.toml
@@ -12,4 +12,3 @@ aws-types = { path = "../../build/aws-sdk/aws-types" }
 tokio = { version = "1", features = ["full"] }
 structopt = { version = "0.3", default-features = false }
 tracing-subscriber = { version = "0.2.16", features = ["fmt"] }
-
diff --git a/aws/sdk/examples/qldb/Cargo.toml b/aws/sdk/examples/qldb/Cargo.toml
@@ -13,5 +13,3 @@ aws-types = { path = "../../build/aws-sdk/aws-types" }
 tokio = { version = "1", features = ["full"] }
 structopt = { version = "0.3", default-features = false }
 tracing-subscriber = { version = "0.2.16", features = ["fmt"] }
-
-
diff --git a/aws/sdk/examples/qldb/README.md b/aws/sdk/examples/qldb/README.md
@@ -51,4 +51,4 @@ where:
   If the environment variable is not set, defaults to **us-west-2**.
 - __-v__ enables displaying additional information.
 
-## 
+##
diff --git a/aws/sdk/examples/rds/Cargo.toml b/aws/sdk/examples/rds/Cargo.toml
@@ -11,4 +11,4 @@ rds = {package = "aws-sdk-rds", path = "../../build/aws-sdk/rds"}
 aws-types = { path = "../../build/aws-sdk/aws-types" }
 tokio = {version = "1", features = ["full"]}
 structopt = { version = "0.3", default-features = false }
-tracing-subscriber = { version = "0.2.16", features = ["fmt"] }
+tracing-subscriber = { version = "0.2.16", features = ["fmt"] }
diff --git a/aws/sdk/examples/rdsdata/Cargo.toml b/aws/sdk/examples/rdsdata/Cargo.toml
@@ -11,4 +11,4 @@ rdsdata = {package = "aws-sdk-rdsdata", path = "../../build/aws-sdk/rdsdata"}
 aws-types = { path = "../../build/aws-sdk/aws-types" }
 tokio = {version = "1", features = ["full"]}
 structopt = { version = "0.3", default-features = false }
-tracing-subscriber = { version = "0.2.16", features = ["fmt"] }
+tracing-subscriber = { version = "0.2.16", features = ["fmt"] }
diff --git a/aws/sdk/examples/sagemaker/Cargo.toml b/aws/sdk/examples/sagemaker/Cargo.toml
@@ -15,4 +15,4 @@ tokio = { version = "1", features = ["full"] }
 env_logger = "0.8.2"
 chrono = "0.4.19"
 structopt = { version = "0.3", default-features = false }
-tracing-subscriber = "0.2.18"
+tracing-subscriber = "0.2.18"
diff --git a/aws/sdk/examples/secretsmanager/Cargo.toml b/aws/sdk/examples/secretsmanager/Cargo.toml
@@ -14,4 +14,3 @@ tokio = { version = "1", features = ["full"]}
 
 structopt = { version = "0.3", default-features = false }
 tracing-subscriber = { version = "0.2.16", features = ["fmt"] }
-
diff --git a/aws/sdk/examples/snowball/Cargo.toml b/aws/sdk/examples/snowball/Cargo.toml
@@ -11,4 +11,4 @@ aws-sdk-snowball = { path = "../../build/aws-sdk/snowball" }
 aws-types = { path = "../../build/aws-sdk/aws-types" }
 tokio = { version = "1", features = ["full"] }
 structopt = { version = "0.3", default-features = false }
-tracing-subscriber = "0.2.18"
+tracing-subscriber = "0.2.18"
diff --git a/aws/sdk/examples/sqs/Cargo.toml b/aws/sdk/examples/sqs/Cargo.toml
@@ -9,4 +9,4 @@ edition = "2018"
 [dependencies]
 sqs = { package = "aws-sdk-sqs", path = "../../build/aws-sdk/sqs" }
 tokio = { version = "1", features = ["full"] }
-tracing-subscriber = "0.2.18"
+tracing-subscriber = "0.2.18"
diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/RustTypes.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/RustTypes.kt
@@ -8,6 +8,17 @@ package software.amazon.smithy.rust.codegen.rustlang
 import software.amazon.smithy.rust.codegen.smithy.RuntimeType
 import software.amazon.smithy.rust.codegen.util.dq
 
+/**
+ * Dereference [input]
+ *
+ * Clippy is upset about `*&`, so if [input] is already referenced, simply strip the leading '&'
+ */
+fun autoDeref(input: String) = if (input.startsWith("&")) {
+    input.removePrefix("&")
+} else {
+    "*$input"
+}
+
 /**
  * A hierarchy of types handled by Smithy codegen
  */

diff --git a/...kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolTestGenerator.kt b/...kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolTestGenerator.kt
@@ -7,6 +7,8 @@ package software.amazon.smithy.rust.codegen.smithy.generators
 
 import software.amazon.smithy.codegen.core.CodegenException
 import software.amazon.smithy.model.knowledge.OperationIndex
+import software.amazon.smithy.model.shapes.DoubleShape
+import software.amazon.smithy.model.shapes.FloatShape
 import software.amazon.smithy.model.shapes.OperationShape
 import software.amazon.smithy.model.shapes.StructureShape
 import software.amazon.smithy.model.traits.ErrorTrait
@@ -306,7 +308,21 @@ class HttpProtocolTestGenerator(
                                     );"""
                     )
                 } else {
-                    rust("""assert_eq!(parsed.$memberName, expected_output.$memberName, "Unexpected value for `$memberName`");""")
+                    when (protocolConfig.model.expectShape(member.target)) {
+                        is DoubleShape, is FloatShape -> {
+                            addUseImports(
+                                RuntimeType.ProtocolTestHelper(protocolConfig.runtimeConfig, "FloatEquals").toSymbol()
+                            )
+                            rust(
+                                """
+                                assert!(parsed.$memberName.float_equals(&expected_output.$memberName),
+                                    "Unexpected value for `$memberName` {:?} vs. {:?}", expected_output.$memberName, parsed.$memberName);
+                                """
+                            )
+                        }
+                        else ->
+                            rust("""assert_eq!(parsed.$memberName, expected_output.$memberName, "Unexpected value for `$memberName`");""")
+                    }
                 }
             }
         }
@@ -428,7 +444,11 @@ class HttpProtocolTestGenerator(
         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>(
+            FailingTest(
+                service = RestJson, id = "RestJsonHostWithPath", action = Action.Request
+            )
+        )
         private val RunOnly: Set<String>? = null
 
         // These tests are not even attempted to be generated, either because they will not compile

diff --git a/...gen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/Instantiator.kt b/...gen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/Instantiator.kt
@@ -114,7 +114,18 @@ class Instantiator(
 
             // Simple Shapes
             is StringShape -> renderString(writer, shape, arg as StringNode)
-            is NumberShape -> writer.write(arg.asNumberNode().get())
+            is NumberShape -> when (arg) {
+                is StringNode -> {
+                    val numberSymbol = symbolProvider.toSymbol(shape)
+                    // support Smithy custom values, such as Infinity
+                    writer.rust(
+                        """<#T as #T>::parse_smithy_primitive(${arg.value.dq()}).expect("invalid string for number")""",
+                        numberSymbol,
+                        CargoDependency.SmithyTypes(runtimeConfig).asType().member("primitive::Parse")
+                    )
+                }
+                is NumberNode -> writer.write(arg.value)
+            }
             is BooleanShape -> writer.write(arg.asBooleanNode().get().toString())
             is DocumentShape -> writer.rustBlock("") {
                 val smithyJson = CargoDependency.smithyJson(runtimeConfig).asType()
-Original file line number
+Diff line change
@@ Expand Up / @@ -4823,4 +4823,4 @@ @@
                 }
             }
         }
-    }
+    }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -12,4 +12,3 @@ aws-types = { path = "../../build/aws-sdk/aws-types" }
		tokio = { version = "1", features = ["full"] }
		structopt = { version = "0.3", default-features = false }
		tracing-subscriber = { version = "0.2.16", features = ["fmt"] }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -13,5 +13,3 @@ aws-types = { path = "../../build/aws-sdk/aws-types" }
		tokio = { version = "1", features = ["full"] }
		structopt = { version = "0.3", default-features = false }
		tracing-subscriber = { version = "0.2.16", features = ["fmt"] }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -14,4 +14,3 @@ tokio = { version = "1", features = ["full"]}

		structopt = { version = "0.3", default-features = false }
		tracing-subscriber = { version = "0.2.16", features = ["fmt"] }