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.

Sign up for GitHub

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
Closed

fix!: Remove mutating 'addInterceptor' methods from client config #901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6ffb9c6
fix!: Remove mutating 'addInterceptor' methods from client config
jbelkins Feb 5, 2025
0f29546
Fix tests and lint
jbelkins Feb 5, 2025
f2a66b1
Remove Function & FunctionParameter
jbelkins Feb 5, 2025
ee3eb9e
Fix codegen build
jbelkins Feb 5, 2025
f0872ff
Merge branch 'main' into jbe/remove_interceptor_add_methods
jbelkins Feb 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions Sources/ClientRuntime/Config/DefaultClientConfiguration.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,5 @@ public protocol DefaultClientConfiguration: ClientConfiguration {
/// If none is provided, only a default logger provider will be used.
var telemetryProvider: TelemetryProvider { get set }

/// Adds an `InterceptorProvider` that will be used to provide interceptors for all operations.
///
/// - 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

/// TODO(plugins): Add Checksum, etc.
}
5 changes: 0 additions & 5 deletions Sources/ClientRuntime/Config/DefaultHttpClientConfiguration.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,4 @@ public protocol DefaultHttpClientConfiguration: ClientConfiguration {
///
/// Default resolver will look for the token in the `~/.aws/sso/cache` directory.
var bearerTokenIdentityResolver: any BearerTokenIdentityResolver { get set }

/// Adds a `HttpInterceptorProvider` that will be used to provide interceptors for all HTTP operations.
///
/// - Parameter provider: The `HttpInterceptorProvider` to add.
func addInterceptorProvider(_ provider: HttpInterceptorProvider)
}
4 changes: 2 additions & 2 deletions smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/SwiftWriter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 •
edited
Loading

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].)

if (symbol.isBuiltIn || symbol.isServiceNestedNamespace || symbol.namespace.isEmpty()) return
val spiNames = symbol.getProperty("spiNames").getOrElse { emptyList<String>() } as List<String>
val decl = symbol.getProperty("decl").getOrNull()?.toString()
Expand All @@ -127,8 +129,6 @@ class SwiftWriter(
addImport(symbol.namespace, internalSPINames = spiNames)
}
symbol.dependencies.forEach { addDependency(it) }
val additionalImports = symbol.getProperty("additionalImports").getOrElse { emptyList<Symbol>() } as List<Symbol>
additionalImports.forEach { addImport(it) }
}

fun addImport(
Expand Down
6 changes: 0 additions & 6 deletions ...odegen/src/main/kotlin/software/amazon/smithy/swift/codegen/config/ClientConfiguration.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package software.amazon.smithy.swift.codegen.config
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.swift.codegen.Dependency
import software.amazon.smithy.swift.codegen.integration.ProtocolGenerator
import software.amazon.smithy.swift.codegen.lang.Function
import software.amazon.smithy.swift.codegen.model.buildSymbol

/**
Expand All @@ -22,11 +21,6 @@ interface ClientConfiguration {

fun getProperties(ctx: ProtocolGenerator.GenerationContext): Set<ConfigProperty>

/**
* 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.


companion object {
fun runtimeSymbol(
name: String,
Expand Down
12 changes: 0 additions & 12 deletions ...src/main/kotlin/software/amazon/smithy/swift/codegen/config/DefaultClientConfiguration.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ package software.amazon.smithy.swift.codegen.config
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.swift.codegen.integration.ProtocolGenerator
import software.amazon.smithy.swift.codegen.lang.AccessModifier
import software.amazon.smithy.swift.codegen.lang.Function
import software.amazon.smithy.swift.codegen.lang.FunctionParameter
import software.amazon.smithy.swift.codegen.model.toOptional
import software.amazon.smithy.swift.codegen.swiftmodules.ClientRuntimeTypes
import software.amazon.smithy.swift.codegen.swiftmodules.SmithyRetriesAPITypes
Expand Down Expand Up @@ -47,14 +45,4 @@ class DefaultClientConfiguration : ClientConfiguration {
accessModifier = AccessModifier.PublicPrivateSet
)
)

override fun getMethods(ctx: ProtocolGenerator.GenerationContext): Set<Function> = setOf(
Function(
name = "addInterceptorProvider",
renderBody = { writer -> writer.write("self.interceptorProviders.append(provider)") },
parameters = listOf(
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.

}
12 changes: 0 additions & 12 deletions ...main/kotlin/software/amazon/smithy/swift/codegen/config/DefaultHttpClientConfiguration.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ package software.amazon.smithy.swift.codegen.config
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.swift.codegen.integration.ProtocolGenerator
import software.amazon.smithy.swift.codegen.lang.AccessModifier
import software.amazon.smithy.swift.codegen.lang.Function
import software.amazon.smithy.swift.codegen.lang.FunctionParameter
import software.amazon.smithy.swift.codegen.model.toGeneric
import software.amazon.smithy.swift.codegen.model.toOptional
import software.amazon.smithy.swift.codegen.swiftmodules.ClientRuntimeTypes
Expand Down Expand Up @@ -54,14 +52,4 @@ class DefaultHttpClientConfiguration : ClientConfiguration {
{ it.format("\$N(token: \$N(token: \"\"))", SmithyIdentityTypes.StaticBearerTokenIdentityResolver, SmithyIdentityTypes.BearerTokenIdentity) }
)
)

override fun getMethods(ctx: ProtocolGenerator.GenerationContext): Set<Function> = setOf(
Function(
name = "addInterceptorProvider",
renderBody = { writer -> writer.write("self.httpInterceptorProviders.append(provider)") },
parameters = listOf(
FunctionParameter.NoLabel("provider", ClientRuntimeTypes.Core.HttpInterceptorProvider)
),
)
)
}
8 changes: 0 additions & 8 deletions ...main/kotlin/software/amazon/smithy/swift/codegen/integration/HttpProtocolServiceClient.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,6 @@ open class HttpProtocolServiceClient(
renderCustomConfigInitializer(properties)

renderPartitionID()

clientConfigs
.flatMap { it.getMethods(ctx) }
.sortedBy { it.accessModifier }
.forEach {
it.render(writer)
writer.write("")
}
}
writer.write("")
}
Expand Down
37 changes: 0 additions & 37 deletions smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/lang/Function.kt
View file
Open in desktop

This file was deleted.

39 changes: 0 additions & 39 deletions ...ft-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/lang/FunctionParameter.kt
View file
Open in desktop

This file was deleted.

8 changes: 0 additions & 8 deletions .../src/test/kotlin/software/amazon/smithy/swift/codegen/HttpProtocolClientGeneratorTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,14 +131,6 @@ extension RestJsonProtocolClient {
return ""
}

public func addInterceptorProvider(_ provider: ClientRuntime.InterceptorProvider) {
self.interceptorProviders.append(provider)
}

public func addInterceptorProvider(_ provider: ClientRuntime.HttpInterceptorProvider) {
self.httpInterceptorProviders.append(provider)
}

}

public static func builder() -> ClientRuntime.ClientBuilder<RestJsonProtocolClient> {
Expand Down
Loading