Skip to content

Commit 8965dee

Browse files
committed
fix: update clientContextParamDefaults to only have conflicting params
1 parent 30fc751 commit 8965dee

File tree

14 files changed

+125
-42
lines changed

14 files changed

+125
-42
lines changed
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { describe, expect, test as it } from "vitest";
2+
import { XYZService } from "xyz";
3+
4+
describe("client context parameters precedence integration test", () => {
5+
it("should handle conflicting vs non-conflicting parameter precedence correctly", async () => {
6+
// For non-conflicting parameter root level takes precedence over nested defaults
7+
const clientWithNonConflicting = new XYZService({
8+
endpoint: "https://localhost",
9+
apiKey: async () => ({ apiKey: "test-key" }),
10+
customParam: "user-custom-value",
11+
clientContextParams: {
12+
apiKey: "test-key",
13+
customParam: "default-custom-value",
14+
},
15+
});
16+
17+
// Verify the resolved config has the user's root-level value
18+
const resolvedConfig = (clientWithNonConflicting as any).config;
19+
expect(resolvedConfig.customParam).toBe("user-custom-value");
20+
21+
// For conflicting parameters nested values are used for endpoint resolution
22+
const clientWithConflicting = new XYZService({
23+
endpoint: "https://localhost",
24+
apiKey: async () => ({ apiKey: "auth-key" }),
25+
clientContextParams: {
26+
apiKey: "endpoint-key",
27+
},
28+
});
29+
30+
// Verify that both auth and endpoint contexts can coexist
31+
const resolvedConfigConflicting = (clientWithConflicting as any).config;
32+
33+
// Verify endpoint context has the nested value
34+
expect(resolvedConfigConflicting.clientContextParams.apiKey).toBe("endpoint-key");
35+
36+
// Verify auth context has the auth provider
37+
const authIdentity = await resolvedConfigConflicting.apiKey();
38+
expect(authIdentity.apiKey).toBe("auth-key");
39+
});
40+
});

