Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Correct clientReady hook in Auto Poll mode #94

Merged
merged 3 commits into from
Nov 9, 2023
Merged

Correct clientReady hook in Auto Poll mode #94

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b20d75b
Always fire the clientReady event in Auto Poll mode regardless of suc…
adams85 Oct 9, 2023
4d61956
Add tests + fix broken tests
adams85 Oct 9, 2023
d5d2b55
Revert to the original clientReady behavior in Auto Poll mode when ma…
adams85 Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/AutoPollConfigService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class AutoPollConfigService extends ConfigServiceBase<AutoPollOptions> im

// This promise will be resolved when
// 1. the cache contains a valid config at startup (see startRefreshWorker) or
// 2. config.json is downloaded the first time (see onConfigUpdated) or
// 2. config json is fetched the first time, regardless of success or failure (see onConfigUpdated) or
// 3. maxInitWaitTimeSeconds > 0 and maxInitWaitTimeSeconds has passed (see the setTimeout call below).
this.initialization = new Promise(resolve => this.signalInitialization = () => {
this.initialized = true;
Expand Down Expand Up @@ -117,8 +117,8 @@ export class AutoPollConfigService extends ConfigServiceBase<AutoPollOptions> im
}
}

protected onConfigUpdated(newConfig: ProjectConfig): void {
super.onConfigUpdated(newConfig);
protected onConfigFetched(newConfig: ProjectConfig): void {
super.onConfigFetched(newConfig);
this.signalInitialization();
}

Expand Down
14 changes: 8 additions & 6 deletions src/ConfigServiceBase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,24 +106,26 @@ export abstract class ConfigServiceBase<TOptions extends OptionsBase> {
protected async refreshConfigCoreAsync(latestConfig: ProjectConfig): Promise<[FetchResult, ProjectConfig]> {
const fetchResult = await this.fetchAsync(latestConfig);

let configChanged = false;
const success = fetchResult.status === FetchStatus.Fetched;
if (success
|| fetchResult.config.timestamp > latestConfig.timestamp && (!fetchResult.config.isEmpty || latestConfig.isEmpty)) {
await this.options.cache.set(this.cacheKey, fetchResult.config);

this.onConfigUpdated(fetchResult.config);
configChanged = success && !ProjectConfig.equals(fetchResult.config, latestConfig);
latestConfig = fetchResult.config;
}

if (success && !ProjectConfig.equals(fetchResult.config, latestConfig)) {
this.onConfigChanged(fetchResult.config);
}
this.onConfigFetched(fetchResult.config);

latestConfig = fetchResult.config;
if (configChanged) {
this.onConfigChanged(fetchResult.config);
}

return [fetchResult, latestConfig];
}

protected onConfigUpdated(newConfig: ProjectConfig): void { }
protected onConfigFetched(newConfig: ProjectConfig): void { }

protected onConfigChanged(newConfig: ProjectConfig): void {
this.options.logger.debug("config changed");
Expand Down
61 changes: 46 additions & 15 deletions test/ConfigCatClientTests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,13 +525,37 @@ describe("ConfigCatClient", () => {
const startDate: number = new Date().getTime();
const client: IConfigCatClient = new ConfigCatClient(options, configCatKernel);
const actualValue = await client.getValueAsync("debug", false);
const ellapsedMilliseconds: number = new Date().getTime() - startDate;
const elapsedMilliseconds: number = new Date().getTime() - startDate;

assert.isAtLeast(ellapsedMilliseconds, 500);
assert.isAtMost(ellapsedMilliseconds, maxInitWaitTimeSeconds * 1000);
assert.isAtLeast(elapsedMilliseconds, 500);
assert.isAtMost(elapsedMilliseconds, maxInitWaitTimeSeconds * 1000);
assert.equal(actualValue, true);
});

for (const statusCode of [403, 404, 500, null]) {
it(`Initialization With AutoPollOptions - with maxInitWaitTimeSeconds - getValueDetailsAsync should not wait maxInitWaitTimeSeconds even if fetch result is ${statusCode ?? "network error"}`, async () => {

const maxInitWaitTimeSeconds = 2;

const configFetchDelay = maxInitWaitTimeSeconds * 1000 / 4;
const configFetcher = new FakeConfigFetcherBase(null, configFetchDelay, () =>
statusCode ? { statusCode, reasonPhrase: "x" } : (() => { throw "network error"; })());

const configCatKernel: FakeConfigCatKernel = { configFetcher, sdkType: "common", sdkVersion: "1.0.0" };
const options: AutoPollOptions = new AutoPollOptions("APIKEY", "common", "1.0.0", { maxInitWaitTimeSeconds }, null);

const startDate: number = new Date().getTime();
const client: IConfigCatClient = new ConfigCatClient(options, configCatKernel);
const actualDetails = await client.getValueDetailsAsync("debug", false);
const elapsedMilliseconds: number = new Date().getTime() - startDate;

assert.isAtLeast(elapsedMilliseconds, 500);
assert.isAtMost(elapsedMilliseconds, configFetchDelay * 2);
assert.equal(actualDetails.isDefaultValue, true);
assert.equal(actualDetails.value, false);
});
}

it("Initialization With AutoPollOptions - with maxInitWaitTimeSeconds - getValueAsync should wait for maxInitWaitTimeSeconds only and return default value", async () => {

const maxInitWaitTimeSeconds = 1;
Expand All @@ -542,10 +566,10 @@ describe("ConfigCatClient", () => {
const startDate: number = new Date().getTime();
const client: IConfigCatClient = new ConfigCatClient(options, configCatKernel);
const actualValue = await client.getValueAsync("debug", false);
const ellapsedMilliseconds: number = new Date().getTime() - startDate;
const elapsedMilliseconds: number = new Date().getTime() - startDate;

assert.isAtLeast(ellapsedMilliseconds, maxInitWaitTimeSeconds * 1000);
assert.isAtMost(ellapsedMilliseconds, (maxInitWaitTimeSeconds * 1000) + 50); // 50 ms for tolerance
assert.isAtLeast(elapsedMilliseconds, maxInitWaitTimeSeconds * 1000);
assert.isAtMost(elapsedMilliseconds, (maxInitWaitTimeSeconds * 1000) + 50); // 50 ms for tolerance
assert.equal(actualValue, false);
});

Expand All @@ -572,24 +596,31 @@ describe("ConfigCatClient", () => {

it("AutoPoll - should wait for maxInitWaitTimeSeconds", async () => {

const configCatKernel: FakeConfigCatKernel = { configFetcher: new FakeConfigFetcherWithNullNewConfig(), sdkType: "common", sdkVersion: "1.0.0" };
const options: AutoPollOptions = new AutoPollOptions("APIKEY", "common", "1.0.0", { maxInitWaitTimeSeconds: maxInitWaitTimeSeconds }, null);
const configFetcher = new FakeConfigFetcherWithNullNewConfig(maxInitWaitTimeSeconds * 2 * 1000);
const configCatKernel: FakeConfigCatKernel = { configFetcher, sdkType: "common", sdkVersion: "1.0.0" };
const options: AutoPollOptions = new AutoPollOptions("APIKEY", "common", "1.0.0", { maxInitWaitTimeSeconds }, null);

const startDate: number = new Date().getTime();
const client: IConfigCatClient = new ConfigCatClient(options, configCatKernel);
const state = await client.waitForReady();
const ellapsedMilliseconds: number = new Date().getTime() - startDate;
const elapsedMilliseconds: number = new Date().getTime() - startDate;

assert.isAtLeast(ellapsedMilliseconds, maxInitWaitTimeSeconds);
assert.isAtMost(ellapsedMilliseconds, (maxInitWaitTimeSeconds * 1000) + 50); // 50 ms for tolerance
assert.isAtLeast(elapsedMilliseconds, maxInitWaitTimeSeconds * 1000);
assert.isAtMost(elapsedMilliseconds, (maxInitWaitTimeSeconds * 1000) + 50); // 50 ms for tolerance

assert.equal(state, ClientReadyState.NoFlagData);
assert.equal(client.snapshot().getValue("debug", false), false);
});

it("AutoPoll - should wait for maxInitWaitTimeSeconds and return cached", async () => {

const configCatKernel: FakeConfigCatKernel = { configFetcher: configFetcher, sdkType: "common", sdkVersion: "1.0.0" };
const configFetcher = new FakeConfigFetcherBase("{}", maxInitWaitTimeSeconds * 2 * 1000, (lastConfig, lastETag) => ({
statusCode: 500,
reasonPhrase: "",
eTag: (lastETag as any | 0) + 1 + "",
body: lastConfig
} as IFetchResponse));
const configCatKernel: FakeConfigCatKernel = { configFetcher, sdkType: "common", sdkVersion: "1.0.0" };
const options: AutoPollOptions = new AutoPollOptions("APIKEY", "common", "1.0.0", {
maxInitWaitTimeSeconds: maxInitWaitTimeSeconds,
cache: new FakeExternalCacheWithInitialData(120_000)
Expand All @@ -598,10 +629,10 @@ describe("ConfigCatClient", () => {
const startDate: number = new Date().getTime();
const client: IConfigCatClient = new ConfigCatClient(options, configCatKernel);
const state = await client.waitForReady();
const ellapsedMilliseconds: number = new Date().getTime() - startDate;
const elapsedMilliseconds: number = new Date().getTime() - startDate;

assert.isAtLeast(ellapsedMilliseconds, maxInitWaitTimeSeconds);
assert.isAtMost(ellapsedMilliseconds, (maxInitWaitTimeSeconds * 1000) + 50); // 50 ms for tolerance
assert.isAtLeast(elapsedMilliseconds, maxInitWaitTimeSeconds * 1000);
assert.isAtMost(elapsedMilliseconds, (maxInitWaitTimeSeconds * 1000) + 50); // 50 ms for tolerance

assert.equal(state, ClientReadyState.HasCachedFlagDataOnly);
assert.equal(client.snapshot().getValue("debug", false), true);
Expand Down