Finish the JSON serialization refactor (#423)

* Only include document bound members in HTTP operations * Add rest-json-extras test for maps with an enum key * Fix serialization of maps with enum keys * Replace SerdeJsonSerializerGenerator with JsonSerializerGenerator * Suppress clippy warning * CR feedback
smithy-lang · May 26, 2021 · 281ee44 · 281ee44
1 parent d79e80c
commit 281ee44
Show file tree

Hide file tree

Showing 11 changed files with 99 additions and 341 deletions.
diff --git a/codegen-test/model/rest-json-extras.smithy b/codegen-test/model/rest-json-extras.smithy
@@ -53,7 +53,7 @@ apply QueryPrecedence @httpRequestTests([
 @restJson1
 service RestJsonExtras {
     version: "2019-12-16",
-    operations: [StringPayload, PrimitiveIntHeader, EnumQuery, StatusResponse]
+    operations: [StringPayload, PrimitiveIntHeader, EnumQuery, StatusResponse, MapWithEnumKeyOp]
 }
 
 @http(uri: "/StringPayload", method: "POST")
@@ -135,3 +135,37 @@ structure StatusOutput {
     @httpResponseCode
     field: PrimitiveInt
 }
+
+map MapWithEnumKey {
+    key: StringEnum,
+    value: String,
+}
+
+structure MapWithEnumKeyInputOutput {
+    map: MapWithEnumKey,
+}
+
+@http(uri: "/map-with-enum-key", method: "POST")
+@httpRequestTests([
+    {
+        id: "MapWithEnumKeyRequest",
+        uri: "/map-with-enum-key",
+        method: "POST",
+        protocol: "aws.protocols#restJson1",
+        body: "{\"map\":{\"enumvalue\":\"something\"}}",
+        params: { map: { "enumvalue": "something" } }
+    },
+])
+@httpResponseTests([
+    {
+        id: "MapWithEnumKeyResponse",
+        protocol: "aws.protocols#restJson1",
+        code: 200,
+        body: "{\"map\":{\"enumvalue\":\"something\"}}",
+        params: { map: { "enumvalue": "something" } },
+    },
+])
+operation MapWithEnumKeyOp {
+    input: MapWithEnumKeyInputOutput,
+    output: MapWithEnumKeyInputOutput,
+}
diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RuntimeTypes.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RuntimeTypes.kt
@@ -140,9 +140,7 @@ data class RuntimeType(val name: String?, val dependency: RustDependency?, val n
             path, dependency = CargoDependency.Serde, namespace = "serde"
         )
 
-        val Serialize = RuntimeType("Serialize", CargoDependency.Serde, namespace = "serde")
         val Deserialize: RuntimeType = RuntimeType("Deserialize", CargoDependency.Serde, namespace = "serde")
-        val Serializer = RuntimeType("Serializer", CargoDependency.Serde, namespace = "serde")
         val Deserializer = RuntimeType("Deserializer", CargoDependency.Serde, namespace = "serde")
         fun SerdeJson(path: String) =
             RuntimeType(path, dependency = CargoDependency.SerdeJson, namespace = "serde_json")

diff --git a/...en/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/EnumGenerator.kt b/...en/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/EnumGenerator.kt
@@ -176,21 +176,13 @@ class EnumGenerator(
     private fun renderSerde() {
         writer.rustTemplate(
             """
-                impl #{serialize} for $enumName {
-                    fn serialize<S>(&self, serializer: S) -> Result<<S as #{serializer}>::Ok, <S as #{serializer}>::Error> where S: #{serializer}{
-                        serializer.serialize_str(self.as_str())
-                    }
-                }
-
                 impl<'de> #{deserialize}<'de> for $enumName {
                     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error> where D: #{deserializer}<'de> {
                         let data = <&str>::deserialize(deserializer)?;
                         Ok(Self::from(data))
                     }
                 }
             """,
-            "serializer" to RuntimeType.Serializer,
-            "serialize" to RuntimeType.Serialize,
             "deserializer" to RuntimeType.Deserializer,
             "deserialize" to RuntimeType.Deserialize
         )

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
@@ -44,7 +44,7 @@ import software.amazon.smithy.rust.codegen.smithy.letIf
 import software.amazon.smithy.rust.codegen.smithy.rustType
 import software.amazon.smithy.rust.codegen.util.dq
 import software.amazon.smithy.rust.codegen.util.expectMember
-import software.amazon.smithy.rust.codegen.util.getTrait
+import software.amazon.smithy.rust.codegen.util.hasTrait
 import software.amazon.smithy.rust.codegen.util.isStreaming
 import software.amazon.smithy.rust.codegen.util.toPascalCase
 
@@ -126,12 +126,7 @@ class Instantiator(
      * If the shape is optional: `Some(inner)` or `None`
      * otherwise: `inner`
      */
-    private fun renderMember(
-        writer: RustWriter,
-        shape: MemberShape,
-        arg: Node,
-        ctx: Ctx
-    ) {
+    private fun renderMember(writer: RustWriter, shape: MemberShape, arg: Node, ctx: Ctx) {
         val target = model.expectShape(shape.target)
         val symbol = symbolProvider.toSymbol(shape)
         if (arg is NullNode) {
@@ -176,28 +171,24 @@ class Instantiator(
      *      ret
      * }
      */
-    private fun renderMap(
-        writer: RustWriter,
-        shape: MapShape,
-        data: ObjectNode,
-        ctx: Ctx,
-    ) {
-        val lowercase = when (ctx.lowercaseMapKeys) {
-            true -> ".to_ascii_lowercase()"
-            else -> ""
-        }
-        if (data.members.isNotEmpty()) {
+    private fun renderMap(writer: RustWriter, shape: MapShape, data: ObjectNode, ctx: Ctx) {
+        if (data.members.isEmpty()) {
+            writer.write("#T::new()", RustType.HashMap.RuntimeType)
+        } else {
             writer.rustBlock("") {
                 write("let mut ret = #T::new();", RustType.HashMap.RuntimeType)
-                data.members.forEach { (k, v) ->
-                    withBlock("ret.insert(${k.value.dq()}.to_string()$lowercase,", ");") {
-                        renderMember(this, shape.value, v, ctx)
+                for ((key, value) in data.members) {
+                    withBlock("ret.insert(", ");") {
+                        renderMember(this, shape.key, key, ctx)
+                        when (ctx.lowercaseMapKeys) {
+                            true -> rust(".to_ascii_lowercase(), ")
+                            else -> rust(", ")
+                        }
+                        renderMember(this, shape.value, value, ctx)
                     }
                 }
                 write("ret")
             }
-        } else {
-            writer.write("#T::new()", RustType.HashMap.RuntimeType)
         }
     }
 
@@ -206,12 +197,7 @@ class Instantiator(
      * MyUnion::Variant(...)
      * ```
      */
-    private fun renderUnion(
-        writer: RustWriter,
-        shape: UnionShape,
-        data: ObjectNode,
-        ctx: Ctx
-    ) {
+    private fun renderUnion(writer: RustWriter, shape: UnionShape, data: ObjectNode, ctx: Ctx) {
         val unionSymbol = symbolProvider.toSymbol(shape)
         check(data.members.size == 1)
         val variant = data.members.iterator().next()
@@ -230,12 +216,7 @@ class Instantiator(
      * vec![..., ..., ...]
      * ```
      */
-    private fun renderList(
-        writer: RustWriter,
-        shape: CollectionShape,
-        data: ArrayNode,
-        ctx: Ctx
-    ) {
+    private fun renderList(writer: RustWriter, shape: CollectionShape, data: ArrayNode, ctx: Ctx) {
         writer.withBlock("vec![", "]") {
             data.elements.forEach { v ->
                 renderMember(this, shape.member, v, ctx)
@@ -244,14 +225,9 @@ class Instantiator(
         }
     }
 
-    private fun renderString(
-        writer: RustWriter,
-        shape: StringShape,
-        arg: StringNode
-    ) {
-        val enumTrait = shape.getTrait<EnumTrait>()
+    private fun renderString(writer: RustWriter, shape: StringShape, arg: StringNode) {
         val data = writer.escape(arg.value).dq()
-        if (enumTrait == null) {
+        if (!shape.hasTrait<EnumTrait>()) {
             writer.rust("$data.to_string()")
         } else {
             val enumSymbol = symbolProvider.toSymbol(shape)
@@ -264,12 +240,7 @@ class Instantiator(
      * MyStruct::builder().field_1("hello").field_2(5).build()
      * ```
      */
-    private fun renderStructure(
-        writer: RustWriter,
-        shape: StructureShape,
-        data: ObjectNode,
-        ctx: Ctx
-    ) {
+    private fun renderStructure(writer: RustWriter, shape: StructureShape, data: ObjectNode, ctx: Ctx) {
         writer.write("#T::builder()", symbolProvider.toSymbol(shape))
         data.members.forEach { (key, value) ->
             val memberShape = shape.expectMember(key.value)

diff --git a/...ust/codegen/smithy/protocols/AwsJson10.kt → .../rust/codegen/smithy/protocols/AwsJson.kt b/...ust/codegen/smithy/protocols/AwsJson10.kt → .../rust/codegen/smithy/protocols/AwsJson.kt
@@ -36,8 +36,8 @@ import software.amazon.smithy.rust.codegen.smithy.generators.error.errorSymbol
 import software.amazon.smithy.rust.codegen.smithy.generators.operationBuildError
 import software.amazon.smithy.rust.codegen.smithy.locatedIn
 import software.amazon.smithy.rust.codegen.smithy.meta
+import software.amazon.smithy.rust.codegen.smithy.protocols.parsers.JsonSerializerGenerator
 import software.amazon.smithy.rust.codegen.smithy.protocols.parsers.SerdeJsonParserGenerator
-import software.amazon.smithy.rust.codegen.smithy.protocols.parsers.SerdeJsonSerializerGenerator
 import software.amazon.smithy.rust.codegen.smithy.rustType
 import software.amazon.smithy.rust.codegen.smithy.traits.InputBodyTrait
 import software.amazon.smithy.rust.codegen.smithy.traits.OutputBodyTrait
@@ -198,7 +198,7 @@ class BasicAwsJsonGenerator(
     }
 
     override fun RustWriter.body(self: String, operationShape: OperationShape): BodyMetadata {
-        val generator = SerdeJsonSerializerGenerator(protocolConfig)
+        val generator = JsonSerializerGenerator(protocolConfig)
         val serializer = generator.operationSerializer(operationShape)
         serializer?.also { sym ->
             rustTemplate(

diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/AwsRestJson.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/AwsRestJson.kt
@@ -18,8 +18,8 @@ import software.amazon.smithy.rust.codegen.smithy.RustSymbolProvider
 import software.amazon.smithy.rust.codegen.smithy.generators.ProtocolConfig
 import software.amazon.smithy.rust.codegen.smithy.generators.ProtocolGeneratorFactory
 import software.amazon.smithy.rust.codegen.smithy.generators.ProtocolSupport
+import software.amazon.smithy.rust.codegen.smithy.protocols.parsers.JsonSerializerGenerator
 import software.amazon.smithy.rust.codegen.smithy.protocols.parsers.SerdeJsonParserGenerator
-import software.amazon.smithy.rust.codegen.smithy.protocols.parsers.SerdeJsonSerializerGenerator
 import software.amazon.smithy.rust.codegen.smithy.protocols.parsers.StructuredDataParserGenerator
 import software.amazon.smithy.rust.codegen.smithy.protocols.parsers.StructuredDataSerializerGenerator
 import software.amazon.smithy.rust.codegen.smithy.transformers.OperationNormalizer
@@ -81,7 +81,7 @@ class RestJson(private val protocolConfig: ProtocolConfig) : Protocol {
     }
 
     override fun structuredDataSerializer(operationShape: OperationShape): StructuredDataSerializerGenerator {
-        return SerdeJsonSerializerGenerator(protocolConfig)
+        return JsonSerializerGenerator(protocolConfig)
     }
 
     override fun parseGenericError(operationShape: OperationShape): RuntimeType {