Also report configCached when new config is synced from external cache

adams85 · adams85 · commit 18edcbd27c30 · 2025-05-05T17:57:58.000+02:00
diff --git a/src/ConfigCatCache.ts b/src/ConfigCatCache.ts
@@ -19,10 +19,13 @@ export interface IConfigCatCache {
   get(key: string): Promise<string | null | undefined> | 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> | void;
 
-  get(key: string): Promise<ProjectConfig> | ProjectConfig;
+  get(key: string): Promise<CacheSyncResult> | 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<ProjectConfig> {
+  get(key: string): Promise<CacheSyncResult> | 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
@@ -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";
@@ -308,16 +309,31 @@ export abstract class ConfigServiceBase<TOptions extends OptionsBase> {
 
     const syncResult = this.options.cache.get(this.cacheKey);
     if (!isPromiseLike(syncResult)) {
-      return syncResult;
+      return this.onCacheSynced(syncResult);
     }
 
     const syncUpAndFinish = syncResult
-      .finally(() => this.pendingCacheSyncUp = null);
+      .finally(() => this.pendingCacheSyncUp = null)
+      .then(syncResult => this.onCacheSynced(syncResult));
+
+    this.pendingCacheSyncUp = syncResult
+      .then(syncResult => !Array.isArray(syncResult) ? syncResult : syncResult[0]);
 
-    this.pendingCacheSyncUp = syncResult;
     return syncUpAndFinish;
   }
 
+  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<ProjectConfig>): Promise<ClientCacheState> {
     return this.getCacheState(await initialCacheSyncUp);
   }
diff --git 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
@@ -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<IConfigFetcher>()
+        .setup(m => m.fetchLogic(It.IsAny<OptionsBase>(), It.IsAny<string>()))
+        .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<OptionsBase>(), It.IsAny<string>()), 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,17 +923,19 @@ 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),
     1,
     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 {
