Skip to content

Commit

Permalink
Changes from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
jbelkins committed Jan 15, 2025
1 parent 58c0582 commit 6eeb569
Show file tree
Hide file tree
Showing 11 changed files with 41 additions and 146 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -102,7 +97,7 @@ private func setFlagsIntoContext(
}

// Handle T
if context.resolvedAWSAccountID != nil {
if context.resolvedAccountID != nil {
context.businessMetrics = ["RESOLVED_ACCOUNT_ID": "T"]
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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)")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class AWSHttpProtocolServiceClient(
}

override fun overrideConfigProperties(properties: List<ConfigProperty>): List<ConfigProperty> {
return properties.map { property ->
return properties.mapNotNull { property ->
when (property.name) {
"authSchemeResolver" -> {
ConfigProperty("authSchemeResolver", SmithyHTTPAuthAPITypes.AuthSchemeResolver, authSchemeResolverDefaultProvider)
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
}
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 6eeb569

Please sign in to comment.