Merge branch 'rb_segments_baseline' into prerequisites_refactors

Author: EmilianoSanchez

CHANGES.txt

@@ -1,6 +1,6 @@
 {
   "name": "@splitsoftware/splitio-commons",
-  "version": "2.2.1-rc.0",
+  "version": "2.3.0",
   "description": "Split JavaScript SDK common components",
   "main": "cjs/index.js",
   "module": "esm/index.js",

package-lock.json

@@ -7,7 +7,9 @@ test('createUserConsentAPI', () => {
   const syncManager = { submitterManager: syncTaskFactory() };
   const storage = {
     events: { clear: jest.fn() },
-    impressions: { clear: jest.fn() }
+    impressions: { clear: jest.fn() },
+    impressionCounts: { clear: jest.fn() },
+    uniqueKeys: { clear: jest.fn() }
   };
 
   // @ts-ignore

package.json

@@ -15,7 +15,7 @@ const ConsentStatus = {
  * The public user consent API exposed via SplitFactory, used to control if the SDK tracks and sends impressions and events or not.
  */
 export function createUserConsentAPI(params: ISdkFactoryContext) {
-  const { settings, settings: { log }, syncManager, storage: { events, impressions, impressionCounts } } = params;
+  const { settings, settings: { log }, syncManager, storage: { events, impressions, impressionCounts, uniqueKeys } } = params;
 
   if (!isConsentGranted(settings)) log.info(USER_CONSENT_INITIAL, [settings.userConsent]);
 
@@ -41,7 +41,8 @@ export function createUserConsentAPI(params: ISdkFactoryContext) {
           // @ts-ignore, clear method is present in storage for standalone and partial consumer mode
           if (events.clear) events.clear(); // @ts-ignore
           if (impressions.clear) impressions.clear(); // @ts-ignore
-          if (impressionCounts && impressionCounts.clear) impressionCounts.clear();
+          if (impressionCounts.clear) impressionCounts.clear(); // @ts-ignore
+          if (uniqueKeys.clear) uniqueKeys.clear();
         }
       } else {
         log.info(USER_CONSENT_NOT_UPDATED, [newConsentStatus]);

src/consent/__tests__/sdkUserConsent.spec.ts

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

src/consent/sdkUserConsent.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: [
       {
@@ -87,8 +95,8 @@ const STORED_RBSEGMENTS: Record<string, IRBSegment> = {
     changeNumber: 123,
     status: 'ACTIVE',
     excluded: {
-      keys: [],
-      segments: []
+      keys: null,
+      segments: null,
     },
     conditions: [{
       matcherGroup: {
@@ -135,6 +143,37 @@ 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
+            }
+          ]
+        }
+      }
+    ],
+  },
+  'rule_based_segment_without_conditions': {
+    name: 'rule_based_segment_without_conditions',
+    changeNumber: 123,
+    status: 'ACTIVE',
+    conditions: []
+  }
 };
 
 const mockStorageSync = {
@@ -149,6 +188,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 +212,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 +239,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);
@@ -238,6 +297,14 @@ describe.each([
 
     // should support feature flag dependency matcher
     expect(await matcherTrueAlwaysOn({ key: 'a-key' }, evaluateFeature)).toBe(true); // Parent split returns one of the expected treatments, so the matcher returns true
+
+    const matcherTrueRuleBasedSegmentWithoutConditions = matcherFactory(loggerMock, {
+      type: matcherTypes.IN_RULE_BASED_SEGMENT,
+      value: 'rule_based_segment_without_conditions'
+    } as IMatcherDto, mockStorageSync)!;
+
+    // should support rule-based segment without conditions
+    expect(await matcherTrueRuleBasedSegmentWithoutConditions({ key: 'a-key' }, evaluateFeature)).toBe(false);
   });
 
 });

src/dtos/types.ts

@@ -1,18 +1,23 @@
-import { IRBSegment, MaybeThenable } from '../../dtos/types';
+import { IExcludedSegment, IRBSegment, MaybeThenable } from '../../dtos/types';
 import { IStorageAsync, IStorageSync } from '../../storages/types';
 import { ILogger } from '../../logger/types';
 import { IDependencyMatcherValue, ISplitEvaluator } from '../types';
 import { thenable } from '../../utils/promise/thenable';
 import { getMatching, keyParser } from '../../utils/key';
 import { parser } from '../parser';
+import { STANDARD_SEGMENT, RULE_BASED_SEGMENT, LARGE_SEGMENT } from '../../utils/constants';
 
 
 export function ruleBasedSegmentMatcherContext(segmentName: string, storage: IStorageSync | IStorageAsync, log: ILogger) {
 
   return function ruleBasedSegmentMatcher({ key, attributes }: IDependencyMatcherValue, splitEvaluator: ISplitEvaluator): MaybeThenable<boolean> {
+    const matchingKey = getMatching(key);
 
     function matchConditions(rbsegment: IRBSegment) {
       const conditions = rbsegment.conditions || [];
+
+      if (!conditions.length) return false;
+
       const evaluator = parser(log, conditions, storage);
 
       const evaluation = evaluator(
@@ -29,22 +34,29 @@ export function ruleBasedSegmentMatcherContext(segmentName: string, storage: ISt
         evaluation ? true : false;
     }
 
+    function isInExcludedSegment({ type, name }: IExcludedSegment) {
+      return type === STANDARD_SEGMENT ?
+        storage.segments.isInSegment(name, matchingKey) :
+        type === RULE_BASED_SEGMENT ?
+          ruleBasedSegmentMatcherContext(name, storage, log)({ key, attributes }, splitEvaluator) :
+          type === LARGE_SEGMENT && (storage as IStorageSync).largeSegments ?
+            (storage as IStorageSync).largeSegments!.isInSegment(name, matchingKey) :
+            false;
+    }
+
     function isExcluded(rbSegment: IRBSegment) {
-      const matchingKey = getMatching(key);
       const excluded = rbSegment.excluded || {};
 
       if (excluded.keys && excluded.keys.indexOf(matchingKey) !== -1) return true;
 
-      const isInSegment = (excluded.segments || []).map(segmentName => {
-        return storage.segments.isInSegment(segmentName, matchingKey);
-      });
-
-      return isInSegment.length && thenable(isInSegment[0]) ?
-        Promise.all(isInSegment).then(results => results.some(result => result)) :
-        isInSegment.some(result => result);
+      return (excluded.segments || []).reduce<MaybeThenable<boolean>>((result, excludedSegment) => {
+        return thenable(result) ?
+          result.then(result => result || isInExcludedSegment(excludedSegment)) :
+          result || isInExcludedSegment(excludedSegment);
+      }, false);
     }
 
-    function isInSegment(rbSegment: IRBSegment | null) {
+    function isInRBSegment(rbSegment: IRBSegment | null) {
       if (!rbSegment) return false;
       const excluded = isExcluded(rbSegment);
 
@@ -56,7 +68,7 @@ export function ruleBasedSegmentMatcherContext(segmentName: string, storage: ISt
     const rbSegment = storage.rbSegments.get(segmentName);
 
     return thenable(rbSegment) ?
-      rbSegment.then(isInSegment) :
-      isInSegment(rbSegment);
+      rbSegment.then(isInRBSegment) :
+      isInRBSegment(rbSegment);
   };
 }