Merge pull request #277 from splitio/redis_clear_methods

Remove usage of Redis `flushdb` operation

src/storages/inRedis/RedisAdapter.ts

@@ -8,7 +8,7 @@ import { timeout } from '../../utils/promise/timeout';
 const LOG_PREFIX = 'storage:redis-adapter: ';
 
 // If we ever decide to fully wrap every method, there's a Commander.getBuiltinCommands from ioredis.
-const METHODS_TO_PROMISE_WRAP = ['set', 'exec', 'del', 'get', 'keys', 'sadd', 'srem', 'sismember', 'smembers', 'incr', 'rpush', 'expire', 'mget', 'lrange', 'ltrim', 'hset', 'hincrby', 'popNRaw', 'flushdb'];
+const METHODS_TO_PROMISE_WRAP = ['set', 'exec', 'del', 'get', 'keys', 'sadd', 'srem', 'sismember', 'smembers', 'incr', 'rpush', 'expire', 'mget', 'lrange', 'ltrim', 'hset', 'hincrby', 'popNRaw'];
 
 // Not part of the settings since it'll vary on each storage. We should be removing storage specific logic from elsewhere.
 const DEFAULT_OPTIONS = {

src/storages/inRedis/SegmentsCacheInRedis.ts

@@ -72,8 +72,8 @@ export class SegmentsCacheInRedis implements ISegmentsCacheAsync {
     return this.redis.smembers(this.keys.buildRegisteredSegmentsKey());
   }
 
-  // @TODO remove/review. It is not being used.
+  // @TODO remove or implement. It is not being used.
   clear() {
-    return this.redis.flushdb().then(status => status === 'OK');
+    return Promise.resolve();
   }
 }

src/storages/inRedis/SplitsCacheInRedis.ts

@@ -213,7 +213,7 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync {
   /**
    * 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.
+   * or rejected if the pipelined redis operation fails (e.g., timeout).
   */
   getNamesByFlagSets(flagSets: string[]): Promise<ISet<string>[]> {
     return this.redis.pipeline(flagSets.map(flagSet => ['smembers', this.keys.buildFlagSetKey(flagSet)])).exec()
@@ -252,13 +252,9 @@ export class SplitsCacheInRedis extends AbstractSplitsCacheAsync {
       });
   }
 
-  /**
-   * Delete everything in the current database.
-   *
-   * @NOTE documentation says it never fails.
-   */
+  // @TODO remove or implement. It is not being used.
   clear() {
-    return this.redis.flushdb().then(status => status === 'OK');
+    return Promise.resolve();
   }
 
   /**

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

@@ -11,7 +11,7 @@ const LOG_PREFIX = 'storage:redis-adapter: ';
 // Mocking ioredis
 
 // The list of methods we're wrapping on a promise (for timeout) on the adapter.
-const METHODS_TO_PROMISE_WRAP = ['set', 'exec', 'del', 'get', 'keys', 'sadd', 'srem', 'sismember', 'smembers', 'incr', 'rpush', 'expire', 'mget', 'lrange', 'ltrim', 'hset', 'hincrby', 'popNRaw', 'flushdb'];
+const METHODS_TO_PROMISE_WRAP = ['set', 'exec', 'del', 'get', 'keys', 'sadd', 'srem', 'sismember', 'smembers', 'incr', 'rpush', 'expire', 'mget', 'lrange', 'ltrim', 'hset', 'hincrby', 'popNRaw'];
 
 const ioredisMock = reduce([...METHODS_TO_PROMISE_WRAP, 'disconnect'], (acc, methodName) => {
   acc[methodName] = jest.fn(() => Promise.resolve(methodName));

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

@@ -4,16 +4,15 @@ import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock';
 import { metadata } from '../../__tests__/KeyBuilder.spec';
 import { RedisAdapter } from '../RedisAdapter';
 
-const keys = new KeyBuilderSS('prefix', metadata);
+const prefix = 'prefix';
+const keys = new KeyBuilderSS(prefix, metadata);
 
 describe('SEGMENTS CACHE IN REDIS', () => {
 
   test('isInSegment, set/getChangeNumber, add/removeFromSegment', async () => {
     const connection = new RedisAdapter(loggerMock);
     const cache = new SegmentsCacheInRedis(loggerMock, keys, connection);
 
-    await cache.clear();
-
     await cache.addToSegment('mocked-segment', ['a', 'b', 'c']);
 
     await cache.setChangeNumber('mocked-segment', 1);
@@ -36,16 +35,15 @@ describe('SEGMENTS CACHE IN REDIS', () => {
     expect(await cache.isInSegment('mocked-segment', 'd')).toBe(true);
     expect(await cache.isInSegment('mocked-segment', 'e')).toBe(true);
 
-    await cache.clear();
+    // Teardown
+    await connection.del(await connection.keys(`${prefix}.segment*`)); // @TODO use `cache.clear` method when implemented
     await connection.disconnect();
   });
 
   test('registerSegment / getRegisteredSegments', async () => {
     const connection = new RedisAdapter(loggerMock);
     const cache = new SegmentsCacheInRedis(loggerMock, keys, connection);
 
-    await cache.clear();
-
     await cache.registerSegments(['s1']);
     await cache.registerSegments(['s2']);
     await cache.registerSegments(['s2', 's3', 's4']);
@@ -54,7 +52,8 @@ describe('SEGMENTS CACHE IN REDIS', () => {
 
     ['s1', 's2', 's3', 's4'].forEach(s => expect(segments.indexOf(s) !== -1).toBe(true));
 
-    await cache.clear();
+    // Teardown
+    await connection.del(await connection.keys(`${prefix}.segment*`)); // @TODO use `cache.clear` method when implemented
     await connection.disconnect();
   });
 

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

@@ -52,6 +52,7 @@ describe('SPLITS CACHE REDIS', () => {
     expect(splits['lol1']).toEqual(null);
     expect(splits['lol2']).toEqual(splitWithAccountTT);
 
+    // Teardown. @TODO use cache clear method when implemented
     await connection.del(keysBuilder.buildTrafficTypeKey('account_tt'));
     await connection.del(keysBuilder.buildSplitKey('lol2'));
     await connection.del(keysBuilder.buildSplitsTillKey());
@@ -100,6 +101,7 @@ describe('SPLITS CACHE REDIS', () => {
     expect(await cache.trafficTypeExists('account_tt')).toBe(true);
     expect(await cache.trafficTypeExists('user_tt')).toBe(false);
 
+    // Teardown. @TODO use cache clear method when implemented
     await connection.del(keysBuilder.buildTrafficTypeKey('account_tt'));
     await connection.del(keysBuilder.buildSplitKey('malformed'));
     await connection.del(keysBuilder.buildSplitKey('split1'));