From 81b438265343c99fabfec131d77060f22880dabe Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 30 Nov 2023 12:53:50 -0300 Subject: [PATCH] Remove usage of Redis flushdb operation --- CHANGES.txt | 2 +- src/storages/inRedis/RedisAdapter.ts | 2 +- src/storages/inRedis/SegmentsCacheInRedis.ts | 4 ++-- src/storages/inRedis/SplitsCacheInRedis.ts | 10 +++------- src/storages/inRedis/__tests__/RedisAdapter.spec.ts | 2 +- .../inRedis/__tests__/SegmentsCacheInRedis.spec.ts | 13 ++++++------- 6 files changed, 14 insertions(+), 19 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 46996d43..25b37ee5 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,7 +1,7 @@ 1.12.0 (December XX, 2023) - 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. - - Updated Redis adapter to wrap missing commands: 'hincrby', 'popNRaw', 'flushdb' and 'pipeline.exec'. + - Updated Redis adapter to wrap missing commands: 'hincrby', 'popNRaw', and 'pipeline.exec'. - Bugfixing - Fixed manager methods in consumer modes to return results in a promise when the SDK is not operational (not ready or destroyed). 1.11.0 (November 3, 2023) diff --git a/src/storages/inRedis/RedisAdapter.ts b/src/storages/inRedis/RedisAdapter.ts index 191834c4..e6425636 100644 --- a/src/storages/inRedis/RedisAdapter.ts +++ b/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 = { diff --git a/src/storages/inRedis/SegmentsCacheInRedis.ts b/src/storages/inRedis/SegmentsCacheInRedis.ts index 3a18b535..7ec2f20f 100644 --- a/src/storages/inRedis/SegmentsCacheInRedis.ts +++ b/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(); } } diff --git a/src/storages/inRedis/SplitsCacheInRedis.ts b/src/storages/inRedis/SplitsCacheInRedis.ts index a9370ea3..e05c56ba 100644 --- a/src/storages/inRedis/SplitsCacheInRedis.ts +++ b/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[]> { 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(); } /** diff --git a/src/storages/inRedis/__tests__/RedisAdapter.spec.ts b/src/storages/inRedis/__tests__/RedisAdapter.spec.ts index 90b8743f..b982d06a 100644 --- a/src/storages/inRedis/__tests__/RedisAdapter.spec.ts +++ b/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)); diff --git a/src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts b/src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts index 009d6b10..6222af95 100644 --- a/src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts +++ b/src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts @@ -4,7 +4,8 @@ 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', () => { @@ -12,8 +13,6 @@ describe('SEGMENTS CACHE IN REDIS', () => { 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,7 +35,8 @@ 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(); }); @@ -44,8 +44,6 @@ describe('SEGMENTS CACHE IN REDIS', () => { 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(); });