Move usesSegments into an utility function for reusability

Author: EmilianoSanchez

src/storages/AbstractSplitsCacheAsync.ts

@@ -31,12 +31,6 @@ export abstract class AbstractSplitsCacheAsync implements ISplitsCacheAsync {
   abstract trafficTypeExists(trafficType: string): Promise<boolean>
   abstract clear(): Promise<boolean | void>
 
-  // @TODO revisit segment-related methods ('usesSegments')
-  // noop, just keeping the interface. This is used by standalone client-side API only, and so only implemented by InMemory and InLocalStorage.
-  usesSegments(): Promise<boolean> {
-    return Promise.resolve(true);
-  }
-
   /**
    * Kill `name` split and set `defaultTreatment` and `changeNumber`.
    * Used for SPLIT_KILL push notifications.

src/storages/AbstractSplitsCacheSync.ts

@@ -1,4 +1,4 @@
-import { ISplitsCacheSync } from './types';
+import { ISplitsCacheSync, IStorageSync } from './types';
 import { IRBSegment, ISplit } from '../dtos/types';
 import { objectAssign } from '../utils/lang/objectAssign';
 import { IN_SEGMENT, IN_LARGE_SEGMENT } from '../utils/constants';
@@ -39,8 +39,6 @@ export abstract class AbstractSplitsCacheSync implements ISplitsCacheSync {
 
   abstract trafficTypeExists(trafficType: string): boolean
 
-  abstract usesSegments(): boolean
-
   abstract clear(): void
 
   /**
@@ -72,7 +70,7 @@ export abstract class AbstractSplitsCacheSync implements ISplitsCacheSync {
  * Given a parsed split, it returns a boolean flagging if its conditions use segments matchers (rules & whitelists).
  * This util is intended to simplify the implementation of `splitsCache::usesSegments` method
  */
-export function usesSegments(ruleEntity: ISplit | IRBSegment) {
+function hasSegments(ruleEntity: ISplit | IRBSegment) {
   const conditions = ruleEntity.conditions || [];
   for (let i = 0; i < conditions.length; i++) {
     const matchers = conditions[i].matcherGroup.matchers;
@@ -88,3 +86,9 @@ export function usesSegments(ruleEntity: ISplit | IRBSegment) {
 
   return false;
 }
+
+export function usesSegments(storage: Pick<IStorageSync, 'splits' | 'rbSegments'>) {
+  return storage.splits.getChangeNumber() === -1 ||
+    storage.splits.getAll().some(split => hasSegments(split)) ||
+    storage.rbSegments.getAll().some(rbSegment => hasSegments(rbSegment));
+}

src/storages/KeyBuilderCS.ts

@@ -51,10 +51,6 @@ export class KeyBuilderCS extends KeyBuilder implements MySegmentsKeyBuilder {
     return startsWith(key, `${this.prefix}.rbsegment.`);
   }
 
-  buildSplitsWithSegmentCountKey() {
-    return `${this.prefix}.splits.usingSegments`;
-  }
-
   buildLastClear() {
     return `${this.prefix}.lastClear`;
   }

src/storages/__tests__/RBSegmentsCacheSync.spec.ts

@@ -4,6 +4,8 @@ import { KeyBuilderCS } from '../KeyBuilderCS';
 import { rbSegment, rbSegmentWithInSegmentMatcher } from '../__tests__/testUtils';
 import { IRBSegmentsCacheSync } from '../types';
 import { fullSettings } from '../../utils/settingsValidation/__tests__/settings.mocks';
+import { usesSegments } from '../AbstractSplitsCacheSync';
+import { SplitsCacheInMemory } from '../inMemory/SplitsCacheInMemory';
 
 const cacheInMemory = new RBSegmentsCacheInMemory();
 const cacheInLocal = new RBSegmentsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'));
@@ -61,15 +63,18 @@ describe.each([cacheInMemory, cacheInLocal])('Rule-based segments cache sync (Me
   });
 
   test('usesSegments should track segments usage correctly', () => {
-    expect(cache.usesSegments()).toBe(false); // No rbSegments, so false
+    const storage = { rbSegments: cache, splits: new SplitsCacheInMemory() };
+    storage.splits.setChangeNumber(1);
+
+    expect(usesSegments(storage)).toBe(false); // No rbSegments, so false
 
     cache.update([rbSegment], [], 1); // rbSegment doesn't have IN_SEGMENT matcher
-    expect(cache.usesSegments()).toBe(false);
+    expect(usesSegments(storage)).toBe(false);
 
     cache.update([rbSegmentWithInSegmentMatcher], [], 2); // rbSegmentWithInSegmentMatcher has IN_SEGMENT matcher
-    expect(cache.usesSegments()).toBe(true);
+    expect(usesSegments(storage)).toBe(true);
 
     cache.clear();
-    expect(cache.usesSegments()).toBe(false); // False after clear since there are no rbSegments
+    expect(usesSegments(storage)).toBe(false); // False after clear since there are no rbSegments
   });
 });

src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts

@@ -1,9 +1,8 @@
 import { IRBSegment } from '../../dtos/types';
 import { ILogger } from '../../logger/types';
 import { ISettings } from '../../types';
-import { isFiniteNumber, isNaNNumber, toNumber } from '../../utils/lang';
+import { isNaNNumber } from '../../utils/lang';
 import { setToArray } from '../../utils/lang/sets';
-import { usesSegments } from '../AbstractSplitsCacheSync';
 import { KeyBuilderCS } from '../KeyBuilderCS';
 import { IRBSegmentsCacheSync } from '../types';
 import { LOG_PREFIX } from './constants';
@@ -38,28 +37,13 @@ export class RBSegmentsCacheInLocal implements IRBSegmentsCacheSync {
     }
   }
 
