Skip to content
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!: Remove mutating 'addInterceptor' methods from client config #901

Closed
wants to merge 5 commits into from

Conversation

jbelkins
Copy link
Contributor

@jbelkins jbelkins commented Feb 5, 2025

Description of changes

Removes the addInterceptorProvider methods from client config for both interceptors & HTTP interceptors. The purpose of this change is to remove mutating methods from client config types.

Interceptors may be added by using the interceptorProvider and httpInterceptorProvider params on the config initializer.

This is technically a breaking change because it removes a means of adding interceptors to a client config. Impact is expected to be minimal because (1) customers are likely not providing their own interceptors, and (2) there is an alternate, non-mutating method (described above) to add interceptors.

Scope

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -106,6 +106,8 @@ class SwiftWriter(

fun addImport(symbol: Symbol) {
symbol.references.forEach { addImport(it.symbol) }
val additionalImports = symbol.getProperty("additionalImports").getOrElse { emptyList<Symbol>() } as List<Symbol>
additionalImports.forEach { addImport(it) }
Copy link
Contributor Author

@jbelkins jbelkins Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional imports were moved from the bottom to the top of this method because the early-exit on line 111 below causes additional imports to not be added when the enclosing type of the symbol is a Swift built-in.

(This was preventing the needed imports from being rendered for the symbols [ClientRuntime.InterceptorProvider] and [ClientRuntime.HttpInterceptorProvider].)

///
/// - Parameter provider: The `InterceptorProvider` to add.
func addInterceptorProvider(_ provider: InterceptorProvider)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The add methods are removed from this & the following default config protocols

FunctionParameter.NoLabel("provider", ClientRuntimeTypes.Core.InterceptorProvider)
),
)
)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen of the add methods into client config is removed here & in the file below.

/**
* The methods to render in the generated client configuration
*/
fun getMethods(ctx: ProtocolGenerator.GenerationContext): Set<Function> = setOf()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was only used for the add interceptor methods, so it is now removed.

@jbelkins jbelkins marked this pull request as ready for review February 5, 2025 18:15
@jbelkins
Copy link
Contributor Author

jbelkins commented Feb 6, 2025

Closing in favor of an alternate approach

@jbelkins jbelkins closed this Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant