diff --git a/.changes/5498a35b-f57f-4c0c-bbf7-43e07830be01.json b/.changes/5498a35b-f57f-4c0c-bbf7-43e07830be01.json new file mode 100644 index 000000000..0d3928741 --- /dev/null +++ b/.changes/5498a35b-f57f-4c0c-bbf7-43e07830be01.json @@ -0,0 +1,9 @@ +{ + "id": "5498a35b-f57f-4c0c-bbf7-43e07830be01", + "type": "bugfix", + "description": "⚠️ **IMPORTANT**: 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" + ], + "requiresMinorVersionBump": true +} \ No newline at end of file diff --git a/.changes/8e87c6dd-5138-4b79-a0c2-255ecf31898b.json b/.changes/8e87c6dd-5138-4b79-a0c2-255ecf31898b.json new file mode 100644 index 000000000..a731d9a92 --- /dev/null +++ b/.changes/8e87c6dd-5138-4b79-a0c2-255ecf31898b.json @@ -0,0 +1,6 @@ +{ + "id": "8e87c6dd-5138-4b79-a0c2-255ecf31898b", + "type": "misc", + "description": "⚠️ **IMPORTANT**: Upgrade to latest versions of OkHttp, Okio, Kotlin", + "requiresMinorVersionBump": true +} \ No newline at end of file diff --git a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/aws/middleware/ClockSkew.kt b/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/aws/middleware/ClockSkew.kt index ae71c8f5b..1a69b8555 100644 --- a/codegen/smithy-aws-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/aws/middleware/ClockSkew.kt +++ b/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 - 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 diff --git a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/RuntimeTypes.kt b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/RuntimeTypes.kt index 80d992e2c..6d0966051 100644 --- a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/RuntimeTypes.kt +++ b/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") diff --git a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/ServiceClientGenerator.kt b/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/ServiceClientGenerator.kt index 70a5438e8..8153f8edf 100644 --- a/codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/ServiceClientGenerator.kt +++ b/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) { * [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) { * Context key for the SDK ID */ val SdkId: SectionKey = 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) { */ val RenderingContext: SectionKey> = 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) { 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) { 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 {", - "}", - 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()", + 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) { diff --git a/codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/rendering/ServiceClientGeneratorTest.kt b/codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/rendering/ServiceClientGeneratorTest.kt index a153fbef8..350215706 100644 --- a/codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/rendering/ServiceClientGeneratorTest.kt +++ b/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 { + public companion object : AbstractSdkClientFactory() { @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(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() { public fun foo(): Int = 1 } """.formatForTest() diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 75c5eb8cb..ebd9c821e 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -1,5 +1,5 @@ [versions] -kotlin-version = "1.9.22" +kotlin-version = "1.9.23" dokka-version = "1.9.10" aws-kotlin-repo-tools-version = "0.4.0" @@ -7,8 +7,8 @@ aws-kotlin-repo-tools-version = "0.4.0" # libs coroutines-version = "1.7.3" atomicfu-version = "0.23.1" -okhttp-version = "5.0.0-alpha.11" -okio-version = "3.6.0" +okhttp-version = "5.0.0-alpha.14" +okio-version = "3.9.0" otel-version = "1.32.0" slf4j-version = "2.0.9" slf4j-v1x-version = "1.7.36" diff --git a/runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpEngine.kt b/runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpEngine.kt index aa65f0aba..263468d3b 100644 --- a/runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpEngine.kt +++ b/runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpEngine.kt @@ -14,8 +14,10 @@ import aws.smithy.kotlin.runtime.net.TlsVersion import aws.smithy.kotlin.runtime.operation.ExecutionContext import aws.smithy.kotlin.runtime.time.Instant import aws.smithy.kotlin.runtime.time.fromEpochMilliseconds +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.job import okhttp3.* +import okhttp3.coroutines.executeAsync import java.util.concurrent.TimeUnit import kotlin.time.toJavaDuration import aws.smithy.kotlin.runtime.net.TlsVersion as SdkTlsVersion @@ -48,6 +50,8 @@ public class OkHttpEngine( val engineRequest = request.toOkHttpRequest(context, callContext, metrics) val engineCall = client.newCall(engineRequest) + + @OptIn(ExperimentalCoroutinesApi::class) val engineResponse = mapOkHttpExceptions { engineCall.executeAsync() } val response = engineResponse.toSdkResponse() diff --git a/runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/test/aws/smithy/kotlin/runtime/http/engine/okhttp/StreamingRequestBodyTest.kt b/runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/test/aws/smithy/kotlin/runtime/http/engine/okhttp/StreamingRequestBodyTest.kt index 73bf3ed8f..da577ded0 100644 --- a/runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/test/aws/smithy/kotlin/runtime/http/engine/okhttp/StreamingRequestBodyTest.kt +++ b/runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/test/aws/smithy/kotlin/runtime/http/engine/okhttp/StreamingRequestBodyTest.kt @@ -13,7 +13,6 @@ import aws.smithy.kotlin.runtime.text.encoding.encodeToHex import kotlinx.coroutines.* import kotlinx.coroutines.test.runTest import okio.Buffer -import okio.BufferedSink import org.junit.jupiter.api.Test import kotlin.coroutines.EmptyCoroutineContext import kotlin.test.* @@ -134,7 +133,7 @@ class StreamingRequestBodyTest { override fun readFrom(): SdkByteReadChannel = chan } - val sink = TestSink() + val sink = Buffer() val callJob = Job() val callContext = coroutineContext + callJob @@ -145,19 +144,14 @@ class StreamingRequestBodyTest { assertEquals(0, callJob.children.toList().size) actual.writeTo(sink) assertEquals(1, callJob.children.toList().size) // writer - assertEquals(sink.buffer.size, 0) + assertEquals(sink.size, 0) chan.writeAll(content.source()) - assertFalse(sink.isClosed) - chan.close() callJob.complete() callJob.join() - // we must manually close the sink given to us when stream completes - assertTrue(sink.isClosed) - - val actualSha256 = sink.buffer.sha256().hex() + val actualSha256 = sink.sha256().hex() assertEquals(expectedSha256, actualSha256) } @@ -174,14 +168,9 @@ class StreamingRequestBodyTest { val callContext = coroutineContext + Job() val actual = StreamingRequestBody(body, callContext) - val sink = TestSink() + val sink = Buffer() actual.writeTo(sink) - assertContentEquals(file.readBytes(), sink.buffer.readByteArray()) + assertContentEquals(file.readBytes(), sink.readByteArray()) } } - -private class TestSink(override val buffer: Buffer = Buffer()) : BufferedSink by buffer { - var isClosed = false - override fun close() { isClosed = true } -} diff --git a/runtime/smithy-client/api/smithy-client.api b/runtime/smithy-client/api/smithy-client.api index 416231d5e..c2a919000 100644 --- a/runtime/smithy-client/api/smithy-client.api +++ b/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 ()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; } diff --git a/runtime/smithy-client/common/src/aws/smithy/kotlin/runtime/client/AbstractSdkClientFactory.kt b/runtime/smithy-client/common/src/aws/smithy/kotlin/runtime/client/AbstractSdkClientFactory.kt new file mode 100644 index 000000000..f867ed224 --- /dev/null +++ b/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, + TClient : SdkClient, + TClientBuilder : SdkClient.Builder, + > : SdkClientFactory { + + /** + * 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() +}