packages/eventstream-serde-universal/src/eventstream-cbor.integ.spec.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,11 @@ describe("local model integration test for cbor eventstreams", () => {
99
it("should read and write cbor event streams", async () => {
1010
const client = new XYZService({
1111
endpoint: "https://localhost",
12-
apiKey: { apiKey: "test-api-key" },
13-
} as any);
12+
apiKey: async () => ({ apiKey: "test-api-key" }),
13+
clientContextParams: {
14+
apiKey: "test-api-key",
15+
},
16+
});
1417

1518
const body = cbor.serialize({
1619
id: "alpha",

packages/middleware-endpoint/src/adaptors/createConfigValueProvider.spec.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,12 @@ describe(createConfigValueProvider.name, () => {
6161

6262
it("should prioritize clientContextParams over direct properties", async () => {
6363
const config = {
64-
apiKey: "direct-api-key",
64+
stage: "prod",
6565
clientContextParams: {
66-
apiKey: "nested-api-key",
66+
stage: "beta",
6767
},
6868
};
69-
expect(await createConfigValueProvider("apiKey", "apiKey", config, true)()).toEqual("nested-api-key");
69+
expect(await createConfigValueProvider("stage", "stage", config, true)()).toEqual("beta");
7070
});
7171

7272
it("should fall back to direct property when clientContextParams is not provided", async () => {

packages/util-retry/src/retries.integ.spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ describe("retries", () => {
2121
const client = new XYZService({
2222
endpoint: "https://localhost/nowhere",
2323
apiKey: { apiKey: "test-api-key" },
24-
} as any);
24+
});
2525

2626
requireRequestsFrom(client)
2727
.toMatch({
@@ -52,7 +52,7 @@ describe("retries", () => {
5252
const client = new XYZService({
5353
endpoint: "https://localhost/nowhere",
5454
apiKey: { apiKey: "test-api-key" },
55-
} as any);
55+
});
5656

5757
requireRequestsFrom(client)
5858
.toMatch({
@@ -83,7 +83,7 @@ describe("retries", () => {
8383
const client = new XYZService({
8484
endpoint: "https://localhost/nowhere",
8585
apiKey: { apiKey: "test-api-key" },
86-
} as any);
86+
});
8787

8888
requireRequestsFrom(client)
8989
.toMatch({
@@ -114,7 +114,7 @@ describe("retries", () => {
114114
const client = new XYZService({
115115
endpoint: "https://localhost/nowhere",
116116
apiKey: { apiKey: "test-api-key" },
117-
} as any);
117+
});
118118

119119
requireRequestsFrom(client)
120120
.toMatch({

private/my-local-model-schema/src/endpoint/EndpointParameters.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,14 @@ import type { Endpoint, EndpointParameters as __EndpointParameters, EndpointV2,
77
export interface ClientInputEndpointParameters {
88
clientContextParams?: {
99
apiKey?: string | undefined | Provider<string | undefined>;
10+
region?: string | undefined | Provider<string | undefined>;
1011
customParam?: string | undefined | Provider<string | undefined>;
1112
enableFeature?: boolean | undefined | Provider<boolean | undefined>;
1213
debugMode?: boolean | undefined | Provider<boolean | undefined>;
1314
nonConflictingParam?: string | undefined | Provider<string | undefined>;
15+
logger?: string | undefined | Provider<string | undefined>;
1416
};
1517
endpoint?: string | Provider<string> | Endpoint | Provider<Endpoint> | EndpointV2 | Provider<EndpointV2>;
16-
apiKey?: string | undefined | Provider<string | undefined>;
1718
customParam?: string | undefined | Provider<string | undefined>;
1819
enableFeature?: boolean | undefined | Provider<boolean | undefined>;
1920
debugMode?: boolean | undefined | Provider<boolean | undefined>;
@@ -31,10 +32,7 @@ export type ClientResolvedEndpointParameters = Omit<ClientInputEndpointParameter
3132
* @internal
3233
*/
3334
const clientContextParamDefaults = {
34-
nonConflictingParam: "non-conflict-default",
35-
customParam: "default-custom-value",
36-
debugMode: false,
37-
enableFeature: true,
35+
logger: "default-logger",
3836
} as const;
3937

4038
/**
@@ -59,6 +57,7 @@ export const resolveClientEndpointParameters = <T>(
5957
export const commonParams = {
6058
ApiKey: { type: "clientContextParams", name: "apiKey" },
6159
nonConflictingParam: { type: "clientContextParams", name: "nonConflictingParam" },
60+
logger: { type: "clientContextParams", name: "logger" },
6261
region: { type: "clientContextParams", name: "region" },
6362
customParam: { type: "clientContextParams", name: "customParam" },
6463
debugMode: { type: "clientContextParams", name: "debugMode" },
@@ -77,4 +76,5 @@ export interface EndpointParameters extends __EndpointParameters {
7776
enableFeature?: boolean | undefined;
7877
debugMode?: boolean | undefined;
7978
nonConflictingParam?: string | undefined;
79+
logger?: string | undefined;
8080
}

private/my-local-model-schema/src/endpoint/ruleset.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ export const ruleSet: RuleSetObject = {
4444
default: "non-conflict-default",
4545
documentation: "Non-conflicting with default",
4646
},
47+
logger: {
48+
type: "String",
49+
required: true,
50+
default: "default-logger",
51+
documentation: "Conflicting logger with default",
52+
},
4753
},
4854
rules: [
4955
{

private/my-local-model/src/endpoint/EndpointParameters.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,14 @@ import type { Endpoint, EndpointParameters as __EndpointParameters, EndpointV2,
77
export interface ClientInputEndpointParameters {
88
clientContextParams?: {
99
apiKey?: string | undefined | Provider<string | undefined>;
10+
region?: string | undefined | Provider<string | undefined>;
1011
customParam?: string | undefined | Provider<string | undefined>;
1112
enableFeature?: boolean | undefined | Provider<boolean | undefined>;
1213
debugMode?: boolean | undefined | Provider<boolean | undefined>;
1314
nonConflictingParam?: string | undefined | Provider<string | undefined>;
15+
logger?: string | undefined | Provider<string | undefined>;
1416
};
1517
endpoint?: string | Provider<string> | Endpoint | Provider<Endpoint> | EndpointV2 | Provider<EndpointV2>;
16-
apiKey?: string | undefined | Provider<string | undefined>;
1718
customParam?: string | undefined | Provider<string | undefined>;
1819
enableFeature?: boolean | undefined | Provider<boolean | undefined>;
1920
debugMode?: boolean | undefined | Provider<boolean | undefined>;
@@ -31,10 +32,7 @@ export type ClientResolvedEndpointParameters = Omit<ClientInputEndpointParameter
3132
* @internal
3233
*/
3334
const clientContextParamDefaults = {
34-
nonConflictingParam: "non-conflict-default",
35-
customParam: "default-custom-value",
36-
debugMode: false,
37-
enableFeature: true,
35+
logger: "default-logger",
3836
} as const;
3937

4038
/**
@@ -59,6 +57,7 @@ export const resolveClientEndpointParameters = <T>(
5957
export const commonParams = {
6058
ApiKey: { type: "clientContextParams", name: "apiKey" },
6159
nonConflictingParam: { type: "clientContextParams", name: "nonConflictingParam" },
60+
logger: { type: "clientContextParams", name: "logger" },
6261
region: { type: "clientContextParams", name: "region" },
6362
customParam: { type: "clientContextParams", name: "customParam" },
6463
debugMode: { type: "clientContextParams", name: "debugMode" },
@@ -77,4 +76,5 @@ export interface EndpointParameters extends __EndpointParameters {
7776
enableFeature?: boolean | undefined;
7877
debugMode?: boolean | undefined;
7978
nonConflictingParam?: string | undefined;
79+
logger?: string | undefined;
8080
}

private/my-local-model/src/endpoint/ruleset.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ export const ruleSet: RuleSetObject = {
4444
default: "non-conflict-default",
4545
documentation: "Non-conflicting with default",
4646
},
47+
logger: {
48+
type: "String",
49+
required: true,
50+
default: "default-logger",
51+
documentation: "Conflicting logger with default",
52+
},
4753
},
4854
rules: [
4955
{

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/ClientConfigKeys.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public final class ClientConfigKeys {
2121
static {
2222
// Initialize with common client config keys
2323
KNOWN_CONFIG_KEYS.add("profile");
24+
KNOWN_CONFIG_KEYS.add("apiKey");
2425
KNOWN_CONFIG_KEYS.add("region");
2526
KNOWN_CONFIG_KEYS.add("credentials");
2627
KNOWN_CONFIG_KEYS.add("endpoint");
@@ -83,6 +84,16 @@ public static void addConfigKey(String key) {
8384
KNOWN_CONFIG_KEYS.add(key);
8485
}
8586

87+
/**
88+
* Check if a key is a known configuration key.
89+
*
90+
* @param key the configuration key to check
91+
* @return true if the key is known
92+
*/
93+
public static boolean isKnownConfigKey(String key) {
94+
return KNOWN_CONFIG_KEYS.contains(key);
95+
}
96+
8697
/**
8798
* Get custom context parameters by filtering out built-in and known config
8899
* keys.

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2Generator.java

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -124,16 +124,31 @@ private void generateEndpointParameters() {
124124
"export interface ClientInputEndpointParameters {",
125125
"}",
126126
() -> {
127-
if (ruleSetParameterFinder.hasCustomClientContextParams()) {
127+
Map<String, String> allClientContextParams = ruleSetParameterFinder.getClientContextParams();
128+
if (!allClientContextParams.isEmpty()) {
128129
writer.write("clientContextParams?: {");
129130
writer.indent();
130-
ruleSetParameterFinder.writeInputConfigCustomClientContextParams(writer);
131+
ObjectNode ruleSet = endpointRuleSetTrait.getRuleSet().expectObjectNode();
132+
ruleSet.getObjectMember("parameters").ifPresent(parameters -> {
133+
parameters.accept(new RuleSetParametersVisitor(writer, allClientContextParams, true));
134+
});
131135
writer.dedent();
132136
writer.write("};");
133137
}
134-
// Add direct params (built-ins + custom context params)
135-
Map<String, String> directParams = new HashMap<>(builtInParams);
136-
directParams.putAll(customContextParams);
138+
// Add direct params (built-ins + custom context params, excluding conflicting)
139+
Map<String, String> directParams = new HashMap<>();
140+
// Add all built-ins (they should always be at root level, even if conflicting)
141+
directParams.putAll(builtInParams);
142+
// Add custom context params excluding conflicting ones
143+
customContextParams.entrySet().forEach(entry -> {
144+
String paramName = entry.getKey();
145+
String localName = EndpointsParamNameMap
146+
.getLocalName(paramName);
147+
if (!ClientConfigKeys.isKnownConfigKey(paramName)
148+
&& !ClientConfigKeys.isKnownConfigKey(localName)) {
149+
directParams.put(paramName, entry.getValue());
150+
}
151+
});
137152
ObjectNode ruleSet = endpointRuleSetTrait.getRuleSet().expectObjectNode();
138153
ruleSet.getObjectMember("parameters").ifPresent(parameters -> {
139154
parameters.accept(new RuleSetParametersVisitor(writer, directParams, true));

0 commit comments

Comments
 (0)