Skip to content

Commit 0bd50d9

Browse files
ianbotsfianbotsf
authored
fix: merge per-op custom metadata to avoid clobbering per-client metadata (#695)
1 parent 72c8e04 commit 0bd50d9

File tree

5 files changed

+86
-39
lines changed

5 files changed

+86
-39
lines changed

‎.changes/6cb06363-7f79-4f03-b8f2-efaac82945ac.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"id": "6cb06363-7f79-4f03-b8f2-efaac82945ac",
3+
"type": "bugfix",
4+
"description": "Merge per-op custom metadata to avoid clobbering per-client metadata",
5+
"issues": [
6+
"awslabs/aws-sdk-kotlin#694"
7+
]
8+
}

‎aws-runtime/aws-http/common/src/aws/sdk/kotlin/runtime/http/middleware/UserAgent.kt‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,11 @@ public class UserAgent(
3636
val customMetadata = req.context.getOrNull(CustomUserAgentMetadata.ContextKey)
3737

3838
// resolve the metadata for the request which is a combination of the static and per/operation metadata
39-
val requestMetadata = staticMetadata.copy(customMetadata = customMetadata)
39+
val requestMetadata = when {
40+
customMetadata == null -> staticMetadata
41+
staticMetadata.customMetadata == null -> staticMetadata.copy(customMetadata = customMetadata)
42+
else -> staticMetadata.copy(customMetadata = staticMetadata.customMetadata + customMetadata)
43+
}
4044

4145
// NOTE: Due to legacy issues with processing the user agent, the original content for
4246
// x-amz-user-agent and User-Agent is swapped here. See top note in the

‎aws-runtime/aws-http/common/src/aws/sdk/kotlin/runtime/http/operation/CustomUserAgentMetadata.kt‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,17 @@ private const val CUSTOM_METADATA_PROP_PREFIX = "aws.customMetadata."
1818
*
1919
* Access via extension property [ExecutionContext.customUserAgentMetadata]
2020
*/
21-
public class CustomUserAgentMetadata {
22-
internal val extras: MutableMap<String, String> = mutableMapOf()
23-
internal val typedExtras: MutableList<TypedUserAgentMetadata> = mutableListOf()
21+
public class CustomUserAgentMetadata(
22+
extras: Map<String, String> = mapOf(),
23+
typedExtras: List<TypedUserAgentMetadata> = listOf(),
24+
) {
25+
internal val extras: MutableMap<String, String>
26+
internal val typedExtras: MutableList<TypedUserAgentMetadata>
27+
28+
init {
29+
this.extras = extras.toMutableMap()
30+
this.typedExtras = typedExtras.toMutableList()
31+
}
2432

2533
internal companion object {
2634
public val ContextKey: AttributeKey<CustomUserAgentMetadata> = AttributeKey("CustomUserAgentMetadata")
@@ -32,8 +40,8 @@ public class CustomUserAgentMetadata {
3240

3341
val envVarMap = provider.getAllEnvVars().findAndStripKeyPrefix(CUSTOM_METADATA_ENV_PREFIX)
3442
val propMap = provider.getAllProperties().findAndStripKeyPrefix(CUSTOM_METADATA_PROP_PREFIX)
35-
val allProps = envVarMap + propMap
36-
return CustomUserAgentMetadata().apply { allProps.forEach { (key, value) -> add(key, value) } }
43+
44+
return CustomUserAgentMetadata(extras = envVarMap + propMap)
3745
}
3846
}
3947

@@ -48,6 +56,9 @@ public class CustomUserAgentMetadata {
4856
public fun add(metadata: TypedUserAgentMetadata) {
4957
typedExtras.add(metadata)
5058
}
59+
60+
public operator fun plus(other: CustomUserAgentMetadata): CustomUserAgentMetadata =
61+
CustomUserAgentMetadata(extras + other.extras, typedExtras + other.typedExtras)
5162
}
5263

5364
/**

‎aws-runtime/aws-http/common/test/aws/sdk/kotlin/runtime/http/AwsUserAgentMetadataTest.kt‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,7 @@ class AwsUserAgentMetadataTest {
2727
val sdkMeta = SdkMetadata("kotlin", apiMeta.version)
2828
val osMetadata = OsMetadata(OsFamily.Linux, "ubuntu-20.04")
2929
val langMeta = LanguageMetadata("1.4.31", mapOf("jvmVersion" to "1.11"))
30-
val custom = CustomUserAgentMetadata().apply {
31-
add("foo", "bar")
32-
}
30+
val custom = CustomUserAgentMetadata(extras = mapOf("foo" to "bar"))
3331
val ua = AwsUserAgentMetadata(sdkMeta, apiMeta, osMetadata, langMeta, customMetadata = custom)
3432
// FIXME re-enable once user agent strings can be longer
3533
val expected = listOf(

‎aws-runtime/aws-http/common/test/aws/sdk/kotlin/runtime/http/middleware/UserAgentTest.kt‎

Lines changed: 56 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import aws.smithy.kotlin.runtime.http.response.HttpCall
2020
import aws.smithy.kotlin.runtime.http.response.HttpResponse
2121
import aws.smithy.kotlin.runtime.http.sdkHttpClient
2222
import aws.smithy.kotlin.runtime.time.Instant
23+
import aws.smithy.kotlin.runtime.util.PlatformProvider
2324
import aws.smithy.kotlin.runtime.util.get
2425
import io.kotest.matchers.string.shouldContain
2526
import io.kotest.matchers.string.shouldNotContain
@@ -41,20 +42,23 @@ class UserAgentTest {
4142

4243
private val client = sdkHttpClient(mockEngine)
4344

44-
@Test
45-
fun itSetsUAHeaders() = runTest {
46-
val op = SdkHttpOperation.build<Unit, HttpResponse> {
45+
private fun initializeOp(platformProvider: PlatformProvider = TestPlatformProvider()) =
46+
SdkHttpOperation.build<Unit, HttpResponse> {
4747
serializer = UnitSerializer
4848
deserializer = IdentityDeserializer
4949
context {
5050
service = "Test Service"
5151
operationName = "testOperation"
5252
}
53+
}.apply {
54+
val apiMd = ApiMetadata("Test Service", "1.2.3")
55+
val metadata = loadAwsUserAgentMetadataFromEnvironment(platformProvider, apiMd)
56+
install(UserAgent(metadata))
5357
}
5458

55-
val provider = TestPlatformProvider()
56-
val metadata = loadAwsUserAgentMetadataFromEnvironment(provider, ApiMetadata("Test Service", "1.2.3"))
57-
op.install(UserAgent(metadata))
59+
@Test
60+
fun itSetsUAHeaders() = runTest {
61+
val op = initializeOp()
5862

5963
op.roundTrip(client, Unit)
6064
val request = op.context[HttpOperationContext.HttpCallList].last().request
@@ -68,19 +72,7 @@ class UserAgentTest {
6872

6973
@Test
7074
fun itAddsPerOperationMetadata() = runTest {
71-
val op = SdkHttpOperation.build<Unit, HttpResponse> {
72-
serializer = UnitSerializer
73-
deserializer = IdentityDeserializer
74-
context {
75-
service = "Test Service"
76-
operationName = "testOperation"
77-
}
78-
}
79-
80-
val provider = TestPlatformProvider()
81-
val staticMeta = loadAwsUserAgentMetadataFromEnvironment(provider, ApiMetadata("Test Service", "1.2.3"))
82-
op.install(UserAgent(staticMeta))
83-
75+
val op = initializeOp()
8476
op.context.customUserAgentMetadata.add("foo", "bar")
8577

8678
op.roundTrip(client, Unit)
@@ -89,17 +81,7 @@ class UserAgentTest {
8981
request.headers[USER_AGENT]!!.shouldContain("md/foo/bar")
9082

9183
// verify per/request metadata is actually per/request
92-
val op2 = SdkHttpOperation.build<Unit, HttpResponse> {
93-
serializer = UnitSerializer
94-
deserializer = IdentityDeserializer
95-
context {
96-
service = "Test Service"
97-
operationName = "testOperation2"
98-
}
99-
}
100-
101-
op2.install(UserAgent(staticMeta))
102-
84+
val op2 = initializeOp()
10385
op2.context.customUserAgentMetadata.add("baz", "quux")
10486

10587
op2.roundTrip(client, Unit)
@@ -108,4 +90,48 @@ class UserAgentTest {
10890
request2.headers[USER_AGENT]!!.shouldNotContain("md/foo/bar")
10991
request2.headers[USER_AGENT]!!.shouldContain("md/baz/quux")
11092
}
93+
94+
@Test
95+
fun itMergesCustomMetadataWithExisting() = runTest {
96+
// see: https://github.com/awslabs/aws-sdk-kotlin/issues/694
97+
val platform = TestPlatformProvider(
98+
props = mapOf(
99+
"aws.customMetadata.foo" to "bar",
100+
"aws.customMetadata.baz" to "qux",
101+
),
102+
)
103+
val op = initializeOp(platform)
104+
op.context.customUserAgentMetadata.apply {
105+
add("baz", "quux")
106+
add("blerg", "blarg")
107+
}
108+
109+
op.roundTrip(client, Unit)
110+
val request = op.context[HttpOperationContext.HttpCallList].last().request
111+
val uaString = request.headers[USER_AGENT]!!
112+
113+
uaString.shouldContain("md/foo/bar")
114+
uaString.shouldContain("md/baz/quux")
115+
uaString.shouldContain("md/blerg/blarg")
116+
uaString.shouldNotContain("md/baz/qux") // This was overwritten by "baz/quux"
117+
}
118+
119+
@Test
120+
fun itDoesNotClobberExistingCustomMetadata() = runTest {
121+
// see: https://github.com/awslabs/aws-sdk-kotlin/issues/694
122+
val platform = TestPlatformProvider(
123+
props = mapOf(
124+
"aws.customMetadata.foo" to "bar",
125+
"aws.customMetadata.baz" to "qux",
126+
),
127+
)
128+
val op = initializeOp(platform)
129+
130+
op.roundTrip(client, Unit)
131+
val request = op.context[HttpOperationContext.HttpCallList].last().request
132+
val uaString = request.headers[USER_AGENT]!!
133+
134+
uaString.shouldContain("md/foo/bar")
135+
uaString.shouldContain("md/baz/qux")
136+
}
111137
}

0 commit comments

Comments
 (0)