diff --git a/.eslintrc b/.eslintrc index a44d175d..9231c4df 100644 --- a/.eslintrc +++ b/.eslintrc @@ -66,7 +66,7 @@ "plugin:compat/recommended" ], "rules": { - "no-restricted-syntax": ["error", "ForOfStatement", "ForInStatement", "ArrayPattern"], + "no-restricted-syntax": ["error", "ForOfStatement", "ForInStatement"], "compat/compat": ["error", "defaults, not ie < 10, not node < 6"], "no-throw-literal": "error" }, diff --git a/src/evaluator/index.ts b/src/evaluator/index.ts index 8dd5aa14..3c12c9b3 100644 --- a/src/evaluator/index.ts +++ b/src/evaluator/index.ts @@ -15,6 +15,14 @@ const treatmentException = { config: null }; +function treatmentsException(splitNames: string[]) { + const evaluations: Record = {}; + splitNames.forEach(splitName => { + evaluations[splitName] = treatmentException; + }); + return evaluations; +} + export function evaluateFeature( log: ILogger, key: SplitIO.SplitKey, @@ -27,10 +35,8 @@ export function evaluateFeature( try { stringifiedSplit = storage.splits.getSplit(splitName); } catch (e) { - // the only scenario where getSplit can throw an error is when the storage - // is redis and there is a connection issue and we can't retrieve the split - // to be evaluated - return Promise.resolve(treatmentException); + // Exception on sync `getSplit` storage. Not possible ATM with InMemory and InLocal storages. + return treatmentException; } if (thenable(stringifiedSplit)) { @@ -40,7 +46,11 @@ export function evaluateFeature( key, attributes, storage, - )); + )).catch( + // Exception on async `getSplit` storage. For example, when the storage is redis or + // pluggable and there is a connection issue and we can't retrieve the split to be evaluated + () => treatmentException + ); } return getEvaluation( @@ -60,22 +70,21 @@ export function evaluateFeatures( storage: IStorageSync | IStorageAsync, ): MaybeThenable> { let stringifiedSplits; - const evaluations: Record = {}; try { stringifiedSplits = storage.splits.getSplits(splitNames); } catch (e) { - // the only scenario where `getSplits` can throw an error is when the storage - // is redis and there is a connection issue and we can't retrieve the split - // to be evaluated - splitNames.forEach(splitName => { - evaluations[splitName] = treatmentException; - }); - return Promise.resolve(evaluations); + // Exception on sync `getSplits` storage. Not possible ATM with InMemory and InLocal storages. + return treatmentsException(splitNames); } return (thenable(stringifiedSplits)) ? - stringifiedSplits.then(splits => getEvaluations(log, splitNames, splits, key, attributes, storage)) : + stringifiedSplits.then(splits => getEvaluations(log, splitNames, splits, key, attributes, storage)) + .catch(() => { + // Exception on async `getSplits` storage. For example, when the storage is redis or + // pluggable and there is a connection issue and we can't retrieve the split to be evaluated + return treatmentsException(splitNames); + }) : getEvaluations(log, splitNames, stringifiedSplits, key, attributes, storage); } diff --git a/src/evaluator/value/sanitize.ts b/src/evaluator/value/sanitize.ts index 46cae859..991ed8e1 100644 --- a/src/evaluator/value/sanitize.ts +++ b/src/evaluator/value/sanitize.ts @@ -23,7 +23,7 @@ function sanitizeString(val: any): string | undefined { return str ? str : undefined; } -function sanitizeArray(val: any): number[] | string[] | undefined { +function sanitizeArray(val: any): string[] | undefined { const arr = Array.isArray(val) ? uniq(val.map(e => e + '')) : []; return arr.length ? arr : undefined; } diff --git a/src/storages/AbstractSplitsCacheSync.ts b/src/storages/AbstractSplitsCacheSync.ts index 36eef20d..3c0c251e 100644 --- a/src/storages/AbstractSplitsCacheSync.ts +++ b/src/storages/AbstractSplitsCacheSync.ts @@ -10,26 +10,13 @@ export default abstract class AbstractSplitsCacheSync implements ISplitsCacheSyn abstract addSplit(name: string, split: string): boolean; addSplits(entries: [string, string][]): boolean[] { - const results: boolean[] = []; - - entries.forEach(keyValuePair => { - results.push(this.addSplit(keyValuePair[0], keyValuePair[1])); - }); - - return results; + return entries.map(keyValuePair => this.addSplit(keyValuePair[0], keyValuePair[1])); } - abstract removeSplit(name: string): number - - removeSplits(names: string[]): number { - let len = names.length; - let counter = 0; - - for (let i = 0; i < len; i++) { - counter += this.removeSplit(names[i]); - } + abstract removeSplit(name: string): boolean - return counter; + removeSplits(names: string[]): boolean[] { + return names.map(name => this.removeSplit(name)); } abstract getSplit(name: string): string | null diff --git a/src/storages/KeyBuilder.ts b/src/storages/KeyBuilder.ts index 36e4badc..801bab3b 100644 --- a/src/storages/KeyBuilder.ts +++ b/src/storages/KeyBuilder.ts @@ -33,6 +33,14 @@ export default class KeyBuilder { return startsWith(key, `${this.prefix}.split.`); } + buildSplitKeyPrefix() { + return `${this.prefix}.split.`; + } + + buildSplitsWithSegmentCountKey() { + return `${this.prefix}.splits.usingSegments`; + } + buildSegmentNameKey(segmentName: string) { return `${this.prefix}.segment.${segmentName}`; } diff --git a/src/storages/KeyBuilderCS.ts b/src/storages/KeyBuilderCS.ts index 7d2c1312..6bfe7adf 100644 --- a/src/storages/KeyBuilderCS.ts +++ b/src/storages/KeyBuilderCS.ts @@ -26,10 +26,6 @@ export default class KeyBuilderCS extends KeyBuilder { return builtSegmentKeyName.substr(prefix.length); } - buildSplitsWithSegmentCountKey() { - return `${this.prefix}.splits.usingSegments`; - } - buildLastUpdatedKey() { return `${this.prefix}.splits.lastUpdated`; } diff --git a/src/storages/KeyBuilderSS.ts b/src/storages/KeyBuilderSS.ts index 903a0c73..1270c6f3 100644 --- a/src/storages/KeyBuilderSS.ts +++ b/src/storages/KeyBuilderSS.ts @@ -53,7 +53,7 @@ export default class KeyBuilderSS extends KeyBuilder { // } searchPatternForSplitKeys() { - return `${this.prefix}.split.*`; + return `${this.buildSplitKeyPrefix()}*`; } // NOT USED diff --git a/src/storages/dataLoader.ts b/src/storages/dataLoader.ts index 97ae6b3b..26e0090d 100644 --- a/src/storages/dataLoader.ts +++ b/src/storages/dataLoader.ts @@ -39,9 +39,7 @@ export function dataLoaderFactory(preloadedData: SplitIO.PreloadedData): DataLoa storage.splits.setChangeNumber(since); // splitsData in an object where the property is the split name and the pertaining value is a stringified json of its data - Object.keys(splitsData).forEach(splitName => { - storage.splits.addSplit(splitName, splitsData[splitName]); - }); + storage.splits.addSplits(Object.keys(splitsData).map(splitName => [splitName, splitsData[splitName]])); // add mySegments data let mySegmentsData = preloadedData.mySegmentsData && preloadedData.mySegmentsData[userId]; diff --git a/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts b/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts index 020f823a..12289470 100644 --- a/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts +++ b/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts @@ -1,7 +1,7 @@ import { ILogger } from '../../logger/types'; import AbstractSegmentsCacheSync from '../AbstractSegmentsCacheSync'; import KeyBuilderCS from '../KeyBuilderCS'; -import { logPrefix, DEFINED } from './constants'; +import { LOG_PREFIX, DEFINED } from './constants'; export default class MySegmentsCacheInLocal extends AbstractSegmentsCacheSync { @@ -20,7 +20,7 @@ export default class MySegmentsCacheInLocal extends AbstractSegmentsCacheSync { * @NOTE this method is not being used at the moment. */ clear() { - this.log.info(logPrefix + 'Flushing MySegments data from localStorage'); + this.log.info(LOG_PREFIX + 'Flushing MySegments data from localStorage'); // We cannot simply call `localStorage.clear()` since that implies removing user items from the storage // We could optimize next sentence, since it implies iterating over all localStorage items @@ -34,7 +34,7 @@ export default class MySegmentsCacheInLocal extends AbstractSegmentsCacheSync { localStorage.setItem(segmentKey, DEFINED); return true; } catch (e) { - this.log.error(logPrefix + e); + this.log.error(LOG_PREFIX + e); return false; } } @@ -46,7 +46,7 @@ export default class MySegmentsCacheInLocal extends AbstractSegmentsCacheSync { localStorage.removeItem(segmentKey); return true; } catch (e) { - this.log.error(logPrefix + e); + this.log.error(LOG_PREFIX + e); return false; } } diff --git a/src/storages/inLocalStorage/SplitsCacheInLocal.ts b/src/storages/inLocalStorage/SplitsCacheInLocal.ts index 58d5ba09..68565b55 100644 --- a/src/storages/inLocalStorage/SplitsCacheInLocal.ts +++ b/src/storages/inLocalStorage/SplitsCacheInLocal.ts @@ -3,7 +3,7 @@ import AbstractSplitsCacheSync, { usesSegments } from '../AbstractSplitsCacheSyn import { isFiniteNumber, toNumber, isNaNNumber } from '../../utils/lang'; import KeyBuilderCS from '../KeyBuilderCS'; import { ILogger } from '../../logger/types'; -import { logPrefix } from './constants'; +import { LOG_PREFIX } from './constants'; /** * ISplitsCacheSync implementation that stores split definitions in browser LocalStorage. @@ -52,7 +52,7 @@ export default class SplitsCacheInLocal extends AbstractSplitsCacheSync { } } } catch (e) { - this.log.error(logPrefix + e); + this.log.error(LOG_PREFIX + e); } } @@ -72,7 +72,7 @@ export default class SplitsCacheInLocal extends AbstractSplitsCacheSync { } } } catch (e) { - this.log.error(logPrefix + e); + this.log.error(LOG_PREFIX + e); } } @@ -82,7 +82,7 @@ export default class SplitsCacheInLocal extends AbstractSplitsCacheSync { * We cannot simply call `localStorage.clear()` since that implies removing user items from the storage. */ clear() { - this.log.info(logPrefix + 'Flushing Splits data from localStorage'); + this.log.info(LOG_PREFIX + 'Flushing Splits data from localStorage'); // collect item keys const len = localStorage.length; @@ -114,12 +114,12 @@ export default class SplitsCacheInLocal extends AbstractSplitsCacheSync { return true; } catch (e) { - this.log.error(logPrefix + e); + this.log.error(LOG_PREFIX + e); return false; } } - removeSplit(name: string): number { + removeSplit(name: string): boolean { try { const split = this.getSplit(name); localStorage.removeItem(this.keys.buildSplitKey(name)); @@ -127,10 +127,10 @@ export default class SplitsCacheInLocal extends AbstractSplitsCacheSync { const parsedSplit = JSON.parse(split as string); this._decrementCounts(parsedSplit); - return 1; + return true; } catch (e) { - this.log.error(logPrefix + e); - return 0; + this.log.error(LOG_PREFIX + e); + return false; } } @@ -147,14 +147,14 @@ export default class SplitsCacheInLocal extends AbstractSplitsCacheSync { // when using a new split query, we must update it at the store if (this.updateNewFilter) { - this.log.info(logPrefix + 'Split filter query was modified. Updating cache.'); + this.log.info(LOG_PREFIX + 'Split filter query was modified. Updating cache.'); const queryKey = this.keys.buildSplitsFilterQueryKey(); const queryString = this.splitFiltersValidation.queryString; try { if (queryString) localStorage.setItem(queryKey, queryString); else localStorage.removeItem(queryKey); } catch (e) { - this.log.error(logPrefix + e); + this.log.error(LOG_PREFIX + e); } this.updateNewFilter = false; } @@ -166,7 +166,7 @@ export default class SplitsCacheInLocal extends AbstractSplitsCacheSync { this.hasSync = true; return true; } catch (e) { - this.log.error(logPrefix + e); + this.log.error(LOG_PREFIX + e); return false; } } @@ -273,7 +273,7 @@ export default class SplitsCacheInLocal extends AbstractSplitsCacheSync { }); } } catch (e) { - this.log.error(logPrefix + e); + this.log.error(LOG_PREFIX + e); } } // if the filter didn't change, nothing is done diff --git a/src/storages/inLocalStorage/constants.ts b/src/storages/inLocalStorage/constants.ts index 74624310..8dcb6c3e 100644 --- a/src/storages/inLocalStorage/constants.ts +++ b/src/storages/inLocalStorage/constants.ts @@ -1,2 +1,2 @@ -export const logPrefix = 'storage:localstorage: '; +export const LOG_PREFIX = 'storage:localstorage: '; export const DEFINED = '1'; diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index 226d771c..a26587d1 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -10,7 +10,7 @@ import MySegmentsCacheInMemory from '../inMemory/MySegmentsCacheInMemory'; import SplitsCacheInMemory from '../inMemory/SplitsCacheInMemory'; import { DEFAULT_CACHE_EXPIRATION_IN_MILLIS } from '../../utils/constants/browser'; import { InMemoryStorageCSFactory } from '../inMemory/InMemoryStorageCS'; -import { logPrefix } from './constants'; +import { LOG_PREFIX } from './constants'; export interface InLocalStorageOptions { prefix?: string @@ -27,7 +27,7 @@ export function InLocalStorage(options: InLocalStorageOptions = {}) { // Fallback to InMemoryStorage if LocalStorage API is not available if (!isLocalStorageAvailable()) { - params.log.warn(logPrefix + 'LocalStorage API is unavailable. Fallbacking into default MEMORY storage'); + params.log.warn(LOG_PREFIX + 'LocalStorage API is unavailable. Fallbacking into default MEMORY storage'); return InMemoryStorageCSFactory(params); } diff --git a/src/storages/inMemory/SplitsCacheInMemory.ts b/src/storages/inMemory/SplitsCacheInMemory.ts index 5388d2a4..39eefdcf 100644 --- a/src/storages/inMemory/SplitsCacheInMemory.ts +++ b/src/storages/inMemory/SplitsCacheInMemory.ts @@ -57,7 +57,7 @@ export default class SplitsCacheInMemory extends AbstractSplitsCacheSync { } } - removeSplit(name: string): number { + removeSplit(name: string): boolean { const split = this.getSplit(name); if (split) { // Delete the Split @@ -74,9 +74,9 @@ export default class SplitsCacheInMemory extends AbstractSplitsCacheSync { // Update the segments count. if (usesSegments(parsedSplit)) this.splitsWithSegmentsCount--; - return 1; + return true; } else { - return 0; + return false; } } diff --git a/src/storages/inRedis/EventsCacheInRedis.ts b/src/storages/inRedis/EventsCacheInRedis.ts index 5a8caab5..97f7d13f 100644 --- a/src/storages/inRedis/EventsCacheInRedis.ts +++ b/src/storages/inRedis/EventsCacheInRedis.ts @@ -4,8 +4,7 @@ import KeyBuilderSS from '../KeyBuilderSS'; import { Redis } from 'ioredis'; import { SplitIO } from '../../types'; import { ILogger } from '../../logger/types'; - -const logPrefix = 'storage:redis: '; +import { LOG_PREFIX } from './constants'; export default class EventsCacheInRedis implements IEventsCacheAsync { @@ -33,7 +32,7 @@ export default class EventsCacheInRedis implements IEventsCacheAsync { // We use boolean values to signal successful queueing .then(() => true) .catch(err => { - this.log.error(logPrefix + `Error adding event to queue: ${err}.`); + this.log.error(LOG_PREFIX + `Error adding event to queue: ${err}.`); return false; }); } diff --git a/src/storages/inRedis/RedisAdapter.ts b/src/storages/inRedis/RedisAdapter.ts index 7d0636bc..de0abee3 100644 --- a/src/storages/inRedis/RedisAdapter.ts +++ b/src/storages/inRedis/RedisAdapter.ts @@ -5,7 +5,7 @@ import { _Set, setToArray, ISet } from '../../utils/lang/sets'; import thenable from '../../utils/promise/thenable'; import timeout from '../../utils/promise/timeout'; -const logPrefix = 'storage:redis-adapter: '; +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', 'pipeline', 'expire', 'mget']; @@ -55,16 +55,16 @@ export default class RedisAdapter extends ioredis { _listenToEvents() { this.once('ready', () => { const commandsCount = this._notReadyCommandsQueue ? this._notReadyCommandsQueue.length : 0; - this.log.info(logPrefix + `Redis connection established. Queued commands: ${commandsCount}.`); + this.log.info(LOG_PREFIX + `Redis connection established. Queued commands: ${commandsCount}.`); this._notReadyCommandsQueue && this._notReadyCommandsQueue.forEach(queued => { - this.log.info(logPrefix + `Executing queued ${queued.name} command.`); + this.log.info(LOG_PREFIX + `Executing queued ${queued.name} command.`); queued.command().then(queued.resolve).catch(queued.reject); }); // After the SDK is ready for the first time we'll stop queueing commands. This is just so we can keep handling BUR for them. this._notReadyCommandsQueue = undefined; }); this.once('close', () => { - this.log.info(logPrefix + 'Redis connection closed.'); + this.log.info(LOG_PREFIX + 'Redis connection closed.'); }); } @@ -78,7 +78,7 @@ export default class RedisAdapter extends ioredis { const params = arguments; function commandWrapper() { - instance.log.debug(logPrefix + `Executing ${method}.`); + instance.log.debug(LOG_PREFIX + `Executing ${method}.`); // Return original method const result = originalMethod.apply(instance, params); @@ -93,7 +93,7 @@ export default class RedisAdapter extends ioredis { result.then(cleanUpRunningCommandsCb, cleanUpRunningCommandsCb); return timeout(instance._options.operationTimeout, result).catch(err => { - instance.log.error(logPrefix + `${method} operation threw an error or exceeded configured timeout of ${instance._options.operationTimeout}ms. Message: ${err}`); + instance.log.error(LOG_PREFIX + `${method} operation threw an error or exceeded configured timeout of ${instance._options.operationTimeout}ms. Message: ${err}`); // Handling is not the adapter responsibility. throw err; }); @@ -126,19 +126,19 @@ export default class RedisAdapter extends ioredis { setTimeout(function deferedDisconnect() { if (instance._runningCommands.size > 0) { - instance.log.info(logPrefix + `Attempting to disconnect but there are ${instance._runningCommands.size} commands still waiting for resolution. Defering disconnection until those finish.`); + instance.log.info(LOG_PREFIX + `Attempting to disconnect but there are ${instance._runningCommands.size} commands still waiting for resolution. Defering disconnection until those finish.`); Promise.all(setToArray(instance._runningCommands)) .then(() => { - instance.log.debug(logPrefix + 'Pending commands finished successfully, disconnecting.'); + instance.log.debug(LOG_PREFIX + 'Pending commands finished successfully, disconnecting.'); originalMethod.apply(instance, params); }) .catch(e => { - instance.log.warn(logPrefix + `Pending commands finished with error: ${e}. Proceeding with disconnection.`); + instance.log.warn(LOG_PREFIX + `Pending commands finished with error: ${e}. Proceeding with disconnection.`); originalMethod.apply(instance, params); }); } else { - instance.log.debug(logPrefix + 'No commands pending execution, disconnect.'); + instance.log.debug(LOG_PREFIX + 'No commands pending execution, disconnect.'); // Nothing pending, just proceed. originalMethod.apply(instance, params); } diff --git a/src/storages/inRedis/SegmentsCacheInRedis.ts b/src/storages/inRedis/SegmentsCacheInRedis.ts index e4b23ccd..73955761 100644 --- a/src/storages/inRedis/SegmentsCacheInRedis.ts +++ b/src/storages/inRedis/SegmentsCacheInRedis.ts @@ -53,6 +53,7 @@ export default class SegmentsCacheInRedis implements ISegmentsCacheAsync { }); } + // @TODO remove: not used and not part of interface registerSegment(segment: string) { return this.registerSegments([segment]); } diff --git a/src/storages/inRedis/SplitsCacheInRedis.ts b/src/storages/inRedis/SplitsCacheInRedis.ts index c601b549..d3c8d082 100644 --- a/src/storages/inRedis/SplitsCacheInRedis.ts +++ b/src/storages/inRedis/SplitsCacheInRedis.ts @@ -3,8 +3,7 @@ import KeyBuilderSS from '../KeyBuilderSS'; import { ISplitsCacheAsync } from '../types'; import { Redis } from 'ioredis'; import { ILogger } from '../../logger/types'; - -const logPrefix = 'storage:redis: '; +import { LOG_PREFIX } from './constants'; /** * Discard errors for an answer of multiple operations. @@ -17,16 +16,18 @@ function processPipelineAnswer(results: Array<[Error | null, string]>): string[] } /** - * Default ISplitsCacheSync implementation that stores split definitions in memory. - * Supported by all JS runtimes. + * ISplitsCacheAsync implementation that stores split definitions in Redis. + * Supported by Node. */ export default class SplitsCacheInRedis implements ISplitsCacheAsync { + private readonly log: ILogger; private readonly redis: Redis; private readonly keys: KeyBuilderSS; private redisError?: string; - constructor(private readonly log: ILogger, keys: KeyBuilderSS, redis: Redis) { + constructor(log: ILogger, keys: KeyBuilderSS, redis: Redis) { + this.log = log; this.redis = redis; this.keys = keys; @@ -39,6 +40,7 @@ export default class SplitsCacheInRedis implements ISplitsCacheAsync { }); } + // @TODO fix: incr/decr TT and segments for producer mode. Follow pluggable storage signature addSplit(name: string, split: string): Promise { return this.redis.set( this.keys.buildSplitKey(name), split @@ -47,6 +49,7 @@ export default class SplitsCacheInRedis implements ISplitsCacheAsync { ); } + // @TODO fix: incr/decr TT and segments for producer mode. Follow pluggable storage signature addSplits(entries: [string, string][]): Promise { if (entries.length) { const cmds = entries.map(keyValuePair => ['set', this.keys.buildSplitKey(keyValuePair[0]), keyValuePair[1]]); @@ -60,17 +63,22 @@ export default class SplitsCacheInRedis implements ISplitsCacheAsync { } } + // @TODO implement for producer mode. Follow pluggable storage signature + killLocally(): Promise { + throw new Error('Method not implemented.'); + } + /** * Remove a given split from Redis. Returns the number of deleted keys. */ - removeSplit(name: string): Promise { + removeSplit(name: string): Promise { return this.redis.del(this.keys.buildSplitKey(name)); } /** * Bulk delete of splits from Redis. Returns the number of deleted keys. */ - removeSplits(names: string[]): Promise { + removeSplits(names: string[]): Promise { if (names.length) { return this.redis.del(names.map(n => this.keys.buildSplitKey(n))); } else { @@ -80,12 +88,13 @@ export default class SplitsCacheInRedis implements ISplitsCacheAsync { /** * Get split definition or null if it's not defined. + * Returned promise is rejected if redis operation fails. */ getSplit(name: string): Promise { if (this.redisError) { - this.log.error(logPrefix + this.redisError); + this.log.error(LOG_PREFIX + this.redisError); - throw this.redisError; + return Promise.reject(this.redisError); // no need to wrap as an SplitError } return this.redis.get(this.keys.buildSplitKey(name)); @@ -103,7 +112,7 @@ export default class SplitsCacheInRedis implements ISplitsCacheAsync { } /** - * Get till number or null if it's not defined. + * Get till number or -1 if it's not defined. * * @TODO pending error handling */ @@ -135,16 +144,18 @@ export default class SplitsCacheInRedis implements ISplitsCacheAsync { // If there is a number there should be > 0, otherwise the TT is considered as not existent. return this.redis.get(this.keys.buildTrafficTypeKey(trafficType)) .then((ttCount: string | null | number) => { + if (ttCount === null) return false; // if entry doesn't exist, means that TT doesn't exist + ttCount = parseInt(ttCount as string, 10); if (!isFiniteNumber(ttCount) || ttCount < 0) { - this.log.info(logPrefix + `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 existance of ${trafficType} due to data corruption of some sorts.`); return false; } return ttCount > 0; }) .catch(e => { - this.log.error(logPrefix + `Could not validate traffic type existance of ${trafficType} due to an error: ${e}.`); + this.log.error(LOG_PREFIX + `Could not validate traffic type existance of ${trafficType} due to an error: ${e}.`); // If there is an error, bypass the validation so the event can get tracked. return true; }); @@ -166,13 +177,15 @@ export default class SplitsCacheInRedis implements ISplitsCacheAsync { /** * Fetches multiple splits definitions. + * Returned promise is rejected if redis operation fails. */ getSplits(names: string[]): Promise> { if (this.redisError) { - this.log.error(logPrefix + this.redisError); + this.log.error(LOG_PREFIX + this.redisError); - throw this.redisError; + return Promise.reject(this.redisError); // no need to wrap as an SplitError } + const splits: Record = {}; const keys = names.map(name => this.keys.buildSplitKey(name)); return this.redis.mget(...keys) @@ -183,7 +196,7 @@ export default class SplitsCacheInRedis implements ISplitsCacheAsync { return Promise.resolve(splits); }) .catch(e => { - this.log.error(logPrefix + `Could not grab splits due to an error: ${e}.`); + this.log.error(LOG_PREFIX + `Could not grab splits due to an error: ${e}.`); return Promise.reject(e); }); } diff --git a/src/storages/inRedis/__tests__/EventsCacheInRedis.spec.ts b/src/storages/inRedis/__tests__/EventsCacheInRedis.spec.ts index c4384726..f5831481 100644 --- a/src/storages/inRedis/__tests__/EventsCacheInRedis.spec.ts +++ b/src/storages/inRedis/__tests__/EventsCacheInRedis.spec.ts @@ -4,7 +4,7 @@ import isEqual from 'lodash/isEqual'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; import KeyBuilderSS from '../../KeyBuilderSS'; import EventsCacheInRedis from '../EventsCacheInRedis'; -import { metadataBuilder } from '../index'; +import { metadataBuilder } from '../../metadataBuilder'; const prefix = 'events_cache_ut'; const metadata = { version: 'js_someversion', ip: 'some_ip', hostname: 'some_hostname' }; diff --git a/src/storages/inRedis/__tests__/RedisAdapter.spec.ts b/src/storages/inRedis/__tests__/RedisAdapter.spec.ts index ba369d8c..9557162d 100644 --- a/src/storages/inRedis/__tests__/RedisAdapter.spec.ts +++ b/src/storages/inRedis/__tests__/RedisAdapter.spec.ts @@ -6,7 +6,7 @@ import { _Set, setToArray } from '../../../utils/lang/sets'; // Mocking sdkLogger import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; -const logPrefix = 'storage:redis-adapter: '; +const LOG_PREFIX = 'storage:redis-adapter: '; // Mocking ioredis @@ -188,7 +188,7 @@ describe('STORAGE Redis Adapter', () => { secondCallArgs[1](); // Execute the callback for "close" expect(loggerMock.info).toBeCalledTimes(1); // The callback for the "close" event will only log info to the user about what is going on. - expect(loggerMock.info.mock.calls[0]).toEqual([logPrefix + 'Redis connection closed.']); // The callback for the "close" event will only log info to the user about what is going on. + expect(loggerMock.info.mock.calls[0]).toEqual([LOG_PREFIX + 'Redis connection closed.']); // The callback for the "close" event will only log info to the user about what is going on. loggerMock.info.mockClear(); expect(loggerMock.info).not.toBeCalled(); // Control assertion @@ -198,7 +198,7 @@ describe('STORAGE Redis Adapter', () => { firstCallArgs[1](); expect(loggerMock.info).toBeCalledTimes(1); // The callback for the "ready" event will inform the user about the trigger. - expect(loggerMock.info).toBeCalledWith(logPrefix + 'Redis connection established. Queued commands: 0.'); // The callback for the "ready" event will inform the user about the trigger. + expect(loggerMock.info).toBeCalledWith(LOG_PREFIX + 'Redis connection established. Queued commands: 0.'); // The callback for the "ready" event will inform the user about the trigger. expect(instance._notReadyCommandsQueue).toBe(undefined); // After the DB is ready, it will clean up the offline commands queue so we do not queue commands anymore. // Don't do this at home @@ -222,9 +222,9 @@ describe('STORAGE Redis Adapter', () => { expect(loggerMock.info).toBeCalledTimes(3); // If we had queued commands, it will log the event (1 call) as well as each executed command (n calls). expect(loggerMock.info.mock.calls).toEqual([ - [logPrefix + 'Redis connection established. Queued commands: 2.'], // The callback for the "ready" event will inform the user about the trigger and the amount of queued commands. - [logPrefix + 'Executing queued GET command.'], // If we had queued, it will log the event as well as each executed command. - [logPrefix + 'Executing queued SET command.'] // If we had queued commands, it will log the event as well as each executed command. + [LOG_PREFIX + 'Redis connection established. Queued commands: 2.'], // The callback for the "ready" event will inform the user about the trigger and the amount of queued commands. + [LOG_PREFIX + 'Executing queued GET command.'], // If we had queued, it will log the event as well as each executed command. + [LOG_PREFIX + 'Executing queued SET command.'] // If we had queued commands, it will log the event as well as each executed command. ]); expect(queuedGetCommand.command).toBeCalledTimes(1); // It will execute each queued command. @@ -287,7 +287,7 @@ describe('STORAGE Redis Adapter', () => { commandTimeoutResolver.rej('test'); setTimeout(() => { // Allow the promises to tick. expect(instance._runningCommands.has(commandTimeoutResolver.originalPromise)).toBe(false); // After a command finishes with error, it's promise is removed from the instance._runningCommands queue. - expect(loggerMock.error.mock.calls[index]).toEqual([`${logPrefix}${methodName} operation threw an error or exceeded configured timeout of 5000ms. Message: test`]); // The log error method should be called with the corresponding messages, depending on the method, error and operationTimeout. + expect(loggerMock.error.mock.calls[index]).toEqual([`${LOG_PREFIX}${methodName} operation threw an error or exceeded configured timeout of 5000ms. Message: test`]); // The log error method should be called with the corresponding messages, depending on the method, error and operationTimeout. }, 0); }); @@ -334,7 +334,7 @@ describe('STORAGE Redis Adapter', () => { expect(ioredisMock.disconnect).not.toBeCalled(); // Original method should not be called right away. setTimeout(() => { // o queued commands timeout wrapper. - expect(loggerMock.debug.mock.calls).toEqual([[logPrefix + 'No commands pending execution, disconnect.']]); + expect(loggerMock.debug.mock.calls).toEqual([[LOG_PREFIX + 'No commands pending execution, disconnect.']]); expect(ioredisMock.disconnect).toBeCalledTimes(1); // Original method should have been called once, asynchronously loggerMock.debug.mockClear(); ioredisMock.disconnect.mockClear(); @@ -347,11 +347,11 @@ describe('STORAGE Redis Adapter', () => { rejectedPromise.catch(() => { }); // Swallow the unhandled to avoid unhandledRejection warns setTimeout(() => { // queued with rejection timeout wrapper - expect(loggerMock.info.mock.calls).toEqual([[logPrefix + 'Attempting to disconnect but there are 2 commands still waiting for resolution. Defering disconnection until those finish.']]); + expect(loggerMock.info.mock.calls).toEqual([[LOG_PREFIX + 'Attempting to disconnect but there are 2 commands still waiting for resolution. Defering disconnection until those finish.']]); Promise.all(setToArray(instance._runningCommands)).catch(e => { setImmediate(() => { // Allow the callback to execute before checking. - expect(loggerMock.warn.mock.calls[0]).toEqual([`${logPrefix}Pending commands finished with error: ${e}. Proceeding with disconnection.`]); // Should warn about the error but tell user that will disconnect anyways. + expect(loggerMock.warn.mock.calls[0]).toEqual([`${LOG_PREFIX}Pending commands finished with error: ${e}. Proceeding with disconnection.`]); // Should warn about the error but tell user that will disconnect anyways. expect(ioredisMock.disconnect).toBeCalledTimes(1); // Original method should have been called once, asynchronously loggerMock.info.mockClear(); @@ -367,11 +367,11 @@ describe('STORAGE Redis Adapter', () => { instance._runningCommands.add(Promise.resolve()); setTimeout(() => { - expect(loggerMock.info.mock.calls).toEqual([[logPrefix + 'Attempting to disconnect but there are 4 commands still waiting for resolution. Defering disconnection until those finish.']]); + expect(loggerMock.info.mock.calls).toEqual([[LOG_PREFIX + 'Attempting to disconnect but there are 4 commands still waiting for resolution. Defering disconnection until those finish.']]); Promise.all(setToArray(instance._runningCommands)).then(() => { // This one will go through success path setImmediate(() => { - expect(loggerMock.debug.mock.calls).toEqual([[logPrefix + 'Pending commands finished successfully, disconnecting.']]); + expect(loggerMock.debug.mock.calls).toEqual([[LOG_PREFIX + 'Pending commands finished successfully, disconnecting.']]); expect(ioredisMock.disconnect).toBeCalledTimes(1); // Original method should have been called once, asynchronously }); }); diff --git a/src/storages/inRedis/__tests__/index.spec.ts b/src/storages/inRedis/__tests__/index.spec.ts index fa62acf3..2ac012f1 100644 --- a/src/storages/inRedis/__tests__/index.spec.ts +++ b/src/storages/inRedis/__tests__/index.spec.ts @@ -1,4 +1,4 @@ -import { metadataBuilder } from '../index'; +import { metadataBuilder } from '../../metadataBuilder'; import { UNKNOWN } from '../../../utils/constants'; import { IMetadata } from '../../../dtos/types'; diff --git a/src/storages/inRedis/constants.ts b/src/storages/inRedis/constants.ts new file mode 100644 index 00000000..a3856844 --- /dev/null +++ b/src/storages/inRedis/constants.ts @@ -0,0 +1 @@ +export const LOG_PREFIX = 'storage:redis: '; diff --git a/src/storages/inRedis/index.ts b/src/storages/inRedis/index.ts index 52093236..935cdf67 100644 --- a/src/storages/inRedis/index.ts +++ b/src/storages/inRedis/index.ts @@ -1,6 +1,5 @@ import RedisAdapter from './RedisAdapter'; import { IStorageAsync, IStorageFactoryParams } from '../types'; -import { IMetadata, IRedisMetadata } from '../../dtos/types'; import KeyBuilderSS from '../KeyBuilderSS'; import SplitsCacheInRedis from './SplitsCacheInRedis'; import SegmentsCacheInRedis from './SegmentsCacheInRedis'; @@ -8,23 +7,14 @@ import ImpressionsCacheInRedis from './ImpressionsCacheInRedis'; import EventsCacheInRedis from './EventsCacheInRedis'; import LatenciesCacheInRedis from './LatenciesCacheInRedis'; import CountsCacheInRedis from './CountsCacheInRedis'; -import { UNKNOWN } from '../../utils/constants'; import { SDK_SPLITS_ARRIVED, SDK_SEGMENTS_ARRIVED } from '../../readiness/constants'; +import { metadataBuilder } from '../metadataBuilder'; export interface InRedisStorageOptions { prefix?: string options?: Record } -// exported for testing purposes -export function metadataBuilder(metadata: IMetadata): IRedisMetadata { - return { - s: metadata.version, - i: metadata.ip || UNKNOWN, - n: metadata.hostname || UNKNOWN, - }; -} - /** * InRedis storage factory for consumer server-side SplitFactory, that uses `Ioredis` Redis client for Node. * @see {@link https://www.npmjs.com/package/ioredis} diff --git a/src/storages/metadataBuilder.ts b/src/storages/metadataBuilder.ts new file mode 100644 index 00000000..ca75572d --- /dev/null +++ b/src/storages/metadataBuilder.ts @@ -0,0 +1,10 @@ +import { IMetadata, IRedisMetadata } from '../dtos/types'; +import { UNKNOWN } from '../utils/constants'; + +export function metadataBuilder(metadata: IMetadata): IRedisMetadata { + return { + s: metadata.version, + i: metadata.ip || UNKNOWN, + n: metadata.hostname || UNKNOWN, + }; +} diff --git a/src/storages/pluggable/EventsCachePluggable.ts b/src/storages/pluggable/EventsCachePluggable.ts new file mode 100644 index 00000000..2af117d7 --- /dev/null +++ b/src/storages/pluggable/EventsCachePluggable.ts @@ -0,0 +1,48 @@ +import { ICustomStorageWrapper, IEventsCacheAsync } from '../types'; +import { IRedisMetadata } from '../../dtos/types'; +import KeyBuilderSS from '../KeyBuilderSS'; +import { SplitIO } from '../../types'; +import { ILogger } from '../../logger/types'; +import { LOG_PREFIX } from './constants'; + +export class EventsCachePluggable implements IEventsCacheAsync { + + private readonly log: ILogger; + private readonly wrapper: ICustomStorageWrapper; + private readonly keys: KeyBuilderSS; + private readonly metadata: IRedisMetadata; + + constructor(log: ILogger, keys: KeyBuilderSS, wrapper: ICustomStorageWrapper, metadata: IRedisMetadata) { + this.log = log; + this.keys = keys; + this.wrapper = wrapper; + this.metadata = metadata; + } + + /** + * Push given event to the storage. + * @param eventData Event item to push. + * @returns A promise that is resolved with a boolean value indicating if the push operation succeeded or failed. + * The promise will never be rejected. + */ + track(eventData: SplitIO.EventData): Promise { + return this.wrapper.pushItems( + this.keys.buildEventsKey(), + [this._toJSON(eventData)] + ) + // We use boolean values to signal successful queueing + .then(() => true) + .catch(e => { + this.log.error(LOG_PREFIX + ` Error adding event to queue: ${e}.`); + return false; + }); + } + + private _toJSON(eventData: SplitIO.EventData): string { + return JSON.stringify({ + m: this.metadata, + e: eventData + }); + } + +} diff --git a/src/storages/pluggable/ImpressionsCachePluggable.ts b/src/storages/pluggable/ImpressionsCachePluggable.ts new file mode 100644 index 00000000..31436c43 --- /dev/null +++ b/src/storages/pluggable/ImpressionsCachePluggable.ts @@ -0,0 +1,64 @@ +import { ICustomStorageWrapper, IImpressionsCacheAsync } from '../types'; +import { IRedisMetadata } from '../../dtos/types'; +import { ImpressionDTO } from '../../types'; +import KeyBuilderSS from '../KeyBuilderSS'; +import { ILogger } from '../../logger/types'; +import { LOG_PREFIX } from './constants'; + +export class ImpressionsCachePluggable implements IImpressionsCacheAsync { + + private readonly log: ILogger; + private readonly keys: KeyBuilderSS; + private readonly wrapper: ICustomStorageWrapper; + private readonly metadata: IRedisMetadata; + + constructor(log: ILogger, keys: KeyBuilderSS, wrapper: ICustomStorageWrapper, metadata: IRedisMetadata) { + this.log = log; + this.keys = keys; + this.wrapper = wrapper; + this.metadata = metadata; + } + + /** + * Push given impressions to the storage. + * @param impressions List of impresions to push. + * @returns A promise that is resolved with a boolean value indicating if the push operation succeeded or failed. + * The promise will never be rejected. + */ + track(impressions: ImpressionDTO[]): Promise { + return this.wrapper.pushItems( + this.keys.buildImpressionsKey(), + this._toJSON(impressions) + ) + // We use boolean values to signal successful queueing + .then(() => true) + .catch((e) => { + this.log.error(LOG_PREFIX + ` Error adding event to queue: ${e}.`); + return false; + }); + } + + private _toJSON(impressions: ImpressionDTO[]): string[] { + return impressions.map(impression => { + const { + keyName, bucketingKey, feature, treatment, label, time, changeNumber + } = impression; + + return JSON.stringify({ + m: this.metadata, + i: { + k: keyName, + b: bucketingKey, + f: feature, + t: treatment, + r: label, + c: changeNumber, + m: time + } + }); + }); + } + + // @TODO implement producer methods + +} diff --git a/src/storages/pluggable/SegmentsCachePluggable.ts b/src/storages/pluggable/SegmentsCachePluggable.ts new file mode 100644 index 00000000..612b5ff0 --- /dev/null +++ b/src/storages/pluggable/SegmentsCachePluggable.ts @@ -0,0 +1,93 @@ +/* eslint-disable @typescript-eslint/no-unused-vars */ +/* eslint-disable no-unused-vars */ +import { isNaNNumber } from '../../utils/lang'; +import KeyBuilder from '../KeyBuilder'; +import { ICustomStorageWrapper, ISegmentsCacheAsync } from '../types'; +import { ILogger } from '../../logger/types'; +import { LOG_PREFIX } from './constants'; + +/** + * ISegmentsCacheAsync implementation for pluggable storages. + */ +export class SegmentsCachePluggable implements ISegmentsCacheAsync { + + private readonly log: ILogger; + private readonly keys: KeyBuilder; + private readonly wrapper: ICustomStorageWrapper; + + constructor(log: ILogger, keys: KeyBuilder, wrapper: ICustomStorageWrapper) { + this.log = log; + this.keys = keys; + this.wrapper = wrapper; + } + + /** + * Returns if `key` is part of `name` segment. + */ + isInSegment(name: string, key: string) { + return this.wrapper.itemContains(this.keys.buildSegmentNameKey(name), key); + } + + /** + * Set till number for the given segment `name`. + * The returned promise is resolved when the operation success, + * or rejected with an SplitError if it fails (e.g., wrapper operation fails). + */ + setChangeNumber(name: string, changeNumber: number) { + return this.wrapper.set( + this.keys.buildSegmentTillKey(name), changeNumber + '' + ); + } + + /** + * Get till number or -1 if it's not defined. + * The returned promise is resolved with the changeNumber or -1 if it doesn't exist or a wrapper operation fails. + * The promise will never be rejected. + */ + getChangeNumber(name: string) { + return this.wrapper.get(this.keys.buildSegmentTillKey(name)).then((value: string | null) => { + const i = parseInt(value as string, 10); + return isNaNNumber(i) ? -1 : i; + }).catch((e) => { + this.log.error(LOG_PREFIX + 'Could not retrieve changeNumber from segments storage. Error: ' + e); + return -1; + }); + } + + /** + * Add a list of `segmentKeys` to the given segment `name`. + * The returned promise is resolved when the operation success + * or rejected with an SplitError if it fails (e.g., wrapper operation fails, JSON parsing error) + * + * @TODO implement for DataLoader/Producer mode + */ + addToSegment(name: string, segmentKeys: string[]) { + return Promise.resolve(true); + } + + /** + * Remove a list of `segmentKeys` from the given segment `name`. + * The returned promise is resolved when the operation success + * or rejected with an SplitError if it fails (e.g., wrapper operation fails, JSON parsing error) + * + * @TODO implement for DataLoader/Producer mode + */ + removeFromSegment(name: string, segmentKeys: string[]) { + return Promise.resolve(true); + } + + /** @TODO implement for DataLoader/Producer mode */ + registerSegments(names: string[]) { + return Promise.resolve(true); + } + + /** @TODO implement for DataLoader/Producer mode */ + getRegisteredSegments() { + return Promise.resolve([]); + } + + /** @TODO implement for DataLoader/Producer mode */ + clear(): Promise { + return Promise.resolve(true); + } +} diff --git a/src/storages/pluggable/SplitsCachePluggable.ts b/src/storages/pluggable/SplitsCachePluggable.ts new file mode 100644 index 00000000..9362aaab --- /dev/null +++ b/src/storages/pluggable/SplitsCachePluggable.ts @@ -0,0 +1,274 @@ +import { isFiniteNumber, toNumber, isNaNNumber } from '../../utils/lang'; +import KeyBuilder from '../KeyBuilder'; +import { ICustomStorageWrapper, ISplitsCacheAsync } from '../types'; +import { ILogger } from '../../logger/types'; +import { usesSegments } from '../AbstractSplitsCacheSync'; +import { ISplit } from '../../dtos/types'; +import { LOG_PREFIX } from './constants'; +import { SplitError } from '../../utils/lang/errors'; + +/** + * ISplitsCacheAsync implementation for pluggable storages. + */ +export class SplitsCachePluggable implements ISplitsCacheAsync { + + private readonly log: ILogger; + private readonly keys: KeyBuilder; + private readonly wrapper: ICustomStorageWrapper; + + /** + * Create a SplitsCache that uses a custom storage wrapper. + * @param log Logger instance. + * @param keys Key builder. + * @param wrapper Adapted wrapper storage. + */ + constructor(log: ILogger, keys: KeyBuilder, wrapper: ICustomStorageWrapper) { + this.log = log; + this.keys = keys; + this.wrapper = wrapper; + } + + private _decrementCounts(split: ISplit) { + const promises = []; + if (split.trafficTypeName) { + const ttKey = this.keys.buildTrafficTypeKey(split.trafficTypeName); + promises.push(this.wrapper.decr(ttKey)); + } + + if (usesSegments(split)) { + const segmentsCountKey = this.keys.buildSplitsWithSegmentCountKey(); + promises.push(this.wrapper.decr(segmentsCountKey)); + } + + return Promise.all(promises); + } + + private _incrementCounts(split: ISplit) { + const promises = []; + if (split.trafficTypeName) { + const ttKey = this.keys.buildTrafficTypeKey(split.trafficTypeName); + promises.push(this.wrapper.incr(ttKey)); + } + + if (usesSegments(split)) { + const segmentsCountKey = this.keys.buildSplitsWithSegmentCountKey(); + promises.push(this.wrapper.incr(segmentsCountKey)); + } + + return Promise.all(promises); + } + + /** + * Add a given split. + * The returned promise is resolved when the operation success + * or rejected with an SplitError if it fails (e.g., wrapper operation fails) + */ + addSplit(name: string, split: string): Promise { + const splitKey = this.keys.buildSplitKey(name); + return this.wrapper.get(splitKey).then(splitFromStorage => { + + // handling parsing error as SplitErrors + let parsedPreviousSplit, parsedSplit; + try { + parsedPreviousSplit = splitFromStorage ? JSON.parse(splitFromStorage) : undefined; + parsedSplit = JSON.parse(split); + } catch (e) { + throw new SplitError('Error parsing split definition: ' + e); + } + + return Promise.all([ + // If it's an update, we decrement the traffic type and segment count of the existing split, + parsedPreviousSplit && this._decrementCounts(parsedPreviousSplit), + this.wrapper.set(splitKey, split), + this._incrementCounts(parsedSplit) + ]); + }).then(() => true); + } + + /** + * Add a list of splits. + * The returned promise is resolved when the operation success + * or rejected with an SplitError if it fails (e.g., wrapper operation fails) + */ + addSplits(entries: [string, string][]): Promise { + return Promise.all(entries.map(keyValuePair => this.addSplit(keyValuePair[0], keyValuePair[1]))); + } + + /** + * Remove a given split. + * The returned promise is resolved when the operation success, with a boolean indicating if the split existed or not. + * or rejected with an SplitError if it fails (e.g., wrapper operation fails). + */ + removeSplit(name: string): Promise { + return this.getSplit(name).then((split) => { + if (split) { + const parsedSplit = JSON.parse(split); + this._decrementCounts(parsedSplit); + } + return this.wrapper.del(this.keys.buildSplitKey(name)); + }); + } + + /** + * Remove a list of splits. + * The returned promise is resolved when the operation success, with a boolean array indicating if the splits existed or not. + * or rejected with an SplitError if it fails (e.g., wrapper operation fails). + */ + removeSplits(names: string[]): Promise { + return Promise.all(names.map(name => this.removeSplit(name))); + } + + /** + * Get split. + * The returned promise is resolved with the split definition or null if it's not defined, + * or rejected with an SplitError if wrapper operation fails. + */ + getSplit(name: string): Promise { + return this.wrapper.get(this.keys.buildSplitKey(name)); + } + + /** + * Get list of splits. + * The returned promise is resolved with a map of split names to their split definition or null if it's not defined, + * or rejected with an SplitError if wrapper operation fails. + */ + getSplits(names: string[]): Promise> { + const keys = names.map(name => this.keys.buildSplitKey(name)); + + return this.wrapper.getMany(keys).then(splitDefinitions => { + const splits: Record = {}; + names.forEach((name, idx) => { + splits[name] = splitDefinitions[idx]; + }); + return Promise.resolve(splits); + }); + } + + /** + * Get list of all split definitions. + * The returned promise is resolved with the list of split definitions, + * or rejected with an SplitError if wrapper operation fails. + */ + getAll(): Promise { + return this.wrapper.getKeysByPrefix(this.keys.buildSplitKeyPrefix()).then( + (listOfKeys) => Promise.all(listOfKeys.map(this.wrapper.get) as Promise[]) + ); + } + + /** + * Get list of split names. + * The returned promise is resolved with the list of split names, + * or rejected with an SplitError if wrapper operation fails. + */ + getSplitNames(): Promise { + return this.wrapper.getKeysByPrefix(this.keys.buildSplitKeyPrefix()).then( + (listOfKeys) => listOfKeys.map(this.keys.extractKey) + ); + } + + /** + * Check traffic type existence. + * The returned promise is resolved with a boolean indicating whether the TT exist or not. + * In case of wrapper operation failures, the promise resolves with a true value, assuming that the TT might exist. + * It will never be rejected. + */ + trafficTypeExists(trafficType: string): Promise { + // If there is a number there should be > 0, otherwise the TT is considered as not existent. + return this.wrapper.get(this.keys.buildTrafficTypeKey(trafficType)) + .then((ttCount: string | null | number) => { + if (ttCount === null) return false; // if entry doesn't exist, means that TT doesn't exist + + ttCount = parseInt(ttCount as string, 10); + if (!isFiniteNumber(ttCount) || ttCount < 0) { + 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 existence of ${trafficType} due to an error: ${e}.`); + // If there is an error, bypass the validation so the event can get tracked. + return true; + }); + } + + /** + * Set till number. + * The returned promise is resolved when the operation success, + * or rejected with an SplitError if it fails (e.g., wrapper operation fails). + */ + setChangeNumber(changeNumber: number): Promise { + return this.wrapper.set(this.keys.buildSplitsTillKey(), changeNumber + ''); + } + + /** + * Get till number or -1 if it's not defined. + * The returned promise is resolved with the changeNumber or -1 if it doesn't exist or a wrapper operation fails. + * The promise will never be rejected. + */ + getChangeNumber(): Promise { + return this.wrapper.get(this.keys.buildSplitsTillKey()).then((value) => { + const i = parseInt(value as string, 10); + + return isNaNNumber(i) ? -1 : i; + }).catch((e) => { + this.log.error(LOG_PREFIX + 'Could not retrieve changeNumber from storage. Error: ' + e); + return -1; + }); + } + + // @TODO revisit segment-related methods ('usesSegments', 'getRegisteredSegments', 'registerSegments') + usesSegments(): Promise { + return this.wrapper.get(this.keys.buildSplitsWithSegmentCountKey()) + .then(storedCount => { + const splitsWithSegmentsCount = storedCount ? toNumber(storedCount) : 0; + if (isFiniteNumber(splitsWithSegmentsCount)) { + return splitsWithSegmentsCount > 0; + } else { + return true; // If stored valus is invalid, assume we need them. + } + }).catch(() => true); // If wrapper operation fails, assume we need them. + } + + // @TODO implement for DataLoader/Producer mode + clear(): Promise { + return Promise.resolve(true); + } + + /** + * Check if the splits information is already stored in cache. + * Noop, just keeping the interface. This is used by client-side implementations only. + */ + checkCache(): Promise { + return Promise.resolve(true); + } + + /** + * Kill `name` split and set `defaultTreatment` and `changeNumber`. + * Used for SPLIT_KILL push notifications. + * + * @param {string} name + * @param {string} defaultTreatment + * @param {number} changeNumber + * @returns {Promise} a promise that is resolved once the split kill operation is performed. The fulfillment value is a boolean: `true` if the kill success updating the split or `false` if no split is updated, + * for instance, if the `changeNumber` is old, or if the split is not found (e.g., `/splitchanges` hasn't been fetched yet), or if the storage fails to apply the update. + * The promise will never be rejected. + */ + killLocally(name: string, defaultTreatment: string, changeNumber: number): Promise { + return this.getSplit(name).then(split => { + + if (split) { + const parsedSplit: ISplit = JSON.parse(split); + if (!parsedSplit.changeNumber || parsedSplit.changeNumber < changeNumber) { + parsedSplit.killed = true; + parsedSplit.defaultTreatment = defaultTreatment; + parsedSplit.changeNumber = changeNumber; + const newSplit = JSON.stringify(parsedSplit); + + return this.addSplit(name, newSplit); + } + } + return false; + }).catch(() => false); + } +} diff --git a/src/storages/pluggable/__tests__/EventsCachePluggable.spec.ts b/src/storages/pluggable/__tests__/EventsCachePluggable.spec.ts new file mode 100644 index 00000000..dfac96f6 --- /dev/null +++ b/src/storages/pluggable/__tests__/EventsCachePluggable.spec.ts @@ -0,0 +1,77 @@ +import find from 'lodash/find'; +import isEqual from 'lodash/isEqual'; +import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; +import KeyBuilderSS from '../../KeyBuilderSS'; +import { EventsCachePluggable } from '../EventsCachePluggable'; +import { metadataBuilder } from '../../metadataBuilder'; +import { wrapperMockFactory } from './wrapper.mock'; +import { wrapperAdapter } from '../wrapperAdapter'; + +const prefix = 'events_cache_ut'; +const metadata = { version: 'js_someversion', ip: 'some_ip', hostname: 'some_hostname' }; +const keys = new KeyBuilderSS(prefix, metadata); +const key = keys.buildEventsKey(); + +const fakeRedisMetadata = metadataBuilder(metadata); +const fakeEvent1 = { event: 1 }; +const fakeEvent2 = { event: '2' }; +const fakeEvent3 = { event: null }; + +describe('PLUGGABLE EVENTS CACHE', () => { + + test('`track` method should push values into the pluggable storage', async () => { + const wrapperMock = wrapperMockFactory(); + const cache = new EventsCachePluggable(loggerMock, keys, wrapperMock, fakeRedisMetadata); + + // @ts-expect-error + expect(await cache.track(fakeEvent1)).toBe(true); // If the queueing operation was successful, it should resolve the returned promise with "true" + // @ts-expect-error + expect(await cache.track(fakeEvent2)).toBe(true); // If the queueing operation was successful, it should resolve the returned promise with "true" + // @ts-expect-error + expect(await cache.track(fakeEvent3)).toBe(true); // If the queueing operation was successful, it should resolve the returned promise with "true" + + const values = wrapperMock._cache[key]; + + expect(values.length).toBe(3); // After pushing we should have as many events as we have stored. + expect(typeof values[0]).toBe('string'); // All elements should be strings since those are stringified JSONs. + expect(typeof values[1]).toBe('string'); // All elements should be strings since those are stringified JSONs. + expect(typeof values[2]).toBe('string'); // All elements should be strings since those are stringified JSONs. + + const findMatchingElem = (event: any) => { + return find(values, elem => { + const parsedElem = JSON.parse(elem); + return isEqual(parsedElem.e, event) && isEqual(parsedElem.m, fakeRedisMetadata); + }); + }; + + /* If the elements are found, then the values are correct. */ + const foundEv1 = findMatchingElem(fakeEvent1); + const foundEv2 = findMatchingElem(fakeEvent2); + const foundEv3 = findMatchingElem(fakeEvent3); + expect(foundEv1).not.toBe(undefined); // stored events matched the values we are expecting. + expect(foundEv2).not.toBe(undefined); // stored events matched the values we are expecting. + expect(foundEv3).not.toBe(undefined); // stored events matched the values we are expecting. + + wrapperMock.mockClear(); + }); + + test('`track` method result should not reject if wrapper operation fails', async () => { + const wrapperMock = wrapperMockFactory(); + // @ts-ignore. I'll use a "bad" queue to force an exception with the pushItems wrapper operation. + wrapperMock._cache['non-list-key'] = 10; + // @ts-expect-error. wrapperMock is adapted this time to properly handle unexpected exceptions + const faultyCache = new EventsCachePluggable(loggerMock, { + buildEventsKey: () => 'non-list-key' + }, wrapperAdapter(loggerMock, wrapperMock), fakeRedisMetadata); + + // @ts-expect-error + expect(await faultyCache.track(fakeEvent1)).toBe(false); // If the queueing operation was NOT successful, it should resolve the returned promise with "false" instead of rejecting it. + // @ts-expect-error + expect(await faultyCache.track(fakeEvent2)).toBe(false); // If the queueing operation was NOT successful, it should resolve the returned promise with "false" instead of rejecting it. + // @ts-expect-error + expect(await faultyCache.track(fakeEvent3)).toBe(false); // If the queueing operation was NOT successful, it should resolve the returned promise with "false" instead of rejecting it. + + wrapperMock.mockClear(); + }); + +}); diff --git a/src/storages/pluggable/__tests__/ImpressionsCachePluggable.spec.ts b/src/storages/pluggable/__tests__/ImpressionsCachePluggable.spec.ts new file mode 100644 index 00000000..00a88d95 --- /dev/null +++ b/src/storages/pluggable/__tests__/ImpressionsCachePluggable.spec.ts @@ -0,0 +1,84 @@ + +import KeyBuilderSS from '../../KeyBuilderSS'; +import { ImpressionsCachePluggable } from '../ImpressionsCachePluggable'; +import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; +import { wrapperMock } from './wrapper.mock'; +import { IRedisMetadata } from '../../../dtos/types'; + +const prefix = 'impr_cache_ut'; +const impressionsKey = `${prefix}.impressions`; +const testMeta: IRedisMetadata = { i: 'some_ip', n: 'some_host', s: 'some_sdk_version' }; // @ts-ignore +const keys = new KeyBuilderSS(prefix, testMeta); + +const o1 = { + feature: 'test1', + keyName: 'facundo@split.io', + treatment: 'on', + time: Date.now(), + label: 'default rule', + changeNumber: 1 +}; + +const o2 = { + feature: 'test2', + keyName: 'pepep@split.io', + treatment: 'A', + time: Date.now(), + bucketingKey: '1234-5678', + label: 'is in segment', + changeNumber: 1 +}; + +const o3 = { + feature: 'test3', + keyName: 'pipiip@split.io', + treatment: 'B', + time: Date.now(), + label: 'default rule', + changeNumber: 1 +}; + +describe('PLUGGABLE IMPRESSIONS CACHE', () => { + + afterEach(() => { + loggerMock.mockClear(); + wrapperMock.mockClear(); + }); + + test('`track` method should push values into the pluggable storage', async () => { + const cache = new ImpressionsCachePluggable(loggerMock, keys, wrapperMock, testMeta); + + expect(await cache.track([o1])).toBe(true); // result should resolve to true + + let state = wrapperMock._cache[impressionsKey]; + expect(state.length).toBe(1); // impression should be stored + + expect(await cache.track([o2, o3])).toBe(true); + + state = wrapperMock._cache[impressionsKey]; + expect(state.length).toBe(3); + // This is testing both the track and the toJSON method. + expect(state[0]).toBe(JSON.stringify({ + m: testMeta, + i: { k: o1.keyName, f: o1.feature, t: o1.treatment, r: o1.label, c: o1.changeNumber, m: o1.time } + })); + expect(state[1]).toBe(JSON.stringify({ + m: testMeta, + i: { k: o2.keyName, b: o2.bucketingKey, f: o2.feature, t: o2.treatment, r: o2.label, c: o2.changeNumber, m: o2.time } + })); + expect(state[2]).toBe(JSON.stringify({ + m: testMeta, + i: { k: o3.keyName, f: o3.feature, t: o3.treatment, r: o3.label, c: o3.changeNumber, m: o3.time } + })); + + }); + + test('`track` method result should not reject if wrapper operation fails', async () => { + // make wrapper operation fail + wrapperMock.pushItems.mockImplementation(() => Promise.reject()); + const cache = new ImpressionsCachePluggable(loggerMock, keys, wrapperMock, testMeta); + + expect(await cache.track([o1])).toBe(false); // result should resolve to false + }); + +}); diff --git a/src/storages/pluggable/__tests__/SegmentsCachePluggable.spec.ts b/src/storages/pluggable/__tests__/SegmentsCachePluggable.spec.ts new file mode 100644 index 00000000..ad95f3ff --- /dev/null +++ b/src/storages/pluggable/__tests__/SegmentsCachePluggable.spec.ts @@ -0,0 +1,36 @@ +import { SegmentsCachePluggable } from '../SegmentsCachePluggable'; +import KeyBuilder from '../../KeyBuilder'; +import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; +import { wrapperMock } from './wrapper.mock'; + +const keyBuilder = new KeyBuilder(); + +describe('SEGMENTS CACHE PLUGGABLE', () => { + + afterEach(() => { + loggerMock.mockClear(); + wrapperMock.mockClear(); + }); + + test('isInSegment, set/getChangeNumber', async () => { + const cache = new SegmentsCachePluggable(loggerMock, keyBuilder, wrapperMock); + + // assert setChangeNumber and getChangeNumber + expect(await cache.setChangeNumber('mocked-segment', 100)).toBe(false); + expect(await cache.getChangeNumber('mocked-segment')).toBe(100); + expect(await cache.setChangeNumber('mocked-segment', 200)).toBe(true); + expect(await cache.getChangeNumber('mocked-segment')).toBe(200); + expect(await cache.getChangeNumber('inexistent-segment')).toBe(-1); // -1 if the segment doesn't exist + + // mock segment keys + wrapperMock._cache[keyBuilder.buildSegmentNameKey('mocked-segment')] = ['b', 'd']; + + expect(await cache.isInSegment('mocked-segment', 'a')).toBe(false); + expect(await cache.isInSegment('mocked-segment', 'b')).toBe(true); + expect(await cache.isInSegment('mocked-segment', 'c')).toBe(false); + expect(await cache.isInSegment('mocked-segment', 'd')).toBe(true); + + expect(await cache.isInSegment('inexistent-segment', 'a')).toBe(false); + }); + +}); diff --git a/src/storages/pluggable/__tests__/SplitsCachePluggable.spec.ts b/src/storages/pluggable/__tests__/SplitsCachePluggable.spec.ts new file mode 100644 index 00000000..d7b5ff00 --- /dev/null +++ b/src/storages/pluggable/__tests__/SplitsCachePluggable.spec.ts @@ -0,0 +1,175 @@ +import { SplitsCachePluggable } from '../SplitsCachePluggable'; +import KeyBuilder from '../../KeyBuilder'; +import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; +import { wrapperMockFactory } from './wrapper.mock'; + +const splitWithUserTT = '{ "trafficTypeName": "user_tt" }'; +const splitWithAccountTT = '{ "trafficTypeName": "account_tt" }'; +const splitWithAccountTTAndUsesSegments = '{ "trafficTypeName": "account_tt", "conditions": [{ "matcherGroup": { "matchers": [{ "matcherType": "IN_SEGMENT" }]}}] }'; +const keysBuilder = new KeyBuilder(); + +describe('SPLITS CACHE PLUGGABLE', () => { + + test('add/remove/get splits', async () => { + const cache = new SplitsCachePluggable(loggerMock, keysBuilder, wrapperMockFactory()); + + // Assert addSplit and addSplits + await cache.addSplits([ + ['lol1', splitWithUserTT], + ['lol2', splitWithAccountTT] + ]); + await cache.addSplit('lol3', splitWithAccountTT); + + // adding malformed or existing splits with the same definitions will not have effects + await expect(cache.addSplits([ + ['lol1', splitWithUserTT], + ['lol2', '{ /'] + ])).rejects.toBeTruthy(); + await expect(cache.addSplit('lol3', '{ /')).rejects.toBeTruthy(); + + // Assert getAll + let values = await cache.getAll(); + + expect(values.length).toBe(3); + expect(values.indexOf(splitWithUserTT) !== -1).toBe(true); + expect(values.indexOf(splitWithAccountTT) !== -1).toBe(true); + + // Assert getSplits + let valuesObj = await cache.getSplits(['lol2', 'lol3']); + + expect(Object.keys(valuesObj).length).toBe(2); + expect(valuesObj.lol2).toBe(splitWithAccountTT); + expect(valuesObj.lol3).toBe(splitWithAccountTT); + + // Assert getSplitNames + let splitNames = await cache.getSplitNames(); + + expect(splitNames.length).toBe(3); + expect(splitNames.indexOf('lol1') !== -1).toBe(true); + expect(splitNames.indexOf('lol2') !== -1).toBe(true); + expect(splitNames.indexOf('lol3') !== -1).toBe(true); + + // Assert removeSplit + await cache.removeSplit('lol1'); + + values = await cache.getAll(); + expect(values.length).toBe(2); + expect(await cache.getSplit('lol1')).toBe(null); + expect(await cache.getSplit('lol2')).toBe(splitWithAccountTT); + + // Assert removeSplits + await cache.addSplit('lol1', splitWithUserTT); + await cache.removeSplits(['lol1', 'lol3']); + + values = await cache.getAll(); + expect(values.length).toBe(1); + splitNames = await cache.getSplitNames(); + expect(splitNames.length).toBe(1); + expect(await cache.getSplit('lol1')).toBe(null); + expect(await cache.getSplit('lol2')).toBe(splitWithAccountTT); + + }); + + test('set/get change number', async () => { + const cache = new SplitsCachePluggable(loggerMock, keysBuilder, wrapperMockFactory()); + + expect(await cache.getChangeNumber()).toBe(-1); // if not set yet, changeNumber is -1 + await cache.setChangeNumber(123); + expect(await cache.getChangeNumber()).toBe(123); + + }); + + test('trafficTypeExists', async () => { + const cache = new SplitsCachePluggable(loggerMock, keysBuilder, wrapperMockFactory()); + + await cache.addSplits([ + ['split1', splitWithUserTT], + ['split2', splitWithAccountTT], + ['split3', splitWithUserTT], + ['malformed', '{}'] + ]); + await cache.addSplit('split4', splitWithUserTT); + await cache.addSplit('split4', splitWithUserTT); // trying to add the same definition for an already added split will not have effect + + expect(await cache.trafficTypeExists('user_tt')).toBe(true); + expect(await cache.trafficTypeExists('account_tt')).toBe(true); + expect(await cache.trafficTypeExists('not_existent_tt')).toBe(false); + + await cache.removeSplit('split4'); + + expect(await cache.trafficTypeExists('user_tt')).toBe(true); + expect(await cache.trafficTypeExists('account_tt')).toBe(true); + + await cache.removeSplits(['split3', 'split2']); // it'll invoke a loop of removeSplit + + expect(await cache.trafficTypeExists('user_tt')).toBe(true); + expect(await cache.trafficTypeExists('account_tt')).toBe(false); + + await cache.removeSplit('split1'); + + expect(await cache.trafficTypeExists('user_tt')).toBe(false); + expect(await cache.trafficTypeExists('account_tt')).toBe(false); + + await cache.addSplit('split1', splitWithUserTT); + expect(await cache.trafficTypeExists('user_tt')).toBe(true); + + await cache.addSplit('split1', splitWithAccountTT); + expect(await cache.trafficTypeExists('account_tt')).toBe(true); + expect(await cache.trafficTypeExists('user_tt')).toBe(false); + + }); + + test('usesSegments', async () => { + const cache = new SplitsCachePluggable(loggerMock, keysBuilder, wrapperMockFactory()); + + await cache.addSplits([['split1', splitWithUserTT], ['split2', splitWithAccountTT],]); + expect(await cache.usesSegments()).toBe(false); // 0 splits using segments + + await cache.addSplit('split3', splitWithAccountTTAndUsesSegments); + expect(await cache.usesSegments()).toBe(true); // 1 split using segments + + await cache.addSplit('split4', splitWithAccountTTAndUsesSegments); + expect(await cache.usesSegments()).toBe(true); // 2 splits using segments + + await cache.removeSplit('split3'); + expect(await cache.usesSegments()).toBe(true); // 1 split using segments + + await cache.removeSplit('split4'); + expect(await cache.usesSegments()).toBe(false); // 0 splits using segments + }); + + test('killLocally', async () => { + const cache = new SplitsCachePluggable(loggerMock, keysBuilder, wrapperMockFactory()); + + await cache.addSplit('lol1', splitWithUserTT); + await cache.addSplit('lol2', splitWithAccountTT); + const initialChangeNumber = await cache.getChangeNumber(); + + // kill an non-existent split + let updated = await cache.killLocally('nonexistent_split', 'other_treatment', 101); + const nonexistentSplit = await cache.getSplit('nonexistent_split'); + + expect(updated).toBe(false); // killLocally resolves without update if split doesn't exist + expect(nonexistentSplit).toBe(null); // non-existent split keeps being non-existent + + // kill an existent split + updated = await cache.killLocally('lol1', 'some_treatment', 100); + let lol1Split = JSON.parse(await cache.getSplit('lol1') as string); + + expect(updated).toBe(true); // killLocally resolves with update if split is changed + expect(lol1Split.killed).toBe(true); // existing split must be killed + expect(lol1Split.defaultTreatment).toBe('some_treatment'); // existing split must have new default treatment + expect(lol1Split.changeNumber).toBe(100); // existing split must have the given change number + expect(await cache.getChangeNumber()).toBe(initialChangeNumber); // cache changeNumber is not changed + + // not update if changeNumber is old + updated = await cache.killLocally('lol1', 'some_treatment_2', 90); + lol1Split = JSON.parse(await cache.getSplit('lol1') as string); + + expect(updated).toBe(false); // killLocally resolves without update if changeNumber is old + expect(lol1Split.defaultTreatment).not.toBe('some_treatment_2'); // existing split is not updated if given changeNumber is older + + }); + +}); + diff --git a/src/storages/pluggable/__tests__/wrapper.mock.ts b/src/storages/pluggable/__tests__/wrapper.mock.ts new file mode 100644 index 00000000..60e8b772 --- /dev/null +++ b/src/storages/pluggable/__tests__/wrapper.mock.ts @@ -0,0 +1,106 @@ +import { startsWith, toNumber } from '../../../utils/lang'; + +// Creates an in memory ICustomStorageWrapper implementation with Jest mocks +export function wrapperMockFactory() { + + /** Holds items and list of items */ + let _cache: Record = {}; + + return { + _cache, + + get: jest.fn((key => { + return Promise.resolve(key in _cache ? _cache[key] as string : null); + })), + set: jest.fn((key: string, value: string) => { + const result = key in _cache; + _cache[key] = value; + return Promise.resolve(result); + }), + getAndSet: jest.fn((key: string, value: string) => { + const result = key in _cache ? _cache[key] as string : null; + _cache[key] = value; + return Promise.resolve(result); + }), + del: jest.fn((key: string) => { + const result = key in _cache; + delete _cache[key]; + return Promise.resolve(result); + }), + getKeysByPrefix: jest.fn((prefix: string) => { + return Promise.resolve(Object.keys(_cache).filter(key => startsWith(key, prefix))); + }), + getByPrefix: jest.fn((prefix: string) => { + return Promise.resolve(Object.keys(_cache).filter(key => startsWith(key, prefix)).map(key => _cache[key] as string)); + }), + incr: jest.fn((key: string) => { + if (key in _cache) { + const count = toNumber(_cache[key]) + 1; + if (isNaN(count)) return Promise.resolve(false); + _cache[key] = count + ''; + } else { + _cache[key] = '1'; + } + return Promise.resolve(true); + }), + decr: jest.fn((key: string) => { + if (key in _cache) { + const count = toNumber(_cache[key]) - 1; + if (isNaN(count)) return Promise.resolve(false); + _cache[key] = count + ''; + } else { + _cache[key] = '-1'; + } + return Promise.resolve(true); + }), + getMany: jest.fn((keys: string[]) => { + return Promise.resolve(keys.map(key => _cache[key] ? _cache[key] as string : null)); + }), + pushItems: jest.fn((key: string, items: string[]) => { // @ts-ignore + if (!(key in _cache)) _cache[key] = []; + const list = _cache[key]; + if (Array.isArray(list)) { + list.push(...items); + return Promise.resolve(); + } + return Promise.reject('key is not a list'); + }), + popItems: jest.fn((key: string, count: number) => { + const list = _cache[key]; + return Promise.resolve(Array.isArray(list) ? list.splice(0, count) : []); + }), + getItemsCount: jest.fn((key: string) => { + const list = _cache[key]; + return Promise.resolve(Array.isArray(list) ? list.length : 0); + }), + itemContains: jest.fn((key: string, item: string) => { + const list = _cache[key]; + return Promise.resolve(Array.isArray(list) && list.indexOf(item) > -1 ? true : false); + }), + + // always connects and close + connect: jest.fn(() => Promise.resolve(true)), + close: jest.fn(() => Promise.resolve()), + + mockClear() { + this._cache = {}; + this.get.mockClear(); + this.set.mockClear(); + this.del.mockClear(); + this.getKeysByPrefix.mockClear(); + this.incr.mockClear(); + this.decr.mockClear(); + this.getMany.mockClear(); + this.connect.mockClear(); + this.close.mockClear(); + this.getAndSet.mockClear(); + this.getByPrefix.mockClear(); + this.pushItems.mockClear(); + this.popItems.mockClear(); + this.getItemsCount.mockClear(); + this.itemContains.mockClear(); + } + }; +} + +export const wrapperMock = wrapperMockFactory(); diff --git a/src/storages/pluggable/__tests__/wrapperAdapter.spec.ts b/src/storages/pluggable/__tests__/wrapperAdapter.spec.ts new file mode 100644 index 00000000..d8f62ecc --- /dev/null +++ b/src/storages/pluggable/__tests__/wrapperAdapter.spec.ts @@ -0,0 +1,107 @@ +// @ts-nocheck +import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; +import thenable from '../../../utils/promise/thenable'; +import { LOG_PREFIX } from '../constants'; +import { SplitError } from '../../../utils/lang/errors'; + +/** Mocks */ + +import { wrapperMock } from './wrapper.mock'; + +function throwsException() { + throw new Error('some error'); +} + +function rejectedPromise() { + return Promise.reject('some error'); +} + +function invalidThenable() { + return { then() { } }; +} + +export const wrapperWithIssues = { + get: undefined, + set: 'invalid value', + del: throwsException, + getKeysByPrefix: rejectedPromise, + incr: invalidThenable, + decr: 'invalid value', + getMany: throwsException, + connect: rejectedPromise, + close: invalidThenable, + getAndSet: 'invalid value', + getByPrefix: throwsException, + pushItems: rejectedPromise, + popItems: invalidThenable, + getItemsCount: 'invalid value', + itemContains: throwsException, +}; + +const VALID_METHOD_CALLS = { + 'get': ['some_key'], + 'set': ['some_key', 'some_value'], + 'del': ['some_key'], + 'getKeysByPrefix': ['some_prefix'], + 'incr': ['some_key'], + 'decr': ['some_key'], + 'getMany': [['some_key_1', 'some_key_2']], + 'connect': [], + 'close': [], + 'getAndSet': ['some_key', 'some_value'], + 'getByPrefix': ['some_prefix'], + 'pushItems': ['some_key_list', ['item1', 'item2']], + 'popItems': ['some_key'], + 'getItemsCount': ['some_key'], + 'itemContains': ['some_key', 'some_value'], +}; + +// Test target +import { wrapperAdapter } from '../wrapperAdapter'; + +describe('Wrapper Adapter', () => { + + afterEach(() => { + loggerMock.mockClear(); + wrapperMock.mockClear(); + }); + + test('calls original wrapper methods', async () => { + const instance = wrapperAdapter(loggerMock, wrapperMock); + const methods = Object.keys(VALID_METHOD_CALLS); + + for (let i = 0; i < methods.length; i++) { + const method = methods[i]; + const result = instance[method](...VALID_METHOD_CALLS[method]); + expect(wrapperMock[method]).toBeCalledTimes(1); + expect(wrapperMock[method]).toBeCalledWith(...VALID_METHOD_CALLS[method]); + expect(wrapperMock[method]).toHaveReturnedWith(result); + await result; + } + + // no logs if there isn't issues + expect(loggerMock.error).not.toBeCalled(); + expect(loggerMock.warn).not.toBeCalled(); + }); + + test('handle wrapper call exceptions', async () => { + const instance = wrapperAdapter(loggerMock, wrapperWithIssues); + const methods = Object.keys(VALID_METHOD_CALLS); + + for (let i = 0; i < methods.length; i++) { + const method = methods[i]; + const result = instance[method](...VALID_METHOD_CALLS[method]); + expect(thenable(result)).toBe(true); + try { + await result; + expect(true).toBe(false); // promise shouldn't be resolved + } catch (e) { + expect(e).toBeInstanceOf(SplitError); + expect(loggerMock.error).toHaveBeenCalledWith(`${LOG_PREFIX} Wrapper '${method}' operation threw an error. Message: ${e.message}`); + } + } + + expect(loggerMock.error).toBeCalledTimes(methods.length); + }); + +}); diff --git a/src/storages/pluggable/constants.ts b/src/storages/pluggable/constants.ts new file mode 100644 index 00000000..1f845f45 --- /dev/null +++ b/src/storages/pluggable/constants.ts @@ -0,0 +1 @@ +export const LOG_PREFIX = 'storage:pluggable:'; diff --git a/src/storages/pluggable/wrapperAdapter.ts b/src/storages/pluggable/wrapperAdapter.ts new file mode 100644 index 00000000..06bcab0f --- /dev/null +++ b/src/storages/pluggable/wrapperAdapter.ts @@ -0,0 +1,57 @@ +import { ILogger } from '../../logger/types'; +import { SplitError } from '../../utils/lang/errors'; +import { ICustomStorageWrapper } from '../types'; +import { LOG_PREFIX } from './constants'; + +const METHODS_TO_PROMISE_WRAP: string[] = [ + 'get', + 'set', + 'getAndSet', + 'del', + 'getKeysByPrefix', + 'getByPrefix', + 'incr', + 'decr', + 'getMany', + 'pushItems', + 'popItems', + 'getItemsCount', + 'itemContains', + 'connect', + 'close' +]; + +/** + * Adapter of the Custom Storage Wrapper. + * Used to properly handle exception and rejected promise results: logs error and wrap them into SplitErrors. + * + * @param log logger instance + * @param wrapper custom storage wrapper to adapt + * @returns an adapted version of the given storage wrapper + */ +export function wrapperAdapter(log: ILogger, wrapper: ICustomStorageWrapper): ICustomStorageWrapper { + + const wrapperAdapter: Record = {}; + + METHODS_TO_PROMISE_WRAP.forEach((method) => { + + // Logs error and wraps it into an SplitError object (required to handle user callback errors in SDK readiness events) + function handleError(e: any) { + log.error(`${LOG_PREFIX} Wrapper '${method}' operation threw an error. Message: ${e}`); + return Promise.reject(new SplitError(e)); + } + + wrapperAdapter[method] = function () { + try { + // @ts-ignore + return wrapper[method].apply(wrapper, arguments).then(value => value).catch(handleError); + } catch (e) { + return handleError(e); + } + }; + + }); + + // @ts-ignore + return wrapperAdapter; +} diff --git a/src/storages/types.ts b/src/storages/types.ts index c4b94ad2..81024db1 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -3,13 +3,105 @@ import { ILogger } from '../logger/types'; import { IReadinessManager } from '../readiness/types'; import { SplitIO, ImpressionDTO } from '../types'; +/** + * Interface to define a custom wrapper storage. + */ +export interface ICustomStorageWrapper { + /** + * Return a promise that resolves with the element value associated with the specified `key`, or null if the key can't be found in the storage. + */ + get: (key: string) => Promise + /** + * Add or update an element with a specified `key` and `value`. + * Returns a promise that resolves with a boolean value: + * - true if the element existed and was updated, + * - or false if the element didn't exist and was added. + */ + set: (key: string, value: string) => Promise + /** + * Add or update an element with a specified `key` and `value`. + * Returns a promise that resolves with the previous value associated to the given `key`, or null if not set. + * - true if the element existed and was updated, + * - or false if the element didn't exist and was added. + */ + getAndSet: (key: string, value: string) => Promise + /** + * Remove the specified element by `key`. + * Return a promise that resolves with a boolean value: + * - true if the element existed and has been removed, + * - or false if the element does not exist. + */ + del: (key: string) => Promise + /** + * Return a promise that resolves with the list of element keys that match with the given `prefix`. + */ + getKeysByPrefix: (prefix: string) => Promise + /** + * Return a promise that resolves with the list of element values that match with the given `prefix`. + */ + getByPrefix: (prefix: string) => Promise + /** + * Increment in 1 the given `key` value or set it in 1 if the value doesn't exist. + * Return a resolved promise with a boolean value: + * - true if the value was incremented, + * - or false if the value already existed and couldn't be parsed into a finite number. + * @param key + */ + incr: (key: string) => Promise + /** + * Decrement in 1 the given `key` value or set it in -1 if the value doesn't exist. + * Return a promise that resolves with a boolean value: + * - true if the value was decremented, + * - or false if the value already existed and couldn't be parsed into a finite number. + * @param key + */ + decr: (key: string) => Promise + /** + * Return a promise that resolves with the list of elements associated with the specified list of `keys`. + */ + getMany: (keys: string[]) => Promise<(string | null)[]> + /** + * Push given `items` to `key` list. If key does not exist, an empty list is created for the key before pushing the items. + * Return a promise that resolves if the operation success, + * or rejects if the operation fails, for example, if there is a connectin error or the key holds a value that is not a list. + */ + pushItems: (key: string, items: string[]) => Promise + /** + * Pop `count` number of items from `key` queue. + * Return a promise that resolves with the list of removed items the removed members, + * or an empty array when key does not exist or is not a list. + */ + popItems: (key: string, count: number) => Promise + /** + * Return a promise that resolves with the number of items at the `key` queue, or 0 when key does not exist or is not a list. + */ + getItemsCount: (key: string) => Promise + /** + * Return a promise that resolves with true boolean value if `item` is a member of the list stored at `key`, + * or false if it is not a member or key does not exist or is not a list. + */ + itemContains: (key: string, item: string) => Promise + + /** + * For storages that requires to be connected, like database servers. + * Return a promise that resolves with a boolean value: + * - true if the operation succeeded, + * - or false if the operation failed. + */ + connect: () => Promise + /** + * For storages that requires to be closed, for example, to release resources. + */ + close: () => Promise +} + /** Splits cache */ export interface ISplitsCacheBase { - addSplit(name: string, split: string): MaybeThenable, + addSplit(name: string, split: string): MaybeThenable, // @TODO remove as in spec addSplits(entries: [string, string][]): MaybeThenable, - removeSplit(name: string): MaybeThenable, - removeSplits(names: string[]): MaybeThenable, + removeSplit(name: string): MaybeThenable, // @TODO remove as in spec + removeSplits(names: string[]): MaybeThenable, getSplit(name: string): MaybeThenable, getSplits(names: string[]): MaybeThenable>, // `fetchMany` in spec setChangeNumber(changeNumber: number): MaybeThenable, @@ -20,13 +112,14 @@ export interface ISplitsCacheBase { usesSegments(): MaybeThenable, clear(): MaybeThenable, checkCache(): MaybeThenable, + killLocally(name: string, defaultTreatment: string, changeNumber: number): MaybeThenable } export interface ISplitsCacheSync extends ISplitsCacheBase { addSplit(name: string, split: string): boolean, addSplits(entries: [string, string][]): boolean[] - removeSplit(name: string): number - removeSplits(names: string[]): number + removeSplit(name: string): boolean + removeSplits(names: string[]): boolean[] getSplit(name: string): string | null getSplits(names: string[]): Record setChangeNumber(changeNumber: number): boolean @@ -43,8 +136,8 @@ export interface ISplitsCacheSync extends ISplitsCacheBase { export interface ISplitsCacheAsync extends ISplitsCacheBase { addSplit(name: string, split: string): Promise, addSplits(entries: [string, string][]): Promise, - removeSplit(name: string): Promise, - removeSplits(names: string[]): Promise, + removeSplit(name: string): Promise, + removeSplits(names: string[]): Promise, getSplit(name: string): Promise, getSplits(names: string[]): Promise>, setChangeNumber(changeNumber: number): Promise, @@ -55,6 +148,7 @@ export interface ISplitsCacheAsync extends ISplitsCacheBase { usesSegments(): Promise, clear(): Promise, checkCache(): Promise, + killLocally(name: string, defaultTreatment: string, changeNumber: number): Promise } /** Segments cache */ diff --git a/src/sync/polling/syncTasks/splitsSyncTask.ts b/src/sync/polling/syncTasks/splitsSyncTask.ts index a69e579a..557e8c0f 100644 --- a/src/sync/polling/syncTasks/splitsSyncTask.ts +++ b/src/sync/polling/syncTasks/splitsSyncTask.ts @@ -1,6 +1,6 @@ import { SplitError } from '../../../utils/lang/errors'; import { _Set, setToArray, ISet } from '../../../utils/lang/sets'; -import { ISegmentsCacheSync, ISplitsCacheSync, IStorageSync } from '../../../storages/types'; +import { ISegmentsCacheSync, ISplitsCacheBase, IStorageSync } from '../../../storages/types'; import { ISplitChangesFetcher } from '../fetchers/types'; import { ISplit, ISplitChangesResponse } from '../../../dtos/types'; import { IReadinessManager, ISplitsEventEmitter } from '../../../readiness/types'; @@ -84,7 +84,7 @@ export function computeSplitsMutation(entries: ISplit[]): ISplitMutations { export function splitChangesUpdaterFactory( log: ILogger, splitChangesFetcher: ISplitChangesFetcher, - splitsCache: ISplitsCacheSync, + splitsCache: ISplitsCacheBase, segmentsCache: ISegmentsCacheSync, splitsEventEmitter: ISplitsEventEmitter, requestTimeoutBeforeReady: number, @@ -129,7 +129,7 @@ export function splitChangesUpdaterFactory( log.debug(SYNC_SPLITS_SEGMENTS, [mutation.segments.length]); // Write into storage - // @TODO if allowing custom storages, wrap errors as SplitErrors to distinguish from user callback errors + // @TODO call `setChangeNumber` only if the other storage operations have succeeded, in order to keep storage consistency return Promise.all([ // calling first `setChangenumber` method, to perform cache flush if split filter queryString changed splitsCache.setChangeNumber(splitChanges.till), diff --git a/src/utils/inputValidation/__tests__/splitExistance.spec.ts b/src/utils/inputValidation/__tests__/splitExistance.spec.ts index 4bc2b8e8..7d0d7b39 100644 --- a/src/utils/inputValidation/__tests__/splitExistance.spec.ts +++ b/src/utils/inputValidation/__tests__/splitExistance.spec.ts @@ -7,7 +7,7 @@ import { validateSplitExistance } from '../splitExistance'; import { IReadinessManager } from '../../../readiness/types'; import { WARN_NOT_EXISTENT_SPLIT } from '../../../logger/constants'; -describe('Split existance (special case)', () => { +describe('Split existence (special case)', () => { afterEach(() => { loggerMock.mockClear(); }); diff --git a/src/utils/inputValidation/__tests__/trafficTypeExistance.spec.ts b/src/utils/inputValidation/__tests__/trafficTypeExistance.spec.ts index ba2f1666..293ccb84 100644 --- a/src/utils/inputValidation/__tests__/trafficTypeExistance.spec.ts +++ b/src/utils/inputValidation/__tests__/trafficTypeExistance.spec.ts @@ -57,7 +57,7 @@ describe('validateTrafficTypeExistance', () => { readinessManagerMock.isReady.mockImplementation(() => true); expect(validateTrafficTypeExistance(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, TEST_EXISTENT_TT, 'test_method')).toBe(true); // If the SDK is in condition to validate but the TT exists, it will return true. - expect(splitsCacheMock.trafficTypeExists.mock.calls).toEqual([[TEST_EXISTENT_TT]]); // If the SDK is in condition to validate, it checks that TT existance with the storage. + expect(splitsCacheMock.trafficTypeExists.mock.calls).toEqual([[TEST_EXISTENT_TT]]); // If the SDK is in condition to validate, it checks that TT existence with the storage. expect(loggerMock.warn).not.toBeCalled(); // If the SDK is in condition to validate but the TT exists, it will not log any warnings. expect(loggerMock.error).not.toBeCalled(); // If the SDK is in condition to validate but the TT exists, it will not log any errors. }); @@ -65,7 +65,7 @@ describe('validateTrafficTypeExistance', () => { test('Should return false and log warning if SDK Ready, not localhost mode and the traffic type does NOT exist in the storage', () => { // Ready, standalone, and the TT not exists in the storage. expect(validateTrafficTypeExistance(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, TEST_NOT_EXISTENT_TT, 'test_method_y')).toBe(false); // If the SDK is in condition to validate but the TT does not exist in the storage, it will return false. - expect(splitsCacheMock.trafficTypeExists.mock.calls).toEqual([[TEST_NOT_EXISTENT_TT]]); // If the SDK is in condition to validate, it checks that TT existance with the storage. + expect(splitsCacheMock.trafficTypeExists.mock.calls).toEqual([[TEST_NOT_EXISTENT_TT]]); // If the SDK is in condition to validate, it checks that TT existence with the storage. expect(loggerMock.warn).toBeCalledWith(WARN_NOT_EXISTENT_TT, ['test_method_y', TEST_NOT_EXISTENT_TT]); // If the SDK is in condition to validate but the TT does not exist in the storage, it will log the expected warning. expect(loggerMock.error).not.toBeCalled(); // It logged a warning so no errors should be logged. }); @@ -75,7 +75,7 @@ describe('validateTrafficTypeExistance', () => { const validationPromise = validateTrafficTypeExistance(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, TEST_EXISTENT_ASYNC_TT, 'test_method_z'); expect(thenable(validationPromise)).toBe(true); // If the storage is async, it should also return a promise. - expect(splitsCacheMock.trafficTypeExists.mock.calls).toEqual([[TEST_EXISTENT_ASYNC_TT]]); // If the SDK is in condition to validate, it checks that TT existance with the async storage. + expect(splitsCacheMock.trafficTypeExists.mock.calls).toEqual([[TEST_EXISTENT_ASYNC_TT]]); // If the SDK is in condition to validate, it checks that TT existence with the async storage. expect(loggerMock.warn).not.toBeCalled(); // We are still fetching the data from the storage, no logs yet. expect(loggerMock.error).not.toBeCalled(); // We are still fetching the data from the storage, no logs yet. @@ -90,7 +90,7 @@ describe('validateTrafficTypeExistance', () => { const validationPromise2 = validateTrafficTypeExistance(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, TEST_NOT_EXISTENT_ASYNC_TT, 'test_method_z'); expect(thenable(validationPromise2)).toBe(true); // If the storage is async, it should also return a promise. - expect(splitsCacheMock.trafficTypeExists.mock.calls).toEqual([[TEST_NOT_EXISTENT_ASYNC_TT]]); // If the SDK is in condition to validate, it checks that TT existance with the async storage. + expect(splitsCacheMock.trafficTypeExists.mock.calls).toEqual([[TEST_NOT_EXISTENT_ASYNC_TT]]); // If the SDK is in condition to validate, it checks that TT existence with the async storage. expect(loggerMock.warn).not.toBeCalled(); // We are still fetching the data from the storage, no logs yet. expect(loggerMock.error).not.toBeCalled(); // We are still fetching the data from the storage, no logs yet.