diff --git a/src/logger/constants.ts b/src/logger/constants.ts index 520a5707..855675ff 100644 --- a/src/logger/constants.ts +++ b/src/logger/constants.ts @@ -20,9 +20,7 @@ export const RETRIEVE_CLIENT_EXISTING = 28; export const RETRIEVE_MANAGER = 29; export const SYNC_OFFLINE_DATA = 30; export const SYNC_SPLITS_FETCH = 31; -export const SYNC_SPLITS_NEW = 32; -export const SYNC_SPLITS_REMOVED = 33; -export const SYNC_SPLITS_SEGMENTS = 34; +export const SYNC_SPLITS_UPDATE = 32; export const STREAMING_NEW_MESSAGE = 35; export const SYNC_TASK_START = 36; export const SYNC_TASK_EXECUTE = 37; diff --git a/src/logger/messages/debug.ts b/src/logger/messages/debug.ts index c89694e6..5dfcace3 100644 --- a/src/logger/messages/debug.ts +++ b/src/logger/messages/debug.ts @@ -21,9 +21,7 @@ export const codesDebug: [number, string][] = codesInfo.concat([ // synchronizer [c.SYNC_OFFLINE_DATA, c.LOG_PREFIX_SYNC_OFFLINE + 'Feature flags data: \n%s'], [c.SYNC_SPLITS_FETCH, c.LOG_PREFIX_SYNC_SPLITS + 'Spin up feature flags update using since = %s'], - [c.SYNC_SPLITS_NEW, c.LOG_PREFIX_SYNC_SPLITS + 'New feature flags %s'], - [c.SYNC_SPLITS_REMOVED, c.LOG_PREFIX_SYNC_SPLITS + 'Removed feature flags %s'], - [c.SYNC_SPLITS_SEGMENTS, c.LOG_PREFIX_SYNC_SPLITS + 'Segment names collected %s'], + [c.SYNC_SPLITS_UPDATE, c.LOG_PREFIX_SYNC_SPLITS + 'New feature flags %s. Removed feature flags %s. Segment names collected %s'], [c.STREAMING_NEW_MESSAGE, c.LOG_PREFIX_SYNC_STREAMING + 'New SSE message received, with data: %s.'], [c.SYNC_TASK_START, c.LOG_PREFIX_SYNC + ': Starting %s. Running each %s millis'], [c.SYNC_TASK_EXECUTE, c.LOG_PREFIX_SYNC + ': Running %s'], diff --git a/src/sdkManager/__tests__/index.asyncCache.spec.ts b/src/sdkManager/__tests__/index.asyncCache.spec.ts index a0277a75..429513d4 100644 --- a/src/sdkManager/__tests__/index.asyncCache.spec.ts +++ b/src/sdkManager/__tests__/index.asyncCache.spec.ts @@ -33,7 +33,7 @@ describe('Manager with async cache', () => { const cache = new SplitsCacheInRedis(loggerMock, keys, connection); const manager = sdkManagerFactory({ mode: 'consumer', log: loggerMock }, cache, sdkReadinessManagerMock); await cache.clear(); - await cache.addSplit(splitObject.name, splitObject as any); + await cache.addSplit(splitObject as any); /** List all splits */ const views = await manager.splits(); diff --git a/src/sdkManager/__tests__/index.syncCache.spec.ts b/src/sdkManager/__tests__/index.syncCache.spec.ts index 3ca1cfa0..391a053c 100644 --- a/src/sdkManager/__tests__/index.syncCache.spec.ts +++ b/src/sdkManager/__tests__/index.syncCache.spec.ts @@ -19,7 +19,7 @@ describe('Manager with sync cache (In Memory)', () => { /** Setup: create manager */ const cache = new SplitsCacheInMemory(); const manager = sdkManagerFactory({ mode: 'standalone', log: loggerMock }, cache, sdkReadinessManagerMock); - cache.addSplit(splitObject.name, splitObject as any); + cache.addSplit(splitObject as any); test('List all splits', () => { diff --git a/src/storages/AbstractSplitsCacheAsync.ts b/src/storages/AbstractSplitsCacheAsync.ts index dcf059ed..a43a57f2 100644 --- a/src/storages/AbstractSplitsCacheAsync.ts +++ b/src/storages/AbstractSplitsCacheAsync.ts @@ -8,12 +8,22 @@ import { objectAssign } from '../utils/lang/objectAssign'; */ export abstract class AbstractSplitsCacheAsync implements ISplitsCacheAsync { - abstract addSplit(name: string, split: ISplit): Promise - abstract addSplits(entries: [string, ISplit][]): Promise - abstract removeSplits(names: string[]): Promise + protected abstract addSplit(split: ISplit): Promise + protected abstract removeSplit(name: string): Promise + protected abstract setChangeNumber(changeNumber: number): Promise + + update(toAdd: ISplit[], toRemove: ISplit[], changeNumber: number): Promise { + return Promise.all([ + this.setChangeNumber(changeNumber), + Promise.all(toAdd.map(addedFF => this.addSplit(addedFF))), + Promise.all(toRemove.map(removedFF => this.removeSplit(removedFF.name))) + ]).then(([, added, removed]) => { + return added.some(result => result) || removed.some(result => result); + }); + } + abstract getSplit(name: string): Promise abstract getSplits(names: string[]): Promise> - abstract setChangeNumber(changeNumber: number): Promise abstract getChangeNumber(): Promise abstract getAll(): Promise abstract getSplitNames(): Promise @@ -52,7 +62,7 @@ export abstract class AbstractSplitsCacheAsync implements ISplitsCacheAsync { newSplit.defaultTreatment = defaultTreatment; newSplit.changeNumber = changeNumber; - return this.addSplit(name, newSplit); + return this.addSplit(newSplit); } return false; }).catch(() => false); diff --git a/src/storages/AbstractSplitsCacheSync.ts b/src/storages/AbstractSplitsCacheSync.ts index f82ebbd6..512d990e 100644 --- a/src/storages/AbstractSplitsCacheSync.ts +++ b/src/storages/AbstractSplitsCacheSync.ts @@ -9,16 +9,14 @@ import { IN_SEGMENT, IN_LARGE_SEGMENT } from '../utils/constants'; */ export abstract class AbstractSplitsCacheSync implements ISplitsCacheSync { - abstract addSplit(name: string, split: ISplit): boolean - - addSplits(entries: [string, ISplit][]): boolean[] { - return entries.map(keyValuePair => this.addSplit(keyValuePair[0], keyValuePair[1])); - } - - abstract removeSplit(name: string): boolean - - removeSplits(names: string[]): boolean[] { - return names.map(name => this.removeSplit(name)); + protected abstract addSplit(split: ISplit): boolean + protected abstract removeSplit(name: string): boolean + protected abstract setChangeNumber(changeNumber: number): boolean | void + + update(toAdd: ISplit[], toRemove: ISplit[], changeNumber: number): boolean { + this.setChangeNumber(changeNumber); + const updated = toAdd.map(addedFF => this.addSplit(addedFF)).some(result => result); + return toRemove.map(removedFF => this.removeSplit(removedFF.name)).some(result => result) || updated; } abstract getSplit(name: string): ISplit | null @@ -31,8 +29,6 @@ export abstract class AbstractSplitsCacheSync implements ISplitsCacheSync { return splits; } - abstract setChangeNumber(changeNumber: number): boolean | void - abstract getChangeNumber(): number getAll(): ISplit[] { @@ -71,7 +67,7 @@ export abstract class AbstractSplitsCacheSync implements ISplitsCacheSync { newSplit.defaultTreatment = defaultTreatment; newSplit.changeNumber = changeNumber; - return this.addSplit(name, newSplit); + return this.addSplit(newSplit); } return false; } diff --git a/src/storages/KeyBuilder.ts b/src/storages/KeyBuilder.ts index e70b251b..2f5dc800 100644 --- a/src/storages/KeyBuilder.ts +++ b/src/storages/KeyBuilder.ts @@ -1,5 +1,4 @@ import { ISettings } from '../types'; -import { startsWith } from '../utils/lang'; import { hash } from '../utils/murmur3/murmur3'; const everythingAtTheEnd = /[^.]+$/; @@ -34,24 +33,10 @@ export class KeyBuilder { return `${this.prefix}.splits.till`; } - // NOT USED - // buildSplitsReady() { - // return `${this.prefix}.splits.ready`; - // } - - isSplitKey(key: string) { - return startsWith(key, `${this.prefix}.split.`); - } - buildSplitKeyPrefix() { return `${this.prefix}.split.`; } - // Only used by InLocalStorage. - buildSplitsWithSegmentCountKey() { - return `${this.prefix}.splits.usingSegments`; - } - buildSegmentNameKey(segmentName: string) { return `${this.prefix}.segment.${segmentName}`; } @@ -60,11 +45,6 @@ export class KeyBuilder { return `${this.prefix}.segment.${segmentName}.till`; } - // NOT USED - // buildSegmentsReady() { - // return `${this.prefix}.segments.ready`; - // } - extractKey(builtKey: string) { const s = builtKey.match(everythingAtTheEnd); diff --git a/src/storages/KeyBuilderCS.ts b/src/storages/KeyBuilderCS.ts index a59d7208..23961f89 100644 --- a/src/storages/KeyBuilderCS.ts +++ b/src/storages/KeyBuilderCS.ts @@ -28,8 +28,7 @@ export class KeyBuilderCS extends KeyBuilder implements MySegmentsKeyBuilder { extractSegmentName(builtSegmentKeyName: string) { const prefix = `${this.prefix}.${this.matchingKey}.segment.`; - if (startsWith(builtSegmentKeyName, prefix)) - return builtSegmentKeyName.substr(prefix.length); + if (startsWith(builtSegmentKeyName, prefix)) return builtSegmentKeyName.slice(prefix.length); } buildLastUpdatedKey() { @@ -43,6 +42,14 @@ export class KeyBuilderCS extends KeyBuilder implements MySegmentsKeyBuilder { buildTillKey() { return `${this.prefix}.${this.matchingKey}.segments.till`; } + + isSplitKey(key: string) { + return startsWith(key, `${this.prefix}.split.`); + } + + buildSplitsWithSegmentCountKey() { + return `${this.prefix}.splits.usingSegments`; + } } export function myLargeSegmentsKeyBuilder(prefix: string, matchingKey: string): MySegmentsKeyBuilder { @@ -54,7 +61,7 @@ export function myLargeSegmentsKeyBuilder(prefix: string, matchingKey: string): extractSegmentName(builtSegmentKeyName: string) { const p = `${prefix}.${matchingKey}.largeSegment.`; - if (startsWith(builtSegmentKeyName, p)) return builtSegmentKeyName.substr(p.length); + if (startsWith(builtSegmentKeyName, p)) return builtSegmentKeyName.slice(p.length); }, buildTillKey() { diff --git a/src/storages/__tests__/KeyBuilder.spec.ts b/src/storages/__tests__/KeyBuilder.spec.ts index e0494ec9..45af194c 100644 --- a/src/storages/__tests__/KeyBuilder.spec.ts +++ b/src/storages/__tests__/KeyBuilder.spec.ts @@ -9,14 +9,9 @@ test('KEYS / splits keys', () => { const expectedKey = `SPLITIO.split.${splitName}`; const expectedTill = 'SPLITIO.splits.till'; - expect(builder.isSplitKey(expectedKey)).toBe(true); - expect(builder.buildSplitKey(splitName) === expectedKey).toBe(true); - expect(builder.buildSplitsTillKey() === expectedTill).toBe(true); - expect(builder.extractKey(builder.buildSplitKey(splitName)) === splitName).toBe(true); - - // NOT USED - // const expectedReady = 'SPLITIO.splits.ready'; - // expect(builder.buildSplitsReady() === expectedReady).toBe(true); + expect(builder.buildSplitKey(splitName)).toBe(expectedKey); + expect(builder.buildSplitsTillKey()).toBe(expectedTill); + expect(builder.extractKey(builder.buildSplitKey(splitName))).toBe(splitName); }); test('KEYS / splits keys with custom prefix', () => { @@ -27,13 +22,8 @@ test('KEYS / splits keys with custom prefix', () => { const expectedKey = `${prefix}.split.${splitName}`; const expectedTill = `${prefix}.splits.till`; - expect(builder.isSplitKey(expectedKey)).toBe(true); expect(builder.buildSplitKey(splitName)).toBe(expectedKey); expect(builder.buildSplitsTillKey() === expectedTill).toBe(true); - - // NOT USED - // const expectedReady = `${prefix}.SPLITIO.splits.ready`; - // expect(builder.buildSplitsReady() === expectedReady).toBe(true); }); const prefix = 'SPLITIO'; diff --git a/src/storages/__tests__/testUtils.ts b/src/storages/__tests__/testUtils.ts index 94e11c36..fa38944f 100644 --- a/src/storages/__tests__/testUtils.ts +++ b/src/storages/__tests__/testUtils.ts @@ -23,9 +23,9 @@ export function assertSyncRecorderCacheInterface(cache: IEventsCacheSync | IImpr // Split mocks //@ts-ignore -export const splitWithUserTT: ISplit = { trafficTypeName: 'user_tt', conditions: [] }; +export const splitWithUserTT: ISplit = { name: 'user_ff', trafficTypeName: 'user_tt', conditions: [] }; //@ts-ignore -export const splitWithAccountTT: ISplit = { trafficTypeName: 'account_tt', conditions: [] }; +export const splitWithAccountTT: ISplit = { name: 'account_ff', trafficTypeName: 'account_tt', conditions: [] }; //@ts-ignore export const splitWithAccountTTAndUsesSegments: ISplit = { trafficTypeName: 'account_tt', conditions: [{ matcherGroup: { matchers: [{ matcherType: 'IN_SEGMENT', userDefinedSegmentMatcherData: { segmentName: 'employees' } }] } }] }; //@ts-ignore diff --git a/src/storages/dataLoader.ts b/src/storages/dataLoader.ts index ce288868..2ab542a9 100644 --- a/src/storages/dataLoader.ts +++ b/src/storages/dataLoader.ts @@ -35,10 +35,9 @@ export function dataLoaderFactory(preloadedData: PreloadedData): DataLoader { // cleaning up the localStorage data, since some cached splits might need be part of the preloaded data storage.splits.clear(); - storage.splits.setChangeNumber(since); // splitsData in an object where the property is the split name and the pertaining value is a stringified json of its data - storage.splits.addSplits(Object.keys(splitsData).map(splitName => JSON.parse(splitsData[splitName]))); + storage.splits.update(Object.keys(splitsData).map(splitName => JSON.parse(splitsData[splitName])), [], since); // add mySegments data let mySegmentsData = preloadedData.mySegmentsData && preloadedData.mySegmentsData[userId]; diff --git a/src/storages/inLocalStorage/SplitsCacheInLocal.ts b/src/storages/inLocalStorage/SplitsCacheInLocal.ts index 93eb6f32..61988139 100644 --- a/src/storages/inLocalStorage/SplitsCacheInLocal.ts +++ b/src/storages/inLocalStorage/SplitsCacheInLocal.ts @@ -96,8 +96,9 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { this.hasSync = false; } - addSplit(name: string, split: ISplit) { + addSplit(split: ISplit) { try { + const name = split.name; const splitKey = this.keys.buildSplitKey(name); const splitFromLocalStorage = localStorage.getItem(splitKey); const previousSplit = splitFromLocalStorage ? JSON.parse(splitFromLocalStorage) : null; @@ -120,6 +121,8 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { removeSplit(name: string): boolean { try { const split = this.getSplit(name); + if (!split) return false; + localStorage.removeItem(this.keys.buildSplitKey(name)); this._decrementCounts(split); @@ -132,7 +135,7 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { } } - getSplit(name: string) { + getSplit(name: string): ISplit | null { const item = localStorage.getItem(this.keys.buildSplitKey(name)); return item && JSON.parse(item); } diff --git a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts index 4d8ec076..db78c741 100644 --- a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts +++ b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts @@ -5,34 +5,33 @@ import { ISplit } from '../../../dtos/types'; import { fullSettings } from '../../../utils/settingsValidation/__tests__/settings.mocks'; -test('SPLIT CACHE / LocalStorage', () => { +test('SPLITS CACHE / LocalStorage', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); cache.clear(); - cache.addSplit('lol1', something); - cache.addSplit('lol2', somethingElse); + cache.update([something, somethingElse], [], -1); let values = cache.getAll(); expect(values).toEqual([something, somethingElse]); - cache.removeSplit('lol1'); + cache.removeSplit(something.name); - const splits = cache.getSplits(['lol1', 'lol2']); - expect(splits['lol1']).toEqual(null); - expect(splits['lol2']).toEqual(somethingElse); + const splits = cache.getSplits([something.name, somethingElse.name]); + expect(splits[something.name]).toEqual(null); + expect(splits[somethingElse.name]).toEqual(somethingElse); values = cache.getAll(); expect(values).toEqual([somethingElse]); - expect(cache.getSplit('lol1')).toEqual(null); - expect(cache.getSplit('lol2')).toEqual(somethingElse); + expect(cache.getSplit(something.name)).toEqual(null); + expect(cache.getSplit(somethingElse.name)).toEqual(somethingElse); expect(cache.checkCache()).toBe(false); // checkCache should return false until localstorage has data. - expect(cache.getChangeNumber() === -1).toBe(true); + expect(cache.getChangeNumber()).toBe(-1); expect(cache.checkCache()).toBe(false); // checkCache should return false until localstorage has data. @@ -40,45 +39,41 @@ test('SPLIT CACHE / LocalStorage', () => { expect(cache.checkCache()).toBe(true); // checkCache should return true once localstorage has data. - expect(cache.getChangeNumber() === 123).toBe(true); + expect(cache.getChangeNumber()).toBe(123); }); -test('SPLIT CACHE / LocalStorage / Get Keys', () => { +test('SPLITS CACHE / LocalStorage / Get Keys', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.addSplit('lol1', something); - cache.addSplit('lol2', somethingElse); + cache.update([something, somethingElse], [], 1); const keys = cache.getSplitNames(); - expect(keys.indexOf('lol1') !== -1).toBe(true); - expect(keys.indexOf('lol2') !== -1).toBe(true); + expect(keys.indexOf(something.name) !== -1).toBe(true); + expect(keys.indexOf(somethingElse.name) !== -1).toBe(true); }); -test('SPLIT CACHE / LocalStorage / Add Splits', () => { +test('SPLITS CACHE / LocalStorage / Update Splits', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.addSplits([ - ['lol1', something], - ['lol2', somethingElse] - ]); + cache.update([something, somethingElse], [], 1); - cache.removeSplits(['lol1', 'lol2']); + cache.update([], [something, somethingElse], 1); - expect(cache.getSplit('lol1') == null).toBe(true); - expect(cache.getSplit('lol2') == null).toBe(true); + expect(cache.getSplit(something.name)).toBe(null); + expect(cache.getSplit(somethingElse.name)).toBe(null); }); -test('SPLIT CACHE / LocalStorage / trafficTypeExists and ttcache tests', () => { +test('SPLITS CACHE / LocalStorage / trafficTypeExists and ttcache tests', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.addSplits([ // loop of addSplit - ['split1', splitWithUserTT], - ['split2', splitWithAccountTT], - ['split3', splitWithUserTT], - ]); - cache.addSplit('split4', splitWithUserTT); + cache.update([ + { ...splitWithUserTT, name: 'split1' }, + { ...splitWithAccountTT, name: 'split2' }, + { ...splitWithUserTT, name: 'split3' }, + ], [], 1); + cache.addSplit({ ...splitWithUserTT, name: 'split4' }); expect(cache.trafficTypeExists('user_tt')).toBe(true); expect(cache.trafficTypeExists('account_tt')).toBe(true); @@ -89,7 +84,8 @@ test('SPLIT CACHE / LocalStorage / trafficTypeExists and ttcache tests', () => { expect(cache.trafficTypeExists('user_tt')).toBe(true); expect(cache.trafficTypeExists('account_tt')).toBe(true); - cache.removeSplits(['split3', 'split2']); // it'll invoke a loop of removeSplit + cache.removeSplit('split3'); + cache.removeSplit('split2'); expect(cache.trafficTypeExists('user_tt')).toBe(true); expect(cache.trafficTypeExists('account_tt')).toBe(false); @@ -99,19 +95,19 @@ test('SPLIT CACHE / LocalStorage / trafficTypeExists and ttcache tests', () => { expect(cache.trafficTypeExists('user_tt')).toBe(false); expect(cache.trafficTypeExists('account_tt')).toBe(false); - cache.addSplit('split1', splitWithUserTT); + cache.addSplit({ ...splitWithUserTT, name: 'split1' }); expect(cache.trafficTypeExists('user_tt')).toBe(true); - cache.addSplit('split1', splitWithAccountTT); + cache.addSplit({ ...splitWithAccountTT, name: 'split1' }); expect(cache.trafficTypeExists('account_tt')).toBe(true); expect(cache.trafficTypeExists('user_tt')).toBe(false); }); -test('SPLIT CACHE / LocalStorage / killLocally', () => { +test('SPLITS CACHE / LocalStorage / killLocally', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.addSplit('lol1', something); - cache.addSplit('lol2', somethingElse); + cache.addSplit(something); + cache.addSplit(somethingElse); const initialChangeNumber = cache.getChangeNumber(); // kill an non-existent split @@ -122,8 +118,8 @@ test('SPLIT CACHE / LocalStorage / killLocally', () => { expect(nonexistentSplit).toBe(null); // non-existent split keeps being non-existent // kill an existent split - updated = cache.killLocally('lol1', 'some_treatment', 100); - let lol1Split = cache.getSplit('lol1') as ISplit; + updated = cache.killLocally(something.name, 'some_treatment', 100); + let lol1Split = cache.getSplit(something.name) as ISplit; expect(updated).toBe(true); // killLocally resolves with update if split is changed expect(lol1Split.killed).toBe(true); // existing split must be killed @@ -132,27 +128,27 @@ test('SPLIT CACHE / LocalStorage / killLocally', () => { expect(cache.getChangeNumber()).toBe(initialChangeNumber); // cache changeNumber is not changed // not update if changeNumber is old - updated = cache.killLocally('lol1', 'some_treatment_2', 90); - lol1Split = cache.getSplit('lol1') as ISplit; + updated = cache.killLocally(something.name, 'some_treatment_2', 90); + lol1Split = cache.getSplit(something.name) as ISplit; expect(updated).toBe(false); // killLocally resolves without update if changeNumber is old expect(lol1Split.defaultTreatment).not.toBe('some_treatment_2'); // existing split is not updated if given changeNumber is older }); -test('SPLIT CACHE / LocalStorage / usesSegments', () => { +test('SPLITS CACHE / LocalStorage / usesSegments', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); expect(cache.usesSegments()).toBe(true); // true initially, until data is synchronized cache.setChangeNumber(1); // to indicate that data has been synced. - cache.addSplits([['split1', splitWithUserTT], ['split2', splitWithAccountTT],]); + cache.update([splitWithUserTT, splitWithAccountTT], [], 1); expect(cache.usesSegments()).toBe(false); // 0 splits using segments - cache.addSplit('split3', splitWithAccountTTAndUsesSegments); + cache.addSplit({ ...splitWithAccountTTAndUsesSegments, name: 'split3' }); expect(cache.usesSegments()).toBe(true); // 1 split using segments - cache.addSplit('split4', splitWithAccountTTAndUsesSegments); + cache.addSplit({ ...splitWithAccountTTAndUsesSegments, name: 'split4' }); expect(cache.usesSegments()).toBe(true); // 2 splits using segments cache.removeSplit('split3'); @@ -162,7 +158,7 @@ test('SPLIT CACHE / LocalStorage / usesSegments', () => { expect(cache.usesSegments()).toBe(false); // 0 splits using segments }); -test('SPLIT CACHE / LocalStorage / flag set cache tests', () => { +test('SPLITS CACHE / LocalStorage / flag set cache tests', () => { // @ts-ignore const cache = new SplitsCacheInLocal({ ...fullSettings, @@ -175,12 +171,12 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests', () => { }, new KeyBuilderCS('SPLITIO', 'user')); const emptySet = new Set([]); - cache.addSplits([ - [featureFlagOne.name, featureFlagOne], - [featureFlagTwo.name, featureFlagTwo], - [featureFlagThree.name, featureFlagThree], - ]); - cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS); + cache.update([ + featureFlagOne, + featureFlagTwo, + featureFlagThree, + ], [], -1); + cache.addSplit(featureFlagWithEmptyFS); expect(cache.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_one', 'ff_two'])]); expect(cache.getNamesByFlagSets(['n'])).toEqual([new Set(['ff_one'])]); @@ -188,13 +184,13 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests', () => { 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, 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]); - cache.addSplit(featureFlagOne.name, { ...featureFlagOne, sets: ['x'] }); + cache.addSplit({ ...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'])]); @@ -206,7 +202,7 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests', () => { expect(cache.getNamesByFlagSets(['y'])).toEqual([emptySet]); // 'y' not in filter expect(cache.getNamesByFlagSets([])).toEqual([]); - cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithoutFS); + cache.addSplit(featureFlagWithoutFS); expect(cache.getNamesByFlagSets([])).toEqual([]); }); @@ -215,12 +211,12 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests without filters', () => const cacheWithoutFilters = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); const emptySet = new Set([]); - cacheWithoutFilters.addSplits([ - [featureFlagOne.name, featureFlagOne], - [featureFlagTwo.name, featureFlagTwo], - [featureFlagThree.name, featureFlagThree], - ]); - cacheWithoutFilters.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS); + cacheWithoutFilters.update([ + featureFlagOne, + featureFlagTwo, + featureFlagThree, + ], [], -1); + cacheWithoutFilters.addSplit(featureFlagWithEmptyFS); expect(cacheWithoutFilters.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_one', 'ff_two'])]); expect(cacheWithoutFilters.getNamesByFlagSets(['n'])).toEqual([new Set(['ff_one'])]); diff --git a/src/storages/inMemory/SplitsCacheInMemory.ts b/src/storages/inMemory/SplitsCacheInMemory.ts index 688b6e24..a8be688a 100644 --- a/src/storages/inMemory/SplitsCacheInMemory.ts +++ b/src/storages/inMemory/SplitsCacheInMemory.ts @@ -26,7 +26,8 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync { this.segmentsCount = 0; } - addSplit(name: string, split: ISplit): boolean { + addSplit(split: ISplit): boolean { + const name = split.name; const previousSplit = this.getSplit(name); if (previousSplit) { // We had this Split already @@ -40,41 +41,35 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync { if (usesSegments(previousSplit)) this.segmentsCount--; } - if (split) { - // Store the Split. - this.splitsCache[name] = split; - // Update TT cache - const ttName = split.trafficTypeName; - this.ttCache[ttName] = (this.ttCache[ttName] || 0) + 1; - this.addToFlagSets(split); + // Store the Split. + this.splitsCache[name] = split; + // Update TT cache + const ttName = split.trafficTypeName; + this.ttCache[ttName] = (this.ttCache[ttName] || 0) + 1; + this.addToFlagSets(split); - // Add to segments count for the new version of the Split - if (usesSegments(split)) this.segmentsCount++; + // Add to segments count for the new version of the Split + if (usesSegments(split)) this.segmentsCount++; - return true; - } else { - return false; - } + return true; } removeSplit(name: string): boolean { const split = this.getSplit(name); - if (split) { - // Delete the Split - delete this.splitsCache[name]; + if (!split) return false; - const ttName = split.trafficTypeName; - this.ttCache[ttName]--; // Update tt cache - if (!this.ttCache[ttName]) delete this.ttCache[ttName]; - this.removeFromFlagSets(split.name, split.sets); + // Delete the Split + delete this.splitsCache[name]; - // Update the segments count. - if (usesSegments(split)) this.segmentsCount--; + const ttName = split.trafficTypeName; + this.ttCache[ttName]--; // Update tt cache + if (!this.ttCache[ttName]) delete this.ttCache[ttName]; + this.removeFromFlagSets(split.name, split.sets); - return true; - } else { - return false; - } + // Update the segments count. + if (usesSegments(split)) this.segmentsCount--; + + return true; } getSplit(name: string): ISplit | null { diff --git a/src/storages/inMemory/__tests__/SplitsCacheInMemory.spec.ts b/src/storages/inMemory/__tests__/SplitsCacheInMemory.spec.ts index 62812586..2f907eca 100644 --- a/src/storages/inMemory/__tests__/SplitsCacheInMemory.spec.ts +++ b/src/storages/inMemory/__tests__/SplitsCacheInMemory.spec.ts @@ -5,54 +5,62 @@ import { splitWithUserTT, splitWithAccountTT, something, somethingElse, featureF test('SPLITS CACHE / In Memory', () => { const cache = new SplitsCacheInMemory(); - cache.addSplit('lol1', something); - cache.addSplit('lol2', somethingElse); + cache.update([something, somethingElse], [], -1); let values = cache.getAll(); - expect(values.indexOf(something) !== -1).toBe(true); - expect(values.indexOf(somethingElse) !== -1).toBe(true); + expect(values).toEqual([something, somethingElse]); - cache.removeSplit('lol1'); + cache.removeSplit(something.name); - const splits = cache.getSplits(['lol1', 'lol2']); - expect(splits['lol1'] === null).toBe(true); - expect(splits['lol2'] === somethingElse).toBe(true); + const splits = cache.getSplits([something.name, somethingElse.name]); + expect(splits[something.name]).toEqual(null); + expect(splits[somethingElse.name]).toEqual(somethingElse); values = cache.getAll(); - expect(values.indexOf(something) === -1).toBe(true); - expect(values.indexOf(somethingElse) !== -1).toBe(true); + expect(values).toEqual([somethingElse]); - expect(cache.getSplit('lol1') == null).toBe(true); - expect(cache.getSplit('lol2') === somethingElse).toBe(true); + expect(cache.getSplit(something.name)).toEqual(null); + expect(cache.getSplit(somethingElse.name)).toEqual(somethingElse); + expect(cache.getChangeNumber()).toBe(-1); cache.setChangeNumber(123); - expect(cache.getChangeNumber() === 123).toBe(true); + expect(cache.getChangeNumber()).toBe(123); }); test('SPLITS CACHE / In Memory / Get Keys', () => { const cache = new SplitsCacheInMemory(); - cache.addSplit('lol1', something); - cache.addSplit('lol2', somethingElse); + cache.update([something, somethingElse], [], 1); - let keys = cache.getSplitNames(); + const keys = cache.getSplitNames(); - expect(keys.indexOf('lol1') !== -1).toBe(true); - expect(keys.indexOf('lol2') !== -1).toBe(true); + expect(keys.indexOf(something.name) !== -1).toBe(true); + expect(keys.indexOf(somethingElse.name) !== -1).toBe(true); +}); + +test('SPLITS CACHE / In Memory / Update Splits', () => { + const cache = new SplitsCacheInMemory(); + + cache.update([something, somethingElse], [], 1); + + cache.update([], [something, somethingElse], 1); + + expect(cache.getSplit(something.name)).toBe(null); + expect(cache.getSplit(somethingElse.name)).toBe(null); }); test('SPLITS CACHE / In Memory / trafficTypeExists and ttcache tests', () => { const cache = new SplitsCacheInMemory(); - cache.addSplits([ // loop of addSplit - ['split1', splitWithUserTT], - ['split2', splitWithAccountTT], - ['split3', splitWithUserTT], - ]); - cache.addSplit('split4', splitWithUserTT); + cache.update([ + { ...splitWithUserTT, name: 'split1' }, + { ...splitWithAccountTT, name: 'split2' }, + { ...splitWithUserTT, name: 'split3' }, + ], [], 1); + cache.addSplit({ ...splitWithUserTT, name: 'split4' }); expect(cache.trafficTypeExists('user_tt')).toBe(true); expect(cache.trafficTypeExists('account_tt')).toBe(true); @@ -63,7 +71,8 @@ test('SPLITS CACHE / In Memory / trafficTypeExists and ttcache tests', () => { expect(cache.trafficTypeExists('user_tt')).toBe(true); expect(cache.trafficTypeExists('account_tt')).toBe(true); - cache.removeSplits(['split3', 'split2']); // it'll invoke a loop of removeSplit + cache.removeSplit('split3'); + cache.removeSplit('split2'); expect(cache.trafficTypeExists('user_tt')).toBe(true); expect(cache.trafficTypeExists('account_tt')).toBe(false); @@ -73,10 +82,10 @@ test('SPLITS CACHE / In Memory / trafficTypeExists and ttcache tests', () => { expect(cache.trafficTypeExists('user_tt')).toBe(false); expect(cache.trafficTypeExists('account_tt')).toBe(false); - cache.addSplit('split1', splitWithUserTT); + cache.addSplit({ ...splitWithUserTT, name: 'split1' }); expect(cache.trafficTypeExists('user_tt')).toBe(true); - cache.addSplit('split1', splitWithAccountTT); + cache.addSplit({ ...splitWithAccountTT, name: 'split1' }); expect(cache.trafficTypeExists('account_tt')).toBe(true); expect(cache.trafficTypeExists('user_tt')).toBe(false); @@ -84,30 +93,30 @@ test('SPLITS CACHE / In Memory / trafficTypeExists and ttcache tests', () => { test('SPLITS CACHE / In Memory / killLocally', () => { const cache = new SplitsCacheInMemory(); - cache.addSplit('lol1', something); - cache.addSplit('lol2', somethingElse); + cache.addSplit(something); + cache.addSplit(somethingElse); const initialChangeNumber = cache.getChangeNumber(); // kill an non-existent split let updated = cache.killLocally('nonexistent_split', 'other_treatment', 101); const nonexistentSplit = cache.getSplit('nonexistent_split'); - expect(updated).toBe(false); // killLocally resolves without update if split doesn\'t exist + expect(updated).toBe(false); // killLocally resolves without update if split doesn't exist expect(nonexistentSplit).toBe(null); // non-existent split keeps being non-existent // kill an existent split - updated = cache.killLocally('lol1', 'some_treatment', 100); - let lol1Split = cache.getSplit('lol1') as ISplit; + updated = cache.killLocally(something.name, 'some_treatment', 100); + let lol1Split = cache.getSplit(something.name) as ISplit; expect(updated).toBe(true); // killLocally resolves with update if split is changed expect(lol1Split.killed).toBe(true); // existing split must be killed - expect(lol1Split.defaultTreatment).toBe('some_treatment'); // existing split must have the given default treatment + expect(lol1Split.defaultTreatment).toBe('some_treatment'); // existing split must have new default treatment expect(lol1Split.changeNumber).toBe(100); // existing split must have the given change number expect(cache.getChangeNumber()).toBe(initialChangeNumber); // cache changeNumber is not changed // not update if changeNumber is old - updated = cache.killLocally('lol1', 'some_treatment_2', 90); - lol1Split = cache.getSplit('lol1') as ISplit; + updated = cache.killLocally(something.name, 'some_treatment_2', 90); + lol1Split = cache.getSplit(something.name) as ISplit; expect(updated).toBe(false); // killLocally resolves without update if changeNumber is old expect(lol1Split.defaultTreatment).not.toBe('some_treatment_2'); // existing split is not updated if given changeNumber is older @@ -119,12 +128,12 @@ test('SPLITS CACHE / In Memory / flag set cache tests', () => { const cache = new SplitsCacheInMemory({ groupedFilters: { bySet: ['o', 'n', 'e', 'x'] } }); const emptySet = new Set([]); - cache.addSplits([ - [featureFlagOne.name, featureFlagOne], - [featureFlagTwo.name, featureFlagTwo], - [featureFlagThree.name, featureFlagThree], - ]); - cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS); + cache.update([ + featureFlagOne, + featureFlagTwo, + featureFlagThree, + ], [], -1); + cache.addSplit(featureFlagWithEmptyFS); expect(cache.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_one', 'ff_two'])]); expect(cache.getNamesByFlagSets(['n'])).toEqual([new Set(['ff_one'])]); @@ -132,13 +141,13 @@ test('SPLITS CACHE / In Memory / flag set cache tests', () => { 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, 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]); - cache.addSplit(featureFlagOne.name, { ...featureFlagOne, sets: ['x'] }); + cache.addSplit({ ...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'])]); @@ -150,21 +159,21 @@ test('SPLITS CACHE / In Memory / flag set cache tests', () => { expect(cache.getNamesByFlagSets(['y'])).toEqual([emptySet]); // 'y' not in filter expect(cache.getNamesByFlagSets([])).toEqual([]); - cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithoutFS); + cache.addSplit(featureFlagWithoutFS); expect(cache.getNamesByFlagSets([])).toEqual([]); }); // if FlagSets are not defined, it should store all FlagSets in memory. -test('SPLIT CACHE / LocalStorage / flag set cache tests without filters', () => { +test('SPLITS CACHE / In Memory / flag set cache tests without filters', () => { const cacheWithoutFilters = new SplitsCacheInMemory(); const emptySet = new Set([]); - cacheWithoutFilters.addSplits([ - [featureFlagOne.name, featureFlagOne], - [featureFlagTwo.name, featureFlagTwo], - [featureFlagThree.name, featureFlagThree], - ]); - cacheWithoutFilters.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS); + cacheWithoutFilters.update([ + featureFlagOne, + featureFlagTwo, + featureFlagThree, + ], [], -1); + cacheWithoutFilters.addSplit(featureFlagWithEmptyFS); expect(cacheWithoutFilters.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_one', 'ff_two'])]); expect(cacheWithoutFilters.getNamesByFlagSets(['n'])).toEqual([new Set(['ff_one'])]); diff --git a/src/storages/inRedis/SplitsCacheInRedis.ts b/src/storages/inRedis/SplitsCacheInRedis.ts index 7ae68e64..ec360b2f 100644 --- a/src/storages/inRedis/SplitsCacheInRedis.ts +++ b/src/storages/inRedis/SplitsCacheInRedis.ts @@ -82,7 +82,8 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync { * The returned promise is resolved when the operation success * or rejected if it fails (e.g., redis operation fails) */ - addSplit(name: string, split: ISplit): Promise { + addSplit(split: ISplit): Promise { + const name = split.name; const splitKey = this.keys.buildSplitKey(name); return this.redis.get(splitKey).then(splitFromStorage => { @@ -107,18 +108,9 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync { }).then(() => true); } - /** - * Add a list of splits. - * The returned promise is resolved when the operation success - * or rejected if it fails (e.g., redis operation fails) - */ - addSplits(entries: [string, ISplit][]): Promise { - return Promise.all(entries.map(keyValuePair => this.addSplit(keyValuePair[0], keyValuePair[1]))); - } - /** * Remove a given split. - * The returned promise is resolved when the operation success, with 1 or 0 indicating if the split existed or not. + * The returned promise is resolved when the operation success, with true or false indicating if the split existed (and was removed) or not. * or rejected if it fails (e.g., redis operation fails). */ removeSplit(name: string) { @@ -127,19 +119,10 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync { return this._decrementCounts(split).then(() => this._updateFlagSets(name, split.sets)); } }).then(() => { - return this.redis.del(this.keys.buildSplitKey(name)); + return this.redis.del(this.keys.buildSplitKey(name)).then(status => status === 1); }); } - /** - * Remove a list of splits. - * The returned promise is resolved when the operation success, - * or rejected if it fails (e.g., redis operation fails). - */ - removeSplits(names: string[]): Promise { - return Promise.all(names.map(name => this.removeSplit(name))); - } - /** * Get split definition or null if it's not defined. * Returned promise is rejected if redis operation fails. diff --git a/src/storages/inRedis/__tests__/SplitsCacheInRedis.spec.ts b/src/storages/inRedis/__tests__/SplitsCacheInRedis.spec.ts index 3f577254..0cbc8914 100644 --- a/src/storages/inRedis/__tests__/SplitsCacheInRedis.spec.ts +++ b/src/storages/inRedis/__tests__/SplitsCacheInRedis.spec.ts @@ -11,14 +11,11 @@ const keysBuilder = new KeyBuilderSS(prefix, metadata); describe('SPLITS CACHE REDIS', () => { - test('add/remove/get splits & set/get change number', async () => { + test('add/remove/get splits', async () => { const connection = new RedisAdapter(loggerMock); const cache = new SplitsCacheInRedis(loggerMock, keysBuilder, connection); - await cache.addSplits([ - ['lol1', splitWithUserTT], - ['lol2', splitWithAccountTT] - ]); + await cache.update([splitWithUserTT, splitWithAccountTT], [], -1); let values = await cache.getAll(); @@ -27,33 +24,34 @@ describe('SPLITS CACHE REDIS', () => { let splitNames = await cache.getSplitNames(); - expect(splitNames.indexOf('lol1') !== -1).toBe(true); - expect(splitNames.indexOf('lol2') !== -1).toBe(true); + expect(splitNames.length).toBe(2); + expect(splitNames.indexOf('user_ff') !== -1).toBe(true); + expect(splitNames.indexOf('account_ff') !== -1).toBe(true); - await cache.removeSplit('lol1'); + await cache.removeSplit('user_ff'); values = await cache.getAll(); expect(values).toEqual([splitWithAccountTT]); - expect(await cache.getSplit('lol1')).toEqual(null); - expect(await cache.getSplit('lol2')).toEqual(splitWithAccountTT); + expect(await cache.getSplit('user_ff')).toEqual(null); + expect(await cache.getSplit('account_ff')).toEqual(splitWithAccountTT); await cache.setChangeNumber(123); - expect(await cache.getChangeNumber() === 123).toBe(true); + expect(await cache.getChangeNumber()).toBe(123); splitNames = await cache.getSplitNames(); - expect(splitNames.indexOf('lol1') === -1).toBe(true); - expect(splitNames.indexOf('lol2') !== -1).toBe(true); + expect(splitNames.indexOf('user_ff') === -1).toBe(true); + expect(splitNames.indexOf('account_ff') !== -1).toBe(true); - const splits = await cache.getSplits(['lol1', 'lol2']); - expect(splits['lol1']).toEqual(null); - expect(splits['lol2']).toEqual(splitWithAccountTT); + const splits = await cache.getSplits(['user_ff', 'account_ff']); + expect(splits['user_ff']).toEqual(null); + expect(splits['account_ff']).toEqual(splitWithAccountTT); // Teardown. @TODO use cache clear method when implemented await connection.del(keysBuilder.buildTrafficTypeKey('account_tt')); - await connection.del(keysBuilder.buildSplitKey('lol2')); + await connection.del(keysBuilder.buildSplitKey('account_ff')); await connection.del(keysBuilder.buildSplitsTillKey()); await connection.disconnect(); }); @@ -62,13 +60,13 @@ describe('SPLITS CACHE REDIS', () => { const connection = new RedisAdapter(loggerMock); const cache = new SplitsCacheInRedis(loggerMock, keysBuilder, connection); - await cache.addSplits([ - ['split1', splitWithUserTT], - ['split2', splitWithAccountTT], - ['split3', splitWithUserTT], - ]); - await cache.addSplit('split4', splitWithUserTT); - await cache.addSplit('split4', splitWithUserTT); // trying to add the same definition for an already added split will not have effect + await cache.update([ + { ...splitWithUserTT, name: 'split1' }, + { ...splitWithAccountTT, name: 'split2' }, + { ...splitWithUserTT, name: 'split3' }, + ], [], -1); + await cache.addSplit({ ...splitWithUserTT, name: 'split4' }); + await cache.addSplit({ ...splitWithUserTT, name: 'split4' }); // trying to add the same definition for an already added split will not have effect expect(await cache.trafficTypeExists('user_tt')).toBe(true); expect(await cache.trafficTypeExists('account_tt')).toBe(true); @@ -81,7 +79,8 @@ describe('SPLITS CACHE REDIS', () => { expect(await connection.get(keysBuilder.buildTrafficTypeKey('account_tt'))).toBe('1'); - await cache.removeSplits(['split3', 'split2']); // it'll invoke a loop of removeSplit + await cache.removeSplit('split3'); + await cache.removeSplit('split2'); expect(await cache.trafficTypeExists('user_tt')).toBe(true); expect(await cache.trafficTypeExists('account_tt')).toBe(false); @@ -93,10 +92,10 @@ describe('SPLITS CACHE REDIS', () => { expect(await cache.trafficTypeExists('user_tt')).toBe(false); expect(await cache.trafficTypeExists('account_tt')).toBe(false); - await cache.addSplit('split1', splitWithUserTT); + await cache.addSplit({ ...splitWithUserTT, name: 'split1' }); expect(await cache.trafficTypeExists('user_tt')).toBe(true); - await cache.addSplit('split1', splitWithAccountTT); + await cache.addSplit({ ...splitWithAccountTT, name: 'split1' }); expect(await cache.trafficTypeExists('account_tt')).toBe(true); expect(await cache.trafficTypeExists('user_tt')).toBe(false); @@ -111,8 +110,7 @@ describe('SPLITS CACHE REDIS', () => { const connection = new RedisAdapter(loggerMock); const cache = new SplitsCacheInRedis(loggerMock, keysBuilder, connection); - await cache.addSplit('lol1', splitWithUserTT); - await cache.addSplit('lol2', splitWithAccountTT); + await cache.update([splitWithUserTT, splitWithAccountTT], [], -1); const initialChangeNumber = await cache.getChangeNumber(); // kill an non-existent split @@ -123,8 +121,8 @@ describe('SPLITS CACHE REDIS', () => { expect(nonexistentSplit).toBe(null); // non-existent split keeps being non-existent // kill an existent split - updated = await cache.killLocally('lol1', 'some_treatment', 100); - let lol1Split = await cache.getSplit('lol1') as ISplit; + updated = await cache.killLocally('user_ff', 'some_treatment', 100); + let lol1Split = await cache.getSplit('user_ff') as ISplit; expect(updated).toBe(true); // killLocally resolves with update if split is changed expect(lol1Split.killed).toBe(true); // existing split must be killed @@ -133,14 +131,15 @@ describe('SPLITS CACHE REDIS', () => { expect(await cache.getChangeNumber()).toBe(initialChangeNumber); // cache changeNumber is not changed // not update if changeNumber is old - updated = await cache.killLocally('lol1', 'some_treatment_2', 90); - lol1Split = await cache.getSplit('lol1') as ISplit; + updated = await cache.killLocally('user_ff', 'some_treatment_2', 90); + lol1Split = await cache.getSplit('user_ff') as ISplit; expect(updated).toBe(false); // killLocally resolves without update if changeNumber is old expect(lol1Split.defaultTreatment).not.toBe('some_treatment_2'); // existing split is not updated if given changeNumber is older // Delete splits and TT keys - await cache.removeSplits(['lol1', 'lol2']); + await cache.update([], [splitWithUserTT, splitWithAccountTT], -1); + await connection.del(keysBuilder.buildSplitsTillKey()); expect(await connection.keys(`${prefix}*`)).toHaveLength(0); await connection.disconnect(); }); @@ -151,12 +150,12 @@ describe('SPLITS CACHE REDIS', () => { const emptySet = new Set([]); - await cache.addSplits([ - [featureFlagOne.name, featureFlagOne], - [featureFlagTwo.name, featureFlagTwo], - [featureFlagThree.name, featureFlagThree], - ]); - await cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS); + await cache.update([ + featureFlagOne, + featureFlagTwo, + featureFlagThree, + ], [], -1); + await cache.addSplit(featureFlagWithEmptyFS); expect(await cache.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_one', 'ff_two'])]); expect(await cache.getNamesByFlagSets(['n'])).toEqual([new Set(['ff_one'])]); @@ -164,13 +163,13 @@ describe('SPLITS CACHE REDIS', () => { 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'] }); + await cache.addSplit({ ...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'] }); + await cache.addSplit({ ...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'])]); @@ -188,11 +187,12 @@ describe('SPLITS CACHE REDIS', () => { expect(await cache.getNamesByFlagSets(['y'])).toEqual([emptySet]); // 'y' not in filter expect(await cache.getNamesByFlagSets([])).toEqual([]); - await cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithoutFS); + await cache.addSplit({ ...featureFlagWithoutFS, name: featureFlagWithEmptyFS.name }); expect(await cache.getNamesByFlagSets([])).toEqual([]); // Delete splits, TT and flag set keys - await cache.removeSplits([featureFlagThree.name, featureFlagTwo.name, featureFlagWithEmptyFS.name]); + await cache.update([], [featureFlagThree, featureFlagTwo, featureFlagWithEmptyFS], -1); + await connection.del(keysBuilder.buildSplitsTillKey()); expect(await connection.keys(`${prefix}*`)).toHaveLength(0); await connection.disconnect(); }); @@ -204,12 +204,12 @@ describe('SPLITS CACHE REDIS', () => { const emptySet = new Set([]); - await cacheWithoutFilters.addSplits([ - [featureFlagOne.name, featureFlagOne], - [featureFlagTwo.name, featureFlagTwo], - [featureFlagThree.name, featureFlagThree], - ]); - await cacheWithoutFilters.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS); + await cacheWithoutFilters.update([ + featureFlagOne, + featureFlagTwo, + featureFlagThree + ], [], -1); + await cacheWithoutFilters.addSplit(featureFlagWithEmptyFS); expect(await cacheWithoutFilters.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_one', 'ff_two'])]); expect(await cacheWithoutFilters.getNamesByFlagSets(['n'])).toEqual([new Set(['ff_one'])]); @@ -219,7 +219,8 @@ describe('SPLITS CACHE REDIS', () => { 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]); + await cacheWithoutFilters.update([], [featureFlagThree, featureFlagTwo, featureFlagOne, featureFlagWithEmptyFS], -1); + await connection.del(keysBuilder.buildSplitsTillKey()); expect(await connection.keys(`${prefix}*`)).toHaveLength(0); await connection.disconnect(); }); diff --git a/src/storages/pluggable/SplitsCachePluggable.ts b/src/storages/pluggable/SplitsCachePluggable.ts index ddb06149..9b53f3a9 100644 --- a/src/storages/pluggable/SplitsCachePluggable.ts +++ b/src/storages/pluggable/SplitsCachePluggable.ts @@ -66,7 +66,8 @@ export class SplitsCachePluggable extends AbstractSplitsCacheAsync { * The returned promise is resolved when the operation success * or rejected if it fails (e.g., wrapper operation fails) */ - addSplit(name: string, split: ISplit): Promise { + addSplit(split: ISplit): Promise { + const name = split.name; const splitKey = this.keys.buildSplitKey(name); return this.wrapper.get(splitKey).then(splitFromStorage => { @@ -91,15 +92,6 @@ export class SplitsCachePluggable extends AbstractSplitsCacheAsync { }).then(() => true); } - /** - * Add a list of splits. - * The returned promise is resolved when the operation success - * or rejected if it fails (e.g., wrapper operation fails) - */ - addSplits(entries: [string, ISplit][]): Promise { - return Promise.all(entries.map(keyValuePair => this.addSplit(keyValuePair[0], keyValuePair[1]))); - } - /** * Remove a given split. * The returned promise is resolved when the operation success, with a boolean indicating if the split existed or not. @@ -115,15 +107,6 @@ export class SplitsCachePluggable extends AbstractSplitsCacheAsync { }); } - /** - * Remove a list of splits. - * The returned promise is resolved when the operation success, with a boolean array indicating if the splits existed or not. - * or rejected if it fails (e.g., wrapper operation fails). - */ - removeSplits(names: string[]): Promise { // @ts-ignore - return Promise.all(names.map(name => this.removeSplit(name))); - } - /** * Get split. * The returned promise is resolved with the split definition or null if it's not defined, diff --git a/src/storages/pluggable/__tests__/SplitsCachePluggable.spec.ts b/src/storages/pluggable/__tests__/SplitsCachePluggable.spec.ts index 57fc34b3..03d1ee6e 100644 --- a/src/storages/pluggable/__tests__/SplitsCachePluggable.spec.ts +++ b/src/storages/pluggable/__tests__/SplitsCachePluggable.spec.ts @@ -12,74 +12,59 @@ describe('SPLITS CACHE PLUGGABLE', () => { test('add/remove/get splits', async () => { const cache = new SplitsCachePluggable(loggerMock, keysBuilder, wrapperMockFactory()); - // Assert addSplit and addSplits - await cache.addSplits([ - ['lol1', splitWithUserTT], - ['lol2', splitWithAccountTT] - ]); - await cache.addSplit('lol3', splitWithAccountTT); - - // Assert getAll + await cache.update([splitWithUserTT, splitWithAccountTT], [], -1); + let values = await cache.getAll(); - expect(values).toEqual([splitWithUserTT, splitWithAccountTT, splitWithAccountTT]); + expect(values).toEqual([splitWithUserTT, splitWithAccountTT]); // Assert getSplits - let valuesObj = await cache.getSplits(['lol2', 'lol3']); - - expect(Object.keys(valuesObj).length).toBe(2); - expect(valuesObj.lol2).toEqual(splitWithAccountTT); - expect(valuesObj.lol3).toEqual(splitWithAccountTT); + let valuesObj = await cache.getSplits([splitWithUserTT.name, splitWithAccountTT.name]); + expect(valuesObj).toEqual(values.reduce>((acc, split) => { + acc[split.name] = split; + return acc; + }, {})); // Assert getSplitNames let splitNames = await cache.getSplitNames(); - expect(splitNames.length).toBe(3); - expect(splitNames.indexOf('lol1') !== -1).toBe(true); - expect(splitNames.indexOf('lol2') !== -1).toBe(true); - expect(splitNames.indexOf('lol3') !== -1).toBe(true); - - // Assert removeSplit - await cache.removeSplit('lol1'); + expect(splitNames.length).toBe(2); + expect(splitNames.indexOf('user_ff') !== -1).toBe(true); + expect(splitNames.indexOf('account_ff') !== -1).toBe(true); - values = await cache.getAll(); - expect(values.length).toBe(2); - expect(await cache.getSplit('lol1')).toEqual(null); - expect(await cache.getSplit('lol2')).toEqual(splitWithAccountTT); - - // Assert removeSplits - await cache.addSplit('lol1', splitWithUserTT); - await cache.removeSplits(['lol1', 'lol3']); + await cache.removeSplit('user_ff'); values = await cache.getAll(); - expect(values.length).toBe(1); - splitNames = await cache.getSplitNames(); - expect(splitNames.length).toBe(1); - expect(await cache.getSplit('lol1')).toEqual(null); - expect(await cache.getSplit('lol2')).toEqual(splitWithAccountTT); - }); + expect(values).toEqual([splitWithAccountTT]); - test('set/get change number', async () => { - const cache = new SplitsCachePluggable(loggerMock, keysBuilder, wrapperMockFactory()); + expect(await cache.getSplit('user_ff')).toEqual(null); + expect(await cache.getSplit('account_ff')).toEqual(splitWithAccountTT); - expect(await cache.getChangeNumber()).toBe(-1); // if not set yet, changeNumber is -1 await cache.setChangeNumber(123); expect(await cache.getChangeNumber()).toBe(123); + splitNames = await cache.getSplitNames(); + + expect(splitNames.indexOf('user_ff') === -1).toBe(true); + expect(splitNames.indexOf('account_ff') !== -1).toBe(true); + + const splits = await cache.getSplits(['user_ff', 'account_ff']); + expect(splits['user_ff']).toEqual(null); + expect(splits['account_ff']).toEqual(splitWithAccountTT); }); test('trafficTypeExists', async () => { const wrapper = wrapperMockFactory(); const cache = new SplitsCachePluggable(loggerMock, keysBuilder, wrapper); - await cache.addSplits([ - ['split1', splitWithUserTT], - ['split2', splitWithAccountTT], - ['split3', splitWithUserTT], - ]); - await cache.addSplit('split4', splitWithUserTT); - await cache.addSplit('split4', splitWithUserTT); // trying to add the same definition for an already added split will not have effect + await cache.update([ + { ...splitWithUserTT, name: 'split1' }, + { ...splitWithAccountTT, name: 'split2' }, + { ...splitWithUserTT, name: 'split3' }, + ], [], -1); + await cache.addSplit({ ...splitWithUserTT, name: 'split4' }); + await cache.addSplit({ ...splitWithUserTT, name: 'split4' }); // trying to add the same definition for an already added split will not have effect expect(await cache.trafficTypeExists('user_tt')).toBe(true); expect(await cache.trafficTypeExists('account_tt')).toBe(true); @@ -92,7 +77,8 @@ describe('SPLITS CACHE PLUGGABLE', () => { expect(await wrapper.get(keysBuilder.buildTrafficTypeKey('account_tt'))).toBe('1'); - await cache.removeSplits(['split3', 'split2']); // it'll invoke a loop of removeSplit + await cache.removeSplit('split3'); + await cache.removeSplit('split2'); expect(await cache.trafficTypeExists('user_tt')).toBe(true); expect(await cache.trafficTypeExists('account_tt')).toBe(false); @@ -104,21 +90,19 @@ describe('SPLITS CACHE PLUGGABLE', () => { expect(await cache.trafficTypeExists('user_tt')).toBe(false); expect(await cache.trafficTypeExists('account_tt')).toBe(false); - await cache.addSplit('split1', splitWithUserTT); + await cache.addSplit({ ...splitWithUserTT, name: 'split1' }); expect(await cache.trafficTypeExists('user_tt')).toBe(true); - await cache.addSplit('split1', splitWithAccountTT); + await cache.addSplit({ ...splitWithAccountTT, name: 'split1' }); expect(await cache.trafficTypeExists('account_tt')).toBe(true); expect(await cache.trafficTypeExists('user_tt')).toBe(false); - }); test('killLocally', async () => { const wrapper = wrapperMockFactory(); const cache = new SplitsCachePluggable(loggerMock, keysBuilder, wrapper); - await cache.addSplit('lol1', splitWithUserTT); - await cache.addSplit('lol2', splitWithAccountTT); + await cache.update([splitWithUserTT, splitWithAccountTT], [], -1); const initialChangeNumber = await cache.getChangeNumber(); // kill an non-existent split @@ -129,8 +113,8 @@ describe('SPLITS CACHE PLUGGABLE', () => { expect(nonexistentSplit).toBe(null); // non-existent split keeps being non-existent // kill an existent split - updated = await cache.killLocally('lol1', 'some_treatment', 100); - let lol1Split = await cache.getSplit('lol1') as ISplit; + updated = await cache.killLocally('user_ff', 'some_treatment', 100); + let lol1Split = await cache.getSplit('user_ff') as ISplit; expect(updated).toBe(true); // killLocally resolves with update if split is changed expect(lol1Split.killed).toBe(true); // existing split must be killed @@ -139,14 +123,15 @@ describe('SPLITS CACHE PLUGGABLE', () => { expect(await cache.getChangeNumber()).toBe(initialChangeNumber); // cache changeNumber is not changed // not update if changeNumber is old - updated = await cache.killLocally('lol1', 'some_treatment_2', 90); - lol1Split = await cache.getSplit('lol1') as ISplit; + updated = await cache.killLocally('user_ff', 'some_treatment_2', 90); + lol1Split = await cache.getSplit('user_ff') as ISplit; expect(updated).toBe(false); // killLocally resolves without update if changeNumber is old expect(lol1Split.defaultTreatment).not.toBe('some_treatment_2'); // existing split is not updated if given changeNumber is older // Delete splits and TT keys - await cache.removeSplits(['lol1', 'lol2']); + await cache.update([], [splitWithUserTT, splitWithAccountTT], -1); + await wrapper.del(keysBuilder.buildSplitsTillKey()); expect(await wrapper.getKeysByPrefix('SPLITIO')).toHaveLength(0); }); @@ -155,12 +140,12 @@ describe('SPLITS CACHE PLUGGABLE', () => { 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); + await cache.update([ + featureFlagOne, + featureFlagTwo, + featureFlagThree, + ], [], -1); + await cache.addSplit(featureFlagWithEmptyFS); expect(await cache.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_one', 'ff_two'])]); expect(await cache.getNamesByFlagSets(['n'])).toEqual([new Set(['ff_one'])]); @@ -168,13 +153,13 @@ describe('SPLITS CACHE PLUGGABLE', () => { 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'] }); + await cache.addSplit({ ...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'] }); + await cache.addSplit({ ...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'])]); @@ -189,7 +174,7 @@ describe('SPLITS CACHE PLUGGABLE', () => { expect(await cache.getNamesByFlagSets(['y'])).toEqual([emptySet]); // 'y' not in filter expect(await cache.getNamesByFlagSets([])).toEqual([]); - await cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithoutFS); + await cache.addSplit({ ...featureFlagWithoutFS, name: featureFlagWithEmptyFS.name }); expect(await cache.getNamesByFlagSets([])).toEqual([]); }); @@ -198,12 +183,12 @@ describe('SPLITS CACHE PLUGGABLE', () => { 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); + await cacheWithoutFilters.update([ + featureFlagOne, + featureFlagTwo, + featureFlagThree + ], [], -1); + await cacheWithoutFilters.addSplit(featureFlagWithEmptyFS); expect(await cacheWithoutFilters.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_one', 'ff_two'])]); expect(await cacheWithoutFilters.getNamesByFlagSets(['n'])).toEqual([new Set(['ff_one'])]); diff --git a/src/storages/types.ts b/src/storages/types.ts index 638c4606..71962254 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -177,11 +177,9 @@ export interface IPluggableStorageWrapper { /** Splits cache */ export interface ISplitsCacheBase { - addSplits(entries: [string, ISplit][]): MaybeThenable, - removeSplits(names: string[]): MaybeThenable, + update(toAdd: ISplit[], toRemove: ISplit[], changeNumber: number): MaybeThenable, getSplit(name: string): MaybeThenable, getSplits(names: string[]): MaybeThenable>, // `fetchMany` in spec - setChangeNumber(changeNumber: number): MaybeThenable, // should never reject or throw an exception. Instead return -1 by default, assuming no splits are present in the storage. getChangeNumber(): MaybeThenable, getAll(): MaybeThenable, @@ -198,11 +196,9 @@ export interface ISplitsCacheBase { } export interface ISplitsCacheSync extends ISplitsCacheBase { - addSplits(entries: [string, ISplit][]): boolean[], - removeSplits(names: string[]): boolean[], + update(toAdd: ISplit[], toRemove: ISplit[], changeNumber: number): boolean, getSplit(name: string): ISplit | null, getSplits(names: string[]): Record, - setChangeNumber(changeNumber: number): boolean | void, getChangeNumber(): number, getAll(): ISplit[], getSplitNames(): string[], @@ -215,11 +211,9 @@ export interface ISplitsCacheSync extends ISplitsCacheBase { } export interface ISplitsCacheAsync extends ISplitsCacheBase { - addSplits(entries: [string, ISplit][]): Promise, - removeSplits(names: string[]): Promise, + update(toAdd: ISplit[], toRemove: ISplit[], changeNumber: number): Promise, getSplit(name: string): Promise, getSplits(names: string[]): Promise>, - setChangeNumber(changeNumber: number): Promise, getChangeNumber(): Promise, getAll(): Promise, getSplitNames(): Promise, @@ -428,13 +422,13 @@ export interface ITelemetryCacheAsync extends ITelemetryEvaluationProducerAsync, */ export interface IStorageBase< - TSplitsCache extends ISplitsCacheBase, - TSegmentsCache extends ISegmentsCacheBase, - TImpressionsCache extends IImpressionsCacheBase, - TImpressionsCountCache extends IImpressionCountsCacheBase, - TEventsCache extends IEventsCacheBase, - TTelemetryCache extends ITelemetryCacheSync | ITelemetryCacheAsync, - TUniqueKeysCache extends IUniqueKeysCacheBase + TSplitsCache extends ISplitsCacheBase = ISplitsCacheBase, + TSegmentsCache extends ISegmentsCacheBase = ISegmentsCacheBase, + TImpressionsCache extends IImpressionsCacheBase = IImpressionsCacheBase, + TImpressionsCountCache extends IImpressionCountsCacheBase = IImpressionCountsCacheBase, + TEventsCache extends IEventsCacheBase = IEventsCacheBase, + TTelemetryCache extends ITelemetryCacheSync | ITelemetryCacheAsync = ITelemetryCacheSync | ITelemetryCacheAsync, + TUniqueKeysCache extends IUniqueKeysCacheBase = IUniqueKeysCacheBase > { splits: TSplitsCache, segments: TSegmentsCache, diff --git a/src/sync/offline/syncTasks/fromObjectSyncTask.ts b/src/sync/offline/syncTasks/fromObjectSyncTask.ts index 84805110..14daccf2 100644 --- a/src/sync/offline/syncTasks/fromObjectSyncTask.ts +++ b/src/sync/offline/syncTasks/fromObjectSyncTask.ts @@ -24,7 +24,7 @@ export function fromObjectUpdaterFactory( let startingUp = true; return function objectUpdater() { - const splits: [string, ISplit][] = []; + const splits: ISplit[] = []; let loadError = null; let splitsMock: false | Record = {}; try { @@ -37,24 +37,23 @@ export function fromObjectUpdaterFactory( if (!loadError && splitsMock) { log.debug(SYNC_OFFLINE_DATA, [JSON.stringify(splitsMock)]); - forOwn(splitsMock, function (val, name) { - splits.push([ // @ts-ignore Split changeNumber and seed is undefined in localhost mode - name, { - name, - status: 'ACTIVE', - killed: false, - trafficAllocation: 100, - defaultTreatment: CONTROL, - conditions: val.conditions || [], - configurations: val.configurations, - trafficTypeName: val.trafficTypeName - } - ]); + forOwn(splitsMock, (val, name) => { + // @ts-ignore Split changeNumber and seed is undefined in localhost mode + splits.push({ + name, + status: 'ACTIVE', + killed: false, + trafficAllocation: 100, + defaultTreatment: CONTROL, + conditions: val.conditions || [], + configurations: val.configurations, + trafficTypeName: val.trafficTypeName + }); }); return Promise.all([ splitsCache.clear(), // required to sync removed splits from mock - splitsCache.addSplits(splits) + splitsCache.update(splits, [], Date.now()) ]).then(() => { readiness.splits.emit(SDK_SPLITS_ARRIVED); diff --git a/src/sync/polling/syncTasks/splitsSyncTask.ts b/src/sync/polling/syncTasks/splitsSyncTask.ts index baef383c..d6fed5a2 100644 --- a/src/sync/polling/syncTasks/splitsSyncTask.ts +++ b/src/sync/polling/syncTasks/splitsSyncTask.ts @@ -22,8 +22,7 @@ export function splitsSyncTaskFactory( splitChangesUpdaterFactory( settings.log, splitChangesFetcherFactory(fetchSplitChanges), - storage.splits, - storage.segments, + storage, settings.sync.__splitFiltersValidation, readiness.splits, settings.startup.requestTimeoutBeforeReady, diff --git a/src/sync/polling/updaters/__tests__/splitChangesUpdater.spec.ts b/src/sync/polling/updaters/__tests__/splitChangesUpdater.spec.ts index b4dca3fe..d59c7013 100644 --- a/src/sync/polling/updaters/__tests__/splitChangesUpdater.spec.ts +++ b/src/sync/polling/updaters/__tests__/splitChangesUpdater.spec.ts @@ -96,59 +96,59 @@ test('splitChangesUpdater / compute splits mutation', () => { let splitsMutation = computeSplitsMutation([activeSplitWithSegments, archivedSplit] as ISplit[], splitFiltersValidation); - expect(splitsMutation.added).toEqual([[activeSplitWithSegments.name, activeSplitWithSegments]]); - expect(splitsMutation.removed).toEqual([archivedSplit.name]); + expect(splitsMutation.added).toEqual([activeSplitWithSegments]); + expect(splitsMutation.removed).toEqual([archivedSplit]); expect(splitsMutation.segments).toEqual(['A', 'B']); // SDK initialization without sets // should process all the notifications splitsMutation = computeSplitsMutation([testFFSetsAB, test2FFSetsX] as ISplit[], splitFiltersValidation); - expect(splitsMutation.added).toEqual([[testFFSetsAB.name, testFFSetsAB],[test2FFSetsX.name, test2FFSetsX]]); + expect(splitsMutation.added).toEqual([testFFSetsAB, test2FFSetsX]); expect(splitsMutation.removed).toEqual([]); expect(splitsMutation.segments).toEqual([]); }); test('splitChangesUpdater / compute splits mutation with filters', () => { // SDK initialization with sets: [set_a, set_b] - let splitFiltersValidation = { queryString: '&sets=set_a,set_b', groupedFilters: { bySet: ['set_a','set_b'], byName: ['name_1'], byPrefix: [] }, validFilters: [] }; + let splitFiltersValidation = { queryString: '&sets=set_a,set_b', groupedFilters: { bySet: ['set_a', 'set_b'], byName: ['name_1'], byPrefix: [] }, validFilters: [] }; // fetching new feature flag in sets A & B let splitsMutation = computeSplitsMutation([testFFSetsAB], splitFiltersValidation); // should add it to mutations - expect(splitsMutation.added).toEqual([[testFFSetsAB.name, testFFSetsAB]]); + expect(splitsMutation.added).toEqual([testFFSetsAB]); expect(splitsMutation.removed).toEqual([]); // fetching existing test feature flag removed from set B splitsMutation = computeSplitsMutation([testFFRemoveSetB], splitFiltersValidation); - expect(splitsMutation.added).toEqual([[testFFRemoveSetB.name, testFFRemoveSetB]]); + expect(splitsMutation.added).toEqual([testFFRemoveSetB]); expect(splitsMutation.removed).toEqual([]); // fetching existing test feature flag removed from set B splitsMutation = computeSplitsMutation([testFFRemoveSetA], splitFiltersValidation); expect(splitsMutation.added).toEqual([]); - expect(splitsMutation.removed).toEqual([testFFRemoveSetA.name]); + expect(splitsMutation.removed).toEqual([testFFRemoveSetA]); // fetching existing test feature flag removed from set B splitsMutation = computeSplitsMutation([testFFEmptySet], splitFiltersValidation); expect(splitsMutation.added).toEqual([]); - expect(splitsMutation.removed).toEqual([testFFEmptySet.name]); + expect(splitsMutation.removed).toEqual([testFFEmptySet]); // SDK initialization with names: ['test2'] splitFiltersValidation = { queryString: '&names=test2', groupedFilters: { bySet: [], byName: ['test2'], byPrefix: [] }, validFilters: [] }; splitsMutation = computeSplitsMutation([testFFSetsAB], splitFiltersValidation); expect(splitsMutation.added).toEqual([]); - expect(splitsMutation.removed).toEqual([testFFSetsAB.name]); + expect(splitsMutation.removed).toEqual([testFFSetsAB]); splitsMutation = computeSplitsMutation([test2FFSetsX, testFFEmptySet], splitFiltersValidation); - expect(splitsMutation.added).toEqual([[test2FFSetsX.name, test2FFSetsX],]); - expect(splitsMutation.removed).toEqual([testFFEmptySet.name]); + expect(splitsMutation.added).toEqual([test2FFSetsX]); + expect(splitsMutation.removed).toEqual([testFFEmptySet]); }); describe('splitChangesUpdater', () => { @@ -158,19 +158,20 @@ describe('splitChangesUpdater', () => { const fetchSplitChanges = jest.spyOn(splitApi, 'fetchSplitChanges'); const splitChangesFetcher = splitChangesFetcherFactory(splitApi.fetchSplitChanges); - const splitsCache = new SplitsCacheInMemory(); - const setChangeNumber = jest.spyOn(splitsCache, 'setChangeNumber'); - const addSplits = jest.spyOn(splitsCache, 'addSplits'); - const removeSplits = jest.spyOn(splitsCache, 'removeSplits'); + const splits = new SplitsCacheInMemory(); + const updateSplits = jest.spyOn(splits, 'update'); + + const segments = new SegmentsCacheInMemory(); + const registerSegments = jest.spyOn(segments, 'registerSegments'); + + const storage = { splits, segments }; - const segmentsCache = new SegmentsCacheInMemory(); - const registerSegments = jest.spyOn(segmentsCache, 'registerSegments'); const readinessManager = readinessManagerFactory(EventEmitter, fullSettings); const splitsEmitSpy = jest.spyOn(readinessManager.splits, 'emit'); let splitFiltersValidation = { queryString: null, groupedFilters: { bySet: [], byName: [], byPrefix: [] }, validFilters: [] }; - let splitChangesUpdater = splitChangesUpdaterFactory(loggerMock, splitChangesFetcher, splitsCache, segmentsCache, splitFiltersValidation, readinessManager.splits, 1000, 1); + let splitChangesUpdater = splitChangesUpdaterFactory(loggerMock, splitChangesFetcher, storage, splitFiltersValidation, readinessManager.splits, 1000, 1); afterEach(() => { jest.clearAllMocks(); @@ -178,12 +179,8 @@ describe('splitChangesUpdater', () => { test('test without payload', async () => { const result = await splitChangesUpdater(); - expect(setChangeNumber).toBeCalledTimes(1); - expect(setChangeNumber).lastCalledWith(splitChangesMock1.till); - expect(addSplits).toBeCalledTimes(1); - expect(addSplits.mock.calls[0][0].length).toBe(splitChangesMock1.splits.length); - expect(removeSplits).toBeCalledTimes(1); - expect(removeSplits).lastCalledWith([]); + expect(updateSplits).toBeCalledTimes(1); + expect(updateSplits).lastCalledWith(splitChangesMock1.splits, [], splitChangesMock1.till); expect(registerSegments).toBeCalledTimes(1); expect(splitsEmitSpy).toBeCalledWith('state::splits-arrived'); expect(result).toBe(true); @@ -195,18 +192,16 @@ describe('splitChangesUpdater', () => { const payload = notification.decoded as Pick; const changeNumber = payload.changeNumber; - await expect(splitChangesUpdater(undefined, undefined, { payload: {...payload, sets:[]}, changeNumber: changeNumber })).resolves.toBe(true); + await expect(splitChangesUpdater(undefined, undefined, { payload, changeNumber: changeNumber })).resolves.toBe(true); // fetch not being called expect(fetchSplitChanges).toBeCalledTimes(0); + expect(updateSplits).toBeCalledTimes(index + 1); // Change number being updated - expect(setChangeNumber).toBeCalledTimes(index + 1); - expect(setChangeNumber.mock.calls[index][0]).toEqual(changeNumber); + expect(updateSplits.mock.calls[index][2]).toEqual(changeNumber); // Add feature flag in notification - expect(addSplits).toBeCalledTimes(index + 1); - expect(addSplits.mock.calls[index][0].length).toBe(payload.status === ARCHIVED_FF ? 0 : 1); + expect(updateSplits.mock.calls[index][0].length).toBe(payload.status === ARCHIVED_FF ? 0 : 1); // Remove feature flag if status is ARCHIVED - expect(removeSplits).toBeCalledTimes(index + 1); - expect(removeSplits.mock.calls[index][0]).toEqual(payload.status === ARCHIVED_FF ? [payload.name] : []); + expect(updateSplits.mock.calls[index][1]).toEqual(payload.status === ARCHIVED_FF ? [payload] : []); // fetch segments after feature flag update expect(registerSegments).toBeCalledTimes(index + 1); expect(registerSegments.mock.calls[index][0]).toEqual(payload.status === ARCHIVED_FF ? [] : ['maur-2']); @@ -214,7 +209,7 @@ describe('splitChangesUpdater', () => { } }); - test('flag sets splits-arrived emition', async () => { + test('flag sets splits-arrived emission', async () => { const payload = splitNotifications[3].decoded as Pick; const setMocks = [ { sets: [], shouldEmit: false }, /* should not emit if flag does not have any set */ @@ -225,24 +220,25 @@ describe('splitChangesUpdater', () => { { sets: ['set_a'], shouldEmit: true }, /* should emit if flag is back in configured sets */ ]; - splitChangesUpdater = splitChangesUpdaterFactory(loggerMock, splitChangesFetcher, new SplitsCacheInMemory(), segmentsCache, splitFiltersValidation, readinessManager.splits, 1000, 1, true); + splitChangesUpdater = splitChangesUpdaterFactory(loggerMock, splitChangesFetcher, storage, splitFiltersValidation, readinessManager.splits, 1000, 1, true); let index = 0; let calls = 0; // emit always if not configured sets for (const setMock of setMocks) { - await expect(splitChangesUpdater(undefined, undefined, { payload: {...payload, sets: setMock.sets, status: 'ACTIVE'}, changeNumber: index })).resolves.toBe(true); + await expect(splitChangesUpdater(undefined, undefined, { payload: { ...payload, sets: setMock.sets, status: 'ACTIVE' }, changeNumber: index })).resolves.toBe(true); expect(splitsEmitSpy.mock.calls[index][0]).toBe('state::splits-arrived'); index++; } // @ts-ignore splitFiltersValidation = { queryString: null, groupedFilters: { bySet: ['set_a'], byName: [], byPrefix: [] }, validFilters: [] }; - splitChangesUpdater = splitChangesUpdaterFactory(loggerMock, splitChangesFetcher, new SplitsCacheInMemory(), segmentsCache, splitFiltersValidation, readinessManager.splits, 1000, 1, true); + storage.splits.clear(); + splitChangesUpdater = splitChangesUpdaterFactory(loggerMock, splitChangesFetcher, storage, splitFiltersValidation, readinessManager.splits, 1000, 1, true); splitsEmitSpy.mockReset(); index = 0; for (const setMock of setMocks) { - await expect(splitChangesUpdater(undefined, undefined, { payload: {...payload, sets: setMock.sets, status: 'ACTIVE'}, changeNumber: index })).resolves.toBe(true); + await expect(splitChangesUpdater(undefined, undefined, { payload: { ...payload, sets: setMock.sets, status: 'ACTIVE' }, changeNumber: index })).resolves.toBe(true); if (setMock.shouldEmit) calls++; expect(splitsEmitSpy.mock.calls.length).toBe(calls); index++; diff --git a/src/sync/polling/updaters/splitChangesUpdater.ts b/src/sync/polling/updaters/splitChangesUpdater.ts index e8153987..065ecb89 100644 --- a/src/sync/polling/updaters/splitChangesUpdater.ts +++ b/src/sync/polling/updaters/splitChangesUpdater.ts @@ -1,11 +1,11 @@ -import { ISegmentsCacheBase, ISplitsCacheBase } from '../../../storages/types'; +import { ISegmentsCacheBase, IStorageBase } from '../../../storages/types'; import { ISplitChangesFetcher } from '../fetchers/types'; import { ISplit, ISplitChangesResponse, ISplitFiltersValidation } from '../../../dtos/types'; import { ISplitsEventEmitter } from '../../../readiness/types'; import { timeout } from '../../../utils/promise/timeout'; import { SDK_SPLITS_ARRIVED, SDK_SPLITS_CACHE_LOADED } from '../../../readiness/constants'; import { ILogger } from '../../../logger/types'; -import { SYNC_SPLITS_FETCH, SYNC_SPLITS_NEW, SYNC_SPLITS_REMOVED, SYNC_SPLITS_SEGMENTS, SYNC_SPLITS_FETCH_FAILS, SYNC_SPLITS_FETCH_RETRY } from '../../../logger/constants'; +import { SYNC_SPLITS_FETCH, SYNC_SPLITS_UPDATE, SYNC_SPLITS_FETCH_FAILS, SYNC_SPLITS_FETCH_RETRY } from '../../../logger/constants'; import { startsWith } from '../../../utils/lang'; import { IN_SEGMENT } from '../../../utils/constants'; import { setToArray } from '../../../utils/lang/sets'; @@ -42,8 +42,8 @@ export function parseSegments({ conditions }: ISplit): Set { } interface ISplitMutations { - added: [string, ISplit][], - removed: string[], + added: ISplit[], + removed: ISplit[], segments: string[] } @@ -77,13 +77,13 @@ export function computeSplitsMutation(entries: ISplit[], filters: ISplitFiltersV const segments = new Set(); const computed = entries.reduce((accum, split) => { if (split.status === 'ACTIVE' && matchFilters(split, filters)) { - accum.added.push([split.name, split]); + accum.added.push(split); parseSegments(split).forEach((segmentName: string) => { segments.add(segmentName); }); } else { - accum.removed.push(split.name); + accum.removed.push(split); } return accum; @@ -111,14 +111,14 @@ export function computeSplitsMutation(entries: ISplit[], filters: ISplitFiltersV export function splitChangesUpdaterFactory( log: ILogger, splitChangesFetcher: ISplitChangesFetcher, - splits: ISplitsCacheBase, - segments: ISegmentsCacheBase, + storage: Pick, splitFiltersValidation: ISplitFiltersValidation, splitsEventEmitter?: ISplitsEventEmitter, requestTimeoutBeforeReady: number = 0, retriesOnFailureBeforeReady: number = 0, isClientSide?: boolean ): ISplitChangesUpdater { + const { splits, segments } = storage; let startingUp = true; @@ -128,16 +128,6 @@ export function splitChangesUpdaterFactory( return promise; } - /** Returns true if at least one split was updated */ - function isThereUpdate(flagsChange: [boolean | void, void | boolean[], void | boolean[], boolean | void] | [any, any, any]) { - const [, added, removed] = flagsChange; - // There is at least one added or modified feature flag - if (added && added.some((update: boolean) => update)) return true; - // There is at least one removed feature flag - if (removed && removed.some((update: boolean) => update)) return true; - return false; - } - /** * SplitChanges updater returns a promise that resolves with a `false` boolean value if it fails to fetch splits or synchronize them with the storage. * Returned promise will not be rejected. @@ -162,22 +152,17 @@ export function splitChangesUpdaterFactory( const mutation = computeSplitsMutation(splitChanges.splits, splitFiltersValidation); - log.debug(SYNC_SPLITS_NEW, [mutation.added.length]); - log.debug(SYNC_SPLITS_REMOVED, [mutation.removed.length]); - log.debug(SYNC_SPLITS_SEGMENTS, [mutation.segments.length]); + log.debug(SYNC_SPLITS_UPDATE, [mutation.added.length, mutation.removed.length, mutation.segments.length]); // Write into storage // @TODO call `setChangeNumber` only if the other storage operations have succeeded, in order to keep storage consistency return Promise.all([ - // calling first `setChangenumber` method, to perform cache flush if split filter queryString changed - splits.setChangeNumber(splitChanges.till), - splits.addSplits(mutation.added), - splits.removeSplits(mutation.removed), + splits.update(mutation.added, mutation.removed, splitChanges.till), segments.registerSegments(mutation.segments) - ]).then((flagsChange) => { + ]).then(([isThereUpdate]) => { if (splitsEventEmitter) { // To emit SDK_SPLITS_ARRIVED for server-side SDK, we must check that all registered segments have been fetched - return Promise.resolve(!splitsEventEmitter.splitsArrived || (since !== splitChanges.till && isThereUpdate(flagsChange) && (isClientSide || checkAllSegmentsExist(segments)))) + return Promise.resolve(!splitsEventEmitter.splitsArrived || (since !== splitChanges.till && isThereUpdate && (isClientSide || checkAllSegmentsExist(segments)))) .catch(() => false /** noop. just to handle a possible `checkAllSegmentsExist` rejection, before emitting SDK event */) .then(emitSplitsArrivedEvent => { // emit SDK events diff --git a/src/sync/streaming/UpdateWorkers/__tests__/SplitsUpdateWorker.spec.ts b/src/sync/streaming/UpdateWorkers/__tests__/SplitsUpdateWorker.spec.ts index d5fd3acd..4de69ca0 100644 --- a/src/sync/streaming/UpdateWorkers/__tests__/SplitsUpdateWorker.spec.ts +++ b/src/sync/streaming/UpdateWorkers/__tests__/SplitsUpdateWorker.spec.ts @@ -169,30 +169,30 @@ describe('SplitsUpdateWorker', () => { test('killSplit', async () => { // setup const cache = new SplitsCacheInMemory(); - cache.addSplit('lol1', '{ "name": "something"}'); - cache.addSplit('lol2', '{ "name": "something else"}'); + cache.addSplit({ name: 'something'}); + cache.addSplit({ name: 'something else'}); const splitsSyncTask = splitsSyncTaskMock(cache); const splitUpdateWorker = SplitsUpdateWorker(loggerMock, cache, splitsSyncTask, splitsEventEmitterMock, telemetryTracker); // assert killing split locally, emitting SDK_SPLITS_ARRIVED event, and synchronizing splits if changeNumber is new - splitUpdateWorker.killSplit({ changeNumber: 100, splitName: 'lol1', defaultTreatment: 'off' }); // splitsCache.killLocally is synchronous + splitUpdateWorker.killSplit({ changeNumber: 100, splitName: 'something', defaultTreatment: 'off' }); // splitsCache.killLocally is synchronous expect(splitsSyncTask.execute).toBeCalledTimes(1); // synchronizes splits if `isExecuting` is false expect(splitsEventEmitterMock.emit.mock.calls).toEqual([[SDK_SPLITS_ARRIVED, true]]); // emits `SDK_SPLITS_ARRIVED` with `isSplitKill` flag in true, if split kill resolves with update - assertKilledSplit(cache, 100, 'lol1', 'off'); + assertKilledSplit(cache, 100, 'something', 'off'); // assert not killing split locally, not emitting SDK_SPLITS_ARRIVED event, and not synchronizes splits, if changeNumber is old splitsSyncTask.__resolveSplitsUpdaterCall(100); await new Promise(res => setTimeout(res)); splitsSyncTask.execute.mockClear(); splitsEventEmitterMock.emit.mockClear(); - splitUpdateWorker.killSplit({ changeNumber: 90, splitName: 'lol1', defaultTreatment: 'on' }); + splitUpdateWorker.killSplit({ changeNumber: 90, splitName: 'something', defaultTreatment: 'on' }); await new Promise(res => setTimeout(res)); expect(splitsSyncTask.execute).toBeCalledTimes(0); // doesn't synchronize splits if killLocally resolved without update expect(splitsEventEmitterMock.emit).toBeCalledTimes(0); // doesn't emit `SDK_SPLITS_ARRIVED` if killLocally resolved without update - assertKilledSplit(cache, 100, 'lol1', 'off'); // calling `killLocally` with an old changeNumber made no effect + assertKilledSplit(cache, 100, 'something', 'off'); // calling `killLocally` with an old changeNumber made no effect }); test('stop', async () => {