Skip to content

Commit 2c4a286

Browse files
committed
resolved comments
1 parent 95c6233 commit 2c4a286

5 files changed

Lines changed: 36 additions & 70 deletions

File tree

src/appConfigurationImpl.ts

Lines changed: 13 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
110110
#secretRefreshEnabled: boolean = false;
111111
#secretReferences: ConfigurationSetting[] = []; // cached key vault references
112112
#secretRefreshTimer: RefreshTimer | undefined = undefined;
113-
#resolveSecretsInParallel: boolean = false;
114113

115114
/**
116115
* Selectors of key-values obtained from @see AzureAppConfigurationOptions.selectors
@@ -202,7 +201,6 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
202201
this.#secretRefreshEnabled = true;
203202
this.#secretRefreshTimer = new RefreshTimer(secretRefreshIntervalInMs);
204203
}
205-
this.#resolveSecretsInParallel = options.keyVaultOptions.parallelSecretResolutionEnabled ?? false;
206204
}
207205
this.#adapters.push(new AzureKeyVaultKeyValueAdapter(options?.keyVaultOptions, this.#secretRefreshTimer));
208206
this.#adapters.push(new JsonKeyValueAdapter());
@@ -558,25 +556,19 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
558556
this.#aiConfigurationTracing.reset();
559557
}
560558

559+
for (const adapter of this.#adapters) {
560+
await adapter.preload?.(loadedSettings.filter(s => adapter.canProcess(s))); // dedup and warm the secret cache
561+
}
562+
561563
for (const setting of loadedSettings) {
562564
if (isSecretReference(setting)) {
563565
this.#secretReferences.push(setting); // cache secret references for resolve/refresh secret separately
564-
continue;
565566
}
566567
// adapt configuration settings to key-values
567568
const [key, value] = await this.#processKeyValue(setting);
568569
keyValues.push([key, value]);
569570
}
570571

571-
if (this.#secretReferences.length > 0) {
572-
for (const adapter of this.#adapters) {
573-
await adapter.preload?.(this.#secretReferences); // dedup and warm the secret cache
574-
}
575-
await this.#resolveSecretReferences(this.#secretReferences, (key, value) => {
576-
keyValues.push([key, value]);
577-
});
578-
}
579-
580572
this.#clearLoadedKeyValues(); // clear existing key-values in case of configuration setting deletion
581573
for (const [k, v] of keyValues) {
582574
this.#configMap.set(k, v); // reset the configuration
@@ -674,7 +666,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
674666

675667
if (needRefresh) {
676668
for (const adapter of this.#adapters) {
677-
await adapter.onChangeDetected();
669+
await adapter.onChangeDetected?.();
678670
}
679671
await this.#loadSelectedKeyValues();
680672

@@ -718,12 +710,16 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
718710
return Promise.resolve(false);
719711
}
720712

721-
for (const adapter of this.#adapters) {
722-
await adapter.preload?.(this.#secretReferences); // dedup and warm the secret cache
713+
const keyVaultRefAdapter = this.#adapters.find(adapter => adapter instanceof AzureKeyVaultKeyValueAdapter) as AzureKeyVaultKeyValueAdapter | undefined;
714+
if (keyVaultRefAdapter) {
715+
// dedup and warm the secret cache
716+
await keyVaultRefAdapter.preload(this.#secretReferences);
723717
}
724-
await this.#resolveSecretReferences(this.#secretReferences, (key, value) => {
718+
719+
for (const setting of this.#secretReferences) {
720+
const [key, value] = await this.#processKeyValue(setting);
725721
this.#configMap.set(key, value);
726-
});
722+
}
727723

728724
this.#secretRefreshTimer.reset();
729725
return Promise.resolve(true);
@@ -899,27 +895,6 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
899895
throw new Error(ErrorMessages.ALL_FALLBACK_CLIENTS_FAILED);
900896
}
901897

902-
async #resolveSecretReferences(secretReferences: ConfigurationSetting[], resultHandler: (key: string, value: unknown) => void): Promise<void> {
903-
if (this.#resolveSecretsInParallel) {
904-
const secretResolutionPromises: Promise<void>[] = [];
905-
for (const setting of secretReferences) {
906-
const secretResolutionPromise = this.#processKeyValue(setting)
907-
.then(([key, value]) => {
908-
resultHandler(key, value);
909-
});
910-
secretResolutionPromises.push(secretResolutionPromise);
911-
}
912-
913-
// Wait for all secret resolution promises to be resolved
914-
await Promise.all(secretResolutionPromises);
915-
} else {
916-
for (const setting of secretReferences) {
917-
const [key, value] = await this.#processKeyValue(setting);
918-
resultHandler(key, value);
919-
}
920-
}
921-
}
922-
923898
async #processKeyValue(setting: ConfigurationSetting<string>): Promise<[string, unknown]> {
924899
this.#setAIConfigurationTracing(setting);
925900

src/jsonKeyValueAdapter.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,7 @@ export class JsonKeyValueAdapter implements IKeyValueAdapter {
4343
return [setting.key, parsedValue];
4444
}
4545

46-
async onChangeDetected(): Promise<void> {
47-
return;
48-
}
49-
50-
#tryParseJson(value: string): { success: true; result: unknown } | { success: false } {
46+
#tryParseJson(value: string): { success: true; result: unknown } | { success: false } {
5147
try {
5248
return { success: true, result: JSON.parse(value) };
5349
} catch (error) {

src/keyValueAdapter.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,5 @@ export interface IKeyValueAdapter {
2222
/**
2323
* This method is called when a change is detected in the configuration setting.
2424
*/
25-
onChangeDetected(): Promise<void>;
25+
onChangeDetected?(): Promise<void>;
2626
}

