[SDKS-7794] Add flag sets support in Redis storage

CHANGES.txt

@@ -1,5 +1,5 @@
 1.12.0 (December XX, 2023)
- - Added support for Flag Sets in "consumer" and "partial consumer" modes for pluggable storage.
+ - Added support for Flag Sets in "consumer" and "partial consumer" modes for Pluggable and Redis storages.
  - Updated evaluation flow to log a warning when using flag sets that don't contain cached feature flags.
 
 1.11.0 (November 3, 2023)
@@ -57,7 +57,7 @@
  - Added a new impressions mode for the SDK called NONE, to be used in factory when there is no desire to capture impressions on an SDK factory to feed Split's analytics engine. Running NONE mode, the SDK will only capture unique keys evaluated for a particular feature flag instead of full blown impressions.
  - Updated SDK telemetry to support pluggable storage, partial consumer mode, and synchronizer.
  - Updated storage implementations to improve the performance of feature flag evaluations (i.e., `getTreatment(s)` method calls) when using the default storage in memory.
- - Updated evaluation flow to avoid unnecessarily storage calls when the SDK is not ready.
+ - Updated evaluation flow (i.e., `getTreatment(s)` method calls) to avoid calling the storage for cached feature flags when the SDK is not ready or ready from cache. It applies to all SDK modes.
 
 1.6.1 (July 22, 2022)
  - Updated GoogleAnalyticsToSplit integration to validate `autoRequire` config parameter and avoid some wrong warning logs when mapping GA hit fields to Split event properties.

package.json

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

src/storages/inRedis/SplitsCacheInRedis.ts

@@ -3,9 +3,9 @@ import { KeyBuilderSS } from '../KeyBuilderSS';
 import { Redis } from 'ioredis';
 import { ILogger } from '../../logger/types';
 import { LOG_PREFIX } from './constants';
-import { ISplit } from '../../dtos/types';
+import { ISplit, ISplitFiltersValidation } from '../../dtos/types';
 import { AbstractSplitsCacheAsync } from '../AbstractSplitsCacheAsync';
-import { ISet } from '../../utils/lang/sets';
+import { ISet, _Set, returnListDifference } from '../../utils/lang/sets';
 
 /**
  * Discard errors for an answer of multiple operations.
@@ -27,12 +27,14 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync {
   private readonly redis: Redis;
   private readonly keys: KeyBuilderSS;
   private redisError?: string;
+  private readonly flagSetsFilter: string[];
 
-  constructor(log: ILogger, keys: KeyBuilderSS, redis: Redis) {
+  constructor(log: ILogger, keys: KeyBuilderSS, redis: Redis, splitFiltersValidation?: ISplitFiltersValidation) {
     super();
     this.log = log;
     this.redis = redis;
     this.keys = keys;
+    this.flagSetsFilter = splitFiltersValidation ? splitFiltersValidation.groupedFilters.bySet : [];
 
     // There is no need to listen for redis 'error' event, because in that case ioredis calls will be rejected and handled by redis storage adapters.
     // But it is done just to avoid getting the ioredis message `Unhandled error event`.
@@ -57,6 +59,24 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync {
     return this.redis.incr(ttKey);
   }
 
+  private _updateFlagSets(featureFlagName: string, flagSetsOfRemovedFlag?: string[], flagSetsOfAddedFlag?: string[]) {
+    const removeFromFlagSets = returnListDifference(flagSetsOfRemovedFlag, flagSetsOfAddedFlag);
+
+    let addToFlagSets = returnListDifference(flagSetsOfAddedFlag, flagSetsOfRemovedFlag);
+    if (this.flagSetsFilter.length > 0) {
+      addToFlagSets = addToFlagSets.filter(flagSet => {
+        return this.flagSetsFilter.some(filterFlagSet => filterFlagSet === flagSet);
+      });
+    }
+
+    const items = [featureFlagName];
+
+    return Promise.all([
+      ...removeFromFlagSets.map(flagSetName => this.redis.srem(this.keys.buildFlagSetKey(flagSetName), items)),
+      ...addToFlagSets.map(flagSetName => this.redis.sadd(this.keys.buildFlagSetKey(flagSetName), items))
+    ]);
+  }
+
   /**
    * Add a given split.
    * The returned promise is resolved when the operation success
@@ -66,24 +86,24 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync {
     const splitKey = this.keys.buildSplitKey(name);
     return this.redis.get(splitKey).then(splitFromStorage => {
 
-      // handling parsing errors
-      let parsedPreviousSplit: ISplit, newStringifiedSplit;
+      // handling parsing error
+      let parsedPreviousSplit: ISplit, stringifiedNewSplit;
       try {
         parsedPreviousSplit = splitFromStorage ? JSON.parse(splitFromStorage) : undefined;
-        newStringifiedSplit = JSON.stringify(split);
+        stringifiedNewSplit = JSON.stringify(split);
       } catch (e) {
         throw new Error('Error parsing feature flag definition: ' + e);
       }
 
-      return this.redis.set(splitKey, newStringifiedSplit).then(() => {
+      return this.redis.set(splitKey, stringifiedNewSplit).then(() => {
         // avoid unnecessary increment/decrement operations
         if (parsedPreviousSplit && parsedPreviousSplit.trafficTypeName === split.trafficTypeName) return;
 
         // update traffic type counts
         return this._incrementCounts(split).then(() => {
           if (parsedPreviousSplit) return this._decrementCounts(parsedPreviousSplit);
         });
-      });
+      }).then(() => this._updateFlagSets(name, parsedPreviousSplit && parsedPreviousSplit.sets, split.sets));
     }).then(() => true);
   }
 
@@ -101,11 +121,12 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync {
    * The returned promise is resolved when the operation success, with 1 or 0 indicating if the split existed or not.
    * or rejected if it fails (e.g., redis operation fails).
    */
