From c035f89272eaa252f73a45b314aff26e0833acb9 Mon Sep 17 00:00:00 2001 From: Yan Zhang <2351748+Eskibear@users.noreply.github.com> Date: Tue, 16 Apr 2024 14:04:00 +0800 Subject: [PATCH 01/19] support feature flags --- src/AzureAppConfigurationImpl.ts | 110 ++++++++++++++++++-- src/AzureAppConfigurationOptions.ts | 7 ++ src/RefreshOptions.ts | 14 +++ src/featureManagement/FeatureFlagOptions.ts | 29 ++++++ src/featureManagement/constants.ts | 5 + test/featureFlag.test.ts | 77 ++++++++++++++ test/refresh.test.ts | 61 ++++++++++- test/utils/testHelper.ts | 18 ++++ 8 files changed, 309 insertions(+), 12 deletions(-) create mode 100644 src/featureManagement/FeatureFlagOptions.ts create mode 100644 src/featureManagement/constants.ts create mode 100644 test/featureFlag.test.ts diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 746fb959..c2a6188b 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, GetConfigurationSettingOptions, GetConfigurationSettingResponse, ListConfigurationSettingsOptions, isFeatureFlag } from "@azure/app-configuration"; +import { AppConfigurationClient, ConfigurationSetting, ConfigurationSettingId, FeatureFlagValue, GetConfigurationSettingOptions, GetConfigurationSettingResponse, ListConfigurationSettingsOptions, featureFlagPrefix, isFeatureFlag, parseFeatureFlag } from "@azure/app-configuration"; import { RestError } from "@azure/core-rest-pipeline"; import { AzureAppConfiguration, ConfigurationObjectConstructionOptions } from "./AzureAppConfiguration"; import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions"; @@ -14,6 +14,7 @@ import { RefreshTimer } from "./refresh/RefreshTimer"; import { CorrelationContextHeaderName } from "./requestTracing/constants"; import { createCorrelationContextHeader, requestTracingEnabled } from "./requestTracing/utils"; import { KeyFilter, LabelFilter, SettingSelector } from "./types"; +import { featureFlagsKeyName, featureManagementKeyName } from "./featureManagement/constants"; export class AzureAppConfigurationImpl implements AzureAppConfiguration { /** @@ -41,6 +42,10 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { #sentinels: ConfigurationSettingId[] = []; #refreshTimer: RefreshTimer; + // Feature flags + #featureFlagRefreshInterval: number = DefaultRefreshIntervalInMs; + #featureFlagRefreshTimer: RefreshTimer; + constructor( client: AppConfigurationClient, options: AzureAppConfigurationOptions | undefined @@ -85,13 +90,27 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { this.#refreshTimer = new RefreshTimer(this.#refreshInterval); } - // TODO: should add more adapters to process different type of values - // feature flag, others + // feature flag options + if (options?.featureFlagOptions?.enabled && options.featureFlagOptions.refresh?.enabled) { + const { refreshIntervalInMs } = options.featureFlagOptions.refresh; + + // custom refresh interval + if (refreshIntervalInMs !== undefined) { + if (refreshIntervalInMs < MinimumRefreshIntervalInMs) { + throw new Error(`The feature flag refresh interval cannot be less than ${MinimumRefreshIntervalInMs} milliseconds.`); + } else { + this.#featureFlagRefreshInterval = refreshIntervalInMs; + } + } + + this.#featureFlagRefreshTimer = new RefreshTimer(this.#featureFlagRefreshInterval); + } + this.#adapters.push(new AzureKeyVaultKeyValueAdapter(options?.keyVaultOptions)); this.#adapters.push(new JsonKeyValueAdapter()); } - // ReadonlyMap APIs + // #region ReadonlyMap APIs get(key: string): T | undefined { return this.#configMap.get(key); } @@ -123,11 +142,20 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { [Symbol.iterator](): IterableIterator<[string, any]> { return this.#configMap[Symbol.iterator](); } + // #endregion get #refreshEnabled(): boolean { return !!this.#options?.refreshOptions?.enabled; } + get #featureFlagEnabled(): boolean { + return !!this.#options?.featureFlagOptions?.enabled; + } + + get #featureFlagRefreshEnabled(): boolean { + return this.#featureFlagEnabled && !!this.#options?.featureFlagOptions?.refresh?.enabled; + } + async #loadSelectedKeyValues(): Promise { const loadedSettings: ConfigurationSetting[] = []; @@ -195,17 +223,58 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { keyValues.push([key, value]); } - this.#configMap.clear(); // clear existing key-values in case of configuration setting deletion + this.#clearLoadedKeyValues(); // clear existing key-values in case of configuration setting deletion for (const [k, v] of keyValues) { this.#configMap.set(k, v); } } + async #clearLoadedKeyValues() { + for(const key of this.#configMap.keys()) { + if (key !== featureManagementKeyName) { + this.#configMap.delete(key); + } + } + } + + async #loadFeatureFlags() { + const featureFlags: FeatureFlagValue[] = []; + const featureFlagSelectors = getValidSelectors(this.#options?.featureFlagOptions?.selectors); + for (const selector of featureFlagSelectors) { + const listOptions: ListConfigurationSettingsOptions = { + keyFilter: `${featureFlagPrefix}${selector.keyFilter}`, + labelFilter: selector.labelFilter + }; + if (this.#requestTracingEnabled) { + listOptions.requestOptions = { + customHeaders: { + [CorrelationContextHeaderName]: createCorrelationContextHeader(this.#options, this.#isInitialLoadCompleted) + } + } + } + + const settings = this.#client.listConfigurationSettings(listOptions); + + for await (const setting of settings) { + if (isFeatureFlag(setting)) { + const flag = parseFeatureFlag(setting); + featureFlags.push(flag.value) + } + } + } + + // feature_management is a reserved key, and feature_flags is an array of feature flags + this.#configMap.set(featureManagementKeyName, { [featureFlagsKeyName]: featureFlags }); + } + /** * Load the configuration store for the first time. */ async load() { await this.#loadSelectedAndWatchedKeyValues(); + if (this.#featureFlagEnabled) { + await this.#loadFeatureFlags(); + } // Mark all settings have loaded at startup. this.#isInitialLoadCompleted = true; } @@ -257,10 +326,19 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { * Refresh the configuration store. */ async refresh(): Promise { - if (!this.#refreshEnabled) { - throw new Error("Refresh is not enabled."); + if (!this.#refreshEnabled && !this.#featureFlagRefreshEnabled) { + throw new Error("Refresh is not enabled for key-values and feature flags."); } + if (this.#refreshEnabled) { + await this.#refreshKeyValues(); + } + if (this.#featureFlagRefreshEnabled) { + await this.#refreshFeatureFlags(); + } + } + + async #refreshKeyValues(): Promise { // if still within refresh interval/backoff, return if (!this.#refreshTimer.canRefresh()) { return Promise.resolve(); @@ -298,9 +376,25 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } } + async #refreshFeatureFlags(): Promise { + // if still within refresh interval/backoff, return + if (!this.#featureFlagRefreshTimer.canRefresh()) { + return Promise.resolve(); + } + + try { + await this.#loadFeatureFlags(); + this.#featureFlagRefreshTimer.reset(); + } catch (error) { + // if refresh failed, backoff + this.#featureFlagRefreshTimer.backoff(); + throw error; + } + } + onRefresh(listener: () => any, thisArg?: any): Disposable { if (!this.#refreshEnabled) { - throw new Error("Refresh is not enabled."); + throw new Error("Refresh is not enabled for key-values."); } const boundedListener = listener.bind(thisArg); diff --git a/src/AzureAppConfigurationOptions.ts b/src/AzureAppConfigurationOptions.ts index b4532804..e780fc89 100644 --- a/src/AzureAppConfigurationOptions.ts +++ b/src/AzureAppConfigurationOptions.ts @@ -5,6 +5,7 @@ import { AppConfigurationClientOptions } from "@azure/app-configuration"; import { KeyVaultOptions } from "./keyvault/KeyVaultOptions"; import { RefreshOptions } from "./RefreshOptions"; import { SettingSelector } from "./types"; +import { FeatureFlagOptions } from "./featureManagement/FeatureFlagOptions"; export const MaxRetries = 2; export const MaxRetryDelayInMs = 60000; @@ -36,8 +37,14 @@ export interface AzureAppConfigurationOptions { * Specifies options used to resolve Vey Vault references. */ keyVaultOptions?: KeyVaultOptions; + /** * Specifies options for dynamic refresh key-values. */ refreshOptions?: RefreshOptions; + + /** + * Specifies options used to configure feature flags. + */ + featureFlagOptions?: FeatureFlagOptions; } \ No newline at end of file diff --git a/src/RefreshOptions.ts b/src/RefreshOptions.ts index fd17183c..a5b76894 100644 --- a/src/RefreshOptions.ts +++ b/src/RefreshOptions.ts @@ -25,3 +25,17 @@ export interface RefreshOptions { */ watchedSettings?: WatchedSetting[]; } + +export interface FeatureFlagRefreshOptions { + /** + * Specifies whether the provider should automatically refresh when the configuration is changed. + */ + enabled: boolean; + + /** + * Specifies the minimum time that must elapse before checking the server for any new changes. + * Default value is 10 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; +} diff --git a/src/featureManagement/FeatureFlagOptions.ts b/src/featureManagement/FeatureFlagOptions.ts new file mode 100644 index 00000000..4b1b0e41 --- /dev/null +++ b/src/featureManagement/FeatureFlagOptions.ts @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { FeatureFlagRefreshOptions } from "../RefreshOptions"; +import { SettingSelector } from "../types"; + +/** + * Options used to configure feature flags. + */ +export interface FeatureFlagOptions { + /** + * Specifies whether feature flag support is enabled. + */ + enabled: boolean; + + /** + * Specifies the selectors used to filter feature flags. + * + * @remarks + * keyFilter of selector will be prefixed with "appconfig.featureflag/" when request is sent. + * If no selectors are specified then no feature flags will be retrieved. + */ + selectors?: SettingSelector[]; + + /** + * Specifies how feature flag refresh is configured. All selected feature flags will be watched for changes. + */ + refresh?: FeatureFlagRefreshOptions; +} \ No newline at end of file diff --git a/src/featureManagement/constants.ts b/src/featureManagement/constants.ts new file mode 100644 index 00000000..6b4e16d8 --- /dev/null +++ b/src/featureManagement/constants.ts @@ -0,0 +1,5 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +export const featureManagementKeyName = "feature_management"; +export const featureFlagsKeyName = "feature_flags"; \ No newline at end of file diff --git a/test/featureFlag.test.ts b/test/featureFlag.test.ts new file mode 100644 index 00000000..75220370 --- /dev/null +++ b/test/featureFlag.test.ts @@ -0,0 +1,77 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import * as chai from "chai"; +import * as chaiAsPromised from "chai-as-promised"; +import { load } from "./exportedApi"; +import { createMockedConnectionString, createMockedFeatureFlag, createMockedKeyValue, mockAppConfigurationClientListConfigurationSettings, restoreMocks } from "./utils/testHelper"; +chai.use(chaiAsPromised); +const expect = chai.expect; + +const mockedKVs = [{ + key: "app.settings.fontColor", + value: "red", +}].map(createMockedKeyValue).concat([ + createMockedFeatureFlag("Beta", true), + createMockedFeatureFlag("Alpha_1", true), + createMockedFeatureFlag("Alpha2", false), +]); + +describe("feature flags", function () { + this.timeout(10000); + + before(() => { + mockAppConfigurationClientListConfigurationSettings(mockedKVs); + }); + + after(() => { + restoreMocks(); + }) + it("should load feature flags if enabled", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + featureFlagOptions: { + enabled: true + } + }); + expect(settings).not.undefined; + expect(settings.get("feature_management")).not.undefined; + expect(settings.get("feature_management").feature_flags).not.undefined; + }); + + it("should not load feature flags if disabled", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + featureFlagOptions: { + enabled: false + } + }); + expect(settings).not.undefined; + expect(settings.get("feature_management")).undefined; + }); + + it("should not load feature flags if not specified", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString); + expect(settings).not.undefined; + expect(settings.get("feature_management")).undefined; + }); + + it("should load feature flags with custom selector", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + featureFlagOptions: { + enabled: true, + selectors: [{ + keyFilter: "Alpha*" + }] + } + }); + expect(settings).not.undefined; + expect(settings.get("feature_management")).not.undefined; + const featureFlags = settings.get("feature_management").feature_flags; + expect(featureFlags).not.undefined; + expect((featureFlags as []).length).equals(2); + }); + +}); diff --git a/test/refresh.test.ts b/test/refresh.test.ts index 2378856f..09099e02 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -6,7 +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, sleepInMs } from "./utils/testHelper"; +import { mockAppConfigurationClientListConfigurationSettings, mockAppConfigurationClientGetConfigurationSetting, restoreMocks, createMockedConnectionString, createMockedKeyValue, sleepInMs, createMockedFeatureFlag } from "./utils/testHelper"; import * as uuid from "uuid"; let mockedKVs: any[] = []; @@ -43,7 +43,7 @@ describe("dynamic refresh", function () { const connectionString = createMockedConnectionString(); const settings = await load(connectionString); const refreshCall = settings.refresh(); - return expect(refreshCall).eventually.rejectedWith("Refresh is not enabled."); + return expect(refreshCall).eventually.rejectedWith("Refresh is not enabled for key-values and feature flags."); }); it("should only allow non-empty list of watched settings when refresh is enabled", async () => { @@ -124,10 +124,10 @@ describe("dynamic refresh", function () { 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."); + expect(() => settings.onRefresh(() => { })).throws("Refresh is not enabled for key-values."); }); - it("should only udpate values after refreshInterval", async () => { + it("should only update values after refreshInterval", async () => { const connectionString = createMockedConnectionString(); const settings = await load(connectionString, { refreshOptions: { @@ -319,4 +319,57 @@ describe("dynamic refresh", function () { // should not refresh expect(settings.get("app.settings.fontColor")).eq("red"); }); + +}); + +describe("dynamic refresh feature flags", function () { + this.timeout(10000); + + beforeEach(() => { + mockedKVs = [ + createMockedFeatureFlag("Beta", { enabled: true }) + ]; + mockAppConfigurationClientListConfigurationSettings(mockedKVs); + mockAppConfigurationClientGetConfigurationSetting(mockedKVs) + }); + + afterEach(() => { + restoreMocks(); + }) + + it("should refresh feature flags when enabled", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + featureFlagOptions: { + enabled: true, + refresh: { + enabled: true, + refreshIntervalInMs: 2000 // 2 seconds for quick test. + } + } + }); + expect(settings).not.undefined; + expect(settings.get("feature_management")).not.undefined; + expect(settings.get("feature_management").feature_flags).not.undefined; + expect(settings.get("feature_management").feature_flags[0].id).eq("Beta"); + expect(settings.get("feature_management").feature_flags[0].enabled).eq(true); + + // change feature flag Beta to false + updateSetting(".appconfig.featureflag/Beta", JSON.stringify({ + "id": "Beta", + "description": "", + "enabled": false, + "conditions": { + "client_filters": [] + } + })); + + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + + expect(settings.get("feature_management").feature_flags[0].id).eq("Beta"); + expect(settings.get("feature_management").feature_flags[0].enabled).eq(false); + + }); + }); \ No newline at end of file diff --git a/test/utils/testHelper.ts b/test/utils/testHelper.ts index d4297823..e013b870 100644 --- a/test/utils/testHelper.ts +++ b/test/utils/testHelper.ts @@ -123,6 +123,23 @@ const createMockedKeyValue = (props: {[key: string]: any}): ConfigurationSetting isReadOnly: false }, props)); +const createMockedFeatureFlag = (name: string, flagProps?: any, props?: any) => (Object.assign({ + key: `.appconfig.featureflag/${name}`, + value: JSON.stringify(Object.assign({ + "id": name, + "description": "", + "enabled": true, + "conditions": { + "client_filters": [] + } + }, flagProps)), + contentType: "application/vnd.microsoft.appconfig.ff+json;charset=utf-8", + lastModified: new Date(), + tags: {}, + etag: uuid.v4(), + isReadOnly: false +}, props)); + export { sinon, mockAppConfigurationClientListConfigurationSettings, @@ -136,6 +153,7 @@ export { createMockedKeyVaultReference, createMockedJsonKeyValue, createMockedKeyValue, + createMockedFeatureFlag, sleepInMs } \ No newline at end of file From 6730571f3a8486103425c9b9604327b653cb36bd Mon Sep 17 00:00:00 2001 From: Yan Zhang <2351748+Eskibear@users.noreply.github.com> Date: Wed, 5 Jun 2024 11:31:32 +0800 Subject: [PATCH 02/19] Apply suggestions from code review Co-authored-by: Avani Gupta --- src/RefreshOptions.ts | 3 ++- src/featureManagement/FeatureFlagOptions.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/RefreshOptions.ts b/src/RefreshOptions.ts index a5b76894..7a9ee9b3 100644 --- a/src/RefreshOptions.ts +++ b/src/RefreshOptions.ts @@ -28,7 +28,8 @@ export interface RefreshOptions { export interface FeatureFlagRefreshOptions { /** - * Specifies whether the provider should automatically refresh when the configuration is changed. + * Specifies whether the provider should automatically refresh all feature flags if any feature flag changes. + */ enabled: boolean; diff --git a/src/featureManagement/FeatureFlagOptions.ts b/src/featureManagement/FeatureFlagOptions.ts index 4b1b0e41..de0ff896 100644 --- a/src/featureManagement/FeatureFlagOptions.ts +++ b/src/featureManagement/FeatureFlagOptions.ts @@ -9,7 +9,8 @@ import { SettingSelector } from "../types"; */ export interface FeatureFlagOptions { /** - * Specifies whether feature flag support is enabled. + * Specifies whether feature flags will be loaded from Azure App Configuration. + */ enabled: boolean; From e269596846f50227cafede8e6c29d88aac471d53 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 5 Jun 2024 14:44:42 +0800 Subject: [PATCH 03/19] fix default refresh interval in comments --- src/RefreshOptions.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/RefreshOptions.ts b/src/RefreshOptions.ts index 7a9ee9b3..c47cd373 100644 --- a/src/RefreshOptions.ts +++ b/src/RefreshOptions.ts @@ -29,13 +29,12 @@ export interface RefreshOptions { export interface FeatureFlagRefreshOptions { /** * Specifies whether the provider should automatically refresh all feature flags if any feature flag changes. - */ enabled: boolean; /** * Specifies the minimum time that must elapse before checking the server for any new changes. - * Default value is 10 seconds. Must be greater than 1 second. + * 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; From 708f7594209092ce53cf71fe8f635da12f951e1c Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Tue, 11 Jun 2024 16:45:16 +0800 Subject: [PATCH 04/19] update constants to SNAKE_CASE --- src/AzureAppConfigurationImpl.ts | 15 ++++++++------- src/featureManagement/constants.ts | 4 ++-- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index c3fc03f5..5c7c6597 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -14,7 +14,7 @@ import { RefreshTimer } from "./refresh/RefreshTimer"; import { CORRELATION_CONTEXT_HEADER_NAME } from "./requestTracing/constants"; import { createCorrelationContextHeader, requestTracingEnabled } from "./requestTracing/utils"; import { KeyFilter, LabelFilter, SettingSelector } from "./types"; -import { featureFlagsKeyName, featureManagementKeyName } from "./featureManagement/constants"; +import { FEATURE_FLAGS_KEY_NAME, FEATURE_MANAGEMENT_KEY_NAME } from "./featureManagement/constants"; export class AzureAppConfigurationImpl implements AzureAppConfiguration { /** @@ -43,7 +43,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { #refreshTimer: RefreshTimer; // Feature flags - #featureFlagRefreshInterval: number = DefaultRefreshIntervalInMs; + #featureFlagRefreshInterval: number = DEFAULT_REFRESH_INTERVAL_IN_MS; #featureFlagRefreshTimer: RefreshTimer; constructor( @@ -96,8 +96,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { // custom refresh interval if (refreshIntervalInMs !== undefined) { - if (refreshIntervalInMs < MinimumRefreshIntervalInMs) { - throw new Error(`The feature flag refresh interval cannot be less than ${MinimumRefreshIntervalInMs} milliseconds.`); + if (refreshIntervalInMs < MIN_REFRESH_INTERVAL_IN_MS) { + throw new Error(`The feature flag refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); } else { this.#featureFlagRefreshInterval = refreshIntervalInMs; } @@ -232,7 +232,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { async #clearLoadedKeyValues() { for(const key of this.#configMap.keys()) { - if (key !== featureManagementKeyName) { + if (key !== FEATURE_MANAGEMENT_KEY_NAME) { this.#configMap.delete(key); } } @@ -247,9 +247,10 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { labelFilter: selector.labelFilter }; if (this.#requestTracingEnabled) { + // TODO: use extracted api listOptions.requestOptions = { customHeaders: { - [CorrelationContextHeaderName]: createCorrelationContextHeader(this.#options, this.#isInitialLoadCompleted) + [CORRELATION_CONTEXT_HEADER_NAME]: createCorrelationContextHeader(this.#options, this.#isInitialLoadCompleted) } } } @@ -265,7 +266,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } // feature_management is a reserved key, and feature_flags is an array of feature flags - this.#configMap.set(featureManagementKeyName, { [featureFlagsKeyName]: featureFlags }); + this.#configMap.set(FEATURE_MANAGEMENT_KEY_NAME, { [FEATURE_FLAGS_KEY_NAME]: featureFlags }); } /** diff --git a/src/featureManagement/constants.ts b/src/featureManagement/constants.ts index 6b4e16d8..d8a6afc6 100644 --- a/src/featureManagement/constants.ts +++ b/src/featureManagement/constants.ts @@ -1,5 +1,5 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -export const featureManagementKeyName = "feature_management"; -export const featureFlagsKeyName = "feature_flags"; \ No newline at end of file +export const FEATURE_MANAGEMENT_KEY_NAME = "feature_management"; +export const FEATURE_FLAGS_KEY_NAME = "feature_flags"; \ No newline at end of file From 3cd5f599efbe7da1a5bb6f70806c5e851f787060 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Tue, 11 Jun 2024 16:48:51 +0800 Subject: [PATCH 05/19] use extracted list api --- src/AzureAppConfigurationImpl.ts | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index d542826d..8e973281 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -9,11 +9,11 @@ import { IKeyValueAdapter } from "./IKeyValueAdapter"; import { JsonKeyValueAdapter } from "./JsonKeyValueAdapter"; import { DEFAULT_REFRESH_INTERVAL_IN_MS, MIN_REFRESH_INTERVAL_IN_MS } from "./RefreshOptions"; import { Disposable } from "./common/disposable"; +import { FEATURE_FLAGS_KEY_NAME, FEATURE_MANAGEMENT_KEY_NAME } from "./featureManagement/constants"; import { AzureKeyVaultKeyValueAdapter } from "./keyvault/AzureKeyVaultKeyValueAdapter"; import { RefreshTimer } from "./refresh/RefreshTimer"; import { getConfigurationSettingWithTrace, listConfigurationSettingsWithTrace, requestTracingEnabled } from "./requestTracing/utils"; import { KeyFilter, LabelFilter, SettingSelector } from "./types"; -import { FEATURE_FLAGS_KEY_NAME, FEATURE_MANAGEMENT_KEY_NAME } from "./featureManagement/constants"; export class AzureAppConfigurationImpl implements AzureAppConfiguration { /** @@ -246,17 +246,16 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { keyFilter: `${featureFlagPrefix}${selector.keyFilter}`, labelFilter: selector.labelFilter }; - if (this.#requestTracingEnabled) { - // TODO: use extracted api - listOptions.requestOptions = { - customHeaders: { - [CORRELATION_CONTEXT_HEADER_NAME]: createCorrelationContextHeader(this.#options, this.#isInitialLoadCompleted) - } - } - } - - const settings = this.#client.listConfigurationSettings(listOptions); - + const requestTraceOptions = { + requestTracingEnabled: this.#requestTracingEnabled, + initialLoadCompleted: this.#isInitialLoadCompleted, + appConfigOptions: this.#options + }; + const settings = listConfigurationSettingsWithTrace( + requestTraceOptions, + this.#client, + listOptions + ); for await (const setting of settings) { if (isFeatureFlag(setting)) { const flag = parseFeatureFlag(setting); From 923c1245b0fbdfee5111d7d3b91e83237f304db0 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 12 Jun 2024 17:18:33 +0800 Subject: [PATCH 06/19] throw error if selectors not provided --- src/AzureAppConfigurationImpl.ts | 28 ++++++++++++++++++++-------- test/featureFlag.test.ts | 16 ++++++++++++++-- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 8e973281..db3fcd1c 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -159,7 +159,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { const loadedSettings: ConfigurationSetting[] = []; // validate selectors - const selectors = getValidSelectors(this.#options?.selectors); + const selectors = getValidKeyValueSelectors(this.#options?.selectors); for (const selector of selectors) { const listOptions: ListConfigurationSettingsOptions = { @@ -240,7 +240,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { async #loadFeatureFlags() { const featureFlags: FeatureFlagValue[] = []; - const featureFlagSelectors = getValidSelectors(this.#options?.featureFlagOptions?.selectors); + const featureFlagSelectors = getValidFeatureFlagSelectors(this.#options?.featureFlagOptions?.selectors); for (const selector of featureFlagSelectors) { const listOptions: ListConfigurationSettingsOptions = { keyFilter: `${featureFlagPrefix}${selector.keyFilter}`, @@ -465,12 +465,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } } -function getValidSelectors(selectors?: SettingSelector[]) { - if (!selectors || selectors.length === 0) { - // Default selector: key: *, label: \0 - return [{ keyFilter: KeyFilter.Any, labelFilter: LabelFilter.Null }]; - } - +function validateSelectors(selectors: SettingSelector[]) { // below code deduplicates selectors by keyFilter and labelFilter, the latter selector wins const uniqueSelectors: SettingSelector[] = []; for (const selector of selectors) { @@ -495,3 +490,20 @@ function getValidSelectors(selectors?: SettingSelector[]) { return selector; }); } + +function getValidKeyValueSelectors(selectors?: SettingSelector[]) { + if (!selectors || selectors.length === 0) { + // Default selector: key: *, label: \0 + return [{ keyFilter: KeyFilter.Any, labelFilter: LabelFilter.Null }]; + } + return validateSelectors(selectors); +} + +function getValidFeatureFlagSelectors(selectors?: SettingSelector[]) { + if (!selectors || selectors.length === 0) { + // selectors must be explicitly provided. + throw new Error("Feature flag selectors must be provided."); + } else { + return validateSelectors(selectors); + } +} \ No newline at end of file diff --git a/test/featureFlag.test.ts b/test/featureFlag.test.ts index 75220370..bae85123 100644 --- a/test/featureFlag.test.ts +++ b/test/featureFlag.test.ts @@ -31,7 +31,10 @@ describe("feature flags", function () { const connectionString = createMockedConnectionString(); const settings = await load(connectionString, { featureFlagOptions: { - enabled: true + enabled: true, + selectors: [{ + keyFilter: "*" + }] } }); expect(settings).not.undefined; @@ -50,13 +53,22 @@ describe("feature flags", function () { expect(settings.get("feature_management")).undefined; }); - it("should not load feature flags if not specified", async () => { + it("should not load feature flags if featureFlagOptions not specified", async () => { const connectionString = createMockedConnectionString(); const settings = await load(connectionString); expect(settings).not.undefined; expect(settings.get("feature_management")).undefined; }); + it("should throw error if selectors not specified", async () => { + const connectionString = createMockedConnectionString(); + return expect(load(connectionString, { + featureFlagOptions: { + enabled: true + } + })).eventually.rejectedWith("Feature flag selectors must be provided."); + }); + it("should load feature flags with custom selector", async () => { const connectionString = createMockedConnectionString(); const settings = await load(connectionString, { From 8e2de54434b60e48636ab08d8a718461d97cd54b Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Fri, 14 Jun 2024 12:57:33 +0800 Subject: [PATCH 07/19] parse feature flag directly --- src/AzureAppConfigurationImpl.ts | 8 ++-- test/featureFlag.test.ts | 71 ++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index db3fcd1c..746002bb 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, FeatureFlagValue, GetConfigurationSettingOptions, GetConfigurationSettingResponse, ListConfigurationSettingsOptions, featureFlagPrefix, isFeatureFlag, parseFeatureFlag } from "@azure/app-configuration"; +import { AppConfigurationClient, ConfigurationSetting, ConfigurationSettingId, GetConfigurationSettingOptions, GetConfigurationSettingResponse, ListConfigurationSettingsOptions, featureFlagPrefix, isFeatureFlag } from "@azure/app-configuration"; import { RestError } from "@azure/core-rest-pipeline"; import { AzureAppConfiguration, ConfigurationObjectConstructionOptions } from "./AzureAppConfiguration"; import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions"; @@ -239,7 +239,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } async #loadFeatureFlags() { - const featureFlags: FeatureFlagValue[] = []; + const featureFlags: unknown[] = []; const featureFlagSelectors = getValidFeatureFlagSelectors(this.#options?.featureFlagOptions?.selectors); for (const selector of featureFlagSelectors) { const listOptions: ListConfigurationSettingsOptions = { @@ -258,8 +258,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { ); for await (const setting of settings) { if (isFeatureFlag(setting)) { - const flag = parseFeatureFlag(setting); - featureFlags.push(flag.value) + const flag = JSON.parse(setting.value); + featureFlags.push(flag) } } } diff --git a/test/featureFlag.test.ts b/test/featureFlag.test.ts index bae85123..426443fe 100644 --- a/test/featureFlag.test.ts +++ b/test/featureFlag.test.ts @@ -8,9 +8,48 @@ import { createMockedConnectionString, createMockedFeatureFlag, createMockedKeyV chai.use(chaiAsPromised); const expect = chai.expect; +const sampleVariantValue = JSON.stringify({ + "id": "variant", + "description": "", + "enabled": true, + "variants": [ + { + "name": "Off", + "configuration_value": false + }, + { + "name": "On", + "configuration_value": true + } + ], + "allocation": { + "percentile": [ + { + "variant": "Off", + "from": 0, + "to": 40 + }, + { + "variant": "On", + "from": 49, + "to": 100 + } + ], + "default_when_enabled": "Off", + "default_when_disabled": "Off" + }, + "telemetry": { + "enabled": false + } +}); + const mockedKVs = [{ key: "app.settings.fontColor", value: "red", +}, { + key: ".appconfig.featureflag/variant", + value: sampleVariantValue, + contentType: "application/vnd.microsoft.appconfig.ff+json;charset=utf-8", }].map(createMockedKeyValue).concat([ createMockedFeatureFlag("Beta", true), createMockedFeatureFlag("Alpha_1", true), @@ -86,4 +125,36 @@ describe("feature flags", function () { expect((featureFlags as []).length).equals(2); }); + it("should parse variant", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + featureFlagOptions: { + enabled: true, + selectors: [{ + keyFilter: "variant" + }] + } + }); + expect(settings).not.undefined; + expect(settings.get("feature_management")).not.undefined; + const featureFlags = settings.get("feature_management").feature_flags; + expect(featureFlags).not.undefined; + expect((featureFlags as []).length).equals(1); + const variant = featureFlags[0]; + expect(variant).not.undefined; + expect(variant.id).equals("variant"); + expect(variant.variants).not.undefined; + expect(variant.variants.length).equals(2); + expect(variant.variants[0].configuration_value).equals(false); + expect(variant.variants[1].configuration_value).equals(true); + expect(variant.allocation).not.undefined; + expect(variant.allocation.percentile).not.undefined; + expect(variant.allocation.percentile.length).equals(2); + expect(variant.allocation.percentile[0].variant).equals("Off"); + expect(variant.allocation.percentile[1].variant).equals("On"); + expect(variant.allocation.default_when_enabled).equals("Off"); + expect(variant.allocation.default_when_disabled).equals("Off"); + expect(variant.telemetry).not.undefined; + }); + }); From 11826380080c5e574a0ac98bea3efd1fa7aef766 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 19 Jun 2024 10:25:56 +0800 Subject: [PATCH 08/19] refactor: rename validateSelectors to getValidSelectors --- src/AzureAppConfigurationImpl.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 746002bb..521110ce 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -465,7 +465,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } } -function validateSelectors(selectors: SettingSelector[]) { +function getValidSelectors(selectors: SettingSelector[]): SettingSelector[] { // below code deduplicates selectors by keyFilter and labelFilter, the latter selector wins const uniqueSelectors: SettingSelector[] = []; for (const selector of selectors) { @@ -491,19 +491,19 @@ function validateSelectors(selectors: SettingSelector[]) { }); } -function getValidKeyValueSelectors(selectors?: SettingSelector[]) { +function getValidKeyValueSelectors(selectors?: SettingSelector[]): SettingSelector[] { if (!selectors || selectors.length === 0) { // Default selector: key: *, label: \0 return [{ keyFilter: KeyFilter.Any, labelFilter: LabelFilter.Null }]; } - return validateSelectors(selectors); + return getValidSelectors(selectors); } -function getValidFeatureFlagSelectors(selectors?: SettingSelector[]) { +function getValidFeatureFlagSelectors(selectors?: SettingSelector[]): SettingSelector[] { if (!selectors || selectors.length === 0) { // selectors must be explicitly provided. throw new Error("Feature flag selectors must be provided."); } else { - return validateSelectors(selectors); + return getValidSelectors(selectors); } } \ No newline at end of file From 99f2c297ec5e031342bac1f9566347fa7f10f153 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 19 Jun 2024 10:53:24 +0800 Subject: [PATCH 09/19] refresh APIs apply for both kv and ff --- src/AzureAppConfiguration.ts | 2 +- src/AzureAppConfigurationImpl.ts | 51 +++++++++++++++++++++++++------- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/src/AzureAppConfiguration.ts b/src/AzureAppConfiguration.ts index 1f38d29b..7d8120d3 100644 --- a/src/AzureAppConfiguration.ts +++ b/src/AzureAppConfiguration.ts @@ -10,7 +10,7 @@ export type AzureAppConfiguration = { refresh(): Promise; /** - * API to register callback listeners, which will be called only when a refresh operation successfully updates key-values. + * API to register callback listeners, which will be called only when a refresh operation successfully updates key-values or feature flags. * * @param listener - Callback function to be registered. * @param thisArg - Optional. Value to use as `this` when executing callback. diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 521110ce..28b7a737 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -331,18 +331,42 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { throw new Error("Refresh is not enabled for key-values and feature flags."); } + const refreshTasks: Promise[] = []; if (this.#refreshEnabled) { - await this.#refreshKeyValues(); + refreshTasks.push(this.#refreshKeyValues()); } if (this.#featureFlagRefreshEnabled) { - await this.#refreshFeatureFlags(); + refreshTasks.push(this.#refreshFeatureFlags()); + } + + // wait until all tasks are either resolved or rejected + const results = await Promise.allSettled(refreshTasks); + + // check if any refresh task failed + for (const result of results) { + if (result.status === "rejected") { + throw result.reason; + } + } + + // check if any refresh task succeeded + const anyRefreshed = results.some(result => result.status === "fulfilled" && result.value === true); + if (anyRefreshed) { + // successfully refreshed, run callbacks in async + for (const listener of this.#onRefreshListeners) { + listener(); + } } } - async #refreshKeyValues(): Promise { + /** + * Refresh key-values. + * @returns true if key-values are refreshed, false otherwise. + */ + async #refreshKeyValues(): Promise { // if still within refresh interval/backoff, return if (!this.#refreshTimer.canRefresh()) { - return Promise.resolve(); + return Promise.resolve(false); } // try refresh if any of watched settings is changed. @@ -360,6 +384,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { break; } } + if (needRefresh) { try { await this.#loadSelectedAndWatchedKeyValues(); @@ -369,21 +394,24 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { this.#refreshTimer.backoff(); throw error; } - - // successfully refreshed, run callbacks in async - for (const listener of this.#onRefreshListeners) { - listener(); - } + return Promise.resolve(true); } + + return Promise.resolve(false); } - async #refreshFeatureFlags(): Promise { + /** + * Refresh feature flags. + * @returns true if feature flags are refreshed, false otherwise. + */ + async #refreshFeatureFlags(): Promise { // if still within refresh interval/backoff, return if (!this.#featureFlagRefreshTimer.canRefresh()) { - return Promise.resolve(); + return Promise.resolve(false); } try { + // TODO: instead of refreshing all feature flags, only refresh the changed ones with etag await this.#loadFeatureFlags(); this.#featureFlagRefreshTimer.reset(); } catch (error) { @@ -391,6 +419,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { this.#featureFlagRefreshTimer.backoff(); throw error; } + return Promise.resolve(true); } onRefresh(listener: () => any, thisArg?: any): Disposable { From dc4f553f4c29a9f740c07909a3a6ac47bdfe6e43 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Thu, 20 Jun 2024 14:50:56 +0800 Subject: [PATCH 10/19] dedup loaded feature flags among selectors --- src/AzureAppConfigurationImpl.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 28b7a737..1435417f 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -239,7 +239,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } async #loadFeatureFlags() { - const featureFlags: unknown[] = []; + // Temporary map to store feature flags, key is the key of the setting, value is the raw value of the setting + const featureFlagsMap = new Map(); const featureFlagSelectors = getValidFeatureFlagSelectors(this.#options?.featureFlagOptions?.selectors); for (const selector of featureFlagSelectors) { const listOptions: ListConfigurationSettingsOptions = { @@ -258,12 +259,14 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { ); for await (const setting of settings) { if (isFeatureFlag(setting)) { - const flag = JSON.parse(setting.value); - featureFlags.push(flag) + featureFlagsMap.set(setting.key, setting.value); } } } + // parse feature flags + const featureFlags = Array.from(featureFlagsMap.values()).map(rawFlag => JSON.parse(rawFlag)); + // feature_management is a reserved key, and feature_flags is an array of feature flags this.#configMap.set(FEATURE_MANAGEMENT_KEY_NAME, { [FEATURE_FLAGS_KEY_NAME]: featureFlags }); } From 6411a30e371de0724616756c419923954170a140 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Mon, 24 Jun 2024 16:56:40 +0800 Subject: [PATCH 11/19] extract requestTraceOptions as private member --- src/AzureAppConfigurationImpl.ts | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 1435417f..aa8dc43d 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -155,6 +155,14 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { return this.#featureFlagEnabled && !!this.#options?.featureFlagOptions?.refresh?.enabled; } + get #requestTraceOptions() { + return { + requestTracingEnabled: this.#requestTracingEnabled, + initialLoadCompleted: this.#isInitialLoadCompleted, + appConfigOptions: this.#options + }; + } + async #loadSelectedKeyValues(): Promise { const loadedSettings: ConfigurationSetting[] = []; @@ -167,13 +175,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { labelFilter: selector.labelFilter }; - const requestTraceOptions = { - requestTracingEnabled: this.#requestTracingEnabled, - initialLoadCompleted: this.#isInitialLoadCompleted, - appConfigOptions: this.#options - }; const settings = listConfigurationSettingsWithTrace( - requestTraceOptions, + this.#requestTraceOptions, this.#client, listOptions ); @@ -214,7 +217,6 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { async #loadSelectedAndWatchedKeyValues() { const keyValues: [key: string, value: unknown][] = []; - const loadedSettings = await this.#loadSelectedKeyValues(); await this.#updateWatchedKeyValuesEtag(loadedSettings); @@ -247,13 +249,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { keyFilter: `${featureFlagPrefix}${selector.keyFilter}`, labelFilter: selector.labelFilter }; - const requestTraceOptions = { - requestTracingEnabled: this.#requestTracingEnabled, - initialLoadCompleted: this.#isInitialLoadCompleted, - appConfigOptions: this.#options - }; const settings = listConfigurationSettingsWithTrace( - requestTraceOptions, + this.#requestTraceOptions, this.#client, listOptions ); @@ -474,18 +471,12 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { async #getConfigurationSetting(configurationSettingId: ConfigurationSettingId, customOptions?: GetConfigurationSettingOptions): Promise { let response: GetConfigurationSettingResponse | undefined; try { - const requestTraceOptions = { - requestTracingEnabled: this.#requestTracingEnabled, - initialLoadCompleted: this.#isInitialLoadCompleted, - appConfigOptions: this.#options - }; response = await getConfigurationSettingWithTrace( - requestTraceOptions, + this.#requestTraceOptions, this.#client, configurationSettingId, customOptions ); - } catch (error) { if (error instanceof RestError && error.statusCode === 404) { response = undefined; From 4ec140a0a4aff9c459dae71c7a0935fc0d8dd695 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Tue, 2 Jul 2024 16:59:39 +0800 Subject: [PATCH 12/19] list feature flags with pageEtags --- src/AzureAppConfigurationImpl.ts | 104 ++++++++++++++++++++++--------- test/utils/testHelper.ts | 1 + 2 files changed, 77 insertions(+), 28 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index aa8dc43d..f7274b4f 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -15,6 +15,13 @@ import { RefreshTimer } from "./refresh/RefreshTimer"; import { getConfigurationSettingWithTrace, listConfigurationSettingsWithTrace, requestTracingEnabled } from "./requestTracing/utils"; import { KeyFilter, LabelFilter, SettingSelector } from "./types"; +type PagedSettingSelector = SettingSelector & { + /** + * Key: page eTag, Value: feature flag configurations + */ + pageEtags?: string[]; +}; + export class AzureAppConfigurationImpl implements AzureAppConfiguration { /** * Hosting key-value pairs in the configuration store. @@ -45,6 +52,9 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { #featureFlagRefreshInterval: number = DEFAULT_REFRESH_INTERVAL_IN_MS; #featureFlagRefreshTimer: RefreshTimer; + // selectors + #featureFlagSelectors: PagedSettingSelector[] = []; + constructor( client: AppConfigurationClient, options: AzureAppConfigurationOptions | undefined @@ -90,19 +100,23 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } // feature flag options - if (options?.featureFlagOptions?.enabled && options.featureFlagOptions.refresh?.enabled) { - const { refreshIntervalInMs } = options.featureFlagOptions.refresh; - - // custom refresh interval - if (refreshIntervalInMs !== undefined) { - if (refreshIntervalInMs < MIN_REFRESH_INTERVAL_IN_MS) { - throw new Error(`The feature flag refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); - } else { - this.#featureFlagRefreshInterval = refreshIntervalInMs; + if (options?.featureFlagOptions?.enabled) { + // validate feature flag selectors + this.#featureFlagSelectors = getValidFeatureFlagSelectors(options.featureFlagOptions.selectors); + + if (options.featureFlagOptions.refresh?.enabled) { + const { refreshIntervalInMs } = options.featureFlagOptions.refresh; + // custom refresh interval + if (refreshIntervalInMs !== undefined) { + if (refreshIntervalInMs < MIN_REFRESH_INTERVAL_IN_MS) { + throw new Error(`The feature flag refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); + } else { + this.#featureFlagRefreshInterval = refreshIntervalInMs; + } } - } - this.#featureFlagRefreshTimer = new RefreshTimer(this.#featureFlagRefreshInterval); + this.#featureFlagRefreshTimer = new RefreshTimer(this.#featureFlagRefreshInterval); + } } this.#adapters.push(new AzureKeyVaultKeyValueAdapter(options?.keyVaultOptions)); @@ -233,7 +247,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } async #clearLoadedKeyValues() { - for(const key of this.#configMap.keys()) { + for (const key of this.#configMap.keys()) { if (key !== FEATURE_MANAGEMENT_KEY_NAME) { this.#configMap.delete(key); } @@ -243,22 +257,31 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { async #loadFeatureFlags() { // Temporary map to store feature flags, key is the key of the setting, value is the raw value of the setting const featureFlagsMap = new Map(); - const featureFlagSelectors = getValidFeatureFlagSelectors(this.#options?.featureFlagOptions?.selectors); - for (const selector of featureFlagSelectors) { + for (const selector of this.#featureFlagSelectors) { const listOptions: ListConfigurationSettingsOptions = { keyFilter: `${featureFlagPrefix}${selector.keyFilter}`, labelFilter: selector.labelFilter }; - const settings = listConfigurationSettingsWithTrace( + + const pageEtags: string[] = []; + const pageIterator = listConfigurationSettingsWithTrace( this.#requestTraceOptions, this.#client, listOptions - ); - for await (const setting of settings) { - if (isFeatureFlag(setting)) { - featureFlagsMap.set(setting.key, setting.value); + ).byPage(); + for await (const page of pageIterator) { + if (page._response.status === 200) { + if (page.etag) { + pageEtags.push(page.etag); + } + } + for (const setting of page.items) { + if (isFeatureFlag(setting)) { + featureFlagsMap.set(setting.key, setting.value); + } } } + selector.pageEtags = pageEtags; } // parse feature flags @@ -410,16 +433,41 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { return Promise.resolve(false); } - try { - // TODO: instead of refreshing all feature flags, only refresh the changed ones with etag - await this.#loadFeatureFlags(); - this.#featureFlagRefreshTimer.reset(); - } catch (error) { - // if refresh failed, backoff - this.#featureFlagRefreshTimer.backoff(); - throw error; + // check if any feature flag is changed + let needRefresh = false; + for (const selector of this.#featureFlagSelectors) { + const listOptions: ListConfigurationSettingsOptions = { + keyFilter: `${featureFlagPrefix}${selector.keyFilter}`, + labelFilter: selector.labelFilter, + pageEtags: selector.pageEtags + }; + const pageIterator = listConfigurationSettingsWithTrace( + this.#requestTraceOptions, + this.#client, + listOptions + ).byPage(); + for await (const page of pageIterator) { + if (page._response.status === 200) { // created or changed + needRefresh = true; + break; + } + // TODO: handle page deleted? + } } - return Promise.resolve(true); + + if (needRefresh) { + try { + await this.#loadFeatureFlags(); + this.#featureFlagRefreshTimer.reset(); + } catch (error) { + // if refresh failed, backoff + this.#featureFlagRefreshTimer.backoff(); + throw error; + } + return Promise.resolve(true); + } + + return Promise.resolve(false); } onRefresh(listener: () => any, thisArg?: any): Disposable { diff --git a/test/utils/testHelper.ts b/test/utils/testHelper.ts index e013b870..45191a7b 100644 --- a/test/utils/testHelper.ts +++ b/test/utils/testHelper.ts @@ -14,6 +14,7 @@ const TEST_CLIENT_ID = "00000000-0000-0000-0000-000000000000"; const TEST_TENANT_ID = "00000000-0000-0000-0000-000000000000"; const TEST_CLIENT_SECRET = "0000000000000000000000000000000000000000"; +// TODO: mock client.listConfigurationSettings().byPage() to test pagination function mockAppConfigurationClientListConfigurationSettings(kvList: ConfigurationSetting[]) { function* testKvSetGenerator(kvs: any[]) { yield* kvs; From 453dad3a4f8ff25cb7e5bf8192558863508d219d Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 3 Jul 2024 15:07:36 +0800 Subject: [PATCH 13/19] update mocked client to support byPage iterator --- test/refresh.test.ts | 3 ++ test/utils/testHelper.ts | 86 +++++++++++++++++++++++++++++----------- 2 files changed, 66 insertions(+), 23 deletions(-) diff --git a/test/refresh.test.ts b/test/refresh.test.ts index 09099e02..8e3d8a4f 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -342,6 +342,9 @@ describe("dynamic refresh feature flags", function () { const settings = await load(connectionString, { featureFlagOptions: { enabled: true, + selectors: [{ + keyFilter: "Beta" + }], refresh: { enabled: true, refreshIntervalInMs: 2000 // 2 seconds for quick test. diff --git a/test/utils/testHelper.ts b/test/utils/testHelper.ts index 45191a7b..6fa96af9 100644 --- a/test/utils/testHelper.ts +++ b/test/utils/testHelper.ts @@ -14,30 +14,70 @@ const TEST_CLIENT_ID = "00000000-0000-0000-0000-000000000000"; const TEST_TENANT_ID = "00000000-0000-0000-0000-000000000000"; const TEST_CLIENT_SECRET = "0000000000000000000000000000000000000000"; -// TODO: mock client.listConfigurationSettings().byPage() to test pagination -function mockAppConfigurationClientListConfigurationSettings(kvList: ConfigurationSetting[]) { - function* testKvSetGenerator(kvs: any[]) { - yield* kvs; - } +function _filterKVs(unfilteredKvs: ConfigurationSetting[], listOptions: any) { + const keyFilter = listOptions?.keyFilter ?? "*"; + const labelFilter = listOptions?.labelFilter ?? "*"; + return unfilteredKvs.filter(kv => { + 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; + }) +} + +/** + * Mocks the listConfigurationSettings method of AppConfigurationClient to return the provided pages of ConfigurationSetting. + * E.g. + * - mockAppConfigurationClientListConfigurationSettings([item1, item2, item3]) // single page + * - mockAppConfigurationClientListConfigurationSettings([item1, item2], [item3], [item4]) // multiple pages + * + * @param pages List of pages, each page is a list of ConfigurationSetting + */ +function mockAppConfigurationClientListConfigurationSettings(...pages: ConfigurationSetting[][]) { + sinon.stub(AppConfigurationClient.prototype, "listConfigurationSettings").callsFake((listOptions) => { - const keyFilter = listOptions?.keyFilter ?? "*"; - const labelFilter = listOptions?.labelFilter ?? "*"; - const kvs = kvList.filter(kv => { - 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; + let kvs = _filterKVs(pages.flat(), listOptions); + + const mockIterator: AsyncIterableIterator & { byPage(): AsyncIterableIterator } = { + [Symbol.asyncIterator](): AsyncIterableIterator { + kvs = _filterKVs(pages.flat(), listOptions); + return this; + }, + next() { + const value = kvs.shift(); + return Promise.resolve({ done: !value, value }); + }, + byPage(): AsyncIterableIterator { + let remainingPages; + return { + [Symbol.asyncIterator](): AsyncIterableIterator { + remainingPages = [ ...pages ]; + return this; + }, + next() { + const pageItems = remainingPages.shift(); + return Promise.resolve({ + done: pageItems === undefined, + value: { + items: _filterKVs(pageItems ?? [], listOptions), + eTag: "etag", + _response: { status: 200 } // TODO: 304 if etag matches + } + }); + } + } } - return keyMatched && labelMatched; - }) - return testKvSetGenerator(kvs) as any; + }; + + return mockIterator as any; }); } @@ -114,7 +154,7 @@ const createMockedJsonKeyValue = (key: string, value: any): ConfigurationSetting isReadOnly: false }); -const createMockedKeyValue = (props: {[key: string]: any}): ConfigurationSetting => (Object.assign({ +const createMockedKeyValue = (props: { [key: string]: any }): ConfigurationSetting => (Object.assign({ value: "TestValue", key: "TestKey", contentType: "", From e4eedc9fd75fe1b1ad2d08fdd022c73b36612a3c Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 3 Jul 2024 15:32:57 +0800 Subject: [PATCH 14/19] handle page deletion --- src/AzureAppConfigurationImpl.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index f7274b4f..7c2321ae 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -446,12 +446,23 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { this.#client, listOptions ).byPage(); + + let pageCount = 0; for await (const page of pageIterator) { if (page._response.status === 200) { // created or changed needRefresh = true; break; } - // TODO: handle page deleted? + // unchanged, check next page + pageCount++; + } + + if (pageCount !== selector.pageEtags?.length) { + needRefresh = true; // page count changed indicating feature flags are added or deleted + } + + if (needRefresh) { + break; // short-circuit if result from any of the selectors is changed } } From 87316f3cb1437c339fecc89282402172df3469d8 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Fri, 19 Jul 2024 11:03:04 +0800 Subject: [PATCH 15/19] upgrade @azure/app-configuration to 1.6.1 --- package-lock.json | 14 +++++++------- package.json | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index deb000fc..bcc8ae93 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,7 @@ "version": "1.0.0-preview", "license": "MIT", "dependencies": { - "@azure/app-configuration": "^1.6.0", + "@azure/app-configuration": "^1.6.1", "@azure/identity": "^4.2.1", "@azure/keyvault-secrets": "^4.7.0" }, @@ -57,9 +57,9 @@ } }, "node_modules/@azure/app-configuration": { - "version": "1.6.0", - "resolved": "https://registry.npmjs.org/@azure/app-configuration/-/app-configuration-1.6.0.tgz", - "integrity": "sha512-5Ae4SB0g4VbTnF7B+bwlkRLesRIYcaeg6e2Qxf0RlOEIetIgfAZiX6S5e7hD83X5RkwiPmzDm8rJm6HDpnVcvQ==", + "version": "1.6.1", + "resolved": "https://registry.npmjs.org/@azure/app-configuration/-/app-configuration-1.6.1.tgz", + "integrity": "sha512-pk8zyG/8Nc6VN7uDA9QY19UFhTXneUbnB+5IcW9uuPyVDXU17TcXBI4xY1ZBm7hmhn0yh3CeZK4kOxa/tjsMqQ==", "dependencies": { "@azure/abort-controller": "^1.0.0", "@azure/core-auth": "^1.3.0", @@ -3608,9 +3608,9 @@ } }, "@azure/app-configuration": { - "version": "1.6.0", - "resolved": "https://registry.npmjs.org/@azure/app-configuration/-/app-configuration-1.6.0.tgz", - "integrity": "sha512-5Ae4SB0g4VbTnF7B+bwlkRLesRIYcaeg6e2Qxf0RlOEIetIgfAZiX6S5e7hD83X5RkwiPmzDm8rJm6HDpnVcvQ==", + "version": "1.6.1", + "resolved": "https://registry.npmjs.org/@azure/app-configuration/-/app-configuration-1.6.1.tgz", + "integrity": "sha512-pk8zyG/8Nc6VN7uDA9QY19UFhTXneUbnB+5IcW9uuPyVDXU17TcXBI4xY1ZBm7hmhn0yh3CeZK4kOxa/tjsMqQ==", "requires": { "@azure/abort-controller": "^1.0.0", "@azure/core-auth": "^1.3.0", diff --git a/package.json b/package.json index 25cdef61..199b16d4 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,7 @@ "uuid": "^9.0.1" }, "dependencies": { - "@azure/app-configuration": "^1.6.0", + "@azure/app-configuration": "^1.6.1", "@azure/identity": "^4.2.1", "@azure/keyvault-secrets": "^4.7.0" } From 0c8a4f28d0d676352fb024048373c49a302cb8ab Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Fri, 19 Jul 2024 16:00:33 +0800 Subject: [PATCH 16/19] add tests for pageEtag based refresh --- src/AzureAppConfigurationImpl.ts | 6 +-- test/featureFlag.test.ts | 70 ++++++++++++++++---------------- test/refresh.test.ts | 62 ++++++++++++++++++++++++---- test/utils/testHelper.ts | 33 ++++++++++----- 4 files changed, 116 insertions(+), 55 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 7c2321ae..b51d7173 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -351,7 +351,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { */ async refresh(): Promise { if (!this.#refreshEnabled && !this.#featureFlagRefreshEnabled) { - throw new Error("Refresh is not enabled for key-values and feature flags."); + throw new Error("Refresh is not enabled for key-values or feature flags."); } const refreshTasks: Promise[] = []; @@ -482,8 +482,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } onRefresh(listener: () => any, thisArg?: any): Disposable { - if (!this.#refreshEnabled) { - throw new Error("Refresh is not enabled for key-values."); + if (!this.#refreshEnabled && !this.#featureFlagRefreshEnabled) { + throw new Error("Refresh is not enabled for key-values or feature flags."); } const boundedListener = listener.bind(thisArg); diff --git a/test/featureFlag.test.ts b/test/featureFlag.test.ts index 426443fe..22db36af 100644 --- a/test/featureFlag.test.ts +++ b/test/featureFlag.test.ts @@ -9,38 +9,38 @@ chai.use(chaiAsPromised); const expect = chai.expect; const sampleVariantValue = JSON.stringify({ - "id": "variant", - "description": "", - "enabled": true, - "variants": [ - { - "name": "Off", - "configuration_value": false - }, - { - "name": "On", - "configuration_value": true - } - ], - "allocation": { - "percentile": [ - { - "variant": "Off", - "from": 0, - "to": 40 - }, - { - "variant": "On", - "from": 49, - "to": 100 - } - ], - "default_when_enabled": "Off", - "default_when_disabled": "Off" - }, - "telemetry": { - "enabled": false - } + "id": "variant", + "description": "", + "enabled": true, + "variants": [ + { + "name": "Off", + "configuration_value": false + }, + { + "name": "On", + "configuration_value": true + } + ], + "allocation": { + "percentile": [ + { + "variant": "Off", + "from": 0, + "to": 40 + }, + { + "variant": "On", + "from": 49, + "to": 100 + } + ], + "default_when_enabled": "Off", + "default_when_disabled": "Off" + }, + "telemetry": { + "enabled": false + } }); const mockedKVs = [{ @@ -51,9 +51,9 @@ const mockedKVs = [{ value: sampleVariantValue, contentType: "application/vnd.microsoft.appconfig.ff+json;charset=utf-8", }].map(createMockedKeyValue).concat([ - createMockedFeatureFlag("Beta", true), - createMockedFeatureFlag("Alpha_1", true), - createMockedFeatureFlag("Alpha2", false), + createMockedFeatureFlag("Beta", { enabled: true }), + createMockedFeatureFlag("Alpha_1", { enabled: true }), + createMockedFeatureFlag("Alpha_2", { enabled: false }), ]); describe("feature flags", function () { diff --git a/test/refresh.test.ts b/test/refresh.test.ts index 8e3d8a4f..b756038a 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -18,6 +18,7 @@ function updateSetting(key: string, value: any) { setting.etag = uuid.v4(); } } + function addSetting(key: string, value: any) { mockedKVs.push(createMockedKeyValue({ key, value })); } @@ -43,7 +44,7 @@ describe("dynamic refresh", function () { const connectionString = createMockedConnectionString(); const settings = await load(connectionString); const refreshCall = settings.refresh(); - return expect(refreshCall).eventually.rejectedWith("Refresh is not enabled for key-values and feature flags."); + return expect(refreshCall).eventually.rejectedWith("Refresh is not enabled for key-values or feature flags."); }); it("should only allow non-empty list of watched settings when refresh is enabled", async () => { @@ -124,7 +125,7 @@ describe("dynamic refresh", function () { 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 for key-values."); + expect(() => settings.onRefresh(() => { })).throws("Refresh is not enabled for key-values or feature flags."); }); it("should only update values after refreshInterval", async () => { @@ -326,11 +327,6 @@ describe("dynamic refresh feature flags", function () { this.timeout(10000); beforeEach(() => { - mockedKVs = [ - createMockedFeatureFlag("Beta", { enabled: true }) - ]; - mockAppConfigurationClientListConfigurationSettings(mockedKVs); - mockAppConfigurationClientGetConfigurationSetting(mockedKVs) }); afterEach(() => { @@ -338,6 +334,12 @@ describe("dynamic refresh feature flags", function () { }) it("should refresh feature flags when enabled", async () => { + mockedKVs = [ + createMockedFeatureFlag("Beta", { enabled: true }) + ]; + mockAppConfigurationClientListConfigurationSettings(mockedKVs); + mockAppConfigurationClientGetConfigurationSetting(mockedKVs) + const connectionString = createMockedConnectionString(); const settings = await load(connectionString, { featureFlagOptions: { @@ -375,4 +377,50 @@ describe("dynamic refresh feature flags", function () { }); + it("should refresh feature flags only on change, based on page etags", async () => { + // mock multiple pages of feature flags + const page1 = [ + createMockedFeatureFlag("Alpha_1", { enabled: true }), + createMockedFeatureFlag("Alpha_2", { enabled: true }), + ]; + const page2 = [ + createMockedFeatureFlag("Beta_1", { enabled: true }), + createMockedFeatureFlag("Beta_2", { enabled: true }), + ]; + mockAppConfigurationClientListConfigurationSettings(page1, page2); + mockAppConfigurationClientGetConfigurationSetting([...page1, ...page2]); + + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + featureFlagOptions: { + enabled: true, + selectors: [{ + keyFilter: "*" + }], + refresh: { + enabled: true, + refreshIntervalInMs: 2000 // 2 seconds for quick test. + } + } + }); + + let refreshSuccessfulCount = 0; + settings.onRefresh(() => { + refreshSuccessfulCount++; + }); + + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + expect(refreshSuccessfulCount).eq(0); // no change in feature flags, because page etags are the same. + + // change feature flag Beta_1 to false + page2[0] = createMockedFeatureFlag("Beta_1", { enabled: false }); + restoreMocks(); + mockAppConfigurationClientListConfigurationSettings(page1, page2); + mockAppConfigurationClientGetConfigurationSetting([...page1, ...page2]); + + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + expect(refreshSuccessfulCount).eq(1); // change in feature flags, because page etags are different. + }); }); \ No newline at end of file diff --git a/test/utils/testHelper.ts b/test/utils/testHelper.ts index 6fa96af9..80da36bc 100644 --- a/test/utils/testHelper.ts +++ b/test/utils/testHelper.ts @@ -9,11 +9,16 @@ import * as uuid from "uuid"; import { RestError } from "@azure/core-rest-pipeline"; import { promisify } from "util"; const sleepInMs = promisify(setTimeout); +import * as crypto from "crypto"; const TEST_CLIENT_ID = "00000000-0000-0000-0000-000000000000"; const TEST_TENANT_ID = "00000000-0000-0000-0000-000000000000"; const TEST_CLIENT_SECRET = "0000000000000000000000000000000000000000"; +function _sha256(input) { + return crypto.createHash("sha256").update(input).digest("hex"); +} + function _filterKVs(unfilteredKvs: ConfigurationSetting[], listOptions: any) { const keyFilter = listOptions?.keyFilter ?? "*"; const labelFilter = listOptions?.labelFilter ?? "*"; @@ -45,7 +50,6 @@ function mockAppConfigurationClientListConfigurationSettings(...pages: Configura sinon.stub(AppConfigurationClient.prototype, "listConfigurationSettings").callsFake((listOptions) => { let kvs = _filterKVs(pages.flat(), listOptions); - const mockIterator: AsyncIterableIterator & { byPage(): AsyncIterableIterator } = { [Symbol.asyncIterator](): AsyncIterableIterator { kvs = _filterKVs(pages.flat(), listOptions); @@ -57,21 +61,30 @@ function mockAppConfigurationClientListConfigurationSettings(...pages: Configura }, byPage(): AsyncIterableIterator { let remainingPages; + const pageEtags = listOptions?.pageEtags ? [...listOptions.pageEtags] : undefined; // a copy of the original list return { [Symbol.asyncIterator](): AsyncIterableIterator { - remainingPages = [ ...pages ]; + remainingPages = [...pages]; return this; }, next() { const pageItems = remainingPages.shift(); - return Promise.resolve({ - done: pageItems === undefined, - value: { - items: _filterKVs(pageItems ?? [], listOptions), - eTag: "etag", - _response: { status: 200 } // TODO: 304 if etag matches - } - }); + const pageEtag = pageEtags?.shift(); + if (pageItems === undefined) { + return Promise.resolve({ done: true, value: undefined }); + } else { + const items = _filterKVs(pageItems ?? [], listOptions); + const etag = _sha256(JSON.stringify(items)); + const statusCode = pageEtag === etag ? 304 : 200; + return Promise.resolve({ + done: false, + value: { + items, + etag, + _response: { status: statusCode } + } + }); + } } } } From fb56a0f33d23b8d097c5ddd663b6b4cb178ad8ca Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Mon, 29 Jul 2024 13:14:05 +0800 Subject: [PATCH 17/19] remove pageCount as service ensure etag changes on page add/delete --- src/AzureAppConfigurationImpl.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index b51d7173..ed68ea18 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -447,18 +447,11 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { listOptions ).byPage(); - let pageCount = 0; for await (const page of pageIterator) { if (page._response.status === 200) { // created or changed needRefresh = true; break; } - // unchanged, check next page - pageCount++; - } - - if (pageCount !== selector.pageEtags?.length) { - needRefresh = true; // page count changed indicating feature flags are added or deleted } if (needRefresh) { From b1cf8099fbf8737de3d38299be2871d4547c7cac Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Mon, 29 Jul 2024 13:23:01 +0800 Subject: [PATCH 18/19] reset timer on 304, maintain refresh interval gap between attempts --- src/AzureAppConfigurationImpl.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index ed68ea18..08bb9031 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -411,16 +411,15 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { if (needRefresh) { try { await this.#loadSelectedAndWatchedKeyValues(); - this.#refreshTimer.reset(); } catch (error) { // if refresh failed, backoff this.#refreshTimer.backoff(); throw error; } - return Promise.resolve(true); } - return Promise.resolve(false); + this.#refreshTimer.reset(); + return Promise.resolve(needRefresh); } /** @@ -462,16 +461,15 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { if (needRefresh) { try { await this.#loadFeatureFlags(); - this.#featureFlagRefreshTimer.reset(); } catch (error) { // if refresh failed, backoff this.#featureFlagRefreshTimer.backoff(); throw error; } - return Promise.resolve(true); } - return Promise.resolve(false); + this.#featureFlagRefreshTimer.reset(); + return Promise.resolve(needRefresh); } onRefresh(listener: () => any, thisArg?: any): Disposable { From 3c12654f132e35c83631f5a67bce20970840584c Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Mon, 29 Jul 2024 13:35:22 +0800 Subject: [PATCH 19/19] use empty string as placeholder when page etag is undefined, to keep the order --- src/AzureAppConfigurationImpl.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 08bb9031..6c650244 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -270,11 +270,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { listOptions ).byPage(); for await (const page of pageIterator) { - if (page._response.status === 200) { - if (page.etag) { - pageEtags.push(page.etag); - } - } + pageEtags.push(page.etag ?? ""); for (const setting of page.items) { if (isFeatureFlag(setting)) { featureFlagsMap.set(setting.key, setting.value);