diff --git a/CHANGES.txt b/CHANGES.txt index 03adce03..89b98466 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,6 @@ 1.12.0 (December XX, 2023) - Added support for Flag Sets in "consumer" and "partial consumer" modes for pluggable storage. + - Updated evaluation flow to log a warning when using flag sets that don't contain cached feature flags. 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): diff --git a/package-lock.json b/package-lock.json index 5c28d3a7..b79d5936 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.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-commons", - "version": "1.11.0", + "version": "1.12.1-rc.0", "license": "Apache-2.0", "dependencies": { "tslib": "^2.3.1" diff --git a/package.json b/package.json index 5d2087b4..65b3d060 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.11.0", + "version": "1.12.1-rc.0", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", @@ -22,7 +22,7 @@ "build": "npm run build:cjs && npm run build:esm", "build:esm": "rimraf esm && tsc -m es2015 --outDir esm -d true --declarationDir types", "build:cjs": "rimraf cjs && tsc -m CommonJS --outDir cjs", - "test": "jest --runInBand", + "test": "jest", "test:coverage": "jest --coverage", "all": "npm run check && npm run build && npm run test", "publish:rc": "npm run check && npm run test && npm run build && npm publish --tag rc", 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 961a1a98..91e601fc 100644 --- a/src/logger/constants.ts +++ b/src/logger/constants.ts @@ -100,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 2dafabdf..3feca12f 100644 --- a/src/logger/messages/warn.ts +++ b/src/logger/messages/warn.ts @@ -36,4 +36,5 @@ export const codesWarn: [number, string][] = codesError.concat([ [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_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/sdkClient/client.ts b/src/sdkClient/client.ts index 31aa5f74..1c10da3a 100644 --- a/src/sdkClient/client.ts +++ b/src/sdkClient/client.ts @@ -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); 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 36a55fe1..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. @@ -105,15 +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) { @@ -128,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/SplitsCacheInRedis.ts b/src/storages/inRedis/SplitsCacheInRedis.ts index f58f49ab..6aa1136b 100644 --- a/src/storages/inRedis/SplitsCacheInRedis.ts +++ b/src/storages/inRedis/SplitsCacheInRedis.ts @@ -195,7 +195,7 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync { * or rejected if wrapper operation fails. * @todo this is a no-op method to be implemented */ - getNamesByFlagSets(): Promise> { + getNamesByFlagSets(): Promise[]> { this.log.error(LOG_PREFIX + 'ByFlagSet/s evaluations are not supported with Redis storage yet.'); return Promise.reject(); } diff --git a/src/storages/pluggable/SplitsCachePluggable.ts b/src/storages/pluggable/SplitsCachePluggable.ts index da409eb2..f485e1a5 100644 --- a/src/storages/pluggable/SplitsCachePluggable.ts +++ b/src/storages/pluggable/SplitsCachePluggable.ts @@ -181,17 +181,11 @@ export class SplitsCachePluggable extends AbstractSplitsCacheAsync { * The returned promise is resolved with the list of split names, * or rejected if any wrapper operation fails. */ - getNamesByFlagSets(flagSets: string[]): Promise> { + getNamesByFlagSets(flagSets: string[]): Promise[]> { return Promise.all(flagSets.map(flagSet => { const flagSetKey = this.keys.buildFlagSetKey(flagSet); return this.wrapper.getItems(flagSetKey); - })).then(namesByFlagSets => { - const featureFlagNames = new _Set(); - namesByFlagSets.forEach(names => { - names.forEach(name => featureFlagNames.add(name)); - }); - return featureFlagNames; - }); + })).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 3d67e454..03221de0 100644 --- a/src/storages/pluggable/__tests__/SplitsCachePluggable.spec.ts +++ b/src/storages/pluggable/__tests__/SplitsCachePluggable.spec.ts @@ -163,31 +163,31 @@ describe('SPLITS CACHE PLUGGABLE', () => { ]); 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', 'ff_three'])); + 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); + 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_one', 'ff_two', 'ff_three'])); + 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'])]); await cache.removeSplit(featureFlagOne.name); - expect(await cache.getNamesByFlagSets(['x'])).toEqual(emptySet); + 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(emptySet); + 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(emptySet); + expect(await cache.getNamesByFlagSets([])).toEqual([]); }); // if FlagSets filter is not defined, it should store all FlagSets in memory. @@ -202,12 +202,12 @@ describe('SPLITS CACHE PLUGGABLE', () => { ]); 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', 'ff_three'])); + 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/types.ts b/src/storages/types.ts index 8f45f4b2..9e8dc563 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -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 */