Merge pull request #406 from splitio/rb_excluded_segments

RBS excluded segments

Author: EmilianoSanchez

src/dtos/types.ts

@@ -199,14 +199,19 @@ export interface ISplitCondition {
   conditionType?: 'ROLLOUT' | 'WHITELIST'
 }
 
+export interface IExcludedSegments {
+  type: 'standard' | 'large' | 'rule-based',
+  name: string,
+}
+
 export interface IRBSegment {
   name: string,
   changeNumber: number,
   status: 'ACTIVE' | 'ARCHIVED',
   conditions?: ISplitCondition[],
   excluded?: {
     keys?: string[],
-    segments?: string[]
+    segments?: IExcludedSegments[]
   }
 }
 

src/evaluator/matchers/__tests__/rbsegment.spec.ts

@@ -13,18 +13,26 @@ const STORED_SPLITS: Record<string, ISplit> = {
 };
 
 const STORED_SEGMENTS: Record<string, Set<string>> = {
-  'segment_test': new Set(['emi@split.io']),
+  'excluded_standard_segment': new Set(['emi@split.io']),
   'regular_segment': new Set(['nadia@split.io'])
 };
 
+const STORED_LARGE_SEGMENTS: Record<string, Set<string>> = {
+  'excluded_large_segment': new Set(['emi-large@split.io'])
+};
+
 const STORED_RBSEGMENTS: Record<string, IRBSegment> = {
   'mauro_rule_based_segment': {
     changeNumber: 5,
     name: 'mauro_rule_based_segment',
     status: 'ACTIVE',
     excluded: {
       keys: ['mauro@split.io', 'gaston@split.io'],
-      segments: ['segment_test']
+      segments: [
+        { type: 'standard', name: 'excluded_standard_segment' },
+        { type: 'large', name: 'excluded_large_segment' },
+        { type: 'rule-based', name: 'excluded_rule_based_segment' }
+      ]
     },
     conditions: [
       {
@@ -135,6 +143,31 @@ const STORED_RBSEGMENTS: Record<string, IRBSegment> = {
       }
     }]
   },
+  'excluded_rule_based_segment': {
+    name: 'excluded_rule_based_segment',
+    changeNumber: 123,
+    status: 'ACTIVE',
+    conditions: [
+      {
+        matcherGroup: {
+          combiner: 'AND',
+          matchers: [
+            {
+              keySelector: null,
+              matcherType: 'WHITELIST',
+              negate: false,
+              userDefinedSegmentMatcherData: null,
+              whitelistMatcherData: {
+                whitelist: ['emi-rule-based@split.io']
+              },
+              unaryNumericMatcherData: null,
+              betweenMatcherData: null
+            }
+          ]
+        }
+      }
+    ],
+  }
 };
 
 const mockStorageSync = {
@@ -149,6 +182,11 @@ const mockStorageSync = {
       return STORED_SEGMENTS[segmentName] ? STORED_SEGMENTS[segmentName].has(matchingKey) : false;
     }
   },
