Skip to content

Commit 9e1ec80

Browse files
adams85adams85
committed
Make clientReady consistent with other SDKs in Auto Poll + offline mode
1 parent 049bfa4 commit 9e1ec80

File tree

3 files changed

+54
-108
lines changed

3 files changed

+54
-108
lines changed

‎src/AutoPollConfigService.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,9 @@ export class AutoPollConfigService extends ConfigServiceBase<AutoPollOptions> im
3333
this.initialized = false;
3434

3535
// This promise will be resolved as soon as
36-
// 1. an up-to-date config is obtained from the cache (see startRefreshWorker),
37-
// 2. or a config fetch operation completes, regardless of success or failure (see onConfigUpdated).
36+
// 1. the initial sync with the external cache completes (see startRefreshWorker),
37+
// 2. and, in case the client is online and the local cache is still empty or expired,
38+
// the first config fetch operation completes, regardless of success or failure (see onConfigFetched).
3839
const initSignalPromise = new Promise<void>(resolve => this.signalInitialization = resolve);
3940

4041
// 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<AutoPollOptions> im
160161
this.options.logger.debug("AutoPollConfigService.refreshWorkerLogic() called.");
161162

162163
const latestConfig = await (initialCacheSyncUp ?? this.options.cache.get(this.cacheKey));
163-
if (!latestConfig.isExpired(this.pollExpirationMs)) {
164-
this.signalInitialization();
165-
return;
166-
}
167-
168-
if (initialCacheSyncUp ? !this.isOfflineExactly : !this.isOffline) {
164+
if (latestConfig.isExpired(this.pollExpirationMs)) {
169165
// Even if the service gets disposed immediately, we allow the first refresh for backward compatibility,
170166
// i.e. to not break usage patterns like this:
171167
// ```
172168
// client.getValueAsync("SOME_KEY", false, user).then(value => { /* ... */ });
173169
// client.dispose();
174170
// ```
175-
176-
await this.refreshConfigCoreAsync(latestConfig);
171+
if (initialCacheSyncUp ? !this.isOfflineExactly : !this.isOffline) {
172+
await this.refreshConfigCoreAsync(latestConfig);
173+
return; // postpone signalling initialization until `onConfigFetched`
174+
}
177175
}
176+
177+
this.signalInitialization();
178178
}
179179

