-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix!: Make config object immutable after client creation #903
base: main
Are you sure you want to change the base?
Changes from all commits
7aadd5a
42acd43
e8e8578
7f6e619
d0cf25c
8230ec3
4437d7a
04baddf
e82e8e2
41111fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,29 +4,36 @@ | |
// | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// | ||
|
||
public class ClientBuilder<ClientType: Client> { | ||
|
||
private var plugins: [Plugin] | ||
private struct PluginContainer: Plugin { | ||
let plugin: any Plugin<ClientType.Config> | ||
|
||
public init(defaultPlugins: [Plugin] = []) { | ||
self.plugins = defaultPlugins | ||
func configureClient(clientConfiguration: inout ClientType.Config) async throws { | ||
try await plugin.configureClient(clientConfiguration: &clientConfiguration) | ||
} | ||
} | ||
|
||
public func withPlugin(_ plugin: any Plugin) -> ClientBuilder<ClientType> { | ||
self.plugins.append(plugin) | ||
private var plugins = [PluginContainer]() | ||
|
||
public init() {} | ||
|
||
public func withPlugin<P: Plugin>(_ plugin: P) -> ClientBuilder<ClientType> where P.Config == ClientType.Config { | ||
self.plugins.append(PluginContainer(plugin: plugin)) | ||
return self | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Plugins are added with a |
||
|
||
public func build() async throws -> ClientType { | ||
let configuration = try await resolve(plugins: self.plugins) | ||
let configuration = try await resolve() | ||
return ClientType(config: configuration) | ||
} | ||
|
||
func resolve(plugins: [any Plugin]) async throws -> ClientType.Config { | ||
let clientConfiguration = try await ClientType.Config() | ||
private func resolve() async throws -> ClientType.Config { | ||
var config = try await ClientType.Config() | ||
for plugin in plugins { | ||
try await plugin.configureClient(clientConfiguration: clientConfiguration) | ||
try await plugin.configureClient(clientConfiguration: &config) | ||
} | ||
return clientConfiguration | ||
return config | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An empty config is created, it is modified by each plugin, then the final config is returned to the caller. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,8 @@ | |
// SPDX-License-Identifier: Apache-2.0 | ||
// | ||
|
||
public protocol Plugin { | ||
func configureClient(clientConfiguration: ClientConfiguration) async throws | ||
public protocol Plugin<Config> { | ||
associatedtype Config: ClientConfiguration | ||
|
||
func configureClient(clientConfiguration: inout Config) async throws | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The type of the config is now generic to the plugin so that additional casting is not needed on the config in order to access its properties. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,17 +7,15 @@ | |
|
||
import struct SmithyRetriesAPI.RetryStrategyOptions | ||
|
||
public class RetryPlugin: Plugin { | ||
public class RetryPlugin<Config: DefaultClientConfiguration>: Plugin { | ||
|
||
private var retryStrategyOptions: RetryStrategyOptions | ||
|
||
public init(retryStrategyOptions: RetryStrategyOptions) { | ||
self.retryStrategyOptions = retryStrategyOptions | ||
} | ||
|
||
public func configureClient(clientConfiguration: ClientConfiguration) { | ||
if var config = clientConfiguration as? DefaultClientConfiguration { | ||
config.retryStrategyOptions = self.retryStrategyOptions | ||
} | ||
public func configureClient(clientConfiguration: inout Config) { | ||
clientConfiguration.retryStrategyOptions = self.retryStrategyOptions | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note there is no need to cast to |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,30 +75,25 @@ open class HttpProtocolServiceClient( | |
ClientRuntimeTypes.Core.ClientBuilder, | ||
serviceSymbol.name, | ||
) { | ||
writer.openBlock( | ||
"return \$N<\$L>(defaultPlugins: [", | ||
"])", | ||
writer.write( | ||
"return \$N<\$L>()", | ||
ClientRuntimeTypes.Core.ClientBuilder, | ||
serviceSymbol.name, | ||
) { | ||
val defaultPlugins: MutableList<Plugin> = mutableListOf(DefaultClientPlugin()) | ||
) | ||
writer.indent() | ||
val defaultPlugins: MutableList<Plugin> = mutableListOf(DefaultClientPlugin()) | ||
|
||
ctx.integrations | ||
.flatMap { it.plugins(serviceConfig) } | ||
.filter { it.isDefault } | ||
.onEach { defaultPlugins.add(it) } | ||
ctx.integrations | ||
.flatMap { it.plugins(serviceConfig) } | ||
.filter { it.isDefault } | ||
.onEach { defaultPlugins.add(it) } | ||
|
||
val pluginsIterator = defaultPlugins.iterator() | ||
val pluginsIterator = defaultPlugins.iterator() | ||
|
||
while (pluginsIterator.hasNext()) { | ||
pluginsIterator.next().customInitialization(writer) | ||
if (pluginsIterator.hasNext()) { | ||
writer.write(",") | ||
} | ||
} | ||
|
||
writer.unwrite(",\n").write("") | ||
while (pluginsIterator.hasNext()) { | ||
writer.write(".withPlugin(\$L)", pluginsIterator.next().customInitialization(writer)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than render the plugins as array elements, they are added with a builder method since Swift will reject a literal array for not having plugins of the same type. |
||
} | ||
writer.dedent() | ||
} | ||
} | ||
writer.write("") | ||
|
@@ -113,7 +108,7 @@ open class HttpProtocolServiceClient( | |
.joinToString(" & ") | ||
|
||
writer.openBlock( | ||
"public class \$LConfiguration: \$L {", | ||
"public struct \$LConfiguration: \$L {", | ||
"}", | ||
serviceConfig.clientName.toUpperCamelCase(), | ||
clientConfigurationProtocols, | ||
|
@@ -156,7 +151,7 @@ open class HttpProtocolServiceClient( | |
open fun overrideConfigProperties(properties: List<ConfigProperty>): List<ConfigProperty> = properties | ||
|
||
private fun renderEmptyAsynchronousConfigInitializer(properties: List<ConfigProperty>) { | ||
writer.openBlock("public convenience required init() async throws {", "}") { | ||
writer.openBlock("public init() async throws {", "}") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In several places |
||
writer.openBlock("try await self.init(", ")") { | ||
properties.forEach { property -> | ||
writer.write("\$L: nil,", property.name) | ||
|
@@ -219,7 +214,7 @@ open class HttpProtocolServiceClient( | |
} | ||
|
||
private fun renderSynchronousConfigInitializer(properties: List<ConfigProperty>) { | ||
writer.openBlock("public convenience init(", ") throws {") { | ||
writer.openBlock("public init(", ") throws {") { | ||
properties.forEach { property -> | ||
writer.write("\$L: \$N = nil,", property.name, property.type.toOptional()) | ||
} | ||
|
@@ -246,7 +241,7 @@ open class HttpProtocolServiceClient( | |
private fun renderAsynchronousConfigInitializer(properties: List<ConfigProperty>) { | ||
if (properties.none { it.default?.isAsync == true }) return | ||
|
||
writer.openBlock("public convenience init(", ") async throws {") { | ||
writer.openBlock("public init(", ") async throws {") { | ||
properties.forEach { property -> | ||
writer.write("\$L: \$L = nil,", property.name, property.type.toOptional().renderSwiftType(writer)) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,18 +20,25 @@ data class Function( | |
val accessModifier: AccessModifier = AccessModifier.Public, | ||
val isAsync: Boolean = false, | ||
val isThrowing: Boolean = false, | ||
val isMutating: Boolean = false, | ||
) { | ||
/** | ||
* Render this function using the given writer. | ||
*/ | ||
fun render(writer: SwiftWriter) { | ||
val renderedMutating = "mutating ".takeIf { isMutating } ?: "" | ||
val renderedParameters = parameters.joinToString(", ") { it.rendered(writer) } | ||
val renderedAsync = if (isAsync) "async " else "" | ||
val renderedThrows = if (isThrowing) "throws " else "" | ||
val renderedReturnType = returnType?.let { writer.format("-> \$N ", it) } ?: "" | ||
writer.openBlock( | ||
"${accessModifier.renderedRightPad()}func $name($renderedParameters) $renderedAsync$renderedThrows$renderedReturnType{", | ||
"\$L\$Lfunc \$L(\$L) \$L$renderedThrows$renderedReturnType{", | ||
"}", | ||
accessModifier.renderedRightPad(), | ||
renderedMutating, | ||
name, | ||
renderedParameters, | ||
renderedAsync, | ||
) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added the optional ( |
||
renderBody.accept(writer) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
PluginContainer
class above is used to type-erase the actual plugin so that multiple plugins may be stored in a collection together.