+  largeSegments: {
+    isInSegment(segmentName: string, matchingKey: string) {
+      return STORED_LARGE_SEGMENTS[segmentName] ? STORED_LARGE_SEGMENTS[segmentName].has(matchingKey) : false;
+    }
+  },
   rbSegments: {
     get(rbsegmentName: string) {
       return STORED_RBSEGMENTS[rbsegmentName];
@@ -168,6 +206,11 @@ const mockStorageAsync = {
       return Promise.resolve(STORED_SEGMENTS[segmentName] ? STORED_SEGMENTS[segmentName].has(matchingKey) : false);
     }
   },
+  largeSegments: {
+    isInSegment(segmentName: string, matchingKey: string) {
+      return Promise.resolve(STORED_LARGE_SEGMENTS[segmentName] ? STORED_LARGE_SEGMENTS[segmentName].has(matchingKey) : false);
+    }
+  },
   rbSegments: {
     get(rbsegmentName: string) {
       return Promise.resolve(STORED_RBSEGMENTS[rbsegmentName]);
@@ -190,18 +233,28 @@ describe.each([
       value: 'depend_on_mauro_rule_based_segment'
     } as IMatcherDto, mockStorage)!;
 
-    [matcher, dependentMatcher].forEach(async matcher => {
+    [matcher, dependentMatcher].forEach(async (matcher) => {
 
       // should return false if the provided key is excluded (even if some condition is met)
       let match = matcher({ key: 'mauro@split.io', attributes: { location: 'mdp' } }, evaluateFeature);
       expect(thenable(match)).toBe(isAsync);
       expect(await match).toBe(false);
 
-      // should return false if the provided key is in some excluded segment (even if some condition is met)
+      // should return false if the provided key is in some excluded standard segment (even if some condition is met)
       match = matcher({ key: 'emi@split.io', attributes: { location: 'tandil' } }, evaluateFeature);
       expect(thenable(match)).toBe(isAsync);
       expect(await match).toBe(false);
 
+      // should return false if the provided key is in some excluded large segment (even if some condition is met)
+      match = matcher({ key: 'emi-large@split.io', attributes: { location: 'tandil' } }, evaluateFeature);
+      expect(thenable(match)).toBe(isAsync);
+      expect(await match).toBe(false);
+
+      // should return false if the provided key is in some excluded rule-based segment (even if some condition is met)
+      match = matcher({ key: 'emi-rule-based@split.io', attributes: { location: 'tandil' } }, evaluateFeature);
+      expect(thenable(match)).toBe(isAsync);
+      expect(await match).toBe(false);
+
       // should return false if doesn't match any condition
       match = matcher({ key: 'zeta@split.io' }, evaluateFeature);
       expect(thenable(match)).toBe(isAsync);

src/evaluator/matchers/rbsegment.ts

@@ -35,8 +35,14 @@ export function ruleBasedSegmentMatcherContext(segmentName: string, storage: ISt
 
       if (excluded.keys && excluded.keys.indexOf(matchingKey) !== -1) return true;
 
-      const isInSegment = (excluded.segments || []).map(segmentName => {
-        return storage.segments.isInSegment(segmentName, matchingKey);
+      const isInSegment = (excluded.segments || []).map(({ type, name }) => {
+        return type === 'standard' ?
+          storage.segments.isInSegment(name, matchingKey) :
+          type === 'rule-based' ?
+            ruleBasedSegmentMatcherContext(name, storage, log)({ key, attributes }, splitEvaluator) :
+            type === 'large' && (storage as IStorageSync).largeSegments ?
+              (storage as IStorageSync).largeSegments!.isInSegment(name, matchingKey) :
+              false;
       });
 
       return isInSegment.length && thenable(isInSegment[0]) ?

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

@@ -14,6 +14,7 @@ import { telemetryTrackerFactory } from '../../../../trackers/telemetryTracker';
 import { splitNotifications } from '../../../streaming/__tests__/dataMocks';
 import { RBSegmentsCacheInMemory } from '../../../../storages/inMemory/RBSegmentsCacheInMemory';
 import { RB_SEGMENT_UPDATE, SPLIT_UPDATE } from '../../../streaming/constants';
+import { IN_RULE_BASED_SEGMENT } from '../../../../utils/constants';
 
 const ARCHIVED_FF = 'ARCHIVED';
 
@@ -84,13 +85,31 @@ const testFFEmptySet: ISplit =
   conditions: [],
   sets: []
 };
+// @ts-ignore
+const rbsWithExcludedSegment: IRBSegment = {
+  name: 'rbs',
+  status: 'ACTIVE',
+  conditions: [],
+  excluded: {
+    segments: [{
+      type: 'standard',
+      name: 'C'
+    }, {
+      type: 'rule-based',
+      name: 'D'
+    }]
+  }
+};
 
 test('splitChangesUpdater / segments parser', () => {
+  let segments = parseSegments(activeSplitWithSegments as ISplit);
+  expect(segments).toEqual(new Set(['A', 'B']));
 
-  const segments = parseSegments(activeSplitWithSegments as ISplit);
+  segments = parseSegments(rbsWithExcludedSegment);
+  expect(segments).toEqual(new Set(['C']));
 
-  expect(segments.has('A')).toBe(true);
-  expect(segments.has('B')).toBe(true);
+  segments = parseSegments(rbsWithExcludedSegment, IN_RULE_BASED_SEGMENT);
+  expect(segments).toEqual(new Set(['D']));
 });
 
 test('splitChangesUpdater / compute splits mutation', () => {

src/sync/polling/updaters/splitChangesUpdater.ts

@@ -26,12 +26,20 @@ function checkAllSegmentsExist(segments: ISegmentsCacheBase): Promise<boolean> {
 }
 
 /**
- * Collect segments from a raw split definition.
+ * Collect segments from a raw FF or RBS definition.
  * 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 segments = new Set<string>(excluded && excluded.segments);
+
+  const segments = new Set<string>();
+  if (excluded && excluded.segments) {
+    excluded.segments.forEach(({ type, name }) => {
+      if ((type === 'standard' && matcherType === IN_SEGMENT) || (type === 'rule-based' && matcherType === IN_RULE_BASED_SEGMENT)) {
+        segments.add(name);
+      }
+    });
+  }
 
   for (let i = 0; i < conditions.length; i++) {
     const matchers = conditions[i].matcherGroup.matchers;