Merge pull request #402 from splitio/rb_segments_polishing

[Rule-based segments] Polishing

Author: EmilianoSanchez

CHANGES.txt

@@ -1,3 +1,6 @@
+2.3.0 (April XXX, 2025)
+ - Added support for targeting rules based on rule-based segments.
+
 2.2.0 (March 28, 2025)
  - Added a new optional argument to the client `getTreatment` methods to allow passing additional evaluation options, such as a map of properties to append to the generated impressions sent to Split backend. Read more in our docs.
  - Added two new configuration options for the SDK storage in browsers when using storage type `LOCALSTORAGE`:

src/dtos/types.ts

@@ -203,10 +203,10 @@ export interface IRBSegment {
   name: string,
   changeNumber: number,
   status: 'ACTIVE' | 'ARCHIVED',
-  conditions: ISplitCondition[],
-  excluded: {
-    keys: string[],
-    segments: string[]
+  conditions?: ISplitCondition[],
+  excluded?: {
+    keys?: string[],
+    segments?: string[]
   }
 }
 

src/evaluator/matchers/rbsegment.ts

@@ -12,7 +12,7 @@ export function ruleBasedSegmentMatcherContext(segmentName: string, storage: ISt
   return function ruleBasedSegmentMatcher({ key, attributes }: IDependencyMatcherValue, splitEvaluator: ISplitEvaluator): MaybeThenable<boolean> {
 
     function matchConditions(rbsegment: IRBSegment) {
-      const conditions = rbsegment.conditions;
+      const conditions = rbsegment.conditions || [];
       const evaluator = parser(log, conditions, storage);
 
       const evaluation = evaluator(
@@ -31,10 +31,11 @@ export function ruleBasedSegmentMatcherContext(segmentName: string, storage: ISt
 
     function isExcluded(rbSegment: IRBSegment) {
       const matchingKey = getMatching(key);
+      const excluded = rbSegment.excluded || {};
 
-      if (rbSegment.excluded.keys.indexOf(matchingKey) !== -1) return true;
+      if (excluded.keys && excluded.keys.indexOf(matchingKey) !== -1) return true;
 
-      const isInSegment = rbSegment.excluded.segments.map(segmentName => {
+      const isInSegment = (excluded.segments || []).map(segmentName => {
         return storage.segments.isInSegment(segmentName, matchingKey);
       });
 

src/storages/KeyBuilderCS.ts

@@ -15,7 +15,7 @@ export class KeyBuilderCS extends KeyBuilder implements MySegmentsKeyBuilder {
   constructor(prefix: string, matchingKey: string) {
     super(prefix);
     this.matchingKey = matchingKey;
-    this.regexSplitsCacheKey = new RegExp(`^${prefix}\\.(splits?|trafficType|flagSet|rbsegment)\\.`);
+    this.regexSplitsCacheKey = new RegExp(`^${prefix}\\.(splits?|trafficType|flagSet)\\.`);
   }
 
   /**

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

@@ -5,6 +5,7 @@ import { fullSettings } from '../../../utils/settingsValidation/__tests__/settin
 import { SplitsCacheInLocal } from '../SplitsCacheInLocal';
 import { nearlyEqual } from '../../../__tests__/testUtils';
 import { MySegmentsCacheInLocal } from '../MySegmentsCacheInLocal';
+import { RBSegmentsCacheInLocal } from '../RBSegmentsCacheInLocal';
 
 const FULL_SETTINGS_HASH = 'dc1f9817';
 
@@ -14,9 +15,11 @@ describe('validateCache', () => {
   const segments = new MySegmentsCacheInLocal(fullSettings.log, keys);
   const largeSegments = new MySegmentsCacheInLocal(fullSettings.log, keys);
   const splits = new SplitsCacheInLocal(fullSettings, keys);
+  const rbSegments = new RBSegmentsCacheInLocal(fullSettings, keys);
 
-  jest.spyOn(splits, 'clear');
   jest.spyOn(splits, 'getChangeNumber');
+  jest.spyOn(splits, 'clear');
+  jest.spyOn(rbSegments, 'clear');
   jest.spyOn(segments, 'clear');
   jest.spyOn(largeSegments, 'clear');
 
@@ -26,11 +29,12 @@ describe('validateCache', () => {
   });
 
   test('if there is no cache, it should return false', () => {
-    expect(validateCache({}, fullSettings, keys, splits, segments, largeSegments)).toBe(false);
+    expect(validateCache({}, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
 
     expect(logSpy).not.toHaveBeenCalled();
 
     expect(splits.clear).not.toHaveBeenCalled();
+    expect(rbSegments.clear).not.toHaveBeenCalled();
     expect(segments.clear).not.toHaveBeenCalled();
     expect(largeSegments.clear).not.toHaveBeenCalled();
     expect(splits.getChangeNumber).toHaveBeenCalledTimes(1);
@@ -43,11 +47,12 @@ describe('validateCache', () => {
     localStorage.setItem(keys.buildSplitsTillKey(), '1');
     localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH);
 
-    expect(validateCache({}, fullSettings, keys, splits, segments, largeSegments)).toBe(true);
+    expect(validateCache({}, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(true);
 
     expect(logSpy).not.toHaveBeenCalled();
 
     expect(splits.clear).not.toHaveBeenCalled();
+    expect(rbSegments.clear).not.toHaveBeenCalled();
     expect(segments.clear).not.toHaveBeenCalled();
     expect(largeSegments.clear).not.toHaveBeenCalled();
     expect(splits.getChangeNumber).toHaveBeenCalledTimes(1);
@@ -61,11 +66,12 @@ describe('validateCache', () => {
     localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH);
     localStorage.setItem(keys.buildLastUpdatedKey(), Date.now() - 1000 * 60 * 60 * 24 * 2 + ''); // 2 days ago
 
-    expect(validateCache({ expirationDays: 1 }, fullSettings, keys, splits, segments, largeSegments)).toBe(false);
+    expect(validateCache({ expirationDays: 1 }, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
 
     expect(logSpy).toHaveBeenCalledWith('storage:localstorage: Cache expired more than 1 days ago. Cleaning up cache');
 
     expect(splits.clear).toHaveBeenCalledTimes(1);
+    expect(rbSegments.clear).toHaveBeenCalledTimes(1);
     expect(segments.clear).toHaveBeenCalledTimes(1);
     expect(largeSegments.clear).toHaveBeenCalledTimes(1);
 
@@ -77,11 +83,12 @@ describe('validateCache', () => {
     localStorage.setItem(keys.buildSplitsTillKey(), '1');
     localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH);
 
-    expect(validateCache({}, { ...fullSettings, core: { ...fullSettings.core, authorizationKey: 'another-sdk-key' } }, keys, splits, segments, largeSegments)).toBe(false);
+    expect(validateCache({}, { ...fullSettings, core: { ...fullSettings.core, authorizationKey: 'another-sdk-key' } }, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
 
     expect(logSpy).toHaveBeenCalledWith('storage:localstorage: SDK key, flags filter criteria, or flags spec version has changed. Cleaning up cache');
 
     expect(splits.clear).toHaveBeenCalledTimes(1);
+    expect(rbSegments.clear).toHaveBeenCalledTimes(1);
     expect(segments.clear).toHaveBeenCalledTimes(1);
     expect(largeSegments.clear).toHaveBeenCalledTimes(1);
 
@@ -94,11 +101,12 @@ describe('validateCache', () => {
     localStorage.setItem(keys.buildSplitsTillKey(), '1');
     localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH);
 
-    expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, segments, largeSegments)).toBe(false);
+    expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
 
     expect(logSpy).toHaveBeenCalledWith('storage:localstorage: clearOnInit was set and cache was not cleared in the last 24 hours. Cleaning up cache');
 
     expect(splits.clear).toHaveBeenCalledTimes(1);
+    expect(rbSegments.clear).toHaveBeenCalledTimes(1);
     expect(segments.clear).toHaveBeenCalledTimes(1);
     expect(largeSegments.clear).toHaveBeenCalledTimes(1);
 
@@ -109,15 +117,16 @@ describe('validateCache', () => {
     // If cache is cleared, it should not clear again until a day has passed
     logSpy.mockClear();
     localStorage.setItem(keys.buildSplitsTillKey(), '1');
-    expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, segments, largeSegments)).toBe(true);
+    expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(true);
     expect(logSpy).not.toHaveBeenCalled();
     expect(localStorage.getItem(keys.buildLastClear())).toBe(lastClear); // Last clear should not have changed
 
     // If a day has passed, it should clear again
     localStorage.setItem(keys.buildLastClear(), (Date.now() - 1000 * 60 * 60 * 24 - 1) + '');
-    expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, segments, largeSegments)).toBe(false);
+    expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
     expect(logSpy).toHaveBeenCalledWith('storage:localstorage: clearOnInit was set and cache was not cleared in the last 24 hours. Cleaning up cache');
     expect(splits.clear).toHaveBeenCalledTimes(2);
+    expect(rbSegments.clear).toHaveBeenCalledTimes(2);
     expect(segments.clear).toHaveBeenCalledTimes(2);
     expect(largeSegments.clear).toHaveBeenCalledTimes(2);
     expect(nearlyEqual(parseInt(localStorage.getItem(keys.buildLastClear()) as string), Date.now())).toBe(true);

src/storages/inLocalStorage/index.ts

@@ -53,7 +53,7 @@ export function InLocalStorage(options: SplitIO.InLocalStorageOptions = {}): ISt
       uniqueKeys: new UniqueKeysCacheInMemoryCS(),
 
       validateCache() {
-        return validateCache(options, settings, keys, splits, segments, largeSegments);
+        return validateCache(options, settings, keys, splits, rbSegments, segments, largeSegments);
       },
 
       destroy() { },

src/storages/inLocalStorage/validateCache.ts

@@ -3,6 +3,7 @@ import { isFiniteNumber, isNaNNumber } from '../../utils/lang';
 import { getStorageHash } from '../KeyBuilder';
 import { LOG_PREFIX } from './constants';
 import type { SplitsCacheInLocal } from './SplitsCacheInLocal';
+import type { RBSegmentsCacheInLocal } from './RBSegmentsCacheInLocal';
 import type { MySegmentsCacheInLocal } from './MySegmentsCacheInLocal';
 import { KeyBuilderCS } from '../KeyBuilderCS';
 import SplitIO from '../../../types/splitio';
@@ -66,13 +67,14 @@ function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: IS
  *
  * @returns `true` if cache is ready to be used, `false` otherwise (cache was cleared or there is no cache)
  */
-export function validateCache(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal, segments: MySegmentsCacheInLocal, largeSegments: MySegmentsCacheInLocal): boolean {
+export function validateCache(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal, rbSegments: RBSegmentsCacheInLocal, segments: MySegmentsCacheInLocal, largeSegments: MySegmentsCacheInLocal): boolean {
 
   const currentTimestamp = Date.now();
   const isThereCache = splits.getChangeNumber() > -1;
 
   if (validateExpiration(options, settings, keys, currentTimestamp, isThereCache)) {
     splits.clear();
+    rbSegments.clear();
     segments.clear();
     largeSegments.clear();
 

src/sync/polling/updaters/__tests__/splitChangesUpdater.spec.ts

@@ -156,12 +156,6 @@ test('splitChangesUpdater / compute splits mutation with filters', () => {
 });
 
 describe('splitChangesUpdater', () => {
-
-  fetchMock.once('*', { status: 200, body: splitChangesMock1 }); // @ts-ignore
-  const splitApi = splitApiFactory(settingsSplitApi, { getFetch: () => fetchMock }, telemetryTrackerFactory());
-  const fetchSplitChanges = jest.spyOn(splitApi, 'fetchSplitChanges');
-  const splitChangesFetcher = splitChangesFetcherFactory(splitApi.fetchSplitChanges);
-
   const splits = new SplitsCacheInMemory();
   const updateSplits = jest.spyOn(splits, 'update');
 
@@ -173,6 +167,11 @@ describe('splitChangesUpdater', () => {
 
   const storage = { splits, rbSegments, segments };
 
+  fetchMock.once('*', { status: 200, body: splitChangesMock1 }); // @ts-ignore
+  const splitApi = splitApiFactory(settingsSplitApi, { getFetch: () => fetchMock }, telemetryTrackerFactory());
+  const fetchSplitChanges = jest.spyOn(splitApi, 'fetchSplitChanges');
+  const splitChangesFetcher = splitChangesFetcherFactory(splitApi.fetchSplitChanges);
+
   const readinessManager = readinessManagerFactory(EventEmitter, fullSettings);
   const splitsEmitSpy = jest.spyOn(readinessManager.splits, 'emit');
 

src/sync/polling/updaters/splitChangesUpdater.ts

@@ -30,7 +30,7 @@ function checkAllSegmentsExist(segments: ISegmentsCacheBase): Promise<boolean> {
  * Exported for testing purposes.
  */
 export function parseSegments(ruleEntity: ISplit | IRBSegment, matcherType: typeof IN_SEGMENT | typeof IN_RULE_BASED_SEGMENT = IN_SEGMENT): Set<string> {
-  const { conditions, excluded } = ruleEntity as IRBSegment;
+  const { conditions = [], excluded } = ruleEntity as IRBSegment;
   const segments = new Set<string>(excluded && excluded.segments);
 
   for (let i = 0; i < conditions.length; i++) {