From 6eeb569c833cc161c4c1830444d9548f867d8dac Mon Sep 17 00:00:00 2001 From: Josh Elkins Date: Wed, 15 Jan 2025 17:27:38 -0600 Subject: [PATCH] Changes from code review --- .../Endpoints/Context+ResolvedEndpoint.swift | 21 ---------- .../EndpointResolverMiddleware.swift | 3 -- .../UserAgent/BusinessMetrics.swift | 7 +--- .../UserAgent/BusinessMetricsTests.swift | 22 ---------- .../Context+ResolvedAccountID.swift | 20 ++++++++++ .../codegen/AWSHTTPProtocolClientGenerator.kt | 39 ------------------ .../codegen/AWSHTTPProtocolCustomizations.kt | 2 +- .../AWSHttpProtocolClientGeneratorFactory.kt | 2 +- .../codegen/AWSHttpProtocolServiceClient.kt | 40 ++++++++----------- .../AWSMiddlewareExecutionGenerator.kt | 29 -------------- .../OperationEndpointResolverMiddleware.kt | 2 +- 11 files changed, 41 insertions(+), 146 deletions(-) delete mode 100644 Sources/Core/AWSClientRuntime/Sources/AWSClientRuntime/Endpoints/Context+ResolvedEndpoint.swift create mode 100644 Sources/Core/AWSSDKIdentity/Sources/AWSSDKIdentity/Context+ResolvedAccountID.swift delete mode 100644 codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSHTTPProtocolClientGenerator.kt delete mode 100644 codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSMiddlewareExecutionGenerator.kt diff --git a/Sources/Core/AWSClientRuntime/Sources/AWSClientRuntime/Endpoints/Context+ResolvedEndpoint.swift b/Sources/Core/AWSClientRuntime/Sources/AWSClientRuntime/Endpoints/Context+ResolvedEndpoint.swift deleted file mode 100644 index 36e5cdae35e..00000000000 --- a/Sources/Core/AWSClientRuntime/Sources/AWSClientRuntime/Endpoints/Context+ResolvedEndpoint.swift +++ /dev/null @@ -1,21 +0,0 @@ -// -// Copyright Amazon.com Inc. or its affiliates. -// All Rights Reserved. -// -// SPDX-License-Identifier: Apache-2.0 -// - -import struct Smithy.Attributes -import struct Smithy.AttributeKey -import class Smithy.Context -import struct SmithyHTTPAPI.Endpoint - -public extension Context { - - var resolvedEndpoint: Endpoint? { - get { get(key: resolvedEndpointKey) } - set { set(key: resolvedEndpointKey, value: newValue) } - } -} - -private let resolvedEndpointKey = AttributeKey(name: "ResolvedEndpoint") diff --git a/Sources/Core/AWSClientRuntime/Sources/AWSClientRuntime/Endpoints/EndpointResolverMiddleware.swift b/Sources/Core/AWSClientRuntime/Sources/AWSClientRuntime/Endpoints/EndpointResolverMiddleware.swift index f15c3631ae1..1576037cab0 100644 --- a/Sources/Core/AWSClientRuntime/Sources/AWSClientRuntime/Endpoints/EndpointResolverMiddleware.swift +++ b/Sources/Core/AWSClientRuntime/Sources/AWSClientRuntime/Endpoints/EndpointResolverMiddleware.swift @@ -47,9 +47,6 @@ extension AWSEndpointResolverMiddleware: ApplyEndpoint { let endpoint = try resolverBlock(paramsBlock(attributes)) - // Put endpoint into context for use in business metrics - attributes.resolvedEndpoint = endpoint - var signingName: String? var signingAlgorithm: String? var signingRegion: String? diff --git a/Sources/Core/AWSClientRuntime/Sources/AWSClientRuntime/UserAgent/BusinessMetrics.swift b/Sources/Core/AWSClientRuntime/Sources/AWSClientRuntime/UserAgent/BusinessMetrics.swift index 02d8b06bc34..614aa290b96 100644 --- a/Sources/Core/AWSClientRuntime/Sources/AWSClientRuntime/UserAgent/BusinessMetrics.swift +++ b/Sources/Core/AWSClientRuntime/Sources/AWSClientRuntime/UserAgent/BusinessMetrics.swift @@ -80,11 +80,6 @@ private func setFlagsIntoContext( context.businessMetrics = ["ENDPOINT_OVERRIDE": "N"] } - // Handle O - if let endpoint = context.resolvedEndpoint, let accountID = context.resolvedAWSAccountID, endpoint.host.contains(accountID) { - context.businessMetrics = ["ACCOUNT_ID_ENDPOINT": "O"] - } - // Handle P, Q, R if let accountIDEndpointMode = context.accountIDEndpointMode { switch accountIDEndpointMode { @@ -102,7 +97,7 @@ private func setFlagsIntoContext( } // Handle T - if context.resolvedAWSAccountID != nil { + if context.resolvedAccountID != nil { context.businessMetrics = ["RESOLVED_ACCOUNT_ID": "T"] } diff --git a/Sources/Core/AWSClientRuntime/Tests/AWSClientRuntimeTests/UserAgent/BusinessMetricsTests.swift b/Sources/Core/AWSClientRuntime/Tests/AWSClientRuntimeTests/UserAgent/BusinessMetricsTests.swift index 4baa37cb4b0..884cb1e3215 100644 --- a/Sources/Core/AWSClientRuntime/Tests/AWSClientRuntimeTests/UserAgent/BusinessMetricsTests.swift +++ b/Sources/Core/AWSClientRuntime/Tests/AWSClientRuntimeTests/UserAgent/BusinessMetricsTests.swift @@ -64,28 +64,6 @@ class BusinessMetricsTests: XCTestCase { XCTAssertEqual(userAgent.businessMetrics?.description, expectedString) } - // MARK: - Account ID in Endpoint - - func test_accountIDInEndpoint_noAccountIDInEndpoint() { - configureContext(host: "dynamodb.us-east-1.amazonaws.com", accountID: "0123456789") - - let userAgent = createTestUserAgent() - - // E comes from retry mode & T comes from resolving account ID - let expectedString = "m/E,T,Z,b" - XCTAssertEqual(userAgent.businessMetrics?.description, expectedString) - } - - func test_accountIDInEndpoint_hasAccountIDInEndpoint() { - configureContext(host: "0123456789.dynamodb.us-east-1.amazonaws.com", accountID: "0123456789") - - let userAgent = createTestUserAgent() - - // E comes from retry mode, O comes from account ID in endpoint, & T comes from resolving account ID - let expectedString = "m/E,O,T,Z,b" - XCTAssertEqual(userAgent.businessMetrics?.description, expectedString) - } - // MARK: - AccountIDEndpointMode func test_accountIDEndpointMode_recordsPreferredCorrectly() { diff --git a/Sources/Core/AWSSDKIdentity/Sources/AWSSDKIdentity/Context+ResolvedAccountID.swift b/Sources/Core/AWSSDKIdentity/Sources/AWSSDKIdentity/Context+ResolvedAccountID.swift new file mode 100644 index 00000000000..11e5f2a286b --- /dev/null +++ b/Sources/Core/AWSSDKIdentity/Sources/AWSSDKIdentity/Context+ResolvedAccountID.swift @@ -0,0 +1,20 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +import class Smithy.Context +import struct Smithy.AttributeKey +import struct SmithyIdentity.AWSCredentialIdentity + +public extension Context { + + /// The AWS account ID associated with the selected auth scheme. + /// + /// Will be `nil` if an auth scheme has not yet been selected, an AWS credential identity was not resolved, or the identity did not resolve with an AWS account ID. + var resolvedAccountID: String? { + (selectedAuthScheme?.identity as? AWSCredentialIdentity)?.accountID + } +} diff --git a/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSHTTPProtocolClientGenerator.kt b/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSHTTPProtocolClientGenerator.kt deleted file mode 100644 index ffe5ab7b1b5..00000000000 --- a/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSHTTPProtocolClientGenerator.kt +++ /dev/null @@ -1,39 +0,0 @@ -package software.amazon.smithy.aws.swift.codegen - -import software.amazon.smithy.swift.codegen.SwiftWriter -import software.amazon.smithy.swift.codegen.integration.HTTPProtocolCustomizable -import software.amazon.smithy.swift.codegen.integration.HttpBindingResolver -import software.amazon.smithy.swift.codegen.integration.HttpProtocolClientGenerator -import software.amazon.smithy.swift.codegen.integration.ProtocolGenerator -import software.amazon.smithy.swift.codegen.integration.ServiceConfig -import software.amazon.smithy.swift.codegen.middleware.MiddlewareExecutionGenerator -import software.amazon.smithy.swift.codegen.middleware.OperationMiddleware - -class AWSHTTPProtocolClientGenerator( - private val ctx: ProtocolGenerator.GenerationContext, - private val writer: SwiftWriter, - serviceConfig: ServiceConfig, - private val httpBindingResolver: HttpBindingResolver, - private val defaultContentType: String, - private val httpProtocolCustomizable: HTTPProtocolCustomizable, - private val operationMiddleware: OperationMiddleware -) : HttpProtocolClientGenerator(ctx, writer, serviceConfig, httpBindingResolver, defaultContentType, httpProtocolCustomizable, operationMiddleware) { - - override fun makeMiddlewareExecutionGenerator( - ctx: ProtocolGenerator.GenerationContext, - writer: SwiftWriter, - httpBindingResolver: HttpBindingResolver, - httpProtocolCustomizable: HTTPProtocolCustomizable, - operationMiddleware: OperationMiddleware, - operationStackName: String - ): MiddlewareExecutionGenerator { - return AWSMiddlewareExecutionGenerator( - ctx, - writer, - httpBindingResolver, - httpProtocolCustomizable, - operationMiddleware, - operationStackName - ) - } -} diff --git a/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSHTTPProtocolCustomizations.kt b/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSHTTPProtocolCustomizations.kt index ce5b349fe2d..758f4aefa5c 100644 --- a/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSHTTPProtocolCustomizations.kt +++ b/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSHTTPProtocolCustomizations.kt @@ -25,8 +25,8 @@ import software.amazon.smithy.swift.codegen.model.isOutputEventStream abstract class AWSHTTPProtocolCustomizations : DefaultHTTPProtocolCustomizations() { override fun renderContextAttributes(ctx: ProtocolGenerator.GenerationContext, writer: SwiftWriter, serviceShape: ServiceShape, op: OperationShape) { - // FIXME handle indentation properly or do swift formatting after the fact + writer.write(" .withAccountIDEndpointMode(value: config.accountIdEndpointMode)") writer.write(" .withIdentityResolver(value: config.awsCredentialIdentityResolver, schemeID: \$S)", "aws.auth#sigv4") writer.write(" .withIdentityResolver(value: config.awsCredentialIdentityResolver, schemeID: \$S)", "aws.auth#sigv4a") writer.write(" .withRegion(value: config.region)") diff --git a/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSHttpProtocolClientGeneratorFactory.kt b/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSHttpProtocolClientGeneratorFactory.kt index d33aff5fa91..1cf3aa03b3d 100644 --- a/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSHttpProtocolClientGeneratorFactory.kt +++ b/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSHttpProtocolClientGeneratorFactory.kt @@ -24,6 +24,6 @@ class AWSHttpProtocolClientGeneratorFactory : HttpProtocolClientGeneratorFactory operationMiddleware: OperationMiddleware ): HttpProtocolClientGenerator { val config = AWSServiceConfig(writer, ctx) - return AWSHTTPProtocolClientGenerator(ctx, writer, config, httpBindingResolver, defaultContentType, httpProtocolCustomizable, operationMiddleware) + return HttpProtocolClientGenerator(ctx, writer, config, httpBindingResolver, defaultContentType, httpProtocolCustomizable, operationMiddleware) } } diff --git a/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSHttpProtocolServiceClient.kt b/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSHttpProtocolServiceClient.kt index 9c736787fdf..300d7c0d7ba 100644 --- a/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSHttpProtocolServiceClient.kt +++ b/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSHttpProtocolServiceClient.kt @@ -45,7 +45,7 @@ class AWSHttpProtocolServiceClient( } override fun overrideConfigProperties(properties: List): List { - return properties.map { property -> + return properties.mapNotNull { property -> when (property.name) { "authSchemeResolver" -> { ConfigProperty("authSchemeResolver", SmithyHTTPAuthAPITypes.AuthSchemeResolver, authSchemeResolverDefaultProvider) @@ -101,6 +101,20 @@ class AWSHttpProtocolServiceClient( { it.format("AWSClientConfigDefaultsProvider.httpClientConfiguration()") }, ) } + "accountId" -> null // do not expose accountId as a client config property + "accountIdEndpointMode" -> { // expose accountIdEndpointMode as a Swift string-backed enum + ConfigProperty( + "accountIdEndpointMode", + AWSClientRuntimeTypes.Core.AccountIDEndpointMode.toOptional(), + { writer -> + writer.format( + "\$N.accountIDEndpointMode()", + AWSClientRuntimeTypes.Core.AWSClientConfigDefaultsProvider, + ) + }, + true + ) + } else -> property } } @@ -132,10 +146,10 @@ class AWSHttpProtocolServiceClient( writer.write("try AWSClientConfigDefaultsProvider.retryStrategyOptions(),") } "requestChecksumCalculation" -> { - "try AWSClientConfigDefaultsProvider.requestChecksumCalculation()" + writer.write("try AWSClientConfigDefaultsProvider.requestChecksumCalculation(),") } "responseChecksumValidation" -> { - "try AWSClientConfigDefaultsProvider.responseChecksumValidation()" + writer.write("try AWSClientConfigDefaultsProvider.responseChecksumValidation(),") } else -> { writer.write("\$L,", property.default?.render(writer) ?: "nil") @@ -164,24 +178,4 @@ class AWSHttpProtocolServiceClient( false, false ) - - override fun customizedClientConfigProperty(property: ConfigProperty): ConfigProperty? { - return when (property.name) { - "accountId" -> null // do not expose accountId as a client config property - "accountIdEndpointMode" -> { // expose accountIdEndpointMode as a Swift string-backed enum - ConfigProperty( - "accountIdEndpointMode", - AWSClientRuntimeTypes.Core.AccountIDEndpointMode.toOptional(), - { writer -> - writer.format( - "\$N.accountIDEndpointMode()", - AWSClientRuntimeTypes.Core.AWSClientConfigDefaultsProvider, - ) - }, - true - ) - } - else -> property - } - } } diff --git a/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSMiddlewareExecutionGenerator.kt b/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSMiddlewareExecutionGenerator.kt deleted file mode 100644 index 6115b3bddc0..00000000000 --- a/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/AWSMiddlewareExecutionGenerator.kt +++ /dev/null @@ -1,29 +0,0 @@ -package software.amazon.smithy.aws.swift.codegen - -import software.amazon.smithy.swift.codegen.SwiftWriter -import software.amazon.smithy.swift.codegen.config.ConfigProperty -import software.amazon.smithy.swift.codegen.integration.HTTPProtocolCustomizable -import software.amazon.smithy.swift.codegen.integration.HttpBindingResolver -import software.amazon.smithy.swift.codegen.integration.ProtocolGenerator -import software.amazon.smithy.swift.codegen.middleware.MiddlewareExecutionGenerator -import software.amazon.smithy.swift.codegen.middleware.OperationMiddleware - -class AWSMiddlewareExecutionGenerator( - ctx: ProtocolGenerator.GenerationContext, - private val writer: SwiftWriter, - httpBindingResolver: HttpBindingResolver, - httpProtocolCustomizable: HTTPProtocolCustomizable, - operationMiddleware: OperationMiddleware, - operationStackName: String -) : MiddlewareExecutionGenerator( - ctx, writer, httpBindingResolver, httpProtocolCustomizable, operationMiddleware, operationStackName -) { - - override fun renderConfigPropertyToContext(configProperty: ConfigProperty) { - when (configProperty.name) { - "accountIdEndpointMode" -> { // Used for business metrics - writer.write(" .withAccountIDEndpointMode(value: config.accountIdEndpointMode)") - } - } - } -} diff --git a/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/middleware/OperationEndpointResolverMiddleware.kt b/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/middleware/OperationEndpointResolverMiddleware.kt index fe830ad20e8..b5e318986d5 100644 --- a/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/middleware/OperationEndpointResolverMiddleware.kt +++ b/codegen/smithy-aws-swift-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/middleware/OperationEndpointResolverMiddleware.kt @@ -173,7 +173,7 @@ class OperationEndpointResolverMiddleware( clientContextParam != null -> { when { param.name.toString() == "AccountId" -> { - writer.format("context.resolvedAWSAccountID") + writer.format("context.resolvedAccountID") } param.name.toString() == "AccountIdEndpointMode" -> { "config.accountIdEndpointMode?.rawValue"