Refresh internal cache in offline mode too

Author: adams85

src/AutoPollConfigService.ts

@@ -32,9 +32,9 @@ export class AutoPollConfigService extends ConfigServiceBase<AutoPollOptions> 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<void>(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<AutoPollOptions> im
 
     this.readyPromise = this.getReadyPromise(initialCacheSyncUp);
 
-    if (!options.offline) {
-      this.startRefreshWorker(initialCacheSyncUp, this.stopToken);
-    }
+    this.startRefreshWorker(initialCacheSyncUp, this.stopToken);
   }
 
   private async waitForInitializationAsync(initSignalPromise: Promise<void>): Promise<boolean> {
@@ -82,7 +80,7 @@ export class AutoPollConfigService extends ConfigServiceBase<AutoPollOptions> 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<AutoPollOptions> 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<ProjectConfig> | 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<AutoPollOptions> 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<AutoPollOptions> im
     this.stopToken.abort();
   }
 
-  private async refreshWorkerLogic(isFirstIteration: boolean, initialCacheSyncUp: ProjectConfig | Promise<ProjectConfig> | null) {
-    this.options.logger.debug("AutoPollConfigService.refreshWorkerLogic() - called.");
+  private async refreshWorkerLogic(initialCacheSyncUp: ProjectConfig | Promise<ProjectConfig> | 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);
     }
   }
 

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<TOptions extends OptionsBase> {
     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<TOptions extends OptionsBase> {
 
   private async fetchLogicAsync(lastConfig: ProjectConfig): Promise<FetchResult> {
     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<TOptions extends OptionsBase> {
   // 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,23 +281,20 @@ export abstract class ConfigServiceBase<TOptions extends OptionsBase> {
     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) {
       this.options.logger.configServiceMethodHasNoEffectDueToDisposedClient("setOnline");
     }
   }
 
-  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) {

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 :(");
-  }
-}

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 {

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[] = [];

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<IConfigFetcher>();
+
+    // 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<IConfigFetcher>();
+
+    // 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

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) {
   }