diff --git a/CHANGES.txt b/CHANGES.txt index a893bd27..36b87426 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,8 @@ +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 wrap missing commands: 'hincrby', 'popNRaw', 'flushdb' and 'pipeline.exec'. + 1.11.0 (November 3, 2023) - Added support for Flag Sets on the SDK, which enables grouping feature flags and interacting with the group rather than individually (more details in our documentation): - Added new variations of the get treatment methods to support evaluating flags in given flag set/s. @@ -53,7 +58,7 @@ - Added a new impressions mode for the SDK called NONE, to be used in factory when there is no desire to capture impressions on an SDK factory to feed Split's analytics engine. Running NONE mode, the SDK will only capture unique keys evaluated for a particular feature flag instead of full blown impressions. - Updated SDK telemetry to support pluggable storage, partial consumer mode, and synchronizer. - Updated storage implementations to improve the performance of feature flag evaluations (i.e., `getTreatment(s)` method calls) when using the default storage in memory. - - Updated evaluation flow to avoid unnecessarily storage calls when the SDK is not ready. + - Updated evaluation flow (i.e., `getTreatment(s)` method calls) to avoid calling the storage for cached feature flags when the SDK is not ready or ready from cache. It applies to all SDK modes. 1.6.1 (July 22, 2022) - Updated GoogleAnalyticsToSplit integration to validate `autoRequire` config parameter and avoid some wrong warning logs when mapping GA hit fields to Split event properties. diff --git a/package-lock.json b/package-lock.json index 5c28d3a7..f131008e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.11.0", + "version": "1.12.1-rc.2", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-commons", - "version": "1.11.0", + "version": "1.12.1-rc.2", "license": "Apache-2.0", "dependencies": { "tslib": "^2.3.1" diff --git a/package.json b/package.json index 2fda9f0b..df3097c7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.11.0", + "version": "1.12.1-rc.2", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/src/evaluator/__tests__/evaluate-features.spec.ts b/src/evaluator/__tests__/evaluate-features.spec.ts index 3b81a37b..e3f1fa96 100644 --- a/src/evaluator/__tests__/evaluate-features.spec.ts +++ b/src/evaluator/__tests__/evaluate-features.spec.ts @@ -3,7 +3,7 @@ import { evaluateFeatures, evaluateFeaturesByFlagSets } from '../index'; import * as LabelsConstants from '../../utils/labels'; import { loggerMock } from '../../logger/__tests__/sdkLogger.mock'; import { _Set } from '../../utils/lang/sets'; -import { returnSetsUnion } from '../../utils/lang/sets'; +import { WARN_FLAGSET_WITHOUT_FLAGS } from '../../logger/constants'; const splitsMock = { regular: { 'changeNumber': 1487277320548, 'trafficAllocationSeed': 1667452163, 'trafficAllocation': 100, 'trafficTypeName': 'user', 'name': 'always-on', 'seed': 1684183541, 'configurations': {}, 'status': 'ACTIVE', 'killed': false, 'defaultTreatment': 'off', 'conditions': [{ 'conditionType': 'ROLLOUT', 'matcherGroup': { 'combiner': 'AND', 'matchers': [{ 'keySelector': { 'trafficType': 'user', 'attribute': '' }, 'matcherType': 'ALL_KEYS', 'negate': false, 'userDefinedSegmentMatcherData': { 'segmentName': '' }, 'unaryNumericMatcherData': { 'dataType': '', 'value': 0 }, 'whitelistMatcherData': { 'whitelist': null }, 'betweenMatcherData': { 'dataType': '', 'start': 0, 'end': 0 } }] }, 'partitions': [{ 'treatment': 'on', 'size': 100 }, { 'treatment': 'off', 'size': 0 }], 'label': 'in segment all' }] }, @@ -38,14 +38,7 @@ const mockStorage = { return splits; }, getNamesByFlagSets(flagSets) { - let toReturn = new _Set([]); - flagSets.forEach(flagset => { - const featureFlagNames = flagSetsMock[flagset]; - if (featureFlagNames) { - toReturn = returnSetsUnion(toReturn, featureFlagNames); - } - }); - return toReturn; + return flagSets.map(flagset => flagSetsMock[flagset] || new _Set()); } } }; @@ -123,7 +116,7 @@ test('EVALUATOR - Multiple evaluations at once / should return right labels, tre }); -test('EVALUATOR - Multiple evaluations at once by flag sets / should return right labels, treatments and configs if storage returns without errors.', async function () { +describe('EVALUATOR - Multiple evaluations at once by flag sets', () => { const expectedOutput = { config: { @@ -135,44 +128,76 @@ test('EVALUATOR - Multiple evaluations at once by flag sets / should return righ }, }; - const getResultsByFlagsets = (flagSets: string[]) => { + const getResultsByFlagsets = (flagSets: string[], storage = mockStorage) => { return evaluateFeaturesByFlagSets( loggerMock, 'fake-key', flagSets, null, - mockStorage, + storage, + 'method-name' ); }; - - - let multipleEvaluationAtOnceByFlagSets = await getResultsByFlagsets(['reg_and_config', 'arch_and_killed']); - - // assert evaluationWithConfig - expect(multipleEvaluationAtOnceByFlagSets['config']).toEqual(expectedOutput['config']); // If the split is retrieved successfully we should get the right evaluation result, label and config. - // @todo assert flag set not found - for input validations - - // assert regular - expect(multipleEvaluationAtOnceByFlagSets['regular']).toEqual({ ...expectedOutput['config'], config: null }); // If the split is retrieved successfully we should get the right evaluation result, label and config. If Split has no config it should have config equal null. - // assert killed - expect(multipleEvaluationAtOnceByFlagSets['killed']).toEqual({ ...expectedOutput['config'], treatment: 'off', config: null, label: LabelsConstants.SPLIT_KILLED }); - // 'If the split is retrieved but is killed, we should get the right evaluation result, label and config. - - // assert archived - expect(multipleEvaluationAtOnceByFlagSets['archived']).toEqual({ ...expectedOutput['config'], treatment: 'control', label: LabelsConstants.SPLIT_ARCHIVED, config: null }); - // If the split is retrieved but is archived, we should get the right evaluation result, label and config. - - // assert not_existent_split not in evaluation if it is not related to defined flag sets - expect(multipleEvaluationAtOnceByFlagSets['not_existent_split']).toEqual(undefined); - - multipleEvaluationAtOnceByFlagSets = await getResultsByFlagsets([]); - expect(multipleEvaluationAtOnceByFlagSets).toEqual({}); - - multipleEvaluationAtOnceByFlagSets = await getResultsByFlagsets(['reg_and_config']); - expect(multipleEvaluationAtOnceByFlagSets['config']).toEqual(expectedOutput['config']); - expect(multipleEvaluationAtOnceByFlagSets['regular']).toEqual({ ...expectedOutput['config'], config: null }); - expect(multipleEvaluationAtOnceByFlagSets['killed']).toEqual(undefined); - expect(multipleEvaluationAtOnceByFlagSets['archived']).toEqual(undefined); - + test('should return right labels, treatments and configs if storage returns without errors', async () => { + + let multipleEvaluationAtOnceByFlagSets = await getResultsByFlagsets(['reg_and_config', 'arch_and_killed']); + + // assert evaluationWithConfig + expect(multipleEvaluationAtOnceByFlagSets['config']).toEqual(expectedOutput['config']); // If the split is retrieved successfully we should get the right evaluation result, label and config. + // @todo assert flag set not found - for input validations + + // assert regular + expect(multipleEvaluationAtOnceByFlagSets['regular']).toEqual({ ...expectedOutput['config'], config: null }); // If the split is retrieved successfully we should get the right evaluation result, label and config. If Split has no config it should have config equal null. + // assert killed + expect(multipleEvaluationAtOnceByFlagSets['killed']).toEqual({ ...expectedOutput['config'], treatment: 'off', config: null, label: LabelsConstants.SPLIT_KILLED }); + // 'If the split is retrieved but is killed, we should get the right evaluation result, label and config. + + // assert archived + expect(multipleEvaluationAtOnceByFlagSets['archived']).toEqual({ ...expectedOutput['config'], treatment: 'control', label: LabelsConstants.SPLIT_ARCHIVED, config: null }); + // If the split is retrieved but is archived, we should get the right evaluation result, label and config. + + // assert not_existent_split not in evaluation if it is not related to defined flag sets + expect(multipleEvaluationAtOnceByFlagSets['not_existent_split']).toEqual(undefined); + + multipleEvaluationAtOnceByFlagSets = await getResultsByFlagsets([]); + expect(multipleEvaluationAtOnceByFlagSets).toEqual({}); + + multipleEvaluationAtOnceByFlagSets = await getResultsByFlagsets(['reg_and_config']); + expect(multipleEvaluationAtOnceByFlagSets['config']).toEqual(expectedOutput['config']); + expect(multipleEvaluationAtOnceByFlagSets['regular']).toEqual({ ...expectedOutput['config'], config: null }); + expect(multipleEvaluationAtOnceByFlagSets['killed']).toEqual(undefined); + expect(multipleEvaluationAtOnceByFlagSets['archived']).toEqual(undefined); + }); + + test('should log a warning if evaluating with flag sets that doesn\'t contain cached feature flags', async () => { + const getSplitsSpy = jest.spyOn(mockStorage.splits, 'getSplits'); + + // No flag set contains cached feature flags -> getSplits method is not called + expect(getResultsByFlagsets(['inexistent_set1', 'inexistent_set2'])).toEqual({}); + expect(getSplitsSpy).not.toHaveBeenCalled(); + expect(loggerMock.warn.mock.calls).toEqual([ + [WARN_FLAGSET_WITHOUT_FLAGS, ['method-name', 'inexistent_set1']], + [WARN_FLAGSET_WITHOUT_FLAGS, ['method-name', 'inexistent_set2']], + ]); + + // One flag set contains cached feature flags -> getSplits method is called + expect(getResultsByFlagsets(['inexistent_set3', 'reg_and_config'])).toEqual(getResultsByFlagsets(['reg_and_config'])); + expect(getSplitsSpy).toHaveBeenLastCalledWith(['regular', 'config']); + expect(loggerMock.warn).toHaveBeenLastCalledWith(WARN_FLAGSET_WITHOUT_FLAGS, ['method-name', 'inexistent_set3']); + + getSplitsSpy.mockRestore(); + loggerMock.warn.mockClear(); + + // Should support async storage too + expect(await getResultsByFlagsets(['inexistent_set1', 'inexistent_set2'], { + splits: { + getNamesByFlagSets(flagSets) { return Promise.resolve(flagSets.map(flagset => flagSetsMock[flagset] || new _Set())); } + } + })).toEqual({}); + expect(loggerMock.warn.mock.calls).toEqual([ + [WARN_FLAGSET_WITHOUT_FLAGS, ['method-name', 'inexistent_set1']], + [WARN_FLAGSET_WITHOUT_FLAGS, ['method-name', 'inexistent_set2']], + ]); + }); }); diff --git a/src/evaluator/index.ts b/src/evaluator/index.ts index 62f82e98..ffcc9321 100644 --- a/src/evaluator/index.ts +++ b/src/evaluator/index.ts @@ -7,7 +7,8 @@ import { IStorageAsync, IStorageSync } from '../storages/types'; import { IEvaluationResult } from './types'; import { SplitIO } from '../types'; import { ILogger } from '../logger/types'; -import { ISet, setToArray } from '../utils/lang/sets'; +import { ISet, setToArray, returnSetsUnion, _Set } from '../utils/lang/sets'; +import { WARN_FLAGSET_WITHOUT_FLAGS } from '../logger/constants'; const treatmentException = { treatment: CONTROL, @@ -94,8 +95,27 @@ export function evaluateFeaturesByFlagSets( flagSets: string[], attributes: SplitIO.Attributes | undefined, storage: IStorageSync | IStorageAsync, + method: string, ): MaybeThenable> { - let storedFlagNames: MaybeThenable>; + let storedFlagNames: MaybeThenable[]>; + + function evaluate( + featureFlagsByFlagSets: ISet[], + ) { + let featureFlags = new _Set(); + for (let i = 0; i < flagSets.length; i++) { + const featureFlagByFlagSet = featureFlagsByFlagSets[i]; + if (featureFlagByFlagSet.size) { + featureFlags = returnSetsUnion(featureFlags, featureFlagByFlagSet); + } else { + log.warn(WARN_FLAGSET_WITHOUT_FLAGS, [method, flagSets[i]]); + } + } + + return featureFlags.size ? + evaluateFeatures(log, key, setToArray(featureFlags), attributes, storage) : + {}; + } // get features by flag sets try { @@ -107,11 +127,11 @@ export function evaluateFeaturesByFlagSets( // evaluate related features return thenable(storedFlagNames) ? - storedFlagNames.then((splitNames) => evaluateFeatures(log, key, setToArray(splitNames), attributes, storage)) + storedFlagNames.then((storedFlagNames) => evaluate(storedFlagNames)) .catch(() => { return {}; }) : - evaluateFeatures(log, key, setToArray(storedFlagNames), attributes, storage); + evaluate(storedFlagNames); } function getEvaluation( diff --git a/src/logger/constants.ts b/src/logger/constants.ts index 18c86f9f..91e601fc 100644 --- a/src/logger/constants.ts +++ b/src/logger/constants.ts @@ -91,6 +91,7 @@ export const WARN_NOT_EXISTENT_SPLIT = 215; export const WARN_LOWERCASE_TRAFFIC_TYPE = 216; export const WARN_NOT_EXISTENT_TT = 217; export const WARN_INTEGRATION_INVALID = 218; +export const WARN_SPLITS_FILTER_IGNORED = 219; export const WARN_SPLITS_FILTER_INVALID = 220; export const WARN_SPLITS_FILTER_EMPTY = 221; export const WARN_SDK_KEY = 222; @@ -99,6 +100,7 @@ export const STREAMING_PARSING_SPLIT_UPDATE = 224; export const WARN_SPLITS_FILTER_INVALID_SET = 225; export const WARN_SPLITS_FILTER_LOWERCASE_SET = 226; export const WARN_FLAGSET_NOT_CONFIGURED = 227; +export const WARN_FLAGSET_WITHOUT_FLAGS = 228; export const ERROR_ENGINE_COMBINER_IFELSEIF = 300; export const ERROR_LOGLEVEL_INVALID = 301; diff --git a/src/logger/messages/warn.ts b/src/logger/messages/warn.ts index 8c6f60ce..3feca12f 100644 --- a/src/logger/messages/warn.ts +++ b/src/logger/messages/warn.ts @@ -24,15 +24,17 @@ export const codesWarn: [number, string][] = codesError.concat([ [c.WARN_NOT_EXISTENT_SPLIT, '%s: feature flag "%s" does not exist in this environment. Please double check what feature flags exist in the Split user interface.'], [c.WARN_LOWERCASE_TRAFFIC_TYPE, '%s: traffic_type_name should be all lowercase - converting string to lowercase.'], [c.WARN_NOT_EXISTENT_TT, '%s: traffic type "%s" does not have any corresponding feature flag in this environment, make sure you\'re tracking your events to a valid traffic type defined in the Split user interface.'], - [c.WARN_FLAGSET_NOT_CONFIGURED, '%s: : you passed %s wich is not part of the configured FlagSetsFilter, ignoring Flag Set.'], + [c.WARN_FLAGSET_NOT_CONFIGURED, '%s: you passed %s which is not part of the configured FlagSetsFilter, ignoring Flag Set.'], // initialization / settings validation - [c.WARN_INTEGRATION_INVALID, c.LOG_PREFIX_SETTINGS+': %s integration item(s) at settings is invalid. %s'], - [c.WARN_SPLITS_FILTER_INVALID, c.LOG_PREFIX_SETTINGS+': feature flag filter at position %s is invalid. It must be an object with a valid filter type ("bySet", "byName" or "byPrefix") and a list of "values".'], - [c.WARN_SPLITS_FILTER_EMPTY, c.LOG_PREFIX_SETTINGS+': feature flag filter configuration must be a non-empty array of filter objects.'], - [c.WARN_SDK_KEY, c.LOG_PREFIX_SETTINGS+': You already have %s. We recommend keeping only one instance of the factory at all times (Singleton pattern) and reusing it throughout your application'], + [c.WARN_INTEGRATION_INVALID, c.LOG_PREFIX_SETTINGS + ': %s integration item(s) at settings is invalid. %s'], + [c.WARN_SPLITS_FILTER_IGNORED, c.LOG_PREFIX_SETTINGS + ': feature flag filters are not applicable for Consumer modes where the SDK does not keep rollout data in sync. Filters were discarded'], + [c.WARN_SPLITS_FILTER_INVALID, c.LOG_PREFIX_SETTINGS + ': feature flag filter at position %s is invalid. It must be an object with a valid filter type ("bySet", "byName" or "byPrefix") and a list of "values".'], + [c.WARN_SPLITS_FILTER_EMPTY, c.LOG_PREFIX_SETTINGS + ': feature flag filter configuration must be a non-empty array of filter objects.'], + [c.WARN_SDK_KEY, c.LOG_PREFIX_SETTINGS + ': You already have %s. We recommend keeping only one instance of the factory at all times (Singleton pattern) and reusing it throughout your application'], [c.STREAMING_PARSING_MY_SEGMENTS_UPDATE_V2, c.LOG_PREFIX_SYNC_STREAMING + 'Fetching MySegments due to an error processing %s notification: %s'], [c.STREAMING_PARSING_SPLIT_UPDATE, c.LOG_PREFIX_SYNC_STREAMING + 'Fetching SplitChanges due to an error processing SPLIT_UPDATE notification: %s'], - [c.WARN_SPLITS_FILTER_INVALID_SET, c.LOG_PREFIX_SETTINGS+': you passed %s, flag set must adhere to the regular expressions %s. This means a flag set must start with a letter or number, be in lowercase, alphanumeric and have a max length of 50 characters. %s was discarded.'], - [c.WARN_SPLITS_FILTER_LOWERCASE_SET, c.LOG_PREFIX_SETTINGS+': flag set %s should be all lowercase - converting string to lowercase.'], + [c.WARN_SPLITS_FILTER_INVALID_SET, c.LOG_PREFIX_SETTINGS + ': you passed %s, flag set must adhere to the regular expressions %s. This means a flag set must start with a letter or number, be in lowercase, alphanumeric and have a max length of 50 characters. %s was discarded.'], + [c.WARN_SPLITS_FILTER_LOWERCASE_SET, c.LOG_PREFIX_SETTINGS + ': flag set %s should be all lowercase - converting string to lowercase.'], + [c.WARN_FLAGSET_WITHOUT_FLAGS, '%s: you passed %s flag set that does not contain cached feature flag names. Please double check what flag sets are in use in the Split user interface.'], ]); diff --git a/src/readiness/__tests__/sdkReadinessManager.spec.ts b/src/readiness/__tests__/sdkReadinessManager.spec.ts index 3c93b7a6..9c407052 100644 --- a/src/readiness/__tests__/sdkReadinessManager.spec.ts +++ b/src/readiness/__tests__/sdkReadinessManager.spec.ts @@ -49,7 +49,7 @@ describe('SDK Readiness Manager - Event emitter', () => { expect(sdkStatus[propName]).toBeTruthy(); // The sdkStatus exposes all minimal EventEmitter functionality. }); - expect(typeof sdkStatus['ready']).toBe('function'); // The sdkStatus exposes a .ready() function. + expect(typeof sdkStatus.ready).toBe('function'); // The sdkStatus exposes a .ready() function. expect(typeof sdkStatus.Event).toBe('object'); // It also exposes the Event map, expect(sdkStatus.Event.SDK_READY).toBe(SDK_READY); // which contains the constants for the events, for backwards compatibility. diff --git a/src/sdkClient/client.ts b/src/sdkClient/client.ts index 31aa5f74..1663af92 100644 --- a/src/sdkClient/client.ts +++ b/src/sdkClient/client.ts @@ -1,8 +1,8 @@ import { evaluateFeature, evaluateFeatures, evaluateFeaturesByFlagSets } from '../evaluator'; import { thenable } from '../utils/promise/thenable'; import { getMatching, getBucketing } from '../utils/key'; -import { validateSplitExistance } from '../utils/inputValidation/splitExistance'; -import { validateTrafficTypeExistance } from '../utils/inputValidation/trafficTypeExistance'; +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 { IEvaluationResult } from '../evaluator/types'; @@ -99,7 +99,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl }; const evaluations = readinessManager.isReady() || readinessManager.isReadyFromCache() ? - evaluateFeaturesByFlagSets(log, key, flagSetNames, attributes, storage) : + evaluateFeaturesByFlagSets(log, key, flagSetNames, attributes, storage, method) : isStorageSync(settings) ? {} : Promise.resolve({}); // Promisify if async return thenable(evaluations) ? evaluations.then((res) => wrapUp(res)) : wrapUp(evaluations); @@ -133,7 +133,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl const { treatment, label, changeNumber, config = null } = evaluation; log.info(IMPRESSION, [featureFlagName, matchingKey, treatment, label]); - if (validateSplitExistance(log, readinessManager, featureFlagName, label, invokingMethodName)) { + if (validateSplitExistence(log, readinessManager, featureFlagName, label, invokingMethodName)) { log.info(IMPRESSION_QUEUEING); queue.push({ feature: featureFlagName, @@ -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. - validateTrafficTypeExistance(log, readinessManager, storage.splits, mode, trafficTypeName, 'track'); + validateTrafficTypeExistence(log, readinessManager, storage.splits, mode, trafficTypeName, 'track'); const result = eventTracker.track(eventData, size); diff --git a/src/sdkManager/__tests__/index.asyncCache.spec.ts b/src/sdkManager/__tests__/index.asyncCache.spec.ts index 4081c10b..71c656c2 100644 --- a/src/sdkManager/__tests__/index.asyncCache.spec.ts +++ b/src/sdkManager/__tests__/index.asyncCache.spec.ts @@ -1,4 +1,3 @@ -import Redis from 'ioredis'; import splitObject from './mocks/input.json'; import splitView from './mocks/output.json'; import { sdkManagerFactory } from '../index'; @@ -9,6 +8,7 @@ import { KeyBuilderSS } from '../../storages/KeyBuilderSS'; import { ISdkReadinessManager } from '../../readiness/types'; import { loggerMock } from '../../logger/__tests__/sdkLogger.mock'; import { metadata } from '../../storages/__tests__/KeyBuilder.spec'; +import { RedisAdapter } from '../../storages/inRedis/RedisAdapter'; // @ts-expect-error const sdkReadinessManagerMock = { @@ -28,7 +28,7 @@ describe('MANAGER API', () => { test('Async cache (In Redis)', async () => { /** Setup: create manager */ - const connection = new Redis({}); + const connection = new RedisAdapter(loggerMock); const cache = new SplitsCacheInRedis(loggerMock, keys, connection); const manager = sdkManagerFactory(loggerMock, cache, sdkReadinessManagerMock); await cache.clear(); @@ -70,7 +70,7 @@ describe('MANAGER API', () => { /** Teardown */ await cache.removeSplit(splitObject.name); - await connection.quit(); + await connection.disconnect(); }); test('Async cache with error', async () => { diff --git a/src/sdkManager/index.ts b/src/sdkManager/index.ts index 1fec8784..5ca1386e 100644 --- a/src/sdkManager/index.ts +++ b/src/sdkManager/index.ts @@ -1,7 +1,7 @@ import { objectAssign } from '../utils/lang/objectAssign'; import { thenable } from '../utils/promise/thenable'; import { find } from '../utils/lang'; -import { validateSplit, validateSplitExistance, validateIfNotDestroyed, validateIfOperational } from '../utils/inputValidation'; +import { validateSplit, validateSplitExistence, validateIfNotDestroyed, validateIfOperational } from '../utils/inputValidation'; import { ISplitsCacheAsync, ISplitsCacheSync } from '../storages/types'; import { ISdkReadinessManager } from '../readiness/types'; import { ISplit } from '../dtos/types'; @@ -71,12 +71,12 @@ export function sdkManagerFactory null).then(result => { // handle possible rejections when using pluggable storage - validateSplitExistance(log, readinessManager, splitName, result, SPLIT_FN_LABEL); + validateSplitExistence(log, readinessManager, splitName, result, SPLIT_FN_LABEL); return objectToView(result); }); } - validateSplitExistance(log, readinessManager, splitName, split, SPLIT_FN_LABEL); + validateSplitExistence(log, readinessManager, splitName, split, SPLIT_FN_LABEL); return objectToView(split); }, diff --git a/src/storages/AbstractSplitsCacheAsync.ts b/src/storages/AbstractSplitsCacheAsync.ts index bbd33e78..9e4e136c 100644 --- a/src/storages/AbstractSplitsCacheAsync.ts +++ b/src/storages/AbstractSplitsCacheAsync.ts @@ -18,7 +18,7 @@ export abstract class AbstractSplitsCacheAsync implements ISplitsCacheAsync { abstract getChangeNumber(): Promise abstract getAll(): Promise abstract getSplitNames(): Promise - abstract getNamesByFlagSets(flagSets: string[]): Promise> + abstract getNamesByFlagSets(flagSets: string[]): Promise[]> abstract trafficTypeExists(trafficType: string): Promise abstract clear(): Promise diff --git a/src/storages/AbstractSplitsCacheSync.ts b/src/storages/AbstractSplitsCacheSync.ts index 92a29c18..841ba26e 100644 --- a/src/storages/AbstractSplitsCacheSync.ts +++ b/src/storages/AbstractSplitsCacheSync.ts @@ -79,7 +79,7 @@ export abstract class AbstractSplitsCacheSync implements ISplitsCacheSync { return false; } - abstract getNamesByFlagSets(flagSets: string[]): ISet + abstract getNamesByFlagSets(flagSets: string[]): ISet[] } diff --git a/src/storages/inLocalStorage/SplitsCacheInLocal.ts b/src/storages/inLocalStorage/SplitsCacheInLocal.ts index 9e02ee15..acca8f3c 100644 --- a/src/storages/inLocalStorage/SplitsCacheInLocal.ts +++ b/src/storages/inLocalStorage/SplitsCacheInLocal.ts @@ -4,7 +4,7 @@ import { isFiniteNumber, toNumber, isNaNNumber } from '../../utils/lang'; import { KeyBuilderCS } from '../KeyBuilderCS'; import { ILogger } from '../../logger/types'; import { LOG_PREFIX } from './constants'; -import { ISet, _Set, returnSetsUnion, setToArray } from '../../utils/lang/sets'; +import { ISet, _Set, setToArray } from '../../utils/lang/sets'; /** * ISplitsCacheSync implementation that stores split definitions in browser LocalStorage. @@ -257,19 +257,13 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { // if the filter didn't change, nothing is done } - getNamesByFlagSets(flagSets: string[]): ISet{ - let toReturn: ISet = new _Set([]); - flagSets.forEach(flagSet => { + getNamesByFlagSets(flagSets: string[]): ISet[] { + return flagSets.map(flagSet => { const flagSetKey = this.keys.buildFlagSetKey(flagSet); - let flagSetFromLocalStorage = localStorage.getItem(flagSetKey); + const flagSetFromLocalStorage = localStorage.getItem(flagSetKey); - if (flagSetFromLocalStorage) { - const flagSetCache = new _Set(JSON.parse(flagSetFromLocalStorage)); - toReturn = returnSetsUnion(toReturn, flagSetCache); - } + return new _Set(flagSetFromLocalStorage ? JSON.parse(flagSetFromLocalStorage) : []); }); - return toReturn; - } private addToFlagSets(featureFlag: ISplit) { @@ -281,10 +275,9 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { const flagSetKey = this.keys.buildFlagSetKey(featureFlagSet); - let flagSetFromLocalStorage = localStorage.getItem(flagSetKey); - if (!flagSetFromLocalStorage) flagSetFromLocalStorage = '[]'; + const flagSetFromLocalStorage = localStorage.getItem(flagSetKey); - const flagSetCache = new _Set(JSON.parse(flagSetFromLocalStorage)); + const flagSetCache = new _Set(flagSetFromLocalStorage ? JSON.parse(flagSetFromLocalStorage) : []); flagSetCache.add(featureFlag.name); localStorage.setItem(flagSetKey, JSON.stringify(setToArray(flagSetCache))); @@ -302,7 +295,7 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { private removeNames(flagSetName: string, featureFlagName: string) { const flagSetKey = this.keys.buildFlagSetKey(flagSetName); - let flagSetFromLocalStorage = localStorage.getItem(flagSetKey); + const flagSetFromLocalStorage = localStorage.getItem(flagSetKey); if (!flagSetFromLocalStorage) return; diff --git a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts index b40f5f8a..1d5bc441 100644 --- a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts +++ b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts @@ -174,32 +174,32 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests', () => { ]); cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS); - expect(cache.getNamesByFlagSets(['o'])).toEqual(new _Set(['ff_one', 'ff_two'])); - expect(cache.getNamesByFlagSets(['n'])).toEqual(new _Set(['ff_one'])); - expect(cache.getNamesByFlagSets(['e'])).toEqual(new _Set(['ff_one','ff_three'])); - expect(cache.getNamesByFlagSets(['t'])).toEqual(emptySet); // 't' not in filter - expect(cache.getNamesByFlagSets(['o','n','e'])).toEqual(new _Set(['ff_one','ff_two','ff_three'])); + expect(cache.getNamesByFlagSets(['o'])).toEqual([new _Set(['ff_one', 'ff_two'])]); + expect(cache.getNamesByFlagSets(['n'])).toEqual([new _Set(['ff_one'])]); + expect(cache.getNamesByFlagSets(['e'])).toEqual([new _Set(['ff_one', 'ff_three'])]); + expect(cache.getNamesByFlagSets(['t'])).toEqual([emptySet]); // 't' not in filter + expect(cache.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new _Set(['ff_one', 'ff_two']), new _Set(['ff_one']), new _Set(['ff_one', 'ff_three'])]); - cache.addSplit(featureFlagOne.name, {...featureFlagOne, sets: ['1']}); + cache.addSplit(featureFlagOne.name, { ...featureFlagOne, sets: ['1'] }); - expect(cache.getNamesByFlagSets(['1'])).toEqual(emptySet); // '1' not in filter - expect(cache.getNamesByFlagSets(['o'])).toEqual(new _Set(['ff_two'])); - expect(cache.getNamesByFlagSets(['n'])).toEqual(emptySet); + expect(cache.getNamesByFlagSets(['1'])).toEqual([emptySet]); // '1' not in filter + expect(cache.getNamesByFlagSets(['o'])).toEqual([new _Set(['ff_two'])]); + expect(cache.getNamesByFlagSets(['n'])).toEqual([emptySet]); - cache.addSplit(featureFlagOne.name, {...featureFlagOne, sets: ['x']}); - expect(cache.getNamesByFlagSets(['x'])).toEqual(new _Set(['ff_one'])); - expect(cache.getNamesByFlagSets(['o','e','x'])).toEqual(new _Set(['ff_one','ff_two','ff_three'])); + cache.addSplit(featureFlagOne.name, { ...featureFlagOne, sets: ['x'] }); + expect(cache.getNamesByFlagSets(['x'])).toEqual([new _Set(['ff_one'])]); + expect(cache.getNamesByFlagSets(['o', 'e', 'x'])).toEqual([new _Set(['ff_two']), new _Set(['ff_three']), new _Set(['ff_one'])]); cache.removeSplit(featureFlagOne.name); - expect(cache.getNamesByFlagSets(['x'])).toEqual(emptySet); + expect(cache.getNamesByFlagSets(['x'])).toEqual([emptySet]); cache.removeSplit(featureFlagOne.name); - expect(cache.getNamesByFlagSets(['y'])).toEqual(emptySet); // 'y' not in filter - expect(cache.getNamesByFlagSets([])).toEqual(emptySet); + expect(cache.getNamesByFlagSets(['y'])).toEqual([emptySet]); // 'y' not in filter + expect(cache.getNamesByFlagSets([])).toEqual([]); cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithoutFS); - expect(cache.getNamesByFlagSets([])).toEqual(emptySet); + expect(cache.getNamesByFlagSets([])).toEqual([]); }); // if FlagSets are not defined, it should store all FlagSets in memory. @@ -214,10 +214,10 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests without filters', () => ]); cacheWithoutFilters.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS); - expect(cacheWithoutFilters.getNamesByFlagSets(['o'])).toEqual(new _Set(['ff_one', 'ff_two'])); - expect(cacheWithoutFilters.getNamesByFlagSets(['n'])).toEqual(new _Set(['ff_one'])); - expect(cacheWithoutFilters.getNamesByFlagSets(['e'])).toEqual(new _Set(['ff_one','ff_three'])); - expect(cacheWithoutFilters.getNamesByFlagSets(['t'])).toEqual(new _Set(['ff_two','ff_three'])); - expect(cacheWithoutFilters.getNamesByFlagSets(['y'])).toEqual(emptySet); - expect(cacheWithoutFilters.getNamesByFlagSets(['o','n','e'])).toEqual(new _Set(['ff_one','ff_two','ff_three'])); + expect(cacheWithoutFilters.getNamesByFlagSets(['o'])).toEqual([new _Set(['ff_one', 'ff_two'])]); + expect(cacheWithoutFilters.getNamesByFlagSets(['n'])).toEqual([new _Set(['ff_one'])]); + expect(cacheWithoutFilters.getNamesByFlagSets(['e'])).toEqual([new _Set(['ff_one', 'ff_three'])]); + expect(cacheWithoutFilters.getNamesByFlagSets(['t'])).toEqual([new _Set(['ff_two', 'ff_three'])]); + expect(cacheWithoutFilters.getNamesByFlagSets(['y'])).toEqual([emptySet]); + expect(cacheWithoutFilters.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new _Set(['ff_one', 'ff_two']), new _Set(['ff_one']), new _Set(['ff_one', 'ff_three'])]); }); diff --git a/src/storages/inMemory/SplitsCacheInMemory.ts b/src/storages/inMemory/SplitsCacheInMemory.ts index 8cb45aef..cf570eea 100644 --- a/src/storages/inMemory/SplitsCacheInMemory.ts +++ b/src/storages/inMemory/SplitsCacheInMemory.ts @@ -1,7 +1,7 @@ import { ISplit, ISplitFiltersValidation } from '../../dtos/types'; import { AbstractSplitsCacheSync, usesSegments } from '../AbstractSplitsCacheSync'; import { isFiniteNumber } from '../../utils/lang'; -import { ISet, _Set, returnSetsUnion } from '../../utils/lang/sets'; +import { ISet, _Set } from '../../utils/lang/sets'; /** * Default ISplitsCacheSync implementation that stores split definitions in memory. @@ -16,9 +16,9 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync { private splitsWithSegmentsCount: number = 0; private flagSetsCache: Record> = {}; - constructor(splitFiltersValidation: ISplitFiltersValidation = { queryString: null, groupedFilters: { bySet: [], byName: [], byPrefix: [] }, validFilters: [] }) { + constructor(splitFiltersValidation?: ISplitFiltersValidation) { super(); - this.flagSetsFilter = splitFiltersValidation.groupedFilters.bySet; + this.flagSetsFilter = splitFiltersValidation ? splitFiltersValidation.groupedFilters.bySet : []; } clear() { @@ -105,16 +105,8 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync { return this.getChangeNumber() === -1 || this.splitsWithSegmentsCount > 0; } - getNamesByFlagSets(flagSets: string[]): ISet{ - let toReturn: ISet = new _Set([]); - flagSets.forEach(flagSet => { - const featureFlagNames = this.flagSetsCache[flagSet]; - if (featureFlagNames) { - toReturn = returnSetsUnion(toReturn, featureFlagNames); - } - }); - return toReturn; - + getNamesByFlagSets(flagSets: string[]): ISet[] { + return flagSets.map(flagSet => this.flagSetsCache[flagSet] || new _Set()); } private addToFlagSets(featureFlag: ISplit) { @@ -129,7 +121,7 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync { }); } - private removeFromFlagSets(featureFlagName :string, flagSets: string[] | undefined) { + private removeFromFlagSets(featureFlagName: string, flagSets: string[] | undefined) { if (!flagSets) return; flagSets.forEach(flagSet => { this.removeNames(flagSet, featureFlagName); diff --git a/src/storages/inMemory/__tests__/SplitsCacheInMemory.spec.ts b/src/storages/inMemory/__tests__/SplitsCacheInMemory.spec.ts index 865705cf..14fa62fd 100644 --- a/src/storages/inMemory/__tests__/SplitsCacheInMemory.spec.ts +++ b/src/storages/inMemory/__tests__/SplitsCacheInMemory.spec.ts @@ -127,32 +127,32 @@ test('SPLITS CACHE / In Memory / flag set cache tests', () => { ]); cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS); - expect(cache.getNamesByFlagSets(['o'])).toEqual(new _Set(['ff_one', 'ff_two'])); - expect(cache.getNamesByFlagSets(['n'])).toEqual(new _Set(['ff_one'])); - expect(cache.getNamesByFlagSets(['e'])).toEqual(new _Set(['ff_one','ff_three'])); - expect(cache.getNamesByFlagSets(['t'])).toEqual(emptySet); // 't' not in filter - expect(cache.getNamesByFlagSets(['o','n','e'])).toEqual(new _Set(['ff_one','ff_two','ff_three'])); + expect(cache.getNamesByFlagSets(['o'])).toEqual([new _Set(['ff_one', 'ff_two'])]); + expect(cache.getNamesByFlagSets(['n'])).toEqual([new _Set(['ff_one'])]); + expect(cache.getNamesByFlagSets(['e'])).toEqual([new _Set(['ff_one', 'ff_three'])]); + expect(cache.getNamesByFlagSets(['t'])).toEqual([emptySet]); // 't' not in filter + expect(cache.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new _Set(['ff_one', 'ff_two']), new _Set(['ff_one']), new _Set(['ff_one', 'ff_three'])]); - cache.addSplit(featureFlagOne.name, {...featureFlagOne, sets: ['1']}); + cache.addSplit(featureFlagOne.name, { ...featureFlagOne, sets: ['1'] }); - expect(cache.getNamesByFlagSets(['1'])).toEqual(emptySet); // '1' not in filter - expect(cache.getNamesByFlagSets(['o'])).toEqual(new _Set(['ff_two'])); - expect(cache.getNamesByFlagSets(['n'])).toEqual(emptySet); + expect(cache.getNamesByFlagSets(['1'])).toEqual([emptySet]); // '1' not in filter + expect(cache.getNamesByFlagSets(['o'])).toEqual([new _Set(['ff_two'])]); + expect(cache.getNamesByFlagSets(['n'])).toEqual([emptySet]); - cache.addSplit(featureFlagOne.name, {...featureFlagOne, sets: ['x']}); - expect(cache.getNamesByFlagSets(['x'])).toEqual(new _Set(['ff_one'])); - expect(cache.getNamesByFlagSets(['o','e','x'])).toEqual(new _Set(['ff_one','ff_two','ff_three'])); + cache.addSplit(featureFlagOne.name, { ...featureFlagOne, sets: ['x'] }); + expect(cache.getNamesByFlagSets(['x'])).toEqual([new _Set(['ff_one'])]); + expect(cache.getNamesByFlagSets(['o', 'e', 'x'])).toEqual([new _Set(['ff_two']), new _Set(['ff_three']), new _Set(['ff_one'])]); cache.removeSplit(featureFlagOne.name); - expect(cache.getNamesByFlagSets(['x'])).toEqual(emptySet); + expect(cache.getNamesByFlagSets(['x'])).toEqual([emptySet]); cache.removeSplit(featureFlagOne.name); - expect(cache.getNamesByFlagSets(['y'])).toEqual(emptySet); // 'y' not in filter - expect(cache.getNamesByFlagSets([])).toEqual(emptySet); + expect(cache.getNamesByFlagSets(['y'])).toEqual([emptySet]); // 'y' not in filter + expect(cache.getNamesByFlagSets([])).toEqual([]); cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithoutFS); - expect(cache.getNamesByFlagSets([])).toEqual(emptySet); + expect(cache.getNamesByFlagSets([])).toEqual([]); }); // if FlagSets are not defined, it should store all FlagSets in memory. @@ -167,10 +167,10 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests without filters', () => ]); cacheWithoutFilters.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS); - expect(cacheWithoutFilters.getNamesByFlagSets(['o'])).toEqual(new _Set(['ff_one', 'ff_two'])); - expect(cacheWithoutFilters.getNamesByFlagSets(['n'])).toEqual(new _Set(['ff_one'])); - expect(cacheWithoutFilters.getNamesByFlagSets(['e'])).toEqual(new _Set(['ff_one','ff_three'])); - expect(cacheWithoutFilters.getNamesByFlagSets(['t'])).toEqual(new _Set(['ff_two','ff_three'])); - expect(cacheWithoutFilters.getNamesByFlagSets(['y'])).toEqual(emptySet); - expect(cacheWithoutFilters.getNamesByFlagSets(['o','n','e'])).toEqual(new _Set(['ff_one','ff_two','ff_three'])); + expect(cacheWithoutFilters.getNamesByFlagSets(['o'])).toEqual([new _Set(['ff_one', 'ff_two'])]); + expect(cacheWithoutFilters.getNamesByFlagSets(['n'])).toEqual([new _Set(['ff_one'])]); + expect(cacheWithoutFilters.getNamesByFlagSets(['e'])).toEqual([new _Set(['ff_one', 'ff_three'])]); + expect(cacheWithoutFilters.getNamesByFlagSets(['t'])).toEqual([new _Set(['ff_two', 'ff_three'])]); + expect(cacheWithoutFilters.getNamesByFlagSets(['y'])).toEqual([emptySet]); + expect(cacheWithoutFilters.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new _Set(['ff_one', 'ff_two']), new _Set(['ff_one']), new _Set(['ff_one', 'ff_three'])]); }); 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 5a616c11..dcdadb4a 100644 --- a/src/storages/inRedis/RedisAdapter.ts +++ b/src/storages/inRedis/RedisAdapter.ts @@ -8,7 +8,7 @@ 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', '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', 'flushdb']; 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. @@ -39,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)); @@ -96,7 +96,7 @@ export class RedisAdapter extends ioredis { const caller: (Pipeline | RedisAdapter) = this; // Need to change this crap to have TypeScript stop complaining const commandWrapper = () => { - instance.log.debug(`${LOG_PREFIX} Executing ${methodName}.`); + instance.log.debug(`${LOG_PREFIX}Executing ${methodName}.`); const result = originalMethod.apply(caller, params); if (thenable(result)) { @@ -152,7 +152,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.`); @@ -205,7 +205,7 @@ export class RedisAdapter extends ioredis { /** * Parses the options into what we care about. */ - static _defineOptions({ connectionTimeout, operationTimeout, url, host, port, db, pass, tls }: Record) { + static _defineOptions({ connectionTimeout, operationTimeout, url, host, port, db, pass, tls }: Record = {}) { const parsedOptions = { connectionTimeout, operationTimeout, url, host, port, db, pass, tls }; diff --git a/src/storages/inRedis/SegmentsCacheInRedis.ts b/src/storages/inRedis/SegmentsCacheInRedis.ts index a144e2b7..3a18b535 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; diff --git a/src/storages/inRedis/SplitsCacheInRedis.ts b/src/storages/inRedis/SplitsCacheInRedis.ts index 06f61074..70abd74b 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 } from '../../dtos/types'; +import { ISplit, ISplitFiltersValidation } from '../../dtos/types'; import { AbstractSplitsCacheAsync } from '../AbstractSplitsCacheAsync'; -import { _Set, ISet } from '../../utils/lang/sets'; +import { ISet, _Set, returnListDifference } from '../../utils/lang/sets'; +import type { RedisAdapter } from './RedisAdapter'; /** * Discard errors for an answer of multiple operations. @@ -24,15 +24,17 @@ 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) { + constructor(log: ILogger, keys: KeyBuilderSS, redis: RedisAdapter, splitFiltersValidation?: ISplitFiltersValidation) { super(); this.log = log; this.redis = redis; this.keys = keys; + this.flagSetsFilter = splitFiltersValidation ? splitFiltersValidation.groupedFilters.bySet : []; // There is no need to listen for redis 'error' event, because in that case ioredis calls will be rejected and handled by redis storage adapters. // But it is done just to avoid getting the ioredis message `Unhandled error event`. @@ -57,6 +59,24 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync { return this.redis.incr(ttKey); } + private _updateFlagSets(featureFlagName: string, flagSetsOfRemovedFlag?: string[], flagSetsOfAddedFlag?: string[]) { + const removeFromFlagSets = returnListDifference(flagSetsOfRemovedFlag, flagSetsOfAddedFlag); + + let addToFlagSets = returnListDifference(flagSetsOfAddedFlag, flagSetsOfRemovedFlag); + if (this.flagSetsFilter.length > 0) { + addToFlagSets = addToFlagSets.filter(flagSet => { + return this.flagSetsFilter.some(filterFlagSet => filterFlagSet === flagSet); + }); + } + + const items = [featureFlagName]; + + return Promise.all([ + ...removeFromFlagSets.map(flagSetName => this.redis.srem(this.keys.buildFlagSetKey(flagSetName), items)), + ...addToFlagSets.map(flagSetName => this.redis.sadd(this.keys.buildFlagSetKey(flagSetName), items)) + ]); + } + /** * Add a given split. * The returned promise is resolved when the operation success @@ -66,16 +86,16 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync { const splitKey = this.keys.buildSplitKey(name); return this.redis.get(splitKey).then(splitFromStorage => { - // handling parsing errors - let parsedPreviousSplit: ISplit, newStringifiedSplit; + // handling parsing error + let parsedPreviousSplit: ISplit, stringifiedNewSplit; try { parsedPreviousSplit = splitFromStorage ? JSON.parse(splitFromStorage) : undefined; - newStringifiedSplit = JSON.stringify(split); + stringifiedNewSplit = JSON.stringify(split); } catch (e) { throw new Error('Error parsing feature flag definition: ' + e); } - return this.redis.set(splitKey, newStringifiedSplit).then(() => { + return this.redis.set(splitKey, stringifiedNewSplit).then(() => { // avoid unnecessary increment/decrement operations if (parsedPreviousSplit && parsedPreviousSplit.trafficTypeName === split.trafficTypeName) return; @@ -83,7 +103,7 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync { return this._incrementCounts(split).then(() => { if (parsedPreviousSplit) return this._decrementCounts(parsedPreviousSplit); }); - }); + }).then(() => this._updateFlagSets(name, parsedPreviousSplit && parsedPreviousSplit.sets, split.sets)); }).then(() => true); } @@ -101,11 +121,12 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync { * The returned promise is resolved when the operation success, with 1 or 0 indicating if the split existed or not. * or rejected if it fails (e.g., redis operation fails). */ - removeSplit(name: string): Promise { + removeSplit(name: string) { return this.getSplit(name).then((split) => { if (split) { - this._decrementCounts(split); + return this._decrementCounts(split).then(() => this._updateFlagSets(name, split.sets)); } + }).then(() => { return this.redis.del(this.keys.buildSplitKey(name)); }); } @@ -174,7 +195,7 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync { .then((listOfKeys) => this.redis.pipeline(listOfKeys.map(k => ['get', k])).exec()) .then(processPipelineAnswer) .then((splitDefinitions) => splitDefinitions.map((splitDefinition) => { - return JSON.parse(splitDefinition as string); + return JSON.parse(splitDefinition); })); } @@ -190,34 +211,18 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync { } /** - * Get list of split names related to a given flag set names list. - * The returned promise is resolved with the list of split names, - * or rejected if wrapper operation fails. - * @todo this is a no-op method to be implemented + * 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. */ - getNamesByFlagSets(flagSets: string[]): Promise> { - const toReturn: ISet = new _Set([]); - const listOfKeys: string[] = []; - - flagSets && flagSets.forEach(flagSet => { - listOfKeys.push(this.keys.buildFlagSetKey(flagSet)); - }); - - if (listOfKeys.length > 0) { - - return this.redis.pipeline(listOfKeys.map(k => ['smembers', k])).exec() - .then(processPipelineAnswer) - .then((setsRaw) => { - this.log.error(setsRaw); - setsRaw.forEach((setContent) => { - (setContent as string[]).forEach(flagName => toReturn.add(flagName)); - }); - - return toReturn; - }); - } else { - return new Promise(() => toReturn); - } + getNamesByFlagSets(flagSets: string[]): Promise[]> { + return this.redis.pipeline(flagSets.map(flagSet => ['smembers', this.keys.buildFlagSetKey(flagSet)])).exec() + .then((results) => results.map(([e, value], index) => { + if (e === null) return value; + + this.log.error(LOG_PREFIX + `Could not read result from get members of flag set ${flagSets[index]} due to an error: ${e}`); + })) + .then(namesByFlagSets => namesByFlagSets.map(namesByFlagSet => new _Set(namesByFlagSet))); } /** @@ -234,14 +239,14 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync { ttCount = parseInt(ttCount as string, 10); if (!isFiniteNumber(ttCount) || ttCount < 0) { - this.log.info(LOG_PREFIX + `Could not validate traffic type existance of ${trafficType} due to data corruption of some sorts.`); + this.log.info(LOG_PREFIX + `Could not validate traffic type existence of ${trafficType} due to data corruption of some sorts.`); return false; } return ttCount > 0; }) .catch(e => { - this.log.error(LOG_PREFIX + `Could not validate traffic type existance of ${trafficType} due to an error: ${e}.`); + this.log.error(LOG_PREFIX + `Could not validate traffic type existence of ${trafficType} due to an error: ${e}.`); // If there is an error, bypass the validation so the event can get tracked. return true; }); 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 708e3fca..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); @@ -54,5 +54,5 @@ test('EVENTS CACHE IN REDIS / `track`, `count`, `popNWithMetadata` and `drop` me // Clean up then end. await connection.del(eventsKey, nonListKey); - await connection.quit(); + await connection.disconnect(); }); diff --git a/src/storages/inRedis/__tests__/ImpressionCountsCacheInRedis.spec.ts b/src/storages/inRedis/__tests__/ImpressionCountsCacheInRedis.spec.ts index e28011c6..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(); @@ -25,11 +26,11 @@ describe('IMPRESSION COUNTS CACHE IN REDIS', () => { expect(counter._makeKey(null, new Date(2020, 9, 2, 10, 53, 12).getTime())).toBe(`null::${timestamp1}`); expect(counter._makeKey(null, 0)).toBe('null::0'); - await connection.quit(); + await connection.disconnect(); }); 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); @@ -76,11 +77,14 @@ describe('IMPRESSION COUNTS CACHE IN REDIS', () => { expect(Object.keys(counter.pop()).length).toBe(0); await connection.del(key); - await connection.quit(); + await connection.disconnect(); }); - test('POST IMPRESSION COUNTS IN REDIS FUNCTION', (done) => { - const connection = new Redis(); + test('POST IMPRESSION COUNTS IN REDIS FUNCTION', async () => { + 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); @@ -95,15 +99,12 @@ describe('IMPRESSION COUNTS CACHE IN REDIS', () => { counter.track('feature2', nextHourTimestamp + 3, 2); counter.track('feature2', nextHourTimestamp + 4, 2); - counter.postImpressionCountsInRedis().then(() => { + await counter.postImpressionCountsInRedis(); - connection.hgetall(key).then(async data => { - expect(data).toStrictEqual(expected); - await connection.del(key); - await connection.quit(); - done(); - }); - }); + const data = await connection.hgetall(key); + expect(data).toStrictEqual(expected); + await connection.del(key); + await connection.disconnect(); }); test('start and stop task', (done) => { @@ -146,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); @@ -183,6 +184,6 @@ describe('IMPRESSION COUNTS CACHE IN REDIS', () => { expect(await connection.hgetall(key)).toStrictEqual({}); await connection.del(key); - await connection.quit(); + await connection.disconnect(); }); }); diff --git a/src/storages/inRedis/__tests__/ImpressionsCacheInRedis.spec.ts b/src/storages/inRedis/__tests__/ImpressionsCacheInRedis.spec.ts index 0657d9ab..c4b17886 100644 --- a/src/storages/inRedis/__tests__/ImpressionsCacheInRedis.spec.ts +++ b/src/storages/inRedis/__tests__/ImpressionsCacheInRedis.spec.ts @@ -44,7 +44,7 @@ describe('IMPRESSIONS CACHE IN REDIS', () => { expect(await c.count()).toBe(0); // storage should be empty after dropping it await connection.del(impressionsKey); - await connection.quit(); + await connection.disconnect(); }); test('`track` should not resolve before calling expire', async () => { @@ -76,7 +76,7 @@ describe('IMPRESSIONS CACHE IN REDIS', () => { // @ts-expect-error await c.track([i1, i2]).then(() => { connection.del(impressionsKey); - connection.quit(); // Try to disconnect right away. + connection.disconnect(); // Try to disconnect right away. expect(spy1).toBeCalled(); // Redis rpush was called once before executing external callback. // Following assertion fails if the expire takes place after disconnected and throws unhandledPromiseRejection expect(spy2).toBeCalled(); // Redis expire was called once before executing external callback. diff --git a/src/storages/inRedis/__tests__/RedisAdapter.spec.ts b/src/storages/inRedis/__tests__/RedisAdapter.spec.ts index ecdf855a..90b8743f 100644 --- a/src/storages/inRedis/__tests__/RedisAdapter.spec.ts +++ b/src/storages/inRedis/__tests__/RedisAdapter.spec.ts @@ -11,7 +11,7 @@ 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', 'flushdb']; const ioredisMock = reduce([...METHODS_TO_PROMISE_WRAP, 'disconnect'], (acc, methodName) => { acc[methodName] = jest.fn(() => Promise.resolve(methodName)); diff --git a/src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts b/src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts index be4b4f3b..009d6b10 100644 --- a/src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts +++ b/src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts @@ -1,15 +1,15 @@ -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); 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(); @@ -37,11 +37,11 @@ describe('SEGMENTS CACHE IN REDIS', () => { expect(await cache.isInSegment('mocked-segment', 'e')).toBe(true); await cache.clear(); - await connection.quit(); + 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(); @@ -55,7 +55,7 @@ describe('SEGMENTS CACHE IN REDIS', () => { ['s1', 's2', 's3', 's4'].forEach(s => expect(segments.indexOf(s) !== -1).toBe(true)); await cache.clear(); - await connection.quit(); + await connection.disconnect(); }); }); diff --git a/src/storages/inRedis/__tests__/SplitsCacheInRedis.spec.ts b/src/storages/inRedis/__tests__/SplitsCacheInRedis.spec.ts index 22d12190..c7ad85c7 100644 --- a/src/storages/inRedis/__tests__/SplitsCacheInRedis.spec.ts +++ b/src/storages/inRedis/__tests__/SplitsCacheInRedis.spec.ts @@ -1,10 +1,11 @@ -import Redis from 'ioredis'; import { SplitsCacheInRedis } from '../SplitsCacheInRedis'; import { KeyBuilderSS } from '../../KeyBuilderSS'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; -import { splitWithUserTT, splitWithAccountTT } from '../../__tests__/testUtils'; +import { splitWithUserTT, splitWithAccountTT, featureFlagOne, featureFlagThree, featureFlagTwo, featureFlagWithEmptyFS, featureFlagWithoutFS } from '../../__tests__/testUtils'; 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); @@ -12,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([ @@ -54,11 +55,11 @@ describe('SPLITS CACHE REDIS', () => { await connection.del(keysBuilder.buildTrafficTypeKey('account_tt')); await connection.del(keysBuilder.buildSplitKey('lol2')); await connection.del(keysBuilder.buildSplitsTillKey()); - await connection.quit(); + await connection.disconnect(); }); test('trafficTypeExists', async () => { - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); const cache = new SplitsCacheInRedis(loggerMock, keysBuilder, connection); await cache.addSplits([ @@ -102,11 +103,11 @@ describe('SPLITS CACHE REDIS', () => { await connection.del(keysBuilder.buildTrafficTypeKey('account_tt')); await connection.del(keysBuilder.buildSplitKey('malformed')); await connection.del(keysBuilder.buildSplitKey('split1')); - await connection.quit(); + await connection.disconnect(); }); test('killLocally', async () => { - const connection = new Redis(); + const connection = new RedisAdapter(loggerMock); const cache = new SplitsCacheInRedis(loggerMock, keysBuilder, connection); await cache.addSplit('lol1', splitWithUserTT); @@ -140,7 +141,86 @@ describe('SPLITS CACHE REDIS', () => { // Delete splits and TT keys await cache.removeSplits(['lol1', 'lol2']); expect(await connection.keys(`${prefix}*`)).toHaveLength(0); - await connection.quit(); + await connection.disconnect(); + }); + + test('flag set cache tests', async () => { + const connection = new RedisAdapter(loggerMock); // @ts-ignore + const cache = new SplitsCacheInRedis(loggerMock, keysBuilder, connection, { groupedFilters: { bySet: ['o', 'n', 'e', 'x'] } }); + + const emptySet = new _Set([]); + + await cache.addSplits([ + [featureFlagOne.name, featureFlagOne], + [featureFlagTwo.name, featureFlagTwo], + [featureFlagThree.name, featureFlagThree], + ]); + await cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS); + + expect(await cache.getNamesByFlagSets(['o'])).toEqual([new _Set(['ff_one', 'ff_two'])]); + expect(await cache.getNamesByFlagSets(['n'])).toEqual([new _Set(['ff_one'])]); + expect(await cache.getNamesByFlagSets(['e'])).toEqual([new _Set(['ff_one', 'ff_three'])]); + expect(await cache.getNamesByFlagSets(['t'])).toEqual([emptySet]); // 't' not in filter + expect(await cache.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new _Set(['ff_one', 'ff_two']), new _Set(['ff_one']), new _Set(['ff_one', 'ff_three'])]); + + await cache.addSplit(featureFlagOne.name, { ...featureFlagOne, sets: ['1'] }); + + expect(await cache.getNamesByFlagSets(['1'])).toEqual([emptySet]); // '1' not in filter + expect(await cache.getNamesByFlagSets(['o'])).toEqual([new _Set(['ff_two'])]); + expect(await cache.getNamesByFlagSets(['n'])).toEqual([emptySet]); + + await cache.addSplit(featureFlagOne.name, { ...featureFlagOne, sets: ['x'] }); + 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 an error in connection.pipeline().exec() + jest.spyOn(connection, 'pipeline').mockImplementationOnce(() => { + 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(); + + await cache.removeSplit(featureFlagOne.name); + expect(await cache.getNamesByFlagSets(['x'])).toEqual([emptySet]); + + await cache.removeSplit(featureFlagOne.name); + expect(await cache.getNamesByFlagSets(['y'])).toEqual([emptySet]); // 'y' not in filter + expect(await cache.getNamesByFlagSets([])).toEqual([]); + + await cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithoutFS); + expect(await cache.getNamesByFlagSets([])).toEqual([]); + + // Delete splits, TT and flag set keys + await cache.removeSplits([featureFlagThree.name, featureFlagTwo.name, featureFlagWithEmptyFS.name]); + expect(await connection.keys(`${prefix}*`)).toHaveLength(0); + await connection.disconnect(); + }); + + // if FlagSets filter is not defined, it should store all FlagSets in memory. + test('flag set cache tests without filters', async () => { + const connection = new RedisAdapter(loggerMock); + const cacheWithoutFilters = new SplitsCacheInRedis(loggerMock, keysBuilder, connection); + + const emptySet = new _Set([]); + + await cacheWithoutFilters.addSplits([ + [featureFlagOne.name, featureFlagOne], + [featureFlagTwo.name, featureFlagTwo], + [featureFlagThree.name, featureFlagThree], + ]); + await cacheWithoutFilters.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS); + + expect(await cacheWithoutFilters.getNamesByFlagSets(['o'])).toEqual([new _Set(['ff_one', 'ff_two'])]); + expect(await cacheWithoutFilters.getNamesByFlagSets(['n'])).toEqual([new _Set(['ff_one'])]); + expect(await cacheWithoutFilters.getNamesByFlagSets(['e'])).toEqual([new _Set(['ff_one', 'ff_three'])]); + expect(await cacheWithoutFilters.getNamesByFlagSets(['t'])).toEqual([new _Set(['ff_two', 'ff_three'])]); + expect(await cacheWithoutFilters.getNamesByFlagSets(['y'])).toEqual([emptySet]); + expect(await cacheWithoutFilters.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new _Set(['ff_one', 'ff_two']), new _Set(['ff_one']), new _Set(['ff_one', 'ff_three'])]); + + // Delete splits, TT and flag set keys + await cacheWithoutFilters.removeSplits([featureFlagThree.name, featureFlagTwo.name, featureFlagOne.name, featureFlagWithEmptyFS.name]); + expect(await connection.keys(`${prefix}*`)).toHaveLength(0); + await connection.disconnect(); }); }); diff --git a/src/storages/inRedis/__tests__/TelemetryCacheInRedis.spec.ts b/src/storages/inRedis/__tests__/TelemetryCacheInRedis.spec.ts index 5eea4329..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 @@ -86,5 +86,5 @@ test('TELEMETRY CACHE IN REDIS', async () => { expect((await cache.popExceptions()).size).toBe(0); expect((await cache.popConfigs()).size).toBe(0); - await connection.quit(); + await connection.disconnect(); }); diff --git a/src/storages/inRedis/__tests__/UniqueKeysCacheInRedis.spec.ts b/src/storages/inRedis/__tests__/UniqueKeysCacheInRedis.spec.ts index 3fcbb3a5..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); @@ -46,12 +46,12 @@ describe('UNIQUE KEYS CACHE IN REDIS', () => { expect(cache.pop()).toEqual({ keys: [] }); expect(cache.isEmpty()).toBe(true); - await connection.quit(); + await connection.disconnect(); }); 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(); }); @@ -75,11 +75,11 @@ describe('UNIQUE KEYS CACHE IN REDIS', () => { expect(cbCalled).toBe(2); // Until the queue is filled with events again. await connection.del(key); - await connection.quit(); + await connection.disconnect(); }); 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'); @@ -99,7 +99,7 @@ describe('UNIQUE KEYS CACHE IN REDIS', () => { expect(data).toStrictEqual(expected); await connection.del(key); - await connection.quit(); + await connection.disconnect(); }); }); @@ -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); @@ -184,6 +186,6 @@ describe('UNIQUE KEYS CACHE IN REDIS', () => { expect(data).toStrictEqual([]); await connection.del(key); - await connection.quit(); + await connection.disconnect(); }); }); diff --git a/src/storages/pluggable/SplitsCachePluggable.ts b/src/storages/pluggable/SplitsCachePluggable.ts index 786fb8a5..9ed55b9f 100644 --- a/src/storages/pluggable/SplitsCachePluggable.ts +++ b/src/storages/pluggable/SplitsCachePluggable.ts @@ -2,10 +2,10 @@ import { isFiniteNumber, isNaNNumber } from '../../utils/lang'; import { KeyBuilder } from '../KeyBuilder'; import { IPluggableStorageWrapper } from '../types'; import { ILogger } from '../../logger/types'; -import { ISplit } from '../../dtos/types'; +import { ISplit, ISplitFiltersValidation } from '../../dtos/types'; import { LOG_PREFIX } from './constants'; import { AbstractSplitsCacheAsync } from '../AbstractSplitsCacheAsync'; -import { ISet } from '../../utils/lang/sets'; +import { ISet, _Set, returnListDifference } from '../../utils/lang/sets'; /** * ISplitsCacheAsync implementation for pluggable storages. @@ -15,6 +15,7 @@ export class SplitsCachePluggable extends AbstractSplitsCacheAsync { private readonly log: ILogger; private readonly keys: KeyBuilder; private readonly wrapper: IPluggableStorageWrapper; + private readonly flagSetsFilter: string[]; /** * Create a SplitsCache that uses a storage wrapper. @@ -22,11 +23,12 @@ export class SplitsCachePluggable extends AbstractSplitsCacheAsync { * @param keys Key builder. * @param wrapper Adapted wrapper storage. */ - constructor(log: ILogger, keys: KeyBuilder, wrapper: IPluggableStorageWrapper) { + constructor(log: ILogger, keys: KeyBuilder, wrapper: IPluggableStorageWrapper, splitFiltersValidation?: ISplitFiltersValidation) { super(); this.log = log; this.keys = keys; this.wrapper = wrapper; + this.flagSetsFilter = splitFiltersValidation ? splitFiltersValidation.groupedFilters.bySet : []; } private _decrementCounts(split: ISplit) { @@ -41,6 +43,24 @@ export class SplitsCachePluggable extends AbstractSplitsCacheAsync { return this.wrapper.incr(ttKey); } + private _updateFlagSets(featureFlagName: string, flagSetsOfRemovedFlag?: string[], flagSetsOfAddedFlag?: string[]) { + const removeFromFlagSets = returnListDifference(flagSetsOfRemovedFlag, flagSetsOfAddedFlag); + + let addToFlagSets = returnListDifference(flagSetsOfAddedFlag, flagSetsOfRemovedFlag); + if (this.flagSetsFilter.length > 0) { + addToFlagSets = addToFlagSets.filter(flagSet => { + return this.flagSetsFilter.some(filterFlagSet => filterFlagSet === flagSet); + }); + } + + const items = [featureFlagName]; + + return Promise.all([ + ...removeFromFlagSets.map(flagSetName => this.wrapper.removeItems(this.keys.buildFlagSetKey(flagSetName), items)), + ...addToFlagSets.map(flagSetName => this.wrapper.addItems(this.keys.buildFlagSetKey(flagSetName), items)) + ]); + } + /** * Add a given split. * The returned promise is resolved when the operation success @@ -67,7 +87,7 @@ export class SplitsCachePluggable extends AbstractSplitsCacheAsync { return this._incrementCounts(split).then(() => { if (parsedPreviousSplit) return this._decrementCounts(parsedPreviousSplit); }); - }); + }).then(() => this._updateFlagSets(name, parsedPreviousSplit && parsedPreviousSplit.sets, split.sets)); }).then(() => true); } @@ -88,8 +108,9 @@ export class SplitsCachePluggable extends AbstractSplitsCacheAsync { removeSplit(name: string) { return this.getSplit(name).then((split) => { if (split) { - this._decrementCounts(split); + return this._decrementCounts(split).then(() => this._updateFlagSets(name, split.sets)); } + }).then(() => { return this.wrapper.del(this.keys.buildSplitKey(name)); }); } @@ -156,14 +177,15 @@ export class SplitsCachePluggable extends AbstractSplitsCacheAsync { } /** - * Get list of split names related to a given flag set names list. - * The returned promise is resolved with the list of split names, - * or rejected if wrapper operation fails. - * @todo this is a no-op method to be implemented - */ - getNamesByFlagSets(): Promise> { - this.log.error(LOG_PREFIX + 'ByFlagSet/s evaluations are not supported with pluggable storage yet.'); - return Promise.reject(); + * 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. + * It never rejects (If there is a wrapper error for some flag set, an empty set is returned for it). + */ + getNamesByFlagSets(flagSets: string[]): Promise[]> { + return Promise.all(flagSets.map(flagSet => { + const flagSetKey = this.keys.buildFlagSetKey(flagSet); + return this.wrapper.getItems(flagSetKey).catch(() => []); + })).then(namesByFlagSets => namesByFlagSets.map(namesByFlagSet => new _Set(namesByFlagSet))); } /** diff --git a/src/storages/pluggable/__tests__/SplitsCachePluggable.spec.ts b/src/storages/pluggable/__tests__/SplitsCachePluggable.spec.ts index 6cad1e2a..ea8aa73e 100644 --- a/src/storages/pluggable/__tests__/SplitsCachePluggable.spec.ts +++ b/src/storages/pluggable/__tests__/SplitsCachePluggable.spec.ts @@ -2,8 +2,9 @@ import { SplitsCachePluggable } from '../SplitsCachePluggable'; import { KeyBuilder } from '../../KeyBuilder'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; import { wrapperMockFactory } from './wrapper.mock'; -import { splitWithUserTT, splitWithAccountTT } from '../../__tests__/testUtils'; +import { splitWithUserTT, splitWithAccountTT, featureFlagOne, featureFlagThree, featureFlagTwo, featureFlagWithEmptyFS, featureFlagWithoutFS } from '../../__tests__/testUtils'; import { ISplit } from '../../../dtos/types'; +import { _Set } from '../../../utils/lang/sets'; const keysBuilder = new KeyBuilder(); @@ -150,4 +151,67 @@ describe('SPLITS CACHE PLUGGABLE', () => { expect(await wrapper.getKeysByPrefix('SPLITIO')).toHaveLength(0); }); + test('flag set cache tests', async () => { + const wrapper = wrapperMockFactory(); // @ts-ignore + const cache = new SplitsCachePluggable(loggerMock, keysBuilder, wrapper, { groupedFilters: { bySet: ['o', 'n', 'e', 'x'] } }); + const emptySet = new _Set([]); + + await cache.addSplits([ + [featureFlagOne.name, featureFlagOne], + [featureFlagTwo.name, featureFlagTwo], + [featureFlagThree.name, featureFlagThree], + ]); + await cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS); + + expect(await cache.getNamesByFlagSets(['o'])).toEqual([new _Set(['ff_one', 'ff_two'])]); + expect(await cache.getNamesByFlagSets(['n'])).toEqual([new _Set(['ff_one'])]); + expect(await cache.getNamesByFlagSets(['e'])).toEqual([new _Set(['ff_one', 'ff_three'])]); + expect(await cache.getNamesByFlagSets(['t'])).toEqual([emptySet]); // 't' not in filter + expect(await cache.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new _Set(['ff_one', 'ff_two']), new _Set(['ff_one']), new _Set(['ff_one', 'ff_three'])]); + + await cache.addSplit(featureFlagOne.name, { ...featureFlagOne, sets: ['1'] }); + + expect(await cache.getNamesByFlagSets(['1'])).toEqual([emptySet]); // '1' not in filter + expect(await cache.getNamesByFlagSets(['o'])).toEqual([new _Set(['ff_two'])]); + expect(await cache.getNamesByFlagSets(['n'])).toEqual([emptySet]); + + await cache.addSplit(featureFlagOne.name, { ...featureFlagOne, sets: ['x'] }); + 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'])]); + + // Simulate one error in getItems + wrapper.getItems.mockImplementationOnce(() => Promise.reject('error')); + expect(await cache.getNamesByFlagSets(['o', 'e', 'x'])).toEqual([emptySet, new _Set(['ff_three']), new _Set(['ff_one'])]); + + await cache.removeSplit(featureFlagOne.name); + expect(await cache.getNamesByFlagSets(['x'])).toEqual([emptySet]); + + await cache.removeSplit(featureFlagOne.name); + expect(await cache.getNamesByFlagSets(['y'])).toEqual([emptySet]); // 'y' not in filter + expect(await cache.getNamesByFlagSets([])).toEqual([]); + + await cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithoutFS); + expect(await cache.getNamesByFlagSets([])).toEqual([]); + }); + + // if FlagSets filter is not defined, it should store all FlagSets in memory. + test('flag set cache tests without filters', async () => { + const cacheWithoutFilters = new SplitsCachePluggable(loggerMock, keysBuilder, wrapperMockFactory()); + const emptySet = new _Set([]); + + await cacheWithoutFilters.addSplits([ + [featureFlagOne.name, featureFlagOne], + [featureFlagTwo.name, featureFlagTwo], + [featureFlagThree.name, featureFlagThree], + ]); + await cacheWithoutFilters.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS); + + expect(await cacheWithoutFilters.getNamesByFlagSets(['o'])).toEqual([new _Set(['ff_one', 'ff_two'])]); + expect(await cacheWithoutFilters.getNamesByFlagSets(['n'])).toEqual([new _Set(['ff_one'])]); + expect(await cacheWithoutFilters.getNamesByFlagSets(['e'])).toEqual([new _Set(['ff_one', 'ff_three'])]); + expect(await cacheWithoutFilters.getNamesByFlagSets(['t'])).toEqual([new _Set(['ff_two', 'ff_three'])]); + expect(await cacheWithoutFilters.getNamesByFlagSets(['y'])).toEqual([emptySet]); + expect(await cacheWithoutFilters.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new _Set(['ff_one', 'ff_two']), new _Set(['ff_one']), new _Set(['ff_one', 'ff_three'])]); + }); + }); diff --git a/src/storages/pluggable/index.ts b/src/storages/pluggable/index.ts index c1516f28..3e72d791 100644 --- a/src/storages/pluggable/index.ts +++ b/src/storages/pluggable/index.ts @@ -105,7 +105,7 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn }); return { - splits: new SplitsCachePluggable(log, keys, wrapper), + splits: new SplitsCachePluggable(log, keys, wrapper, settings.sync.__splitFiltersValidation), segments: new SegmentsCachePluggable(log, keys, wrapper), impressions: isPartialConsumer ? new ImpressionsCacheInMemory(impressionsQueueSize) : new ImpressionsCachePluggable(log, keys.buildImpressionsKey(), wrapper, metadata), impressionCounts: impressionCountsCache, diff --git a/src/storages/types.ts b/src/storages/types.ts index 30832f5e..9e8dc563 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -44,10 +44,10 @@ export interface IPluggableStorageWrapper { * * @function del * @param {string} key Item to delete - * @returns {Promise} A promise that resolves if the operation success, whether the key existed and was removed or it didn't exist. + * @returns {Promise} A promise that resolves if the operation success, whether the key existed and was removed (resolves with true) or it didn't exist (resolves with false). * The promise rejects if the operation fails, for example, if there is a connection error. */ - del: (key: string) => Promise + del: (key: string) => Promise /** * Returns all keys matching the given prefix. * @@ -210,7 +210,7 @@ export interface ISplitsCacheBase { // should never reject or throw an exception. Instead return false by default, to avoid emitting SDK_READY_FROM_CACHE. checkCache(): MaybeThenable, killLocally(name: string, defaultTreatment: string, changeNumber: number): MaybeThenable, - getNamesByFlagSets(flagSets: string[]): MaybeThenable> + getNamesByFlagSets(flagSets: string[]): MaybeThenable[]> } export interface ISplitsCacheSync extends ISplitsCacheBase { @@ -227,7 +227,7 @@ export interface ISplitsCacheSync extends ISplitsCacheBase { clear(): void, checkCache(): boolean, killLocally(name: string, defaultTreatment: string, changeNumber: number): boolean, - getNamesByFlagSets(flagSets: string[]): ISet + getNamesByFlagSets(flagSets: string[]): ISet[] } export interface ISplitsCacheAsync extends ISplitsCacheBase { @@ -244,7 +244,7 @@ export interface ISplitsCacheAsync extends ISplitsCacheBase { clear(): Promise, checkCache(): Promise, killLocally(name: string, defaultTreatment: string, changeNumber: number): Promise, - getNamesByFlagSets(flagSets: string[]): Promise> + getNamesByFlagSets(flagSets: string[]): Promise[]> } /** Segments cache */ diff --git a/src/types.ts b/src/types.ts index b71f6e80..020c5909 100644 --- a/src/types.ts +++ b/src/types.ts @@ -197,8 +197,6 @@ interface ISharedSettings { * List of feature flag filters. These filters are used to fetch a subset of the feature flag definitions in your environment, in order to reduce the delay of the SDK to be ready. * This configuration is only meaningful when the SDK is working in "standalone" mode. * - * At the moment, only one type of feature flag filter is supported: by name. - * * Example: * `splitFilter: [ * { type: 'byName', values: ['my_feature_flag_1', 'my_feature_flag_2'] }, // will fetch feature flags named 'my_feature_flag_1' and 'my_feature_flag_2' diff --git a/src/utils/inputValidation/__tests__/splitExistance.spec.ts b/src/utils/inputValidation/__tests__/splitExistence.spec.ts similarity index 81% rename from src/utils/inputValidation/__tests__/splitExistance.spec.ts rename to src/utils/inputValidation/__tests__/splitExistence.spec.ts index 7d0d7b39..9d78df9e 100644 --- a/src/utils/inputValidation/__tests__/splitExistance.spec.ts +++ b/src/utils/inputValidation/__tests__/splitExistence.spec.ts @@ -3,7 +3,7 @@ import * as LabelConstants from '../../labels'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; -import { validateSplitExistance } from '../splitExistance'; +import { validateSplitExistence } from '../splitExistence'; import { IReadinessManager } from '../../../readiness/types'; import { WARN_NOT_EXISTENT_SPLIT } from '../../../logger/constants'; @@ -17,11 +17,11 @@ describe('Split existence (special case)', () => { isReady: jest.fn(() => false) // Fake the signal for the non ready SDK } as IReadinessManager; - expect(validateSplitExistance(loggerMock, readinessManagerMock, 'some_split', {}, 'test_method')).toBe(true); // Should always return true when the SDK is not ready. - expect(validateSplitExistance(loggerMock, readinessManagerMock, 'some_split', null, 'test_method')).toBe(true); // Should always return true when the SDK is not ready. - expect(validateSplitExistance(loggerMock, readinessManagerMock, 'some_split', undefined, 'test_method')).toBe(true); // Should always return true when the SDK is not ready. - expect(validateSplitExistance(loggerMock, readinessManagerMock, 'some_split', 'a label', 'test_method')).toBe(true); // Should always return true when the SDK is not ready. - expect(validateSplitExistance(loggerMock, readinessManagerMock, 'some_split', LabelConstants.SPLIT_NOT_FOUND, 'test_method')).toBe(true); // Should always return true when the SDK is not ready. + expect(validateSplitExistence(loggerMock, readinessManagerMock, 'some_split', {}, 'test_method')).toBe(true); // Should always return true when the SDK is not ready. + expect(validateSplitExistence(loggerMock, readinessManagerMock, 'some_split', null, 'test_method')).toBe(true); // Should always return true when the SDK is not ready. + expect(validateSplitExistence(loggerMock, readinessManagerMock, 'some_split', undefined, 'test_method')).toBe(true); // Should always return true when the SDK is not ready. + expect(validateSplitExistence(loggerMock, readinessManagerMock, 'some_split', 'a label', 'test_method')).toBe(true); // Should always return true when the SDK is not ready. + expect(validateSplitExistence(loggerMock, readinessManagerMock, 'some_split', LabelConstants.SPLIT_NOT_FOUND, 'test_method')).toBe(true); // Should always return true when the SDK is not ready. expect(loggerMock.warn).not.toBeCalled(); // There should have been no warning logs since the SDK was not ready yet. expect(loggerMock.error).not.toBeCalled(); // There should have been no error logs since the SDK was not ready yet. @@ -29,15 +29,15 @@ describe('Split existence (special case)', () => { // Prepare the mock to fake that the SDK is ready now. (readinessManagerMock.isReady as jest.Mock).mockImplementation(() => true); - expect(validateSplitExistance(loggerMock, readinessManagerMock, 'other_split', {}, 'other_method')).toBe(true); // Should return true if it receives a Split Object instead of null (when the object is not found, for manager). - expect(validateSplitExistance(loggerMock, readinessManagerMock, 'other_split', 'a label', 'other_method')).toBe(true); // Should return true if it receives a Label and it is not split not found (when the Split was not found on the storage, for client). + expect(validateSplitExistence(loggerMock, readinessManagerMock, 'other_split', {}, 'other_method')).toBe(true); // Should return true if it receives a Split Object instead of null (when the object is not found, for manager). + expect(validateSplitExistence(loggerMock, readinessManagerMock, 'other_split', 'a label', 'other_method')).toBe(true); // Should return true if it receives a Label and it is not split not found (when the Split was not found on the storage, for client). expect(loggerMock.warn).not.toBeCalled(); // There should have been no warning logs since the values we used so far were considered valid. expect(loggerMock.error).not.toBeCalled(); // There should have been no error logs since the values we used so far were considered valid. - expect(validateSplitExistance(loggerMock, readinessManagerMock, 'other_split', null, 'other_method')).toBe(false); // Should return false if it receives a non-truthy value as a split object or label - expect(validateSplitExistance(loggerMock, readinessManagerMock, 'other_split', undefined, 'other_method')).toBe(false); // Should return false if it receives a non-truthy value as a split object or label - expect(validateSplitExistance(loggerMock, readinessManagerMock, 'other_split', LabelConstants.SPLIT_NOT_FOUND, 'other_method')).toBe(false); // Should return false if it receives a label but it is the split not found one. + expect(validateSplitExistence(loggerMock, readinessManagerMock, 'other_split', null, 'other_method')).toBe(false); // Should return false if it receives a non-truthy value as a split object or label + expect(validateSplitExistence(loggerMock, readinessManagerMock, 'other_split', undefined, 'other_method')).toBe(false); // Should return false if it receives a non-truthy value as a split object or label + expect(validateSplitExistence(loggerMock, readinessManagerMock, 'other_split', LabelConstants.SPLIT_NOT_FOUND, 'other_method')).toBe(false); // Should return false if it receives a label but it is the split not found one. expect(loggerMock.warn).toBeCalledTimes(3); // It should have logged 3 warnings, one per each time we called it loggerMock.warn.mock.calls.forEach(call => expect(call).toEqual([WARN_NOT_EXISTENT_SPLIT, ['other_method', 'other_split']])); // Warning logs should have the correct message. diff --git a/src/utils/inputValidation/__tests__/trafficTypeExistance.spec.ts b/src/utils/inputValidation/__tests__/trafficTypeExistence.spec.ts similarity index 92% rename from src/utils/inputValidation/__tests__/trafficTypeExistance.spec.ts rename to src/utils/inputValidation/__tests__/trafficTypeExistence.spec.ts index ad19302f..a71912cf 100644 --- a/src/utils/inputValidation/__tests__/trafficTypeExistance.spec.ts +++ b/src/utils/inputValidation/__tests__/trafficTypeExistence.spec.ts @@ -28,9 +28,9 @@ const splitsCacheMock = { } as ISplitsCacheBase & { trafficTypeExists: jest.Mock }; /** Test target */ -import { validateTrafficTypeExistance } from '../trafficTypeExistance'; +import { validateTrafficTypeExistence } from '../trafficTypeExistence'; -describe('validateTrafficTypeExistance', () => { +describe('validateTrafficTypeExistence', () => { afterEach(() => { loggerMock.mockClear(); @@ -39,14 +39,14 @@ describe('validateTrafficTypeExistance', () => { test('Should return true without going to the storage and log nothing if the SDK is not ready or in localhost mode', () => { // Not ready and not localstorage - expect(validateTrafficTypeExistance(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, 'test_tt', 'test_method')).toBe(true); // If the SDK is not ready yet, it will return true. + expect(validateTrafficTypeExistence(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, 'test_tt', 'test_method')).toBe(true); // If the SDK is not ready yet, it will return true. expect(splitsCacheMock.trafficTypeExists).not.toBeCalled(); // If the SDK is not ready yet, it does not try to go to the storage. expect(loggerMock.error).not.toBeCalled(); // If the SDK is not ready yet, it will not log any errors. expect(loggerMock.error).not.toBeCalled(); // If the SDK is not ready yet, it will not log any errors. // Ready and in localstorage mode. readinessManagerMock.isReady.mockImplementation(() => true); - expect(validateTrafficTypeExistance(loggerMock, readinessManagerMock, splitsCacheMock, LOCALHOST_MODE, 'test_tt', 'test_method')).toBe(true); // If the SDK is in localhost mode, it will return true. + expect(validateTrafficTypeExistence(loggerMock, readinessManagerMock, splitsCacheMock, LOCALHOST_MODE, 'test_tt', 'test_method')).toBe(true); // If the SDK is in localhost mode, it will return true. expect(splitsCacheMock.trafficTypeExists).not.toBeCalled(); // If the SDK is in localhost mode, it does not try to go to the storage. expect(loggerMock.warn).not.toBeCalled(); // If the SDK is in localhost mode, it will not log any warnings. expect(loggerMock.error).not.toBeCalled(); // If the SDK is in localhost mode, it will not log any errors. @@ -56,7 +56,7 @@ describe('validateTrafficTypeExistance', () => { // Ready, standalone, and the TT exists in the storage. readinessManagerMock.isReady.mockImplementation(() => true); - expect(validateTrafficTypeExistance(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, TEST_EXISTENT_TT, 'test_method')).toBe(true); // If the SDK is in condition to validate but the TT exists, it will return true. + expect(validateTrafficTypeExistence(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, TEST_EXISTENT_TT, 'test_method')).toBe(true); // If the SDK is in condition to validate but the TT exists, it will return true. expect(splitsCacheMock.trafficTypeExists.mock.calls).toEqual([[TEST_EXISTENT_TT]]); // If the SDK is in condition to validate, it checks that TT existence with the storage. expect(loggerMock.warn).not.toBeCalled(); // If the SDK is in condition to validate but the TT exists, it will not log any warnings. expect(loggerMock.error).not.toBeCalled(); // If the SDK is in condition to validate but the TT exists, it will not log any errors. @@ -64,16 +64,16 @@ describe('validateTrafficTypeExistance', () => { test('Should return false and log warning if SDK Ready, not localhost mode and the traffic type does NOT exist in the storage', () => { // Ready, standalone, and the TT not exists in the storage. - expect(validateTrafficTypeExistance(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, TEST_NOT_EXISTENT_TT, 'test_method_y')).toBe(false); // If the SDK is in condition to validate but the TT does not exist in the storage, it will return false. + expect(validateTrafficTypeExistence(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, TEST_NOT_EXISTENT_TT, 'test_method_y')).toBe(false); // If the SDK is in condition to validate but the TT does not exist in the storage, it will return false. expect(splitsCacheMock.trafficTypeExists.mock.calls).toEqual([[TEST_NOT_EXISTENT_TT]]); // If the SDK is in condition to validate, it checks that TT existence with the storage. expect(loggerMock.warn).toBeCalledWith(WARN_NOT_EXISTENT_TT, ['test_method_y', TEST_NOT_EXISTENT_TT]); // If the SDK is in condition to validate but the TT does not exist in the storage, it will log the expected warning. expect(loggerMock.error).not.toBeCalled(); // It logged a warning so no errors should be logged. }); - test('validateTrafficTypeExistance w/async storage - If the storage is async but the SDK is in condition to validate, it will validate that the TT exists on the storage', async () => { + test('validateTrafficTypeExistence w/async storage - If the storage is async but the SDK is in condition to validate, it will validate that the TT exists on the storage', async () => { // Ready, standalone, the TT exists in the storage. - const validationPromise = validateTrafficTypeExistance(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, TEST_EXISTENT_ASYNC_TT, 'test_method_z'); + const validationPromise = validateTrafficTypeExistence(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, TEST_EXISTENT_ASYNC_TT, 'test_method_z'); expect(thenable(validationPromise)).toBe(true); // If the storage is async, it should also return a promise. expect(splitsCacheMock.trafficTypeExists.mock.calls).toEqual([[TEST_EXISTENT_ASYNC_TT]]); // If the SDK is in condition to validate, it checks that TT existence with the async storage. expect(loggerMock.warn).not.toBeCalled(); // We are still fetching the data from the storage, no logs yet. @@ -88,7 +88,7 @@ describe('validateTrafficTypeExistance', () => { // Second round, a TT that does not exist on the asnyc storage splitsCacheMock.trafficTypeExists.mockClear(); - const validationPromise2 = validateTrafficTypeExistance(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, TEST_NOT_EXISTENT_ASYNC_TT, 'test_method_z'); + const validationPromise2 = validateTrafficTypeExistence(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, TEST_NOT_EXISTENT_ASYNC_TT, 'test_method_z'); expect(thenable(validationPromise2)).toBe(true); // If the storage is async, it should also return a promise. expect(splitsCacheMock.trafficTypeExists.mock.calls).toEqual([[TEST_NOT_EXISTENT_ASYNC_TT]]); // If the SDK is in condition to validate, it checks that TT existence with the async storage. expect(loggerMock.warn).not.toBeCalled(); // We are still fetching the data from the storage, no logs yet. diff --git a/src/utils/inputValidation/index.ts b/src/utils/inputValidation/index.ts index 31835a24..93b09ab7 100644 --- a/src/utils/inputValidation/index.ts +++ b/src/utils/inputValidation/index.ts @@ -8,6 +8,6 @@ export { validateSplit } from './split'; export { validateSplits } from './splits'; export { validateTrafficType } from './trafficType'; export { validateIfNotDestroyed, validateIfOperational } from './isOperational'; -export { validateSplitExistance } from './splitExistance'; -export { validateTrafficTypeExistance } from './trafficTypeExistance'; +export { validateSplitExistence } from './splitExistence'; +export { validateTrafficTypeExistence } from './trafficTypeExistence'; export { validatePreloadedData } from './preloadedData'; diff --git a/src/utils/inputValidation/splitExistance.ts b/src/utils/inputValidation/splitExistence.ts similarity index 93% rename from src/utils/inputValidation/splitExistance.ts rename to src/utils/inputValidation/splitExistence.ts index ddafa767..2f3105f9 100644 --- a/src/utils/inputValidation/splitExistance.ts +++ b/src/utils/inputValidation/splitExistence.ts @@ -7,7 +7,7 @@ import { WARN_NOT_EXISTENT_SPLIT } from '../../logger/constants'; * This is defined here and in this format mostly because of the logger and the fact that it's considered a validation at product level. * But it's not going to run on the input validation layer. In any case, the most compeling reason to use it as we do is to avoid going to Redis and get a split twice. */ -export function validateSplitExistance(log: ILogger, readinessManager: IReadinessManager, splitName: string, labelOrSplitObj: any, method: string): boolean { +export function validateSplitExistence(log: ILogger, readinessManager: IReadinessManager, splitName: string, labelOrSplitObj: any, method: string): boolean { if (readinessManager.isReady()) { // Only if it's ready we validate this, otherwise it may just be that the SDK is not ready yet. if (labelOrSplitObj === SPLIT_NOT_FOUND || labelOrSplitObj == null) { log.warn(WARN_NOT_EXISTENT_SPLIT, [method, splitName]); diff --git a/src/utils/inputValidation/trafficTypeExistance.ts b/src/utils/inputValidation/trafficTypeExistence.ts similarity index 81% rename from src/utils/inputValidation/trafficTypeExistance.ts rename to src/utils/inputValidation/trafficTypeExistence.ts index 743ceb8f..9619f197 100644 --- a/src/utils/inputValidation/trafficTypeExistance.ts +++ b/src/utils/inputValidation/trafficTypeExistence.ts @@ -7,14 +7,14 @@ import { MaybeThenable } from '../../dtos/types'; import { ILogger } from '../../logger/types'; import { WARN_NOT_EXISTENT_TT } from '../../logger/constants'; -function logTTExistanceWarning(log: ILogger, maybeTT: string, method: string) { +function logTTExistenceWarning(log: ILogger, maybeTT: string, method: string) { log.warn(WARN_NOT_EXISTENT_TT, [method, maybeTT]); } /** * Separated from the previous method since on some cases it'll be async. */ -export function validateTrafficTypeExistance(log: ILogger, readinessManager: IReadinessManager, splitsCache: ISplitsCacheBase, mode: SDKMode, maybeTT: string, method: string): MaybeThenable { +export function validateTrafficTypeExistence(log: ILogger, readinessManager: IReadinessManager, splitsCache: ISplitsCacheBase, mode: SDKMode, maybeTT: string, method: string): MaybeThenable { // If not ready or in localhost mode, we won't run the validation if (!readinessManager.isReady() || mode === LOCALHOST_MODE) return true; @@ -23,11 +23,11 @@ export function validateTrafficTypeExistance(log: ILogger, readinessManager: IRe if (thenable(res)) { return res.then(function (isValid) { - if (!isValid) logTTExistanceWarning(log, maybeTT, method); + if (!isValid) logTTExistenceWarning(log, maybeTT, method); return isValid; // propagate result }); } else { - if (!res) logTTExistanceWarning(log, maybeTT, method); + if (!res) logTTExistenceWarning(log, maybeTT, method); return res; } } diff --git a/src/utils/lang/sets.ts b/src/utils/lang/sets.ts index f89fc29f..bda2c612 100644 --- a/src/utils/lang/sets.ts +++ b/src/utils/lang/sets.ts @@ -114,8 +114,16 @@ export const _Set = __getSetConstructor(); export function returnSetsUnion(set: ISet, set2: ISet): ISet { const result = new _Set(setToArray(set)); - set2.forEach( value => { + set2.forEach(value => { result.add(value); }); return result; } + +export function returnListDifference(list: T[] = [], list2: T[] = []): T[] { + const result = new _Set(list); + list2.forEach(item => { + result.delete(item); + }); + return setToArray(result); +} 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 5fe5dfb2..553c2604 100644 --- a/src/utils/settingsValidation/__tests__/splitFilters.spec.ts +++ b/src/utils/settingsValidation/__tests__/splitFilters.spec.ts @@ -1,11 +1,12 @@ import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; +import { STANDALONE_MODE, CONSUMER_MODE, CONSUMER_PARTIAL_MODE, LOCALHOST_MODE, PRODUCER_MODE } from '../../constants'; // Split filter and QueryStrings examples import { splitFilters, queryStrings, groupedFilters } from '../../../__tests__/mocks/fetchSpecificSplits'; // Test target import { flagSetsAreValid, 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 } from '../../../logger/constants'; +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', () => { @@ -28,17 +29,21 @@ describe('validateSplitFilters', () => { afterEach(() => { loggerMock.mockClear(); }); - test('Returns default output with empty values if `splitFilters` is an invalid object or `mode` is not \'standalone\'', () => { + test('Returns default output with empty values if `splitFilters` is an invalid object or `mode` is \'consumer\' or \'consumer_partial\'', () => { - expect(validateSplitFilters(loggerMock, undefined)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array - expect(validateSplitFilters(loggerMock, null)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array + expect(validateSplitFilters(loggerMock, undefined, STANDALONE_MODE)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array + expect(validateSplitFilters(loggerMock, null, STANDALONE_MODE)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array expect(loggerMock.warn).not.toBeCalled(); - expect(validateSplitFilters(loggerMock, true)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array - expect(validateSplitFilters(loggerMock, 15)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array - expect(validateSplitFilters(loggerMock, 'string')).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array - expect(validateSplitFilters(loggerMock, [])).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array - expect(loggerMock.warn.mock.calls).toEqual([[WARN_SPLITS_FILTER_EMPTY], [WARN_SPLITS_FILTER_EMPTY], [WARN_SPLITS_FILTER_EMPTY], [WARN_SPLITS_FILTER_EMPTY]]); + expect(validateSplitFilters(loggerMock, true, STANDALONE_MODE)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array + expect(validateSplitFilters(loggerMock, 15, STANDALONE_MODE)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array + expect(validateSplitFilters(loggerMock, 'string', STANDALONE_MODE)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array + expect(validateSplitFilters(loggerMock, [], STANDALONE_MODE)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array + + expect(validateSplitFilters(loggerMock, [{ type: 'byName', values: ['split_1'] }], CONSUMER_MODE)).toEqual(defaultOutput); // splitFilters ignored if in consumer mode + expect(validateSplitFilters(loggerMock, [{ type: 'byName', values: ['split_1'] }], CONSUMER_PARTIAL_MODE)).toEqual(defaultOutput); // splitFilters ignored if in partial consumer mode + + expect(loggerMock.warn.mock.calls).toEqual([[WARN_SPLITS_FILTER_EMPTY], [WARN_SPLITS_FILTER_EMPTY], [WARN_SPLITS_FILTER_EMPTY], [WARN_SPLITS_FILTER_EMPTY], [WARN_SPLITS_FILTER_IGNORED], [WARN_SPLITS_FILTER_IGNORED]]); expect(loggerMock.debug).not.toBeCalled(); expect(loggerMock.error).not.toBeCalled(); @@ -56,7 +61,7 @@ describe('validateSplitFilters', () => { queryString: null, groupedFilters: { bySet: [], byName: [], byPrefix: [] }, }; - expect(validateSplitFilters(loggerMock, splitFilters)).toEqual(output); // filters without values + expect(validateSplitFilters(loggerMock, splitFilters, STANDALONE_MODE)).toEqual(output); // filters without values expect(loggerMock.debug).toBeCalledWith(SETTINGS_SPLITS_FILTER, [null]); loggerMock.debug.mockClear(); @@ -66,7 +71,7 @@ describe('validateSplitFilters', () => { { type: null, values: [] }, { type: 'byName', values: [13] }); output.validFilters.push({ type: 'byName', values: [13] }); - expect(validateSplitFilters(loggerMock, splitFilters)).toEqual(output); // some filters are invalid + expect(validateSplitFilters(loggerMock, splitFilters, STANDALONE_MODE)).toEqual(output); // some filters are invalid expect(loggerMock.debug.mock.calls).toEqual([[SETTINGS_SPLITS_FILTER, [null]]]); expect(loggerMock.warn.mock.calls).toEqual([ [WARN_SPLITS_FILTER_INVALID, [4]], // invalid value of `type` property @@ -90,24 +95,24 @@ describe('validateSplitFilters', () => { queryString: queryStrings[i], groupedFilters: groupedFilters[i], }; - expect(validateSplitFilters(loggerMock, splitFilters[i])).toEqual(output); // splitFilters #${i} + expect(validateSplitFilters(loggerMock, splitFilters[i], STANDALONE_MODE)).toEqual(output); // splitFilters #${i} expect(loggerMock.debug).lastCalledWith(SETTINGS_SPLITS_FILTER, [queryStrings[i]]); } else { // tests where validateSplitFilters throws an exception - expect(() => validateSplitFilters(loggerMock, splitFilters[i])).toThrow(queryStrings[i]); + expect(() => validateSplitFilters(loggerMock, splitFilters[i], STANDALONE_MODE)).toThrow(queryStrings[i]); } } }); test('Validates flag set filters', () => { // extra spaces trimmed and sorted query output - expect(validateSplitFilters(loggerMock, splitFilters[6])).toEqual(getOutput(6)); // trim & sort + expect(validateSplitFilters(loggerMock, splitFilters[6], STANDALONE_MODE)).toEqual(getOutput(6)); // trim & sort expect(loggerMock.warn.mock.calls[0]).toEqual([WARN_TRIMMING, ['settings', 'bySet filter value', ' set_1']]); expect(loggerMock.warn.mock.calls[1]).toEqual([WARN_TRIMMING, ['settings', 'bySet filter value', 'set_3 ']]); expect(loggerMock.warn.mock.calls[2]).toEqual([WARN_TRIMMING, ['settings', 'bySet filter value', ' set_a ']]); expect(loggerMock.error.mock.calls[0]).toEqual([ERROR_SETS_FILTER_EXCLUSIVE]); - expect(validateSplitFilters(loggerMock, splitFilters[7])).toEqual(getOutput(7)); // lowercase and regexp + expect(validateSplitFilters(loggerMock, splitFilters[7], LOCALHOST_MODE)).toEqual(getOutput(7)); // lowercase and regexp expect(loggerMock.warn.mock.calls[3]).toEqual([WARN_SPLITS_FILTER_LOWERCASE_SET, ['seT_c']]); // lowercase expect(loggerMock.warn.mock.calls[4]).toEqual([WARN_SPLITS_FILTER_LOWERCASE_SET, ['set_B']]); // lowercase expect(loggerMock.warn.mock.calls[5]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set_ 1', regexp, 'set_ 1']]); // empty spaces @@ -116,7 +121,7 @@ describe('validateSplitFilters', () => { expect(loggerMock.warn.mock.calls[8]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set_1234567890_1234567890_234567890_1234567890_1234567890', regexp, 'set_1234567890_1234567890_234567890_1234567890_1234567890']]); // max of 50 characters expect(loggerMock.error.mock.calls[1]).toEqual([ERROR_SETS_FILTER_EXCLUSIVE]); - expect(validateSplitFilters(loggerMock, splitFilters[8])).toEqual(getOutput(8)); // lowercase and dedupe + expect(validateSplitFilters(loggerMock, splitFilters[8], PRODUCER_MODE)).toEqual(getOutput(8)); // lowercase and dedupe expect(loggerMock.warn.mock.calls[9]).toEqual([WARN_SPLITS_FILTER_LOWERCASE_SET, ['SET_2']]); // lowercase expect(loggerMock.warn.mock.calls[10]).toEqual([WARN_SPLITS_FILTER_LOWERCASE_SET, ['set_B']]); // lowercase expect(loggerMock.warn.mock.calls[11]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set_3!', regexp, 'set_3!']]); // special character @@ -147,21 +152,21 @@ describe('validateSplitFilters', () => { 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(flagSetsAreValid(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(flagSetsAreValid(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(flagSetsAreValid(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(loggerMock.warn.mock.calls[7]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, ['test_method','set_3']]); + expect(loggerMock.warn.mock.calls[7]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, ['test_method', 'set_3']]); // empty config @@ -179,17 +184,17 @@ describe('validateSplitFilters', () => { 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(flagSetsAreValid(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(flagSetsAreValid(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_1', 'set_2'], [])).toEqual(['set_1', 'set_2']); expect(flagSetsAreValid(loggerMock, 'test_method', ['set_3'], [])).toEqual(['set_3']); }); diff --git a/src/utils/settingsValidation/index.ts b/src/utils/settingsValidation/index.ts index 8affa738..700ada43 100644 --- a/src/utils/settingsValidation/index.ts +++ b/src/utils/settingsValidation/index.ts @@ -202,7 +202,7 @@ export function settingsValidation(config: unknown, validationParams: ISettingsV } // validate the `splitFilters` settings and parse splits query - const splitFiltersValidation = validateSplitFilters(log, withDefaults.sync.splitFilters); + const splitFiltersValidation = validateSplitFilters(log, withDefaults.sync.splitFilters, withDefaults.mode); withDefaults.sync.splitFilters = splitFiltersValidation.validFilters; withDefaults.sync.__splitFiltersValidation = splitFiltersValidation; diff --git a/src/utils/settingsValidation/splitFilters.ts b/src/utils/settingsValidation/splitFilters.ts index 023c3fc1..a1d9e6eb 100644 --- a/src/utils/settingsValidation/splitFilters.ts +++ b/src/utils/settingsValidation/splitFilters.ts @@ -1,8 +1,9 @@ +import { CONSUMER_MODE, CONSUMER_PARTIAL_MODE } from '../constants'; import { validateSplits } from '../inputValidation/splits'; import { ISplitFiltersValidation } from '../../dtos/types'; import { SplitIO } from '../../types'; import { ILogger } from '../../logger/types'; -import { WARN_SPLITS_FILTER_EMPTY, WARN_SPLITS_FILTER_INVALID, SETTINGS_SPLITS_FILTER, LOG_PREFIX_SETTINGS, ERROR_SETS_FILTER_EXCLUSIVE, WARN_SPLITS_FILTER_LOWERCASE_SET, WARN_SPLITS_FILTER_INVALID_SET, WARN_FLAGSET_NOT_CONFIGURED } from '../../logger/constants'; +import { WARN_SPLITS_FILTER_IGNORED, WARN_SPLITS_FILTER_EMPTY, WARN_SPLITS_FILTER_INVALID, SETTINGS_SPLITS_FILTER, LOG_PREFIX_SETTINGS, ERROR_SETS_FILTER_EXCLUSIVE, WARN_SPLITS_FILTER_LOWERCASE_SET, WARN_SPLITS_FILTER_INVALID_SET, WARN_FLAGSET_NOT_CONFIGURED } from '../../logger/constants'; import { objectAssign } from '../lang/objectAssign'; import { find, uniq } from '../lang'; @@ -102,15 +103,15 @@ function queryStringBuilder(groupedFilters: Record { - if (CAPITAL_LETTERS_REGEX.test(flagSet)){ - log.warn(WARN_SPLITS_FILTER_LOWERCASE_SET,[flagSet]); + if (CAPITAL_LETTERS_REGEX.test(flagSet)) { + log.warn(WARN_SPLITS_FILTER_LOWERCASE_SET, [flagSet]); flagSet = flagSet.toLowerCase(); } return flagSet; }) .filter(flagSet => { - if (!VALID_FLAGSET_REGEX.test(flagSet)){ - log.warn(WARN_SPLITS_FILTER_INVALID_SET, [flagSet,VALID_FLAGSET_REGEX,flagSet]); + if (!VALID_FLAGSET_REGEX.test(flagSet)) { + log.warn(WARN_SPLITS_FILTER_INVALID_SET, [flagSet, VALID_FLAGSET_REGEX, flagSet]); return false; } if (typeof flagSet !== 'string') return false; @@ -128,6 +129,7 @@ function configuredFilter(validFilters: SplitIO.SplitFilter[], filterType: Split * * @param {ILogger} log logger * @param {any} maybeSplitFilters split filters configuration param provided by the user + * @param {string} mode settings mode * @returns it returns an object with the following properties: * - `validFilters`: the validated `splitFilters` configuration object defined by the user. * - `queryString`: the parsed split filter query. it is null if all filters are invalid or all values in filters are invalid. @@ -135,7 +137,7 @@ function configuredFilter(validFilters: SplitIO.SplitFilter[], filterType: Split * * @throws Error if the some of the grouped list of values per filter exceeds the max allowed length */ -export function validateSplitFilters(log: ILogger, maybeSplitFilters: any): ISplitFiltersValidation { +export function validateSplitFilters(log: ILogger, maybeSplitFilters: any, mode: string): ISplitFiltersValidation { // Validation result schema const res = { validFilters: [], @@ -145,6 +147,11 @@ export function validateSplitFilters(log: ILogger, maybeSplitFilters: any): ISpl // do nothing if `splitFilters` param is not a non-empty array or mode is not STANDALONE if (!maybeSplitFilters) return res; + // Warn depending on the mode + if (mode === CONSUMER_MODE || mode === CONSUMER_PARTIAL_MODE) { + log.warn(WARN_SPLITS_FILTER_IGNORED); + return res; + } // Check collection type if (!Array.isArray(maybeSplitFilters) || maybeSplitFilters.length === 0) { log.warn(WARN_SPLITS_FILTER_EMPTY);