Skip to content

Commit

Permalink
Minor improvements and fixes (#86)
Browse files Browse the repository at this point in the history
* Simplify IRolloutEvaluator.evaluate

* Fix return value of getAllValues when one or more settings evaluate with error

* Precalculate millisec interval values

* Bump version
  • Loading branch information
adams85 authored Jun 23, 2023
1 parent 184320d commit 406d749
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 54 deletions.
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "configcat-common",
"version": "8.0.0",
"version": "8.0.1",
"description": "ConfigCat is a configuration as a service that lets you manage your features and configurations without actually deploying new code.",
"main": "lib/index.js",
"types": "lib/index.d.ts",
Expand Down
11 changes: 7 additions & 4 deletions src/AutoPollConfigService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@ export class AutoPollConfigService extends ConfigServiceBase<AutoPollOptions> im
private readonly initialization: Promise<void>;
private signalInitialization: () => void = () => { /* Intentional no-op. */ };
private timerId?: ReturnType<typeof setTimeout>;
private readonly pollIntervalMs: number;

constructor(configFetcher: IConfigFetcher, options: AutoPollOptions) {

super(configFetcher, options);

this.pollIntervalMs = options.pollIntervalSeconds * 1000;

if (options.maxInitWaitTimeSeconds !== 0) {
this.initialized = false;

Expand Down Expand Up @@ -72,7 +75,7 @@ export class AutoPollConfigService extends ConfigServiceBase<AutoPollOptions> im
let cachedConfig: ProjectConfig;
if (!this.isOffline && !this.initialized) {
cachedConfig = await this.options.cache.get(this.cacheKey);
if (!cachedConfig.isExpired(this.options.pollIntervalSeconds * 1000)) {
if (!cachedConfig.isExpired(this.pollIntervalMs)) {
logSuccess(this.options.logger);
return cachedConfig;
}
Expand All @@ -82,7 +85,7 @@ export class AutoPollConfigService extends ConfigServiceBase<AutoPollOptions> im
}

cachedConfig = await this.options.cache.get(this.cacheKey);
if (!cachedConfig.isExpired(this.options.pollIntervalSeconds * 1000)) {
if (!cachedConfig.isExpired(this.pollIntervalMs)) {
logSuccess(this.options.logger);
}
else {
Expand Down Expand Up @@ -121,10 +124,10 @@ export class AutoPollConfigService extends ConfigServiceBase<AutoPollOptions> im
private async startRefreshWorker() {
this.options.logger.debug("AutoPollConfigService.startRefreshWorker() called.");

const delayMs = this.options.pollIntervalSeconds * 1000;
const delayMs = this.pollIntervalMs;

const latestConfig = await this.options.cache.get(this.cacheKey);
if (latestConfig.isExpired(this.options.pollIntervalSeconds * 1000)) {
if (latestConfig.isExpired(this.pollIntervalMs)) {
// Even if the service gets disposed immediately, we allow the first refresh for backward compatibility,
// i.e. to not break usage patterns like this:
// ```
Expand Down
10 changes: 5 additions & 5 deletions src/ConfigCatClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import { ManualPollConfigService } from "./ManualPollConfigService";
import { getWeakRefStub, isWeakRefAvailable } from "./Polyfills";
import type { ProjectConfig, RolloutPercentageItem, RolloutRule, Setting, SettingValue } from "./ProjectConfig";
import type { IEvaluationDetails, IRolloutEvaluator, SettingTypeOf, User } from "./RolloutEvaluator";
import { RolloutEvaluator, checkSettingsAvailable, evaluate, evaluateAll, evaluationDetailsFromDefaultValue, isAllowedValue } from "./RolloutEvaluator";
import { errorToString, getTimestampAsDate } from "./Utils";
import { RolloutEvaluator, checkSettingsAvailable, evaluate, evaluateAll, evaluationDetailsFromDefaultValue, getTimestampAsDate, isAllowedValue } from "./RolloutEvaluator";
import { errorToString } from "./Utils";

/** ConfigCat SDK client. */
export interface IConfigCatClient extends IProvidesHooks {
Expand Down Expand Up @@ -371,7 +371,7 @@ export class ConfigCatClient implements IConfigCatClient {
this.options.logger.debug("getAllValuesAsync() called.");

const defaultReturnValue = "empty array";
let result: SettingKeyValue[], evaluationDetailsArray: IEvaluationDetails[];
let evaluationDetailsArray: IEvaluationDetails[];
user ??= this.defaultUser;
try {
const [settings, remoteConfig] = await this.getSettingsAsync();
Expand All @@ -380,14 +380,14 @@ export class ConfigCatClient implements IConfigCatClient {
if (errors?.length) {
throw typeof AggregateError !== "undefined" ? new AggregateError(errors) : errors.pop();
}
result = evaluationDetailsArray.map(details => new SettingKeyValue(details.key, details.value));
}
catch (err) {
this.options.logger.settingEvaluationError("getAllValuesAsync", defaultReturnValue, err);
evaluationDetailsArray ??= [];
result = [];
}

const result = evaluationDetailsArray.map(details => new SettingKeyValue(details.key, details.value));

for (const evaluationDetail of evaluationDetailsArray) {
this.options.hooks.emit("flagEvaluated", evaluationDetail);
}
Expand Down
6 changes: 3 additions & 3 deletions src/LazyLoadConfigService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ import type { ProjectConfig } from "./ProjectConfig";

export class LazyLoadConfigService extends ConfigServiceBase<LazyLoadOptions> implements IConfigService {

private readonly cacheTimeToLiveSeconds: number;
private readonly cacheTimeToLiveMs: number;

constructor(configFetcher: IConfigFetcher, options: LazyLoadOptions) {

super(configFetcher, options);

this.cacheTimeToLiveSeconds = options.cacheTimeToLiveSeconds;
this.cacheTimeToLiveMs = options.cacheTimeToLiveSeconds * 1000;

options.hooks.emit("clientReady");
}
Expand All @@ -27,7 +27,7 @@ export class LazyLoadConfigService extends ConfigServiceBase<LazyLoadOptions> im

let cachedConfig = await this.options.cache.get(this.cacheKey);

if (cachedConfig.isExpired(this.cacheTimeToLiveSeconds * 1000)) {
if (cachedConfig.isExpired(this.cacheTimeToLiveMs)) {
if (!this.isOffline) {
logExpired(this.options.logger, ", calling refreshConfigCoreAsync()");
[, cachedConfig] = await this.refreshConfigCoreAsync(cachedConfig);
Expand Down
56 changes: 31 additions & 25 deletions src/RolloutEvaluator.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import type { LoggerWrapper } from "./ConfigCatLogger";
import type { IPercentageOption, ITargetingRule, ProjectConfig, Setting, SettingValue, VariationIdValue } from "./ProjectConfig";
import { Comparator, RolloutPercentageItem, RolloutRule } from "./ProjectConfig";
import type { IPercentageOption, ITargetingRule, ProjectConfig, RolloutPercentageItem, RolloutRule, Setting, SettingValue, VariationIdValue } from "./ProjectConfig";
import { Comparator } from "./ProjectConfig";
import * as semver from "./Semver";
import { sha1 } from "./Sha1";
import { errorToString, getTimestampAsDate } from "./Utils";
import { errorToString } from "./Utils";

export type SettingTypeOf<T> =
T extends boolean ? boolean :
Expand All @@ -14,7 +14,7 @@ export type SettingTypeOf<T> =
any;

export interface IRolloutEvaluator {
evaluate(setting: Setting, key: string, defaultValue: SettingValue, user: User | undefined, remoteConfig: ProjectConfig | null): IEvaluationDetails;
evaluate(setting: Setting, key: string, defaultValue: SettingValue, user: User | undefined, remoteConfig: ProjectConfig | null): IEvaluateResult;
}

/** The evaluated value and additional information about the evaluation of a feature flag or setting. */
Expand Down Expand Up @@ -85,7 +85,7 @@ export class RolloutEvaluator implements IRolloutEvaluator {
this.logger = logger;
}

evaluate(setting: Setting, key: string, defaultValue: SettingValue, user: User | undefined, remoteConfig: ProjectConfig | null): IEvaluationDetails {
evaluate(setting: Setting, key: string, defaultValue: SettingValue, user: User | undefined, remoteConfig: ProjectConfig | null): IEvaluateResult {
this.logger.debug("RolloutEvaluator.Evaluate() called.");

// A negative setting type indicates a flag override (see also Setting.fromValue)
Expand All @@ -102,7 +102,7 @@ export class RolloutEvaluator implements IRolloutEvaluator {
eLog.keyName = key;
eLog.returnValue = defaultValue;

let result: IEvaluateResult<object | undefined> | null;
let result: IEvaluateResult | null;

try {
if (user) {
Expand All @@ -112,7 +112,7 @@ export class RolloutEvaluator implements IRolloutEvaluator {
if (result !== null) {
eLog.returnValue = result.value;

return evaluationDetailsFromEvaluateResult(key, result, getTimestampAsDate(remoteConfig), user);
return result;
}

// evaluate percentage-based rules
Expand All @@ -126,7 +126,7 @@ export class RolloutEvaluator implements IRolloutEvaluator {
if (result !== null) {
eLog.returnValue = result.value;

return evaluationDetailsFromEvaluateResult(key, result, getTimestampAsDate(remoteConfig), user);
return result;
}
}
else {
Expand All @@ -143,14 +143,14 @@ export class RolloutEvaluator implements IRolloutEvaluator {
};
eLog.returnValue = result.value;

return evaluationDetailsFromEvaluateResult(key, result, getTimestampAsDate(remoteConfig), user);
return result;
}
finally {
this.logger.settingEvaluated(eLog);
}
}

private evaluateRules(rolloutRules: ReadonlyArray<RolloutRule>, user: User, eLog: EvaluateLogger): IEvaluateResult<RolloutRule> | null {
private evaluateRules(rolloutRules: ReadonlyArray<RolloutRule>, user: User, eLog: EvaluateLogger): IEvaluateResult | null {

this.logger.debug("RolloutEvaluator.EvaluateRules() called.");

Expand All @@ -174,10 +174,10 @@ export class RolloutEvaluator implements IRolloutEvaluator {
continue;
}

const result: IEvaluateResult<RolloutRule> = {
const result: IEvaluateResult = {
value: rule.value,
variationId: rule.variationId,
matchedRule: rule
matchedTargetingRule: rule
};

switch (comparator) {
Expand Down Expand Up @@ -330,7 +330,7 @@ export class RolloutEvaluator implements IRolloutEvaluator {
return null;
}

private evaluatePercentageRules(rolloutPercentageItems: ReadonlyArray<RolloutPercentageItem>, key: string, user: User): IEvaluateResult<RolloutPercentageItem> | null {
private evaluatePercentageRules(rolloutPercentageItems: ReadonlyArray<RolloutPercentageItem>, key: string, user: User): IEvaluateResult | null {
this.logger.debug("RolloutEvaluator.EvaluateVariations() called.");
if (rolloutPercentageItems && rolloutPercentageItems.length > 0) {

Expand All @@ -347,7 +347,7 @@ export class RolloutEvaluator implements IRolloutEvaluator {
return {
value: percentageRule.value,
variationId: percentageRule.variationId,
matchedRule: percentageRule
matchedPercentageOption: percentageRule
};
}
}
Expand Down Expand Up @@ -531,10 +531,11 @@ export class RolloutEvaluator implements IRolloutEvaluator {
}
}

interface IEvaluateResult<TRule> {
export interface IEvaluateResult {
value: SettingValue;
variationId?: string;
matchedRule?: TRule;
matchedTargetingRule?: RolloutRule;
matchedPercentageOption?: RolloutPercentageItem;
}

class EvaluateLogger {
Expand All @@ -560,16 +561,16 @@ class EvaluateLogger {

/* Helper functions */

function evaluationDetailsFromEvaluateResult(key: string, evaluateResult: IEvaluateResult<object | undefined>, fetchTime?: Date, user?: User): IEvaluationDetails {
function evaluationDetailsFromEvaluateResult<T extends SettingValue>(key: string, evaluateResult: IEvaluateResult, fetchTime?: Date, user?: User): IEvaluationDetails<SettingTypeOf<T>> {
return {
key,
value: evaluateResult.value,
value: evaluateResult.value as SettingTypeOf<T>,
variationId: evaluateResult.variationId,
fetchTime,
user,
isDefaultValue: false,
matchedEvaluationRule: evaluateResult.matchedRule instanceof RolloutRule ? evaluateResult.matchedRule : void 0,
matchedEvaluationPercentageRule: evaluateResult.matchedRule instanceof RolloutPercentageItem ? evaluateResult.matchedRule : void 0,
matchedEvaluationRule: evaluateResult.matchedTargetingRule,
matchedEvaluationPercentageRule: evaluateResult.matchedPercentageOption,
};
}

Expand Down Expand Up @@ -600,13 +601,13 @@ export function evaluate<T extends SettingValue>(evaluator: IRolloutEvaluator, s
return evaluationDetailsFromDefaultValue(key, defaultValue, getTimestampAsDate(remoteConfig), user, errorMessage);
}

const evaluationDetails = evaluator.evaluate(setting, key, defaultValue, user, remoteConfig);
const evaluateResult = evaluator.evaluate(setting, key, defaultValue, user, remoteConfig);

if (defaultValue !== null && defaultValue !== void 0 && typeof defaultValue !== typeof evaluationDetails.value) {
throw new TypeError(`The type of a setting must match the type of the given default value.\nThe setting's type was ${typeof defaultValue}, the given default value's type was ${typeof evaluationDetails.value}.\nPlease pass a corresponding default value type.`);
if (defaultValue !== null && defaultValue !== void 0 && typeof defaultValue !== typeof evaluateResult.value) {
throw new TypeError(`The type of a setting must match the type of the given default value.\nThe setting's type was ${typeof defaultValue}, the given default value's type was ${typeof evaluateResult.value}.\nPlease pass a corresponding default value type.`);
}

return evaluationDetails as IEvaluationDetails<SettingTypeOf<T>>;
return evaluationDetailsFromEvaluateResult<T>(key, evaluateResult, getTimestampAsDate(remoteConfig), user);
}

export function evaluateAll(evaluator: IRolloutEvaluator, settings: { [name: string]: Setting } | null,
Expand All @@ -623,7 +624,8 @@ export function evaluateAll(evaluator: IRolloutEvaluator, settings: { [name: str
for (const [key, setting] of Object.entries(settings)) {
let evaluationDetails: IEvaluationDetails;
try {
evaluationDetails = evaluator.evaluate(setting, key, null, user, remoteConfig);
const evaluateResult = evaluator.evaluate(setting, key, null, user, remoteConfig);
evaluationDetails = evaluationDetailsFromEvaluateResult(key, evaluateResult, getTimestampAsDate(remoteConfig), user);
}
catch (err) {
errors ??= [];
Expand Down Expand Up @@ -654,6 +656,10 @@ export function isAllowedValue(value: SettingValue): boolean {
|| typeof value === "string";
}

export function getTimestampAsDate(projectConfig: ProjectConfig | null): Date | undefined {
return projectConfig ? new Date(projectConfig.timestamp) : void 0;
}

function keysToString(settings: { [name: string]: Setting }) {
return Object.keys(settings).map(key => `'${key}'`).join(", ");
}
6 changes: 0 additions & 6 deletions src/Utils.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,10 @@
import type { ProjectConfig } from "./ProjectConfig";

export function delay(delayMs: number, obtainCancel?: (cancel: () => void) => void): Promise<void> {
let timerId: ReturnType<typeof setTimeout>;
const promise = new Promise<void>(resolve => timerId = setTimeout(resolve, delayMs));
obtainCancel?.(() => clearTimeout(timerId));
return promise;
}

export function getTimestampAsDate(projectConfig: ProjectConfig | null): Date | undefined {
return projectConfig ? new Date(projectConfig.timestamp) : void 0;
}

export function errorToString(err: any, includeStackTrace = false): string {
return err instanceof Error
? includeStackTrace && err.stack ? err.stack : err.toString()
Expand Down
10 changes: 5 additions & 5 deletions test/ConfigCatClientTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { IProvidesHooks } from "../src/Hooks";
import { LazyLoadConfigService } from "../src/LazyLoadConfigService";
import { isWeakRefAvailable, setupPolyfills } from "../src/Polyfills";
import { Config, IConfig, ProjectConfig, Setting } from "../src/ProjectConfig";
import { IEvaluationDetails, IRolloutEvaluator, User } from "../src/RolloutEvaluator";
import { IEvaluateResult, IEvaluationDetails, IRolloutEvaluator, User } from "../src/RolloutEvaluator";
import { delay } from "../src/Utils";
import "./helpers/ConfigCatClientCacheExtensions";
import { FakeCache, FakeConfigCatKernel, FakeConfigFetcher, FakeConfigFetcherBase, FakeConfigFetcherWithAlwaysVariableEtag, FakeConfigFetcherWithNullNewConfig, FakeConfigFetcherWithPercantageRules, FakeConfigFetcherWithRules, FakeConfigFetcherWithTwoCaseSensitiveKeys, FakeConfigFetcherWithTwoKeys, FakeConfigFetcherWithTwoKeysAndRules, FakeLogger } from "./helpers/fakes";
Expand Down Expand Up @@ -326,7 +326,7 @@ describe("ConfigCatClient", () => {

const err = new Error("Something went wrong.");
client["evaluator"] = new class implements IRolloutEvaluator {
evaluate(setting: Setting, key: string, defaultValue: any, user: User | undefined, remoteConfig: ProjectConfig | null, defaultVariationId?: any): IEvaluationDetails {
evaluate(setting: Setting, key: string, defaultValue: any, user: User | undefined, remoteConfig: ProjectConfig | null, defaultVariationId?: any): IEvaluateResult {
throw err;
}
};
Expand Down Expand Up @@ -429,7 +429,7 @@ describe("ConfigCatClient", () => {

const err = new Error("Something went wrong.");
client["evaluator"] = new class implements IRolloutEvaluator {
evaluate(setting: Setting, key: string, defaultValue: any, user: User | undefined, remoteConfig: ProjectConfig | null, defaultVariationId?: any): IEvaluationDetails {
evaluate(setting: Setting, key: string, defaultValue: any, user: User | undefined, remoteConfig: ProjectConfig | null, defaultVariationId?: any): IEvaluateResult {
throw err;
}
};
Expand Down Expand Up @@ -502,9 +502,9 @@ describe("ConfigCatClient", () => {

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

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;

Expand All @@ -519,9 +519,9 @@ describe("ConfigCatClient", () => {

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 client: IConfigCatClient = new ConfigCatClient(options, configCatKernel);

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;

Expand Down
Loading

0 comments on commit 406d749

Please sign in to comment.