From a0c7ca360dc0585f838ba1beb14dc9189b9583ed Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Wed, 26 Jan 2022 14:13:27 -0500 Subject: [PATCH 1/3] Remove unneeded doc parser --- .../http/ServerRequestBindingGenerator.kt | 5 +--- .../protocols/ServerHttpProtocolGenerator.kt | 7 ------ .../generators/http/HttpBindingGenerator.kt | 13 ++++------ .../http/ResponseBindingGenerator.kt | 6 ++--- .../protocols/HttpBoundProtocolGenerator.kt | 12 ++-------- .../smithy/protocols/InlineFunctionNamer.kt | 2 ++ .../protocols/parse/JsonParserGenerator.kt | 24 +------------------ .../parse/StructuredDataParserGenerator.kt | 9 ------- .../parse/XmlBindingTraitParserGenerator.kt | 4 ---- .../parse/JsonParserGeneratorTest.kt | 2 -- 10 files changed, 12 insertions(+), 72 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerRequestBindingGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerRequestBindingGenerator.kt index 8fe477e6b2..7d5279a133 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerRequestBindingGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerRequestBindingGenerator.kt @@ -5,7 +5,6 @@ package software.amazon.smithy.rust.codegen.server.smithy.generators.http - import software.amazon.smithy.model.shapes.OperationShape import software.amazon.smithy.rust.codegen.rustlang.RustWriter import software.amazon.smithy.rust.codegen.smithy.CodegenContext @@ -29,14 +28,12 @@ class ServerRequestBindingGenerator( operationShape: OperationShape, binding: HttpBindingDescriptor, errorT: RuntimeType, - structuredHandler: RustWriter.(String) -> Unit, - docHandler: RustWriter.(String) -> Unit + structuredHandler: RustWriter.(String) -> Unit ): RuntimeType = httpBindingGenerator.generateDeserializePayloadFn( operationShape, binding, errorT, structuredHandler, - docHandler, HttpMessageType.REQUEST ) } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpProtocolGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpProtocolGenerator.kt index da31f42c76..6a217d7087 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpProtocolGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpProtocolGenerator.kt @@ -561,12 +561,6 @@ private class ServerHttpProtocolImplGenerator( return when (binding.location) { HttpLocation.HEADER -> writable { serverRenderHeaderParser(this, binding, operationShape) } HttpLocation.PAYLOAD -> { - val docShapeHandler: RustWriter.(String) -> Unit = { body -> - rust( - "#T($body)", - structuredDataParser.documentParser(operationShape), - ) - } val structureShapeHandler: RustWriter.(String) -> Unit = { body -> rust("#T($body)", structuredDataParser.payloadParser(binding.member)) } @@ -574,7 +568,6 @@ private class ServerHttpProtocolImplGenerator( operationShape, binding, errorSymbol, - docHandler = docShapeHandler, structuredHandler = structureShapeHandler ) return if (binding.member.isStreaming(model)) { diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/http/HttpBindingGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/http/HttpBindingGenerator.kt index d15c921b40..463b9b2fcd 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/http/HttpBindingGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/http/HttpBindingGenerator.kt @@ -160,10 +160,8 @@ class HttpBindingGenerator( operationShape: OperationShape, binding: HttpBindingDescriptor, errorT: RuntimeType, - // Deserialize a single structure or union member marked as a payload - structuredHandler: RustWriter.(String) -> Unit, - // Deserialize a document type marked as a payload - docHandler: RustWriter.(String) -> Unit, + // Deserialize a single structure, union or document member marked as a payload + payloadParser: RustWriter.(String) -> Unit, httpMessageType: HttpMessageType = HttpMessageType.RESPONSE ): RuntimeType { check(binding.location == HttpBinding.Location.PAYLOAD) @@ -190,8 +188,7 @@ class HttpBindingGenerator( deserializePayloadBody( binding, errorT, - structuredHandler = structuredHandler, - docShapeHandler = docHandler, + structuredHandler = payloadParser, httpMessageType ) } @@ -239,7 +236,6 @@ class HttpBindingGenerator( binding: HttpBindingDescriptor, errorSymbol: RuntimeType, structuredHandler: RustWriter.(String) -> Unit, - docShapeHandler: RustWriter.(String) -> Unit, httpMessageType: HttpMessageType = HttpMessageType.RESPONSE ) { val member = binding.member @@ -248,7 +244,7 @@ class HttpBindingGenerator( // of an empty instance of the response type. withBlock("(!body.is_empty()).then(||{", "}).transpose()") { when (targetShape) { - is StructureShape, is UnionShape -> this.structuredHandler("body") + is StructureShape, is UnionShape, is DocumentShape -> this.structuredHandler("body") is StringShape -> { when (httpMessageType) { HttpMessageType.RESPONSE -> { @@ -274,7 +270,6 @@ class HttpBindingGenerator( "Ok(#T::new(body))", RuntimeType.Blob(runtimeConfig) ) - is DocumentShape -> this.docShapeHandler("body") // `httpPayload` can be applied to set/map/list shapes. // However, none of the AWS protocols support it. // Smithy CLI will refuse to build the model if you apply the trait to these shapes, so this branch diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/http/ResponseBindingGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/http/ResponseBindingGenerator.kt index cf607d6ee4..a17814b0ed 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/http/ResponseBindingGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/http/ResponseBindingGenerator.kt @@ -29,13 +29,11 @@ class ResponseBindingGenerator( operationShape: OperationShape, binding: HttpBindingDescriptor, errorT: RuntimeType, - structuredHandler: RustWriter.(String) -> Unit, - docHandler: RustWriter.(String) -> Unit + payloadParser: RustWriter.(String) -> Unit ): RuntimeType = httpBindingGenerator.generateDeserializePayloadFn( operationShape, binding, errorT, - structuredHandler, - docHandler + payloadParser ) } diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/HttpBoundProtocolGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/HttpBoundProtocolGenerator.kt index ce8d32d273..32ebca01db 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/HttpBoundProtocolGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/HttpBoundProtocolGenerator.kt @@ -361,22 +361,14 @@ class HttpBoundProtocolTraitImplGenerator( null } HttpLocation.PAYLOAD -> { - val docShapeHandler: RustWriter.(String) -> Unit = { body -> - rust( - "#T($body).map_err(#T::unhandled)", - structuredDataParser.documentParser(operationShape), - errorSymbol - ) - } - val structureShapeHandler: RustWriter.(String) -> Unit = { body -> + val payloadParser: RustWriter.(String) -> Unit = { body -> rust("#T($body).map_err(#T::unhandled)", structuredDataParser.payloadParser(member), errorSymbol) } val deserializer = httpBindingGenerator.generateDeserializePayloadFn( operationShape, binding, errorSymbol, - docHandler = docShapeHandler, - structuredHandler = structureShapeHandler + payloadParser = payloadParser ) return if (binding.member.isStreaming(model)) { writable { rust("Some(#T(response.body_mut())?)", deserializer) } diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/InlineFunctionNamer.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/InlineFunctionNamer.kt index 0f43eb11c6..86ef21d292 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/InlineFunctionNamer.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/InlineFunctionNamer.kt @@ -5,6 +5,7 @@ package software.amazon.smithy.rust.codegen.smithy.protocols +import software.amazon.smithy.model.shapes.DocumentShape import software.amazon.smithy.model.shapes.ListShape import software.amazon.smithy.model.shapes.MapShape import software.amazon.smithy.model.shapes.MemberShape @@ -66,6 +67,7 @@ private fun RustSymbolProvider.shapeFunctionName(prefix: String, shape: Shape): is SetShape -> "set_${shape.id.toRustIdentifier()}" is StructureShape -> "structure_$symbolNameSnakeCase" is UnionShape -> "union_$symbolNameSnakeCase" + is DocumentShape -> "document" else -> PANIC("SerializerFunctionNamer.name: $shape") } } diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/JsonParserGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/JsonParserGenerator.kt index 040781e566..829800f7d2 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/JsonParserGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/JsonParserGenerator.kt @@ -118,7 +118,7 @@ class JsonParserGenerator( override fun payloadParser(member: MemberShape): RuntimeType { val shape = model.expectShape(member.target) - check(shape is UnionShape || shape is StructureShape) { "payload parser should only be used on structures & unions" } + check(shape is UnionShape || shape is StructureShape || shape is DocumentShape) { "payload parser should only be used on structures & unions" } val fnName = symbolProvider.deserializeFunctionName(shape) + "_payload" return RuntimeType.forInlineFun(fnName, jsonDeserModule) { it.rustBlockTemplate( @@ -161,28 +161,6 @@ class JsonParserGenerator( return structureParser(fnName, errorShape, errorShape.members().toList()) } - override fun documentParser(operationShape: OperationShape): RuntimeType { - val fnName = "parse_document" - return RuntimeType.forInlineFun(fnName, jsonDeserModule) { - it.rustBlockTemplate( - "pub fn $fnName(input: &[u8]) -> Result<#{Document}, #{Error}>", - "Document" to RuntimeType.Document(runtimeConfig), - *codegenScope, - ) { - rustTemplate( - """ - let mut tokens_owned = #{json_token_iter}(input).peekable(); - let tokens = &mut tokens_owned; - """, - *codegenScope - ) - rustTemplate("let result = #{expect_document}(tokens);", *codegenScope) - expectEndOfTokenStream() - rust("result") - } - } - } - private fun orEmptyJson(): RuntimeType = RuntimeType.forInlineFun("or_empty_doc", jsonDeserModule) { it.rust( """ diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/StructuredDataParserGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/StructuredDataParserGenerator.kt index 07029d8a0b..3f1c767e4b 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/StructuredDataParserGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/StructuredDataParserGenerator.kt @@ -47,15 +47,6 @@ interface StructuredDataParserGenerator { */ fun errorParser(errorShape: StructureShape): RuntimeType? - /** - * ```rust - * fn parse_document(inp: &[u8]) -> Result { - * ... - * } - * ``` - */ - fun documentParser(operationShape: OperationShape): RuntimeType - /** * Generate a parser for a server operation input structure * ```rust diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/XmlBindingTraitParserGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/XmlBindingTraitParserGenerator.kt index bdb5759e88..1b85992c28 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/XmlBindingTraitParserGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/XmlBindingTraitParserGenerator.kt @@ -241,10 +241,6 @@ class XmlBindingTraitParserGenerator( } } - override fun documentParser(operationShape: OperationShape): RuntimeType { - PANIC("Document shapes are not supported by rest XML") - } - override fun serverInputParser(operationShape: OperationShape): RuntimeType? { val inputShape = operationShape.inputShape(model) val fnName = symbolProvider.deserializeFunctionName(operationShape) diff --git a/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/JsonParserGeneratorTest.kt b/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/JsonParserGeneratorTest.kt index f27fa1b9b8..16adf7d43f 100644 --- a/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/JsonParserGeneratorTest.kt +++ b/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/JsonParserGeneratorTest.kt @@ -120,7 +120,6 @@ class JsonParserGeneratorTest { ::restJsonFieldName ) val operationGenerator = parserGenerator.operationParser(model.lookup("test#Op")) - val documentGenerator = parserGenerator.documentParser(model.lookup("test#Op")) val payloadGenerator = parserGenerator.payloadParser(model.lookup("test#OpOutput\$top")) val errorParser = parserGenerator.errorParser(model.lookup("test#Error")) @@ -132,7 +131,6 @@ class JsonParserGeneratorTest { use model::Choice; // Generate the document serializer even though it's not tested directly - // ${writer.format(documentGenerator)} // ${writer.format(payloadGenerator)} let json = br#" From a052d9d9a8dc10c7bb9d01502d34e1d86652e366 Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Wed, 26 Jan 2022 15:51:53 -0500 Subject: [PATCH 2/3] Maintain identical behavior for empty documents --- .../smithy/protocols/parse/JsonParserGenerator.kt | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/JsonParserGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/JsonParserGenerator.kt index 829800f7d2..04210739de 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/JsonParserGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/JsonParserGenerator.kt @@ -126,9 +126,15 @@ class JsonParserGenerator( *codegenScope, "Shape" to symbolProvider.toSymbol(shape) ) { + val input = if (shape is DocumentShape) { + "input" + } else { + "#{or_empty}(input)" + } + rustTemplate( """ - let mut tokens_owned = #{json_token_iter}(#{or_empty}(input)).peekable(); + let mut tokens_owned = #{json_token_iter}($input).peekable(); let tokens = &mut tokens_owned; """, *codegenScope @@ -363,8 +369,9 @@ class JsonParserGenerator( withBlock("Ok(Some(builder.build()", "))") { if (StructureGenerator.fallibleBuilder(shape, symbolProvider)) { rustTemplate( - ".map_err(|err| #{Error}::new(#{ErrorReason}::Custom(" + - "format!(\"{}\", err).into()), None))?", + """.map_err(|err| #{Error}::new( + #{ErrorReason}::Custom(format!({}, err).into()), None) + )?""", *codegenScope ) } From bf9b14bc8e0ff57699591c3e24d8080715932bea Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Wed, 26 Jan 2022 17:36:23 -0500 Subject: [PATCH 3/3] Changelog --- CHANGELOG.next.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 72d79628e7..6482da194b 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -10,3 +10,8 @@ # references = ["smithy-rs#920"] # meta = { "breaking" = false, "tada" = false, "bug" = false } # author = "rcoh" +[[smithy-rs]] +message = "Refactor `Document` shape parser generation" +references = ["smithy-rs#1123"] +meta = { "breaking" = false, "tada" = false, "bug" = false } +author = "rcoh"