Merge pull request #389 from splitio/refactor_splits_cache_update_method

Refactor splits cache: replace `add/removeSplits` with `update` method

Author: EmilianoSanchez

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;

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'],

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();

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', () => {
 

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<boolean>
-  abstract addSplits(entries: [string, ISplit][]): Promise<boolean[] | void>
-  abstract removeSplits(names: string[]): Promise<boolean[] | void>
+  protected abstract addSplit(split: ISplit): Promise<boolean>
+  protected abstract removeSplit(name: string): Promise<boolean>
+  protected abstract setChangeNumber(changeNumber: number): Promise<boolean | void>
+
+  update(toAdd: ISplit[], toRemove: ISplit[], changeNumber: number): Promise<boolean> {
+    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<ISplit | null>
   abstract getSplits(names: string[]): Promise<Record<string, ISplit | null>>
-  abstract setChangeNumber(changeNumber: number): Promise<boolean | void>
   abstract getChangeNumber(): Promise<number>
   abstract getAll(): Promise<ISplit[]>
   abstract getSplitNames(): Promise<string[]>
@@ -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);

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;
   }

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);
 

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() {

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';

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