From a0dae4e50c6923a8b2939dda3a8c255ffe881464 Mon Sep 17 00:00:00 2001 From: Matas Lauzadis Date: Mon, 5 Aug 2024 15:49:27 -0400 Subject: [PATCH 1/6] fix: correctly deserialize old-style enums --- .../rendering/serde/XmlParserGenerator.kt | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/XmlParserGenerator.kt b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/XmlParserGenerator.kt index 03153ea6f..50d4baddf 100644 --- a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/XmlParserGenerator.kt +++ b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/XmlParserGenerator.kt @@ -16,6 +16,7 @@ import software.amazon.smithy.kotlin.codegen.model.traits.SyntheticClone import software.amazon.smithy.kotlin.codegen.model.traits.UnwrappedXmlOutput import software.amazon.smithy.kotlin.codegen.rendering.protocol.ProtocolGenerator import software.amazon.smithy.model.shapes.* +import software.amazon.smithy.model.traits.EnumTrait import software.amazon.smithy.model.traits.SparseTrait import software.amazon.smithy.model.traits.TimestampFormatTrait import software.amazon.smithy.model.traits.XmlAttributeTrait @@ -575,10 +576,11 @@ open class XmlParserGenerator( ) { val target = ctx.model.expectShape(member.target) - val parseFn = when (target.type) { - ShapeType.BLOB -> writer.format("#T { it.#T() } ", RuntimeTypes.Serde.parse, RuntimeTypes.Core.Text.Encoding.decodeBase64Bytes) - ShapeType.BOOLEAN -> writer.format("#T()", RuntimeTypes.Serde.parseBoolean) - ShapeType.STRING -> { + val parseFn = when { + target.type == ShapeType.BLOB -> writer.format("#T { it.#T() } ", RuntimeTypes.Serde.parse, RuntimeTypes.Core.Text.Encoding.decodeBase64Bytes) + target.type == ShapeType.BOOLEAN -> writer.format("#T()", RuntimeTypes.Serde.parseBoolean) + target.type == ShapeType.STRING && !target.hasTrait() -> { + println("Shape ${member.defaultName()} is a string! textExprIsResult is $textExprIsResult") if (!textExprIsResult) { writer.write(textExpr) return @@ -586,32 +588,37 @@ open class XmlParserGenerator( null } } - ShapeType.TIMESTAMP -> { + target.type == ShapeType.TIMESTAMP -> { val trait = member.getTrait() ?: target.getTrait() val tsFormat = trait?.format ?: defaultTimestampFormat val runtimeEnum = tsFormat.toRuntimeEnum(writer) writer.format("#T(#L)", RuntimeTypes.Serde.parseTimestamp, runtimeEnum) } - ShapeType.BYTE -> writer.format("#T()", RuntimeTypes.Serde.parseByte) - ShapeType.SHORT -> writer.format("#T()", RuntimeTypes.Serde.parseShort) - ShapeType.INTEGER -> writer.format("#T()", RuntimeTypes.Serde.parseInt) - ShapeType.LONG -> writer.format("#T()", RuntimeTypes.Serde.parseLong) - ShapeType.FLOAT -> writer.format("#T()", RuntimeTypes.Serde.parseFloat) - ShapeType.DOUBLE -> writer.format("#T()", RuntimeTypes.Serde.parseDouble) - ShapeType.BIG_DECIMAL -> writer.format("#T()", RuntimeTypes.Serde.parseBigDecimal) - ShapeType.BIG_INTEGER -> writer.format("#T()", RuntimeTypes.Serde.parseBigInteger) - ShapeType.ENUM -> { + target.type == ShapeType.BYTE -> writer.format("#T()", RuntimeTypes.Serde.parseByte) + target.type == ShapeType.SHORT -> writer.format("#T()", RuntimeTypes.Serde.parseShort) + target.type == ShapeType.INTEGER -> writer.format("#T()", RuntimeTypes.Serde.parseInt) + target.type == ShapeType.LONG -> writer.format("#T()", RuntimeTypes.Serde.parseLong) + target.type == ShapeType.FLOAT -> writer.format("#T()", RuntimeTypes.Serde.parseFloat) + target.type == ShapeType.DOUBLE -> writer.format("#T()", RuntimeTypes.Serde.parseDouble) + target.type == ShapeType.BIG_DECIMAL -> writer.format("#T()", RuntimeTypes.Serde.parseBigDecimal) + target.type == ShapeType.BIG_INTEGER -> writer.format("#T()", RuntimeTypes.Serde.parseBigInteger) + target.type == ShapeType.ENUM || (target.type == ShapeType.STRING && target.hasTrait()) -> { + println("Shape ${member.defaultName()} is an enum! textExprIsResult is $textExprIsResult") if (!textExprIsResult) { writer.write("#T.fromValue(#L)", ctx.symbolProvider.toSymbol(target), textExpr) return + } else { + writer.format("#T { #T.fromValue(it) } ", RuntimeTypes.Serde.parse, ctx.symbolProvider.toSymbol(target)).also { + println("Shape ${member.defaultName()} is an enum, parseFn is $it") + } } - writer.format("#T { #T.fromValue(it) } ", RuntimeTypes.Serde.parse, ctx.symbolProvider.toSymbol(target)) } - ShapeType.INT_ENUM -> { + target.type == ShapeType.INT_ENUM -> { writer.format("#T { #T.fromValue(it.toInt()) } ", RuntimeTypes.Serde.parse, ctx.symbolProvider.toSymbol(target)) } else -> error("unknown primitive member shape $member") } + println("parseFn for ${member.defaultName()}: $parseFn") val escapedErrMessage = "expected $target".replace("$", "$$") writer.write(textExpr) From e52545f4b02f8390a42c2f0b2617f48ecb0a1d21 Mon Sep 17 00:00:00 2001 From: Matas Lauzadis Date: Mon, 5 Aug 2024 17:04:46 -0400 Subject: [PATCH 2/6] Changelog --- .changes/6a37ba36-d945-422c-a4ba-811ef498ca71.json | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changes/6a37ba36-d945-422c-a4ba-811ef498ca71.json diff --git a/.changes/6a37ba36-d945-422c-a4ba-811ef498ca71.json b/.changes/6a37ba36-d945-422c-a4ba-811ef498ca71.json new file mode 100644 index 000000000..31c8c4cd5 --- /dev/null +++ b/.changes/6a37ba36-d945-422c-a4ba-811ef498ca71.json @@ -0,0 +1,8 @@ +{ + "id": "6a37ba36-d945-422c-a4ba-811ef498ca71", + "type": "bugfix", + "description": "Correctly deserialize string-based enums in XML protocols", + "issues": [ + "https://github.com/smithy-lang/smithy-kotlin/issues/1125" + ] +} \ No newline at end of file From 417f8434ceb3fe2ac4c9177bafe48bdfb06104d7 Mon Sep 17 00:00:00 2001 From: Matas Lauzadis Date: Thu, 8 Aug 2024 11:01:36 -0400 Subject: [PATCH 3/6] Remove println --- .../kotlin/codegen/rendering/serde/XmlParserGenerator.kt | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/XmlParserGenerator.kt b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/XmlParserGenerator.kt index 50d4baddf..dd0452d1b 100644 --- a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/XmlParserGenerator.kt +++ b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/XmlParserGenerator.kt @@ -580,7 +580,6 @@ open class XmlParserGenerator( target.type == ShapeType.BLOB -> writer.format("#T { it.#T() } ", RuntimeTypes.Serde.parse, RuntimeTypes.Core.Text.Encoding.decodeBase64Bytes) target.type == ShapeType.BOOLEAN -> writer.format("#T()", RuntimeTypes.Serde.parseBoolean) target.type == ShapeType.STRING && !target.hasTrait() -> { - println("Shape ${member.defaultName()} is a string! textExprIsResult is $textExprIsResult") if (!textExprIsResult) { writer.write(textExpr) return @@ -603,14 +602,11 @@ open class XmlParserGenerator( target.type == ShapeType.BIG_DECIMAL -> writer.format("#T()", RuntimeTypes.Serde.parseBigDecimal) target.type == ShapeType.BIG_INTEGER -> writer.format("#T()", RuntimeTypes.Serde.parseBigInteger) target.type == ShapeType.ENUM || (target.type == ShapeType.STRING && target.hasTrait()) -> { - println("Shape ${member.defaultName()} is an enum! textExprIsResult is $textExprIsResult") if (!textExprIsResult) { writer.write("#T.fromValue(#L)", ctx.symbolProvider.toSymbol(target), textExpr) return } else { - writer.format("#T { #T.fromValue(it) } ", RuntimeTypes.Serde.parse, ctx.symbolProvider.toSymbol(target)).also { - println("Shape ${member.defaultName()} is an enum, parseFn is $it") - } + writer.format("#T { #T.fromValue(it) } ", RuntimeTypes.Serde.parse, ctx.symbolProvider.toSymbol(target)) } } target.type == ShapeType.INT_ENUM -> { @@ -618,7 +614,6 @@ open class XmlParserGenerator( } else -> error("unknown primitive member shape $member") } - println("parseFn for ${member.defaultName()}: $parseFn") val escapedErrMessage = "expected $target".replace("$", "$$") writer.write(textExpr) From 8281792e835851c8bd52c338bb8a855579cb1dec Mon Sep 17 00:00:00 2001 From: Matas Lauzadis Date: Thu, 8 Aug 2024 14:27:57 -0400 Subject: [PATCH 4/6] Add compilation test --- tests/compile/build.gradle.kts | 1 + .../smithy/kotlin/codegen/SmithySdkTest.kt | 49 ++++++++++++++++++ .../codegen/util/CodegenTestIntegration.kt | 51 ++++++++++++++++++- 3 files changed, 100 insertions(+), 1 deletion(-) diff --git a/tests/compile/build.gradle.kts b/tests/compile/build.gradle.kts index 80c79d29e..c0e7efd63 100644 --- a/tests/compile/build.gradle.kts +++ b/tests/compile/build.gradle.kts @@ -18,6 +18,7 @@ dependencies { testImplementation(project(":runtime:protocol:http")) testImplementation(project(":runtime:protocol:http-client-engines:http-client-engine-default")) testImplementation(project(":runtime:serde:serde-json")) + testImplementation(project(":runtime:serde:serde-xml")) testImplementation(project(":runtime:observability:telemetry-api")) testImplementation(project(":runtime:observability:telemetry-defaults")) diff --git a/tests/compile/src/test/kotlin/software/amazon/smithy/kotlin/codegen/SmithySdkTest.kt b/tests/compile/src/test/kotlin/software/amazon/smithy/kotlin/codegen/SmithySdkTest.kt index 6de4de5a9..8cad9fc62 100644 --- a/tests/compile/src/test/kotlin/software/amazon/smithy/kotlin/codegen/SmithySdkTest.kt +++ b/tests/compile/src/test/kotlin/software/amazon/smithy/kotlin/codegen/SmithySdkTest.kt @@ -175,6 +175,55 @@ class SmithySdkTest { assertEquals(KotlinCompilation.ExitCode.OK, compilationResult.exitCode, compileOutputStream.toString()) } + + // https://github.com/smithy-lang/smithy-kotlin/issues/1125 + @Test + fun `it compiles models with string enums`() { + val model = """ + namespace com.test + + use aws.protocols#restXml + + @restXml + service Example { + version: "1.0.0", + operations: [ + Foo, + ] + } + + @http(method: "POST", uri: "/foo-no-input") + operation Foo { + output: FooResponse + } + + structure FooResponse { + payload: StringBasedEnumList + } + + @enum([ + { + value: "blarg", + name: "Blarg" + }, + { + value: "blergh", + name: "Blergh" + } + ]) + string StringBasedEnum + + list StringBasedEnumList { + member: StringBasedEnum + } + """.asSmithy() + + val compileOutputStream = ByteArrayOutputStream() + val compilationResult = compileSdkAndTest(model = model, outputSink = compileOutputStream, emitSourcesToTmp = Debug.emitSourcesToTemp) + compileOutputStream.flush() + + assertEquals(KotlinCompilation.ExitCode.OK, compilationResult.exitCode, compileOutputStream.toString()) + } } /** diff --git a/tests/compile/src/test/kotlin/software/amazon/smithy/kotlin/codegen/util/CodegenTestIntegration.kt b/tests/compile/src/test/kotlin/software/amazon/smithy/kotlin/codegen/util/CodegenTestIntegration.kt index 0ded3a3d8..3de05c6f2 100644 --- a/tests/compile/src/test/kotlin/software/amazon/smithy/kotlin/codegen/util/CodegenTestIntegration.kt +++ b/tests/compile/src/test/kotlin/software/amazon/smithy/kotlin/codegen/util/CodegenTestIntegration.kt @@ -5,6 +5,7 @@ package software.amazon.smithy.kotlin.codegen.util import software.amazon.smithy.aws.traits.protocols.RestJson1Trait +import software.amazon.smithy.aws.traits.protocols.RestXmlTrait import software.amazon.smithy.codegen.core.Symbol import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes import software.amazon.smithy.kotlin.codegen.core.withBlock @@ -20,7 +21,10 @@ import software.amazon.smithy.model.traits.TimestampFormatTrait * Integration that registers protocol generators this package provides */ class CodegenTestIntegration : KotlinIntegration { - override val protocolGenerators: List = listOf(RestJsonTestProtocolGenerator()) + override val protocolGenerators: List = listOf( + RestJsonTestProtocolGenerator(), + RestXmlTestProtocolGenerator() + ) } /** @@ -70,3 +74,48 @@ class MockRestJsonProtocolClientGenerator( middleware: List, httpBindingResolver: HttpBindingResolver, ) : HttpProtocolClientGenerator(ctx, middleware, httpBindingResolver) + +/** + * A partial ProtocolGenerator to generate minimal sdks for tests of restXml models. + */ +class RestXmlTestProtocolGenerator( + override val defaultTimestampFormat: TimestampFormatTrait.Format = TimestampFormatTrait.Format.EPOCH_SECONDS, + override val protocol: ShapeId = RestXmlTrait.ID, +) : HttpBindingProtocolGenerator() { + + override fun getProtocolHttpBindingResolver(model: Model, serviceShape: ServiceShape): HttpBindingResolver = + HttpTraitResolver(model, serviceShape, ProtocolContentTypes.consistent("application/xml")) + + override fun generateProtocolUnitTests(ctx: ProtocolGenerator.GenerationContext) { + // NOOP + } + + override fun getHttpProtocolClientGenerator(ctx: ProtocolGenerator.GenerationContext): HttpProtocolClientGenerator = + MockRestXmlProtocolClientGenerator(ctx, getHttpMiddleware(ctx), getProtocolHttpBindingResolver(ctx.model, ctx.service)) + + override fun structuredDataSerializer(ctx: ProtocolGenerator.GenerationContext): StructuredDataSerializerGenerator = + XmlSerializerGenerator(this, TimestampFormatTrait.Format.EPOCH_SECONDS) + + override fun structuredDataParser(ctx: ProtocolGenerator.GenerationContext): StructuredDataParserGenerator = + XmlParserGenerator(TimestampFormatTrait.Format.EPOCH_SECONDS) + + override fun operationErrorHandler(ctx: ProtocolGenerator.GenerationContext, op: OperationShape): Symbol = + op.errorHandler(ctx.settings) { writer -> + writer.withBlock( + "private fun ${op.errorHandlerName()}(context: #T, call: #T, payload: #T?): Nothing {", + "}", + RuntimeTypes.Core.ExecutionContext, + RuntimeTypes.Http.HttpCall, + KotlinTypes.ByteArray, + ) { + write("error(\"not needed for compile tests\")") + } + } +} + +class MockRestXmlProtocolClientGenerator( + ctx: ProtocolGenerator.GenerationContext, + middleware: List, + httpBindingResolver: HttpBindingResolver, +) : HttpProtocolClientGenerator(ctx, middleware, httpBindingResolver) + From 7a91d95d46871ec4977e7d9843429ee0bf7ba645 Mon Sep 17 00:00:00 2001 From: Matas Lauzadis Date: Thu, 8 Aug 2024 14:29:31 -0400 Subject: [PATCH 5/6] Use `isStringEnumShape`, reorder `when` conditions --- .../rendering/serde/XmlParserGenerator.kt | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/XmlParserGenerator.kt b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/XmlParserGenerator.kt index dd0452d1b..7c5c2a36f 100644 --- a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/XmlParserGenerator.kt +++ b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/XmlParserGenerator.kt @@ -579,7 +579,15 @@ open class XmlParserGenerator( val parseFn = when { target.type == ShapeType.BLOB -> writer.format("#T { it.#T() } ", RuntimeTypes.Serde.parse, RuntimeTypes.Core.Text.Encoding.decodeBase64Bytes) target.type == ShapeType.BOOLEAN -> writer.format("#T()", RuntimeTypes.Serde.parseBoolean) - target.type == ShapeType.STRING && !target.hasTrait() -> { + target.isStringEnumShape -> { + if (!textExprIsResult) { + writer.write("#T.fromValue(#L)", ctx.symbolProvider.toSymbol(target), textExpr) + return + } else { + writer.format("#T { #T.fromValue(it) } ", RuntimeTypes.Serde.parse, ctx.symbolProvider.toSymbol(target)) + } + } + target.type == ShapeType.STRING -> { if (!textExprIsResult) { writer.write(textExpr) return @@ -601,14 +609,6 @@ open class XmlParserGenerator( target.type == ShapeType.DOUBLE -> writer.format("#T()", RuntimeTypes.Serde.parseDouble) target.type == ShapeType.BIG_DECIMAL -> writer.format("#T()", RuntimeTypes.Serde.parseBigDecimal) target.type == ShapeType.BIG_INTEGER -> writer.format("#T()", RuntimeTypes.Serde.parseBigInteger) - target.type == ShapeType.ENUM || (target.type == ShapeType.STRING && target.hasTrait()) -> { - if (!textExprIsResult) { - writer.write("#T.fromValue(#L)", ctx.symbolProvider.toSymbol(target), textExpr) - return - } else { - writer.format("#T { #T.fromValue(it) } ", RuntimeTypes.Serde.parse, ctx.symbolProvider.toSymbol(target)) - } - } target.type == ShapeType.INT_ENUM -> { writer.format("#T { #T.fromValue(it.toInt()) } ", RuntimeTypes.Serde.parse, ctx.symbolProvider.toSymbol(target)) } From 54dada7456d00653ba8d0e340c40d66784751686 Mon Sep 17 00:00:00 2001 From: Matas Lauzadis Date: Thu, 8 Aug 2024 14:33:58 -0400 Subject: [PATCH 6/6] ktlint --- .../kotlin/codegen/rendering/serde/XmlParserGenerator.kt | 1 - .../smithy/kotlin/codegen/util/CodegenTestIntegration.kt | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/XmlParserGenerator.kt b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/XmlParserGenerator.kt index 7c5c2a36f..b2d65e6ea 100644 --- a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/XmlParserGenerator.kt +++ b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/XmlParserGenerator.kt @@ -16,7 +16,6 @@ import software.amazon.smithy.kotlin.codegen.model.traits.SyntheticClone import software.amazon.smithy.kotlin.codegen.model.traits.UnwrappedXmlOutput import software.amazon.smithy.kotlin.codegen.rendering.protocol.ProtocolGenerator import software.amazon.smithy.model.shapes.* -import software.amazon.smithy.model.traits.EnumTrait import software.amazon.smithy.model.traits.SparseTrait import software.amazon.smithy.model.traits.TimestampFormatTrait import software.amazon.smithy.model.traits.XmlAttributeTrait diff --git a/tests/compile/src/test/kotlin/software/amazon/smithy/kotlin/codegen/util/CodegenTestIntegration.kt b/tests/compile/src/test/kotlin/software/amazon/smithy/kotlin/codegen/util/CodegenTestIntegration.kt index 3de05c6f2..dc0155964 100644 --- a/tests/compile/src/test/kotlin/software/amazon/smithy/kotlin/codegen/util/CodegenTestIntegration.kt +++ b/tests/compile/src/test/kotlin/software/amazon/smithy/kotlin/codegen/util/CodegenTestIntegration.kt @@ -23,7 +23,7 @@ import software.amazon.smithy.model.traits.TimestampFormatTrait class CodegenTestIntegration : KotlinIntegration { override val protocolGenerators: List = listOf( RestJsonTestProtocolGenerator(), - RestXmlTestProtocolGenerator() + RestXmlTestProtocolGenerator(), ) } @@ -118,4 +118,3 @@ class MockRestXmlProtocolClientGenerator( middleware: List, httpBindingResolver: HttpBindingResolver, ) : HttpProtocolClientGenerator(ctx, middleware, httpBindingResolver) -