-  removeSplit(name: string): Promise<number> {
+  removeSplit(name: string) {
     return this.getSplit(name).then((split) => {
       if (split) {
-        this._decrementCounts(split);
+        return this._decrementCounts(split).then(() => this._updateFlagSets(name, split.sets));
       }
+    }).then(() => {
       return this.redis.del(this.keys.buildSplitKey(name));
     });
   }
@@ -174,7 +195,7 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync {
       .then((listOfKeys) => this.redis.pipeline(listOfKeys.map(k => ['get', k])).exec())
       .then(processPipelineAnswer)
       .then((splitDefinitions) => splitDefinitions.map((splitDefinition) => {
-        return JSON.parse(splitDefinition as string);
+        return JSON.parse(splitDefinition);
       }));
   }
 
@@ -190,14 +211,18 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync {
   }
 
   /**
-   * Get list of split names related to a given flag set names list.
-   * The returned promise is resolved with the list of split names,
-   * or rejected if wrapper operation fails.
-   * @todo this is a no-op method to be implemented
+   * Get list of feature flag names related to a given list of flag set names.
+   * The returned promise is resolved with the list of feature flag names per flag set,
+   * or rejected if the pipelined redis operation fails.
   */
-  getNamesByFlagSets(): Promise<ISet<string>[]> {
-    this.log.error(LOG_PREFIX + 'ByFlagSet/s evaluations are not supported with Redis storage yet.');
-    return Promise.reject();
+  getNamesByFlagSets(flagSets: string[]): Promise<ISet<string>[]> {
+    return this.redis.pipeline(flagSets.map(flagSet => ['smembers', this.keys.buildFlagSetKey(flagSet)])).exec()
+      .then((results) => results.map(([e, value], index) => {
+        if (e === null) return value;
+
+        this.log.error(LOG_PREFIX + `Could not read result from get members of flag set ${flagSets[index]} due to an error: ${e}`);
+      }))
+      .then(namesByFlagSets => namesByFlagSets.map(namesByFlagSet => new _Set(namesByFlagSet)));
   }
 
   /**
@@ -214,14 +239,14 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync {
 
         ttCount = parseInt(ttCount as string, 10);
         if (!isFiniteNumber(ttCount) || ttCount < 0) {
-          this.log.info(LOG_PREFIX + `Could not validate traffic type existance of ${trafficType} due to data corruption of some sorts.`);
+          this.log.info(LOG_PREFIX + `Could not validate traffic type existence of ${trafficType} due to data corruption of some sorts.`);
           return false;
         }
 
         return ttCount > 0;
       })
       .catch(e => {
-        this.log.error(LOG_PREFIX + `Could not validate traffic type existance of ${trafficType} due to an error: ${e}.`);
+        this.log.error(LOG_PREFIX + `Could not validate traffic type existence of ${trafficType} due to an error: ${e}.`);
         // If there is an error, bypass the validation so the event can get tracked.
         return true;
       });

src/storages/inRedis/__tests__/SplitsCacheInRedis.spec.ts

@@ -2,9 +2,10 @@ import Redis from 'ioredis';
 import { SplitsCacheInRedis } from '../SplitsCacheInRedis';
 import { KeyBuilderSS } from '../../KeyBuilderSS';
 import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock';
-import { splitWithUserTT, splitWithAccountTT } from '../../__tests__/testUtils';
+import { splitWithUserTT, splitWithAccountTT, featureFlagOne, featureFlagThree, featureFlagTwo, featureFlagWithEmptyFS, featureFlagWithoutFS } from '../../__tests__/testUtils';
 import { ISplit } from '../../../dtos/types';
 import { metadata } from '../../__tests__/KeyBuilder.spec';
+import { _Set } from '../../../utils/lang/sets';
 
 const prefix = 'splits_cache_ut';
 const keysBuilder = new KeyBuilderSS(prefix, metadata);
@@ -143,4 +144,85 @@ describe('SPLITS CACHE REDIS', () => {
     await connection.quit();
   });
 
+  test('flag set cache tests', async () => {
+    const connection = new Redis(); // @ts-ignore
+    const cache = new SplitsCacheInRedis(loggerMock, keysBuilder, connection, { groupedFilters: { bySet: ['o', 'n', 'e', 'x'] } });
+
+    const emptySet = new _Set([]);
+
+    await cache.addSplits([
+      [featureFlagOne.name, featureFlagOne],
+      [featureFlagTwo.name, featureFlagTwo],
+      [featureFlagThree.name, featureFlagThree],
+    ]);
+    await cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS);
+
+    expect(await cache.getNamesByFlagSets(['o'])).toEqual([new _Set(['ff_one', 'ff_two'])]);
+    expect(await cache.getNamesByFlagSets(['n'])).toEqual([new _Set(['ff_one'])]);
+    expect(await cache.getNamesByFlagSets(['e'])).toEqual([new _Set(['ff_one', 'ff_three'])]);
+    expect(await cache.getNamesByFlagSets(['t'])).toEqual([emptySet]); // 't' not in filter
+    expect(await cache.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new _Set(['ff_one', 'ff_two']), new _Set(['ff_one']), new _Set(['ff_one', 'ff_three'])]);
+
+    await cache.addSplit(featureFlagOne.name, { ...featureFlagOne, sets: ['1'] });
+
+    expect(await cache.getNamesByFlagSets(['1'])).toEqual([emptySet]); // '1' not in filter
+    expect(await cache.getNamesByFlagSets(['o'])).toEqual([new _Set(['ff_two'])]);
+    expect(await cache.getNamesByFlagSets(['n'])).toEqual([emptySet]);
+
+    await cache.addSplit(featureFlagOne.name, { ...featureFlagOne, sets: ['x'] });
+    expect(await cache.getNamesByFlagSets(['x'])).toEqual([new _Set(['ff_one'])]);
+    expect(await cache.getNamesByFlagSets(['o', 'e', 'x'])).toEqual([new _Set(['ff_two']), new _Set(['ff_three']), new _Set(['ff_one'])]);
+
+    // @ts-ignore Simulate one error in connection.pipeline().exec()
+    jest.spyOn(connection, 'pipeline').mockImplementationOnce(() => {
+      return {
+        exec: () => Promise.resolve([['error', null], [null, ['ff_three']], [null, ['ff_one']]]),
+      };
+    });
+    expect(await cache.getNamesByFlagSets(['o', 'e', 'x'])).toEqual([emptySet, new _Set(['ff_three']), new _Set(['ff_one'])]);
+    (connection.pipeline as jest.Mock).mockRestore();
+
+    await cache.removeSplit(featureFlagOne.name);
+    expect(await cache.getNamesByFlagSets(['x'])).toEqual([emptySet]);
+
+    await cache.removeSplit(featureFlagOne.name);
+    expect(await cache.getNamesByFlagSets(['y'])).toEqual([emptySet]); // 'y' not in filter
+    expect(await cache.getNamesByFlagSets([])).toEqual([]);
+
+    await cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithoutFS);
+    expect(await cache.getNamesByFlagSets([])).toEqual([]);
+
+    // Delete splits, TT and flag set keys
+    await cache.removeSplits([featureFlagThree.name, featureFlagTwo.name, featureFlagWithEmptyFS.name]);
+    expect(await connection.keys(`${prefix}*`)).toHaveLength(0);
+    await connection.quit();
+  });
+
+  // if FlagSets filter is not defined, it should store all FlagSets in memory.
+  test('flag set cache tests without filters', async () => {
+    const connection = new Redis(); // @ts-ignore
+    const cacheWithoutFilters = new SplitsCacheInRedis(loggerMock, keysBuilder, connection);
+
+    const emptySet = new _Set([]);
+
+    await cacheWithoutFilters.addSplits([
+      [featureFlagOne.name, featureFlagOne],
+      [featureFlagTwo.name, featureFlagTwo],
+      [featureFlagThree.name, featureFlagThree],
+    ]);
+    await cacheWithoutFilters.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS);
+
+    expect(await cacheWithoutFilters.getNamesByFlagSets(['o'])).toEqual([new _Set(['ff_one', 'ff_two'])]);
+    expect(await cacheWithoutFilters.getNamesByFlagSets(['n'])).toEqual([new _Set(['ff_one'])]);
+    expect(await cacheWithoutFilters.getNamesByFlagSets(['e'])).toEqual([new _Set(['ff_one', 'ff_three'])]);
+    expect(await cacheWithoutFilters.getNamesByFlagSets(['t'])).toEqual([new _Set(['ff_two', 'ff_three'])]);
+    expect(await cacheWithoutFilters.getNamesByFlagSets(['y'])).toEqual([emptySet]);
+    expect(await cacheWithoutFilters.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new _Set(['ff_one', 'ff_two']), new _Set(['ff_one']), new _Set(['ff_one', 'ff_three'])]);
+
+    // Delete splits, TT and flag set keys
+    await cacheWithoutFilters.removeSplits([featureFlagThree.name, featureFlagTwo.name, featureFlagOne.name, featureFlagWithEmptyFS.name]);
+    expect(await connection.keys(`${prefix}*`)).toHaveLength(0);
+    await connection.quit();
+  });
+
 });

src/storages/pluggable/SplitsCachePluggable.ts

@@ -177,14 +177,14 @@ export class SplitsCachePluggable extends AbstractSplitsCacheAsync {
   }
 
   /**
-   * Get list of split names related to a given flag set names list.
-   * The returned promise is resolved with the list of split names,
-   * or rejected if any wrapper operation fails.
-  */
+   * Get list of feature flag names related to a given list of flag set names.
+   * The returned promise is resolved with the list of feature flag names per flag set.
+   * It never rejects (If there is a wrapper error for some flag set, an empty set is returned for it).
+   */
   getNamesByFlagSets(flagSets: string[]): Promise<ISet<string>[]> {
     return Promise.all(flagSets.map(flagSet => {
       const flagSetKey = this.keys.buildFlagSetKey(flagSet);
-      return this.wrapper.getItems(flagSetKey);
+      return this.wrapper.getItems(flagSetKey).catch(() => []);
     })).then(namesByFlagSets => namesByFlagSets.map(namesByFlagSet => new _Set(namesByFlagSet)));
   }
 

src/storages/pluggable/__tests__/SplitsCachePluggable.spec.ts

@@ -152,8 +152,8 @@ describe('SPLITS CACHE PLUGGABLE', () => {
   });
 
   test('flag set cache tests', async () => {
-    // @ts-ignore
-    const cache = new SplitsCachePluggable(loggerMock, keysBuilder, wrapperMockFactory(), { groupedFilters: { bySet: ['o', 'n', 'e', 'x'] } });
+    const wrapper = wrapperMockFactory(); // @ts-ignore
+    const cache = new SplitsCachePluggable(loggerMock, keysBuilder, wrapper, { groupedFilters: { bySet: ['o', 'n', 'e', 'x'] } });
     const emptySet = new _Set([]);
 
     await cache.addSplits([
@@ -179,6 +179,10 @@ describe('SPLITS CACHE PLUGGABLE', () => {
     expect(await cache.getNamesByFlagSets(['x'])).toEqual([new _Set(['ff_one'])]);
     expect(await cache.getNamesByFlagSets(['o', 'e', 'x'])).toEqual([new _Set(['ff_two']), new _Set(['ff_three']), new _Set(['ff_one'])]);
 
+    // Simulate one error in getItems
+    wrapper.getItems.mockImplementationOnce(() => Promise.reject('error'));
+    expect(await cache.getNamesByFlagSets(['o', 'e', 'x'])).toEqual([emptySet, new _Set(['ff_three']), new _Set(['ff_one'])]);
+
     await cache.removeSplit(featureFlagOne.name);
     expect(await cache.getNamesByFlagSets(['x'])).toEqual([emptySet]);