src/keyvault/keyVaultKeyValueAdapter.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter {
5151

5252
async preload(settings: ConfigurationSetting[]): Promise<void> {
5353
if (!this.#keyVaultOptions) {
54-
return; // nothing to do; processKeyValue will throw the proper ArgumentError
54+
return; // no-op when keyVaultOptions is not configured
5555
}
5656
const uniqueSecretIdentifiers = new Map<string, KeyVaultSecretIdentifier>();
5757
for (const setting of settings) {
@@ -67,7 +67,22 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter {
6767
// Skip invalid references; processKeyValue re-parses and raises KeyVaultReferenceError with context.
6868
}
6969
}
70-
await this.#keyVaultSecretProvider.preloadSecrets([...uniqueSecretIdentifiers.values()]);
70+
71+
const loadSecret = async (secretIdentifier: KeyVaultSecretIdentifier) => {
72+
try {
73+
await this.#keyVaultSecretProvider.loadSecretValue(secretIdentifier);
74+
} catch {
75+
// Leave uncached; getSecretValue re-fetches and surfaces the error during resolution.
76+
}
77+
};
78+
79+
if (this.#keyVaultOptions?.parallelSecretResolutionEnabled) {
80+
await Promise.all([...uniqueSecretIdentifiers.values()].map(loadSecret));
81+
} else {
82+
for (const secretIdentifier of uniqueSecretIdentifiers.values()) {
83+
await loadSecret(secretIdentifier);
84+
}
85+
}
7186
}
7287

7388
async onChangeDetected(): Promise<void> {

src/keyvault/keyVaultSecretProvider.ts

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -33,26 +33,7 @@ export class AzureKeyVaultSecretProvider {
3333
}
3434
}
3535

36-
// Fetches the given unique secrets to warm the cache.
37-
async preloadSecrets(secretIdentifiers: KeyVaultSecretIdentifier[]): Promise<void> {
38-
const loadSecret = async (secretIdentifier: KeyVaultSecretIdentifier) => {
39-
try {
40-
await this.#loadSecretValue(secretIdentifier);
41-
} catch {
42-
// Leave uncached; getSecretValue re-fetches and surfaces the error during resolution.
43-
}
44-
};
45-
46-
if (this.#keyVaultOptions?.parallelSecretResolutionEnabled) {
47-
await Promise.all(secretIdentifiers.map(loadSecret));
48-
} else {
49-
for (const secretIdentifier of secretIdentifiers) {
50-
await loadSecret(secretIdentifier);
51-
}
52-
}
53-
}
54-
55-
async #loadSecretValue(secretIdentifier: KeyVaultSecretIdentifier): Promise<void> {
36+
async loadSecretValue(secretIdentifier: KeyVaultSecretIdentifier): Promise<void> {
5637
const identifierKey = secretIdentifier.sourceId;
5738
const shouldRefresh = this.#secretRefreshTimer?.canRefresh() ?? false;
5839
if (this.#cachedSecretValues.has(identifierKey) && !shouldRefresh) {
@@ -67,10 +48,9 @@ export class AzureKeyVaultSecretProvider {
6748
return this.#cachedSecretValues.get(identifierKey);
6849
}
6950

70-
// Fallback for secrets that preload skipped or failed to fetch.
71-
const secretValue = await this.#getSecretValueFromKeyVault(secretIdentifier);
72-
this.#cachedSecretValues.set(identifierKey, secretValue);
73-
return secretValue;
51+
// Fallback for secrets that preload skipped or failed to fetch. loadSecretValue populates the cache.
52+
await this.loadSecretValue(secretIdentifier);
53+
return this.#cachedSecretValues.get(identifierKey);
7454
}
7555

7656
clearCache(): void {

0 commit comments

Comments
 (0)