From bdf210d4f3c787f466a8770dcf31de5ec65aabc3 Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Tue, 29 Jun 2021 16:53:23 -0700 Subject: [PATCH 1/2] Backfill message & parse without case sensitivity --- codegen-test/build.gradle.kts | 27 +++++++++---- codegen-test/model/rest-json-extras.smithy | 21 ++++++++++ .../rust/codegen/smithy/CodegenVisitor.kt | 5 ++- .../rust/codegen/smithy/RustSettings.kt | 15 ++++++- .../generators/HttpProtocolTestGenerator.kt | 2 + .../smithy/generators/error/ErrorGenerator.kt | 8 ++-- .../protocols/HttpBoundProtocolGenerator.kt | 39 +++++++++++++++---- .../smithy/transformers/AddErrorMessage.kt | 28 +++++++++++++ 8 files changed, 123 insertions(+), 22 deletions(-) create mode 100644 codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/transformers/AddErrorMessage.kt diff --git a/codegen-test/build.gradle.kts b/codegen-test/build.gradle.kts index b1121e9a5a..f815edecac 100644 --- a/codegen-test/build.gradle.kts +++ b/codegen-test/build.gradle.kts @@ -30,21 +30,34 @@ val CodegenTests = listOf( CodegenTest("aws.protocoltests.json10#JsonRpc10", "json_rpc10"), CodegenTest("aws.protocoltests.json#JsonProtocol", "json_rpc11"), CodegenTest("aws.protocoltests.restjson#RestJson", "rest_json"), - CodegenTest("aws.protocoltests.restjson#RestJsonExtras", "rest_json_extas"), - CodegenTest("aws.protocoltests.restxml#RestXml", "rest_xml"), - CodegenTest("aws.protocoltests.query#AwsQuery", "aws_query"), - CodegenTest("aws.protocoltests.ec2#AwsEc2", "ec2_query"), + CodegenTest("aws.protocoltests.restjson#RestJsonExtras", "rest_json_extras"), + CodegenTest( + "aws.protocoltests.restxml#RestXml", "rest_xml", + extraConfig = """, "codegen": { "addMessageToErrors": false } """ + ), + + CodegenTest( + "aws.protocoltests.query#AwsQuery", "aws_query", + extraConfig = """, "codegen": { "addMessageToErrors": false } """ + ), + CodegenTest( + "aws.protocoltests.ec2#AwsEc2", "ec2_query", + extraConfig = """, "codegen": { "addMessageToErrors": false } """ + ), CodegenTest( "aws.protocoltests.restxml.xmlns#RestXmlWithNamespace", - "rest_xml_namespace" + "rest_xml_namespace", + extraConfig = """, "codegen": { "addMessageToErrors": false } """ ), CodegenTest( "aws.protocoltests.restxml#RestXmlExtras", - "rest_xml_extras" + "rest_xml_extras", + extraConfig = """, "codegen": { "addMessageToErrors": false } """ ), CodegenTest( "aws.protocoltests.restxmlunwrapped#RestXmlExtrasUnwrappedErrors", - "rest_xml_extras_unwrapped" + "rest_xml_extras_unwrapped", + extraConfig = """, "codegen": { "addMessageToErrors": false } """ ), CodegenTest( "crate#Config", diff --git a/codegen-test/model/rest-json-extras.smithy b/codegen-test/model/rest-json-extras.smithy index 41e76b158e..92ef9be49d 100644 --- a/codegen-test/model/rest-json-extras.smithy +++ b/codegen-test/model/rest-json-extras.smithy @@ -62,6 +62,7 @@ service RestJsonExtras { PrimitiveIntOp, EscapedStringValues, NullInNonSparse, + CaseInsensitiveErrorOperation ] } @@ -268,3 +269,23 @@ structure NullInNonSparseOutput { operation NullInNonSparse { output: NullInNonSparseOutput, } + +@http(uri: "/error-sensitive", method: "POST") +operation CaseInsensitiveErrorOperation { + errors: [CaseInsensitiveError] +} + +@httpResponseTests([ + { + id: "Upper case error modeled lower case", + protocol: "aws.protocols#restJson1", + code: 500, + body: "{\"Message\": \"hello\"}", + headers: { "X-Amzn-Errortype": "CaseInsensitiveError" }, + params: { message: "hello" } + } +]) +@error("server") +structure CaseInsensitiveError { + message: String +} diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/CodegenVisitor.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/CodegenVisitor.kt index d86bee70ae..fa89bab71f 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/CodegenVisitor.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/CodegenVisitor.kt @@ -27,6 +27,7 @@ import software.amazon.smithy.rust.codegen.smithy.generators.UnionGenerator import software.amazon.smithy.rust.codegen.smithy.generators.implBlock import software.amazon.smithy.rust.codegen.smithy.protocols.ProtocolLoader import software.amazon.smithy.rust.codegen.smithy.traits.SyntheticInputTrait +import software.amazon.smithy.rust.codegen.smithy.transformers.AddErrorMessage import software.amazon.smithy.rust.codegen.smithy.transformers.RecursiveShapeBoxer import software.amazon.smithy.rust.codegen.util.CommandFailed import software.amazon.smithy.rust.codegen.util.getTrait @@ -74,7 +75,9 @@ class CodegenVisitor(context: PluginContext, private val codegenDecorator: RustC httpGenerator = protocolGenerator.buildProtocolGenerator(protocolConfig) } - private fun baselineTransform(model: Model) = model.let(RecursiveShapeBoxer::transform) + private fun baselineTransform(model: Model) = + model.let(RecursiveShapeBoxer::transform) + .letIf(settings.codegenConfig.addMessageToErrors, AddErrorMessage::transform) fun execute() { logger.info("generating Rust client...") diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RustSettings.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RustSettings.kt index 8d56b877a1..f1df744c36 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RustSettings.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RustSettings.kt @@ -28,14 +28,25 @@ private const val RUNTIME_CONFIG = "runtimeConfig" private const val CODEGEN_SETTINGS = "codegen" private const val LICENSE = "license" -data class CodegenConfig(val renameExceptions: Boolean = true, val includeFluentClient: Boolean = true, val formatTimeoutSeconds: Int = 20) { +data class CodegenConfig( + // / Rename `Exception` to `Error` in the generated SDK + val renameExceptions: Boolean = true, + // / Include a fluent client in the generated SDK + val includeFluentClient: Boolean = true, + // / Add a message field to all shapes that have the error trait + val addMessageToErrors: Boolean = true, + // / Timeout when running `cargo fmt` on a generate service + val formatTimeoutSeconds: Int = 20 +) { companion object { fun fromNode(node: Optional): CodegenConfig { return if (node.isPresent) { CodegenConfig( node.get().getBooleanMemberOrDefault("renameErrors", true), node.get().getBooleanMemberOrDefault("includeFluentClient", true), - node.get().getNumberMemberOrDefault("formatTimeoutSeconds", 20).toInt() + node.get().getBooleanMemberOrDefault("addMessageToErrors", true), + node.get().getNumberMemberOrDefault("formatTimeoutSeconds", 20).toInt(), + ) } else { CodegenConfig() diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolTestGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolTestGenerator.kt index 44bd0483dc..22c329a535 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolTestGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolTestGenerator.kt @@ -426,6 +426,8 @@ class HttpProtocolTestGenerator( private val AwsJson11 = "aws.protocoltests.json#JsonProtocol" private val RestJson = "aws.protocoltests.restjson#RestJson" 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() private val RunOnly: Set? = null diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/error/ErrorGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/error/ErrorGenerator.kt index d6a14c303f..9567855a63 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/error/ErrorGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/error/ErrorGenerator.kt @@ -20,6 +20,7 @@ import software.amazon.smithy.rust.codegen.smithy.RuntimeType.Companion.StdError import software.amazon.smithy.rust.codegen.smithy.RuntimeType.Companion.stdfmt import software.amazon.smithy.rust.codegen.smithy.RustSymbolProvider import software.amazon.smithy.rust.codegen.smithy.letIf +import software.amazon.smithy.rust.codegen.smithy.transformers.errorMessageMember import software.amazon.smithy.rust.codegen.util.dq import software.amazon.smithy.rust.codegen.util.getTrait @@ -71,9 +72,8 @@ class ErrorGenerator( private fun renderError() { val symbol = symbolProvider.toSymbol(shape) - val messageShape = - shape.getMember("message").or { shape.getMember("Message") }.or { shape.getMember("errorMessage") } - val message = messageShape.map { "self.${symbolProvider.toMemberName(it)}.as_deref()" }.orElse("None") + val messageShape = shape.errorMessageMember() + val message = messageShape?.let { "self.${symbolProvider.toMemberName(it)}.as_deref()" } ?: "None" val errorKindT = RuntimeType.errorKind(symbolProvider.config().runtimeConfig) writer.rustBlock("impl ${symbol.name}") { val retryKindWriteable = shape.modeledRetryKind(error)?.writable(symbolProvider.config().runtimeConfig) @@ -97,7 +97,7 @@ class ErrorGenerator( "$symbolName [${shape.id.name}]" } write("write!(f, ${errorDesc.dq()})?;") - messageShape.map { + messageShape?.let { ifSet(it, symbolProvider.toSymbol(it), "&self.message") { field -> write("""write!(f, ": {}", $field)?;""") } 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 0b0b554bbc..9830ee03f1 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 @@ -19,7 +19,9 @@ import software.amazon.smithy.model.traits.TimestampFormatTrait import software.amazon.smithy.rust.codegen.rustlang.Attribute import software.amazon.smithy.rust.codegen.rustlang.RustWriter import software.amazon.smithy.rust.codegen.rustlang.Writable +import software.amazon.smithy.rust.codegen.rustlang.assignment import software.amazon.smithy.rust.codegen.rustlang.rust +import software.amazon.smithy.rust.codegen.rustlang.rustBlock import software.amazon.smithy.rust.codegen.rustlang.rustBlockTemplate import software.amazon.smithy.rust.codegen.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.rustlang.withBlock @@ -37,6 +39,7 @@ import software.amazon.smithy.rust.codegen.smithy.generators.setterName import software.amazon.smithy.rust.codegen.smithy.isOptional import software.amazon.smithy.rust.codegen.smithy.protocols.parse.StructuredDataParserGenerator import software.amazon.smithy.rust.codegen.smithy.protocols.serialize.StructuredDataSerializerGenerator +import software.amazon.smithy.rust.codegen.smithy.transformers.errorMessageMember import software.amazon.smithy.rust.codegen.util.dq import software.amazon.smithy.rust.codegen.util.expectMember import software.amazon.smithy.rust.codegen.util.hasStreamingMember @@ -288,7 +291,10 @@ class HttpBoundProtocolGenerator( let error_code = match generic.code() { Some(code) => code, None => return Err(#{error_symbol}::unhandled(generic)) - };""", + }; + + let _error_message = generic.message().map(|msg|msg.to_owned()); + """, "error_symbol" to errorSymbol, ) withBlock("Err(match error_code {", "})") { @@ -296,16 +302,33 @@ class HttpBoundProtocolGenerator( val errorShape = model.expectShape(error, StructureShape::class.java) val variantName = symbolProvider.toSymbol(model.expectShape(error)).name withBlock( - "${httpBindingResolver.errorCode(errorShape).dq()} => #1T { meta: generic, kind: #1TKind::$variantName({", + "${ + httpBindingResolver.errorCode(errorShape).dq() + } => #1T { meta: generic, kind: #1TKind::$variantName({", "})},", errorSymbol ) { - renderShapeParser( - operationShape, - errorShape, - httpBindingResolver.errorResponseBindings(errorShape), - errorSymbol - ) + Attribute.AllowUnusedMut.render(this) + assignment("mut tmp") { + rustBlock("") { + renderShapeParser( + operationShape, + errorShape, + httpBindingResolver.errorResponseBindings(errorShape), + errorSymbol + ) + } + } + if (errorShape.errorMessageMember() != null) { + rust( + """ + if (&tmp.message).is_none() { + tmp.message = _error_message; + } + """ + ) + } + rust("tmp") } } rust("_ => #T::generic(generic)", errorSymbol) diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/transformers/AddErrorMessage.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/transformers/AddErrorMessage.kt new file mode 100644 index 0000000000..71f6fb342d --- /dev/null +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/transformers/AddErrorMessage.kt @@ -0,0 +1,28 @@ +package software.amazon.smithy.rust.codegen.smithy.transformers + +import software.amazon.smithy.model.Model +import software.amazon.smithy.model.shapes.MemberShape +import software.amazon.smithy.model.shapes.ShapeId +import software.amazon.smithy.model.shapes.StructureShape +import software.amazon.smithy.model.traits.ErrorTrait +import software.amazon.smithy.model.transform.ModelTransformer +import software.amazon.smithy.rust.codegen.util.hasTrait +import software.amazon.smithy.rust.codegen.util.orNull +import java.util.logging.Logger + +fun StructureShape.errorMessageMember(): MemberShape? = this.getMember("message").or { this.getMember("Message") }.orNull() + +object AddErrorMessage { + private val logger = Logger.getLogger("AddErrorMessage") + fun transform(model: Model): Model { + return ModelTransformer.create().mapShapes(model) { shape -> + val addMessageField = shape.hasTrait() && shape is StructureShape && shape.errorMessageMember() == null + if (addMessageField && shape is StructureShape) { + logger.info("Adding message field to ${shape.id}") + shape.toBuilder().addMember("message", ShapeId.from("smithy.api#String")).build() + } else { + shape + } + } + } +} From 5faa8861a93284b182e01433d90006a6f27ad32b Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Wed, 30 Jun 2021 09:03:44 -0700 Subject: [PATCH 2/2] Allow empty errors for XML protocols --- .../smithy/rust/codegen/smithy/RustSettings.kt | 14 ++++++++++---- .../parse/XmlBindingTraitParserGenerator.kt | 3 +-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RustSettings.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RustSettings.kt index f1df744c36..dad980683b 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RustSettings.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RustSettings.kt @@ -28,14 +28,20 @@ private const val RUNTIME_CONFIG = "runtimeConfig" private const val CODEGEN_SETTINGS = "codegen" private const val LICENSE = "license" +/** + * Configuration of codegen settings + * + * [renameExceptions]: Rename `Exception` to `Error` in the generated SDK + * [includeFluentClient]: Generate a `client` module in the generated SDK (currently the AWS SDK sets this to false + * and generates its own client) + * + * [addMessageToErrors]: Adds a `message` field automatically to all error shapes + * [formatTimeoutSeconds]: Timeout for running cargo fmt at the end of code generation + */ data class CodegenConfig( - // / Rename `Exception` to `Error` in the generated SDK val renameExceptions: Boolean = true, - // / Include a fluent client in the generated SDK val includeFluentClient: Boolean = true, - // / Add a message field to all shapes that have the error trait val addMessageToErrors: Boolean = true, - // / Timeout when running `cargo fmt` on a generate service val formatTimeoutSeconds: Int = 20 ) { companion object { 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 0b42acc7e2..1a2cb050f0 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 @@ -214,6 +214,7 @@ class XmlBindingTraitParserGenerator( xmlError ) { val members = errorShape.errorXmlMembers() + rust("if inp.is_empty() { return Ok(builder) }") if (members.isNotEmpty()) { rustTemplate( """ @@ -226,8 +227,6 @@ class XmlBindingTraitParserGenerator( "xml_errors" to xmlErrors ) parseStructureInner(members, builder = "builder", Ctx(tag = "error_decoder", accum = null)) - } else { - rust("let _ = inp;") } rust("Ok(builder)") }