From 3b0dd1fa2a0a7803ce7e61181a4b239572f378d4 Mon Sep 17 00:00:00 2001 From: Ken Gilmer Date: Mon, 15 Nov 2021 15:47:43 -0800 Subject: [PATCH 1/6] Refactor from smithy-kotlin java builder removal --- .../codegen/AwsServiceConfigIntegration.kt | 4 +- .../AwsServiceConfigIntegrationTest.kt | 6 +-- .../kotlin/codegen/PresignerGeneratorTest.kt | 45 +++---------------- .../GetBucketLocationOperationDeserializer.kt | 2 +- 4 files changed, 13 insertions(+), 44 deletions(-) diff --git a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/AwsServiceConfigIntegration.kt b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/AwsServiceConfigIntegration.kt index 0bbfe81a5ed..3497b6738b5 100644 --- a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/AwsServiceConfigIntegration.kt +++ b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/AwsServiceConfigIntegration.kt @@ -47,13 +47,13 @@ class AwsServiceConfigIntegration : KotlinIntegration { val serviceSymbol: Symbol = writer.getContextValue(ServiceGenerator.SectionServiceCompanionObject.ServiceSymbol) writer.withBlock("companion object {", "}") { withBlock( - "operator fun invoke(sharedConfig: #T? = null, block: Config.DslBuilder.() -> Unit = {}): #L {", + "operator fun invoke(sharedConfig: #T? = null, block: Config.Builder.() -> Unit = {}): #L {", "}", AwsRuntimeTypes.Types.AwsClientConfig, serviceSymbol.name ) { withBlock( - "val config = Config.BuilderImpl().apply { ", + "val config = Config.Builder().apply { ", "}.apply(block).build()" ) { write("region = sharedConfig?.region") diff --git a/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/AwsServiceConfigIntegrationTest.kt b/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/AwsServiceConfigIntegrationTest.kt index 6230eecb66c..b1dceaa1f02 100644 --- a/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/AwsServiceConfigIntegrationTest.kt +++ b/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/AwsServiceConfigIntegrationTest.kt @@ -50,9 +50,9 @@ class AwsServiceConfigIntegrationTest { contents.shouldContainOnlyOnceWithDiff(expectedProps) val expectedImpl = """ - override var credentialsProvider: CredentialsProvider? = null - override var endpointResolver: AwsEndpointResolver? = null - override var region: String? = null + var credentialsProvider: CredentialsProvider? = null + var endpointResolver: AwsEndpointResolver? = null + var region: String? = null """ contents.shouldContainOnlyOnceWithDiff(expectedImpl) } diff --git a/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt b/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt index b039f7fc500..306f618d82f 100644 --- a/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt +++ b/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt @@ -224,53 +224,22 @@ class PresignerGeneratorTest { * This type can be used to presign requests in cases where an existing service client * instance is not available. */ - class TestPresignConfig private constructor(builder: BuilderImpl): ServicePresignConfig { + class TestPresignConfig private constructor(builder: Builder): ServicePresignConfig { override val credentialsProvider: CredentialsProvider = builder.credentialsProvider ?: DefaultChainCredentialsProvider() override val endpointResolver: AwsEndpointResolver = builder.endpointResolver ?: DefaultEndpointResolver() override val region: String = requireNotNull(builder.region) { "region is a required configuration property" } override val serviceId: String = "example" override val signingName: String = "example-signing-name" companion object { - @JvmStatic - fun fluentBuilder(): FluentBuilder = BuilderImpl() - - operator fun invoke(block: DslBuilder.() -> kotlin.Unit): ServicePresignConfig = BuilderImpl().apply(block).build() - } - - interface FluentBuilder { - fun credentialsProvider(credentialsProvider: CredentialsProvider): FluentBuilder - fun endpointResolver(endpointResolver: AwsEndpointResolver): FluentBuilder - fun region(region: String): FluentBuilder - fun build(): TestPresignConfig - } - - interface DslBuilder { - /** - * The AWS credentials provider to use for authenticating requests. If not provided a [aws.sdk.kotlin.runtime.auth.credentials.DefaultChainCredentialsProvider] instance will be used. - */ - var credentialsProvider: CredentialsProvider? - - /** - * Determines the endpoint (hostname) to make requests to. When not provided a default resolver is configured automatically. This is an advanced client option. - */ - var endpointResolver: AwsEndpointResolver? - - /** - * AWS region to make requests for - */ - var region: String? - + operator fun invoke(block: Builder.() -> kotlin.Unit): ServicePresignConfig = Builder().apply(block).build() } - internal class BuilderImpl() : FluentBuilder, DslBuilder { - override var credentialsProvider: CredentialsProvider? = null - override var endpointResolver: AwsEndpointResolver? = null - override var region: String? = null + public class Builder() { + var credentialsProvider: CredentialsProvider? = null + var endpointResolver: AwsEndpointResolver? = null + var region: String? = null - override fun build(): TestPresignConfig = TestPresignConfig(this) - override fun credentialsProvider(credentialsProvider: CredentialsProvider): FluentBuilder = apply { this.credentialsProvider = credentialsProvider } - override fun endpointResolver(endpointResolver: AwsEndpointResolver): FluentBuilder = apply { this.endpointResolver = endpointResolver } - override fun region(region: String): FluentBuilder = apply { this.region = region } + fun build(): TestPresignConfig = TestPresignConfig(this) } } """.trimIndent() diff --git a/services/s3/common/src/aws/sdk/kotlin/services/s3/internal/GetBucketLocationOperationDeserializer.kt b/services/s3/common/src/aws/sdk/kotlin/services/s3/internal/GetBucketLocationOperationDeserializer.kt index 0513f4ec64a..4231b84878e 100644 --- a/services/s3/common/src/aws/sdk/kotlin/services/s3/internal/GetBucketLocationOperationDeserializer.kt +++ b/services/s3/common/src/aws/sdk/kotlin/services/s3/internal/GetBucketLocationOperationDeserializer.kt @@ -28,7 +28,7 @@ internal class GetBucketLocationOperationDeserializer : HttpDeserialize Date: Mon, 15 Nov 2021 16:43:21 -0800 Subject: [PATCH 2/6] More concise builder decl --- .../kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt b/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt index 306f618d82f..ced3582e234 100644 --- a/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt +++ b/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt @@ -234,7 +234,7 @@ class PresignerGeneratorTest { operator fun invoke(block: Builder.() -> kotlin.Unit): ServicePresignConfig = Builder().apply(block).build() } - public class Builder() { + class Builder { var credentialsProvider: CredentialsProvider? = null var endpointResolver: AwsEndpointResolver? = null var region: String? = null From b12a4cf44eaafa35abb7a32fbec296e6853a2013 Mon Sep 17 00:00:00 2001 From: Ken Gilmer Date: Mon, 15 Nov 2021 17:28:51 -0800 Subject: [PATCH 3/6] Remove builder factory function, construct Builder directly --- .../src/main/kotlin/aws/sdk/kotlin/codegen/protocols/RestXml.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/protocols/RestXml.kt b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/protocols/RestXml.kt index 5bffdb8bc58..4b92b6d574d 100644 --- a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/protocols/RestXml.kt +++ b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/protocols/RestXml.kt @@ -175,7 +175,7 @@ open class RestXml : AwsHttpBindingProtocolGenerator() { val boundMemberName = boundMember.capitalizedDefaultName() val deserializeLambdaIdent = "deserialize$boundMemberName" writer.withBlock("val $deserializeLambdaIdent = {", "}") { - write("val builder = #T.builder()", memberSymbol) + write("val builder = #T.Builder()", memberSymbol) renderDeserializerBody(ctx, copyWithMemberTraits, targetShape.members().toList(), writer) write("builder.build()") } From a0ae73d4aa07041ad34cd881091e3888ee882219 Mon Sep 17 00:00:00 2001 From: Ken Gilmer Date: Tue, 16 Nov 2021 18:35:11 -0800 Subject: [PATCH 4/6] Update service customization for new builder --- .../s3/internal/GetBucketLocationOperationDeserializer.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/s3/common/src/aws/sdk/kotlin/services/s3/internal/GetBucketLocationOperationDeserializer.kt b/services/s3/common/src/aws/sdk/kotlin/services/s3/internal/GetBucketLocationOperationDeserializer.kt index d15a5eceb56..6f4d50ffc9e 100644 --- a/services/s3/common/src/aws/sdk/kotlin/services/s3/internal/GetBucketLocationOperationDeserializer.kt +++ b/services/s3/common/src/aws/sdk/kotlin/services/s3/internal/GetBucketLocationOperationDeserializer.kt @@ -18,7 +18,7 @@ import aws.smithy.kotlin.runtime.serde.xml.xmlStreamReader internal class GetBucketLocationOperationDeserializer : HttpDeserialize { override suspend fun deserialize(context: ExecutionContext, response: HttpResponse): GetBucketLocationResponse { - val builder = GetBucketLocationResponse.builder() + val builder = GetBucketLocationResponse.Builder() val payload = response.body.readAll() if (payload != null) { From 14baf82283643f52f3567aff195e364b54da6e31 Mon Sep 17 00:00:00 2001 From: Ken Gilmer Date: Wed, 17 Nov 2021 11:36:55 -0800 Subject: [PATCH 5/6] Fix test failures --- .../kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt b/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt index ced3582e234..f7c62b3b027 100644 --- a/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt +++ b/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt @@ -231,7 +231,7 @@ class PresignerGeneratorTest { override val serviceId: String = "example" override val signingName: String = "example-signing-name" companion object { - operator fun invoke(block: Builder.() -> kotlin.Unit): ServicePresignConfig = Builder().apply(block).build() + inline operator fun invoke(block: Builder.() -> kotlin.Unit): ServicePresignConfig = Builder().apply(block).build() } class Builder { @@ -239,7 +239,8 @@ class PresignerGeneratorTest { var endpointResolver: AwsEndpointResolver? = null var region: String? = null - fun build(): TestPresignConfig = TestPresignConfig(this) + @PublishedApi + internal fun build(): TestPresignConfig = TestPresignConfig(this) } } """.trimIndent() From 93534765c3abf52a7e8e1fefdc673dc610d12ce7 Mon Sep 17 00:00:00 2001 From: Ken Gilmer Date: Wed, 17 Nov 2021 14:37:38 -0800 Subject: [PATCH 6/6] updates from PR feedback --- .../kotlin/codegen/AwsServiceConfigIntegrationTest.kt | 11 +++++++++++ .../aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt | 9 +++++++++ 2 files changed, 20 insertions(+) diff --git a/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/AwsServiceConfigIntegrationTest.kt b/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/AwsServiceConfigIntegrationTest.kt index b1dceaa1f02..6b9c9c0d0e0 100644 --- a/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/AwsServiceConfigIntegrationTest.kt +++ b/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/AwsServiceConfigIntegrationTest.kt @@ -50,8 +50,19 @@ class AwsServiceConfigIntegrationTest { contents.shouldContainOnlyOnceWithDiff(expectedProps) val expectedImpl = """ + /** + * The AWS credentials provider to use for authenticating requests. If not provided a + * [aws.sdk.kotlin.runtime.auth.credentials.DefaultChainCredentialsProvider] instance will be used. + */ var credentialsProvider: CredentialsProvider? = null + /** + * Determines the endpoint (hostname) to make requests to. When not provided a default + * resolver is configured automatically. This is an advanced client option. + */ var endpointResolver: AwsEndpointResolver? = null + /** + * AWS region to make requests to + */ var region: String? = null """ contents.shouldContainOnlyOnceWithDiff(expectedImpl) diff --git a/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt b/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt index f7c62b3b027..3410946f1b1 100644 --- a/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt +++ b/codegen/smithy-aws-kotlin-codegen/src/test/kotlin/aws/sdk/kotlin/codegen/PresignerGeneratorTest.kt @@ -235,8 +235,17 @@ class PresignerGeneratorTest { } class Builder { + /** + * The AWS credentials provider to use for authenticating requests. If not provided a [aws.sdk.kotlin.runtime.auth.credentials.DefaultChainCredentialsProvider] instance will be used. + */ var credentialsProvider: CredentialsProvider? = null + /** + * Determines the endpoint (hostname) to make requests to. When not provided a default resolver is configured automatically. This is an advanced client option. + */ var endpointResolver: AwsEndpointResolver? = null + /** + * AWS region to make requests for + */ var region: String? = null @PublishedApi