fix!: Make config object immutable after client creation #903
Conversation
| self.plugins = defaultPlugins | ||
| func configureClient(clientConfiguration: inout ClientType.Config) async throws { | ||
| try await plugin.configureClient(clientConfiguration: &clientConfiguration) | ||
| } | ||
| } |
There was a problem hiding this comment.
The PluginContainer class above is used to type-erase the actual plugin so that multiple plugins may be stored in a collection together.
| public protocol Plugin<Config> { | ||
| associatedtype Config: ClientConfiguration | ||
|
|
||
| func configureClient(clientConfiguration: inout Config) async throws |
There was a problem hiding this comment.
Plugin now takes an inout Config so that it may make permanent changes to the passed config.
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.
| config.retryStrategyOptions = self.retryStrategyOptions | ||
| } | ||
| public func configureClient(clientConfiguration: inout Config) { | ||
| clientConfiguration.retryStrategyOptions = self.retryStrategyOptions |
There was a problem hiding this comment.
Note there is no need to cast to DefaultClientConfiguration any more, because that constraint is built into the type of the plugin
| @@ -55,6 +55,7 @@ class DefaultClientConfiguration : ClientConfiguration { | |||
| parameters = listOf( | |||
| FunctionParameter.NoLabel("provider", ClientRuntimeTypes.Core.InterceptorProvider) | |||
| ), | |||
| isMutating = true, | |||
There was a problem hiding this comment.
Places the mutating modifier on these methods, since they now change properties of a value type
|
|
||
| writer.unwrite(",\n").write("") | ||
| while (pluginsIterator.hasNext()) { | ||
| writer.write(".withPlugin(\$L)", pluginsIterator.next().customInitialization(writer)) |
There was a problem hiding this comment.
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.
| @@ -113,7 +108,7 @@ open class HttpProtocolServiceClient( | |||
| .joinToString(" & ") | |||
|
|
|||
| writer.openBlock( | |||
| "public class \$LConfiguration: \$L {", "}", | |||
| "public struct \$LConfiguration: \$L {", "}", | |||
There was a problem hiding this comment.
This is the key change: client config is changed from a class to a struct. This allows config to be modified by plugins, etc. but to be "frozen" when assigned to an immutable property on the client, at client creation.
| @@ -156,7 +151,7 @@ open class HttpProtocolServiceClient( | |||
| } | |||
|
|
|||
| 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.
In several places convenience and required are removed from config initializers since these modifiers are not used for value types.
| fun customInitialization(writer: SwiftWriter) { | ||
| writer.writeInline("\$N()", className) | ||
| fun customInitialization(writer: SwiftWriter): String { | ||
| return writer.format("\$N()", className) |
There was a problem hiding this comment.
This method now creates the text with a writer & returns it to the caller.
| name, | ||
| renderedParameters, | ||
| renderedAsync, | ||
| ) { |
There was a problem hiding this comment.
Added the optional mutating modifier here & did a lot of code cleanup.
(renderedThrows and renderedReturnType remain interpolated because openBlock takes a max number of arguments.)
| } | ||
| return clientConfiguration | ||
| return config |
There was a problem hiding this comment.
An empty config is created, it is modified by each plugin, then the final config is returned to the caller.
| 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.
Plugins are added with a .withPlugin() method so that each can be placed in a type-erased container for storage in an array.
Issue #
Description of changes
Scope
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.