Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove usage of Redis flushdb operation #277

Merged
merged 3 commits into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/storages/inRedis/RedisAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
4 changes: 2 additions & 2 deletions src/storages/inRedis/SegmentsCacheInRedis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
10 changes: 3 additions & 7 deletions src/storages/inRedis/SplitsCacheInRedis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/storages/inRedis/__tests__/RedisAdapter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
13 changes: 6 additions & 7 deletions src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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']);
Expand All @@ -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();
});

Expand Down
2 changes: 2 additions & 0 deletions src/storages/inRedis/__tests__/SplitsCacheInRedis.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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'));
Expand Down
Loading