diff --git a/CHANGES.txt b/CHANGES.txt index 95030536..4d6e3899 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,6 +1,7 @@ 1.12.0 (December XX, 2023) - Added support for Flag Sets in "consumer" and "partial consumer" modes for Pluggable and Redis storages. - Updated evaluation flow to log a warning when using flag sets that don't contain cached feature flags. + - Updated Redis adapter to handle timeouts and queueing of some missing commands: 'hincrby', 'popNRaw', and 'pipeline.exec'. - Bugfixing - Fixed manager methods in consumer modes to return results in a promise when the SDK is not operational (not ready or destroyed). 1.11.0 (November 3, 2023) diff --git a/package-lock.json b/package-lock.json index a0e6bd3b..d19598ea 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.12.1-rc.1", + "version": "1.12.1-rc.4", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-commons", - "version": "1.12.1-rc.1", + "version": "1.12.1-rc.4", "license": "Apache-2.0", "dependencies": { "tslib": "^2.3.1" diff --git a/package.json b/package.json index a1b84218..d666fba7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.12.1-rc.1", + "version": "1.12.1-rc.4", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/src/sdkClient/client.ts b/src/sdkClient/client.ts index 1663af92..8da54dd3 100644 --- a/src/sdkClient/client.ts +++ b/src/sdkClient/client.ts @@ -4,7 +4,7 @@ import { getMatching, getBucketing } from '../utils/key'; import { validateSplitExistence } from '../utils/inputValidation/splitExistence'; import { validateTrafficTypeExistence } from '../utils/inputValidation/trafficTypeExistence'; import { SDK_NOT_READY } from '../utils/labels'; -import { CONTROL, TREATMENT, TREATMENTS, TREATMENT_WITH_CONFIG, TREATMENTS_WITH_CONFIG, TRACK, TREATMENTS_WITH_CONFIG_BY_FLAGSETS, TREATMENTS_BY_FLAGSETS, TREATMENTS_BY_FLAGSET, TREATMENTS_WITH_CONFIG_BY_FLAGSET } from '../utils/constants'; +import { CONTROL, TREATMENT, TREATMENTS, TREATMENT_WITH_CONFIG, TREATMENTS_WITH_CONFIG, TRACK, TREATMENTS_WITH_CONFIG_BY_FLAGSETS, TREATMENTS_BY_FLAGSETS, TREATMENTS_BY_FLAGSET, TREATMENTS_WITH_CONFIG_BY_FLAGSET, GET_TREATMENTS_WITH_CONFIG, GET_TREATMENTS_BY_FLAG_SETS, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SETS, GET_TREATMENTS_BY_FLAG_SET, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SET, GET_TREATMENT_WITH_CONFIG, GET_TREATMENT, GET_TREATMENTS, TRACK_FN_LABEL } from '../utils/constants'; import { IEvaluationResult } from '../evaluator/types'; import { SplitIO, ImpressionDTO } from '../types'; import { IMPRESSION, IMPRESSION_QUEUEING } from '../logger/constants'; @@ -29,12 +29,12 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl const { sdkReadinessManager: { readinessManager }, storage, settings, impressionsTracker, eventTracker, telemetryTracker } = params; const { log, mode } = settings; - function getTreatment(key: SplitIO.SplitKey, featureFlagName: string, attributes: SplitIO.Attributes | undefined, withConfig = false) { + function getTreatment(key: SplitIO.SplitKey, featureFlagName: string, attributes: SplitIO.Attributes | undefined, withConfig = false, methodName = GET_TREATMENT) { const stopTelemetryTracker = telemetryTracker.trackEval(withConfig ? TREATMENT_WITH_CONFIG : TREATMENT); const wrapUp = (evaluationResult: IEvaluationResult) => { const queue: ImpressionDTO[] = []; - const treatment = processEvaluation(evaluationResult, featureFlagName, key, attributes, withConfig, `getTreatment${withConfig ? 'withConfig' : ''}`, queue); + const treatment = processEvaluation(evaluationResult, featureFlagName, key, attributes, withConfig, methodName, queue); impressionsTracker.track(queue, attributes); stopTelemetryTracker(queue[0] && queue[0].label); @@ -51,17 +51,17 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl } function getTreatmentWithConfig(key: SplitIO.SplitKey, featureFlagName: string, attributes: SplitIO.Attributes | undefined) { - return getTreatment(key, featureFlagName, attributes, true); + return getTreatment(key, featureFlagName, attributes, true, GET_TREATMENT_WITH_CONFIG); } - function getTreatments(key: SplitIO.SplitKey, featureFlagNames: string[], attributes: SplitIO.Attributes | undefined, withConfig = false) { + function getTreatments(key: SplitIO.SplitKey, featureFlagNames: string[], attributes: SplitIO.Attributes | undefined, withConfig = false, methodName = GET_TREATMENTS) { const stopTelemetryTracker = telemetryTracker.trackEval(withConfig ? TREATMENTS_WITH_CONFIG : TREATMENTS); const wrapUp = (evaluationResults: Record) => { const queue: ImpressionDTO[] = []; const treatments: Record = {}; Object.keys(evaluationResults).forEach(featureFlagName => { - treatments[featureFlagName] = processEvaluation(evaluationResults[featureFlagName], featureFlagName, key, attributes, withConfig, `getTreatments${withConfig ? 'withConfig' : ''}`, queue); + treatments[featureFlagName] = processEvaluation(evaluationResults[featureFlagName], featureFlagName, key, attributes, withConfig, methodName, queue); }); impressionsTracker.track(queue, attributes); @@ -79,18 +79,18 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl } function getTreatmentsWithConfig(key: SplitIO.SplitKey, featureFlagNames: string[], attributes: SplitIO.Attributes | undefined) { - return getTreatments(key, featureFlagNames, attributes, true); + return getTreatments(key, featureFlagNames, attributes, true, GET_TREATMENTS_WITH_CONFIG); } - function getTreatmentsByFlagSets(key: SplitIO.SplitKey, flagSetNames: string[], attributes: SplitIO.Attributes | undefined, withConfig = false, method: Method = TREATMENTS_BY_FLAGSETS) { + function getTreatmentsByFlagSets(key: SplitIO.SplitKey, flagSetNames: string[], attributes: SplitIO.Attributes | undefined, withConfig = false, method: Method = TREATMENTS_BY_FLAGSETS, methodName = GET_TREATMENTS_BY_FLAG_SETS) { const stopTelemetryTracker = telemetryTracker.trackEval(method); - const wrapUp = (evaluationResults: Record) => { + const wrapUp = (evaluationResults: Record) => { const queue: ImpressionDTO[] = []; const treatments: Record = {}; const evaluations = evaluationResults; Object.keys(evaluations).forEach(featureFlagName => { - treatments[featureFlagName] = processEvaluation(evaluations[featureFlagName], featureFlagName, key, attributes, withConfig, `getTreatmentsByFlagSets${withConfig ? 'WithConfig' : ''}`, queue); + treatments[featureFlagName] = processEvaluation(evaluations[featureFlagName], featureFlagName, key, attributes, withConfig, methodName, queue); }); impressionsTracker.track(queue, attributes); @@ -99,22 +99,22 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl }; const evaluations = readinessManager.isReady() || readinessManager.isReadyFromCache() ? - evaluateFeaturesByFlagSets(log, key, flagSetNames, attributes, storage, method) : + evaluateFeaturesByFlagSets(log, key, flagSetNames, attributes, storage, methodName) : isStorageSync(settings) ? {} : Promise.resolve({}); // Promisify if async return thenable(evaluations) ? evaluations.then((res) => wrapUp(res)) : wrapUp(evaluations); } function getTreatmentsWithConfigByFlagSets(key: SplitIO.SplitKey, flagSetNames: string[], attributes: SplitIO.Attributes | undefined) { - return getTreatmentsByFlagSets(key, flagSetNames, attributes, true, TREATMENTS_WITH_CONFIG_BY_FLAGSETS); + return getTreatmentsByFlagSets(key, flagSetNames, attributes, true, TREATMENTS_WITH_CONFIG_BY_FLAGSETS, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SETS); } function getTreatmentsByFlagSet(key: SplitIO.SplitKey, flagSetName: string, attributes: SplitIO.Attributes | undefined) { - return getTreatmentsByFlagSets(key, [flagSetName], attributes, false, TREATMENTS_BY_FLAGSET); + return getTreatmentsByFlagSets(key, [flagSetName], attributes, false, TREATMENTS_BY_FLAGSET, GET_TREATMENTS_BY_FLAG_SET); } function getTreatmentsWithConfigByFlagSet(key: SplitIO.SplitKey, flagSetName: string, attributes: SplitIO.Attributes | undefined) { - return getTreatmentsByFlagSets(key, [flagSetName], attributes, true, TREATMENTS_WITH_CONFIG_BY_FLAGSET); + return getTreatmentsByFlagSets(key, [flagSetName], attributes, true, TREATMENTS_WITH_CONFIG_BY_FLAGSET, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SET); } // Internal function @@ -171,7 +171,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl }; // This may be async but we only warn, we don't actually care if it is valid or not in terms of queueing the event. - validateTrafficTypeExistence(log, readinessManager, storage.splits, mode, trafficTypeName, 'track'); + validateTrafficTypeExistence(log, readinessManager, storage.splits, mode, trafficTypeName, TRACK_FN_LABEL); const result = eventTracker.track(eventData, size); diff --git a/src/sdkClient/clientInputValidation.ts b/src/sdkClient/clientInputValidation.ts index da0b87ff..c5b49b49 100644 --- a/src/sdkClient/clientInputValidation.ts +++ b/src/sdkClient/clientInputValidation.ts @@ -12,12 +12,12 @@ import { validateIfOperational } from '../utils/inputValidation'; import { startsWith } from '../utils/lang'; -import { CONTROL, CONTROL_WITH_CONFIG } from '../utils/constants'; +import { CONTROL, CONTROL_WITH_CONFIG, GET_TREATMENT, GET_TREATMENTS, GET_TREATMENTS_BY_FLAG_SET, GET_TREATMENTS_BY_FLAG_SETS, GET_TREATMENTS_WITH_CONFIG, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SET, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SETS, GET_TREATMENT_WITH_CONFIG, TRACK_FN_LABEL } from '../utils/constants'; import { IReadinessManager } from '../readiness/types'; import { MaybeThenable } from '../dtos/types'; import { ISettings, SplitIO } from '../types'; import { isStorageSync } from '../trackers/impressionObserver/utils'; -import { flagSetsAreValid } from '../utils/settingsValidation/splitFilters'; +import { validateFlagSets } from '../utils/settingsValidation/splitFilters'; /** * Decorator that validates the input before actually executing the client methods. @@ -32,7 +32,7 @@ export function clientInputValidationDecorator { test('returns the expected data from the cache', async () => { /** Setup: create manager */ - const connection = new Redis({}); + const connection = new RedisAdapter(loggerMock); const cache = new SplitsCacheInRedis(loggerMock, keys, connection); const manager = sdkManagerFactory({ mode: 'consumer', log: loggerMock }, cache, sdkReadinessManagerMock); await cache.clear(); diff --git a/src/sdkManager/index.ts b/src/sdkManager/index.ts index 7e82424a..c075418e 100644 --- a/src/sdkManager/index.ts +++ b/src/sdkManager/index.ts @@ -7,10 +7,7 @@ import { ISdkReadinessManager } from '../readiness/types'; import { ISplit } from '../dtos/types'; import { ISettings, SplitIO } from '../types'; import { isStorageSync } from '../trackers/impressionObserver/utils'; - -const SPLIT_FN_LABEL = 'split'; -const SPLITS_FN_LABEL = 'splits'; -const NAMES_FN_LABEL = 'names'; +import { SPLIT_FN_LABEL, SPLITS_FN_LABEL, NAMES_FN_LABEL } from '../utils/constants'; function collectTreatments(splitObject: ISplit) { const conditions = splitObject.conditions; diff --git a/src/storages/inRedis/EventsCacheInRedis.ts b/src/storages/inRedis/EventsCacheInRedis.ts index 20ae13af..ecd14a32 100644 --- a/src/storages/inRedis/EventsCacheInRedis.ts +++ b/src/storages/inRedis/EventsCacheInRedis.ts @@ -1,19 +1,19 @@ import { IEventsCacheAsync } from '../types'; import { IMetadata } from '../../dtos/types'; -import { Redis } from 'ioredis'; import { SplitIO } from '../../types'; import { ILogger } from '../../logger/types'; import { LOG_PREFIX } from './constants'; import { StoredEventWithMetadata } from '../../sync/submitters/types'; +import type { RedisAdapter } from './RedisAdapter'; export class EventsCacheInRedis implements IEventsCacheAsync { private readonly log: ILogger; private readonly key: string; - private readonly redis: Redis; + private readonly redis: RedisAdapter; private readonly metadata: IMetadata; - constructor(log: ILogger, key: string, redis: Redis, metadata: IMetadata) { + constructor(log: ILogger, key: string, redis: RedisAdapter, metadata: IMetadata) { this.log = log; this.key = key; this.redis = redis; diff --git a/src/storages/inRedis/ImpressionCountsCacheInRedis.ts b/src/storages/inRedis/ImpressionCountsCacheInRedis.ts index b0c563c0..efbb0e9b 100644 --- a/src/storages/inRedis/ImpressionCountsCacheInRedis.ts +++ b/src/storages/inRedis/ImpressionCountsCacheInRedis.ts @@ -1,19 +1,19 @@ -import { Redis } from 'ioredis'; import { ILogger } from '../../logger/types'; import { ImpressionCountsPayload } from '../../sync/submitters/types'; import { forOwn } from '../../utils/lang'; import { ImpressionCountsCacheInMemory } from '../inMemory/ImpressionCountsCacheInMemory'; import { LOG_PREFIX, REFRESH_RATE, TTL_REFRESH } from './constants'; +import type { RedisAdapter } from './RedisAdapter'; export class ImpressionCountsCacheInRedis extends ImpressionCountsCacheInMemory { private readonly log: ILogger; private readonly key: string; - private readonly redis: Redis; + private readonly redis: RedisAdapter; private readonly refreshRate: number; private intervalId: any; - constructor(log: ILogger, key: string, redis: Redis, impressionCountsCacheSize?: number, refreshRate = REFRESH_RATE) { + constructor(log: ILogger, key: string, redis: RedisAdapter, impressionCountsCacheSize?: number, refreshRate = REFRESH_RATE) { super(impressionCountsCacheSize); this.log = log; this.key = key; diff --git a/src/storages/inRedis/ImpressionsCacheInRedis.ts b/src/storages/inRedis/ImpressionsCacheInRedis.ts index 12ee277c..4ac0acaa 100644 --- a/src/storages/inRedis/ImpressionsCacheInRedis.ts +++ b/src/storages/inRedis/ImpressionsCacheInRedis.ts @@ -1,10 +1,10 @@ import { IImpressionsCacheAsync } from '../types'; import { IMetadata } from '../../dtos/types'; import { ImpressionDTO } from '../../types'; -import { Redis } from 'ioredis'; import { StoredImpressionWithMetadata } from '../../sync/submitters/types'; import { ILogger } from '../../logger/types'; import { impressionsToJSON } from '../utils'; +import type { RedisAdapter } from './RedisAdapter'; const IMPRESSIONS_TTL_REFRESH = 3600; // 1 hr @@ -12,10 +12,10 @@ export class ImpressionsCacheInRedis implements IImpressionsCacheAsync { private readonly log: ILogger; private readonly key: string; - private readonly redis: Redis; + private readonly redis: RedisAdapter; private readonly metadata: IMetadata; - constructor(log: ILogger, key: string, redis: Redis, metadata: IMetadata) { + constructor(log: ILogger, key: string, redis: RedisAdapter, metadata: IMetadata) { this.log = log; this.key = key; this.redis = redis; diff --git a/src/storages/inRedis/RedisAdapter.ts b/src/storages/inRedis/RedisAdapter.ts index 7beb2988..6d738606 100644 --- a/src/storages/inRedis/RedisAdapter.ts +++ b/src/storages/inRedis/RedisAdapter.ts @@ -1,4 +1,4 @@ -import ioredis from 'ioredis'; +import ioredis, { Pipeline } from 'ioredis'; import { ILogger } from '../../logger/types'; import { merge, isString } from '../../utils/lang'; import { _Set, setToArray, ISet } from '../../utils/lang/sets'; @@ -8,7 +8,8 @@ import { timeout } from '../../utils/promise/timeout'; const LOG_PREFIX = 'storage:redis-adapter: '; // If we ever decide to fully wrap every method, there's a Commander.getBuiltinCommands from ioredis. -const METHODS_TO_PROMISE_WRAP = ['set', 'exec', 'del', 'get', 'keys', 'sadd', 'srem', 'sismember', 'smembers', 'incr', 'rpush', 'pipeline', 'expire', 'mget', 'lrange', 'ltrim', 'hset']; +const METHODS_TO_PROMISE_WRAP = ['set', 'exec', 'del', 'get', 'keys', 'sadd', 'srem', 'sismember', 'smembers', 'incr', 'rpush', 'expire', 'mget', 'lrange', 'ltrim', 'hset', 'hincrby', 'popNRaw']; +const METHODS_TO_PROMISE_WRAP_EXEC = ['pipeline']; // Not part of the settings since it'll vary on each storage. We should be removing storage specific logic from elsewhere. const DEFAULT_OPTIONS = { @@ -38,7 +39,7 @@ export class RedisAdapter extends ioredis { private _notReadyCommandsQueue?: IRedisCommand[]; private _runningCommands: ISet>; - constructor(log: ILogger, storageSettings: Record) { + constructor(log: ILogger, storageSettings: Record = {}) { const options = RedisAdapter._defineOptions(storageSettings); // Call the ioredis constructor super(...RedisAdapter._defineLibrarySettings(options)); @@ -56,6 +57,7 @@ export class RedisAdapter extends ioredis { this.once('ready', () => { const commandsCount = this._notReadyCommandsQueue ? this._notReadyCommandsQueue.length : 0; this.log.info(LOG_PREFIX + `Redis connection established. Queued commands: ${commandsCount}.`); + this._notReadyCommandsQueue && this._notReadyCommandsQueue.forEach(queued => { this.log.info(LOG_PREFIX + `Executing queued ${queued.name} command.`); queued.command().then(queued.resolve).catch(queued.reject); @@ -71,16 +73,16 @@ export class RedisAdapter extends ioredis { _setTimeoutWrappers() { const instance: Record = this; - METHODS_TO_PROMISE_WRAP.forEach(method => { - const originalMethod = instance[method]; - - instance[method] = function () { + const wrapCommand = (originalMethod: Function, methodName: string) => { + // The value of "this" in this function should be the instance actually executing the method. It might be the instance referred (the base one) + // or it can be the instance of a Pipeline object. + return function (this: RedisAdapter | Pipeline) { const params = arguments; + const caller = this; function commandWrapper() { - instance.log.debug(LOG_PREFIX + `Executing ${method}.`); - // Return original method - const result = originalMethod.apply(instance, params); + instance.log.debug(`${LOG_PREFIX}Executing ${methodName}.`); + const result = originalMethod.apply(caller, params); if (thenable(result)) { // For handling pending commands on disconnect, add to the set and remove once finished. @@ -93,7 +95,7 @@ export class RedisAdapter extends ioredis { result.then(cleanUpRunningCommandsCb, cleanUpRunningCommandsCb); return timeout(instance._options.operationTimeout, result).catch(err => { - instance.log.error(LOG_PREFIX + `${method} operation threw an error or exceeded configured timeout of ${instance._options.operationTimeout}ms. Message: ${err}`); + instance.log.error(`${LOG_PREFIX}${methodName} operation threw an error or exceeded configured timeout of ${instance._options.operationTimeout}ms. Message: ${err}`); // Handling is not the adapter responsibility. throw err; }); @@ -103,18 +105,38 @@ export class RedisAdapter extends ioredis { } if (instance._notReadyCommandsQueue) { - return new Promise((res, rej) => { + return new Promise((resolve, reject) => { instance._notReadyCommandsQueue.unshift({ - resolve: res, - reject: rej, + resolve, + reject, command: commandWrapper, - name: method.toUpperCase() + name: methodName.toUpperCase() }); }); } else { return commandWrapper(); } }; + }; + + // Wrap regular async methods to track timeouts and queue when Redis is not yet executing commands. + METHODS_TO_PROMISE_WRAP.forEach(methodName => { + const originalFn = instance[methodName]; + instance[methodName] = wrapCommand(originalFn, methodName); + }); + + // Special handling for pipeline~like methods. We need to wrap the async trigger, which is exec, but return the Pipeline right away. + METHODS_TO_PROMISE_WRAP_EXEC.forEach(methodName => { + const originalFn = instance[methodName]; + // "First level wrapper" to handle the sync execution and wrap async, queueing later if applicable. + instance[methodName] = function () { + const res = originalFn.apply(instance, arguments); + const originalExec = res.exec; + + res.exec = wrapCommand(originalExec, methodName + '.exec').bind(res); + + return res; + }; }); } @@ -124,7 +146,7 @@ export class RedisAdapter extends ioredis { instance.disconnect = function disconnect(...params: []) { - setTimeout(function deferedDisconnect() { + setTimeout(function deferredDisconnect() { if (instance._runningCommands.size > 0) { instance.log.info(LOG_PREFIX + `Attempting to disconnect but there are ${instance._runningCommands.size} commands still waiting for resolution. Defering disconnection until those finish.`); diff --git a/src/storages/inRedis/SegmentsCacheInRedis.ts b/src/storages/inRedis/SegmentsCacheInRedis.ts index a144e2b7..7ec2f20f 100644 --- a/src/storages/inRedis/SegmentsCacheInRedis.ts +++ b/src/storages/inRedis/SegmentsCacheInRedis.ts @@ -1,17 +1,17 @@ -import { Redis } from 'ioredis'; import { ILogger } from '../../logger/types'; import { isNaNNumber } from '../../utils/lang'; import { LOG_PREFIX } from '../inLocalStorage/constants'; import { KeyBuilderSS } from '../KeyBuilderSS'; import { ISegmentsCacheAsync } from '../types'; +import type { RedisAdapter } from './RedisAdapter'; export class SegmentsCacheInRedis implements ISegmentsCacheAsync { private readonly log: ILogger; - private readonly redis: Redis; + private readonly redis: RedisAdapter; private readonly keys: KeyBuilderSS; - constructor(log: ILogger, keys: KeyBuilderSS, redis: Redis) { + constructor(log: ILogger, keys: KeyBuilderSS, redis: RedisAdapter) { this.log = log; this.redis = redis; this.keys = keys; @@ -72,8 +72,8 @@ export class SegmentsCacheInRedis implements ISegmentsCacheAsync { return this.redis.smembers(this.keys.buildRegisteredSegmentsKey()); } - // @TODO remove/review. It is not being used. + // @TODO remove or implement. It is not being used. clear() { - return this.redis.flushdb().then(status => status === 'OK'); + return Promise.resolve(); } } diff --git a/src/storages/inRedis/SplitsCacheInRedis.ts b/src/storages/inRedis/SplitsCacheInRedis.ts index 77582d23..e05c56ba 100644 --- a/src/storages/inRedis/SplitsCacheInRedis.ts +++ b/src/storages/inRedis/SplitsCacheInRedis.ts @@ -1,11 +1,11 @@ import { isFiniteNumber, isNaNNumber } from '../../utils/lang'; import { KeyBuilderSS } from '../KeyBuilderSS'; -import { Redis } from 'ioredis'; import { ILogger } from '../../logger/types'; import { LOG_PREFIX } from './constants'; import { ISplit, ISplitFiltersValidation } from '../../dtos/types'; import { AbstractSplitsCacheAsync } from '../AbstractSplitsCacheAsync'; import { ISet, _Set, returnListDifference } from '../../utils/lang/sets'; +import type { RedisAdapter } from './RedisAdapter'; /** * Discard errors for an answer of multiple operations. @@ -24,12 +24,12 @@ function processPipelineAnswer(results: Array<[Error | null, string]>): string[] export class SplitsCacheInRedis extends AbstractSplitsCacheAsync { private readonly log: ILogger; - private readonly redis: Redis; + private readonly redis: RedisAdapter; private readonly keys: KeyBuilderSS; private redisError?: string; private readonly flagSetsFilter: string[]; - constructor(log: ILogger, keys: KeyBuilderSS, redis: Redis, splitFiltersValidation?: ISplitFiltersValidation) { + constructor(log: ILogger, keys: KeyBuilderSS, redis: RedisAdapter, splitFiltersValidation?: ISplitFiltersValidation) { super(); this.log = log; this.redis = redis; @@ -213,7 +213,7 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync { /** * Get list of feature flag names related to a given list of flag set names. * The returned promise is resolved with the list of feature flag names per flag set, - * or rejected if the pipelined redis operation fails. + * or rejected if the pipelined redis operation fails (e.g., timeout). */ getNamesByFlagSets(flagSets: string[]): Promise[]> { return this.redis.pipeline(flagSets.map(flagSet => ['smembers', this.keys.buildFlagSetKey(flagSet)])).exec() @@ -252,13 +252,9 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync { }); } - /** - * Delete everything in the current database. - * - * @NOTE documentation says it never fails. - */ + // @TODO remove or implement. It is not being used. clear() { - return this.redis.flushdb().then(status => status === 'OK'); + return Promise.resolve(); } /** diff --git a/src/storages/inRedis/TelemetryCacheInRedis.ts b/src/storages/inRedis/TelemetryCacheInRedis.ts index ab50fbe5..e77400cc 100644 --- a/src/storages/inRedis/TelemetryCacheInRedis.ts +++ b/src/storages/inRedis/TelemetryCacheInRedis.ts @@ -3,13 +3,13 @@ import { Method, MultiConfigs, MultiMethodExceptions, MultiMethodLatencies } fro import { KeyBuilderSS } from '../KeyBuilderSS'; import { ITelemetryCacheAsync } from '../types'; import { findLatencyIndex } from '../findLatencyIndex'; -import { Redis } from 'ioredis'; import { getTelemetryConfigStats } from '../../sync/submitters/telemetrySubmitter'; import { CONSUMER_MODE, STORAGE_REDIS } from '../../utils/constants'; import { isNaNNumber, isString } from '../../utils/lang'; import { _Map } from '../../utils/lang/maps'; import { MAX_LATENCY_BUCKET_COUNT, newBuckets } from '../inMemory/TelemetryCacheInMemory'; import { parseLatencyField, parseExceptionField, parseMetadata } from '../utils'; +import type { RedisAdapter } from './RedisAdapter'; export class TelemetryCacheInRedis implements ITelemetryCacheAsync { @@ -19,7 +19,7 @@ export class TelemetryCacheInRedis implements ITelemetryCacheAsync { * @param keys Key builder. * @param redis Redis client. */ - constructor(private readonly log: ILogger, private readonly keys: KeyBuilderSS, private readonly redis: Redis) { } + constructor(private readonly log: ILogger, private readonly keys: KeyBuilderSS, private readonly redis: RedisAdapter) { } recordLatency(method: Method, latencyMs: number) { const [key, field] = this.keys.buildLatencyKey(method, findLatencyIndex(latencyMs)).split('::'); diff --git a/src/storages/inRedis/UniqueKeysCacheInRedis.ts b/src/storages/inRedis/UniqueKeysCacheInRedis.ts index c6dd71dd..6abdb88a 100644 --- a/src/storages/inRedis/UniqueKeysCacheInRedis.ts +++ b/src/storages/inRedis/UniqueKeysCacheInRedis.ts @@ -1,21 +1,21 @@ import { IUniqueKeysCacheBase } from '../types'; -import { Redis } from 'ioredis'; import { UniqueKeysCacheInMemory } from '../inMemory/UniqueKeysCacheInMemory'; import { setToArray } from '../../utils/lang/sets'; import { DEFAULT_CACHE_SIZE, REFRESH_RATE, TTL_REFRESH } from './constants'; import { LOG_PREFIX } from './constants'; import { ILogger } from '../../logger/types'; import { UniqueKeysItemSs } from '../../sync/submitters/types'; +import type { RedisAdapter } from './RedisAdapter'; export class UniqueKeysCacheInRedis extends UniqueKeysCacheInMemory implements IUniqueKeysCacheBase { private readonly log: ILogger; private readonly key: string; - private readonly redis: Redis; + private readonly redis: RedisAdapter; private readonly refreshRate: number; private intervalId: any; - constructor(log: ILogger, key: string, redis: Redis, uniqueKeysQueueSize = DEFAULT_CACHE_SIZE, refreshRate = REFRESH_RATE) { + constructor(log: ILogger, key: string, redis: RedisAdapter, uniqueKeysQueueSize = DEFAULT_CACHE_SIZE, refreshRate = REFRESH_RATE) { super(uniqueKeysQueueSize); this.log = log; this.key = key; diff --git a/src/storages/inRedis/__tests__/EventsCacheInRedis.spec.ts b/src/storages/inRedis/__tests__/EventsCacheInRedis.spec.ts index 134c746e..953b96d3 100644 --- a/src/storages/inRedis/__tests__/EventsCacheInRedis.spec.ts +++ b/src/storages/inRedis/__tests__/EventsCacheInRedis.spec.ts @@ -1,14 +1,14 @@ -import Redis from 'ioredis'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; import { EventsCacheInRedis } from '../EventsCacheInRedis'; import { metadata, fakeEvent1, fakeEvent1stored, fakeEvent2, fakeEvent2stored, fakeEvent3, fakeEvent3stored } from '../../pluggable/__tests__/EventsCachePluggable.spec'; +import { RedisAdapter } from '../RedisAdapter'; const prefix = 'events_cache_ut'; const eventsKey = `${prefix}.events`; const nonListKey = 'non-list-key'; test('EVENTS CACHE IN REDIS / `track`, `count`, `popNWithMetadata` and `drop` methods', async () => { - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); // Clean up in case there are still keys there. await connection.del(eventsKey); diff --git a/src/storages/inRedis/__tests__/ImpressionCountsCacheInRedis.spec.ts b/src/storages/inRedis/__tests__/ImpressionCountsCacheInRedis.spec.ts index a2f0452a..e9caf0fd 100644 --- a/src/storages/inRedis/__tests__/ImpressionCountsCacheInRedis.spec.ts +++ b/src/storages/inRedis/__tests__/ImpressionCountsCacheInRedis.spec.ts @@ -1,22 +1,23 @@ // @ts-nocheck import { ImpressionCountsCacheInRedis } from '../ImpressionCountsCacheInRedis'; import { truncateTimeFrame } from '../../../utils/time'; -import Redis from 'ioredis'; import { RedisMock } from '../../../utils/redis/RedisMock'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; +import { RedisAdapter } from '../RedisAdapter'; describe('IMPRESSION COUNTS CACHE IN REDIS', () => { const key = 'impression_count_post'; const timestamp = new Date(2020, 9, 2, 10, 10, 12).getTime(); const nextHourTimestamp = new Date(2020, 9, 2, 11, 10, 12).getTime(); - const expected = {}; - expected[`feature1::${truncateTimeFrame(timestamp)}`] = '3'; - expected[`feature1::${truncateTimeFrame(nextHourTimestamp)}`] = '3'; - expected[`feature2::${truncateTimeFrame(timestamp)}`] = '4'; - expected[`feature2::${truncateTimeFrame(nextHourTimestamp)}`] = '4'; + const expected = { + [`feature1::${truncateTimeFrame(timestamp)}`]: '3', + [`feature1::${truncateTimeFrame(nextHourTimestamp)}`]: '3', + [`feature2::${truncateTimeFrame(timestamp)}`]: '4', + [`feature2::${truncateTimeFrame(nextHourTimestamp)}`]: '4' + }; test('Impression Counter Test makeKey', async () => { - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); const counter = new ImpressionCountsCacheInRedis(loggerMock, key, connection); const timestamp1 = new Date(2020, 9, 2, 10, 0, 0).getTime(); @@ -29,7 +30,7 @@ describe('IMPRESSION COUNTS CACHE IN REDIS', () => { }); test('Impression Counter Test BasicUsage', async () => { - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); const counter = new ImpressionCountsCacheInRedis(loggerMock, key, connection); counter.track('feature1', timestamp, 1); @@ -80,7 +81,10 @@ describe('IMPRESSION COUNTS CACHE IN REDIS', () => { }); test('POST IMPRESSION COUNTS IN REDIS FUNCTION', async () => { - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); + // @TODO next line is not required with ioredis + await new Promise(res => connection.once('ready', res)); + const counter = new ImpressionCountsCacheInRedis(loggerMock, key, connection); // Clean up in case there are still keys there. connection.del(key); @@ -143,7 +147,7 @@ describe('IMPRESSION COUNTS CACHE IN REDIS', () => { }); test('Should call "onFullQueueCb" when the queue is full. "getImpressionsCount" should pop data.', async () => { - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); const counter = new ImpressionCountsCacheInRedis(loggerMock, key, connection, 5); // Clean up in case there are still keys there. await connection.del(key); diff --git a/src/storages/inRedis/__tests__/RedisAdapter.spec.ts b/src/storages/inRedis/__tests__/RedisAdapter.spec.ts index ecdf855a..a8ef69da 100644 --- a/src/storages/inRedis/__tests__/RedisAdapter.spec.ts +++ b/src/storages/inRedis/__tests__/RedisAdapter.spec.ts @@ -11,14 +11,28 @@ const LOG_PREFIX = 'storage:redis-adapter: '; // Mocking ioredis // The list of methods we're wrapping on a promise (for timeout) on the adapter. -const METHODS_TO_PROMISE_WRAP = ['set', 'exec', 'del', 'get', 'keys', 'sadd', 'srem', 'sismember', 'smembers', 'incr', 'rpush', 'pipeline', 'expire', 'mget', 'lrange', 'ltrim', 'hset']; +const METHODS_TO_PROMISE_WRAP = ['set', 'exec', 'del', 'get', 'keys', 'sadd', 'srem', 'sismember', 'smembers', 'incr', 'rpush', 'expire', 'mget', 'lrange', 'ltrim', 'hset', 'hincrby', 'popNRaw']; +const METHODS_TO_PROMISE_WRAP_EXEC = ['pipeline']; +const pipelineExecMock = jest.fn(() => Promise.resolve('exec')); const ioredisMock = reduce([...METHODS_TO_PROMISE_WRAP, 'disconnect'], (acc, methodName) => { acc[methodName] = jest.fn(() => Promise.resolve(methodName)); return acc; +}, reduce(METHODS_TO_PROMISE_WRAP_EXEC, (acc, methodName) => { + acc[methodName] = jest.fn(() => { + const pipelineAlikeMock = Object.assign(reduce(METHODS_TO_PROMISE_WRAP, (acc, methodName) => { + acc[methodName] = jest.fn(() => pipelineAlikeMock); + return acc; + }, {}), { + exec: pipelineExecMock + }); + + return pipelineAlikeMock; + }); + return acc; }, { once: jest.fn() -}) as { once: jest.Mock }; +}) as { once: jest.Mock }); let constructorParams: any = false; @@ -247,14 +261,16 @@ describe('STORAGE Redis Adapter', () => { url: 'redis://localhost:6379/0' }); - forEach(METHODS_TO_PROMISE_WRAP, methodName => { + forEach([...METHODS_TO_PROMISE_WRAP, ...METHODS_TO_PROMISE_WRAP_EXEC], methodName => { expect(instance[methodName]).not.toBe(ioredisMock[methodName]); // Method "${methodName}" from ioredis library should be wrapped. expect(ioredisMock[methodName]).not.toBeCalled(); // Checking that the method was not called yet. const startingQueueLength = instance._notReadyCommandsQueue.length; // We do have the commands queue on this state, so a call for this methods will queue the command. - const wrapperResult = instance[methodName](methodName); + const wrapperResult = METHODS_TO_PROMISE_WRAP_EXEC.includes(methodName) ? + instance[methodName](methodName).exec() : + instance[methodName](methodName); expect(wrapperResult instanceof Promise).toBe(true); // The result is a promise since we are queueing commands on this state. expect(instance._notReadyCommandsQueue.length).toBe(startingQueueLength + 1); // The queue should have one more item. @@ -263,19 +279,24 @@ describe('STORAGE Redis Adapter', () => { expect(typeof queuedCommand.resolve).toBe('function'); // The queued item should have the correct form. expect(typeof queuedCommand.reject).toBe('function'); // The queued item should have the correct form. expect(typeof queuedCommand.command).toBe('function'); // The queued item should have the correct form. - expect(queuedCommand.name).toBe(methodName.toUpperCase()); // The queued item should have the correct form. + expect(queuedCommand.name).toBe((METHODS_TO_PROMISE_WRAP_EXEC.includes(methodName) ? methodName + '.exec' : methodName).toUpperCase()); // The queued item should have the correct form. }); instance._notReadyCommandsQueue = false; // Remove the queue. loggerMock.error.resetHistory; - forEach(METHODS_TO_PROMISE_WRAP, (methodName, index) => { + forEach([...METHODS_TO_PROMISE_WRAP, ...METHODS_TO_PROMISE_WRAP_EXEC], (methodName, index) => { // We do NOT have the commands queue on this state, so a call for this methods will execute the command. - expect(ioredisMock[methodName]).not.toBeCalled(); // Control assertion - Original method (${methodName}) was not called yet + if (METHODS_TO_PROMISE_WRAP.includes(methodName)) expect(ioredisMock[methodName]).not.toBeCalled(); // Control assertion - Original method (${methodName}) was not called yet + else expect(pipelineExecMock).not.toBeCalled(); // Control assertion - Original Pipeline exec method was not called yet const previousTimeoutCalls = timeout.mock.calls.length; let previousRunningCommandsSize = instance._runningCommands.size; - instance[methodName](methodName).catch(() => { }); // Swallow exception so it's not spread to logs. + + (METHODS_TO_PROMISE_WRAP_EXEC.includes(methodName) ? + instance[methodName](methodName).exec() : + instance[methodName](methodName) + ).catch(() => { }); // Swallow exception so it's not spread to logs. expect(ioredisMock[methodName]).toBeCalled(); // Original method (${methodName}) is called right away (through wrapper) when we are not queueing anymore. expect(instance._runningCommands.size).toBe(previousRunningCommandsSize + 1); // If the result of the operation was a thenable it will add the item to the running commands queue. @@ -290,7 +311,7 @@ describe('STORAGE Redis Adapter', () => { commandTimeoutResolver.rej('test'); setTimeout(() => { // Allow the promises to tick. expect(instance._runningCommands.has(commandTimeoutResolver.originalPromise)).toBe(false); // After a command finishes with error, it's promise is removed from the instance._runningCommands queue. - expect(loggerMock.error.mock.calls[index]).toEqual([`${LOG_PREFIX}${methodName} operation threw an error or exceeded configured timeout of 5000ms. Message: test`]); // The log error method should be called with the corresponding messages, depending on the method, error and operationTimeout. + expect(loggerMock.error.mock.calls[index]).toEqual([`${LOG_PREFIX}${METHODS_TO_PROMISE_WRAP_EXEC.includes(methodName) ? methodName + '.exec' : methodName} operation threw an error or exceeded configured timeout of 5000ms. Message: test`]); // The log error method should be called with the corresponding messages, depending on the method, error and operationTimeout. }, 0); }); @@ -306,9 +327,10 @@ describe('STORAGE Redis Adapter', () => { instance._notReadyCommandsQueue = false; // Connection is "ready" - forEach(METHODS_TO_PROMISE_WRAP, methodName => { + forEach([...METHODS_TO_PROMISE_WRAP, ...METHODS_TO_PROMISE_WRAP_EXEC], methodName => { // Just call the wrapped method, we don't care about all the paths tested on the previous case, just how it behaves when the command is resolved. - instance[methodName](methodName); + if (METHODS_TO_PROMISE_WRAP_EXEC.includes(methodName)) instance[methodName](methodName).exec(); + else instance[methodName](methodName); // Get the original promise (the one passed to timeout) const commandTimeoutResolver = timeoutPromiseResolvers[0]; diff --git a/src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts b/src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts index 3e3bf503..6222af95 100644 --- a/src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts +++ b/src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts @@ -1,19 +1,18 @@ -import Redis from 'ioredis'; import { SegmentsCacheInRedis } from '../SegmentsCacheInRedis'; import { KeyBuilderSS } from '../../KeyBuilderSS'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; import { metadata } from '../../__tests__/KeyBuilder.spec'; +import { RedisAdapter } from '../RedisAdapter'; -const keys = new KeyBuilderSS('prefix', metadata); +const prefix = 'prefix'; +const keys = new KeyBuilderSS(prefix, metadata); describe('SEGMENTS CACHE IN REDIS', () => { test('isInSegment, set/getChangeNumber, add/removeFromSegment', async () => { - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); const cache = new SegmentsCacheInRedis(loggerMock, keys, connection); - await cache.clear(); - await cache.addToSegment('mocked-segment', ['a', 'b', 'c']); await cache.setChangeNumber('mocked-segment', 1); @@ -36,16 +35,15 @@ describe('SEGMENTS CACHE IN REDIS', () => { expect(await cache.isInSegment('mocked-segment', 'd')).toBe(true); expect(await cache.isInSegment('mocked-segment', 'e')).toBe(true); - await cache.clear(); + // Teardown + await connection.del(await connection.keys(`${prefix}.segment*`)); // @TODO use `cache.clear` method when implemented await connection.disconnect(); }); test('registerSegment / getRegisteredSegments', async () => { - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); const cache = new SegmentsCacheInRedis(loggerMock, keys, connection); - await cache.clear(); - await cache.registerSegments(['s1']); await cache.registerSegments(['s2']); await cache.registerSegments(['s2', 's3', 's4']); @@ -54,7 +52,8 @@ describe('SEGMENTS CACHE IN REDIS', () => { ['s1', 's2', 's3', 's4'].forEach(s => expect(segments.indexOf(s) !== -1).toBe(true)); - await cache.clear(); + // Teardown + await connection.del(await connection.keys(`${prefix}.segment*`)); // @TODO use `cache.clear` method when implemented await connection.disconnect(); }); diff --git a/src/storages/inRedis/__tests__/SplitsCacheInRedis.spec.ts b/src/storages/inRedis/__tests__/SplitsCacheInRedis.spec.ts index 638af5d1..d10db711 100644 --- a/src/storages/inRedis/__tests__/SplitsCacheInRedis.spec.ts +++ b/src/storages/inRedis/__tests__/SplitsCacheInRedis.spec.ts @@ -1,4 +1,3 @@ -import Redis from 'ioredis'; import { SplitsCacheInRedis } from '../SplitsCacheInRedis'; import { KeyBuilderSS } from '../../KeyBuilderSS'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; @@ -6,6 +5,7 @@ import { splitWithUserTT, splitWithAccountTT, featureFlagOne, featureFlagThree, import { ISplit } from '../../../dtos/types'; import { metadata } from '../../__tests__/KeyBuilder.spec'; import { _Set } from '../../../utils/lang/sets'; +import { RedisAdapter } from '../RedisAdapter'; const prefix = 'splits_cache_ut'; const keysBuilder = new KeyBuilderSS(prefix, metadata); @@ -13,7 +13,7 @@ const keysBuilder = new KeyBuilderSS(prefix, metadata); describe('SPLITS CACHE REDIS', () => { test('add/remove/get splits & set/get change number', async () => { - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); const cache = new SplitsCacheInRedis(loggerMock, keysBuilder, connection); await cache.addSplits([ @@ -52,6 +52,7 @@ describe('SPLITS CACHE REDIS', () => { expect(splits['lol1']).toEqual(null); expect(splits['lol2']).toEqual(splitWithAccountTT); + // Teardown. @TODO use cache clear method when implemented await connection.del(keysBuilder.buildTrafficTypeKey('account_tt')); await connection.del(keysBuilder.buildSplitKey('lol2')); await connection.del(keysBuilder.buildSplitsTillKey()); @@ -59,7 +60,7 @@ describe('SPLITS CACHE REDIS', () => { }); test('trafficTypeExists', async () => { - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); const cache = new SplitsCacheInRedis(loggerMock, keysBuilder, connection); await cache.addSplits([ @@ -100,6 +101,7 @@ describe('SPLITS CACHE REDIS', () => { expect(await cache.trafficTypeExists('account_tt')).toBe(true); expect(await cache.trafficTypeExists('user_tt')).toBe(false); + // Teardown. @TODO use cache clear method when implemented await connection.del(keysBuilder.buildTrafficTypeKey('account_tt')); await connection.del(keysBuilder.buildSplitKey('malformed')); await connection.del(keysBuilder.buildSplitKey('split1')); @@ -107,7 +109,7 @@ describe('SPLITS CACHE REDIS', () => { }); test('killLocally', async () => { - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); const cache = new SplitsCacheInRedis(loggerMock, keysBuilder, connection); await cache.addSplit('lol1', splitWithUserTT); @@ -145,7 +147,7 @@ describe('SPLITS CACHE REDIS', () => { }); test('flag set cache tests', async () => { - const connection = new Redis(); // @ts-ignore + const connection = new RedisAdapter(loggerMock); // @ts-ignore const cache = new SplitsCacheInRedis(loggerMock, keysBuilder, connection, { groupedFilters: { bySet: ['o', 'n', 'e', 'x'] } }); const emptySet = new _Set([]); @@ -173,11 +175,9 @@ describe('SPLITS CACHE REDIS', () => { expect(await cache.getNamesByFlagSets(['x'])).toEqual([new _Set(['ff_one'])]); expect(await cache.getNamesByFlagSets(['o', 'e', 'x'])).toEqual([new _Set(['ff_two']), new _Set(['ff_three']), new _Set(['ff_one'])]); - // @ts-ignore Simulate one error in connection.pipeline().exec() + // @ts-ignore Simulate an error in connection.pipeline().exec() jest.spyOn(connection, 'pipeline').mockImplementationOnce(() => { - return { - exec: () => Promise.resolve([['error', null], [null, ['ff_three']], [null, ['ff_one']]]), - }; + return { exec: () => Promise.resolve([['error', null], [null, ['ff_three']], [null, ['ff_one']]]) }; }); expect(await cache.getNamesByFlagSets(['o', 'e', 'x'])).toEqual([emptySet, new _Set(['ff_three']), new _Set(['ff_one'])]); (connection.pipeline as jest.Mock).mockRestore(); @@ -200,7 +200,7 @@ describe('SPLITS CACHE REDIS', () => { // if FlagSets filter is not defined, it should store all FlagSets in memory. test('flag set cache tests without filters', async () => { - const connection = new Redis(); // @ts-ignore + const connection = new RedisAdapter(loggerMock); const cacheWithoutFilters = new SplitsCacheInRedis(loggerMock, keysBuilder, connection); const emptySet = new _Set([]); diff --git a/src/storages/inRedis/__tests__/TelemetryCacheInRedis.spec.ts b/src/storages/inRedis/__tests__/TelemetryCacheInRedis.spec.ts index f55addd6..82685d18 100644 --- a/src/storages/inRedis/__tests__/TelemetryCacheInRedis.spec.ts +++ b/src/storages/inRedis/__tests__/TelemetryCacheInRedis.spec.ts @@ -1,9 +1,9 @@ -import Redis from 'ioredis'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; import { KeyBuilderSS } from '../../KeyBuilderSS'; import { TelemetryCacheInRedis } from '../TelemetryCacheInRedis'; import { newBuckets } from '../../inMemory/TelemetryCacheInMemory'; import { metadata } from '../../__tests__/KeyBuilder.spec'; +import { RedisAdapter } from '../RedisAdapter'; const prefix = 'telemetry_cache_ut'; const exceptionKey = `${prefix}.telemetry.exceptions`; @@ -14,7 +14,7 @@ const fieldVersionablePrefix = `${metadata.s}/${metadata.n}/${metadata.i}`; test('TELEMETRY CACHE IN REDIS', async () => { const keysBuilder = new KeyBuilderSS(prefix, metadata); - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); const cache = new TelemetryCacheInRedis(loggerMock, keysBuilder, connection); // recordException diff --git a/src/storages/inRedis/__tests__/UniqueKeysCacheInRedis.spec.ts b/src/storages/inRedis/__tests__/UniqueKeysCacheInRedis.spec.ts index e5f723cd..e1fa2148 100644 --- a/src/storages/inRedis/__tests__/UniqueKeysCacheInRedis.spec.ts +++ b/src/storages/inRedis/__tests__/UniqueKeysCacheInRedis.spec.ts @@ -1,15 +1,15 @@ //@ts-nocheck -import Redis from 'ioredis'; import { UniqueKeysCacheInRedis } from '../UniqueKeysCacheInRedis'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; import { RedisMock } from '../../../utils/redis/RedisMock'; +import { RedisAdapter } from '../RedisAdapter'; describe('UNIQUE KEYS CACHE IN REDIS', () => { const key = 'unique_key_post'; test('should incrementally store values, clear the queue, and tell if it is empty', async () => { - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); const cache = new UniqueKeysCacheInRedis(loggerMock, key, connection); @@ -51,7 +51,7 @@ describe('UNIQUE KEYS CACHE IN REDIS', () => { test('Should call "onFullQueueCb" when the queue is full.', async () => { let cbCalled = 0; - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); const cache = new UniqueKeysCacheInRedis(loggerMock, key, connection, 3); // small uniqueKeysCache size to be reached cache.setOnFullQueueCb(() => { cbCalled++; cache.clear(); }); @@ -79,7 +79,7 @@ describe('UNIQUE KEYS CACHE IN REDIS', () => { }); test('post unique keys in redis method', async () => { - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); const cache = new UniqueKeysCacheInRedis(loggerMock, key, connection, 20); cache.track('key1', 'feature1'); @@ -145,7 +145,9 @@ describe('UNIQUE KEYS CACHE IN REDIS', () => { }); test('Should call "onFullQueueCb" when the queue is full. "popNRaw" should pop items.', async () => { - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); + // @TODO next line is not required with ioredis + await new Promise(res => connection.once('ready', res)); const cache = new UniqueKeysCacheInRedis(loggerMock, key, connection, 3); diff --git a/src/utils/constants/index.ts b/src/utils/constants/index.ts index 18dc1ccb..d52a6e57 100644 --- a/src/utils/constants/index.ts +++ b/src/utils/constants/index.ts @@ -39,6 +39,22 @@ export const CONSENT_GRANTED = 'GRANTED'; // The user has granted consent for tr export const CONSENT_DECLINED = 'DECLINED'; // The user has declined consent for tracking events and impressions export const CONSENT_UNKNOWN = 'UNKNOWN'; // The user has neither granted nor declined consent for tracking events and impressions +// Client method names +export const GET_TREATMENT = 'getTreatment'; +export const GET_TREATMENTS = 'getTreatments'; +export const GET_TREATMENT_WITH_CONFIG = 'getTreatmentWithConfig'; +export const GET_TREATMENTS_WITH_CONFIG = 'getTreatmentsWithConfig'; +export const GET_TREATMENTS_BY_FLAG_SET = 'getTreatmentsByFlagSet'; +export const GET_TREATMENTS_BY_FLAG_SETS = 'getTreatmentsByFlagSets'; +export const GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SET = 'getTreatmentsWithConfigByFlagSet'; +export const GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SETS = 'getTreatmentsWithConfigByFlagSets'; +export const TRACK_FN_LABEL = 'track'; + +// Manager method names +export const SPLIT_FN_LABEL = 'split'; +export const SPLITS_FN_LABEL = 'splits'; +export const NAMES_FN_LABEL = 'names'; + // Telemetry export const QUEUED = 0; export const DROPPED = 1; diff --git a/src/utils/redis/RedisMock.ts b/src/utils/redis/RedisMock.ts index e4df1136..71c0c654 100644 --- a/src/utils/redis/RedisMock.ts +++ b/src/utils/redis/RedisMock.ts @@ -26,8 +26,6 @@ export class RedisMock { this.pipelineMethods[method] = this[method]; }); - this.pipeline = jest.fn(() => {return this.pipelineMethods;}); + this.pipeline = jest.fn(() => { return this.pipelineMethods; }); } - - } diff --git a/src/utils/settingsValidation/__tests__/splitFilters.spec.ts b/src/utils/settingsValidation/__tests__/splitFilters.spec.ts index 553c2604..d8cf6df0 100644 --- a/src/utils/settingsValidation/__tests__/splitFilters.spec.ts +++ b/src/utils/settingsValidation/__tests__/splitFilters.spec.ts @@ -5,7 +5,7 @@ import { STANDALONE_MODE, CONSUMER_MODE, CONSUMER_PARTIAL_MODE, LOCALHOST_MODE, import { splitFilters, queryStrings, groupedFilters } from '../../../__tests__/mocks/fetchSpecificSplits'; // Test target -import { flagSetsAreValid, validateSplitFilters } from '../splitFilters'; +import { validateFlagSets, validateSplitFilters } from '../splitFilters'; import { SETTINGS_SPLITS_FILTER, ERROR_INVALID, ERROR_EMPTY_ARRAY, ERROR_SETS_FILTER_EXCLUSIVE, WARN_SPLITS_FILTER_INVALID, WARN_SPLITS_FILTER_EMPTY, WARN_TRIMMING, WARN_SPLITS_FILTER_INVALID_SET, WARN_SPLITS_FILTER_LOWERCASE_SET, WARN_FLAGSET_NOT_CONFIGURED, WARN_SPLITS_FILTER_IGNORED } from '../../../logger/constants'; describe('validateSplitFilters', () => { @@ -131,71 +131,71 @@ describe('validateSplitFilters', () => { expect(loggerMock.error.mock.calls.length).toEqual(3); }); - test('flagSetsAreValid - Flag set validation for evaluations', () => { + test('validateFlagSets - Flag set validation for evaluations', () => { let flagSetsFilter = ['set_1', 'set_2']; // empty array - expect(flagSetsAreValid(loggerMock, 'test_method', [], flagSetsFilter)).toEqual([]); + expect(validateFlagSets(loggerMock, 'test_method', [], flagSetsFilter)).toEqual([]); // must start with a letter or number - expect(flagSetsAreValid(loggerMock, 'test_method', ['_set_1'], flagSetsFilter)).toEqual([]); + expect(validateFlagSets(loggerMock, 'test_method', ['_set_1'], flagSetsFilter)).toEqual([]); expect(loggerMock.warn.mock.calls[0]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['_set_1', regexp, '_set_1']]); // can contain _a-z0-9 - expect(flagSetsAreValid(loggerMock, 'test_method', ['set*1'], flagSetsFilter)).toEqual([]); + expect(validateFlagSets(loggerMock, 'test_method', ['set*1'], flagSetsFilter)).toEqual([]); expect(loggerMock.warn.mock.calls[1]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*1', regexp, 'set*1']]); // have a max length of 50 characters const longName = '1234567890_1234567890_1234567890_1234567890_1234567890'; - expect(flagSetsAreValid(loggerMock, 'test_method', [longName], flagSetsFilter)).toEqual([]); + expect(validateFlagSets(loggerMock, 'test_method', [longName], flagSetsFilter)).toEqual([]); expect(loggerMock.warn.mock.calls[2]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, [longName, regexp, longName]]); // both set names invalid -> empty list & warn - expect(flagSetsAreValid(loggerMock, 'test_method', ['set*1', 'set*3'], flagSetsFilter)).toEqual([]); + expect(validateFlagSets(loggerMock, 'test_method', ['set*1', 'set*3'], flagSetsFilter)).toEqual([]); expect(loggerMock.warn.mock.calls[3]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*1', regexp, 'set*1']]); expect(loggerMock.warn.mock.calls[4]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*3', regexp, 'set*3']]); // only set_1 is valid => [set_1] & warn - expect(flagSetsAreValid(loggerMock, 'test_method', ['set_1', 'set*3'], flagSetsFilter)).toEqual(['set_1']); + expect(validateFlagSets(loggerMock, 'test_method', ['set_1', 'set*3'], flagSetsFilter)).toEqual(['set_1']); expect(loggerMock.warn.mock.calls[5]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*3', regexp, 'set*3']]); // set_3 not included in configuration but set_1 included => [set_1] & warn - expect(flagSetsAreValid(loggerMock, 'test_method', ['set_1', 'set_3'], flagSetsFilter)).toEqual(['set_1']); + expect(validateFlagSets(loggerMock, 'test_method', ['set_1', 'set_3'], flagSetsFilter)).toEqual(['set_1']); expect(loggerMock.warn.mock.calls[6]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, ['test_method', 'set_3']]); // set_3 not included in configuration => [] & warn - expect(flagSetsAreValid(loggerMock, 'test_method', ['set_3'], flagSetsFilter)).toEqual([]); + expect(validateFlagSets(loggerMock, 'test_method', ['set_3'], flagSetsFilter)).toEqual([]); expect(loggerMock.warn.mock.calls[7]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, ['test_method', 'set_3']]); // empty config // must start with a letter or number - expect(flagSetsAreValid(loggerMock, 'test_method', ['_set_1'], [])).toEqual([]); + expect(validateFlagSets(loggerMock, 'test_method', ['_set_1'], [])).toEqual([]); expect(loggerMock.warn.mock.calls[8]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['_set_1', regexp, '_set_1']]); // can contain _a-z0-9 - expect(flagSetsAreValid(loggerMock, 'test_method', ['set*1'], [])).toEqual([]); + expect(validateFlagSets(loggerMock, 'test_method', ['set*1'], [])).toEqual([]); expect(loggerMock.warn.mock.calls[9]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*1', regexp, 'set*1']]); // have a max length of 50 characters - expect(flagSetsAreValid(loggerMock, 'test_method', [longName], [])).toEqual([]); + expect(validateFlagSets(loggerMock, 'test_method', [longName], [])).toEqual([]); expect(loggerMock.warn.mock.calls[10]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, [longName, regexp, longName]]); // both set names invalid -> empty list & warn - expect(flagSetsAreValid(loggerMock, 'test_method', ['set*1', 'set*3'], [])).toEqual([]); + expect(validateFlagSets(loggerMock, 'test_method', ['set*1', 'set*3'], [])).toEqual([]); expect(loggerMock.warn.mock.calls[11]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*1', regexp, 'set*1']]); expect(loggerMock.warn.mock.calls[12]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*3', regexp, 'set*3']]); // only set_1 is valid => [set_1] & warn - expect(flagSetsAreValid(loggerMock, 'test_method', ['set_1', 'set*3'], [])).toEqual(['set_1']); + expect(validateFlagSets(loggerMock, 'test_method', ['set_1', 'set*3'], [])).toEqual(['set_1']); expect(loggerMock.warn.mock.calls[13]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*3', regexp, 'set*3']]); // any set should be returned if there isn't flag sets in filter - expect(flagSetsAreValid(loggerMock, 'test_method', ['set_1'], [])).toEqual(['set_1']); - expect(flagSetsAreValid(loggerMock, 'test_method', ['set_1', 'set_2'], [])).toEqual(['set_1', 'set_2']); - expect(flagSetsAreValid(loggerMock, 'test_method', ['set_3'], [])).toEqual(['set_3']); + expect(validateFlagSets(loggerMock, 'test_method', ['set_1'], [])).toEqual(['set_1']); + expect(validateFlagSets(loggerMock, 'test_method', ['set_1', 'set_2'], [])).toEqual(['set_1', 'set_2']); + expect(validateFlagSets(loggerMock, 'test_method', ['set_3'], [])).toEqual(['set_3']); }); diff --git a/src/utils/settingsValidation/splitFilters.ts b/src/utils/settingsValidation/splitFilters.ts index a1d9e6eb..74548e36 100644 --- a/src/utils/settingsValidation/splitFilters.ts +++ b/src/utils/settingsValidation/splitFilters.ts @@ -88,7 +88,7 @@ function queryStringBuilder(groupedFilters: Record 0) {