Skip to content

Commit 5b28321

Browse files
authored
feat: make HTTP engines copyable/reconfigurable in service client config (#851)
1 parent 014b6cf commit 5b28321

File tree

50 files changed

+784
-366
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+784
-366
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"id": "b35b5256-bb67-415b-84af-49be743aed14",
3+
"type": "feature",
4+
"description": "**Breaking**: Make HTTP engines configurable in client config during initialization and during `withCopy`. See [this discussion post](https://github.com/awslabs/aws-sdk-kotlin/discussions/new?category=announcements) for more information."
5+
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ object RuntimeTypes {
7272

7373
object Config : RuntimeTypePackage(KotlinDependency.HTTP, "config") {
7474
val HttpClientConfig = symbol("HttpClientConfig")
75+
val HttpEngineConfig = symbol("HttpEngineConfig")
7576
}
7677

7778
object Engine : RuntimeTypePackage(KotlinDependency.HTTP, "engine") {
@@ -335,6 +336,7 @@ object RuntimeTypes {
335336
object HttpClientEngines {
336337
object Default : RuntimeTypePackage(KotlinDependency.DEFAULT_HTTP_ENGINE) {
337338
val DefaultHttpEngine = symbol("DefaultHttpEngine")
339+
val HttpEngineConfigImpl = symbol("HttpEngineConfigImpl")
338340
}
339341
}
340342

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ class ServiceClientConfigGenerator(
4444
add(RuntimeConfigProperty.ClientName)
4545
add(RuntimeConfigProperty.LogMode)
4646
if (context.protocolGenerator?.applicationProtocol?.isHttpProtocol == true) {
47-
add(RuntimeConfigProperty.HttpClientEngine)
47+
add(RuntimeConfigProperty.HttpClient)
4848
add(RuntimeConfigProperty.HttpInterceptors)
4949
add(RuntimeConfigProperty.HttpAuthSchemes)
5050
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ abstract class HttpProtocolClientGenerator(
8181
*/
8282
protected open fun renderProperties(writer: KotlinWriter) {
8383
writer.write("private val managedResources = #T()", RuntimeTypes.Core.IO.SdkManagedGroup)
84-
writer.write("private val client = #T(config.httpClientEngine)", RuntimeTypes.HttpClient.SdkHttpClient)
84+
writer.write("private val client = #T(config.httpClient)", RuntimeTypes.HttpClient.SdkHttpClient)
8585

8686
// render auth resolver related properties
8787
writer.write("private val identityProviderConfig = #T(config)", IdentityProviderConfigGenerator.getSymbol(ctx.settings))
@@ -133,7 +133,7 @@ abstract class HttpProtocolClientGenerator(
133133
*/
134134
protected open fun renderInit(writer: KotlinWriter) {
135135
writer.withBlock("init {", "}") {
136-
write("managedResources.#T(config.httpClientEngine)", RuntimeTypes.Core.IO.addIfManaged)
136+
write("managedResources.#T(config.httpClient)", RuntimeTypes.Core.IO.addIfManaged)
137137
}
138138
}
139139

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ open class HttpProtocolUnitTestRequestGenerator protected constructor(builder: B
115115
* configures a mock HttpClientEngine and an idempotency token generator appropriate for protocol tests.
116116
*/
117117
open fun renderConfigureServiceClient(test: HttpRequestTestCase) {
118-
writer.write("httpClientEngine = mockEngine")
118+
writer.write("httpClient = mockEngine")
119119
if (idempotentFieldsInModel) {
120120
writer.write("idempotencyTokenProvider = #T { \"00000000-0000-4000-8000-000000000000\" }", RuntimeTypes.SmithyClient.IdempotencyTokenProvider)
121121
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ open class HttpProtocolUnitTestResponseGenerator protected constructor(builder:
108108
* configures a mock HttpClientEngine and an idempotency token generator appropriate for protocol tests.
109109
*/
110110
open fun renderConfigureServiceClient(test: HttpResponseTestCase) {
111-
writer.write("httpClientEngine = mockEngine")
111+
writer.write("httpClient = mockEngine")
112112
if (idempotentFieldsInModel) {
113113
// see: https://awslabs.github.io/smithy/1.0/spec/http-protocol-compliance-tests.html#parameter-format
114114
writer.write("idempotencyTokenProvider = #T { \"00000000-0000-4000-8000-000000000000\" }", RuntimeTypes.SmithyClient.IdempotencyTokenProvider)

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

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package software.amazon.smithy.kotlin.codegen.rendering.util
77

8+
import software.amazon.smithy.codegen.core.Symbol
89
import software.amazon.smithy.codegen.core.SymbolReference
910
import software.amazon.smithy.kotlin.codegen.core.CodegenContext
1011
import software.amazon.smithy.kotlin.codegen.core.KotlinWriter
@@ -49,7 +50,7 @@ abstract class AbstractConfigGenerator {
4950
addPropertyImports(props, writer)
5051

5152
val sortedProps = props.sortedWith(compareBy({ it.order }, { it.propertyName }))
52-
val formattedBaseClasses = props.formattedBaseClasses { it.baseClass?.name }
53+
val formattedBaseClasses = props.formattedBaseClasses { it.baseClass to it.baseClassDelegate }
5354

5455
writer.withBlock(
5556
"$visibility class #configClass.name:L private constructor(builder: Builder)$formattedBaseClasses {",
@@ -75,14 +76,12 @@ abstract class AbstractConfigGenerator {
7576
*/
7677
private fun addPropertyImports(props: Collection<ConfigProperty>, writer: KotlinWriter) {
7778
props.forEach {
78-
it.baseClass?.let { baseClass ->
79-
writer.addImport(baseClass)
80-
}
79+
it.baseClass?.let(writer::addImport)
80+
it.baseClassDelegate?.symbol?.let(writer::addImport)
81+
it.builderBaseClassDelegate?.symbol?.let(writer::addImport)
8182
writer.addImport(it.symbol)
8283
writer.addImportReferences(it.symbol, SymbolReference.ContextOption.USE)
83-
it.additionalImports.forEach { symbol ->
84-
writer.addImport(symbol)
85-
}
84+
it.additionalImports.forEach(writer::addImport)
8685
}
8786
}
8887

@@ -130,7 +129,7 @@ abstract class AbstractConfigGenerator {
130129
}
131130

132131
protected open fun renderBuilder(props: Collection<ConfigProperty>, writer: KotlinWriter) {
133-
val formattedBaseClasses = props.formattedBaseClasses { it.builderBaseClass?.name }
132+
val formattedBaseClasses = props.formattedBaseClasses { it.builderBaseClass to it.builderBaseClassDelegate }
134133

135134
writer.write("")
136135
.withBlock("$visibility class Builder$formattedBaseClasses {", "}") {
@@ -159,8 +158,11 @@ abstract class AbstractConfigGenerator {
159158
* Return the formatted base classes for the config property
160159
* @param transform the selector from config property that maps the property to a base class name
161160
*/
162-
private inline fun Collection<ConfigProperty>.formattedBaseClasses(transform: (ConfigProperty) -> String?): String {
163-
val baseClasses = mapNotNull(transform)
161+
private inline fun Collection<ConfigProperty>.formattedBaseClasses(
162+
transform: (ConfigProperty) -> Pair<Symbol?, Delegate?>,
163+
): String {
164+
val baseClasses = map(transform)
165+
.mapNotNull { (symbol, delegate) -> symbol?.format(delegate) }
164166
.sorted()
165167
.toSet()
166168
.joinToString(", ")
@@ -178,3 +180,6 @@ abstract class AbstractConfigGenerator {
178180
}
179181
}
180182
}
183+
184+
private fun Symbol.format(delegate: Delegate?): String =
185+
name + (delegate?.let { " by ${it.delegationExpression}" } ?: "")

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,33 @@ class ConfigProperty private constructor(builder: Builder) {
7878
*/
7979
val baseClass: Symbol? = builder.baseClass
8080

81+
/**
82+
* A delegate the config object should use as the backing implementation of this property's base class. This enables
83+
* class specifications such as:
84+
*
85+
* ```kotlin
86+
* class Config : BaseClass by BaseClassDelegate(...)
87+
* ```
88+
*/
89+
val baseClassDelegate: Delegate? = builder.baseClassDelegate
90+
8191
/**
8292
* Additional base classes the config builder should inherit from
8393
*
8494
* NOTE: Adding 1 or more base classes will implicitly render the property with an `override` modifier
8595
*/
8696
val builderBaseClass: Symbol? = builder.builderBaseClass
8797

98+
/**
99+
* A delegate the config builder object should use as the backing implementation of this property's builder base
100+
* class. This enables class specifications such as:
101+
*
102+
* ```kotlin
103+
* class Builder : BuilderBaseClass by BuilderBaseClassDelegate(...)
104+
* ```
105+
*/
106+
val builderBaseClassDelegate: Delegate? = builder.builderBaseClassDelegate
107+
88108
/**
89109
* The configuration property type. This controls how the property is constructed and rendered
90110
*/
@@ -193,7 +213,9 @@ class ConfigProperty private constructor(builder: Builder) {
193213
var documentation: String? = null
194214

195215
var baseClass: Symbol? = null
216+
var baseClassDelegate: Delegate? = null
196217
var builderBaseClass: Symbol? = null
218+
var builderBaseClassDelegate: Delegate? = null
197219

198220
var propertyType: ConfigPropertyType = ConfigPropertyType.SymbolDefault
199221

@@ -231,6 +253,14 @@ class ConfigProperty private constructor(builder: Builder) {
231253
}
232254
}
233255

256+
/**
257+
* Represents a type which may be used for [inheritance delegation](https://kotlinlang.org/docs/delegation.html).
258+
* @param symbol The symbol for the delegate target. Codegenerators should import this symbol where appropriate so that
259+
* the [delegationExpression] has the correct symbol in scope.
260+
* @param delegationExpression The code expression for how to delegate
261+
*/
262+
data class Delegate(val symbol: Symbol?, val delegationExpression: String)
263+
234264
private fun builtInSymbol(symbolName: String, defaultValue: String?): Symbol {
235265
val builder = Symbol.builder()
236266
.name(symbolName)

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

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -42,28 +42,25 @@ object RuntimeConfigProperty {
4242
order = -100
4343
}
4444

45-
val HttpClientEngine = ConfigProperty {
45+
val HttpClient = ConfigProperty {
46+
name = "httpClient"
4647
symbol = RuntimeTypes.HttpClient.Engine.HttpClientEngine
47-
baseClass = RuntimeTypes.HttpClient.Config.HttpClientConfig
48-
useNestedBuilderBaseClass()
49-
documentation = """
50-
Override the default HTTP client engine used to make SDK requests (e.g. configure proxy behavior, timeouts, concurrency, etc).
51-
NOTE: The caller is responsible for managing the lifetime of the engine when set. The SDK
52-
client will not close it when the client is closed.
53-
""".trimIndent()
54-
order = -100
5548

56-
propertyType = ConfigPropertyType.Custom(
57-
render = { prop, writer ->
58-
writer.write(
59-
"override val #1L: #2T = builder.#1L ?: #3T().#4T()",
60-
prop.propertyName,
61-
prop.symbol,
62-
RuntimeTypes.HttpClientEngines.Default.DefaultHttpEngine,
63-
RuntimeTypes.HttpClient.Engine.manage,
64-
)
65-
},
49+
baseClass = RuntimeTypes.HttpClient.Config.HttpEngineConfig
50+
baseClassDelegate = Delegate(null, "builder.buildHttpEngineConfig()")
51+
52+
builderBaseClass = RuntimeTypes.HttpClient.Config.HttpEngineConfig.let {
53+
buildSymbol {
54+
name = "${it.name}.Builder"
55+
namespace = it.namespace
56+
}
57+
}
58+
builderBaseClassDelegate = Delegate(
59+
RuntimeTypes.HttpClientEngines.Default.HttpEngineConfigImpl,
60+
"HttpEngineConfigImpl.BuilderImpl()",
6661
)
62+
63+
propertyType = ConfigPropertyType.Custom({ _, _ -> }, { _, _ -> })
6764
}
6865

6966
val IdempotencyTokenProvider = ConfigProperty {

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

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,12 @@ class ServiceClientConfigGeneratorTest {
4242
contents.assertBalancedBracesAndParens()
4343

4444
val expectedCtor = """
45-
public class Config private constructor(builder: Builder) : HttpAuthConfig, HttpClientConfig, IdempotencyTokenConfig, SdkClientConfig, TracingClientConfig {
45+
public class Config private constructor(builder: Builder) : HttpAuthConfig, HttpClientConfig, HttpEngineConfig by builder.buildHttpEngineConfig(), IdempotencyTokenConfig, SdkClientConfig, TracingClientConfig {
4646
"""
4747
contents.shouldContainWithDiff(expectedCtor)
4848

4949
val expectedProps = """
5050
override val clientName: String = builder.clientName
51-
override val httpClientEngine: HttpClientEngine = builder.httpClientEngine ?: DefaultHttpEngine().manage()
5251
override val authSchemes: kotlin.collections.List<aws.smithy.kotlin.runtime.http.auth.HttpAuthScheme> = builder.authSchemes
5352
public val endpointProvider: EndpointProvider = requireNotNull(builder.endpointProvider) { "endpointProvider is a required configuration property" }
5453
override val idempotencyTokenProvider: IdempotencyTokenProvider = builder.idempotencyTokenProvider ?: IdempotencyTokenProvider.Default
@@ -61,19 +60,12 @@ public class Config private constructor(builder: Builder) : HttpAuthConfig, Http
6160
contents.shouldContainWithDiff(expectedProps)
6261

6362
val expectedBuilder = """
64-
public class Builder : HttpAuthConfig.Builder, HttpClientConfig.Builder, IdempotencyTokenConfig.Builder, SdkClientConfig.Builder<Config>, TracingClientConfig.Builder {
63+
public class Builder : HttpAuthConfig.Builder, HttpClientConfig.Builder, HttpEngineConfig.Builder by HttpEngineConfigImpl.BuilderImpl(), IdempotencyTokenConfig.Builder, SdkClientConfig.Builder<Config>, TracingClientConfig.Builder {
6564
/**
6665
* A reader-friendly name for the client.
6766
*/
6867
override var clientName: String = "Test"
6968
70-
/**
71-
* Override the default HTTP client engine used to make SDK requests (e.g. configure proxy behavior, timeouts, concurrency, etc).
72-
* NOTE: The caller is responsible for managing the lifetime of the engine when set. The SDK
73-
* client will not close it when the client is closed.
74-
*/
75-
override var httpClientEngine: HttpClientEngine? = null
76-
7769
/**
7870
* Register new or override default [HttpAuthScheme]s configured for this client. By default, the set
7971
* of auth schemes configured comes from the service model. An auth scheme configured explicitly takes
@@ -256,7 +248,6 @@ public class Config private constructor(builder: Builder) {
256248
// Expect logMode config value to override default to LogMode.Request
257249
val expectedConfigValues = """
258250
override val clientName: String = builder.clientName
259-
override val httpClientEngine: HttpClientEngine = builder.httpClientEngine ?: DefaultHttpEngine().manage()
260251
override val authSchemes: kotlin.collections.List<aws.smithy.kotlin.runtime.http.auth.HttpAuthScheme> = builder.authSchemes
261252
public val customProp: Int? = builder.customProp
262253
public val endpointProvider: EndpointProvider = requireNotNull(builder.endpointProvider) { "endpointProvider is a required configuration property" }

0 commit comments

Comments
 (0)