180180
getCacheState(cachedConfig: ProjectConfig): ClientCacheState {

‎test/ConfigCatClientTests.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,8 +1217,9 @@ describe("ConfigCatClient", () => {
12171217

12181218
for (const [pollingMode, optionsFactory] of optionsFactoriesForOfflineModeTests) {
12191219
it(`setOnline() should make a(n) ${PollingMode[pollingMode]} client created in offline mode transition to online mode`, async () => {
1220+
const configFetcherDelayMs = 100;
12201221

1221-
const configFetcher = new FakeConfigFetcherBase("{}", 100, (lastConfig, lastETag) => ({
1222+
const configFetcher = new FakeConfigFetcherBase("{}", configFetcherDelayMs, (lastConfig, lastETag) => ({
12221223
statusCode: 200,
12231224
reasonPhrase: "OK",
12241225
eTag: (lastETag as any | 0) + 1 + "",
@@ -1246,7 +1247,7 @@ describe("ConfigCatClient", () => {
12461247
client.setOnline();
12471248

12481249
if (configService instanceof AutoPollConfigService) {
1249-
assert.isTrue(await configService["initializationPromise"]);
1250+
await delay(configFetcherDelayMs + 50);
12501251
expectedFetchTimes++;
12511252
}
12521253

‎test/ConfigServiceBaseTests.ts

Lines changed: 41 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -312,117 +312,62 @@ describe("ConfigServiceBaseTests", () => {
312312
service.dispose();
313313
});
314314

315-
it("AutoPollConfigService - Should wait maxInitWaitTime in offline mode when external cache is expired", async () => {
315+
for (const expectedCacheState of [ClientCacheState.NoFlagData, ClientCacheState.HasCachedFlagDataOnly, ClientCacheState.HasUpToDateFlagData]) {
316+
it(`AutoPollConfigService - Should emit clientReady in offline mode when sync with external cache is completed - expectedCacheState: ${expectedCacheState}`, async () => {
316317

317-
// Arrange
318-
319-
const pollIntervalSeconds = 1;
320-
const maxInitWaitTimeSeconds = 2.5;
321-
322-
const frOld: FetchResult = createFetchResult("oldEtag");
323-
const projectConfigOld = createConfigFromFetchResult(frOld)
324-
.with(ProjectConfig.generateTimestamp() - (1.5 * pollIntervalSeconds * 1000) + 0.5 * POLL_EXPIRATION_TOLERANCE_MS);
325-
326-
const logger = new LoggerWrapper(new FakeLogger());
327-
const cache = new ExternalConfigCache(new FakeExternalCache(), logger);
328-
329-
const options = createAutoPollOptions(
330-
"APIKEY",
331-
{
332-
pollIntervalSeconds,
333-
maxInitWaitTimeSeconds,
334-
offline: true,
335-
},
336-
createKernel({ defaultCacheFactory: () => cache })
337-
);
318+
// Arrange
338319

339-
cache.set(options.getCacheKey(), projectConfigOld);
320+
const pollIntervalSeconds = 1;
340321

341-
const fetcherMock = new Mock<IConfigFetcher>();
342-
343-
// Act
344-
345-
const service: AutoPollConfigService = new AutoPollConfigService(
346-
fetcherMock.object(),
347-
options);
322+
let projectConfig: ProjectConfig | undefined;
323+
if (expectedCacheState !== ClientCacheState.NoFlagData) {
324+
const fr: FetchResult = createFetchResult("oldEtag");
325+
projectConfig = createConfigFromFetchResult(fr);
348326

349-
const { readyPromise } = service;
350-
const delayAbortToken = new AbortToken();
351-
const delayPromise = delay(maxInitWaitTimeSeconds * 1000 - 250, delayAbortToken);
352-
const raceResult = await Promise.race([readyPromise, delayPromise]);
353-
delayAbortToken.abort();
354-
355-
// Assert
356-
357-
assert.strictEqual(raceResult, true);
358-
359-
// Cleanup
360-
361-
service.dispose();
362-
});
327+
if (expectedCacheState === ClientCacheState.HasCachedFlagDataOnly) {
328+
projectConfig = projectConfig
329+
.with(ProjectConfig.generateTimestamp() - (1.5 * pollIntervalSeconds * 1000) + 0.5 * POLL_EXPIRATION_TOLERANCE_MS);
330+
}
331+
}
363332

364-
it("AutoPollConfigService - Should initialize in offline mode when external cache becomes up-to-date", async () => {
333+
const logger = new LoggerWrapper(new FakeLogger());
334+
const cache = new ExternalConfigCache(new FakeExternalCache(), logger);
365335

366-
// Arrange
367-
368-
const pollIntervalSeconds = 1;
369-
const maxInitWaitTimeSeconds = 2.5;
370-
const cacheSetDelayMs = 0.5 * pollIntervalSeconds * 1000;
371-
372-
const frOld: FetchResult = createFetchResult("oldEtag");
373-
const projectConfigOld = createConfigFromFetchResult(frOld)
374-
.with(ProjectConfig.generateTimestamp() - (1.5 * pollIntervalSeconds * 1000) + 0.5 * POLL_EXPIRATION_TOLERANCE_MS);
375-
376-
const logger = new LoggerWrapper(new FakeLogger());
377-
const cache = new ExternalConfigCache(new FakeExternalCache(), logger);
378-
379-
const options = createAutoPollOptions(
380-
"APIKEY",
381-
{
382-
pollIntervalSeconds,
383-
maxInitWaitTimeSeconds,
384-
offline: true,
385-
},
386-
createKernel({ defaultCacheFactory: () => cache })
387-
);
336+
const options = createAutoPollOptions(
337+
"APIKEY",
338+
{
339+
pollIntervalSeconds,
340+
offline: true,
341+
},
342+
createKernel({ defaultCacheFactory: () => cache })
343+
);
388344

389-
cache.set(options.getCacheKey(), projectConfigOld);
345+
if (projectConfig) {
346+
cache.set(options.getCacheKey(), projectConfig);
347+
}
390348

391-
const fetcherMock = new Mock<IConfigFetcher>();
349+
const fetcherMock = new Mock<IConfigFetcher>();
392350

393-
// Act
351+
// Act
394352

395-
const service: AutoPollConfigService = new AutoPollConfigService(
396-
fetcherMock.object(),
397-
options);
353+
const service: AutoPollConfigService = new AutoPollConfigService(
354+
fetcherMock.object(),
355+
options);
398356

399-
const { readyPromise } = service;
400-
const delayAbortToken = new AbortToken();
401-
const delayPromise = delay(maxInitWaitTimeSeconds * 1000 - 250, delayAbortToken);
402-
const racePromise = Promise.race([readyPromise, delayPromise]);
357+
const { readyPromise } = service;
358+
const delayAbortToken = new AbortToken();
359+
const delayPromise = delay(pollIntervalSeconds * 1000 - 250, delayAbortToken);
360+
const raceResult = await Promise.race([readyPromise, delayPromise]);
403361

404-
const cacheSetDelayAbortToken = new AbortToken();
405-
const cacheSetDelayPromise = delay(cacheSetDelayMs, cacheSetDelayAbortToken);
406-
const cacheSetRaceResult = await Promise.race([readyPromise, cacheSetDelayPromise]);
407-
cacheSetDelayAbortToken.abort();
408-
assert.strictEqual(cacheSetRaceResult, true);
362+
// Assert
409363

410-
const frNew: FetchResult = createFetchResult("newEtag");
411-
const projectConfigNew: ProjectConfig = createConfigFromFetchResult(frNew)
412-
.with(ProjectConfig.generateTimestamp() + (pollIntervalSeconds * 1000) - cacheSetDelayMs + 0.5 * POLL_EXPIRATION_TOLERANCE_MS);
413-
cache.set(options.getCacheKey(), projectConfigNew);
364+
assert.strictEqual(raceResult, expectedCacheState);
414365

415-
const raceResult = await racePromise;
416-
delayAbortToken.abort();
366+
// Cleanup
417367

418-
// Assert
419-
420-
assert.strictEqual(raceResult, ClientCacheState.HasUpToDateFlagData);
421-
422-
// Cleanup
423-
424-
service.dispose();
425-
});
368+
service.dispose();
369+
});
370+
}
426371

427372
it("LazyLoadConfigService - ProjectConfig is different in the cache - should fetch a new config and put into cache", async () => {
428373

0 commit comments

Comments
 (0)