-  private updateSegmentCount(diff: number) {
-    const segmentsCountKey = this.keys.buildSplitsWithSegmentCountKey();
-    const count = toNumber(localStorage.getItem(segmentsCountKey)) + diff;
-    // @ts-expect-error
-    if (count > 0) localStorage.setItem(segmentsCountKey, count);
-    else localStorage.removeItem(segmentsCountKey);
-  }
-
   private add(rbSegment: IRBSegment): boolean {
     try {
       const name = rbSegment.name;
       const rbSegmentKey = this.keys.buildRBSegmentKey(name);
-      const rbSegmentFromLocalStorage = localStorage.getItem(rbSegmentKey);
-      const previous = rbSegmentFromLocalStorage ? JSON.parse(rbSegmentFromLocalStorage) : null;
 
       localStorage.setItem(rbSegmentKey, JSON.stringify(rbSegment));
 
-      let usesSegmentsDiff = 0;
-      if (previous && usesSegments(previous)) usesSegmentsDiff--;
-      if (usesSegments(rbSegment)) usesSegmentsDiff++;
-      if (usesSegmentsDiff !== 0) this.updateSegmentCount(usesSegmentsDiff);
-
       return true;
     } catch (e) {
       this.log.error(LOG_PREFIX + e);
@@ -74,8 +58,6 @@ export class RBSegmentsCacheInLocal implements IRBSegmentsCacheSync {
 
       localStorage.removeItem(this.keys.buildRBSegmentKey(name));
 
-      if (usesSegments(rbSegment)) this.updateSegmentCount(-1);
-
       return true;
     } catch (e) {
       this.log.error(LOG_PREFIX + e);
@@ -128,13 +110,4 @@ export class RBSegmentsCacheInLocal implements IRBSegmentsCacheSync {
     return n;
   }
 
-  usesSegments(): boolean {
-    const storedCount = localStorage.getItem(this.keys.buildSplitsWithSegmentCountKey());
-    const splitsWithSegmentsCount = storedCount === null ? 0 : toNumber(storedCount);
-
-    return isFiniteNumber(splitsWithSegmentsCount) ?
-      splitsWithSegmentsCount > 0 :
-      true;
-  }
-
 }

src/storages/inLocalStorage/SplitsCacheInLocal.ts

@@ -1,5 +1,5 @@
 import { ISplit } from '../../dtos/types';
-import { AbstractSplitsCacheSync, usesSegments } from '../AbstractSplitsCacheSync';
+import { AbstractSplitsCacheSync } from '../AbstractSplitsCacheSync';
 import { isFiniteNumber, toNumber, isNaNNumber } from '../../utils/lang';
 import { KeyBuilderCS } from '../KeyBuilderCS';
 import { ILogger } from '../../logger/types';
@@ -35,11 +35,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync {
     try {
       const ttKey = this.keys.buildTrafficTypeKey(split.trafficTypeName);
       this._decrementCount(ttKey);
-
-      if (usesSegments(split)) {
-        const segmentsCountKey = this.keys.buildSplitsWithSegmentCountKey();
-        this._decrementCount(segmentsCountKey);
-      }
     } catch (e) {
       this.log.error(LOG_PREFIX + e);
     }
@@ -50,12 +45,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync {
       const ttKey = this.keys.buildTrafficTypeKey(split.trafficTypeName);
       // @ts-expect-error
       localStorage.setItem(ttKey, toNumber(localStorage.getItem(ttKey)) + 1);
-
-      if (usesSegments(split)) {
-        const segmentsCountKey = this.keys.buildSplitsWithSegmentCountKey();
-        // @ts-expect-error
-        localStorage.setItem(segmentsCountKey, toNumber(localStorage.getItem(segmentsCountKey)) + 1);
-      }
     } catch (e) {
       this.log.error(LOG_PREFIX + e);
     }
@@ -176,18 +165,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync {
     return isFiniteNumber(ttCount) && ttCount > 0;
   }
 
-  usesSegments() {
-    // If cache hasn't been synchronized with the cloud, assume we need them.
-    if (!this.hasSync) return true;
-
-    const storedCount = localStorage.getItem(this.keys.buildSplitsWithSegmentCountKey());
-    const splitsWithSegmentsCount = storedCount === null ? 0 : toNumber(storedCount);
-
-    return isFiniteNumber(splitsWithSegmentsCount) ?
-      splitsWithSegmentsCount > 0 :
-      true;
-  }
-
   getNamesByFlagSets(flagSets: string[]): Set<string>[] {
     return flagSets.map(flagSet => {
       const flagSetKey = this.keys.buildFlagSetKey(flagSet);

src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts

@@ -3,6 +3,8 @@ import { KeyBuilderCS } from '../../KeyBuilderCS';
 import { splitWithUserTT, splitWithAccountTT, splitWithAccountTTAndUsesSegments, something, somethingElse, featureFlagOne, featureFlagTwo, featureFlagThree, featureFlagWithEmptyFS, featureFlagWithoutFS } from '../../__tests__/testUtils';
 import { ISplit } from '../../../dtos/types';
 import { fullSettings } from '../../../utils/settingsValidation/__tests__/settings.mocks';
+import { RBSegmentsCacheInMemory } from '../../inMemory/RBSegmentsCacheInMemory';
+import { usesSegments } from '../../AbstractSplitsCacheSync';
 
 
 test('SPLITS CACHE / LocalStorage', () => {
@@ -132,24 +134,26 @@ test('SPLITS CACHE / LocalStorage / killLocally', () => {
 
 test('SPLITS CACHE / LocalStorage / usesSegments', () => {
   const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'));
+  cache.clear();
+  const storage = { splits: cache, rbSegments: new RBSegmentsCacheInMemory() };
 
-  expect(cache.usesSegments()).toBe(true); // true initially, until data is synchronized
+  expect(usesSegments(storage)).toBe(true); // true initially, until data is synchronized
   cache.setChangeNumber(1); // to indicate that data has been synced.
 
   cache.update([splitWithUserTT, splitWithAccountTT], [], 1);
-  expect(cache.usesSegments()).toBe(false); // 0 splits using segments
+  expect(usesSegments(storage)).toBe(false); // 0 splits using segments
 
   cache.addSplit({ ...splitWithAccountTTAndUsesSegments, name: 'split3' });
-  expect(cache.usesSegments()).toBe(true); // 1 split using segments
+  expect(usesSegments(storage)).toBe(true); // 1 split using segments
 
   cache.addSplit({ ...splitWithAccountTTAndUsesSegments, name: 'split4' });
-  expect(cache.usesSegments()).toBe(true); // 2 splits using segments
+  expect(usesSegments(storage)).toBe(true); // 2 splits using segments
 
   cache.removeSplit('split3');
-  expect(cache.usesSegments()).toBe(true); // 1 split using segments
+  expect(usesSegments(storage)).toBe(true); // 1 split using segments
 
   cache.removeSplit('split4');
-  expect(cache.usesSegments()).toBe(false); // 0 splits using segments
+  expect(usesSegments(storage)).toBe(false); // 0 splits using segments
 });
 
 test('SPLITS CACHE / LocalStorage / flag set cache tests', () => {

src/storages/inMemory/RBSegmentsCacheInMemory.ts

@@ -1,18 +1,15 @@
 import { IRBSegment } from '../../dtos/types';
 import { setToArray } from '../../utils/lang/sets';
-import { usesSegments } from '../AbstractSplitsCacheSync';
 import { IRBSegmentsCacheSync } from '../types';
 
 export class RBSegmentsCacheInMemory implements IRBSegmentsCacheSync {
 
   private cache: Record<string, IRBSegment> = {};
   private changeNumber: number = -1;
-  private segmentsCount: number = 0;
 
   clear() {
     this.cache = {};
     this.changeNumber = -1;
-    this.segmentsCount = 0;
   }
 
   update(toAdd: IRBSegment[], toRemove: IRBSegment[], changeNumber: number): boolean {
@@ -23,11 +20,8 @@ export class RBSegmentsCacheInMemory implements IRBSegmentsCacheSync {
 
   private add(rbSegment: IRBSegment): boolean {
     const name = rbSegment.name;
-    const previous = this.get(name);
-    if (previous && usesSegments(previous)) this.segmentsCount--;
 
     this.cache[name] = rbSegment;
-    if (usesSegments(rbSegment)) this.segmentsCount++;
 
     return true;
   }
@@ -38,8 +32,6 @@ export class RBSegmentsCacheInMemory implements IRBSegmentsCacheSync {
 
     delete this.cache[name];
 
-    if (usesSegments(rbSegment)) this.segmentsCount--;
-
     return true;
   }
 
@@ -65,8 +57,4 @@ export class RBSegmentsCacheInMemory implements IRBSegmentsCacheSync {
     return this.changeNumber;
   }
 
-  usesSegments(): boolean {
-    return this.segmentsCount > 0;
-  }
-
 }

src/storages/inMemory/SplitsCacheInMemory.ts

@@ -1,5 +1,5 @@
 import { ISplit, ISplitFiltersValidation } from '../../dtos/types';
-import { AbstractSplitsCacheSync, usesSegments } from '../AbstractSplitsCacheSync';
+import { AbstractSplitsCacheSync } from '../AbstractSplitsCacheSync';
 import { isFiniteNumber } from '../../utils/lang';
 
 /**
@@ -11,7 +11,6 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync {
   private splitsCache: Record<string, ISplit> = {};
   private ttCache: Record<string, number> = {};
   private changeNumber: number = -1;
-  private segmentsCount: number = 0;
   private flagSetsCache: Record<string, Set<string>> = {};
 
   constructor(splitFiltersValidation?: ISplitFiltersValidation) {
@@ -23,7 +22,6 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync {
     this.splitsCache = {};
     this.ttCache = {};
     this.changeNumber = -1;
-    this.segmentsCount = 0;
     this.flagSetsCache = {};
   }
 
@@ -37,9 +35,6 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync {
       if (!this.ttCache[previousTtName]) delete this.ttCache[previousTtName];
 
       this.removeFromFlagSets(previousSplit.name, previousSplit.sets);
-
-      // Subtract from segments count for the previous version of this Split
-      if (usesSegments(previousSplit)) this.segmentsCount--;
     }
 
     // Store the Split.
@@ -49,9 +44,6 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync {
     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++;
-
     return true;
   }
 
@@ -67,9 +59,6 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync {
     if (!this.ttCache[ttName]) delete this.ttCache[ttName];
     this.removeFromFlagSets(split.name, split.sets);
 
-    // Update the segments count.
-    if (usesSegments(split)) this.segmentsCount--;
-
     return true;
   }
 
@@ -94,10 +83,6 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync {
     return isFiniteNumber(this.ttCache[trafficType]) && this.ttCache[trafficType] > 0;
   }
 
-  usesSegments(): boolean {
-    return this.getChangeNumber() === -1 || this.segmentsCount > 0;
-  }
-
   getNamesByFlagSets(flagSets: string[]): Set<string>[] {
     return flagSets.map(flagSet => this.flagSetsCache[flagSet] || new Set());
   }

src/storages/types.ts

@@ -186,8 +186,6 @@ export interface ISplitsCacheBase {
   getSplitNames(): MaybeThenable<string[]>,
   // should never reject or throw an exception. Instead return true by default, asssuming the TT might exist.
   trafficTypeExists(trafficType: string): MaybeThenable<boolean>,
-  // only for Client-Side. Returns true if the storage is not synchronized yet (getChangeNumber() === -1) or contains a FF using segments or large segments
-  usesSegments(): MaybeThenable<boolean>,
   clear(): MaybeThenable<boolean | void>,
   killLocally(name: string, defaultTreatment: string, changeNumber: number): MaybeThenable<boolean>,
   getNamesByFlagSets(flagSets: string[]): MaybeThenable<Set<string>[]>
@@ -201,7 +199,6 @@ export interface ISplitsCacheSync extends ISplitsCacheBase {
   getAll(): ISplit[],
   getSplitNames(): string[],
   trafficTypeExists(trafficType: string): boolean,
-  usesSegments(): boolean,
   clear(): void,
   killLocally(name: string, defaultTreatment: string, changeNumber: number): boolean,
   getNamesByFlagSets(flagSets: string[]): Set<string>[]
@@ -215,7 +212,6 @@ export interface ISplitsCacheAsync extends ISplitsCacheBase {
   getAll(): Promise<ISplit[]>,
   getSplitNames(): Promise<string[]>,
   trafficTypeExists(trafficType: string): Promise<boolean>,
-  usesSegments(): Promise<boolean>,
   clear(): Promise<boolean | void>,
   killLocally(name: string, defaultTreatment: string, changeNumber: number): Promise<boolean>,
   getNamesByFlagSets(flagSets: string[]): Promise<Set<string>[]>
@@ -239,8 +235,6 @@ export interface IRBSegmentsCacheSync extends IRBSegmentsCacheBase {
   clear(): void,
   contains(names: Set<string>): boolean,
   getAll(): IRBSegment[],
-  // Used only for smart pausing in client-side standalone. Returns true if the storage contains a RBSegment using segments or large segments matchers
-  usesSegments(): boolean,
 }
 
 export interface IRBSegmentsCacheAsync extends IRBSegmentsCacheBase {