From 4f9f44123f42877e4212e51ff655718df4469623 Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Sun, 20 Apr 2025 11:07:14 +0200 Subject: [PATCH 1/9] Simplify ConfigServiceBase.getReadyPromise --- src/AutoPollConfigService.ts | 10 ++++++---- src/ConfigServiceBase.ts | 13 +++++++++---- src/LazyLoadConfigService.ts | 2 +- src/ManualPollConfigService.ts | 2 +- 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/AutoPollConfigService.ts b/src/AutoPollConfigService.ts index 6f74b65..9030d1f 100644 --- a/src/AutoPollConfigService.ts +++ b/src/AutoPollConfigService.ts @@ -47,10 +47,7 @@ export class AutoPollConfigService extends ConfigServiceBase im this.initializationPromise = Promise.resolve(false); } - this.readyPromise = this.getReadyPromise(this.initializationPromise, async initializationPromise => { - await initializationPromise; - return this.getCacheState(this.options.cache.getInMemory()); - }); + this.readyPromise = this.getReadyPromise(initialCacheSyncUp); if (!options.offline) { this.startRefreshWorker(initialCacheSyncUp, this.stopToken); @@ -72,6 +69,11 @@ export class AutoPollConfigService extends ConfigServiceBase im return success; } + protected override async waitForReadyAsync(): Promise { + await this.initializationPromise; + return this.getCacheState(this.options.cache.getInMemory()); + } + async getConfig(): Promise { this.options.logger.debug("AutoPollConfigService.getConfig() called."); diff --git a/src/ConfigServiceBase.ts b/src/ConfigServiceBase.ts index 8b376c8..0ef240a 100644 --- a/src/ConfigServiceBase.ts +++ b/src/ConfigServiceBase.ts @@ -308,9 +308,14 @@ export abstract class ConfigServiceBase { return this.options.cache.get(this.cacheKey); } - protected async getReadyPromise(state: TState, waitForReadyAsync: (state: TState) => Promise): Promise { - const cacheState = await waitForReadyAsync(state); - this.options.hooks.emit("clientReady", cacheState); - return cacheState; + protected async waitForReadyAsync(initialCacheSyncUp: ProjectConfig | Promise): Promise { + return this.getCacheState(await initialCacheSyncUp); + } + + protected getReadyPromise(initialCacheSyncUp: ProjectConfig | Promise): Promise { + return this.waitForReadyAsync(initialCacheSyncUp).then(cacheState => { + this.options.hooks.emit("clientReady", cacheState); + return cacheState; + }); } } diff --git a/src/LazyLoadConfigService.ts b/src/LazyLoadConfigService.ts index fb6a14c..34d88c7 100644 --- a/src/LazyLoadConfigService.ts +++ b/src/LazyLoadConfigService.ts @@ -17,7 +17,7 @@ export class LazyLoadConfigService extends ConfigServiceBase im this.cacheTimeToLiveMs = options.cacheTimeToLiveSeconds * 1000; const initialCacheSyncUp = this.syncUpWithCache(); - this.readyPromise = this.getReadyPromise(initialCacheSyncUp, async initialCacheSyncUp => this.getCacheState(await initialCacheSyncUp)); + this.readyPromise = this.getReadyPromise(initialCacheSyncUp); } async getConfig(): Promise { diff --git a/src/ManualPollConfigService.ts b/src/ManualPollConfigService.ts index 0988112..c3d50a8 100644 --- a/src/ManualPollConfigService.ts +++ b/src/ManualPollConfigService.ts @@ -13,7 +13,7 @@ export class ManualPollConfigService extends ConfigServiceBase this.getCacheState(await initialCacheSyncUp)); + this.readyPromise = this.getReadyPromise(initialCacheSyncUp); } getCacheState(cachedConfig: ProjectConfig): ClientCacheState { From 110b4c2e3f7f292504cd6a0b5beb05a1be4d3e36 Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Tue, 22 Apr 2025 14:03:09 +0200 Subject: [PATCH 2/9] Use monitonic clock for scheduling poll iterations in Auto Poll mode for improved precision and resistance to system/user clock adjustments --- src/AutoPollConfigService.ts | 6 +++--- src/Utils.ts | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/AutoPollConfigService.ts b/src/AutoPollConfigService.ts index 9030d1f..e4283a6 100644 --- a/src/AutoPollConfigService.ts +++ b/src/AutoPollConfigService.ts @@ -4,7 +4,7 @@ import type { IConfigFetcher } from "./ConfigFetcher"; import type { IConfigService, RefreshResult } from "./ConfigServiceBase"; import { ClientCacheState, ConfigServiceBase } from "./ConfigServiceBase"; import type { ProjectConfig } from "./ProjectConfig"; -import { AbortToken, delay } from "./Utils"; +import { AbortToken, delay, getMonotonicTimeMs } from "./Utils"; export const POLL_EXPIRATION_TOLERANCE_MS = 500; @@ -136,14 +136,14 @@ export class AutoPollConfigService extends ConfigServiceBase im let isFirstIteration = true; while (!stopToken.aborted) { try { - const scheduledNextTimeMs = new Date().getTime() + this.pollIntervalMs; + const scheduledNextTimeMs = getMonotonicTimeMs() + this.pollIntervalMs; try { await this.refreshWorkerLogic(isFirstIteration, initialCacheSyncUp); } catch (err) { this.options.logger.autoPollConfigServiceErrorDuringPolling(err); } - const realNextTimeMs = scheduledNextTimeMs - new Date().getTime(); + const realNextTimeMs = scheduledNextTimeMs - getMonotonicTimeMs(); if (realNextTimeMs > 0) { await delay(realNextTimeMs, stopToken); } diff --git a/src/Utils.ts b/src/Utils.ts index e1eb999..96fdcda 100644 --- a/src/Utils.ts +++ b/src/Utils.ts @@ -46,6 +46,10 @@ export function delay(delayMs: number, abortToken?: AbortToken | null): Promise< }); } +export const getMonotonicTimeMs = typeof performance !== "undefined" && typeof performance.now === "function" + ? () => performance.now() + : () => new Date().getTime(); + export function errorToString(err: any, includeStackTrace = false): string { return err instanceof Error ? includeStackTrace && err.stack ? err.stack : err.toString() From b6f64c73561f207004170273659277c2350a51d6 Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Fri, 2 May 2025 19:26:52 +0200 Subject: [PATCH 3/9] Use monotonic clock in tests for measuring elapsed time + increase the tolerance of time-sensitive tests --- test/ConfigCatClientTests.ts | 50 ++++++++++++++-------------- test/ConfigServiceBaseTests.ts | 12 +++---- test/browser/HttpTests.ts | 5 +-- test/chromium-extension/HttpTests.ts | 5 +-- test/helpers/fakes.ts | 2 +- test/node/HttpTests.ts | 5 +-- 6 files changed, 41 insertions(+), 38 deletions(-) diff --git a/test/ConfigCatClientTests.ts b/test/ConfigCatClientTests.ts index 61d1162..a326cbb 100644 --- a/test/ConfigCatClientTests.ts +++ b/test/ConfigCatClientTests.ts @@ -15,7 +15,7 @@ import { isWeakRefAvailable, setupPolyfills } from "#lib/Polyfills"; import { Config, IConfig, ProjectConfig, SettingValue, SettingValueContainer } from "#lib/ProjectConfig"; import { EvaluateContext, IEvaluateResult, IEvaluationDetails, IRolloutEvaluator } from "#lib/RolloutEvaluator"; import { User } from "#lib/User"; -import { delay } from "#lib/Utils"; +import { delay, getMonotonicTimeMs } from "#lib/Utils"; import "./helpers/ConfigCatClientCacheExtensions"; describe("ConfigCatClient", () => { @@ -195,7 +195,7 @@ describe("ConfigCatClient", () => { const key = "notexists"; const defaultValue = false; - const timestamp = new Date().getTime(); + const timestamp = ProjectConfig.generateTimestamp(); const configFetcherClass = FakeConfigFetcherWithTwoKeys; const cachedPc = new ProjectConfig(configFetcherClass.configJson, Config.deserialize(configFetcherClass.configJson), timestamp, "etag"); @@ -238,7 +238,7 @@ describe("ConfigCatClient", () => { const key = "debug"; const defaultValue = false; - const timestamp = new Date().getTime(); + const timestamp = ProjectConfig.generateTimestamp(); const configFetcherClass = FakeConfigFetcherWithTwoKeys; const cachedPc = new ProjectConfig(configFetcherClass.configJson, Config.deserialize(configFetcherClass.configJson), timestamp, "etag"); @@ -281,7 +281,7 @@ describe("ConfigCatClient", () => { const key = "debug"; const defaultValue = "N/A"; - const timestamp = new Date().getTime(); + const timestamp = ProjectConfig.generateTimestamp(); const configFetcherClass = FakeConfigFetcherWithRules; const cachedPc = new ProjectConfig(configFetcherClass.configJson, Config.deserialize(configFetcherClass.configJson), timestamp, "etag"); @@ -327,7 +327,7 @@ describe("ConfigCatClient", () => { const key = "string25Cat25Dog25Falcon25Horse"; const defaultValue = "N/A"; - const timestamp = new Date().getTime(); + const timestamp = ProjectConfig.generateTimestamp(); const configFetcherClass = FakeConfigFetcherWithPercentageOptions; const cachedPc = new ProjectConfig(configFetcherClass.configJson, Config.deserialize(configFetcherClass.configJson), timestamp, "etag"); @@ -372,7 +372,7 @@ describe("ConfigCatClient", () => { const key = "debug"; const defaultValue = false; - const timestamp = new Date().getTime(); + const timestamp = ProjectConfig.generateTimestamp(); const configFetcherClass = FakeConfigFetcherWithTwoKeys; const cachedPc = new ProjectConfig(configFetcherClass.configJson, Config.deserialize(configFetcherClass.configJson), timestamp, "etag"); @@ -427,7 +427,7 @@ describe("ConfigCatClient", () => { // Arrange - const timestamp = new Date().getTime(); + const timestamp = ProjectConfig.generateTimestamp(); const configFetcherClass = FakeConfigFetcherWithTwoKeys; const cachedPc = new ProjectConfig(configFetcherClass.configJson, Config.deserialize(configFetcherClass.configJson), timestamp, "etag"); @@ -479,7 +479,7 @@ describe("ConfigCatClient", () => { // Arrange - const timestamp = new Date().getTime(); + const timestamp = ProjectConfig.generateTimestamp(); const configFetcherClass = FakeConfigFetcherWithTwoKeys; const cachedPc = new ProjectConfig(configFetcherClass.configJson, Config.deserialize(configFetcherClass.configJson), timestamp, "etag"); @@ -585,13 +585,13 @@ describe("ConfigCatClient", () => { const configCatKernel = createKernel({ configFetcher: new FakeConfigFetcher(500) }); const options: AutoPollOptions = createAutoPollOptions("APIKEY", { maxInitWaitTimeSeconds }, configCatKernel); - const startDate: number = new Date().getTime(); + const startTime: number = getMonotonicTimeMs(); const client: IConfigCatClient = new ConfigCatClient(options, configCatKernel); const actualValue = await client.getValueAsync("debug", false); - const elapsedMilliseconds: number = new Date().getTime() - startDate; + const elapsedMilliseconds: number = getMonotonicTimeMs() - startTime; assert.isAtLeast(elapsedMilliseconds, 500 - 25); // 25 ms for tolerance - assert.isAtMost(elapsedMilliseconds, maxInitWaitTimeSeconds * 1000 + 75); // 75 ms for tolerance + assert.isAtMost(elapsedMilliseconds, maxInitWaitTimeSeconds * 1000 + 100); // 100 ms for tolerance assert.equal(actualValue, true); client.dispose(); @@ -609,13 +609,13 @@ describe("ConfigCatClient", () => { const configCatKernel = createKernel({ configFetcher }); const options: AutoPollOptions = createAutoPollOptions("APIKEY", { maxInitWaitTimeSeconds }, configCatKernel); - const startDate: number = new Date().getTime(); + const startTime: number = getMonotonicTimeMs(); const client: IConfigCatClient = new ConfigCatClient(options, configCatKernel); const actualDetails = await client.getValueDetailsAsync("debug", false); - const elapsedMilliseconds: number = new Date().getTime() - startDate; + const elapsedMilliseconds: number = getMonotonicTimeMs() - startTime; assert.isAtLeast(elapsedMilliseconds, 500 - 25); // 25 ms for tolerance - assert.isAtMost(elapsedMilliseconds, configFetchDelay * 2 + 75); // 75 ms for tolerance + assert.isAtMost(elapsedMilliseconds, configFetchDelay * 2 + 100); // 100 ms for tolerance assert.equal(actualDetails.isDefaultValue, true); assert.equal(actualDetails.value, false); @@ -630,13 +630,13 @@ describe("ConfigCatClient", () => { const configCatKernel = createKernel({ configFetcher: new FakeConfigFetcherWithNullNewConfig(10000) }); const options: AutoPollOptions = createAutoPollOptions("APIKEY", { maxInitWaitTimeSeconds }, configCatKernel); - const startDate: number = new Date().getTime(); + const startTime: number = getMonotonicTimeMs(); const client: IConfigCatClient = new ConfigCatClient(options, configCatKernel); const actualValue = await client.getValueAsync("debug", false); - const elapsedMilliseconds: number = new Date().getTime() - startDate; + const elapsedMilliseconds: number = getMonotonicTimeMs() - startTime; assert.isAtLeast(elapsedMilliseconds, (maxInitWaitTimeSeconds * 1000) - 25); // 25 ms for tolerance - assert.isAtMost(elapsedMilliseconds, (maxInitWaitTimeSeconds * 1000) + 75); // 75 ms for tolerance + assert.isAtMost(elapsedMilliseconds, (maxInitWaitTimeSeconds * 1000) + 100); // 100 ms for tolerance assert.equal(actualValue, false); client.dispose(); @@ -671,13 +671,13 @@ describe("ConfigCatClient", () => { const configCatKernel = createKernel({ configFetcher }); const options: AutoPollOptions = createAutoPollOptions("APIKEY", { maxInitWaitTimeSeconds }, configCatKernel); - const startDate: number = new Date().getTime(); + const startTime: number = getMonotonicTimeMs(); const client: IConfigCatClient = new ConfigCatClient(options, configCatKernel); const state = await client.waitForReady(); - const elapsedMilliseconds: number = new Date().getTime() - startDate; + const elapsedMilliseconds: number = getMonotonicTimeMs() - startTime; assert.isAtLeast(elapsedMilliseconds, (maxInitWaitTimeSeconds * 1000) - 25); // 25 ms for tolerance - assert.isAtMost(elapsedMilliseconds, (maxInitWaitTimeSeconds * 1000) + 75); // 75 ms for tolerance + assert.isAtMost(elapsedMilliseconds, (maxInitWaitTimeSeconds * 1000) + 100); // 100 ms for tolerance assert.equal(state, ClientCacheState.NoFlagData); @@ -704,13 +704,13 @@ describe("ConfigCatClient", () => { cache: new FakeExternalCacheWithInitialData(120_000), }, configCatKernel); - const startDate: number = new Date().getTime(); + const startTime: number = getMonotonicTimeMs(); const client: IConfigCatClient = new ConfigCatClient(options, configCatKernel); const state = await client.waitForReady(); - const elapsedMilliseconds: number = new Date().getTime() - startDate; + const elapsedMilliseconds: number = getMonotonicTimeMs() - startTime; assert.isAtLeast(elapsedMilliseconds, (maxInitWaitTimeSeconds * 1000) - 25); // 25 ms for tolerance - assert.isAtMost(elapsedMilliseconds, (maxInitWaitTimeSeconds * 1000) + 75); // 75 ms for tolerance + assert.isAtMost(elapsedMilliseconds, (maxInitWaitTimeSeconds * 1000) + 100); // 100 ms for tolerance assert.equal(state, ClientCacheState.HasCachedFlagDataOnly); @@ -948,7 +948,7 @@ describe("ConfigCatClient", () => { const configFetcher = new FakeConfigFetcher(500); const configJson = "{\"f\": { \"debug\": { \"v\": { \"b\": false }, \"i\": \"abcdefgh\", \"t\": 0, \"p\": [], \"r\": [] } } }"; - const configCache = new FakeCache(new ProjectConfig(configJson, Config.deserialize(configJson), new Date().getTime() - 10000000, "etag2")); + const configCache = new FakeCache(new ProjectConfig(configJson, Config.deserialize(configJson), ProjectConfig.generateTimestamp() - 10000000, "etag2")); const configCatKernel = createKernel({ configFetcher, defaultCacheFactory: () => configCache }); const options: AutoPollOptions = createAutoPollOptions("APIKEY", { maxInitWaitTimeSeconds: 10 }, configCatKernel); const client: IConfigCatClient = new ConfigCatClient(options, configCatKernel); @@ -963,7 +963,7 @@ describe("ConfigCatClient", () => { const configFetcher = new FakeConfigFetcher(500); const configJson = "{\"f\": { \"debug\": { \"v\": { \"b\": false }, \"i\": \"abcdefgh\", \"t\": 0, \"p\": [], \"r\": [] } } }"; - const configCache = new FakeCache(new ProjectConfig(configJson, Config.deserialize(configJson), new Date().getTime() - 10000000, "etag2")); + const configCache = new FakeCache(new ProjectConfig(configJson, Config.deserialize(configJson), ProjectConfig.generateTimestamp() - 10000000, "etag2")); const configCatKernel = createKernel({ configFetcher, defaultCacheFactory: () => configCache }); const options: AutoPollOptions = createAutoPollOptions("APIKEY", { maxInitWaitTimeSeconds: 10 }, configCatKernel); const client: IConfigCatClient = new ConfigCatClient(options, configCatKernel); diff --git a/test/ConfigServiceBaseTests.ts b/test/ConfigServiceBaseTests.ts index 197d0d0..26acd88 100644 --- a/test/ConfigServiceBaseTests.ts +++ b/test/ConfigServiceBaseTests.ts @@ -176,7 +176,7 @@ describe("ConfigServiceBaseTests", () => { const projectConfigNew: ProjectConfig = createConfigFromFetchResult(frNew); - const time: number = new Date().getTime(); + const time: number = ProjectConfig.generateTimestamp(); const projectConfigOld: ProjectConfig = createConfigFromFetchResult(frOld).with(time - (1.5 * pollInterval * 1000) + 0.5 * POLL_EXPIRATION_TOLERANCE_MS); const cache = new InMemoryConfigCache(); @@ -224,7 +224,7 @@ describe("ConfigServiceBaseTests", () => { const pollInterval = 10; - const time: number = new Date().getTime(); + const time: number = ProjectConfig.generateTimestamp(); const projectConfigOld = createConfigFromFetchResult(frOld).with(time - (pollInterval * 1000) + 0.5 * POLL_EXPIRATION_TOLERANCE_MS); const cache = new InMemoryConfigCache(); @@ -315,7 +315,7 @@ describe("ConfigServiceBaseTests", () => { // Arrange const cacheTimeToLiveSeconds = 10; - const oldConfig: ProjectConfig = createProjectConfig("oldConfig").with(new Date().getTime() - (cacheTimeToLiveSeconds * 1000) - 1000); + const oldConfig: ProjectConfig = createProjectConfig("oldConfig").with(ProjectConfig.generateTimestamp() - (cacheTimeToLiveSeconds * 1000) - 1000); const fr: FetchResult = createFetchResult("newConfig"); @@ -358,7 +358,7 @@ describe("ConfigServiceBaseTests", () => { // Arrange - const config: ProjectConfig = createProjectConfig().with(new Date().getTime()); + const config: ProjectConfig = createProjectConfig().with(ProjectConfig.generateTimestamp()); const fetcherMock = new Mock(); @@ -392,7 +392,7 @@ describe("ConfigServiceBaseTests", () => { // Arrange - const config: ProjectConfig = createProjectConfig().with(new Date().getTime() - 1000); + const config: ProjectConfig = createProjectConfig().with(ProjectConfig.generateTimestamp() - 1000); const fr: FetchResult = createFetchResult(); @@ -433,7 +433,7 @@ describe("ConfigServiceBaseTests", () => { // Arrange - const config: ProjectConfig = createProjectConfig().with(new Date().getTime() - 1000); + const config: ProjectConfig = createProjectConfig().with(ProjectConfig.generateTimestamp() - 1000); const fr: FetchResult = createFetchResult(); diff --git a/test/browser/HttpTests.ts b/test/browser/HttpTests.ts index f3b9359..bd8a30a 100644 --- a/test/browser/HttpTests.ts +++ b/test/browser/HttpTests.ts @@ -3,6 +3,7 @@ import * as mockxmlhttprequest from "mock-xmlhttprequest"; import { FakeLogger } from "../helpers/fakes"; import { platform } from "."; import { LogLevel } from "#lib"; +import { getMonotonicTimeMs } from "#lib/Utils"; describe("HTTP tests", () => { const sdkKey = "configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/AG6C1ngVb0CvM07un6JisQ"; @@ -25,9 +26,9 @@ describe("HTTP tests", () => { baseUrl, logger, }); - const startTime = new Date().getTime(); + const startTime = getMonotonicTimeMs(); await client.forceRefreshAsync(); - const duration = new Date().getTime() - startTime; + const duration = getMonotonicTimeMs() - startTime; assert.isTrue(duration > 1000 && duration < 2000); const defaultValue = "NOT_CAT"; diff --git a/test/chromium-extension/HttpTests.ts b/test/chromium-extension/HttpTests.ts index beb76e6..8ca50a8 100644 --- a/test/chromium-extension/HttpTests.ts +++ b/test/chromium-extension/HttpTests.ts @@ -3,6 +3,7 @@ import fetchMock from "fetch-mock"; import { FakeLogger } from "../helpers/fakes"; import { platform } from "."; import { LogLevel } from "#lib"; +import { getMonotonicTimeMs } from "#lib/Utils"; describe("HTTP tests", () => { const sdkKey = "PKDVCLf-Hq-h-kCzMp-L7Q/psuH7BGHoUmdONrzzUOY7A"; @@ -23,9 +24,9 @@ describe("HTTP tests", () => { baseUrl, logger, }); - const startTime = new Date().getTime(); + const startTime = getMonotonicTimeMs(); await client.forceRefreshAsync(); - const duration = new Date().getTime() - startTime; + const duration = getMonotonicTimeMs() - startTime; assert.isTrue(duration > 1000 && duration < 2000); const defaultValue = "NOT_CAT"; diff --git a/test/helpers/fakes.ts b/test/helpers/fakes.ts index 04d046e..79966f3 100644 --- a/test/helpers/fakes.ts +++ b/test/helpers/fakes.ts @@ -111,7 +111,7 @@ export class FakeExternalCacheWithInitialData implements IConfigCatCache { } get(key: string): string | Promise | null | undefined { const cachedJson = '{"f":{"debug":{"t":0,"v":{"b":true},"i":"abcdefgh"}}}'; - const config = new ProjectConfig(cachedJson, JSON.parse(cachedJson), (new Date().getTime()) - this.expirationDelta, "\"ETAG\""); + const config = new ProjectConfig(cachedJson, JSON.parse(cachedJson), ProjectConfig.generateTimestamp() - this.expirationDelta, "\"ETAG\""); return ProjectConfig.serialize(config); } diff --git a/test/node/HttpTests.ts b/test/node/HttpTests.ts index c73cbaf..abdf95d 100644 --- a/test/node/HttpTests.ts +++ b/test/node/HttpTests.ts @@ -3,6 +3,7 @@ import * as mockttp from "mockttp"; import { FakeLogger } from "../helpers/fakes"; import { platform } from "."; import { LogLevel } from "#lib"; +import { getMonotonicTimeMs } from "#lib/Utils"; // If the tests are failing with strange https or proxy errors, it is most likely that the local .key and .pem files are expired. // You can regenerate them anytime (./test/cert/regenerate.md). @@ -31,9 +32,9 @@ describe("HTTP tests", () => { baseUrl: server.url, logger, }); - const startTime = new Date().getTime(); + const startTime = getMonotonicTimeMs(); await client.forceRefreshAsync(); - const duration = new Date().getTime() - startTime; + const duration = getMonotonicTimeMs() - startTime; assert.isTrue(duration > 1000 && duration < 2000); const defaultValue = "NOT_CAT"; From 049bfa47d15f8d44317714b715a90f30e32ecf0a Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Tue, 22 Apr 2025 15:58:43 +0200 Subject: [PATCH 4/9] Refresh internal cache in offline mode too --- src/AutoPollConfigService.ts | 45 +++++------ src/ConfigServiceBase.ts | 14 ++-- test/ConfigCatCacheTests.ts | 24 +----- test/ConfigCatClientOptionsTests.ts | 2 +- test/ConfigCatClientTests.ts | 6 +- test/ConfigServiceBaseTests.ts | 120 +++++++++++++++++++++++++++- test/helpers/fakes.ts | 13 ++- 7 files changed, 162 insertions(+), 62 deletions(-) diff --git a/src/AutoPollConfigService.ts b/src/AutoPollConfigService.ts index e4283a6..0ab1864 100644 --- a/src/AutoPollConfigService.ts +++ b/src/AutoPollConfigService.ts @@ -32,9 +32,9 @@ export class AutoPollConfigService extends ConfigServiceBase im if (options.maxInitWaitTimeSeconds !== 0) { this.initialized = false; - // This promise will be resolved when - // 1. the cache contains a valid config at startup (see startRefreshWorker) or - // 2. config json is fetched the first time, regardless of success or failure (see onConfigUpdated). + // This promise will be resolved as soon as + // 1. an up-to-date config is obtained from the cache (see startRefreshWorker), + // 2. or a config fetch operation completes, regardless of success or failure (see onConfigUpdated). const initSignalPromise = new Promise(resolve => this.signalInitialization = resolve); // This promise will be resolved when either initialization ready is signalled by signalInitialization() or maxInitWaitTimeSeconds pass. @@ -49,9 +49,7 @@ export class AutoPollConfigService extends ConfigServiceBase im this.readyPromise = this.getReadyPromise(initialCacheSyncUp); - if (!options.offline) { - this.startRefreshWorker(initialCacheSyncUp, this.stopToken); - } + this.startRefreshWorker(initialCacheSyncUp, this.stopToken); } private async waitForInitializationAsync(initSignalPromise: Promise): Promise { @@ -82,7 +80,7 @@ export class AutoPollConfigService extends ConfigServiceBase im } let cachedConfig: ProjectConfig; - if (!this.isOffline && !this.initialized) { + if (!this.initialized) { cachedConfig = await this.options.cache.get(this.cacheKey); if (!cachedConfig.isExpired(this.pollIntervalMs)) { logSuccess(this.options.logger); @@ -121,24 +119,22 @@ export class AutoPollConfigService extends ConfigServiceBase im super.onConfigFetched(newConfig); } - protected setOnlineCore(): void { - this.startRefreshWorker(null, this.stopToken); - } - - protected setOfflineCore(): void { + protected goOnline(): void { + // We need to restart the polling loop because going from offline to online should trigger a refresh operation + // immediately instead of waiting for the next tick (which might not happen until much later). this.stopRefreshWorker(); this.stopToken = new AbortToken(); + this.startRefreshWorker(null, this.stopToken); } private async startRefreshWorker(initialCacheSyncUp: ProjectConfig | Promise | null, stopToken: AbortToken) { this.options.logger.debug("AutoPollConfigService.startRefreshWorker() called."); - let isFirstIteration = true; while (!stopToken.aborted) { try { const scheduledNextTimeMs = getMonotonicTimeMs() + this.pollIntervalMs; try { - await this.refreshWorkerLogic(isFirstIteration, initialCacheSyncUp); + await this.refreshWorkerLogic(initialCacheSyncUp); } catch (err) { this.options.logger.autoPollConfigServiceErrorDuringPolling(err); } @@ -151,7 +147,6 @@ export class AutoPollConfigService extends ConfigServiceBase im this.options.logger.autoPollConfigServiceErrorDuringPolling(err); } - isFirstIteration = false; initialCacheSyncUp = null; // allow GC to collect the Promise and its result } } @@ -161,22 +156,24 @@ export class AutoPollConfigService extends ConfigServiceBase im this.stopToken.abort(); } - private async refreshWorkerLogic(isFirstIteration: boolean, initialCacheSyncUp: ProjectConfig | Promise | null) { - this.options.logger.debug("AutoPollConfigService.refreshWorkerLogic() - called."); + private async refreshWorkerLogic(initialCacheSyncUp: ProjectConfig | Promise | null) { + this.options.logger.debug("AutoPollConfigService.refreshWorkerLogic() called."); const latestConfig = await (initialCacheSyncUp ?? this.options.cache.get(this.cacheKey)); - if (latestConfig.isExpired(this.pollExpirationMs)) { + if (!latestConfig.isExpired(this.pollExpirationMs)) { + this.signalInitialization(); + return; + } + + if (initialCacheSyncUp ? !this.isOfflineExactly : !this.isOffline) { // Even if the service gets disposed immediately, we allow the first refresh for backward compatibility, // i.e. to not break usage patterns like this: // ``` - // client.getValueAsync("SOME_KEY", false).then(value => { /* ... */ }, user); + // client.getValueAsync("SOME_KEY", false, user).then(value => { /* ... */ }); // client.dispose(); // ``` - if (isFirstIteration ? !this.isOfflineExactly : !this.isOffline) { - await this.refreshConfigCoreAsync(latestConfig); - } - } else if (isFirstIteration) { - this.signalInitialization(); + + await this.refreshConfigCoreAsync(latestConfig); } } diff --git a/src/ConfigServiceBase.ts b/src/ConfigServiceBase.ts index 0ef240a..7bf9747 100644 --- a/src/ConfigServiceBase.ts +++ b/src/ConfigServiceBase.ts @@ -1,3 +1,4 @@ +import { ExternalConfigCache } from "./ConfigCatCache"; import type { OptionsBase } from "./ConfigCatClientOptions"; import type { FetchErrorCauses, IConfigFetcher, IFetchResponse } from "./ConfigFetcher"; import { FetchError, FetchResult, FetchStatus } from "./ConfigFetcher"; @@ -106,6 +107,8 @@ export abstract class ConfigServiceBase { if (!this.isOffline) { const [fetchResult, config] = await this.refreshConfigCoreAsync(latestConfig); return [RefreshResult.from(fetchResult), config]; + } else if (this.options.cache instanceof ExternalConfigCache) { + return [RefreshResult.success(), latestConfig]; } else { const errorMessage = this.options.logger.configServiceCannotInitiateHttpCalls().toString(); return [RefreshResult.failure(errorMessage), latestConfig]; @@ -155,7 +158,7 @@ export abstract class ConfigServiceBase { private async fetchLogicAsync(lastConfig: ProjectConfig): Promise { const options = this.options; - options.logger.debug("ConfigServiceBase.fetchLogicAsync() - called."); + options.logger.debug("ConfigServiceBase.fetchLogicAsync() called."); let errorMessage: string; try { @@ -207,7 +210,7 @@ export abstract class ConfigServiceBase { // eslint-disable-next-line @typescript-eslint/no-redundant-type-constituents private async fetchRequestAsync(lastETag: string | null, maxRetryCount = 2): Promise<[IFetchResponse, (Config | any)?]> { const options = this.options; - options.logger.debug("ConfigServiceBase.fetchRequestAsync() - called."); + options.logger.debug("ConfigServiceBase.fetchRequestAsync() called."); for (let retryNumber = 0; ; retryNumber++) { options.logger.debug(`ConfigServiceBase.fetchRequestAsync(): calling fetchLogic()${retryNumber > 0 ? `, retry ${retryNumber}/${maxRetryCount}` : ""}`); @@ -278,11 +281,11 @@ export abstract class ConfigServiceBase { return this.status !== ConfigServiceStatus.Online; } - protected setOnlineCore(): void { /* Intentionally empty. */ } + protected goOnline(): void { /* Intentionally empty. */ } setOnline(): void { if (this.status === ConfigServiceStatus.Offline) { - this.setOnlineCore(); + this.goOnline(); this.status = ConfigServiceStatus.Online; this.options.logger.configServiceStatusChanged(nameOfConfigServiceStatus(this.status)); } else if (this.disposed) { @@ -290,11 +293,8 @@ export abstract class ConfigServiceBase { } } - protected setOfflineCore(): void { /* Intentionally empty. */ } - setOffline(): void { if (this.status === ConfigServiceStatus.Online) { - this.setOfflineCore(); this.status = ConfigServiceStatus.Offline; this.options.logger.configServiceStatusChanged(nameOfConfigServiceStatus(this.status)); } else if (this.disposed) { diff --git a/test/ConfigCatCacheTests.ts b/test/ConfigCatCacheTests.ts index 3d00ee1..0b9dc39 100644 --- a/test/ConfigCatCacheTests.ts +++ b/test/ConfigCatCacheTests.ts @@ -1,6 +1,6 @@ import { assert } from "chai"; -import { createManualPollOptions, FakeLogger } from "./helpers/fakes"; -import { ExternalConfigCache, IConfigCache, IConfigCatCache, InMemoryConfigCache } from "#lib/ConfigCatCache"; +import { createManualPollOptions, FakeExternalCache, FakeLogger, FaultyFakeExternalCache } from "./helpers/fakes"; +import { ExternalConfigCache, IConfigCache, InMemoryConfigCache } from "#lib/ConfigCatCache"; import { LoggerWrapper, LogLevel } from "#lib/ConfigCatLogger"; import { Config, ProjectConfig } from "#lib/ProjectConfig"; @@ -111,23 +111,3 @@ describe("ConfigCatCache", () => { }); } }); - -class FakeExternalCache implements IConfigCatCache { - cachedValue?: string; - - set(key: string, value: string): void { - this.cachedValue = value; - } - get(key: string): string | undefined { - return this.cachedValue; - } -} - -class FaultyFakeExternalCache implements IConfigCatCache { - set(key: string, value: string): never { - throw new Error("Operation failed :("); - } - get(key: string): never { - throw new Error("Operation failed :("); - } -} diff --git a/test/ConfigCatClientOptionsTests.ts b/test/ConfigCatClientOptionsTests.ts index ee2ef4f..70ea94f 100644 --- a/test/ConfigCatClientOptionsTests.ts +++ b/test/ConfigCatClientOptionsTests.ts @@ -415,7 +415,7 @@ class FakeCache implements IConfigCache { } } -export class FakeLogger implements IConfigCatLogger { +class FakeLogger implements IConfigCatLogger { level?: LogLevel | undefined; log(level: LogLevel, eventId: LogEventId, message: LogMessage, exception?: any): void { diff --git a/test/ConfigCatClientTests.ts b/test/ConfigCatClientTests.ts index a326cbb..04238b1 100644 --- a/test/ConfigCatClientTests.ts +++ b/test/ConfigCatClientTests.ts @@ -1216,7 +1216,7 @@ describe("ConfigCatClient", () => { ]; for (const [pollingMode, optionsFactory] of optionsFactoriesForOfflineModeTests) { - it(`setOnline() should make a(n) ${PollingMode[pollingMode]} client created in offline mode transition to online mode.`, async () => { + it(`setOnline() should make a(n) ${PollingMode[pollingMode]} client created in offline mode transition to online mode`, async () => { const configFetcher = new FakeConfigFetcherBase("{}", 100, (lastConfig, lastETag) => ({ statusCode: 200, @@ -1283,7 +1283,7 @@ describe("ConfigCatClient", () => { } for (const [pollingMode, optionsFactory] of optionsFactoriesForOfflineModeTests) { - it(`setOffline() should make a(n) ${PollingMode[pollingMode]} client created in online mode transition to offline mode.`, async () => { + it(`setOffline() should make a(n) ${PollingMode[pollingMode]} client created in online mode transition to offline mode`, async () => { const configFetcher = new FakeConfigFetcherBase("{}", 100, (lastConfig, lastETag) => ({ statusCode: 200, @@ -1352,7 +1352,7 @@ describe("ConfigCatClient", () => { } for (const addListenersViaOptions of [false, true]) { - it(`ConfigCatClient should emit events, which listeners added ${addListenersViaOptions ? "via options" : "directly on the client"} should get notified of.`, async () => { + it(`ConfigCatClient should emit events, which listeners added ${addListenersViaOptions ? "via options" : "directly on the client"} should get notified of`, async () => { let clientReadyEventCount = 0; const configChangedEvents: IConfig[] = []; const flagEvaluatedEvents: IEvaluationDetails[] = []; diff --git a/test/ConfigServiceBaseTests.ts b/test/ConfigServiceBaseTests.ts index 26acd88..d576141 100644 --- a/test/ConfigServiceBaseTests.ts +++ b/test/ConfigServiceBaseTests.ts @@ -3,15 +3,17 @@ import { assert } from "chai"; import { EqualMatchingInjectorConfig, It, Mock, RejectedPromiseFactory, ResolvedPromiseFactory, Times } from "moq.ts"; import { MimicsRejectedAsyncPresetFactory, MimicsResolvedAsyncPresetFactory, Presets, ReturnsAsyncPresetFactory, RootMockProvider, ThrowsAsyncPresetFactory } from "moq.ts/internal"; /* eslint-enable import/no-duplicates */ -import { createAutoPollOptions, createKernel, createLazyLoadOptions, createManualPollOptions, FakeCache } from "./helpers/fakes"; +import { createAutoPollOptions, createKernel, createLazyLoadOptions, createManualPollOptions, FakeCache, FakeExternalCache, FakeLogger } from "./helpers/fakes"; +import { ClientCacheState } from "#lib"; import { AutoPollConfigService, POLL_EXPIRATION_TOLERANCE_MS } from "#lib/AutoPollConfigService"; -import { IConfigCache, InMemoryConfigCache } from "#lib/ConfigCatCache"; +import { ExternalConfigCache, IConfigCache, InMemoryConfigCache } from "#lib/ConfigCatCache"; import { OptionsBase } from "#lib/ConfigCatClientOptions"; +import { LoggerWrapper } from "#lib/ConfigCatLogger"; import { FetchResult, IConfigFetcher, IFetchResponse } from "#lib/ConfigFetcher"; import { LazyLoadConfigService } from "#lib/LazyLoadConfigService"; import { ManualPollConfigService } from "#lib/ManualPollConfigService"; import { Config, ProjectConfig } from "#lib/ProjectConfig"; -import { delay } from "#lib/Utils"; +import { AbortToken, delay } from "#lib/Utils"; describe("ConfigServiceBaseTests", () => { @@ -310,6 +312,118 @@ describe("ConfigServiceBaseTests", () => { service.dispose(); }); + it("AutoPollConfigService - Should wait maxInitWaitTime in offline mode when external cache is expired", async () => { + + // Arrange + + const pollIntervalSeconds = 1; + const maxInitWaitTimeSeconds = 2.5; + + const frOld: FetchResult = createFetchResult("oldEtag"); + const projectConfigOld = createConfigFromFetchResult(frOld) + .with(ProjectConfig.generateTimestamp() - (1.5 * pollIntervalSeconds * 1000) + 0.5 * POLL_EXPIRATION_TOLERANCE_MS); + + const logger = new LoggerWrapper(new FakeLogger()); + const cache = new ExternalConfigCache(new FakeExternalCache(), logger); + + const options = createAutoPollOptions( + "APIKEY", + { + pollIntervalSeconds, + maxInitWaitTimeSeconds, + offline: true, + }, + createKernel({ defaultCacheFactory: () => cache }) + ); + + cache.set(options.getCacheKey(), projectConfigOld); + + const fetcherMock = new Mock(); + + // Act + + const service: AutoPollConfigService = new AutoPollConfigService( + fetcherMock.object(), + options); + + const { readyPromise } = service; + const delayAbortToken = new AbortToken(); + const delayPromise = delay(maxInitWaitTimeSeconds * 1000 - 250, delayAbortToken); + const raceResult = await Promise.race([readyPromise, delayPromise]); + delayAbortToken.abort(); + + // Assert + + assert.strictEqual(raceResult, true); + + // Cleanup + + service.dispose(); + }); + + it("AutoPollConfigService - Should initialize in offline mode when external cache becomes up-to-date", async () => { + + // Arrange + + const pollIntervalSeconds = 1; + const maxInitWaitTimeSeconds = 2.5; + const cacheSetDelayMs = 0.5 * pollIntervalSeconds * 1000; + + const frOld: FetchResult = createFetchResult("oldEtag"); + const projectConfigOld = createConfigFromFetchResult(frOld) + .with(ProjectConfig.generateTimestamp() - (1.5 * pollIntervalSeconds * 1000) + 0.5 * POLL_EXPIRATION_TOLERANCE_MS); + + const logger = new LoggerWrapper(new FakeLogger()); + const cache = new ExternalConfigCache(new FakeExternalCache(), logger); + + const options = createAutoPollOptions( + "APIKEY", + { + pollIntervalSeconds, + maxInitWaitTimeSeconds, + offline: true, + }, + createKernel({ defaultCacheFactory: () => cache }) + ); + + cache.set(options.getCacheKey(), projectConfigOld); + + const fetcherMock = new Mock(); + + // Act + + const service: AutoPollConfigService = new AutoPollConfigService( + fetcherMock.object(), + options); + + const { readyPromise } = service; + const delayAbortToken = new AbortToken(); + const delayPromise = delay(maxInitWaitTimeSeconds * 1000 - 250, delayAbortToken); + const racePromise = Promise.race([readyPromise, delayPromise]); + + const cacheSetDelayAbortToken = new AbortToken(); + const cacheSetDelayPromise = delay(cacheSetDelayMs, cacheSetDelayAbortToken); + const cacheSetRaceResult = await Promise.race([readyPromise, cacheSetDelayPromise]); + cacheSetDelayAbortToken.abort(); + assert.strictEqual(cacheSetRaceResult, true); + + const frNew: FetchResult = createFetchResult("newEtag"); + const projectConfigNew: ProjectConfig = createConfigFromFetchResult(frNew) + .with(ProjectConfig.generateTimestamp() + (pollIntervalSeconds * 1000) - cacheSetDelayMs + 0.5 * POLL_EXPIRATION_TOLERANCE_MS); + cache.set(options.getCacheKey(), projectConfigNew); + + const raceResult = await racePromise; + delayAbortToken.abort(); + + // Assert + + assert.strictEqual(raceResult, ClientCacheState.HasUpToDateFlagData); + + // Cleanup + + service.dispose(); + }); + it("LazyLoadConfigService - ProjectConfig is different in the cache - should fetch a new config and put into cache", async () => { // Arrange diff --git a/test/helpers/fakes.ts b/test/helpers/fakes.ts index 79966f3..3d905b6 100644 --- a/test/helpers/fakes.ts +++ b/test/helpers/fakes.ts @@ -71,7 +71,7 @@ export class FakeCache implements IConfigCache { } export class FakeExternalCache implements IConfigCatCache { - private cachedValue: string | undefined; + cachedValue: string | undefined; set(key: string, value: string): void { this.cachedValue = value; @@ -82,8 +82,17 @@ export class FakeExternalCache implements IConfigCatCache { } } +export class FaultyFakeExternalCache implements IConfigCatCache { + set(key: string, value: string): never { + throw new Error("Operation failed :("); + } + get(key: string): never { + throw new Error("Operation failed :("); + } +} + export class FakeExternalAsyncCache implements IConfigCatCache { - private cachedValue: string | undefined; + cachedValue: string | undefined; constructor(private readonly delayMs = 0) { } From 9e1ec802f4c7c68cb43d11628dce6ff87519fd89 Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Tue, 22 Apr 2025 23:42:57 +0200 Subject: [PATCH 5/9] Make clientReady consistent with other SDKs in Auto Poll + offline mode --- src/AutoPollConfigService.ts | 20 ++--- test/ConfigCatClientTests.ts | 5 +- test/ConfigServiceBaseTests.ts | 137 ++++++++++----------------------- 3 files changed, 54 insertions(+), 108 deletions(-) diff --git a/src/AutoPollConfigService.ts b/src/AutoPollConfigService.ts index 0ab1864..1631b3e 100644 --- a/src/AutoPollConfigService.ts +++ b/src/AutoPollConfigService.ts @@ -33,8 +33,9 @@ export class AutoPollConfigService extends ConfigServiceBase im this.initialized = false; // This promise will be resolved as soon as - // 1. an up-to-date config is obtained from the cache (see startRefreshWorker), - // 2. or a config fetch operation completes, regardless of success or failure (see onConfigUpdated). + // 1. the initial sync with the external cache completes (see startRefreshWorker), + // 2. and, in case the client is online and the local cache is still empty or expired, + // the first config fetch operation completes, regardless of success or failure (see onConfigFetched). const initSignalPromise = new Promise(resolve => this.signalInitialization = resolve); // This promise will be resolved when either initialization ready is signalled by signalInitialization() or maxInitWaitTimeSeconds pass. @@ -160,21 +161,20 @@ export class AutoPollConfigService extends ConfigServiceBase im this.options.logger.debug("AutoPollConfigService.refreshWorkerLogic() called."); const latestConfig = await (initialCacheSyncUp ?? this.options.cache.get(this.cacheKey)); - if (!latestConfig.isExpired(this.pollExpirationMs)) { - this.signalInitialization(); - return; - } - - if (initialCacheSyncUp ? !this.isOfflineExactly : !this.isOffline) { + if (latestConfig.isExpired(this.pollExpirationMs)) { // Even if the service gets disposed immediately, we allow the first refresh for backward compatibility, // i.e. to not break usage patterns like this: // ``` // client.getValueAsync("SOME_KEY", false, user).then(value => { /* ... */ }); // client.dispose(); // ``` - - await this.refreshConfigCoreAsync(latestConfig); + if (initialCacheSyncUp ? !this.isOfflineExactly : !this.isOffline) { + await this.refreshConfigCoreAsync(latestConfig); + return; // postpone signalling initialization until `onConfigFetched` + } } + + this.signalInitialization(); } getCacheState(cachedConfig: ProjectConfig): ClientCacheState { diff --git a/test/ConfigCatClientTests.ts b/test/ConfigCatClientTests.ts index 04238b1..673e1b1 100644 --- a/test/ConfigCatClientTests.ts +++ b/test/ConfigCatClientTests.ts @@ -1217,8 +1217,9 @@ describe("ConfigCatClient", () => { for (const [pollingMode, optionsFactory] of optionsFactoriesForOfflineModeTests) { it(`setOnline() should make a(n) ${PollingMode[pollingMode]} client created in offline mode transition to online mode`, async () => { + const configFetcherDelayMs = 100; - const configFetcher = new FakeConfigFetcherBase("{}", 100, (lastConfig, lastETag) => ({ + const configFetcher = new FakeConfigFetcherBase("{}", configFetcherDelayMs, (lastConfig, lastETag) => ({ statusCode: 200, reasonPhrase: "OK", eTag: (lastETag as any | 0) + 1 + "", @@ -1246,7 +1247,7 @@ describe("ConfigCatClient", () => { client.setOnline(); if (configService instanceof AutoPollConfigService) { - assert.isTrue(await configService["initializationPromise"]); + await delay(configFetcherDelayMs + 50); expectedFetchTimes++; } diff --git a/test/ConfigServiceBaseTests.ts b/test/ConfigServiceBaseTests.ts index d576141..331b39e 100644 --- a/test/ConfigServiceBaseTests.ts +++ b/test/ConfigServiceBaseTests.ts @@ -312,117 +312,62 @@ describe("ConfigServiceBaseTests", () => { service.dispose(); }); - it("AutoPollConfigService - Should wait maxInitWaitTime in offline mode when external cache is expired", async () => { + for (const expectedCacheState of [ClientCacheState.NoFlagData, ClientCacheState.HasCachedFlagDataOnly, ClientCacheState.HasUpToDateFlagData]) { + it(`AutoPollConfigService - Should emit clientReady in offline mode when sync with external cache is completed - expectedCacheState: ${expectedCacheState}`, async () => { - // Arrange - - const pollIntervalSeconds = 1; - const maxInitWaitTimeSeconds = 2.5; - - const frOld: FetchResult = createFetchResult("oldEtag"); - const projectConfigOld = createConfigFromFetchResult(frOld) - .with(ProjectConfig.generateTimestamp() - (1.5 * pollIntervalSeconds * 1000) + 0.5 * POLL_EXPIRATION_TOLERANCE_MS); - - const logger = new LoggerWrapper(new FakeLogger()); - const cache = new ExternalConfigCache(new FakeExternalCache(), logger); - - const options = createAutoPollOptions( - "APIKEY", - { - pollIntervalSeconds, - maxInitWaitTimeSeconds, - offline: true, - }, - createKernel({ defaultCacheFactory: () => cache }) - ); + // Arrange - cache.set(options.getCacheKey(), projectConfigOld); + const pollIntervalSeconds = 1; - const fetcherMock = new Mock(); - - // Act - - const service: AutoPollConfigService = new AutoPollConfigService( - fetcherMock.object(), - options); + let projectConfig: ProjectConfig | undefined; + if (expectedCacheState !== ClientCacheState.NoFlagData) { + const fr: FetchResult = createFetchResult("oldEtag"); + projectConfig = createConfigFromFetchResult(fr); - const { readyPromise } = service; - const delayAbortToken = new AbortToken(); - const delayPromise = delay(maxInitWaitTimeSeconds * 1000 - 250, delayAbortToken); - const raceResult = await Promise.race([readyPromise, delayPromise]); - delayAbortToken.abort(); - - // Assert - - assert.strictEqual(raceResult, true); - - // Cleanup - - service.dispose(); - }); + if (expectedCacheState === ClientCacheState.HasCachedFlagDataOnly) { + projectConfig = projectConfig + .with(ProjectConfig.generateTimestamp() - (1.5 * pollIntervalSeconds * 1000) + 0.5 * POLL_EXPIRATION_TOLERANCE_MS); + } + } - it("AutoPollConfigService - Should initialize in offline mode when external cache becomes up-to-date", async () => { + const logger = new LoggerWrapper(new FakeLogger()); + const cache = new ExternalConfigCache(new FakeExternalCache(), logger); - // Arrange - - const pollIntervalSeconds = 1; - const maxInitWaitTimeSeconds = 2.5; - const cacheSetDelayMs = 0.5 * pollIntervalSeconds * 1000; - - const frOld: FetchResult = createFetchResult("oldEtag"); - const projectConfigOld = createConfigFromFetchResult(frOld) - .with(ProjectConfig.generateTimestamp() - (1.5 * pollIntervalSeconds * 1000) + 0.5 * POLL_EXPIRATION_TOLERANCE_MS); - - const logger = new LoggerWrapper(new FakeLogger()); - const cache = new ExternalConfigCache(new FakeExternalCache(), logger); - - const options = createAutoPollOptions( - "APIKEY", - { - pollIntervalSeconds, - maxInitWaitTimeSeconds, - offline: true, - }, - createKernel({ defaultCacheFactory: () => cache }) - ); + const options = createAutoPollOptions( + "APIKEY", + { + pollIntervalSeconds, + offline: true, + }, + createKernel({ defaultCacheFactory: () => cache }) + ); - cache.set(options.getCacheKey(), projectConfigOld); + if (projectConfig) { + cache.set(options.getCacheKey(), projectConfig); + } - const fetcherMock = new Mock(); + const fetcherMock = new Mock(); - // Act + // Act - const service: AutoPollConfigService = new AutoPollConfigService( - fetcherMock.object(), - options); + const service: AutoPollConfigService = new AutoPollConfigService( + fetcherMock.object(), + options); - const { readyPromise } = service; - const delayAbortToken = new AbortToken(); - const delayPromise = delay(maxInitWaitTimeSeconds * 1000 - 250, delayAbortToken); - const racePromise = Promise.race([readyPromise, delayPromise]); + const { readyPromise } = service; + const delayAbortToken = new AbortToken(); + const delayPromise = delay(pollIntervalSeconds * 1000 - 250, delayAbortToken); + const raceResult = await Promise.race([readyPromise, delayPromise]); - const cacheSetDelayAbortToken = new AbortToken(); - const cacheSetDelayPromise = delay(cacheSetDelayMs, cacheSetDelayAbortToken); - const cacheSetRaceResult = await Promise.race([readyPromise, cacheSetDelayPromise]); - cacheSetDelayAbortToken.abort(); - assert.strictEqual(cacheSetRaceResult, true); + // Assert - const frNew: FetchResult = createFetchResult("newEtag"); - const projectConfigNew: ProjectConfig = createConfigFromFetchResult(frNew) - .with(ProjectConfig.generateTimestamp() + (pollIntervalSeconds * 1000) - cacheSetDelayMs + 0.5 * POLL_EXPIRATION_TOLERANCE_MS); - cache.set(options.getCacheKey(), projectConfigNew); + assert.strictEqual(raceResult, expectedCacheState); - const raceResult = await racePromise; - delayAbortToken.abort(); + // Cleanup - // Assert - - assert.strictEqual(raceResult, ClientCacheState.HasUpToDateFlagData); - - // Cleanup - - service.dispose(); - }); + service.dispose(); + }); + } it("LazyLoadConfigService - ProjectConfig is different in the cache - should fetch a new config and put into cache", async () => { From eafddc62a6a806d78d1f37d2a96dc26dd8083491 Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Wed, 23 Apr 2025 18:54:44 +0200 Subject: [PATCH 6/9] Correct hook intellisense docs --- src/ConfigCatClient.ts | 12 ++++++++++-- src/Hooks.ts | 11 ++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/ConfigCatClient.ts b/src/ConfigCatClient.ts index a3c5670..ae9a870 100644 --- a/src/ConfigCatClient.ts +++ b/src/ConfigCatClient.ts @@ -85,8 +85,16 @@ export interface IConfigCatClient extends IProvidesHooks { forceRefreshAsync(): Promise; /** - * Waits for the client initialization. - * @returns A promise that fulfills with the client's initialization state. + * Waits for the client to reach the ready state, i.e. to complete initialization. + * + * @remarks Ready state is reached as soon as the initial sync with the external cache (if any) completes. + * If this does not provide up-to-date config data, and the client is online (i.e. HTTP requests are allowed), + * the first config fetch operation is also awaited in Auto Poll mode before ready state is reported. + * + * That is, reaching the ready state usually means that the client is ready to evaluate feature flags. + * However, please note that it is not guaranteed. You can determine this by checking the return value. + * + * @returns A promise that fulfills with the state of the local cache at the time initialization was completed. */ waitForReady(): Promise; diff --git a/src/Hooks.ts b/src/Hooks.ts index 4867913..cd82235 100644 --- a/src/Hooks.ts +++ b/src/Hooks.ts @@ -6,7 +6,16 @@ import type { IEvaluationDetails } from "./RolloutEvaluator"; /** Hooks (events) that can be emitted by `ConfigCatClient`. */ export type HookEvents = { - /** Occurs when the client is ready to provide the actual value of feature flags or settings. */ + /** + * Occurs when the client reaches the ready state, i.e. completes initialization. + * + * @remarks Ready state is reached as soon as the initial sync with the external cache (if any) completes. + * If this does not produce up-to-date config data, and the client is online (i.e. HTTP requests are allowed), + * the first config fetch operation is also awaited in Auto Poll mode before ready state is reported. + * + * That is, reaching the ready state usually means that the client is ready to evaluate feature flags. + * However, please note that it is not guaranteed. You can determine this by checking the `cacheState` parameter. + */ clientReady: [cacheState: ClientCacheState]; /** Occurs after the value of a feature flag of setting has been evaluated. */ flagEvaluated: [evaluationDetails: IEvaluationDetails]; From fc4fe3be0aaf65609321dfd5814f1207be52ce8e Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Wed, 23 Apr 2025 00:06:43 +0200 Subject: [PATCH 7/9] Eliminate race condition between initial and user-initiated cache sync ups --- src/AutoPollConfigService.ts | 6 +++--- src/ConfigServiceBase.ts | 18 +++++++++--------- src/LazyLoadConfigService.ts | 2 +- src/ManualPollConfigService.ts | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/AutoPollConfigService.ts b/src/AutoPollConfigService.ts index 1631b3e..df17411 100644 --- a/src/AutoPollConfigService.ts +++ b/src/AutoPollConfigService.ts @@ -82,7 +82,7 @@ export class AutoPollConfigService extends ConfigServiceBase im let cachedConfig: ProjectConfig; if (!this.initialized) { - cachedConfig = await this.options.cache.get(this.cacheKey); + cachedConfig = await this.syncUpWithCache(); if (!cachedConfig.isExpired(this.pollIntervalMs)) { logSuccess(this.options.logger); return cachedConfig; @@ -92,7 +92,7 @@ export class AutoPollConfigService extends ConfigServiceBase im await this.initializationPromise; } - cachedConfig = await this.options.cache.get(this.cacheKey); + cachedConfig = await this.syncUpWithCache(); if (!cachedConfig.isExpired(this.pollIntervalMs)) { logSuccess(this.options.logger); } else { @@ -160,7 +160,7 @@ export class AutoPollConfigService extends ConfigServiceBase im private async refreshWorkerLogic(initialCacheSyncUp: ProjectConfig | Promise | null) { this.options.logger.debug("AutoPollConfigService.refreshWorkerLogic() called."); - const latestConfig = await (initialCacheSyncUp ?? this.options.cache.get(this.cacheKey)); + const latestConfig = await (initialCacheSyncUp ?? this.syncUpWithCache()); if (latestConfig.isExpired(this.pollExpirationMs)) { // Even if the service gets disposed immediately, we allow the first refresh for backward compatibility, // i.e. to not break usage patterns like this: diff --git a/src/ConfigServiceBase.ts b/src/ConfigServiceBase.ts index 7bf9747..7104b84 100644 --- a/src/ConfigServiceBase.ts +++ b/src/ConfigServiceBase.ts @@ -4,6 +4,7 @@ import type { FetchErrorCauses, IConfigFetcher, IFetchResponse } from "./ConfigF import { FetchError, FetchResult, FetchStatus } from "./ConfigFetcher"; import { RedirectMode } from "./ConfigJson"; import { Config, ProjectConfig } from "./ProjectConfig"; +import { isPromiseLike } from "./Utils"; /** Contains the result of an `IConfigCatClient.forceRefresh` or `IConfigCatClient.forceRefreshAsync` operation. */ export class RefreshResult { @@ -79,6 +80,7 @@ export abstract class ConfigServiceBase { protected readonly cacheKey: string; + private pendingCacheSyncUp: Promise | null = null; abstract readonly readyPromise: Promise; constructor( @@ -103,7 +105,7 @@ export abstract class ConfigServiceBase { abstract getConfig(): Promise; async refreshConfigAsync(): Promise<[RefreshResult, ProjectConfig]> { - const latestConfig = await this.options.cache.get(this.cacheKey); + const latestConfig = await this.syncUpWithCache(); if (!this.isOffline) { const [fetchResult, config] = await this.refreshConfigCoreAsync(latestConfig); return [RefreshResult.from(fetchResult), config]; @@ -147,13 +149,8 @@ export abstract class ConfigServiceBase { } private fetchAsync(lastConfig: ProjectConfig): Promise { - return this.pendingFetch ??= (async () => { - try { - return await this.fetchLogicAsync(lastConfig); - } finally { - this.pendingFetch = null; - } - })(); + return this.pendingFetch ??= + this.fetchLogicAsync(lastConfig).finally(() => this.pendingFetch = null); } private async fetchLogicAsync(lastConfig: ProjectConfig): Promise { @@ -305,7 +302,10 @@ export abstract class ConfigServiceBase { abstract getCacheState(cachedConfig: ProjectConfig): ClientCacheState; protected syncUpWithCache(): ProjectConfig | Promise { - return this.options.cache.get(this.cacheKey); + let cacheSyncUpResult: ProjectConfig | Promise; + return this.pendingCacheSyncUp ? this.pendingCacheSyncUp + : !isPromiseLike(cacheSyncUpResult = this.options.cache.get(this.cacheKey)) ? cacheSyncUpResult + : (this.pendingCacheSyncUp = cacheSyncUpResult.finally(() => this.pendingCacheSyncUp = null)); } protected async waitForReadyAsync(initialCacheSyncUp: ProjectConfig | Promise): Promise { diff --git a/src/LazyLoadConfigService.ts b/src/LazyLoadConfigService.ts index 34d88c7..58ad5ba 100644 --- a/src/LazyLoadConfigService.ts +++ b/src/LazyLoadConfigService.ts @@ -27,7 +27,7 @@ export class LazyLoadConfigService extends ConfigServiceBase im logger.debug(`LazyLoadConfigService.getConfig(): cache is empty or expired${appendix}.`); } - let cachedConfig = await this.options.cache.get(this.cacheKey); + let cachedConfig = await this.syncUpWithCache(); if (cachedConfig.isExpired(this.cacheTimeToLiveMs)) { if (!this.isOffline) { diff --git a/src/ManualPollConfigService.ts b/src/ManualPollConfigService.ts index c3d50a8..08d79cb 100644 --- a/src/ManualPollConfigService.ts +++ b/src/ManualPollConfigService.ts @@ -26,7 +26,7 @@ export class ManualPollConfigService extends ConfigServiceBase { this.options.logger.debug("ManualPollService.getConfig() called."); - return await this.options.cache.get(this.cacheKey); + return await this.syncUpWithCache(); } refreshConfigAsync(): Promise<[RefreshResult, ProjectConfig]> { From 648ecc1d2120ec61c95e6b56e49790130629f2c8 Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Wed, 23 Apr 2025 17:52:29 +0200 Subject: [PATCH 8/9] Also report configCached when new config is synced from external cache --- src/ConfigCatCache.ts | 27 ++++++--- src/ConfigServiceBase.ts | 33 +++++++++-- src/Hooks.ts | 5 +- test/ConfigServiceBaseTests.ts | 101 +++++++++++++++++++++++++++++++-- 4 files changed, 145 insertions(+), 21 deletions(-) diff --git a/src/ConfigCatCache.ts b/src/ConfigCatCache.ts index 8345cf4..6ba01c9 100644 --- a/src/ConfigCatCache.ts +++ b/src/ConfigCatCache.ts @@ -19,10 +19,13 @@ export interface IConfigCatCache { get(key: string): Promise | string | null | undefined; } +/** @remarks Unchanged config is returned as is, changed config is wrapped in an array so we can distinguish between the two cases. */ +export type CacheSyncResult = ProjectConfig | [changedConfig: ProjectConfig]; + export interface IConfigCache { set(key: string, config: ProjectConfig): Promise | void; - get(key: string): Promise | ProjectConfig; + get(key: string): Promise | CacheSyncResult; getInMemory(): ProjectConfig; } @@ -71,39 +74,47 @@ export class ExternalConfigCache implements IConfigCache { } } - private updateCachedConfig(externalSerializedConfig: string | null | undefined): void { + private updateCachedConfig(externalSerializedConfig: string | null | undefined): CacheSyncResult { if (externalSerializedConfig == null || externalSerializedConfig === this.cachedSerializedConfig) { - return; + return this.cachedConfig; } this.cachedConfig = ProjectConfig.deserialize(externalSerializedConfig); this.cachedSerializedConfig = externalSerializedConfig; + return [this.cachedConfig]; } - get(key: string): Promise { + get(key: string): Promise | CacheSyncResult { + let cacheSyncResult: CacheSyncResult; + try { const cacheGetResult = this.cache.get(key); // Take the async path only when the IConfigCatCache.get operation is asynchronous. if (isPromiseLike(cacheGetResult)) { return (async (cacheGetPromise) => { + let cacheSyncResult: CacheSyncResult; + try { - this.updateCachedConfig(await cacheGetPromise); + cacheSyncResult = this.updateCachedConfig(await cacheGetPromise); } catch (err) { + cacheSyncResult = this.cachedConfig; this.logger.configServiceCacheReadError(err); } - return this.cachedConfig; + + return cacheSyncResult; })(cacheGetResult); } // Otherwise, keep the code flow synchronous so the config services can sync up // with the cache in their ctors synchronously (see ConfigServiceBase.syncUpWithCache). - this.updateCachedConfig(cacheGetResult); + cacheSyncResult = this.updateCachedConfig(cacheGetResult); } catch (err) { + cacheSyncResult = this.cachedConfig; this.logger.configServiceCacheReadError(err); } - return Promise.resolve(this.cachedConfig); + return cacheSyncResult; } getInMemory(): ProjectConfig { diff --git a/src/ConfigServiceBase.ts b/src/ConfigServiceBase.ts index 7104b84..d1f9644 100644 --- a/src/ConfigServiceBase.ts +++ b/src/ConfigServiceBase.ts @@ -1,3 +1,4 @@ +import type { CacheSyncResult } from "./ConfigCatCache"; import { ExternalConfigCache } from "./ConfigCatCache"; import type { OptionsBase } from "./ConfigCatClientOptions"; import type { FetchErrorCauses, IConfigFetcher, IFetchResponse } from "./ConfigFetcher"; @@ -149,8 +150,8 @@ export abstract class ConfigServiceBase { } private fetchAsync(lastConfig: ProjectConfig): Promise { - return this.pendingFetch ??= - this.fetchLogicAsync(lastConfig).finally(() => this.pendingFetch = null); + return this.pendingFetch ??= this.fetchLogicAsync(lastConfig) + .finally(() => this.pendingFetch = null); } private async fetchLogicAsync(lastConfig: ProjectConfig): Promise { @@ -302,10 +303,30 @@ export abstract class ConfigServiceBase { abstract getCacheState(cachedConfig: ProjectConfig): ClientCacheState; protected syncUpWithCache(): ProjectConfig | Promise { - let cacheSyncUpResult: ProjectConfig | Promise; - return this.pendingCacheSyncUp ? this.pendingCacheSyncUp - : !isPromiseLike(cacheSyncUpResult = this.options.cache.get(this.cacheKey)) ? cacheSyncUpResult - : (this.pendingCacheSyncUp = cacheSyncUpResult.finally(() => this.pendingCacheSyncUp = null)); + if (this.pendingCacheSyncUp) { + return this.pendingCacheSyncUp; + } + + const cacheSyncResult = this.options.cache.get(this.cacheKey); + if (isPromiseLike(cacheSyncResult)) { + return this.pendingCacheSyncUp = cacheSyncResult + .finally(() => this.pendingCacheSyncUp = null) + .then(syncResult => this.onCacheSynced(syncResult)); + } + + return this.onCacheSynced(cacheSyncResult); + } + + private onCacheSynced(syncResult: CacheSyncResult): ProjectConfig { + if (!Array.isArray(syncResult)) { + return syncResult; + } + + const [newConfig] = syncResult; + if (!newConfig.isEmpty) { + this.onConfigChanged(newConfig); + } + return newConfig; } protected async waitForReadyAsync(initialCacheSyncUp: ProjectConfig | Promise): Promise { diff --git a/src/Hooks.ts b/src/Hooks.ts index cd82235..fd3909f 100644 --- a/src/Hooks.ts +++ b/src/Hooks.ts @@ -19,7 +19,10 @@ export type HookEvents = { clientReady: [cacheState: ClientCacheState]; /** Occurs after the value of a feature flag of setting has been evaluated. */ flagEvaluated: [evaluationDetails: IEvaluationDetails]; - /** Occurs after the locally cached config has been updated. */ + /** + * Occurs after the locally cached config has been updated to a newer version, either as a result of synchronization + * with the external cache, or as a result of fetching a newer version from the remote server. + */ configChanged: [newConfig: IConfig]; /** Occurs in the case of a failure in the client. */ clientError: [message: string, exception?: any]; diff --git a/test/ConfigServiceBaseTests.ts b/test/ConfigServiceBaseTests.ts index 331b39e..56c27f2 100644 --- a/test/ConfigServiceBaseTests.ts +++ b/test/ConfigServiceBaseTests.ts @@ -3,7 +3,7 @@ import { assert } from "chai"; import { EqualMatchingInjectorConfig, It, Mock, RejectedPromiseFactory, ResolvedPromiseFactory, Times } from "moq.ts"; import { MimicsRejectedAsyncPresetFactory, MimicsResolvedAsyncPresetFactory, Presets, ReturnsAsyncPresetFactory, RootMockProvider, ThrowsAsyncPresetFactory } from "moq.ts/internal"; /* eslint-enable import/no-duplicates */ -import { createAutoPollOptions, createKernel, createLazyLoadOptions, createManualPollOptions, FakeCache, FakeExternalCache, FakeLogger } from "./helpers/fakes"; +import { createAutoPollOptions, createKernel, createLazyLoadOptions, createManualPollOptions, FakeCache, FakeExternalAsyncCache, FakeExternalCache, FakeLogger } from "./helpers/fakes"; import { ClientCacheState } from "#lib"; import { AutoPollConfigService, POLL_EXPIRATION_TOLERANCE_MS } from "#lib/AutoPollConfigService"; import { ExternalConfigCache, IConfigCache, InMemoryConfigCache } from "#lib/ConfigCatCache"; @@ -12,7 +12,7 @@ import { LoggerWrapper } from "#lib/ConfigCatLogger"; import { FetchResult, IConfigFetcher, IFetchResponse } from "#lib/ConfigFetcher"; import { LazyLoadConfigService } from "#lib/LazyLoadConfigService"; import { ManualPollConfigService } from "#lib/ManualPollConfigService"; -import { Config, ProjectConfig } from "#lib/ProjectConfig"; +import { Config, IConfig, ProjectConfig } from "#lib/ProjectConfig"; import { AbortToken, delay } from "#lib/Utils"; describe("ConfigServiceBaseTests", () => { @@ -369,6 +369,93 @@ describe("ConfigServiceBaseTests", () => { }); } + for (const useSyncCache of [false, true]) { + it(`AutoPollConfigService - Should refresh local cache in offline mode and report configChanged when new config is synced from external cache - useSyncCache: ${useSyncCache}`, async () => { + + // Arrange + + const pollIntervalSeconds = 1; + + const logger = new LoggerWrapper(new FakeLogger()); + const fakeExternalCache = useSyncCache ? new FakeExternalCache() : new FakeExternalAsyncCache(50); + const cache = new ExternalConfigCache(fakeExternalCache, logger); + + const clientReadyEvents: ClientCacheState[] = []; + const configChangedEvents: IConfig[] = []; + + const options = createAutoPollOptions( + "APIKEY", + { + pollIntervalSeconds, + setupHooks: hooks => { + hooks.on("clientReady", cacheState => clientReadyEvents.push(cacheState)); + hooks.on("configChanged", config => configChangedEvents.push(config)); + }, + }, + createKernel({ defaultCacheFactory: () => cache }) + ); + + const fr: FetchResult = createFetchResult(); + const fetcherMock = new Mock() + .setup(m => m.fetchLogic(It.IsAny(), It.IsAny())) + .returnsAsync({ statusCode: 200, reasonPhrase: "OK", eTag: fr.config.httpETag, body: fr.config.configJson }); + + // Act + + const service: AutoPollConfigService = new AutoPollConfigService( + fetcherMock.object(), + options); + + assert.isUndefined(fakeExternalCache.cachedValue); + + assert.isEmpty(clientReadyEvents); + assert.isEmpty(configChangedEvents); + + await service.readyPromise; + + const getConfigPromise = service.getConfig(); + await service.getConfig(); // simulate concurrent cache sync up + await getConfigPromise; + + await delay(100); // allow a little time for the client to raise ConfigChanged + + assert.isDefined(fakeExternalCache.cachedValue); + + assert.strictEqual(1, clientReadyEvents.length); + assert.strictEqual(ClientCacheState.HasUpToDateFlagData, clientReadyEvents[0]); + assert.strictEqual(1, configChangedEvents.length); + assert.strictEqual(JSON.stringify(fr.config.config), JSON.stringify(configChangedEvents[0])); + + fetcherMock.verify(m => m.fetchLogic(It.IsAny(), It.IsAny()), Times.Once()); + + service.setOffline(); // no HTTP fetching from this point on + + await delay(pollIntervalSeconds * 1000 + 50); + + assert.strictEqual(1, clientReadyEvents.length); + assert.strictEqual(1, configChangedEvents.length); + + const fr2: FetchResult = createFetchResult("etag2", "{}"); + const projectConfig2 = createConfigFromFetchResult(fr2); + fakeExternalCache.cachedValue = ProjectConfig.serialize(projectConfig2); + + const [refreshResult, projectConfigFromRefresh] = await service.refreshConfigAsync(); + + // Assert + + assert.isTrue(refreshResult.isSuccess); + + assert.strictEqual(1, clientReadyEvents.length); + assert.strictEqual(2, configChangedEvents.length); + assert.strictEqual(JSON.stringify(fr2.config.config), JSON.stringify(configChangedEvents[1])); + assert.strictEqual(configChangedEvents[1], projectConfigFromRefresh.config); + + // Cleanup + + service.dispose(); + }); + } + it("LazyLoadConfigService - ProjectConfig is different in the cache - should fetch a new config and put into cache", async () => { // Arrange @@ -836,8 +923,10 @@ describe("ConfigServiceBaseTests", () => { }); }); -function createProjectConfig(eTag = "etag"): ProjectConfig { - const configJson = "{\"f\": { \"debug\": { \"v\": { \"b\": true }, \"i\": \"abcdefgh\", \"t\": 0, \"p\": [], \"r\": [] } } }"; +const DEFAULT_ETAG = "etag"; +const DEFAULT_CONFIG_JSON = '{"f": { "debug": { "v": { "b": true }, "i": "abcdefgh", "t": 0, "p": [], "r": [] } } }'; + +function createProjectConfig(eTag = DEFAULT_ETAG, configJson = DEFAULT_CONFIG_JSON): ProjectConfig { return new ProjectConfig( configJson, Config.deserialize(configJson), @@ -845,8 +934,8 @@ function createProjectConfig(eTag = "etag"): ProjectConfig { eTag); } -function createFetchResult(eTag = "etag"): FetchResult { - return FetchResult.success(createProjectConfig(eTag)); +function createFetchResult(eTag = DEFAULT_ETAG, configJson = DEFAULT_CONFIG_JSON): FetchResult { + return FetchResult.success(createProjectConfig(eTag, configJson)); } function createConfigFromFetchResult(result: FetchResult): ProjectConfig { From 220d8ca9cf12cca63cf8bc8ab9de8802e6e9d0d0 Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Wed, 23 Apr 2025 19:28:06 +0200 Subject: [PATCH 9/9] Introduce ConfigFetched hook --- src/AutoPollConfigService.ts | 8 ++++---- src/ConfigServiceBase.ts | 9 +++++---- src/Hooks.ts | 6 +++++- src/LazyLoadConfigService.ts | 2 +- test/ConfigCatClientTests.ts | 10 ++++++++++ test/DataGovernanceTests.ts | 2 +- 6 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/AutoPollConfigService.ts b/src/AutoPollConfigService.ts index df17411..e6e83ca 100644 --- a/src/AutoPollConfigService.ts +++ b/src/AutoPollConfigService.ts @@ -1,6 +1,6 @@ import type { AutoPollOptions } from "./ConfigCatClientOptions"; import type { LoggerWrapper } from "./ConfigCatLogger"; -import type { IConfigFetcher } from "./ConfigFetcher"; +import type { FetchResult, IConfigFetcher } from "./ConfigFetcher"; import type { IConfigService, RefreshResult } from "./ConfigServiceBase"; import { ClientCacheState, ConfigServiceBase } from "./ConfigServiceBase"; import type { ProjectConfig } from "./ProjectConfig"; @@ -115,9 +115,9 @@ export class AutoPollConfigService extends ConfigServiceBase im } } - protected onConfigFetched(newConfig: ProjectConfig): void { + protected onConfigFetched(fetchResult: FetchResult, isInitiatedByUser: boolean): void { this.signalInitialization(); - super.onConfigFetched(newConfig); + super.onConfigFetched(fetchResult, isInitiatedByUser); } protected goOnline(): void { @@ -169,7 +169,7 @@ export class AutoPollConfigService extends ConfigServiceBase im // client.dispose(); // ``` if (initialCacheSyncUp ? !this.isOfflineExactly : !this.isOffline) { - await this.refreshConfigCoreAsync(latestConfig); + await this.refreshConfigCoreAsync(latestConfig, false); return; // postpone signalling initialization until `onConfigFetched` } } diff --git a/src/ConfigServiceBase.ts b/src/ConfigServiceBase.ts index d1f9644..c24fb56 100644 --- a/src/ConfigServiceBase.ts +++ b/src/ConfigServiceBase.ts @@ -108,7 +108,7 @@ export abstract class ConfigServiceBase { async refreshConfigAsync(): Promise<[RefreshResult, ProjectConfig]> { const latestConfig = await this.syncUpWithCache(); if (!this.isOffline) { - const [fetchResult, config] = await this.refreshConfigCoreAsync(latestConfig); + const [fetchResult, config] = await this.refreshConfigCoreAsync(latestConfig, true); return [RefreshResult.from(fetchResult), config]; } else if (this.options.cache instanceof ExternalConfigCache) { return [RefreshResult.success(), latestConfig]; @@ -118,7 +118,7 @@ export abstract class ConfigServiceBase { } } - protected async refreshConfigCoreAsync(latestConfig: ProjectConfig): Promise<[FetchResult, ProjectConfig]> { + protected async refreshConfigCoreAsync(latestConfig: ProjectConfig, isInitiatedByUser: boolean): Promise<[FetchResult, ProjectConfig]> { const fetchResult = await this.fetchAsync(latestConfig); let configChanged = false; @@ -131,7 +131,7 @@ export abstract class ConfigServiceBase { latestConfig = fetchResult.config; } - this.onConfigFetched(fetchResult.config); + this.onConfigFetched(fetchResult, isInitiatedByUser); if (configChanged) { this.onConfigChanged(fetchResult.config); @@ -140,8 +140,9 @@ export abstract class ConfigServiceBase { return [fetchResult, latestConfig]; } - protected onConfigFetched(newConfig: ProjectConfig): void { + protected onConfigFetched(fetchResult: FetchResult, isInitiatedByUser: boolean): void { this.options.logger.debug("config fetched"); + this.options.hooks.emit("configFetched", RefreshResult.from(fetchResult), isInitiatedByUser); } protected onConfigChanged(newConfig: ProjectConfig): void { diff --git a/src/Hooks.ts b/src/Hooks.ts index fd3909f..3bceb6c 100644 --- a/src/Hooks.ts +++ b/src/Hooks.ts @@ -1,4 +1,4 @@ -import type { ClientCacheState } from "./ConfigServiceBase"; +import type { ClientCacheState, RefreshResult } from "./ConfigServiceBase"; import type { IEventEmitter, IEventProvider } from "./EventEmitter"; import { NullEventEmitter } from "./EventEmitter"; import type { IConfig } from "./ProjectConfig"; @@ -19,6 +19,10 @@ export type HookEvents = { clientReady: [cacheState: ClientCacheState]; /** Occurs after the value of a feature flag of setting has been evaluated. */ flagEvaluated: [evaluationDetails: IEvaluationDetails]; + /** + * Occurs after attempting to refresh the locally cached config by fetching the latest version from the remote server. + */ + configFetched: [result: RefreshResult, isInitiatedByUser: boolean]; /** * Occurs after the locally cached config has been updated to a newer version, either as a result of synchronization * with the external cache, or as a result of fetching a newer version from the remote server. diff --git a/src/LazyLoadConfigService.ts b/src/LazyLoadConfigService.ts index 58ad5ba..d3b70bc 100644 --- a/src/LazyLoadConfigService.ts +++ b/src/LazyLoadConfigService.ts @@ -32,7 +32,7 @@ export class LazyLoadConfigService extends ConfigServiceBase im if (cachedConfig.isExpired(this.cacheTimeToLiveMs)) { if (!this.isOffline) { logExpired(this.options.logger, ", calling refreshConfigCoreAsync()"); - [, cachedConfig] = await this.refreshConfigCoreAsync(cachedConfig); + [, cachedConfig] = await this.refreshConfigCoreAsync(cachedConfig, false); } else { logExpired(this.options.logger); } diff --git a/test/ConfigCatClientTests.ts b/test/ConfigCatClientTests.ts index 673e1b1..0d8b366 100644 --- a/test/ConfigCatClientTests.ts +++ b/test/ConfigCatClientTests.ts @@ -1355,17 +1355,20 @@ describe("ConfigCatClient", () => { for (const addListenersViaOptions of [false, true]) { it(`ConfigCatClient should emit events, which listeners added ${addListenersViaOptions ? "via options" : "directly on the client"} should get notified of`, async () => { let clientReadyEventCount = 0; + const configFetchedEvents: [RefreshResult, boolean][] = []; const configChangedEvents: IConfig[] = []; const flagEvaluatedEvents: IEvaluationDetails[] = []; const errorEvents: [string, any][] = []; const handleClientReady = () => clientReadyEventCount++; + const handleConfigFetched = (result: RefreshResult, isInitiatedByUser: boolean) => configFetchedEvents.push([result, isInitiatedByUser]); const handleConfigChanged = (pc: IConfig) => configChangedEvents.push(pc); const handleFlagEvaluated = (ed: IEvaluationDetails) => flagEvaluatedEvents.push(ed); const handleClientError = (msg: string, err: any) => errorEvents.push([msg, err]); function setupHooks(hooks: IProvidesHooks) { hooks.on("clientReady", handleClientReady); + hooks.on("configFetched", handleConfigFetched); hooks.on("configChanged", handleConfigChanged); hooks.on("flagEvaluated", handleFlagEvaluated); hooks.on("clientError", handleClientError); @@ -1391,6 +1394,7 @@ describe("ConfigCatClient", () => { assert.equal(state, ClientCacheState.NoFlagData); assert.equal(clientReadyEventCount, 1); + assert.equal(configFetchedEvents.length, 0); assert.equal(configChangedEvents.length, 0); assert.equal(flagEvaluatedEvents.length, 0); assert.equal(errorEvents.length, 0); @@ -1410,6 +1414,7 @@ describe("ConfigCatClient", () => { await client.forceRefreshAsync(); + assert.equal(configFetchedEvents.length, 0); assert.equal(configChangedEvents.length, 0); assert.equal(errorEvents.length, 1); const [actualErrorMessage, actualErrorException] = errorEvents[0]; @@ -1422,6 +1427,10 @@ describe("ConfigCatClient", () => { await client.forceRefreshAsync(); const cachedPc = await configCache.get(""); + assert.equal(configFetchedEvents.length, 1); + const [refreshResult, isInitiatedByUser] = configFetchedEvents[0]; + assert.isTrue(isInitiatedByUser); + assert.isTrue(refreshResult.isSuccess); assert.equal(configChangedEvents.length, 1); assert.strictEqual(configChangedEvents[0], cachedPc.config); @@ -1438,6 +1447,7 @@ describe("ConfigCatClient", () => { // 5. Client gets disposed client.dispose(); + assert.equal(configFetchedEvents.length, 1); assert.equal(clientReadyEventCount, 1); assert.equal(configChangedEvents.length, 1); assert.equal(evaluationDetails.length, flagEvaluatedEvents.length); diff --git a/test/DataGovernanceTests.ts b/test/DataGovernanceTests.ts index f58093d..332049d 100644 --- a/test/DataGovernanceTests.ts +++ b/test/DataGovernanceTests.ts @@ -293,7 +293,7 @@ export class FakeConfigServiceBase extends ConfigServiceBase { getConfig(): Promise { return Promise.resolve(ProjectConfig.empty); } refreshLogicAsync(): Promise<[FetchResult, ProjectConfig]> { - return this.refreshConfigCoreAsync(ProjectConfig.empty); + return this.refreshConfigCoreAsync(ProjectConfig.empty, false); } prepareResponse(baseUrl: string, jsonBaseUrl: string, jsonRedirect: number, jsonFeatureFlags: any): void {