fix: apply clock skew interceptor to clients created via invoke (#1067

)

.changes/5498a35b-f57f-4c0c-bbf7-43e07830be01.json

@@ -0,0 +1,8 @@
+{
+    "id": "5498a35b-f57f-4c0c-bbf7-43e07830be01",
+    "type": "bugfix",
+    "description": "Add config finalization to service clients via new abstract factory class; apply clock skew interceptor to clients created via `invoke`",
+    "issues": [
+        "awslabs/aws-sdk-kotlin#1211"
+    ]
+}

codegen/smithy-aws-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/aws/middleware/ClockSkew.kt

@@ -15,7 +15,12 @@ import software.amazon.smithy.kotlin.codegen.rendering.ServiceClientGenerator
  */
 class ClockSkew : KotlinIntegration {
     override val sectionWriters: List<SectionWriterBinding>
-        get() = listOf(SectionWriterBinding(ServiceClientGenerator.Sections.FinalizeConfig, clockSkewSectionWriter))
+        get() = listOf(
+            SectionWriterBinding(
+                ServiceClientGenerator.Sections.CompanionObject.FinalizeConfig,
+                clockSkewSectionWriter,
+            ),
+        )
 
     private val clockSkewSectionWriter = AppendingSectionWriter { writer ->
         val interceptorSymbol = RuntimeTypes.AwsProtocolCore.ClockSkewInterceptor

codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/RuntimeTypes.kt

@@ -190,6 +190,7 @@ object RuntimeTypes {
     object SmithyClient : RuntimeTypePackage(KotlinDependency.SMITHY_CLIENT) {
         val SdkClient = symbol("SdkClient")
         val AbstractSdkClientBuilder = symbol("AbstractSdkClientBuilder")
+        val AbstractSdkClientFactory = symbol("AbstractSdkClientFactory")
         val LogMode = symbol("LogMode")
         val RetryClientConfig = symbol("RetryClientConfig")
         val RetryStrategyClientConfig = symbol("RetryStrategyClientConfig")

codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/ServiceClientGenerator.kt

@@ -37,6 +37,12 @@ class ServiceClientGenerator(private val ctx: RenderingContext<ServiceShape>) {
          * [SectionId] used when rendering the service interface companion object
          */
         object CompanionObject : SectionId {
+
+            /**
+             * [SectionId] used when rendering the finalizeConfig block of a service client companion object
+             */
+            object FinalizeConfig : SectionId
+
             /**
              * Context key for the service symbol
              */
@@ -46,6 +52,11 @@ class ServiceClientGenerator(private val ctx: RenderingContext<ServiceShape>) {
              * Context key for the SDK ID
              */
             val SdkId: SectionKey<String> = SectionKey("SdkId")
+
+            /**
+             * [SectionId] used when rendering the supertype(s) of the companion object
+             */
+            object SuperTypes : SectionId
         }
 
         /**
@@ -57,11 +68,6 @@ class ServiceClientGenerator(private val ctx: RenderingContext<ServiceShape>) {
              */
             val RenderingContext: SectionKey<RenderingContext<ServiceShape>> = SectionKey("RenderingContext")
         }
-
-        /**
-         * [SectionId] used when rendering the finalizeConfig block of a service client
-         */
-        object FinalizeConfig : SectionId
     }
 
     init {
@@ -88,38 +94,28 @@ class ServiceClientGenerator(private val ctx: RenderingContext<ServiceShape>) {
 
         writer.renderDocumentation(service)
         writer.renderAnnotations(service)
-        writer.openBlock(
+        writer.withBlock(
             "#L interface ${serviceSymbol.name} : #T {",
+            "}",
             ctx.settings.api.visibility,
             RuntimeTypes.SmithyClient.SdkClient,
-        )
-            .call {
-                // allow access to client's Config
-                writer.dokka("${serviceSymbol.name}'s configuration")
-                writer.write("public override val config: Config")
-            }
-            .call {
-                // allow integrations to add additional fields to companion object or configuration
-                writer.write("")
-                writer.declareSection(
-                    Sections.CompanionObject,
-                    context = mapOf(
-                        Sections.CompanionObject.ServiceSymbol to serviceSymbol,
-                        Sections.CompanionObject.SdkId to ctx.settings.sdkId,
-                    ),
-                ) {
-                    renderCompanionObject()
-                }
-                writer.write("")
-                renderServiceBuilder()
+        ) {
+            // allow access to client's Config
+            dokka("${serviceSymbol.name}'s configuration")
+            write("public override val config: Config")
 
-                writer.write("")
-                renderServiceConfig()
-            }
-            .call {
-                operations.forEach { renderOperation(operationsIndex, it) }
-            }
-            .closeBlock("}")
+            // allow integrations to add additional fields to companion object or configuration
+            write("")
+            renderCompanionObject()
+
+            write("")
+            renderServiceBuilder()
+
+            write("")
+            renderServiceConfig()
+
+            operations.forEach { renderOperation(operationsIndex, it) }
+        }
             .write("")
 
         if (ctx.protocolGenerator != null) { // returns default impl, which only exists if there's a protocol generator
@@ -172,15 +168,39 @@ class ServiceClientGenerator(private val ctx: RenderingContext<ServiceShape>) {
     private fun renderCompanionObject() {
         // don't render a companion object which is used for building a service client unless we have a protocol generator
         if (ctx.protocolGenerator == null) return
-        writer.withBlock(
-            "public companion object : #T<Config, Config.Builder, #T, Builder> {",
-            "}",
-            RuntimeTypes.SmithyClient.SdkClientFactory,
-            serviceSymbol,
-        ) {
-            write("@#T", KotlinTypes.Jvm.JvmStatic)
-            write("override fun builder(): Builder = Builder()")
-        }
+
+        writer
+            .writeInline("public companion object : ")
+            .declareSection(
+                Sections.CompanionObject.SuperTypes,
+                context = mapOf(
+                    Sections.CompanionObject.ServiceSymbol to serviceSymbol,
+                ),
+            ) {
+                writeInline(
+                    "#T<Config, Config.Builder, #T, Builder>()",
+                    RuntimeTypes.SmithyClient.AbstractSdkClientFactory,
+                    serviceSymbol,
+                )
+            }
+            .withBlock(" {", "}") {
+                declareSection(
+                    Sections.CompanionObject,
+                    context = mapOf(
+                        Sections.CompanionObject.ServiceSymbol to serviceSymbol,
+                        Sections.CompanionObject.SdkId to ctx.settings.sdkId,
+                    ),
+                ) {
+                    write("@#T", KotlinTypes.Jvm.JvmStatic)
+                    write("override fun builder(): Builder = Builder()")
+                    write("")
+                    withBlock("override fun finalizeConfig(builder: Builder) {", "}") {
+                        declareSection(Sections.CompanionObject.FinalizeConfig) {
+                            write("super.finalizeConfig(builder)")
+                        }
+                    }
+                }
+            }
     }
 
     private fun renderOperation(opIndex: OperationIndex, op: OperationShape) {

codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/rendering/ServiceClientGeneratorTest.kt

@@ -55,9 +55,13 @@ class ServiceClientGeneratorTest {
     @Test
     fun `it renders a companion object with default client factory if protocol generator`() {
         val expected = """
-            public companion object : SdkClientFactory<Config, Config.Builder, TestClient, Builder> {
+            public companion object : AbstractSdkClientFactory<Config, Config.Builder, TestClient, Builder>() {
                 @JvmStatic
                 override fun builder(): Builder = Builder()
+            
+                override fun finalizeConfig(builder: Builder) {
+                    super.finalizeConfig(builder)
+                }
             }
         """.formatForTest()
         commonWithProtocolTestContents.shouldContainOnlyOnceWithDiff(expected)
@@ -66,7 +70,7 @@ class ServiceClientGeneratorTest {
     @Test
     fun `it renders a companion object without default client factory if no protocol generator`() {
         val expected = """
-            public companion object : SdkClientFactory
+            public companion object : AbstractSdkClientFactory
         """.formatForTest()
         commonTestContents.shouldNotContain(expected)
     }
@@ -91,9 +95,7 @@ class ServiceClientGeneratorTest {
         val writer = KotlinWriter(TestModelDefault.NAMESPACE)
         val service = model.expectShape<ServiceShape>(TestModelDefault.SERVICE_SHAPE_ID)
         writer.registerSectionWriter(ServiceClientGenerator.Sections.CompanionObject) { codeWriter, _ ->
-            codeWriter.openBlock("public companion object {")
-                .write("public fun foo(): Int = 1")
-                .closeBlock("}")
+            codeWriter.write("public fun foo(): Int = 1")
         }
 
         writer.registerSectionWriter(ServiceClientGenerator.Sections.ServiceConfig) { codeWriter, _ ->
@@ -103,13 +105,17 @@ class ServiceClientGeneratorTest {
         }
 
         val settings = KotlinSettings(service.id, KotlinSettings.PackageSettings("test", "0.0"), sdkId = service.id.name)
-        val renderingCtx = RenderingContext(writer, service, model, provider, settings)
+
+        // Without a protocol generator the companion object won't be generated so use a fake protocol here.
+        val protocol = MockHttpProtocolGenerator(model)
+        val renderingCtx = RenderingContext(writer, service, model, provider, settings, protocol)
+
         val generator = ServiceClientGenerator(renderingCtx)
         generator.render()
         val contents = writer.toString()
 
         val expectedCompanionOverride = """
-            public companion object {
+            public companion object : AbstractSdkClientFactory<Config, Config.Builder, TestClient, Builder>() {
                 public fun foo(): Int = 1
             }
         """.formatForTest()

runtime/smithy-client/api/smithy-client.api

@@ -5,6 +5,12 @@ public abstract class aws/smithy/kotlin/runtime/client/AbstractSdkClientBuilder
 	protected abstract fun newClient (Laws/smithy/kotlin/runtime/client/SdkClientConfig;)Laws/smithy/kotlin/runtime/client/SdkClient;
 }
 
+public abstract class aws/smithy/kotlin/runtime/client/AbstractSdkClientFactory : aws/smithy/kotlin/runtime/client/SdkClientFactory {
+	public fun <init> ()V
+	protected fun finalizeConfig (Laws/smithy/kotlin/runtime/client/SdkClient$Builder;)V
+	public fun invoke (Lkotlin/jvm/functions/Function1;)Laws/smithy/kotlin/runtime/client/SdkClient;
+}
+
 public abstract interface class aws/smithy/kotlin/runtime/client/IdempotencyTokenConfig {
 	public abstract fun getIdempotencyTokenProvider ()Laws/smithy/kotlin/runtime/client/IdempotencyTokenProvider;
 }

runtime/smithy-client/common/src/aws/smithy/kotlin/runtime/client/AbstractSdkClientFactory.kt

@@ -0,0 +1,23 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package aws.smithy.kotlin.runtime.client
+
+public abstract class AbstractSdkClientFactory<
+    TConfig : SdkClientConfig,
+    TConfigBuilder : SdkClientConfig.Builder<TConfig>,
+    TClient : SdkClient,
+    TClientBuilder : SdkClient.Builder<TConfig, TConfigBuilder, TClient>,
+    > : SdkClientFactory<TConfig, TConfigBuilder, TClient, TClientBuilder> {
+
+    /**
+     * Inject any client-specific config
+     */
+    protected open fun finalizeConfig(builder: TClientBuilder) { }
+
+    public override operator fun invoke(block: TConfigBuilder.() -> Unit): TClient = builder().apply {
+        config.apply(block)
+        finalizeConfig(this)
+    }.build()
+}