From c8ab1c965b9c0ab1868f12c2bf67ef1d29027b5a Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Mon, 23 Oct 2023 13:53:01 +0800 Subject: [PATCH 01/43] Support dynamic refresh --- .eslintrc | 3 +- examples/refresh.mjs | 43 +++++++++ src/AzureAppConfiguration.ts | 17 +++- src/AzureAppConfigurationImpl.ts | 89 ++++++++++++++---- src/AzureAppConfigurationOptions.ts | 5 + src/RefreshOptions.ts | 21 +++++ src/WatchedSetting.ts | 7 ++ src/common/disposable.ts | 15 +++ src/common/linkedList.ts | 140 ++++++++++++++++++++++++++++ src/requestTracing/utils.ts | 4 +- test/refresh.test.js | 140 ++++++++++++++++++++++++++++ test/utils/testHelper.js | 9 ++ 12 files changed, 472 insertions(+), 21 deletions(-) create mode 100644 examples/refresh.mjs create mode 100644 src/RefreshOptions.ts create mode 100644 src/WatchedSetting.ts create mode 100644 src/common/disposable.ts create mode 100644 src/common/linkedList.ts create mode 100644 test/refresh.test.js diff --git a/.eslintrc b/.eslintrc index 754a9f38..8b96047e 100644 --- a/.eslintrc +++ b/.eslintrc @@ -40,5 +40,6 @@ "avoidEscape": true } ], - }, + "@typescript-eslint/no-explicit-any": "off" + } } \ No newline at end of file diff --git a/examples/refresh.mjs b/examples/refresh.mjs new file mode 100644 index 00000000..5d49fdd2 --- /dev/null +++ b/examples/refresh.mjs @@ -0,0 +1,43 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import * as dotenv from "dotenv"; +import { promisify } from "util"; +dotenv.config(); +const sleepInMs = promisify(setTimeout); + +/** + * This example retrives all settings with key following pattern "app.settings.*", i.e. starting with "app.settings.". + * With the option `trimKeyPrefixes`, it trims the prefix "app.settings." from keys for simplicity. + * Value of config "app.settings.message" will be printed. + * + * Below environment variables are required for this example: + * - APPCONFIG_CONNECTION_STRING + */ + +import { load } from "@azure/app-configuration-provider"; +const connectionString = process.env.APPCONFIG_CONNECTION_STRING; +const settings = await load(connectionString, { + selectors: [{ + keyFilter: "app.settings.*" + }], + trimKeyPrefixes: ["app.settings."], + refreshOptions: { + watchedSettings: [{ key: "app.settings.sentinel" }], + refreshIntervalInMs: 10 * 1000 // Default value is 30 seconds, shorted for this sample + } +}); + +console.log("Update the `message` in your App Configuration store using Azure portal or CLI.") +console.log("First, update the `message` value, and then update the `sentinel` key value.") + +while (true) { + // Refreshing the configuration setting + await settings.refresh(); + + // Current value of message + console.log(settings.get("message")); + + // Waiting before the next refresh + await sleepInMs(5000); +} \ No newline at end of file diff --git a/src/AzureAppConfiguration.ts b/src/AzureAppConfiguration.ts index cd4cdc2a..c7f9efec 100644 --- a/src/AzureAppConfiguration.ts +++ b/src/AzureAppConfiguration.ts @@ -1,6 +1,19 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. +import { Disposable } from "./common/disposable"; + export type AzureAppConfiguration = { - // methods for advanced features, e.g. refresh() -} & ReadonlyMap; + /** + * API to trigger refresh operation. + */ + refresh(): Promise; + + /** + * API to register callback listeners, which will be called only when a refresh operation successfully updates key-values. + * + * @param listener Callback funtion to be registered. + * @param thisArg Optional. Value to use as this when executing callback. + */ + onRefresh(listener: () => any, thisArg?: any): Disposable; +} & ReadonlyMap; diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index faaf2524..878bb396 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -9,8 +9,11 @@ import { JsonKeyValueAdapter } from "./JsonKeyValueAdapter"; import { KeyFilter } from "./KeyFilter"; import { LabelFilter } from "./LabelFilter"; import { AzureKeyVaultKeyValueAdapter } from "./keyvault/AzureKeyVaultKeyValueAdapter"; -import { CorrelationContextHeaderName } from "./requestTracing/constants"; +import { CorrelationContextHeaderName, RequestType } from "./requestTracing/constants"; import { createCorrelationContextHeader, requestTracingEnabled } from "./requestTracing/utils"; +import { DefaultRefreshIntervalInMs, MinimumRefreshIntervalInMs } from "./RefreshOptions"; +import { LinkedList } from "./common/linkedList"; +import { Disposable } from "./common/disposable"; export class AzureAppConfigurationImpl extends Map implements AzureAppConfiguration { private adapters: IKeyValueAdapter[] = []; @@ -20,7 +23,10 @@ export class AzureAppConfigurationImpl extends Map implements A */ private sortedTrimKeyPrefixes: string[] | undefined; private readonly requestTracingEnabled: boolean; - private correlationContextHeader: string | undefined; + // Refresh + private refreshIntervalInMs: number; + private onRefreshListeners: LinkedList<() => any>; + private lastUpdateTimestamp: number; constructor( private client: AppConfigurationClient, @@ -29,20 +35,32 @@ export class AzureAppConfigurationImpl extends Map implements A super(); // Enable request tracing if not opt-out this.requestTracingEnabled = requestTracingEnabled(); - if (this.requestTracingEnabled) { - this.enableRequestTracing(); - } if (options?.trimKeyPrefixes) { this.sortedTrimKeyPrefixes = [...options.trimKeyPrefixes].sort((a, b) => b.localeCompare(a)); } + + if (options?.refreshOptions) { + this.onRefreshListeners = new LinkedList(); + this.refreshIntervalInMs = DefaultRefreshIntervalInMs; + + const refreshIntervalInMs = this.options?.refreshOptions?.refreshIntervalInMs; + if (refreshIntervalInMs !== undefined) { + if (refreshIntervalInMs < MinimumRefreshIntervalInMs) { + throw new Error(`The refresh interval time cannot be less than ${MinimumRefreshIntervalInMs} milliseconds.`); + } else { + this.refreshIntervalInMs = refreshIntervalInMs; + } + } + } + // TODO: should add more adapters to process different type of values // feature flag, others this.adapters.push(new AzureKeyVaultKeyValueAdapter(options?.keyVaultOptions)); this.adapters.push(new JsonKeyValueAdapter()); } - public async load() { + public async load(requestType: RequestType = RequestType.Startup) { const keyValues: [key: string, value: unknown][] = []; // validate selectors @@ -55,7 +73,7 @@ export class AzureAppConfigurationImpl extends Map implements A }; if (this.requestTracingEnabled) { listOptions.requestOptions = { - customHeaders: this.customHeaders() + customHeaders: this.customHeaders(requestType) } } @@ -63,15 +81,58 @@ export class AzureAppConfigurationImpl extends Map implements A for await (const setting of settings) { if (setting.key) { - const [key, value] = await this.processAdapters(setting); - const trimmedKey = this.keyWithPrefixesTrimmed(key); - keyValues.push([trimmedKey, value]); + const keyValuePair = await this.processKeyValues(setting); + keyValues.push(keyValuePair); } } } for (const [k, v] of keyValues) { this.set(k, v); } + this.lastUpdateTimestamp = Date.now(); + } + + public async refresh(): Promise { + // if no refreshOptions set, return + if (this.options?.refreshOptions === undefined || this.options.refreshOptions.watchedSettings.length === 0) { + return Promise.resolve(); + } + // if still within refresh interval, return + const now = Date.now(); + if (now < this.lastUpdateTimestamp + this.refreshIntervalInMs) { + return Promise.resolve(); + } + + // try refresh if any of watched settings is changed. + // TODO: watchedSettings as optional, etag based refresh if not specified. + let needRefresh = false; + for (const watchedSetting of this.options.refreshOptions.watchedSettings) { + const response = await this.client.getConfigurationSetting(watchedSetting); + const [key, value] = await this.processKeyValues(response); + if (value !== this.get(key)) { + needRefresh = true; + break; + } + } + if (needRefresh) { + await this.load(RequestType.Watch); + // run callbacks in async + for (const listener of this.onRefreshListeners) { + listener(); + } + } + } + + public onRefresh(listener: () => any, thisArg?: any): Disposable { + const boundedListener = listener.bind(thisArg); + const remove = this.onRefreshListeners.push(boundedListener); + return new Disposable(remove); + } + + private async processKeyValues(setting: ConfigurationSetting): Promise<[string, unknown]> { + const [key, value] = await this.processAdapters(setting); + const trimmedKey = this.keyWithPrefixesTrimmed(key); + return [trimmedKey, value]; } private async processAdapters(setting: ConfigurationSetting): Promise<[string, unknown]> { @@ -94,17 +155,13 @@ export class AzureAppConfigurationImpl extends Map implements A return key; } - private enableRequestTracing() { - this.correlationContextHeader = createCorrelationContextHeader(this.options); - } - - private customHeaders() { + private customHeaders(requestType: RequestType) { if (!this.requestTracingEnabled) { return undefined; } const headers = {}; - headers[CorrelationContextHeaderName] = this.correlationContextHeader; + headers[CorrelationContextHeaderName] = createCorrelationContextHeader(this.options, requestType); return headers; } } diff --git a/src/AzureAppConfigurationOptions.ts b/src/AzureAppConfigurationOptions.ts index 11a9ba5b..21bce58f 100644 --- a/src/AzureAppConfigurationOptions.ts +++ b/src/AzureAppConfigurationOptions.ts @@ -3,6 +3,7 @@ import { AppConfigurationClientOptions } from "@azure/app-configuration"; import { AzureAppConfigurationKeyVaultOptions } from "./keyvault/AzureAppConfigurationKeyVaultOptions"; +import { RefreshOptions } from "./RefreshOptions"; export const MaxRetries = 2; export const MaxRetryDelayInMs = 60000; @@ -31,4 +32,8 @@ export interface AzureAppConfigurationOptions { trimKeyPrefixes?: string[]; clientOptions?: AppConfigurationClientOptions; keyVaultOptions?: AzureAppConfigurationKeyVaultOptions; + /** + * Specifies options for dynamic refresh key-values. + */ + refreshOptions?: RefreshOptions; } \ No newline at end of file diff --git a/src/RefreshOptions.ts b/src/RefreshOptions.ts new file mode 100644 index 00000000..620f8a3c --- /dev/null +++ b/src/RefreshOptions.ts @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { WatchedSetting } from "./WatchedSetting"; + +export const DefaultRefreshIntervalInMs = 30 * 1000; +export const MinimumRefreshIntervalInMs = 1 * 1000; + +export interface RefreshOptions { + /** + * Specifies the interval for refresh to really update the values. + * Default value is 30 seconds. Must be greater than 1 second. + * Any refresh operation triggered will not update the value for a key until after the interval. + */ + refreshIntervalInMs?: number; + + /** + * Specifies settings to be watched, to determine whether the provider triggers a refresh. + */ + watchedSettings: WatchedSetting[]; +} \ No newline at end of file diff --git a/src/WatchedSetting.ts b/src/WatchedSetting.ts new file mode 100644 index 00000000..815a26b9 --- /dev/null +++ b/src/WatchedSetting.ts @@ -0,0 +1,7 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +export interface WatchedSetting { + key: string; + label?: string; +} \ No newline at end of file diff --git a/src/common/disposable.ts b/src/common/disposable.ts new file mode 100644 index 00000000..88960137 --- /dev/null +++ b/src/common/disposable.ts @@ -0,0 +1,15 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +export class Disposable { + private disposed = false; + constructor(private callOnDispose: () => any) { } + + dispose() { + if (!this.disposed) { + this.callOnDispose(); + } + this.disposed = true; + } + +} \ No newline at end of file diff --git a/src/common/linkedList.ts b/src/common/linkedList.ts new file mode 100644 index 00000000..5d932c7d --- /dev/null +++ b/src/common/linkedList.ts @@ -0,0 +1,140 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +class Node { + + static readonly Undefined = new Node(undefined); + + element: E; + next: Node; + prev: Node; + + constructor(element: E) { + this.element = element; + this.next = Node.Undefined; + this.prev = Node.Undefined; + } +} + +export class LinkedList { + + private _first: Node = Node.Undefined; + private _last: Node = Node.Undefined; + private _size: number = 0; + + get size(): number { + return this._size; + } + + isEmpty(): boolean { + return this._first === Node.Undefined; + } + + clear(): void { + let node = this._first; + while (node !== Node.Undefined) { + const next = node.next; + node.prev = Node.Undefined; + node.next = Node.Undefined; + node = next; + } + + this._first = Node.Undefined; + this._last = Node.Undefined; + this._size = 0; + } + + unshift(element: E): () => void { + return this._insert(element, false); + } + + push(element: E): () => void { + return this._insert(element, true); + } + + private _insert(element: E, atTheEnd: boolean): () => void { + const newNode = new Node(element); + if (this._first === Node.Undefined) { + this._first = newNode; + this._last = newNode; + + } else if (atTheEnd) { + // push + const oldLast = this._last!; + this._last = newNode; + newNode.prev = oldLast; + oldLast.next = newNode; + + } else { + // unshift + const oldFirst = this._first; + this._first = newNode; + newNode.next = oldFirst; + oldFirst.prev = newNode; + } + this._size += 1; + + let didRemove = false; + return () => { + if (!didRemove) { + didRemove = true; + this._remove(newNode); + } + }; + } + + shift(): E | undefined { + if (this._first === Node.Undefined) { + return undefined; + } else { + const res = this._first.element; + this._remove(this._first); + return res; + } + } + + pop(): E | undefined { + if (this._last === Node.Undefined) { + return undefined; + } else { + const res = this._last.element; + this._remove(this._last); + return res; + } + } + + private _remove(node: Node): void { + if (node.prev !== Node.Undefined && node.next !== Node.Undefined) { + // middle + const anchor = node.prev; + anchor.next = node.next; + node.next.prev = anchor; + + } else if (node.prev === Node.Undefined && node.next === Node.Undefined) { + // only node + this._first = Node.Undefined; + this._last = Node.Undefined; + + } else if (node.next === Node.Undefined) { + // last + this._last = this._last!.prev!; + this._last.next = Node.Undefined; + + } else if (node.prev === Node.Undefined) { + // first + this._first = this._first!.next!; + this._first.prev = Node.Undefined; + } + + // done + this._size -= 1; + } + + *[Symbol.iterator](): Iterator { + let node = this._first; + while (node !== Node.Undefined) { + yield node.element; + node = node.next; + } + } +} diff --git a/src/requestTracing/utils.ts b/src/requestTracing/utils.ts index 95483bfa..519a31e3 100644 --- a/src/requestTracing/utils.ts +++ b/src/requestTracing/utils.ts @@ -21,7 +21,7 @@ import { } from "./constants"; // Utils -export function createCorrelationContextHeader(options: AzureAppConfigurationOptions | undefined): string { +export function createCorrelationContextHeader(options: AzureAppConfigurationOptions | undefined, requestType: RequestType): string { /* RequestType: 'Startup' during application starting up, 'Watch' after startup completed. Host: identify with defined envs @@ -29,7 +29,7 @@ export function createCorrelationContextHeader(options: AzureAppConfigurationOpt UsersKeyVault */ const keyValues = new Map(); - keyValues.set(RequestTypeKey, RequestType.Startup); // TODO: now always "Startup", until refresh is supported. + keyValues.set(RequestTypeKey, requestType); keyValues.set(HostTypeKey, getHostType()); keyValues.set(EnvironmentKey, isDevEnvironment() ? DevEnvironmentValue : undefined); diff --git a/test/refresh.test.js b/test/refresh.test.js new file mode 100644 index 00000000..426afa70 --- /dev/null +++ b/test/refresh.test.js @@ -0,0 +1,140 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +const chai = require("chai"); +const chaiAsPromised = require("chai-as-promised"); +chai.use(chaiAsPromised); +const expect = chai.expect; +const { load } = require("../dist/index"); +const { + mockAppConfigurationClientListConfigurationSettings, + mockAppConfigurationClientGetConfigurationSetting, + restoreMocks, + createMockedConnectionString, + createMockedKeyValue, +} = require("./utils/testHelper"); +const { promisify } = require("util") +const sleepInMs = promisify(setTimeout); + +let mockedKVs = []; + +function updateSetting(key, value) { + const setting = mockedKVs.find(elem => elem.key === key); + if (setting) { + setting.value = value; + } +} +function addSetting(key, value) { + mockedKVs.push(createMockedKeyValue({key, value})); +} + +describe("dynamic refresh", function () { + beforeEach(() => { + mockedKVs = [ + { value: "red", key: "app.settings.fontColor" }, + { value: "40", key: "app.settings.fontSize" } + ].map(createMockedKeyValue); + mockAppConfigurationClientListConfigurationSettings(mockedKVs); + mockAppConfigurationClientGetConfigurationSetting(mockedKVs) + }); + + afterEach(() => { + restoreMocks(); + }) + + it("should only udpate values after refreshInterval", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + refreshOptions: { + refreshIntervalInMs: 2000, + watchedSettings: [ + { key: "app.settings.fontColor" } + ] + } + }); + expect(settings).not.undefined; + expect(settings.get("app.settings.fontColor")).eq("red"); + expect(settings.get("app.settings.fontSize")).eq("40"); + + // change setting + updateSetting("app.settings.fontColor", "blue"); + + // within refreshInterval, should not really refresh + await settings.refresh(); + expect(settings.get("app.settings.fontColor")).eq("red"); + + // after refreshInterval, should really refresh + await sleepInMs(2 * 1000); + await settings.refresh(); + expect(settings.get("app.settings.fontColor")).eq("blue"); + }); + + it("should not update values when unwatched setting changes", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + refreshOptions: { + refreshIntervalInMs: 2000, + watchedSettings: [ + { key: "app.settings.fontColor" } + ] + } + }); + expect(settings).not.undefined; + expect(settings.get("app.settings.fontColor")).eq("red"); + expect(settings.get("app.settings.fontSize")).eq("40"); + + updateSetting("app.settings.fontSize", "50"); // unwatched setting + await sleepInMs(2 * 1000); + await settings.refresh(); + expect(settings.get("app.settings.fontSize")).eq("40"); + }); + + it("should watch multiple settings if specified", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + refreshOptions: { + refreshIntervalInMs: 2000, + watchedSettings: [ + { key: "app.settings.fontColor" }, + { key: "app.settings.fontSize" } + ] + } + }); + expect(settings).not.undefined; + expect(settings.get("app.settings.fontColor")).eq("red"); + expect(settings.get("app.settings.fontSize")).eq("40"); + + // change setting + addSetting("app.settings.bgColor", "white"); + updateSetting("app.settings.fontSize", "50"); + await sleepInMs(2 * 1000); + await settings.refresh(); + expect(settings.get("app.settings.fontSize")).eq("50"); + expect(settings.get("app.settings.bgColor")).eq("white"); + }); + + it("should execute callbacks on successful refresh", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + refreshOptions: { + refreshIntervalInMs: 2000, + watchedSettings: [ + { key: "app.settings.fontColor" } + ] + } + }); + let count = 0; + const callback = settings.onRefresh(() => count++); + + updateSetting("app.settings.fontColor", "blue"); + await settings.refresh(); + expect(count).eq(0); + + await sleepInMs(2000); + await settings.refresh(); + expect(count).eq(1); + + // can dispose callbacks + callback.dispose(); + }); +}); \ No newline at end of file diff --git a/test/utils/testHelper.js b/test/utils/testHelper.js index b58aa859..e216935d 100644 --- a/test/utils/testHelper.js +++ b/test/utils/testHelper.js @@ -28,6 +28,14 @@ function mockAppConfigurationClientListConfigurationSettings(kvList) { }); } +function mockAppConfigurationClientGetConfigurationSetting(kvList) { + sinon.stub(AppConfigurationClient.prototype, "getConfigurationSetting").callsFake((settingId) => { + const key = settingId.key; + const label = settingId.label ?? null; + return kvList.find(elem => elem.key === key && elem.label === label); + }); +} + // uriValueList: [["", "value"], ...] function mockSecretClientGetSecret(uriValueList) { const dict = new Map(); @@ -99,6 +107,7 @@ const createMockedKeyValue = (props) => (Object.assign({ module.exports = { sinon, mockAppConfigurationClientListConfigurationSettings, + mockAppConfigurationClientGetConfigurationSetting, mockSecretClientGetSecret, restoreMocks, From b2822117ade28e19f709fee952fcaf488e8debab Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Tue, 31 Oct 2023 17:51:52 +0800 Subject: [PATCH 02/43] etag-based refresh --- examples/refresh.mjs | 1 + src/AzureAppConfigurationImpl.ts | 46 ++++++++++++++++++++++++-------- test/refresh.test.js | 12 +++++---- test/utils/testHelper.js | 11 ++++++-- 4 files changed, 52 insertions(+), 18 deletions(-) diff --git a/examples/refresh.mjs b/examples/refresh.mjs index 5d49fdd2..9d7cb77f 100644 --- a/examples/refresh.mjs +++ b/examples/refresh.mjs @@ -31,6 +31,7 @@ const settings = await load(connectionString, { console.log("Update the `message` in your App Configuration store using Azure portal or CLI.") console.log("First, update the `message` value, and then update the `sentinel` key value.") +// eslint-disable-next-line no-constant-condition while (true) { // Refreshing the configuration setting await settings.refresh(); diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 878bb396..744ba60e 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { AppConfigurationClient, ConfigurationSetting, ListConfigurationSettingsOptions } from "@azure/app-configuration"; +import { AppConfigurationClient, ConfigurationSetting, ConfigurationSettingId, ListConfigurationSettingsOptions } from "@azure/app-configuration"; import { AzureAppConfiguration } from "./AzureAppConfiguration"; import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions"; import { IKeyValueAdapter } from "./IKeyValueAdapter"; @@ -24,9 +24,10 @@ export class AzureAppConfigurationImpl extends Map implements A private sortedTrimKeyPrefixes: string[] | undefined; private readonly requestTracingEnabled: boolean; // Refresh - private refreshIntervalInMs: number; - private onRefreshListeners: LinkedList<() => any>; + private refreshIntervalInMs: number | undefined; + private onRefreshListeners: LinkedList<() => any> | undefined; private lastUpdateTimestamp: number; + private sentinels: ConfigurationSettingId[] | undefined; constructor( private client: AppConfigurationClient, @@ -52,6 +53,15 @@ export class AzureAppConfigurationImpl extends Map implements A this.refreshIntervalInMs = refreshIntervalInMs; } } + + this.sentinels = options.refreshOptions.watchedSettings?.map(setting => { + const key = setting.key; + const label = setting.label; + if (key.includes("*") || label?.includes("*")) { + throw new Error("Wildcard key or label filters are not supported for refresh."); + } + return { key, label }; + }); } // TODO: should add more adapters to process different type of values @@ -84,6 +94,11 @@ export class AzureAppConfigurationImpl extends Map implements A const keyValuePair = await this.processKeyValues(setting); keyValues.push(keyValuePair); } + // update etag of sentinels + const matchedSentinel = this.sentinels?.find(s => s.key === setting.key && (s.label ?? null) === setting.label); // Workaround: as undefined label represents the same with null. + if (matchedSentinel) { + matchedSentinel.etag = setting.etag; + } } } for (const [k, v] of keyValues) { @@ -94,7 +109,7 @@ export class AzureAppConfigurationImpl extends Map implements A public async refresh(): Promise { // if no refreshOptions set, return - if (this.options?.refreshOptions === undefined || this.options.refreshOptions.watchedSettings.length === 0) { + if (this.sentinels === undefined || this.sentinels.length === 0 || this.refreshIntervalInMs === undefined) { return Promise.resolve(); } // if still within refresh interval, return @@ -104,12 +119,15 @@ export class AzureAppConfigurationImpl extends Map implements A } // try refresh if any of watched settings is changed. - // TODO: watchedSettings as optional, etag based refresh if not specified. let needRefresh = false; - for (const watchedSetting of this.options.refreshOptions.watchedSettings) { - const response = await this.client.getConfigurationSetting(watchedSetting); - const [key, value] = await this.processKeyValues(response); - if (value !== this.get(key)) { + for (const sentinel of this.sentinels) { + const response = await this.client.getConfigurationSetting(sentinel, { + onlyIfChanged: true + // TODO: do we trace this request by adding custom headers? + }); + if (response.statusCode !== 304) { // TODO: can be more robust, e.g. === 200? + // sentinel changed. + sentinel.etag = response.etag;// update etag of the sentinel needRefresh = true; break; } @@ -117,13 +135,19 @@ export class AzureAppConfigurationImpl extends Map implements A if (needRefresh) { await this.load(RequestType.Watch); // run callbacks in async - for (const listener of this.onRefreshListeners) { - listener(); + if (this.onRefreshListeners !== undefined) { + for (const listener of this.onRefreshListeners) { + listener(); + } } } } public onRefresh(listener: () => any, thisArg?: any): Disposable { + if (this.onRefreshListeners === undefined) { + // TODO: Add unit tests + throw new Error("Illegal operation because refreshOptions is not provided on loading."); + } const boundedListener = listener.bind(thisArg); const remove = this.onRefreshListeners.push(boundedListener); return new Disposable(remove); diff --git a/test/refresh.test.js b/test/refresh.test.js index 426afa70..b932ac87 100644 --- a/test/refresh.test.js +++ b/test/refresh.test.js @@ -15,6 +15,7 @@ const { } = require("./utils/testHelper"); const { promisify } = require("util") const sleepInMs = promisify(setTimeout); +const uuid = require("uuid"); let mockedKVs = []; @@ -22,10 +23,11 @@ function updateSetting(key, value) { const setting = mockedKVs.find(elem => elem.key === key); if (setting) { setting.value = value; + setting.etag = uuid.v4(); } } function addSetting(key, value) { - mockedKVs.push(createMockedKeyValue({key, value})); + mockedKVs.push(createMockedKeyValue({ key, value })); } describe("dynamic refresh", function () { @@ -64,7 +66,7 @@ describe("dynamic refresh", function () { expect(settings.get("app.settings.fontColor")).eq("red"); // after refreshInterval, should really refresh - await sleepInMs(2 * 1000); + await sleepInMs(2 * 1000 + 1); await settings.refresh(); expect(settings.get("app.settings.fontColor")).eq("blue"); }); @@ -84,7 +86,7 @@ describe("dynamic refresh", function () { expect(settings.get("app.settings.fontSize")).eq("40"); updateSetting("app.settings.fontSize", "50"); // unwatched setting - await sleepInMs(2 * 1000); + await sleepInMs(2 * 1000 + 1); await settings.refresh(); expect(settings.get("app.settings.fontSize")).eq("40"); }); @@ -107,7 +109,7 @@ describe("dynamic refresh", function () { // change setting addSetting("app.settings.bgColor", "white"); updateSetting("app.settings.fontSize", "50"); - await sleepInMs(2 * 1000); + await sleepInMs(2 * 1000 + 1); await settings.refresh(); expect(settings.get("app.settings.fontSize")).eq("50"); expect(settings.get("app.settings.bgColor")).eq("white"); @@ -130,7 +132,7 @@ describe("dynamic refresh", function () { await settings.refresh(); expect(count).eq(0); - await sleepInMs(2000); + await sleepInMs(2 * 1000 + 1); await settings.refresh(); expect(count).eq(1); diff --git a/test/utils/testHelper.js b/test/utils/testHelper.js index e216935d..e0f88636 100644 --- a/test/utils/testHelper.js +++ b/test/utils/testHelper.js @@ -29,10 +29,17 @@ function mockAppConfigurationClientListConfigurationSettings(kvList) { } function mockAppConfigurationClientGetConfigurationSetting(kvList) { - sinon.stub(AppConfigurationClient.prototype, "getConfigurationSetting").callsFake((settingId) => { + sinon.stub(AppConfigurationClient.prototype, "getConfigurationSetting").callsFake((settingId, options) => { const key = settingId.key; const label = settingId.label ?? null; - return kvList.find(elem => elem.key === key && elem.label === label); + const found = kvList.find(elem => elem.key === key && elem.label === label); + if (found) { + if (options?.onlyIfChanged && settingId.etag === found.etag) { + return { statusCode: 304 }; + } else { + return { statusCode: 200, ...found }; + } + } }); } From 8ba125267981331b73cf24ccf5aeae92a662f560 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 9 Nov 2023 09:03:49 +0800 Subject: [PATCH 03/43] rewrite refresh.test with TS --- test/{refresh.test.js => refresh.test.ts} | 24 +++++++++-------------- 1 file changed, 9 insertions(+), 15 deletions(-) rename test/{refresh.test.js => refresh.test.ts} (86%) diff --git a/test/refresh.test.js b/test/refresh.test.ts similarity index 86% rename from test/refresh.test.js rename to test/refresh.test.ts index b932ac87..baf1a974 100644 --- a/test/refresh.test.js +++ b/test/refresh.test.ts @@ -1,32 +1,26 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -const chai = require("chai"); -const chaiAsPromised = require("chai-as-promised"); +import * as chai from "chai"; +import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; -const { load } = require("../dist/index"); -const { - mockAppConfigurationClientListConfigurationSettings, - mockAppConfigurationClientGetConfigurationSetting, - restoreMocks, - createMockedConnectionString, - createMockedKeyValue, -} = require("./utils/testHelper"); -const { promisify } = require("util") +import { load } from "./exportedApi"; +import { mockAppConfigurationClientListConfigurationSettings, mockAppConfigurationClientGetConfigurationSetting, restoreMocks, createMockedConnectionString, createMockedKeyValue } from "./utils/testHelper"; +import { promisify } from "util"; const sleepInMs = promisify(setTimeout); -const uuid = require("uuid"); +import * as uuid from "uuid"; -let mockedKVs = []; +let mockedKVs: any[] = []; -function updateSetting(key, value) { +function updateSetting(key: string, value: any) { const setting = mockedKVs.find(elem => elem.key === key); if (setting) { setting.value = value; setting.etag = uuid.v4(); } } -function addSetting(key, value) { +function addSetting(key: string, value: any) { mockedKVs.push(createMockedKeyValue({ key, value })); } From 261e72f489755e070a8ef7d95472ee3f7d99ac51 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 9 Nov 2023 14:00:32 +0800 Subject: [PATCH 04/43] add refreshOptions.enabled --- src/AzureAppConfigurationImpl.ts | 38 +++++++++++++++++++------------- src/RefreshOptions.ts | 10 +++++++-- test/refresh.test.ts | 29 ++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 17 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 744ba60e..42d41d25 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -15,7 +15,7 @@ import { DefaultRefreshIntervalInMs, MinimumRefreshIntervalInMs } from "./Refres import { LinkedList } from "./common/linkedList"; import { Disposable } from "./common/disposable"; -export class AzureAppConfigurationImpl extends Map implements AzureAppConfiguration { +export class AzureAppConfigurationImpl extends Map implements AzureAppConfiguration { private adapters: IKeyValueAdapter[] = []; /** * Trim key prefixes sorted in descending order. @@ -25,7 +25,7 @@ export class AzureAppConfigurationImpl extends Map implements A private readonly requestTracingEnabled: boolean; // Refresh private refreshIntervalInMs: number | undefined; - private onRefreshListeners: LinkedList<() => any> | undefined; + private onRefreshListeners: LinkedList<() => any> = new LinkedList(); private lastUpdateTimestamp: number; private sentinels: ConfigurationSettingId[] | undefined; @@ -41,26 +41,29 @@ export class AzureAppConfigurationImpl extends Map implements A this.sortedTrimKeyPrefixes = [...options.trimKeyPrefixes].sort((a, b) => b.localeCompare(a)); } - if (options?.refreshOptions) { - this.onRefreshListeners = new LinkedList(); - this.refreshIntervalInMs = DefaultRefreshIntervalInMs; + if (options?.refreshOptions?.enabled) { + const { watchedSettings, refreshIntervalInMs } = options.refreshOptions; + // validate watched settings + if (watchedSettings === undefined || watchedSettings.length === 0) { + throw new Error("Refresh is enabled but no watched settings are specified."); + } - const refreshIntervalInMs = this.options?.refreshOptions?.refreshIntervalInMs; + // process refresh interval if (refreshIntervalInMs !== undefined) { if (refreshIntervalInMs < MinimumRefreshIntervalInMs) { throw new Error(`The refresh interval time cannot be less than ${MinimumRefreshIntervalInMs} milliseconds.`); } else { this.refreshIntervalInMs = refreshIntervalInMs; } + } else { + this.refreshIntervalInMs = DefaultRefreshIntervalInMs; } - this.sentinels = options.refreshOptions.watchedSettings?.map(setting => { - const key = setting.key; - const label = setting.label; - if (key.includes("*") || label?.includes("*")) { - throw new Error("Wildcard key or label filters are not supported for refresh."); + this.sentinels = watchedSettings.map(setting => { + if (setting.key.includes("*") || setting.label?.includes("*")) { + throw new Error("The character '*' is not supported in key or label."); } - return { key, label }; + return { ...setting }; }); } @@ -70,6 +73,11 @@ export class AzureAppConfigurationImpl extends Map implements A this.adapters.push(new JsonKeyValueAdapter()); } + + get _refreshEnabled(): boolean { + return !!this.options?.refreshOptions?.enabled; + } + public async load(requestType: RequestType = RequestType.Startup) { const keyValues: [key: string, value: unknown][] = []; @@ -144,10 +152,10 @@ export class AzureAppConfigurationImpl extends Map implements A } public onRefresh(listener: () => any, thisArg?: any): Disposable { - if (this.onRefreshListeners === undefined) { - // TODO: Add unit tests - throw new Error("Illegal operation because refreshOptions is not provided on loading."); + if (!this._refreshEnabled) { + throw new Error("Refresh is not enabled."); } + const boundedListener = listener.bind(thisArg); const remove = this.onRefreshListeners.push(boundedListener); return new Disposable(remove); diff --git a/src/RefreshOptions.ts b/src/RefreshOptions.ts index 620f8a3c..16217483 100644 --- a/src/RefreshOptions.ts +++ b/src/RefreshOptions.ts @@ -7,6 +7,11 @@ export const DefaultRefreshIntervalInMs = 30 * 1000; export const MinimumRefreshIntervalInMs = 1 * 1000; export interface RefreshOptions { + /** + * Specifies whether the provider should automatically refresh when the configuration is changed. + */ + enabled: boolean; + /** * Specifies the interval for refresh to really update the values. * Default value is 30 seconds. Must be greater than 1 second. @@ -15,7 +20,8 @@ export interface RefreshOptions { refreshIntervalInMs?: number; /** - * Specifies settings to be watched, to determine whether the provider triggers a refresh. + * Specifies settings to be watched. + * Only if any of the settings is updated, the refresh will acutally update loaded configuration settings. */ - watchedSettings: WatchedSetting[]; + watchedSettings?: WatchedSetting[]; } \ No newline at end of file diff --git a/test/refresh.test.ts b/test/refresh.test.ts index baf1a974..adaf70c5 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -38,10 +38,36 @@ describe("dynamic refresh", function () { restoreMocks(); }) + it("should only allow non-empty list of watched settings when refresh is enabled", async () => { + const connectionString = createMockedConnectionString(); + const loadWithEmptyWatchedSettings = load(connectionString, { + refreshOptions: { + enabled: true, + watchedSettings: [] + } + }); + const loadWithUndefinedWatchedSettings = load(connectionString, { + refreshOptions: { + enabled: true + } + }); + return Promise.all([ + expect(loadWithEmptyWatchedSettings).eventually.rejectedWith("Refresh is enabled but no watched settings are specified."), + expect(loadWithUndefinedWatchedSettings).eventually.rejectedWith("Refresh is enabled but no watched settings are specified.") + ]); + }); + + it("should throw error when calling onRefresh when refresh is not enabled", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString); + expect(() => settings.onRefresh(() => { })).throws("Refresh is not enabled."); + }); + it("should only udpate values after refreshInterval", async () => { const connectionString = createMockedConnectionString(); const settings = await load(connectionString, { refreshOptions: { + enabled: true, refreshIntervalInMs: 2000, watchedSettings: [ { key: "app.settings.fontColor" } @@ -69,6 +95,7 @@ describe("dynamic refresh", function () { const connectionString = createMockedConnectionString(); const settings = await load(connectionString, { refreshOptions: { + enabled: true, refreshIntervalInMs: 2000, watchedSettings: [ { key: "app.settings.fontColor" } @@ -89,6 +116,7 @@ describe("dynamic refresh", function () { const connectionString = createMockedConnectionString(); const settings = await load(connectionString, { refreshOptions: { + enabled: true, refreshIntervalInMs: 2000, watchedSettings: [ { key: "app.settings.fontColor" }, @@ -113,6 +141,7 @@ describe("dynamic refresh", function () { const connectionString = createMockedConnectionString(); const settings = await load(connectionString, { refreshOptions: { + enabled: true, refreshIntervalInMs: 2000, watchedSettings: [ { key: "app.settings.fontColor" } From 7ccf26fadc7bc3c2d5b3e04e5a38d8e09f4a3bbb Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 9 Nov 2023 14:25:16 +0800 Subject: [PATCH 05/43] prepend _ to private members for clarity --- src/AzureAppConfigurationImpl.ts | 83 +++++++++++++++----------------- 1 file changed, 39 insertions(+), 44 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 42d41d25..7bc6d86d 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -16,50 +16,48 @@ import { LinkedList } from "./common/linkedList"; import { Disposable } from "./common/disposable"; export class AzureAppConfigurationImpl extends Map implements AzureAppConfiguration { - private adapters: IKeyValueAdapter[] = []; + private _adapters: IKeyValueAdapter[] = []; /** * Trim key prefixes sorted in descending order. * Since multiple prefixes could start with the same characters, we need to trim the longest prefix first. */ - private sortedTrimKeyPrefixes: string[] | undefined; - private readonly requestTracingEnabled: boolean; + private _sortedTrimKeyPrefixes: string[] | undefined; + private readonly _requestTracingEnabled: boolean; // Refresh - private refreshIntervalInMs: number | undefined; - private onRefreshListeners: LinkedList<() => any> = new LinkedList(); - private lastUpdateTimestamp: number; - private sentinels: ConfigurationSettingId[] | undefined; + private _refreshInterval: number = DefaultRefreshIntervalInMs; + private _onRefreshListeners: LinkedList<() => any> = new LinkedList(); + private _lastUpdateTimestamp: number; + private _sentinels: ConfigurationSettingId[]; constructor( - private client: AppConfigurationClient, - private options: AzureAppConfigurationOptions | undefined + private _client: AppConfigurationClient, + private _options: AzureAppConfigurationOptions | undefined ) { super(); // Enable request tracing if not opt-out - this.requestTracingEnabled = requestTracingEnabled(); + this._requestTracingEnabled = requestTracingEnabled(); - if (options?.trimKeyPrefixes) { - this.sortedTrimKeyPrefixes = [...options.trimKeyPrefixes].sort((a, b) => b.localeCompare(a)); + if (_options?.trimKeyPrefixes) { + this._sortedTrimKeyPrefixes = [..._options.trimKeyPrefixes].sort((a, b) => b.localeCompare(a)); } - if (options?.refreshOptions?.enabled) { - const { watchedSettings, refreshIntervalInMs } = options.refreshOptions; + if (_options?.refreshOptions?.enabled) { + const { watchedSettings, refreshIntervalInMs } = _options.refreshOptions; // validate watched settings if (watchedSettings === undefined || watchedSettings.length === 0) { throw new Error("Refresh is enabled but no watched settings are specified."); } - // process refresh interval + // custom refresh interval if (refreshIntervalInMs !== undefined) { if (refreshIntervalInMs < MinimumRefreshIntervalInMs) { throw new Error(`The refresh interval time cannot be less than ${MinimumRefreshIntervalInMs} milliseconds.`); } else { - this.refreshIntervalInMs = refreshIntervalInMs; + this._refreshInterval = refreshIntervalInMs; } - } else { - this.refreshIntervalInMs = DefaultRefreshIntervalInMs; } - this.sentinels = watchedSettings.map(setting => { + this._sentinels = watchedSettings.map(setting => { if (setting.key.includes("*") || setting.label?.includes("*")) { throw new Error("The character '*' is not supported in key or label."); } @@ -69,33 +67,33 @@ export class AzureAppConfigurationImpl extends Map implements Azure // TODO: should add more adapters to process different type of values // feature flag, others - this.adapters.push(new AzureKeyVaultKeyValueAdapter(options?.keyVaultOptions)); - this.adapters.push(new JsonKeyValueAdapter()); + this._adapters.push(new AzureKeyVaultKeyValueAdapter(_options?.keyVaultOptions)); + this._adapters.push(new JsonKeyValueAdapter()); } - get _refreshEnabled(): boolean { - return !!this.options?.refreshOptions?.enabled; + private get _refreshEnabled(): boolean { + return !!this._options?.refreshOptions?.enabled; } public async load(requestType: RequestType = RequestType.Startup) { const keyValues: [key: string, value: unknown][] = []; // validate selectors - const selectors = getValidSelectors(this.options?.selectors); + const selectors = getValidSelectors(this._options?.selectors); for (const selector of selectors) { const listOptions: ListConfigurationSettingsOptions = { keyFilter: selector.keyFilter, labelFilter: selector.labelFilter }; - if (this.requestTracingEnabled) { + if (this._requestTracingEnabled) { listOptions.requestOptions = { customHeaders: this.customHeaders(requestType) } } - const settings = this.client.listConfigurationSettings(listOptions); + const settings = this._client.listConfigurationSettings(listOptions); for await (const setting of settings) { if (setting.key) { @@ -103,7 +101,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure keyValues.push(keyValuePair); } // update etag of sentinels - const matchedSentinel = this.sentinels?.find(s => s.key === setting.key && (s.label ?? null) === setting.label); // Workaround: as undefined label represents the same with null. + const matchedSentinel = this._sentinels?.find(s => s.key === setting.key && (s.label ?? null) === setting.label); // Workaround: as undefined label represents the same with null. if (matchedSentinel) { matchedSentinel.etag = setting.etag; } @@ -112,28 +110,27 @@ export class AzureAppConfigurationImpl extends Map implements Azure for (const [k, v] of keyValues) { this.set(k, v); } - this.lastUpdateTimestamp = Date.now(); + this._lastUpdateTimestamp = Date.now(); } public async refresh(): Promise { - // if no refreshOptions set, return - if (this.sentinels === undefined || this.sentinels.length === 0 || this.refreshIntervalInMs === undefined) { + if (!this._refreshEnabled) { return Promise.resolve(); } // if still within refresh interval, return const now = Date.now(); - if (now < this.lastUpdateTimestamp + this.refreshIntervalInMs) { + if (now < this._lastUpdateTimestamp + this._refreshInterval) { return Promise.resolve(); } // try refresh if any of watched settings is changed. let needRefresh = false; - for (const sentinel of this.sentinels) { - const response = await this.client.getConfigurationSetting(sentinel, { + for (const sentinel of this._sentinels) { + const response = await this._client.getConfigurationSetting(sentinel, { onlyIfChanged: true // TODO: do we trace this request by adding custom headers? }); - if (response.statusCode !== 304) { // TODO: can be more robust, e.g. === 200? + if (response.statusCode === 200) { // sentinel changed. sentinel.etag = response.etag;// update etag of the sentinel needRefresh = true; @@ -143,10 +140,8 @@ export class AzureAppConfigurationImpl extends Map implements Azure if (needRefresh) { await this.load(RequestType.Watch); // run callbacks in async - if (this.onRefreshListeners !== undefined) { - for (const listener of this.onRefreshListeners) { - listener(); - } + for (const listener of this._onRefreshListeners) { + listener(); } } } @@ -157,7 +152,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure } const boundedListener = listener.bind(thisArg); - const remove = this.onRefreshListeners.push(boundedListener); + const remove = this._onRefreshListeners.push(boundedListener); return new Disposable(remove); } @@ -168,7 +163,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure } private async processAdapters(setting: ConfigurationSetting): Promise<[string, unknown]> { - for (const adapter of this.adapters) { + for (const adapter of this._adapters) { if (adapter.canProcess(setting)) { return adapter.processKeyValue(setting); } @@ -177,8 +172,8 @@ export class AzureAppConfigurationImpl extends Map implements Azure } private keyWithPrefixesTrimmed(key: string): string { - if (this.sortedTrimKeyPrefixes) { - for (const prefix of this.sortedTrimKeyPrefixes) { + if (this._sortedTrimKeyPrefixes) { + for (const prefix of this._sortedTrimKeyPrefixes) { if (key.startsWith(prefix)) { return key.slice(prefix.length); } @@ -188,12 +183,12 @@ export class AzureAppConfigurationImpl extends Map implements Azure } private customHeaders(requestType: RequestType) { - if (!this.requestTracingEnabled) { + if (!this._requestTracingEnabled) { return undefined; } const headers = {}; - headers[CorrelationContextHeaderName] = createCorrelationContextHeader(this.options, requestType); + headers[CorrelationContextHeaderName] = createCorrelationContextHeader(this._options, requestType); return headers; } } From 6522085c2d2840c4651ca16c42814249527c553b Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 9 Nov 2023 14:57:56 +0800 Subject: [PATCH 06/43] Update mocked func for app-configuration v1.5.0 --- test/utils/testHelper.ts | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/test/utils/testHelper.ts b/test/utils/testHelper.ts index 80393032..db209383 100644 --- a/test/utils/testHelper.ts +++ b/test/utils/testHelper.ts @@ -2,7 +2,7 @@ // Licensed under the MIT license. import * as sinon from "sinon"; -import { AppConfigurationClient } from "@azure/app-configuration"; +import { AppConfigurationClient, ConfigurationSetting } from "@azure/app-configuration"; import { ClientSecretCredential } from "@azure/identity"; import { KeyVaultSecret, SecretClient } from "@azure/keyvault-secrets"; import * as uuid from "uuid"; @@ -11,7 +11,7 @@ const TEST_CLIENT_ID = "00000000-0000-0000-0000-000000000000"; const TEST_TENANT_ID = "00000000-0000-0000-0000-000000000000"; const TEST_CLIENT_SECRET = "0000000000000000000000000000000000000000"; -function mockAppConfigurationClientListConfigurationSettings(kvList: any[]) { +function mockAppConfigurationClientListConfigurationSettings(kvList: ConfigurationSetting[]) { function* testKvSetGnerator(kvs: any[]) { yield* kvs; } @@ -19,9 +19,18 @@ function mockAppConfigurationClientListConfigurationSettings(kvList: any[]) { const keyFilter = listOptions?.keyFilter ?? "*"; const labelFilter = listOptions?.labelFilter ?? "*"; const kvs = kvList.filter(kv => { - const keyMatched = keyFilter.endsWith("*") ? kv.key.startsWith(keyFilter.slice(0, keyFilter.length - 1)) : kv.key === keyFilter; - const labelMatched = labelFilter.endsWith("*") ? kv.label.startsWith(labelFilter.slice(0, labelFilter.length - 1)) - : (labelFilter === "\0" ? kv.label === null : kv.label === labelFilter); // '\0' in labelFilter, null in config setting. + const keyMatched = keyFilter.endsWith("*") ? kv.key.startsWith(keyFilter.slice(0, -1)) : kv.key === keyFilter; + + let labelMatched = false; + if (labelFilter === "*") { + labelMatched = true; + } else if (labelFilter === "\0") { + labelMatched = kv.label === undefined; + } else if (labelFilter.endsWith("*")) { + labelMatched = kv.label !== undefined && kv.label.startsWith(labelFilter.slice(0, -1)); + } else { + labelMatched = kv.label === labelFilter; + } return keyMatched && labelMatched; }) return testKvSetGnerator(kvs) as any; @@ -79,36 +88,33 @@ const createMockedTokenCredential = (tenantId = TEST_TENANT_ID, clientId = TEST_ return new ClientSecretCredential(tenantId, clientId, clientSecret); } -const createMockedKeyVaultReference = (key: string, vaultUri: string) => ({ +const createMockedKeyVaultReference = (key: string, vaultUri: string): ConfigurationSetting => ({ // https://${vaultName}.vault.azure.net/secrets/${secretName} value: `{"uri":"${vaultUri}"}`, key, - label: null, contentType: "application/vnd.microsoft.appconfig.keyvaultref+json;charset=utf-8", - lastModified: "2023-05-09T08:51:11.000Z", + lastModified: new Date(), tags: { }, etag: "SPJSMnJ2ph4BAjftWfdIctV2VIyQxtcIzRbh1oxTBkM", isReadOnly: false, }); -const createMockedJsonKeyValue = (key: string, value: any) => ({ +const createMockedJsonKeyValue = (key: string, value: any): ConfigurationSetting => ({ value: value, key: key, - label: null, contentType: "application/json", - lastModified: "2023-05-04T04:32:56.000Z", + lastModified: new Date(), tags: {}, etag: "GdmsLWq3mFjFodVEXUYRmvFr3l_qRiKAW_KdpFbxZKk", isReadOnly: false }); -const createMockedKeyValue = (props: {[key: string]: any}) => (Object.assign({ +const createMockedKeyValue = (props: {[key: string]: any}): ConfigurationSetting => (Object.assign({ value: "TestValue", key: "TestKey", - label: null, contentType: "", - lastModified: new Date().toISOString(), + lastModified: new Date(), tags: {}, etag: uuid.v4(), isReadOnly: false From 47e5c344acd0957a15f5d92efd8d04af0a0f7269 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 9 Nov 2023 15:12:37 +0800 Subject: [PATCH 07/43] remove workaround for null label --- src/AzureAppConfigurationImpl.ts | 10 ++++++---- test/utils/testHelper.ts | 7 ++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 7bc6d86d..5791695d 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -100,10 +100,12 @@ export class AzureAppConfigurationImpl extends Map implements Azure const keyValuePair = await this.processKeyValues(setting); keyValues.push(keyValuePair); } - // update etag of sentinels - const matchedSentinel = this._sentinels?.find(s => s.key === setting.key && (s.label ?? null) === setting.label); // Workaround: as undefined label represents the same with null. - if (matchedSentinel) { - matchedSentinel.etag = setting.etag; + // update etag of sentinels if refresh is enabled + if (this._refreshEnabled) { + const matchedSentinel = this._sentinels.find(s => s.key === setting.key && s.label === setting.label); + if (matchedSentinel) { + matchedSentinel.etag = setting.etag; + } } } } diff --git a/test/utils/testHelper.ts b/test/utils/testHelper.ts index db209383..69d09fb9 100644 --- a/test/utils/testHelper.ts +++ b/test/utils/testHelper.ts @@ -6,6 +6,7 @@ import { AppConfigurationClient, ConfigurationSetting } from "@azure/app-configu import { ClientSecretCredential } from "@azure/identity"; import { KeyVaultSecret, SecretClient } from "@azure/keyvault-secrets"; import * as uuid from "uuid"; +import { RestError } from "@azure/core-rest-pipeline"; const TEST_CLIENT_ID = "00000000-0000-0000-0000-000000000000"; const TEST_TENANT_ID = "00000000-0000-0000-0000-000000000000"; @@ -39,15 +40,15 @@ function mockAppConfigurationClientListConfigurationSettings(kvList: Configurati function mockAppConfigurationClientGetConfigurationSetting(kvList) { sinon.stub(AppConfigurationClient.prototype, "getConfigurationSetting").callsFake((settingId, options) => { - const key = settingId.key; - const label = settingId.label ?? null; - const found = kvList.find(elem => elem.key === key && elem.label === label); + const found = kvList.find(elem => elem.key === settingId.key && elem.label === settingId.label); if (found) { if (options?.onlyIfChanged && settingId.etag === found.etag) { return { statusCode: 304 }; } else { return { statusCode: 200, ...found }; } + } else { + throw new RestError("", { statusCode: 404 }); } }); } From 7b8c9b9cc436fa0aa377ebd992a20e9b0e64ba23 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 16 Nov 2023 14:06:49 +0800 Subject: [PATCH 08/43] also trace requests for refresh --- src/AzureAppConfigurationImpl.ts | 6 ++++-- test/refresh.test.ts | 4 +--- test/requestTracing.test.ts | 30 +++++++++++++++++++++++++++++- test/utils/testHelper.ts | 6 +++++- 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 5791695d..411b9fb6 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -129,8 +129,10 @@ export class AzureAppConfigurationImpl extends Map implements Azure let needRefresh = false; for (const sentinel of this._sentinels) { const response = await this._client.getConfigurationSetting(sentinel, { - onlyIfChanged: true - // TODO: do we trace this request by adding custom headers? + onlyIfChanged: true, + requestOptions: { + customHeaders: this.customHeaders(RequestType.Watch) + } }); if (response.statusCode === 200) { // sentinel changed. diff --git a/test/refresh.test.ts b/test/refresh.test.ts index adaf70c5..222cc82b 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -6,9 +6,7 @@ import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; import { load } from "./exportedApi"; -import { mockAppConfigurationClientListConfigurationSettings, mockAppConfigurationClientGetConfigurationSetting, restoreMocks, createMockedConnectionString, createMockedKeyValue } from "./utils/testHelper"; -import { promisify } from "util"; -const sleepInMs = promisify(setTimeout); +import { mockAppConfigurationClientListConfigurationSettings, mockAppConfigurationClientGetConfigurationSetting, restoreMocks, createMockedConnectionString, createMockedKeyValue, sleepInMs } from "./utils/testHelper"; import * as uuid from "uuid"; let mockedKVs: any[] = []; diff --git a/test/requestTracing.test.ts b/test/requestTracing.test.ts index aaedae20..8126d83f 100644 --- a/test/requestTracing.test.ts +++ b/test/requestTracing.test.ts @@ -5,7 +5,7 @@ import * as chai from "chai"; import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; -import { createMockedConnectionString, createMockedTokenCredential } from "./utils/testHelper"; +import { createMockedConnectionString, createMockedKeyValue, createMockedTokenCredential, mockAppConfigurationClientListConfigurationSettings, restoreMocks, sleepInMs } from "./utils/testHelper"; import { load } from "./exportedApi"; class HttpRequestHeadersPolicy { headers: any; @@ -118,4 +118,32 @@ describe("request tracing", function () { // clean up delete process.env.AZURE_APP_CONFIGURATION_TRACING_DISABLED; }); + + it("should have request type in correlation-context header when refresh is enabled", async () => { + mockAppConfigurationClientListConfigurationSettings([{ + key: "app.settings.fontColor", + value: "red" + }].map(createMockedKeyValue)); + + const settings = await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + refreshOptions: { + enabled: true, + refreshIntervalInMs: 1000, + watchedSettings: [{ + key: "app.settings.fontColor" + }] + } + }); + await sleepInMs(1000 + 1); + try { + await settings.refresh(); + } catch (e) { /* empty */ } + expect(headerPolicy.headers).not.undefined; + const correlationContext = headerPolicy.headers.get("Correlation-Context"); + expect(correlationContext).not.undefined; + expect(correlationContext.includes("RequestType=Watch")).eq(true); + + restoreMocks(); + }); }); \ No newline at end of file diff --git a/test/utils/testHelper.ts b/test/utils/testHelper.ts index 69d09fb9..82f9c888 100644 --- a/test/utils/testHelper.ts +++ b/test/utils/testHelper.ts @@ -7,6 +7,8 @@ import { ClientSecretCredential } from "@azure/identity"; import { KeyVaultSecret, SecretClient } from "@azure/keyvault-secrets"; import * as uuid from "uuid"; import { RestError } from "@azure/core-rest-pipeline"; +import { promisify } from "util"; +const sleepInMs = promisify(setTimeout); const TEST_CLIENT_ID = "00000000-0000-0000-0000-000000000000"; const TEST_TENANT_ID = "00000000-0000-0000-0000-000000000000"; @@ -133,5 +135,7 @@ export { createMockedTokenCredential, createMockedKeyVaultReference, createMockedJsonKeyValue, - createMockedKeyValue + createMockedKeyValue, + + sleepInMs } \ No newline at end of file From 1b64fb7b780dc6c848cfa8d1a22162c5d0dd53c2 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 16 Nov 2023 14:14:06 +0800 Subject: [PATCH 09/43] add more test cases for unexpected refreshOptions --- test/refresh.test.ts | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/test/refresh.test.ts b/test/refresh.test.ts index 222cc82b..34e436f0 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -55,6 +55,44 @@ describe("dynamic refresh", function () { ]); }); + it("should not allow refresh interval less than 1 second", async () => { + const connectionString = createMockedConnectionString(); + const loadWithInvalidRefreshInterval = load(connectionString, { + refreshOptions: { + enabled: true, + watchedSettings: [ + { key: "app.settings.fontColor" } + ], + refreshIntervalInMs: 999 + } + }); + return expect(loadWithInvalidRefreshInterval).eventually.rejectedWith("The refresh interval time cannot be less than 1000 milliseconds."); + }); + + it("should not allow '*' in key or label", async () => { + const connectionString = createMockedConnectionString(); + const loadWithInvalidKey = load(connectionString, { + refreshOptions: { + enabled: true, + watchedSettings: [ + { key: "app.settings.*" } + ] + } + }); + const loadWithInvalidLabel = load(connectionString, { + refreshOptions: { + enabled: true, + watchedSettings: [ + { key: "app.settings.fontColor", label: "*" } + ] + } + }); + return Promise.all([ + expect(loadWithInvalidKey).eventually.rejectedWith("The character '*' is not supported in key or label."), + expect(loadWithInvalidLabel).eventually.rejectedWith("The character '*' is not supported in key or label.") + ]); + }); + it("should throw error when calling onRefresh when refresh is not enabled", async () => { const connectionString = createMockedConnectionString(); const settings = await load(connectionString); From 179ce3e721d2c9660ff3f92d26734f6f4f0f14aa Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Tue, 21 Nov 2023 16:30:46 +0800 Subject: [PATCH 10/43] revert renaming private fields with _ prefix --- src/AzureAppConfigurationImpl.ts | 76 ++++++++++++++++---------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 411b9fb6..f776c203 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -16,33 +16,33 @@ import { LinkedList } from "./common/linkedList"; import { Disposable } from "./common/disposable"; export class AzureAppConfigurationImpl extends Map implements AzureAppConfiguration { - private _adapters: IKeyValueAdapter[] = []; + private adapters: IKeyValueAdapter[] = []; /** * Trim key prefixes sorted in descending order. * Since multiple prefixes could start with the same characters, we need to trim the longest prefix first. */ - private _sortedTrimKeyPrefixes: string[] | undefined; - private readonly _requestTracingEnabled: boolean; + private sortedTrimKeyPrefixes: string[] | undefined; + private readonly requestTracingEnabled: boolean; // Refresh - private _refreshInterval: number = DefaultRefreshIntervalInMs; - private _onRefreshListeners: LinkedList<() => any> = new LinkedList(); - private _lastUpdateTimestamp: number; - private _sentinels: ConfigurationSettingId[]; + private refreshInterval: number = DefaultRefreshIntervalInMs; + private onRefreshListeners: LinkedList<() => any> = new LinkedList(); + private lastUpdateTimestamp: number; + private sentinels: ConfigurationSettingId[]; constructor( - private _client: AppConfigurationClient, - private _options: AzureAppConfigurationOptions | undefined + private client: AppConfigurationClient, + private options: AzureAppConfigurationOptions | undefined ) { super(); // Enable request tracing if not opt-out - this._requestTracingEnabled = requestTracingEnabled(); + this.requestTracingEnabled = requestTracingEnabled(); - if (_options?.trimKeyPrefixes) { - this._sortedTrimKeyPrefixes = [..._options.trimKeyPrefixes].sort((a, b) => b.localeCompare(a)); + if (options?.trimKeyPrefixes) { + this.sortedTrimKeyPrefixes = [...options.trimKeyPrefixes].sort((a, b) => b.localeCompare(a)); } - if (_options?.refreshOptions?.enabled) { - const { watchedSettings, refreshIntervalInMs } = _options.refreshOptions; + if (options?.refreshOptions?.enabled) { + const { watchedSettings, refreshIntervalInMs } = options.refreshOptions; // validate watched settings if (watchedSettings === undefined || watchedSettings.length === 0) { throw new Error("Refresh is enabled but no watched settings are specified."); @@ -53,11 +53,11 @@ export class AzureAppConfigurationImpl extends Map implements Azure if (refreshIntervalInMs < MinimumRefreshIntervalInMs) { throw new Error(`The refresh interval time cannot be less than ${MinimumRefreshIntervalInMs} milliseconds.`); } else { - this._refreshInterval = refreshIntervalInMs; + this.refreshInterval = refreshIntervalInMs; } } - this._sentinels = watchedSettings.map(setting => { + this.sentinels = watchedSettings.map(setting => { if (setting.key.includes("*") || setting.label?.includes("*")) { throw new Error("The character '*' is not supported in key or label."); } @@ -67,33 +67,33 @@ export class AzureAppConfigurationImpl extends Map implements Azure // TODO: should add more adapters to process different type of values // feature flag, others - this._adapters.push(new AzureKeyVaultKeyValueAdapter(_options?.keyVaultOptions)); - this._adapters.push(new JsonKeyValueAdapter()); + this.adapters.push(new AzureKeyVaultKeyValueAdapter(options?.keyVaultOptions)); + this.adapters.push(new JsonKeyValueAdapter()); } - private get _refreshEnabled(): boolean { - return !!this._options?.refreshOptions?.enabled; + private get refreshEnabled(): boolean { + return !!this.options?.refreshOptions?.enabled; } public async load(requestType: RequestType = RequestType.Startup) { const keyValues: [key: string, value: unknown][] = []; // validate selectors - const selectors = getValidSelectors(this._options?.selectors); + const selectors = getValidSelectors(this.options?.selectors); for (const selector of selectors) { const listOptions: ListConfigurationSettingsOptions = { keyFilter: selector.keyFilter, labelFilter: selector.labelFilter }; - if (this._requestTracingEnabled) { + if (this.requestTracingEnabled) { listOptions.requestOptions = { customHeaders: this.customHeaders(requestType) } } - const settings = this._client.listConfigurationSettings(listOptions); + const settings = this.client.listConfigurationSettings(listOptions); for await (const setting of settings) { if (setting.key) { @@ -101,8 +101,8 @@ export class AzureAppConfigurationImpl extends Map implements Azure keyValues.push(keyValuePair); } // update etag of sentinels if refresh is enabled - if (this._refreshEnabled) { - const matchedSentinel = this._sentinels.find(s => s.key === setting.key && s.label === setting.label); + if (this.refreshEnabled) { + const matchedSentinel = this.sentinels.find(s => s.key === setting.key && s.label === setting.label); if (matchedSentinel) { matchedSentinel.etag = setting.etag; } @@ -112,23 +112,23 @@ export class AzureAppConfigurationImpl extends Map implements Azure for (const [k, v] of keyValues) { this.set(k, v); } - this._lastUpdateTimestamp = Date.now(); + this.lastUpdateTimestamp = Date.now(); } public async refresh(): Promise { - if (!this._refreshEnabled) { + if (!this.refreshEnabled) { return Promise.resolve(); } // if still within refresh interval, return const now = Date.now(); - if (now < this._lastUpdateTimestamp + this._refreshInterval) { + if (now < this.lastUpdateTimestamp + this.refreshInterval) { return Promise.resolve(); } // try refresh if any of watched settings is changed. let needRefresh = false; - for (const sentinel of this._sentinels) { - const response = await this._client.getConfigurationSetting(sentinel, { + for (const sentinel of this.sentinels) { + const response = await this.client.getConfigurationSetting(sentinel, { onlyIfChanged: true, requestOptions: { customHeaders: this.customHeaders(RequestType.Watch) @@ -144,19 +144,19 @@ export class AzureAppConfigurationImpl extends Map implements Azure if (needRefresh) { await this.load(RequestType.Watch); // run callbacks in async - for (const listener of this._onRefreshListeners) { + for (const listener of this.onRefreshListeners) { listener(); } } } public onRefresh(listener: () => any, thisArg?: any): Disposable { - if (!this._refreshEnabled) { + if (!this.refreshEnabled) { throw new Error("Refresh is not enabled."); } const boundedListener = listener.bind(thisArg); - const remove = this._onRefreshListeners.push(boundedListener); + const remove = this.onRefreshListeners.push(boundedListener); return new Disposable(remove); } @@ -167,7 +167,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure } private async processAdapters(setting: ConfigurationSetting): Promise<[string, unknown]> { - for (const adapter of this._adapters) { + for (const adapter of this.adapters) { if (adapter.canProcess(setting)) { return adapter.processKeyValue(setting); } @@ -176,8 +176,8 @@ export class AzureAppConfigurationImpl extends Map implements Azure } private keyWithPrefixesTrimmed(key: string): string { - if (this._sortedTrimKeyPrefixes) { - for (const prefix of this._sortedTrimKeyPrefixes) { + if (this.sortedTrimKeyPrefixes) { + for (const prefix of this.sortedTrimKeyPrefixes) { if (key.startsWith(prefix)) { return key.slice(prefix.length); } @@ -187,12 +187,12 @@ export class AzureAppConfigurationImpl extends Map implements Azure } private customHeaders(requestType: RequestType) { - if (!this._requestTracingEnabled) { + if (!this.requestTracingEnabled) { return undefined; } const headers = {}; - headers[CorrelationContextHeaderName] = createCorrelationContextHeader(this._options, requestType); + headers[CorrelationContextHeaderName] = createCorrelationContextHeader(this.options, requestType); return headers; } } From 03d53b94f2e61352228d2df449ef53bd520895dd Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Tue, 26 Dec 2023 17:08:16 +0800 Subject: [PATCH 11/43] add backoff timer --- examples/package.json | 2 +- src/AzureAppConfigurationImpl.ts | 12 ++++--- src/refresh/RefreshTimer.ts | 61 ++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 6 deletions(-) create mode 100644 src/refresh/RefreshTimer.ts diff --git a/examples/package.json b/examples/package.json index cb0fdf19..9c1e5678 100644 --- a/examples/package.json +++ b/examples/package.json @@ -1,6 +1,6 @@ { "dependencies": { - "@azure/app-configuration-provider": "latest", + "@azure/app-configuration-provider": "file:..", "@azure/identity": "^3.3.0", "dotenv": "^16.3.1" } diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index ba01ef26..6efc8e89 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -14,6 +14,7 @@ import { DefaultRefreshIntervalInMs, MinimumRefreshIntervalInMs } from "./Refres import { LinkedList } from "./common/linkedList"; import { Disposable } from "./common/disposable"; import { SettingSelector } from "./types"; +import { RefreshTimer } from "./refresh/RefreshTimer"; export class AzureAppConfigurationImpl extends Map implements AzureAppConfiguration { private adapters: IKeyValueAdapter[] = []; @@ -26,8 +27,8 @@ export class AzureAppConfigurationImpl extends Map implements Azure // Refresh private refreshInterval: number = DefaultRefreshIntervalInMs; private onRefreshListeners: LinkedList<() => any> = new LinkedList(); - private lastUpdateTimestamp: number; private sentinels: ConfigurationSettingId[]; + private refreshTimer: RefreshTimer; constructor( private client: AppConfigurationClient, @@ -63,6 +64,8 @@ export class AzureAppConfigurationImpl extends Map implements Azure } return { ...setting }; }); + + this.refreshTimer = new RefreshTimer(this.refreshInterval); } // TODO: should add more adapters to process different type of values @@ -112,16 +115,15 @@ export class AzureAppConfigurationImpl extends Map implements Azure for (const [k, v] of keyValues) { this.set(k, v); } - this.lastUpdateTimestamp = Date.now(); } public async refresh(): Promise { if (!this.refreshEnabled) { return Promise.resolve(); } - // if still within refresh interval, return - const now = Date.now(); - if (now < this.lastUpdateTimestamp + this.refreshInterval) { + + // if still within refresh interval/backoff, return + if (this.refreshTimer.isDuringBackoff()) { return Promise.resolve(); } diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts new file mode 100644 index 00000000..58c006b2 --- /dev/null +++ b/src/refresh/RefreshTimer.ts @@ -0,0 +1,61 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +const MinimumBackoffInMs = 30 * 1000; // 30s +const MaximumBackoffInMs = 10 * 60 * 1000; // 10min +const MaxAttempts = 63; +const JitterRatio = 0.25; + +export class RefreshTimer { + private _minBackoff: number; + private _maxBackoff: number; + private _attempts: number; + private _backoffEnd: number; // Timestamp + constructor( + private _interval: number + ) { + this._minBackoff = Math.min(this._interval, MinimumBackoffInMs); + this._maxBackoff = Math.min(this._interval, MaximumBackoffInMs); + this._attempts = 0; + this._backoffEnd = Date.now() + this._interval; + } + + public isDuringBackoff(): boolean { + return Date.now() < this._backoffEnd; + } + + public backoff(): void { + this._backoffEnd = Date.now() + this._calculateBackoffTime(); + this._attempts += 1; + } + + public reset(): void { + this._attempts = 0; + } + + private _calculateBackoffTime(): number { + let minBackoffMs: number; + let maxBackoffMs: number; + if (this._interval <= this._minBackoff) { + return this._interval; + } else if (this._interval <= this._maxBackoff) { + minBackoffMs = MinimumBackoffInMs + maxBackoffMs = this._interval; + } else { + // _interval > _maxBackoff + minBackoffMs = MinimumBackoffInMs; + maxBackoffMs = MaximumBackoffInMs; + } + + // exponential + let calculatedBackoffMs = Math.max(1, minBackoffMs) * (1 << Math.min(this._attempts, MaxAttempts)); + if (calculatedBackoffMs > maxBackoffMs) { + calculatedBackoffMs = maxBackoffMs; + } + + // jitter + const jitter = JitterRatio * (Math.random() * 2 - 1); + return calculatedBackoffMs * (1 + jitter); + } + +} \ No newline at end of file From 431e91eca954aa57ad89e2432793bf5ef8631e0e Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 27 Dec 2023 15:31:33 +0800 Subject: [PATCH 12/43] backoff when error occurs during refresh --- examples/package.json | 2 +- src/AzureAppConfigurationImpl.ts | 15 +++++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/examples/package.json b/examples/package.json index 9c1e5678..cb0fdf19 100644 --- a/examples/package.json +++ b/examples/package.json @@ -1,6 +1,6 @@ { "dependencies": { - "@azure/app-configuration-provider": "file:..", + "@azure/app-configuration-provider": "latest", "@azure/identity": "^3.3.0", "dotenv": "^16.3.1" } diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 6efc8e89..8c24cdd0 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -117,6 +117,9 @@ export class AzureAppConfigurationImpl extends Map implements Azure } } + /** + * Refresh the configuration store. + */ public async refresh(): Promise { if (!this.refreshEnabled) { return Promise.resolve(); @@ -144,8 +147,16 @@ export class AzureAppConfigurationImpl extends Map implements Azure } } if (needRefresh) { - await this.load(RequestType.Watch); - // run callbacks in async + try { + await this.load(RequestType.Watch); + this.refreshTimer.reset(); + } catch (error) { + // if refresh failed, backoff + this.refreshTimer.backoff(); + throw error; + } + + // successfully refreshed, run callbacks in async for (const listener of this.onRefreshListeners) { listener(); } From af9ea33ef7b4dce8dff3a17197568af2d1c891de Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Tue, 2 Jan 2024 10:44:49 +0800 Subject: [PATCH 13/43] update comment docs --- src/AzureAppConfiguration.ts | 4 ++-- src/WatchedSetting.ts | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/AzureAppConfiguration.ts b/src/AzureAppConfiguration.ts index c7f9efec..4a90aaf8 100644 --- a/src/AzureAppConfiguration.ts +++ b/src/AzureAppConfiguration.ts @@ -12,8 +12,8 @@ export type AzureAppConfiguration = { /** * API to register callback listeners, which will be called only when a refresh operation successfully updates key-values. * - * @param listener Callback funtion to be registered. - * @param thisArg Optional. Value to use as this when executing callback. + * @param listener - Callback funtion to be registered. + * @param thisArg - Optional. Value to use as `this` when executing callback. */ onRefresh(listener: () => any, thisArg?: any): Disposable; } & ReadonlyMap; diff --git a/src/WatchedSetting.ts b/src/WatchedSetting.ts index 815a26b9..b714f2ef 100644 --- a/src/WatchedSetting.ts +++ b/src/WatchedSetting.ts @@ -1,7 +1,18 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. +/** + * Fields that uniquely identify a watched configuration setting. + */ export interface WatchedSetting { + /** + * The key for this setting. + */ key: string; + + /** + * The label for this setting. + * Leaving this undefined means this setting does not have a label. + */ label?: string; } \ No newline at end of file From 677eaa009aee246b5743a44d0bb946862916a9a6 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Tue, 2 Jan 2024 15:19:17 +0800 Subject: [PATCH 14/43] fix backoff end time on reset --- src/refresh/RefreshTimer.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index 58c006b2..341e2537 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -30,6 +30,7 @@ export class RefreshTimer { } public reset(): void { + this._backoffEnd = Date.now() + this._interval; this._attempts = 0; } From 8e1c1a4f5b33949f396c1a9b47975ec829aa70b5 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Tue, 2 Jan 2024 15:24:43 +0800 Subject: [PATCH 15/43] make backoff time calc clearer --- src/refresh/RefreshTimer.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index 341e2537..12f7a833 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -39,11 +39,13 @@ export class RefreshTimer { let maxBackoffMs: number; if (this._interval <= this._minBackoff) { return this._interval; - } else if (this._interval <= this._maxBackoff) { + } + + // _minBackoff <= _interval + if (this._interval <= this._maxBackoff) { minBackoffMs = MinimumBackoffInMs maxBackoffMs = this._interval; } else { - // _interval > _maxBackoff minBackoffMs = MinimumBackoffInMs; maxBackoffMs = MaximumBackoffInMs; } From 1da9647555fbbf19d049d10a02ed5f18a7ea513e Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 3 Jan 2024 16:29:37 +0800 Subject: [PATCH 16/43] Block wildcard chars in watched settings --- src/AzureAppConfigurationImpl.ts | 7 +++++-- test/refresh.test.ts | 22 ++++++++++++++++++++-- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 8c24cdd0..fe477903 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -59,8 +59,11 @@ export class AzureAppConfigurationImpl extends Map implements Azure } this.sentinels = watchedSettings.map(setting => { - if (setting.key.includes("*") || setting.label?.includes("*")) { - throw new Error("The character '*' is not supported in key or label."); + if (setting.key.includes("*") || setting.key.includes(",") ) { + throw new Error("The characters '*' and ',' are not supported in key of watched settings."); + } + if (setting.label?.includes("*") || setting.label?.includes(",")) { + throw new Error("The characters '*' and ',' are not supported in label of watched settings."); } return { ...setting }; }); diff --git a/test/refresh.test.ts b/test/refresh.test.ts index 34e436f0..fd903c7e 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -79,6 +79,14 @@ describe("dynamic refresh", function () { ] } }); + const loadWithInvalidKey2 = load(connectionString, { + refreshOptions: { + enabled: true, + watchedSettings: [ + { key: "keyA,KeyB" } + ] + } + }); const loadWithInvalidLabel = load(connectionString, { refreshOptions: { enabled: true, @@ -87,9 +95,19 @@ describe("dynamic refresh", function () { ] } }); + const loadWithInvalidLabel2 = load(connectionString, { + refreshOptions: { + enabled: true, + watchedSettings: [ + { key: "app.settings.fontColor", label: "labelA,labelB" } + ] + } + }); return Promise.all([ - expect(loadWithInvalidKey).eventually.rejectedWith("The character '*' is not supported in key or label."), - expect(loadWithInvalidLabel).eventually.rejectedWith("The character '*' is not supported in key or label.") + expect(loadWithInvalidKey).eventually.rejectedWith("The characters '*' and ',' are not supported in key of watched settings."), + expect(loadWithInvalidKey2).eventually.rejectedWith("The characters '*' and ',' are not supported in key of watched settings."), + expect(loadWithInvalidLabel).eventually.rejectedWith("The characters '*' and ',' are not supported in label of watched settings."), + expect(loadWithInvalidLabel2).eventually.rejectedWith("The characters '*' and ',' are not supported in label of watched settings.") ]); }); From 13e06a5b4175cf1cf9ffe5e1a48afc3b6415b9b3 Mon Sep 17 00:00:00 2001 From: Yan Zhang <2351748+Eskibear@users.noreply.github.com> Date: Wed, 3 Jan 2024 16:52:57 +0800 Subject: [PATCH 17/43] Apply wording suggestions Co-authored-by: Avani Gupta --- examples/refresh.mjs | 4 ++-- src/AzureAppConfigurationImpl.ts | 3 ++- src/RefreshOptions.ts | 6 +++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/examples/refresh.mjs b/examples/refresh.mjs index 9d7cb77f..546385ba 100644 --- a/examples/refresh.mjs +++ b/examples/refresh.mjs @@ -28,8 +28,8 @@ const settings = await load(connectionString, { } }); -console.log("Update the `message` in your App Configuration store using Azure portal or CLI.") -console.log("First, update the `message` value, and then update the `sentinel` key value.") +console.log("Using Azure portal or CLI, update the `app.settings.message` value, and then update the `app.settings.sentinel` value in your App Configuration store.") + // eslint-disable-next-line no-constant-condition while (true) { diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index fe477903..5463930d 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -52,7 +52,8 @@ export class AzureAppConfigurationImpl extends Map implements Azure // custom refresh interval if (refreshIntervalInMs !== undefined) { if (refreshIntervalInMs < MinimumRefreshIntervalInMs) { - throw new Error(`The refresh interval time cannot be less than ${MinimumRefreshIntervalInMs} milliseconds.`); + throw new Error(`The refresh interval cannot be less than ${MinimumRefreshIntervalInMs} milliseconds.`); + } else { this.refreshInterval = refreshIntervalInMs; } diff --git a/src/RefreshOptions.ts b/src/RefreshOptions.ts index 16217483..28a0d03f 100644 --- a/src/RefreshOptions.ts +++ b/src/RefreshOptions.ts @@ -13,15 +13,15 @@ export interface RefreshOptions { enabled: boolean; /** - * Specifies the interval for refresh to really update the values. + * Specifies the minimum time that must elapse before checking the server for any new changes. * Default value is 30 seconds. Must be greater than 1 second. * Any refresh operation triggered will not update the value for a key until after the interval. */ refreshIntervalInMs?: number; /** - * Specifies settings to be watched. - * Only if any of the settings is updated, the refresh will acutally update loaded configuration settings. + * One or more configuration settings to be watched for changes on the server. + * Any modifications to watched settings will refresh all settings loaded by the configuration provider. */ watchedSettings?: WatchedSetting[]; } \ No newline at end of file From e72887a13942e64eba70b0b036668000206b1b20 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 3 Jan 2024 18:57:15 +0800 Subject: [PATCH 18/43] Remove LinkedList and update onRefreshListeners to use an array --- src/AzureAppConfigurationImpl.ts | 12 ++- src/common/linkedList.ts | 140 ------------------------------- test/refresh.test.ts | 3 + 3 files changed, 12 insertions(+), 143 deletions(-) delete mode 100644 src/common/linkedList.ts diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 5463930d..9c81dae6 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -11,7 +11,6 @@ import { AzureKeyVaultKeyValueAdapter } from "./keyvault/AzureKeyVaultKeyValueAd import { CorrelationContextHeaderName, RequestType } from "./requestTracing/constants"; import { createCorrelationContextHeader, requestTracingEnabled } from "./requestTracing/utils"; import { DefaultRefreshIntervalInMs, MinimumRefreshIntervalInMs } from "./RefreshOptions"; -import { LinkedList } from "./common/linkedList"; import { Disposable } from "./common/disposable"; import { SettingSelector } from "./types"; import { RefreshTimer } from "./refresh/RefreshTimer"; @@ -26,7 +25,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure private readonly requestTracingEnabled: boolean; // Refresh private refreshInterval: number = DefaultRefreshIntervalInMs; - private onRefreshListeners: LinkedList<() => any> = new LinkedList(); + private onRefreshListeners: Array<() => any> = []; private sentinels: ConfigurationSettingId[]; private refreshTimer: RefreshTimer; @@ -173,7 +172,14 @@ export class AzureAppConfigurationImpl extends Map implements Azure } const boundedListener = listener.bind(thisArg); - const remove = this.onRefreshListeners.push(boundedListener); + this.onRefreshListeners.push(boundedListener); + + const remove = () => { + const index = this.onRefreshListeners.indexOf(boundedListener); + if (index >= 0) { + this.onRefreshListeners.splice(index, 1); + } + } return new Disposable(remove); } diff --git a/src/common/linkedList.ts b/src/common/linkedList.ts deleted file mode 100644 index 5d932c7d..00000000 --- a/src/common/linkedList.ts +++ /dev/null @@ -1,140 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -class Node { - - static readonly Undefined = new Node(undefined); - - element: E; - next: Node; - prev: Node; - - constructor(element: E) { - this.element = element; - this.next = Node.Undefined; - this.prev = Node.Undefined; - } -} - -export class LinkedList { - - private _first: Node = Node.Undefined; - private _last: Node = Node.Undefined; - private _size: number = 0; - - get size(): number { - return this._size; - } - - isEmpty(): boolean { - return this._first === Node.Undefined; - } - - clear(): void { - let node = this._first; - while (node !== Node.Undefined) { - const next = node.next; - node.prev = Node.Undefined; - node.next = Node.Undefined; - node = next; - } - - this._first = Node.Undefined; - this._last = Node.Undefined; - this._size = 0; - } - - unshift(element: E): () => void { - return this._insert(element, false); - } - - push(element: E): () => void { - return this._insert(element, true); - } - - private _insert(element: E, atTheEnd: boolean): () => void { - const newNode = new Node(element); - if (this._first === Node.Undefined) { - this._first = newNode; - this._last = newNode; - - } else if (atTheEnd) { - // push - const oldLast = this._last!; - this._last = newNode; - newNode.prev = oldLast; - oldLast.next = newNode; - - } else { - // unshift - const oldFirst = this._first; - this._first = newNode; - newNode.next = oldFirst; - oldFirst.prev = newNode; - } - this._size += 1; - - let didRemove = false; - return () => { - if (!didRemove) { - didRemove = true; - this._remove(newNode); - } - }; - } - - shift(): E | undefined { - if (this._first === Node.Undefined) { - return undefined; - } else { - const res = this._first.element; - this._remove(this._first); - return res; - } - } - - pop(): E | undefined { - if (this._last === Node.Undefined) { - return undefined; - } else { - const res = this._last.element; - this._remove(this._last); - return res; - } - } - - private _remove(node: Node): void { - if (node.prev !== Node.Undefined && node.next !== Node.Undefined) { - // middle - const anchor = node.prev; - anchor.next = node.next; - node.next.prev = anchor; - - } else if (node.prev === Node.Undefined && node.next === Node.Undefined) { - // only node - this._first = Node.Undefined; - this._last = Node.Undefined; - - } else if (node.next === Node.Undefined) { - // last - this._last = this._last!.prev!; - this._last.next = Node.Undefined; - - } else if (node.prev === Node.Undefined) { - // first - this._first = this._first!.next!; - this._first.prev = Node.Undefined; - } - - // done - this._size -= 1; - } - - *[Symbol.iterator](): Iterator { - let node = this._first; - while (node !== Node.Undefined) { - yield node.element; - node = node.next; - } - } -} diff --git a/test/refresh.test.ts b/test/refresh.test.ts index fd903c7e..4d9da1e4 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -215,5 +215,8 @@ describe("dynamic refresh", function () { // can dispose callbacks callback.dispose(); + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + expect(count).eq(1); }); }); \ No newline at end of file From 2b867e90ef01f3ea578ab38e110953f9845130e3 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 3 Jan 2024 19:14:47 +0800 Subject: [PATCH 19/43] fix error message in test case --- test/refresh.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/refresh.test.ts b/test/refresh.test.ts index 4d9da1e4..335b0f50 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -66,7 +66,7 @@ describe("dynamic refresh", function () { refreshIntervalInMs: 999 } }); - return expect(loadWithInvalidRefreshInterval).eventually.rejectedWith("The refresh interval time cannot be less than 1000 milliseconds."); + return expect(loadWithInvalidRefreshInterval).eventually.rejectedWith("The refresh interval cannot be less than 1000 milliseconds."); }); it("should not allow '*' in key or label", async () => { From 46cf7b99204f23b026570ec862217bdf5e914929 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 3 Jan 2024 19:20:10 +0800 Subject: [PATCH 20/43] adopt private properties --- src/AzureAppConfigurationImpl.ts | 49 ++++++++++++++++---------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 5e79f671..2b8eb469 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -28,10 +28,10 @@ export class AzureAppConfigurationImpl extends Map implements Azure #options: AzureAppConfigurationOptions | undefined; // Refresh - private refreshInterval: number = DefaultRefreshIntervalInMs; - private onRefreshListeners: Array<() => any> = []; - private sentinels: ConfigurationSettingId[]; - private refreshTimer: RefreshTimer; + #refreshInterval: number = DefaultRefreshIntervalInMs; + #onRefreshListeners: Array<() => any> = []; + #sentinels: ConfigurationSettingId[]; + #refreshTimer: RefreshTimer; constructor( client: AppConfigurationClient, @@ -61,11 +61,11 @@ export class AzureAppConfigurationImpl extends Map implements Azure throw new Error(`The refresh interval cannot be less than ${MinimumRefreshIntervalInMs} milliseconds.`); } else { - this.refreshInterval = refreshIntervalInMs; + this.#refreshInterval = refreshIntervalInMs; } } - this.sentinels = watchedSettings.map(setting => { + this.#sentinels = watchedSettings.map(setting => { if (setting.key.includes("*") || setting.key.includes(",")) { throw new Error("The characters '*' and ',' are not supported in key of watched settings."); } @@ -75,7 +75,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure return { ...setting }; }); - this.refreshTimer = new RefreshTimer(this.refreshInterval); + this.#refreshTimer = new RefreshTimer(this.#refreshInterval); } // TODO: should add more adapters to process different type of values @@ -85,7 +85,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure } - private get refreshEnabled(): boolean { + get #refreshEnabled(): boolean { return !!this.#options?.refreshOptions?.enabled; } @@ -110,13 +110,12 @@ export class AzureAppConfigurationImpl extends Map implements Azure for await (const setting of settings) { if (setting.key) { - const [key, value] = await this.#processAdapters(setting); - const trimmedKey = this.#keyWithPrefixesTrimmed(key); - keyValues.push([trimmedKey, value]); + const keyValuePair = await this.#processKeyValues(setting); + keyValues.push(keyValuePair); } // update etag of sentinels if refresh is enabled - if (this.refreshEnabled) { - const matchedSentinel = this.sentinels.find(s => s.key === setting.key && s.label === setting.label); + if (this.#refreshEnabled) { + const matchedSentinel = this.#sentinels.find(s => s.key === setting.key && s.label === setting.label); if (matchedSentinel) { matchedSentinel.etag = setting.etag; } @@ -132,18 +131,18 @@ export class AzureAppConfigurationImpl extends Map implements Azure * Refresh the configuration store. */ public async refresh(): Promise { - if (!this.refreshEnabled) { + if (!this.#refreshEnabled) { return Promise.resolve(); } // if still within refresh interval/backoff, return - if (this.refreshTimer.isDuringBackoff()) { + if (this.#refreshTimer.isDuringBackoff()) { return Promise.resolve(); } // try refresh if any of watched settings is changed. let needRefresh = false; - for (const sentinel of this.sentinels) { + for (const sentinel of this.#sentinels) { const response = await this.#client.getConfigurationSetting(sentinel, { onlyIfChanged: true, requestOptions: { @@ -160,38 +159,38 @@ export class AzureAppConfigurationImpl extends Map implements Azure if (needRefresh) { try { await this.load(RequestType.Watch); - this.refreshTimer.reset(); + this.#refreshTimer.reset(); } catch (error) { // if refresh failed, backoff - this.refreshTimer.backoff(); + this.#refreshTimer.backoff(); throw error; } // successfully refreshed, run callbacks in async - for (const listener of this.onRefreshListeners) { + for (const listener of this.#onRefreshListeners) { listener(); } } } - public onRefresh(listener: () => any, thisArg?: any): Disposable { - if (!this.refreshEnabled) { + onRefresh(listener: () => any, thisArg?: any): Disposable { + if (!this.#refreshEnabled) { throw new Error("Refresh is not enabled."); } const boundedListener = listener.bind(thisArg); - this.onRefreshListeners.push(boundedListener); + this.#onRefreshListeners.push(boundedListener); const remove = () => { - const index = this.onRefreshListeners.indexOf(boundedListener); + const index = this.#onRefreshListeners.indexOf(boundedListener); if (index >= 0) { - this.onRefreshListeners.splice(index, 1); + this.#onRefreshListeners.splice(index, 1); } } return new Disposable(remove); } - private async processKeyValues(setting: ConfigurationSetting): Promise<[string, unknown]> { + async #processKeyValues(setting: ConfigurationSetting): Promise<[string, unknown]> { const [key, value] = await this.#processAdapters(setting); const trimmedKey = this.#keyWithPrefixesTrimmed(key); return [trimmedKey, value]; From 5b3ab06cf73cd73d15753126e3f45f0cafe64507 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 3 Jan 2024 19:21:29 +0800 Subject: [PATCH 21/43] Refactor refresh timer method name --- src/AzureAppConfigurationImpl.ts | 2 +- src/refresh/RefreshTimer.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 2b8eb469..57129cc6 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -136,7 +136,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure } // if still within refresh interval/backoff, return - if (this.#refreshTimer.isDuringBackoff()) { + if (this.#refreshTimer.canRefresh()) { return Promise.resolve(); } diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index 12f7a833..3a61e5ad 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -20,7 +20,7 @@ export class RefreshTimer { this._backoffEnd = Date.now() + this._interval; } - public isDuringBackoff(): boolean { + public canRefresh(): boolean { return Date.now() < this._backoffEnd; } From 18bbd4e9c6e43a04423a4e318b85b272817e6bd4 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 3 Jan 2024 19:25:15 +0800 Subject: [PATCH 22/43] explain refresh scenario in example comments --- examples/refresh.mjs | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/refresh.mjs b/examples/refresh.mjs index 546385ba..b3945f3d 100644 --- a/examples/refresh.mjs +++ b/examples/refresh.mjs @@ -10,6 +10,7 @@ const sleepInMs = promisify(setTimeout); * This example retrives all settings with key following pattern "app.settings.*", i.e. starting with "app.settings.". * With the option `trimKeyPrefixes`, it trims the prefix "app.settings." from keys for simplicity. * Value of config "app.settings.message" will be printed. + * It also watches for changes to the key "app.settings.sentinel" and refreshes the configuration when it changes. * * Below environment variables are required for this example: * - APPCONFIG_CONNECTION_STRING From 8e1a2ab78e77e01e86db96c40554cdae208b75e5 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 4 Jan 2024 09:56:15 +0800 Subject: [PATCH 23/43] Add timeout to dynamic refresh test --- test/refresh.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/refresh.test.ts b/test/refresh.test.ts index 335b0f50..e60c2bff 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -23,6 +23,8 @@ function addSetting(key: string, value: any) { } describe("dynamic refresh", function () { + this.timeout(10000); + beforeEach(() => { mockedKVs = [ { value: "red", key: "app.settings.fontColor" }, From 0e347bec6a1c917d4872fb61bebf74d6a7314d70 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 4 Jan 2024 09:56:29 +0800 Subject: [PATCH 24/43] Fix refresh timer logic in AzureAppConfigurationImpl.ts --- src/AzureAppConfigurationImpl.ts | 2 +- src/refresh/RefreshTimer.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 57129cc6..1b0e699c 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -136,7 +136,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure } // if still within refresh interval/backoff, return - if (this.#refreshTimer.canRefresh()) { + if (!this.#refreshTimer.canRefresh()) { return Promise.resolve(); } diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index 3a61e5ad..cfd22ca1 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -21,7 +21,7 @@ export class RefreshTimer { } public canRefresh(): boolean { - return Date.now() < this._backoffEnd; + return Date.now() >= this._backoffEnd; } public backoff(): void { From 46be338299477061c6d4ae3e16ce21e91d225579 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 4 Jan 2024 11:17:12 +0800 Subject: [PATCH 25/43] support refresh on watched setting deletion --- src/AzureAppConfigurationImpl.ts | 39 ++++++++++++++++++++------------ test/refresh.test.ts | 27 ++++++++++++++++++++++ 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 1b0e699c..a51fd8b0 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -1,19 +1,19 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { AppConfigurationClient, ConfigurationSetting, ConfigurationSettingId, ListConfigurationSettingsOptions } from "@azure/app-configuration"; +import { AppConfigurationClient, ConfigurationSetting, ConfigurationSettingId, GetConfigurationSettingResponse, ListConfigurationSettingsOptions } from "@azure/app-configuration"; +import { RestError } from "@azure/core-rest-pipeline"; import { AzureAppConfiguration } from "./AzureAppConfiguration"; import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions"; import { IKeyValueAdapter } from "./IKeyValueAdapter"; import { JsonKeyValueAdapter } from "./JsonKeyValueAdapter"; -import { KeyFilter, LabelFilter } from "./types"; -import { AzureKeyVaultKeyValueAdapter } from "./keyvault/AzureKeyVaultKeyValueAdapter"; -import { CorrelationContextHeaderName, RequestType } from "./requestTracing/constants"; -import { createCorrelationContextHeader, requestTracingEnabled } from "./requestTracing/utils"; import { DefaultRefreshIntervalInMs, MinimumRefreshIntervalInMs } from "./RefreshOptions"; import { Disposable } from "./common/disposable"; -import { SettingSelector } from "./types"; +import { AzureKeyVaultKeyValueAdapter } from "./keyvault/AzureKeyVaultKeyValueAdapter"; import { RefreshTimer } from "./refresh/RefreshTimer"; +import { CorrelationContextHeaderName, RequestType } from "./requestTracing/constants"; +import { createCorrelationContextHeader, requestTracingEnabled } from "./requestTracing/utils"; +import { KeyFilter, LabelFilter, SettingSelector } from "./types"; export class AzureAppConfigurationImpl extends Map implements AzureAppConfiguration { #adapters: IKeyValueAdapter[] = []; @@ -122,6 +122,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure } } } + this.clear(); // clear existing key-values in case of configuration setting deletion for (const [k, v] of keyValues) { this.set(k, v); } @@ -143,15 +144,25 @@ export class AzureAppConfigurationImpl extends Map implements Azure // try refresh if any of watched settings is changed. let needRefresh = false; for (const sentinel of this.#sentinels) { - const response = await this.#client.getConfigurationSetting(sentinel, { - onlyIfChanged: true, - requestOptions: { - customHeaders: this.#customHeaders(RequestType.Watch) + let response: GetConfigurationSettingResponse | undefined; + try { + response = await this.#client.getConfigurationSetting(sentinel, { + onlyIfChanged: true, + requestOptions: { + customHeaders: this.#customHeaders(RequestType.Watch) + } + }); + } catch (error) { + if (error instanceof RestError && error.statusCode === 404) { + response = undefined; + } else { + throw error; } - }); - if (response.statusCode === 200) { - // sentinel changed. - sentinel.etag = response.etag;// update etag of the sentinel + } + + if (response === undefined || response.statusCode === 200) { + // sentinel deleted / changed. + sentinel.etag = response?.etag;// update etag of the sentinel needRefresh = true; break; } diff --git a/test/refresh.test.ts b/test/refresh.test.ts index e60c2bff..34b674a0 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -147,6 +147,32 @@ describe("dynamic refresh", function () { expect(settings.get("app.settings.fontColor")).eq("blue"); }); + it("should update values when watched setting is deleted", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + refreshOptions: { + enabled: true, + refreshIntervalInMs: 2000, + watchedSettings: [ + { key: "app.settings.fontColor" } + ] + } + }); + expect(settings).not.undefined; + expect(settings.get("app.settings.fontColor")).eq("red"); + expect(settings.get("app.settings.fontSize")).eq("40"); + + // delete setting 'app.settings.fontColor' + const newMockedKVs = mockedKVs.filter(elem => elem.key !== "app.settings.fontColor"); + restoreMocks(); + mockAppConfigurationClientListConfigurationSettings(newMockedKVs); + mockAppConfigurationClientGetConfigurationSetting(newMockedKVs); + + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + expect(settings.get("app.settings.fontColor")).eq(undefined); + }); + it("should not update values when unwatched setting changes", async () => { const connectionString = createMockedConnectionString(); const settings = await load(connectionString, { @@ -221,4 +247,5 @@ describe("dynamic refresh", function () { await settings.refresh(); expect(count).eq(1); }); + }); \ No newline at end of file From c8c8d8e74adec1540908086101d3aac658c7114f Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 4 Jan 2024 11:17:42 +0800 Subject: [PATCH 26/43] Remove unused variable --- src/AzureAppConfigurationImpl.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index a51fd8b0..2bb255aa 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -23,7 +23,6 @@ export class AzureAppConfigurationImpl extends Map implements Azure */ #sortedTrimKeyPrefixes: string[] | undefined; readonly #requestTracingEnabled: boolean; - #correlationContextHeader: string | undefined; #client: AppConfigurationClient; #options: AzureAppConfigurationOptions | undefined; From 7663a2733becdabca6970b805d99da42bd3f1aed Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 4 Jan 2024 11:29:00 +0800 Subject: [PATCH 27/43] export type Disposable --- examples/README.md | 2 +- examples/refresh.mjs | 1 - src/index.ts | 5 +++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/README.md b/examples/README.md index 9405d884..43efa207 100644 --- a/examples/README.md +++ b/examples/README.md @@ -39,7 +39,7 @@ To run the examples using the published version of the package: ```bash npx cross-env APPCONFIG_CONNECTION_STRING="" ``` - + 3. Run the examples: ```bash node helloworld.mjs diff --git a/examples/refresh.mjs b/examples/refresh.mjs index b3945f3d..12231fcb 100644 --- a/examples/refresh.mjs +++ b/examples/refresh.mjs @@ -31,7 +31,6 @@ const settings = await load(connectionString, { console.log("Using Azure portal or CLI, update the `app.settings.message` value, and then update the `app.settings.sentinel` value in your App Configuration store.") - // eslint-disable-next-line no-constant-condition while (true) { // Refreshing the configuration setting diff --git a/src/index.ts b/src/index.ts index 524dbec7..dd246046 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -export { load } from "./load"; export { AzureAppConfiguration } from "./AzureAppConfiguration"; -export { KeyFilter, LabelFilter } from "./types"; \ No newline at end of file +export { Disposable } from "./common/disposable"; +export { load } from "./load"; +export { KeyFilter, LabelFilter } from "./types"; From ead82d00cbb09811bf4e4e69237f76dc9e2a35f8 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 4 Jan 2024 14:11:23 +0800 Subject: [PATCH 28/43] add detailed description for refresh timer --- src/refresh/RefreshTimer.ts | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index cfd22ca1..08d10bf1 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -1,6 +1,21 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. +/** + * The backoff time is between the minimum and maximum backoff time, based on the number of attempts. + * An exponential backoff strategy is used, with a jitter factor to prevent clients from retrying at the same time. + * + * The backoff time is calculated as follows: + * - `basic backoff time` = `MinimumBackoffInMs` * 2 ^ `attempts`, and it is no larger than the `MaximumBackoffInMs`. + * - based on jitter ratio, the jittered time is between [-1, 1) * `JitterRatio` * basic backoff time. + * - the final backoff time is the basic backoff time plus the jittered time. + * + * Note: the backoff time usually is no larger than the refresh interval, which is specified by the user. + * - If the interval is less than the minimum backoff, the interval is used. + * - If the interval is between the minimum and maximum backoff, the interval is used as the maximum backoff. + * - Because of the jitter, the maximum backoff time is actually `MaximumBackoffInMs` * (1 + `JitterRatio`). + */ + const MinimumBackoffInMs = 30 * 1000; // 30s const MaximumBackoffInMs = 10 * 60 * 1000; // 10min const MaxAttempts = 63; @@ -50,14 +65,15 @@ export class RefreshTimer { maxBackoffMs = MaximumBackoffInMs; } - // exponential + // exponential: minBackoffMs * 2^attempts let calculatedBackoffMs = Math.max(1, minBackoffMs) * (1 << Math.min(this._attempts, MaxAttempts)); if (calculatedBackoffMs > maxBackoffMs) { calculatedBackoffMs = maxBackoffMs; } - // jitter + // jitter: random value between [-1, 1) * jitterRatio * calculatedBackoffMs const jitter = JitterRatio * (Math.random() * 2 - 1); + return calculatedBackoffMs * (1 + jitter); } From e0c9736bf0d8e10ce29547e0b0360080d74f5be7 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 4 Jan 2024 16:04:40 +0800 Subject: [PATCH 29/43] Refactor RefreshTimer class to use efficient power of two calculation This commit refactors the RefreshTimer class in RefreshTimer.ts to use an efficient power of two calculation method. The new method, efficientPowerOfTwo, replaces the previous calculation using Math.pow and bitwise operations. This improves performance and avoids potential overflow issues. --- src/refresh/RefreshTimer.ts | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index 08d10bf1..55321969 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -18,7 +18,7 @@ const MinimumBackoffInMs = 30 * 1000; // 30s const MaximumBackoffInMs = 10 * 60 * 1000; // 10min -const MaxAttempts = 63; +const MaxSafeExponential = 53; // Used to avoid overflow, as Number.MAX_SAFE_INTEGER = 2^53 - 1. const JitterRatio = 0.25; export class RefreshTimer { @@ -29,6 +29,10 @@ export class RefreshTimer { constructor( private _interval: number ) { + if (this._interval <= 0) { + throw new Error(`Refresh interval must be greater than 0. Given: ${this._interval}`); + } + this._minBackoff = Math.min(this._interval, MinimumBackoffInMs); this._maxBackoff = Math.min(this._interval, MaximumBackoffInMs); this._attempts = 0; @@ -66,7 +70,8 @@ export class RefreshTimer { } // exponential: minBackoffMs * 2^attempts - let calculatedBackoffMs = Math.max(1, minBackoffMs) * (1 << Math.min(this._attempts, MaxAttempts)); + const exponential = Math.min(this._attempts, MaxSafeExponential); + let calculatedBackoffMs = minBackoffMs * (efficientPowerOfTwo(exponential)); if (calculatedBackoffMs > maxBackoffMs) { calculatedBackoffMs = maxBackoffMs; } @@ -77,4 +82,25 @@ export class RefreshTimer { return calculatedBackoffMs * (1 + jitter); } -} \ No newline at end of file +} + +/** + * Efficient way to calculate 2^exponential. + * + * `Math.pow(base, exp)` is not used because it is less efficient and accurate by operating floating-point number. + * `1 << exp` is not used because it returns wrong results when exp >= 31. + */ +function efficientPowerOfTwo(positiveExponential: number) { + if (positiveExponential < 0) { + throw new Error("exponential must be a non-negative integer."); + } else if (positiveExponential > MaxSafeExponential) { + throw new Error(`exponential must be less than or equal to ${MaxSafeExponential}.`); + } + + // bitwise operations in JavaScript are limited to 32 bits. It overflows at 2^31 - 1. + if (positiveExponential <= 30) { + return 1 << positiveExponential; + } else { + return (1 << 30) * (1 << (positiveExponential - 30)); + } +} From 0486c103a3e0f54ae9b9e81664affa9bc26d618c Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 4 Jan 2024 17:03:31 +0800 Subject: [PATCH 30/43] rename variable name for clarity --- src/refresh/RefreshTimer.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index 55321969..fe6d8c76 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -24,7 +24,7 @@ const JitterRatio = 0.25; export class RefreshTimer { private _minBackoff: number; private _maxBackoff: number; - private _attempts: number; + private _failedAttempts: number; private _backoffEnd: number; // Timestamp constructor( private _interval: number @@ -35,7 +35,7 @@ export class RefreshTimer { this._minBackoff = Math.min(this._interval, MinimumBackoffInMs); this._maxBackoff = Math.min(this._interval, MaximumBackoffInMs); - this._attempts = 0; + this._failedAttempts = 0; this._backoffEnd = Date.now() + this._interval; } @@ -45,12 +45,12 @@ export class RefreshTimer { public backoff(): void { this._backoffEnd = Date.now() + this._calculateBackoffTime(); - this._attempts += 1; + this._failedAttempts += 1; } public reset(): void { this._backoffEnd = Date.now() + this._interval; - this._attempts = 0; + this._failedAttempts = 0; } private _calculateBackoffTime(): number { @@ -69,8 +69,8 @@ export class RefreshTimer { maxBackoffMs = MaximumBackoffInMs; } - // exponential: minBackoffMs * 2^attempts - const exponential = Math.min(this._attempts, MaxSafeExponential); + // exponential: minBackoffMs * 2^failedAttempts + const exponential = Math.min(this._failedAttempts, MaxSafeExponential); let calculatedBackoffMs = minBackoffMs * (efficientPowerOfTwo(exponential)); if (calculatedBackoffMs > maxBackoffMs) { calculatedBackoffMs = maxBackoffMs; From fe3614f67dffde3ba3886ea9aa7367de23052320 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 4 Jan 2024 17:14:15 +0800 Subject: [PATCH 31/43] remove redundant code --- src/refresh/RefreshTimer.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index fe6d8c76..bb0581f3 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -22,9 +22,9 @@ const MaxSafeExponential = 53; // Used to avoid overflow, as Number.MAX_SAFE_INT const JitterRatio = 0.25; export class RefreshTimer { - private _minBackoff: number; - private _maxBackoff: number; - private _failedAttempts: number; + private _minBackoff: number = MinimumBackoffInMs; + private _maxBackoff: number = MaximumBackoffInMs; + private _failedAttempts: number = 0; private _backoffEnd: number; // Timestamp constructor( private _interval: number @@ -33,9 +33,6 @@ export class RefreshTimer { throw new Error(`Refresh interval must be greater than 0. Given: ${this._interval}`); } - this._minBackoff = Math.min(this._interval, MinimumBackoffInMs); - this._maxBackoff = Math.min(this._interval, MaximumBackoffInMs); - this._failedAttempts = 0; this._backoffEnd = Date.now() + this._interval; } @@ -62,11 +59,11 @@ export class RefreshTimer { // _minBackoff <= _interval if (this._interval <= this._maxBackoff) { - minBackoffMs = MinimumBackoffInMs + minBackoffMs = this._minBackoff; maxBackoffMs = this._interval; } else { - minBackoffMs = MinimumBackoffInMs; - maxBackoffMs = MaximumBackoffInMs; + minBackoffMs = this._minBackoff; + maxBackoffMs = this._maxBackoff; } // exponential: minBackoffMs * 2^failedAttempts From 98b12e295298a402fe6747256a92964f07fdb11e Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 10 Jan 2024 14:15:44 +0800 Subject: [PATCH 32/43] limit max exponential to 30 and remove utils no longer needed --- src/refresh/RefreshTimer.ts | 33 ++++++--------------------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index bb0581f3..ac26f31b 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -18,7 +18,7 @@ const MinimumBackoffInMs = 30 * 1000; // 30s const MaximumBackoffInMs = 10 * 60 * 1000; // 10min -const MaxSafeExponential = 53; // Used to avoid overflow, as Number.MAX_SAFE_INTEGER = 2^53 - 1. +const MaxSafeExponential = 30; // Used to avoid overflow. bitwise operations in JavaScript are limited to 32 bits. It overflows at 2^31 - 1. const JitterRatio = 0.25; export class RefreshTimer { @@ -41,13 +41,13 @@ export class RefreshTimer { } public backoff(): void { - this._backoffEnd = Date.now() + this._calculateBackoffTime(); this._failedAttempts += 1; + this._backoffEnd = Date.now() + this._calculateBackoffTime(); } public reset(): void { - this._backoffEnd = Date.now() + this._interval; this._failedAttempts = 0; + this._backoffEnd = Date.now() + this._interval; } private _calculateBackoffTime(): number { @@ -66,9 +66,9 @@ export class RefreshTimer { maxBackoffMs = this._maxBackoff; } - // exponential: minBackoffMs * 2^failedAttempts - const exponential = Math.min(this._failedAttempts, MaxSafeExponential); - let calculatedBackoffMs = minBackoffMs * (efficientPowerOfTwo(exponential)); + // exponential: minBackoffMs * 2^(failedAttempts-1) + const exponential = Math.min(this._failedAttempts - 1, MaxSafeExponential); + let calculatedBackoffMs = minBackoffMs * (1 << exponential); if (calculatedBackoffMs > maxBackoffMs) { calculatedBackoffMs = maxBackoffMs; } @@ -80,24 +80,3 @@ export class RefreshTimer { } } - -/** - * Efficient way to calculate 2^exponential. - * - * `Math.pow(base, exp)` is not used because it is less efficient and accurate by operating floating-point number. - * `1 << exp` is not used because it returns wrong results when exp >= 31. - */ -function efficientPowerOfTwo(positiveExponential: number) { - if (positiveExponential < 0) { - throw new Error("exponential must be a non-negative integer."); - } else if (positiveExponential > MaxSafeExponential) { - throw new Error(`exponential must be less than or equal to ${MaxSafeExponential}.`); - } - - // bitwise operations in JavaScript are limited to 32 bits. It overflows at 2^31 - 1. - if (positiveExponential <= 30) { - return 1 << positiveExponential; - } else { - return (1 << 30) * (1 << (positiveExponential - 30)); - } -} From 506c8a61ae4210f9b2dadf06ebca8127e08dfb3c Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 10 Jan 2024 14:22:15 +0800 Subject: [PATCH 33/43] throw error on refresh when refresh is not enabled --- src/AzureAppConfigurationImpl.ts | 2 +- test/refresh.test.ts | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 2bb255aa..30b79440 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -132,7 +132,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure */ public async refresh(): Promise { if (!this.#refreshEnabled) { - return Promise.resolve(); + throw new Error("Refresh is not enabled."); } // if still within refresh interval/backoff, return diff --git a/test/refresh.test.ts b/test/refresh.test.ts index 34b674a0..e90b6472 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -38,6 +38,13 @@ describe("dynamic refresh", function () { restoreMocks(); }) + it("should throw error when refresh is not enabled but refresh is called", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString); + const refreshCall = settings.refresh(); + return expect(refreshCall).eventually.rejectedWith("Refresh is not enabled."); + }); + it("should only allow non-empty list of watched settings when refresh is enabled", async () => { const connectionString = createMockedConnectionString(); const loadWithEmptyWatchedSettings = load(connectionString, { From 5d80ccca527000e6cb2083f93557172d28463582 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 10 Jan 2024 19:22:34 +0800 Subject: [PATCH 34/43] load watched settings if not coverred by selectors --- src/AzureAppConfigurationImpl.ts | 119 ++++++++++++++++++++++--------- test/refresh.test.ts | 39 +++++++++- 2 files changed, 123 insertions(+), 35 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 30b79440..76ef40b2 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { AppConfigurationClient, ConfigurationSetting, ConfigurationSettingId, GetConfigurationSettingResponse, ListConfigurationSettingsOptions } from "@azure/app-configuration"; +import { AppConfigurationClient, ConfigurationSetting, ConfigurationSettingId, GetConfigurationSettingOptions, GetConfigurationSettingResponse, ListConfigurationSettingsOptions } from "@azure/app-configuration"; import { RestError } from "@azure/core-rest-pipeline"; import { AzureAppConfiguration } from "./AzureAppConfiguration"; import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions"; @@ -15,6 +15,11 @@ import { CorrelationContextHeaderName, RequestType } from "./requestTracing/cons import { createCorrelationContextHeader, requestTracingEnabled } from "./requestTracing/utils"; import { KeyFilter, LabelFilter, SettingSelector } from "./types"; +type KeyValueIdentifier = string; // key::label +function toKeyValueIderntifier(key: string, label: string | undefined): KeyValueIdentifier { + return `${key}::${label ?? ""}`; +} + export class AzureAppConfigurationImpl extends Map implements AzureAppConfiguration { #adapters: IKeyValueAdapter[] = []; /** @@ -29,7 +34,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure // Refresh #refreshInterval: number = DefaultRefreshIntervalInMs; #onRefreshListeners: Array<() => any> = []; - #sentinels: ConfigurationSettingId[]; + #sentinels: Map = new Map(); #refreshTimer: RefreshTimer; constructor( @@ -64,15 +69,16 @@ export class AzureAppConfigurationImpl extends Map implements Azure } } - this.#sentinels = watchedSettings.map(setting => { + for (const setting of watchedSettings) { if (setting.key.includes("*") || setting.key.includes(",")) { throw new Error("The characters '*' and ',' are not supported in key of watched settings."); } if (setting.label?.includes("*") || setting.label?.includes(",")) { throw new Error("The characters '*' and ',' are not supported in label of watched settings."); } - return { ...setting }; - }); + const id = toKeyValueIderntifier(setting.key, setting.label); + this.#sentinels.set(id, setting); + } this.#refreshTimer = new RefreshTimer(this.#refreshInterval); } @@ -88,8 +94,8 @@ export class AzureAppConfigurationImpl extends Map implements Azure return !!this.#options?.refreshOptions?.enabled; } - async load(requestType: RequestType = RequestType.Startup) { - const keyValues: [key: string, value: unknown][] = []; + async #loadSelectedKeyValues(requestType: RequestType): Promise> { + const loadedSettings = new Map(); // validate selectors const selectors = getValidSelectors(this.#options?.selectors); @@ -108,19 +114,57 @@ export class AzureAppConfigurationImpl extends Map implements Azure const settings = this.#client.listConfigurationSettings(listOptions); for await (const setting of settings) { - if (setting.key) { - const keyValuePair = await this.#processKeyValues(setting); - keyValues.push(keyValuePair); - } - // update etag of sentinels if refresh is enabled - if (this.#refreshEnabled) { - const matchedSentinel = this.#sentinels.find(s => s.key === setting.key && s.label === setting.label); - if (matchedSentinel) { - matchedSentinel.etag = setting.etag; - } + const id = toKeyValueIderntifier(setting.key, setting.label); + loadedSettings.set(id, setting); + } + } + return loadedSettings; + } + + /** + * Load watched key-values from Azure App Configuration service if not coverred by the selectors. Update etag of sentinels. + */ + async #loadWatchedKeyValues(requestType: RequestType, existingSettings: Map): Promise> { + const watchedSettings = new Map(); + + if (!this.#refreshEnabled) { + return watchedSettings; + } + + for (const [id, sentinel] of this.#sentinels) { + const matchedSetting = existingSettings.get(id); + if (matchedSetting) { + watchedSettings.set(id, matchedSetting); + sentinel.etag = matchedSetting.etag; + } else { + // Send a request to retrieve key-value since it may be either not loaded or loaded with a different label or different casing + const { key, label } = sentinel; + const response = await this.#getConfigurationSettingWithTrace(requestType, { key, label }); + if (response) { + watchedSettings.set(id, response); + sentinel.etag = response.etag; + } else { + sentinel.etag = undefined; } } } + return watchedSettings; + } + + async load(requestType: RequestType = RequestType.Startup) { + const keyValues: [key: string, value: unknown][] = []; + + const loadedSettings = await this.#loadSelectedKeyValues(requestType); + const watchedSettings = await this.#loadWatchedKeyValues(requestType, loadedSettings); + + // process key-values, watched settings have higher priority + for (const setting of [...loadedSettings.values(), ...watchedSettings.values()]) { + if (setting.key) { + const [key, value] = await this.#processKeyValues(setting); + keyValues.push([key, value]); + } + } + this.clear(); // clear existing key-values in case of configuration setting deletion for (const [k, v] of keyValues) { this.set(k, v); @@ -142,22 +186,10 @@ export class AzureAppConfigurationImpl extends Map implements Azure // try refresh if any of watched settings is changed. let needRefresh = false; - for (const sentinel of this.#sentinels) { - let response: GetConfigurationSettingResponse | undefined; - try { - response = await this.#client.getConfigurationSetting(sentinel, { - onlyIfChanged: true, - requestOptions: { - customHeaders: this.#customHeaders(RequestType.Watch) - } - }); - } catch (error) { - if (error instanceof RestError && error.statusCode === 404) { - response = undefined; - } else { - throw error; - } - } + for (const sentinel of this.#sentinels.values()) { + const response = await this.#getConfigurationSettingWithTrace(RequestType.Watch, sentinel, { + onlyIfChanged: true + }); if (response === undefined || response.statusCode === 200) { // sentinel deleted / changed. @@ -235,6 +267,25 @@ export class AzureAppConfigurationImpl extends Map implements Azure headers[CorrelationContextHeaderName] = createCorrelationContextHeader(this.#options, requestType); return headers; } + + async #getConfigurationSettingWithTrace(requestType: RequestType, configurationSettingId: ConfigurationSettingId, options?: GetConfigurationSettingOptions): Promise { + let response: GetConfigurationSettingResponse | undefined; + try { + response = await this.#client.getConfigurationSetting(configurationSettingId, { + ...options ?? {}, + requestOptions: { + customHeaders: this.#customHeaders(requestType) + } + }); + } catch (error) { + if (error instanceof RestError && error.statusCode === 404) { + response = undefined; + } else { + throw error; + } + } + return response; + } } function getValidSelectors(selectors?: SettingSelector[]) { @@ -266,4 +317,4 @@ function getValidSelectors(selectors?: SettingSelector[]) { } return selector; }); -} \ No newline at end of file +} diff --git a/test/refresh.test.ts b/test/refresh.test.ts index e90b6472..7d9896f9 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -28,7 +28,8 @@ describe("dynamic refresh", function () { beforeEach(() => { mockedKVs = [ { value: "red", key: "app.settings.fontColor" }, - { value: "40", key: "app.settings.fontSize" } + { value: "40", key: "app.settings.fontSize" }, + { value: "30", key: "app.settings.fontSize", label: "prod" } ].map(createMockedKeyValue); mockAppConfigurationClientListConfigurationSettings(mockedKVs); mockAppConfigurationClientGetConfigurationSetting(mockedKVs) @@ -255,4 +256,40 @@ describe("dynamic refresh", function () { expect(count).eq(1); }); + it("should also load watched settings if not specified in selectors", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + selectors: [ + { keyFilter: "app.settings.fontColor" } + ], + refreshOptions: { + enabled: true, + refreshIntervalInMs: 2000, + watchedSettings: [ + { key: "app.settings.fontSize" } + ] + } + }); + expect(settings).not.undefined; + expect(settings.get("app.settings.fontColor")).eq("red"); + expect(settings.get("app.settings.fontSize")).eq("40"); + }); + + it("watched settings have higher priority than selectors", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + selectors: [ + { keyFilter: "app.settings.*" } + ], + refreshOptions: { + enabled: true, + refreshIntervalInMs: 2000, + watchedSettings: [ + { key: "app.settings.fontSize", label: "prod" } + ] + } + }); + expect(settings).not.undefined; + expect(settings.get("app.settings.fontSize")).eq("30"); + }); }); \ No newline at end of file From e311ce950bc5285870685308f48d9a48edcb835b Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 10 Jan 2024 19:34:37 +0800 Subject: [PATCH 35/43] add comments for the Map key trick --- src/AzureAppConfigurationImpl.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 76ef40b2..5369840f 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -15,8 +15,12 @@ import { CorrelationContextHeaderName, RequestType } from "./requestTracing/cons import { createCorrelationContextHeader, requestTracingEnabled } from "./requestTracing/utils"; import { KeyFilter, LabelFilter, SettingSelector } from "./types"; +/** + * Key-value identifier, used as key of a Map. + * A primitive string type is actually used, because operator === is used to compare identifiers and we cannot override it. + */ type KeyValueIdentifier = string; // key::label -function toKeyValueIderntifier(key: string, label: string | undefined): KeyValueIdentifier { +function toKeyValueIdentifier(key: string, label: string | undefined): KeyValueIdentifier { return `${key}::${label ?? ""}`; } @@ -76,7 +80,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure if (setting.label?.includes("*") || setting.label?.includes(",")) { throw new Error("The characters '*' and ',' are not supported in label of watched settings."); } - const id = toKeyValueIderntifier(setting.key, setting.label); + const id = toKeyValueIdentifier(setting.key, setting.label); this.#sentinels.set(id, setting); } @@ -114,7 +118,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure const settings = this.#client.listConfigurationSettings(listOptions); for await (const setting of settings) { - const id = toKeyValueIderntifier(setting.key, setting.label); + const id = toKeyValueIdentifier(setting.key, setting.label); loadedSettings.set(id, setting); } } From e3ed82f0a34a57b69aabf3681d6b5436fb85b3e0 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 10 Jan 2024 19:52:44 +0800 Subject: [PATCH 36/43] deduce type from state isInitialLoadCompleted --- src/AzureAppConfigurationImpl.ts | 40 +++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 5369840f..696799fd 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -34,6 +34,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure readonly #requestTracingEnabled: boolean; #client: AppConfigurationClient; #options: AzureAppConfigurationOptions | undefined; + #isInitialLoadCompleted: boolean = false; // Refresh #refreshInterval: number = DefaultRefreshIntervalInMs; @@ -98,7 +99,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure return !!this.#options?.refreshOptions?.enabled; } - async #loadSelectedKeyValues(requestType: RequestType): Promise> { + async #loadSelectedKeyValues(): Promise> { const loadedSettings = new Map(); // validate selectors @@ -110,6 +111,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure labelFilter: selector.labelFilter }; if (this.#requestTracingEnabled) { + const requestType: RequestType = this.#isInitialLoadCompleted ? RequestType.Watch : RequestType.Startup; listOptions.requestOptions = { customHeaders: this.#customHeaders(requestType) } @@ -128,7 +130,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure /** * Load watched key-values from Azure App Configuration service if not coverred by the selectors. Update etag of sentinels. */ - async #loadWatchedKeyValues(requestType: RequestType, existingSettings: Map): Promise> { + async #loadWatchedKeyValues(existingSettings: Map): Promise> { const watchedSettings = new Map(); if (!this.#refreshEnabled) { @@ -143,7 +145,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure } else { // Send a request to retrieve key-value since it may be either not loaded or loaded with a different label or different casing const { key, label } = sentinel; - const response = await this.#getConfigurationSettingWithTrace(requestType, { key, label }); + const response = await this.#getConfigurationSettingWithTrace({ key, label }); if (response) { watchedSettings.set(id, response); sentinel.etag = response.etag; @@ -155,11 +157,11 @@ export class AzureAppConfigurationImpl extends Map implements Azure return watchedSettings; } - async load(requestType: RequestType = RequestType.Startup) { + async #loadSelectedAndWatchedKeyValues() { const keyValues: [key: string, value: unknown][] = []; - const loadedSettings = await this.#loadSelectedKeyValues(requestType); - const watchedSettings = await this.#loadWatchedKeyValues(requestType, loadedSettings); + const loadedSettings = await this.#loadSelectedKeyValues(); + const watchedSettings = await this.#loadWatchedKeyValues(loadedSettings); // process key-values, watched settings have higher priority for (const setting of [...loadedSettings.values(), ...watchedSettings.values()]) { @@ -175,6 +177,16 @@ export class AzureAppConfigurationImpl extends Map implements Azure } } + /** + * Load the configuration store for the first time. + */ + async load() { + await this.#loadSelectedAndWatchedKeyValues(); + + // Mark all settings have loaded at startup. + this.#isInitialLoadCompleted = true; + } + /** * Refresh the configuration store. */ @@ -191,7 +203,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure // try refresh if any of watched settings is changed. let needRefresh = false; for (const sentinel of this.#sentinels.values()) { - const response = await this.#getConfigurationSettingWithTrace(RequestType.Watch, sentinel, { + const response = await this.#getConfigurationSettingWithTrace(sentinel, { onlyIfChanged: true }); @@ -204,7 +216,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure } if (needRefresh) { try { - await this.load(RequestType.Watch); + await this.#loadSelectedAndWatchedKeyValues(); this.#refreshTimer.reset(); } catch (error) { // if refresh failed, backoff @@ -272,15 +284,17 @@ export class AzureAppConfigurationImpl extends Map implements Azure return headers; } - async #getConfigurationSettingWithTrace(requestType: RequestType, configurationSettingId: ConfigurationSettingId, options?: GetConfigurationSettingOptions): Promise { + async #getConfigurationSettingWithTrace(configurationSettingId: ConfigurationSettingId, customOptions?: GetConfigurationSettingOptions): Promise { let response: GetConfigurationSettingResponse | undefined; try { - response = await this.#client.getConfigurationSetting(configurationSettingId, { - ...options ?? {}, - requestOptions: { + const options = {...customOptions ?? {}}; + if (this.#requestTracingEnabled) { + const requestType = this.#isInitialLoadCompleted ? RequestType.Watch : RequestType.Startup; + options.requestOptions = { customHeaders: this.#customHeaders(requestType) } - }); + } + response = await this.#client.getConfigurationSetting(configurationSettingId, options); } catch (error) { if (error instanceof RestError && error.statusCode === 404) { response = undefined; From d838076b3bc536ed87e7497493d755cabd6ce891 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 11 Jan 2024 13:05:44 +0800 Subject: [PATCH 37/43] revert unnecessary whitespace change --- examples/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/README.md b/examples/README.md index 43efa207..9405d884 100644 --- a/examples/README.md +++ b/examples/README.md @@ -39,7 +39,7 @@ To run the examples using the published version of the package: ```bash npx cross-env APPCONFIG_CONNECTION_STRING="" ``` - + 3. Run the examples: ```bash node helloworld.mjs From e03036dcbda28b1ac999760b153b3cf526f8d99e Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 11 Jan 2024 13:14:02 +0800 Subject: [PATCH 38/43] simplify request tracing header utils --- src/AzureAppConfigurationImpl.ts | 22 +++++++--------------- src/requestTracing/utils.ts | 4 ++-- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 696799fd..7838d173 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -11,7 +11,7 @@ import { DefaultRefreshIntervalInMs, MinimumRefreshIntervalInMs } from "./Refres import { Disposable } from "./common/disposable"; import { AzureKeyVaultKeyValueAdapter } from "./keyvault/AzureKeyVaultKeyValueAdapter"; import { RefreshTimer } from "./refresh/RefreshTimer"; -import { CorrelationContextHeaderName, RequestType } from "./requestTracing/constants"; +import { CorrelationContextHeaderName } from "./requestTracing/constants"; import { createCorrelationContextHeader, requestTracingEnabled } from "./requestTracing/utils"; import { KeyFilter, LabelFilter, SettingSelector } from "./types"; @@ -111,9 +111,10 @@ export class AzureAppConfigurationImpl extends Map implements Azure labelFilter: selector.labelFilter }; if (this.#requestTracingEnabled) { - const requestType: RequestType = this.#isInitialLoadCompleted ? RequestType.Watch : RequestType.Startup; listOptions.requestOptions = { - customHeaders: this.#customHeaders(requestType) + customHeaders: { + [CorrelationContextHeaderName]: createCorrelationContextHeader(this.#options, this.#isInitialLoadCompleted) + } } } @@ -274,24 +275,15 @@ export class AzureAppConfigurationImpl extends Map implements Azure return key; } - #customHeaders(requestType: RequestType) { - if (!this.#requestTracingEnabled) { - return undefined; - } - - const headers = {}; - headers[CorrelationContextHeaderName] = createCorrelationContextHeader(this.#options, requestType); - return headers; - } - async #getConfigurationSettingWithTrace(configurationSettingId: ConfigurationSettingId, customOptions?: GetConfigurationSettingOptions): Promise { let response: GetConfigurationSettingResponse | undefined; try { const options = {...customOptions ?? {}}; if (this.#requestTracingEnabled) { - const requestType = this.#isInitialLoadCompleted ? RequestType.Watch : RequestType.Startup; options.requestOptions = { - customHeaders: this.#customHeaders(requestType) + customHeaders: { + [CorrelationContextHeaderName]: createCorrelationContextHeader(this.#options, this.#isInitialLoadCompleted) + } } } response = await this.#client.getConfigurationSetting(configurationSettingId, options); diff --git a/src/requestTracing/utils.ts b/src/requestTracing/utils.ts index 519a31e3..8966a308 100644 --- a/src/requestTracing/utils.ts +++ b/src/requestTracing/utils.ts @@ -21,7 +21,7 @@ import { } from "./constants"; // Utils -export function createCorrelationContextHeader(options: AzureAppConfigurationOptions | undefined, requestType: RequestType): string { +export function createCorrelationContextHeader(options: AzureAppConfigurationOptions | undefined, isInitialLoadCompleted: boolean): string { /* RequestType: 'Startup' during application starting up, 'Watch' after startup completed. Host: identify with defined envs @@ -29,7 +29,7 @@ export function createCorrelationContextHeader(options: AzureAppConfigurationOpt UsersKeyVault */ const keyValues = new Map(); - keyValues.set(RequestTypeKey, requestType); + keyValues.set(RequestTypeKey, isInitialLoadCompleted ? RequestType.Watch : RequestType.Startup); keyValues.set(HostTypeKey, getHostType()); keyValues.set(EnvironmentKey, isDevEnvironment() ? DevEnvironmentValue : undefined); From 97e03504873d4d0e1076c13a87c96b5ad2c9897f Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 11 Jan 2024 15:58:44 +0800 Subject: [PATCH 39/43] Exclude watched settings from configuration --- src/AzureAppConfigurationImpl.ts | 18 ++++++++---------- test/refresh.test.ts | 22 ++-------------------- 2 files changed, 10 insertions(+), 30 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 7838d173..3940438a 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -39,6 +39,9 @@ export class AzureAppConfigurationImpl extends Map implements Azure // Refresh #refreshInterval: number = DefaultRefreshIntervalInMs; #onRefreshListeners: Array<() => any> = []; + /** + * Aka watched settings. + */ #sentinels: Map = new Map(); #refreshTimer: RefreshTimer; @@ -129,43 +132,38 @@ export class AzureAppConfigurationImpl extends Map implements Azure } /** - * Load watched key-values from Azure App Configuration service if not coverred by the selectors. Update etag of sentinels. + * Update etag of watched settings from loaded data. If a watched setting is not covered by any selector, a request will be sent to retrieve it. */ - async #loadWatchedKeyValues(existingSettings: Map): Promise> { - const watchedSettings = new Map(); - + async #updateWatchedKeyValuesEtag(existingSettings: Map): Promise { if (!this.#refreshEnabled) { - return watchedSettings; + return; } for (const [id, sentinel] of this.#sentinels) { const matchedSetting = existingSettings.get(id); if (matchedSetting) { - watchedSettings.set(id, matchedSetting); sentinel.etag = matchedSetting.etag; } else { // Send a request to retrieve key-value since it may be either not loaded or loaded with a different label or different casing const { key, label } = sentinel; const response = await this.#getConfigurationSettingWithTrace({ key, label }); if (response) { - watchedSettings.set(id, response); sentinel.etag = response.etag; } else { sentinel.etag = undefined; } } } - return watchedSettings; } async #loadSelectedAndWatchedKeyValues() { const keyValues: [key: string, value: unknown][] = []; const loadedSettings = await this.#loadSelectedKeyValues(); - const watchedSettings = await this.#loadWatchedKeyValues(loadedSettings); + await this.#updateWatchedKeyValuesEtag(loadedSettings); // process key-values, watched settings have higher priority - for (const setting of [...loadedSettings.values(), ...watchedSettings.values()]) { + for (const setting of loadedSettings.values()) { if (setting.key) { const [key, value] = await this.#processKeyValues(setting); keyValues.push([key, value]); diff --git a/test/refresh.test.ts b/test/refresh.test.ts index 7d9896f9..2fb05704 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -256,7 +256,7 @@ describe("dynamic refresh", function () { expect(count).eq(1); }); - it("should also load watched settings if not specified in selectors", async () => { + it("should not include watched settings into configuration if not specified in selectors", async () => { const connectionString = createMockedConnectionString(); const settings = await load(connectionString, { selectors: [ @@ -272,24 +272,6 @@ describe("dynamic refresh", function () { }); expect(settings).not.undefined; expect(settings.get("app.settings.fontColor")).eq("red"); - expect(settings.get("app.settings.fontSize")).eq("40"); - }); - - it("watched settings have higher priority than selectors", async () => { - const connectionString = createMockedConnectionString(); - const settings = await load(connectionString, { - selectors: [ - { keyFilter: "app.settings.*" } - ], - refreshOptions: { - enabled: true, - refreshIntervalInMs: 2000, - watchedSettings: [ - { key: "app.settings.fontSize", label: "prod" } - ] - } - }); - expect(settings).not.undefined; - expect(settings.get("app.settings.fontSize")).eq("30"); + expect(settings.get("app.settings.fontSize")).undefined; }); }); \ No newline at end of file From d0440dc7e27b15902e1c275016fe28165742de55 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Fri, 12 Jan 2024 10:01:08 +0800 Subject: [PATCH 40/43] Change sentinels to array type to ensure correctness --- src/AzureAppConfigurationImpl.ts | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 3940438a..375a8930 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -15,15 +15,6 @@ import { CorrelationContextHeaderName } from "./requestTracing/constants"; import { createCorrelationContextHeader, requestTracingEnabled } from "./requestTracing/utils"; import { KeyFilter, LabelFilter, SettingSelector } from "./types"; -/** - * Key-value identifier, used as key of a Map. - * A primitive string type is actually used, because operator === is used to compare identifiers and we cannot override it. - */ -type KeyValueIdentifier = string; // key::label -function toKeyValueIdentifier(key: string, label: string | undefined): KeyValueIdentifier { - return `${key}::${label ?? ""}`; -} - export class AzureAppConfigurationImpl extends Map implements AzureAppConfiguration { #adapters: IKeyValueAdapter[] = []; /** @@ -42,7 +33,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure /** * Aka watched settings. */ - #sentinels: Map = new Map(); + #sentinels: ConfigurationSettingId[] = []; #refreshTimer: RefreshTimer; constructor( @@ -84,8 +75,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure if (setting.label?.includes("*") || setting.label?.includes(",")) { throw new Error("The characters '*' and ',' are not supported in label of watched settings."); } - const id = toKeyValueIdentifier(setting.key, setting.label); - this.#sentinels.set(id, setting); + this.#sentinels.push(setting); } this.#refreshTimer = new RefreshTimer(this.#refreshInterval); @@ -102,8 +92,8 @@ export class AzureAppConfigurationImpl extends Map implements Azure return !!this.#options?.refreshOptions?.enabled; } - async #loadSelectedKeyValues(): Promise> { - const loadedSettings = new Map(); + async #loadSelectedKeyValues(): Promise { + const loadedSettings: ConfigurationSetting[] = []; // validate selectors const selectors = getValidSelectors(this.#options?.selectors); @@ -124,8 +114,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure const settings = this.#client.listConfigurationSettings(listOptions); for await (const setting of settings) { - const id = toKeyValueIdentifier(setting.key, setting.label); - loadedSettings.set(id, setting); + loadedSettings.push(setting); } } return loadedSettings; @@ -134,13 +123,13 @@ export class AzureAppConfigurationImpl extends Map implements Azure /** * Update etag of watched settings from loaded data. If a watched setting is not covered by any selector, a request will be sent to retrieve it. */ - async #updateWatchedKeyValuesEtag(existingSettings: Map): Promise { + async #updateWatchedKeyValuesEtag(existingSettings: ConfigurationSetting[]): Promise { if (!this.#refreshEnabled) { return; } - for (const [id, sentinel] of this.#sentinels) { - const matchedSetting = existingSettings.get(id); + for (const sentinel of this.#sentinels) { + const matchedSetting = existingSettings.find(s => s.key === sentinel.key && s.label === sentinel.label); if (matchedSetting) { sentinel.etag = matchedSetting.etag; } else { @@ -163,7 +152,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure await this.#updateWatchedKeyValuesEtag(loadedSettings); // process key-values, watched settings have higher priority - for (const setting of loadedSettings.values()) { + for (const setting of loadedSettings) { if (setting.key) { const [key, value] = await this.#processKeyValues(setting); keyValues.push([key, value]); From 4bc37e197db33c6026335e476f4a43c6c63a582f Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Fri, 12 Jan 2024 10:02:54 +0800 Subject: [PATCH 41/43] remove unnecessary check, as key is non-null --- src/AzureAppConfigurationImpl.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 375a8930..dbc0e604 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -153,10 +153,8 @@ export class AzureAppConfigurationImpl extends Map implements Azure // process key-values, watched settings have higher priority for (const setting of loadedSettings) { - if (setting.key) { - const [key, value] = await this.#processKeyValues(setting); - keyValues.push([key, value]); - } + const [key, value] = await this.#processKeyValues(setting); + keyValues.push([key, value]); } this.clear(); // clear existing key-values in case of configuration setting deletion From 8acf87b7575b59329adc039b604d52fc6cca1a3d Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 18 Jan 2024 11:14:50 +0800 Subject: [PATCH 42/43] Do not refresh when watched setting remains not loaded --- src/AzureAppConfigurationImpl.ts | 12 +++++---- test/refresh.test.ts | 45 ++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index dbc0e604..08097526 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -194,10 +194,12 @@ export class AzureAppConfigurationImpl extends Map implements Azure }); if (response === undefined || response.statusCode === 200) { - // sentinel deleted / changed. - sentinel.etag = response?.etag;// update etag of the sentinel - needRefresh = true; - break; + // sentinel deleted / changed / created. + if (sentinel.etag !== response?.etag) { + sentinel.etag = response?.etag;// update etag of the sentinel + needRefresh = true; + break; + } } } if (needRefresh) { @@ -263,7 +265,7 @@ export class AzureAppConfigurationImpl extends Map implements Azure async #getConfigurationSettingWithTrace(configurationSettingId: ConfigurationSettingId, customOptions?: GetConfigurationSettingOptions): Promise { let response: GetConfigurationSettingResponse | undefined; try { - const options = {...customOptions ?? {}}; + const options = { ...customOptions ?? {} }; if (this.#requestTracingEnabled) { options.requestOptions = { customHeaders: { diff --git a/test/refresh.test.ts b/test/refresh.test.ts index 2fb05704..2378856f 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -274,4 +274,49 @@ describe("dynamic refresh", function () { expect(settings.get("app.settings.fontColor")).eq("red"); expect(settings.get("app.settings.fontSize")).undefined; }); + + it("should refresh when watched setting is added", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + refreshOptions: { + enabled: true, + refreshIntervalInMs: 2000, + watchedSettings: [ + { key: "app.settings.bgColor" } + ] + } + }); + expect(settings).not.undefined; + expect(settings.get("app.settings.fontColor")).eq("red"); + expect(settings.get("app.settings.fontSize")).eq("40"); + + // add setting 'app.settings.bgColor' + addSetting("app.settings.bgColor", "white"); + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + expect(settings.get("app.settings.bgColor")).eq("white"); + }); + + it("should not refresh when watched setting keeps not existing", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + refreshOptions: { + enabled: true, + refreshIntervalInMs: 2000, + watchedSettings: [ + { key: "app.settings.bgColor" } + ] + } + }); + expect(settings).not.undefined; + expect(settings.get("app.settings.fontColor")).eq("red"); + expect(settings.get("app.settings.fontSize")).eq("40"); + + // update an unwatched setting + updateSetting("app.settings.fontColor", "blue"); + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + // should not refresh + expect(settings.get("app.settings.fontColor")).eq("red"); + }); }); \ No newline at end of file From fd608623d8a1abec317dcc7f5242113473de2f42 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 18 Jan 2024 11:43:35 +0800 Subject: [PATCH 43/43] simplify nested if blocks --- src/AzureAppConfigurationImpl.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 08097526..0b808a5f 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -193,13 +193,12 @@ export class AzureAppConfigurationImpl extends Map implements Azure onlyIfChanged: true }); - if (response === undefined || response.statusCode === 200) { - // sentinel deleted / changed / created. - if (sentinel.etag !== response?.etag) { - sentinel.etag = response?.etag;// update etag of the sentinel - needRefresh = true; - break; - } + if (response?.statusCode === 200 // created or changed + || (response === undefined && sentinel.etag !== undefined) // deleted + ) { + sentinel.etag = response?.etag;// update etag of the sentinel + needRefresh = true; + break; } } if (needRefresh) {