Skip to content

Commit

Permalink
Merge pull request #275 from splitio/redis_storage_improvements
Browse files Browse the repository at this point in the history
Redis Adapter fixes
  • Loading branch information
EmilianoSanchez authored Dec 1, 2023
2 parents 02f25c9 + 67d0e61 commit f5861d8
Show file tree
Hide file tree
Showing 26 changed files with 217 additions and 160 deletions.
1 change: 1 addition & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
1.12.0 (December XX, 2023)
- Added support for Flag Sets in "consumer" and "partial consumer" modes for Pluggable and Redis storages.
- Updated evaluation flow to log a warning when using flag sets that don't contain cached feature flags.
- Updated Redis adapter to handle timeouts and queueing of some missing commands: 'hincrby', 'popNRaw', and 'pipeline.exec'.
- Bugfixing - Fixed manager methods in consumer modes to return results in a promise when the SDK is not operational (not ready or destroyed).

1.11.0 (November 3, 2023)
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@splitsoftware/splitio-commons",
"version": "1.12.1-rc.1",
"version": "1.12.1-rc.4",
"description": "Split Javascript SDK common components",
"main": "cjs/index.js",
"module": "esm/index.js",
Expand Down
30 changes: 15 additions & 15 deletions src/sdkClient/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { getMatching, getBucketing } from '../utils/key';
import { validateSplitExistence } from '../utils/inputValidation/splitExistence';
import { validateTrafficTypeExistence } from '../utils/inputValidation/trafficTypeExistence';
import { SDK_NOT_READY } from '../utils/labels';
import { CONTROL, TREATMENT, TREATMENTS, TREATMENT_WITH_CONFIG, TREATMENTS_WITH_CONFIG, TRACK, TREATMENTS_WITH_CONFIG_BY_FLAGSETS, TREATMENTS_BY_FLAGSETS, TREATMENTS_BY_FLAGSET, TREATMENTS_WITH_CONFIG_BY_FLAGSET } from '../utils/constants';
import { CONTROL, TREATMENT, TREATMENTS, TREATMENT_WITH_CONFIG, TREATMENTS_WITH_CONFIG, TRACK, TREATMENTS_WITH_CONFIG_BY_FLAGSETS, TREATMENTS_BY_FLAGSETS, TREATMENTS_BY_FLAGSET, TREATMENTS_WITH_CONFIG_BY_FLAGSET, GET_TREATMENTS_WITH_CONFIG, GET_TREATMENTS_BY_FLAG_SETS, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SETS, GET_TREATMENTS_BY_FLAG_SET, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SET, GET_TREATMENT_WITH_CONFIG, GET_TREATMENT, GET_TREATMENTS, TRACK_FN_LABEL } from '../utils/constants';
import { IEvaluationResult } from '../evaluator/types';
import { SplitIO, ImpressionDTO } from '../types';
import { IMPRESSION, IMPRESSION_QUEUEING } from '../logger/constants';
Expand All @@ -29,12 +29,12 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl
const { sdkReadinessManager: { readinessManager }, storage, settings, impressionsTracker, eventTracker, telemetryTracker } = params;
const { log, mode } = settings;

function getTreatment(key: SplitIO.SplitKey, featureFlagName: string, attributes: SplitIO.Attributes | undefined, withConfig = false) {
function getTreatment(key: SplitIO.SplitKey, featureFlagName: string, attributes: SplitIO.Attributes | undefined, withConfig = false, methodName = GET_TREATMENT) {
const stopTelemetryTracker = telemetryTracker.trackEval(withConfig ? TREATMENT_WITH_CONFIG : TREATMENT);

const wrapUp = (evaluationResult: IEvaluationResult) => {
const queue: ImpressionDTO[] = [];
const treatment = processEvaluation(evaluationResult, featureFlagName, key, attributes, withConfig, `getTreatment${withConfig ? 'withConfig' : ''}`, queue);
const treatment = processEvaluation(evaluationResult, featureFlagName, key, attributes, withConfig, methodName, queue);
impressionsTracker.track(queue, attributes);

stopTelemetryTracker(queue[0] && queue[0].label);
Expand All @@ -51,17 +51,17 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl
}

function getTreatmentWithConfig(key: SplitIO.SplitKey, featureFlagName: string, attributes: SplitIO.Attributes | undefined) {
return getTreatment(key, featureFlagName, attributes, true);
return getTreatment(key, featureFlagName, attributes, true, GET_TREATMENT_WITH_CONFIG);
}

function getTreatments(key: SplitIO.SplitKey, featureFlagNames: string[], attributes: SplitIO.Attributes | undefined, withConfig = false) {
function getTreatments(key: SplitIO.SplitKey, featureFlagNames: string[], attributes: SplitIO.Attributes | undefined, withConfig = false, methodName = GET_TREATMENTS) {
const stopTelemetryTracker = telemetryTracker.trackEval(withConfig ? TREATMENTS_WITH_CONFIG : TREATMENTS);

const wrapUp = (evaluationResults: Record<string, IEvaluationResult>) => {
const queue: ImpressionDTO[] = [];
const treatments: Record<string, SplitIO.Treatment | SplitIO.TreatmentWithConfig> = {};
Object.keys(evaluationResults).forEach(featureFlagName => {
treatments[featureFlagName] = processEvaluation(evaluationResults[featureFlagName], featureFlagName, key, attributes, withConfig, `getTreatments${withConfig ? 'withConfig' : ''}`, queue);
treatments[featureFlagName] = processEvaluation(evaluationResults[featureFlagName], featureFlagName, key, attributes, withConfig, methodName, queue);
});
impressionsTracker.track(queue, attributes);

Expand All @@ -79,18 +79,18 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl
}

function getTreatmentsWithConfig(key: SplitIO.SplitKey, featureFlagNames: string[], attributes: SplitIO.Attributes | undefined) {
return getTreatments(key, featureFlagNames, attributes, true);
return getTreatments(key, featureFlagNames, attributes, true, GET_TREATMENTS_WITH_CONFIG);
}

function getTreatmentsByFlagSets(key: SplitIO.SplitKey, flagSetNames: string[], attributes: SplitIO.Attributes | undefined, withConfig = false, method: Method = TREATMENTS_BY_FLAGSETS) {
function getTreatmentsByFlagSets(key: SplitIO.SplitKey, flagSetNames: string[], attributes: SplitIO.Attributes | undefined, withConfig = false, method: Method = TREATMENTS_BY_FLAGSETS, methodName = GET_TREATMENTS_BY_FLAG_SETS) {
const stopTelemetryTracker = telemetryTracker.trackEval(method);

const wrapUp = (evaluationResults: Record<string,IEvaluationResult>) => {
const wrapUp = (evaluationResults: Record<string, IEvaluationResult>) => {
const queue: ImpressionDTO[] = [];
const treatments: Record<string, SplitIO.Treatment | SplitIO.TreatmentWithConfig> = {};
const evaluations = evaluationResults;
Object.keys(evaluations).forEach(featureFlagName => {
treatments[featureFlagName] = processEvaluation(evaluations[featureFlagName], featureFlagName, key, attributes, withConfig, `getTreatmentsByFlagSets${withConfig ? 'WithConfig' : ''}`, queue);
treatments[featureFlagName] = processEvaluation(evaluations[featureFlagName], featureFlagName, key, attributes, withConfig, methodName, queue);
});
impressionsTracker.track(queue, attributes);

Expand All @@ -99,22 +99,22 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl
};

const evaluations = readinessManager.isReady() || readinessManager.isReadyFromCache() ?
evaluateFeaturesByFlagSets(log, key, flagSetNames, attributes, storage, method) :
evaluateFeaturesByFlagSets(log, key, flagSetNames, attributes, storage, methodName) :
isStorageSync(settings) ? {} : Promise.resolve({}); // Promisify if async

return thenable(evaluations) ? evaluations.then((res) => wrapUp(res)) : wrapUp(evaluations);
}

function getTreatmentsWithConfigByFlagSets(key: SplitIO.SplitKey, flagSetNames: string[], attributes: SplitIO.Attributes | undefined) {
return getTreatmentsByFlagSets(key, flagSetNames, attributes, true, TREATMENTS_WITH_CONFIG_BY_FLAGSETS);
return getTreatmentsByFlagSets(key, flagSetNames, attributes, true, TREATMENTS_WITH_CONFIG_BY_FLAGSETS, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SETS);
}

function getTreatmentsByFlagSet(key: SplitIO.SplitKey, flagSetName: string, attributes: SplitIO.Attributes | undefined) {
return getTreatmentsByFlagSets(key, [flagSetName], attributes, false, TREATMENTS_BY_FLAGSET);
return getTreatmentsByFlagSets(key, [flagSetName], attributes, false, TREATMENTS_BY_FLAGSET, GET_TREATMENTS_BY_FLAG_SET);
}

function getTreatmentsWithConfigByFlagSet(key: SplitIO.SplitKey, flagSetName: string, attributes: SplitIO.Attributes | undefined) {
return getTreatmentsByFlagSets(key, [flagSetName], attributes, true, TREATMENTS_WITH_CONFIG_BY_FLAGSET);
return getTreatmentsByFlagSets(key, [flagSetName], attributes, true, TREATMENTS_WITH_CONFIG_BY_FLAGSET, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SET);
}

// Internal function
Expand Down Expand Up @@ -171,7 +171,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl
};

// This may be async but we only warn, we don't actually care if it is valid or not in terms of queueing the event.
validateTrafficTypeExistence(log, readinessManager, storage.splits, mode, trafficTypeName, 'track');
validateTrafficTypeExistence(log, readinessManager, storage.splits, mode, trafficTypeName, TRACK_FN_LABEL);

const result = eventTracker.track(eventData, size);

Expand Down
36 changes: 18 additions & 18 deletions src/sdkClient/clientInputValidation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ import {
validateIfOperational
} from '../utils/inputValidation';
import { startsWith } from '../utils/lang';
import { CONTROL, CONTROL_WITH_CONFIG } from '../utils/constants';
import { CONTROL, CONTROL_WITH_CONFIG, GET_TREATMENT, GET_TREATMENTS, GET_TREATMENTS_BY_FLAG_SET, GET_TREATMENTS_BY_FLAG_SETS, GET_TREATMENTS_WITH_CONFIG, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SET, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SETS, GET_TREATMENT_WITH_CONFIG, TRACK_FN_LABEL } from '../utils/constants';
import { IReadinessManager } from '../readiness/types';
import { MaybeThenable } from '../dtos/types';
import { ISettings, SplitIO } from '../types';
import { isStorageSync } from '../trackers/impressionObserver/utils';
import { flagSetsAreValid } from '../utils/settingsValidation/splitFilters';
import { validateFlagSets } from '../utils/settingsValidation/splitFilters';

/**
* Decorator that validates the input before actually executing the client methods.
Expand All @@ -32,7 +32,7 @@ export function clientInputValidationDecorator<TClient extends SplitIO.IClient |
* Avoid repeating this validations code
*/
function validateEvaluationParams(maybeKey: SplitIO.SplitKey, maybeFeatureFlagNameOrNames: string | string[] | undefined, maybeAttributes: SplitIO.Attributes | undefined, methodName: string, maybeFlagSetNameOrNames?: string[]) {
const multi = startsWith(methodName, 'getTreatments');
const multi = startsWith(methodName, GET_TREATMENTS);
const key = validateKey(log, maybeKey, methodName);
let splitOrSplits: string | string[] | false = false;
let flagSetOrFlagSets: string[] = [];
Expand All @@ -42,7 +42,7 @@ export function clientInputValidationDecorator<TClient extends SplitIO.IClient |
const attributes = validateAttributes(log, maybeAttributes, methodName);
const isNotDestroyed = validateIfNotDestroyed(log, readinessManager, methodName);
if (maybeFlagSetNameOrNames) {
flagSetOrFlagSets = flagSetsAreValid(log, methodName, maybeFlagSetNameOrNames, settings.sync.__splitFiltersValidation.groupedFilters.bySet);
flagSetOrFlagSets = validateFlagSets(log, methodName, maybeFlagSetNameOrNames, settings.sync.__splitFiltersValidation.groupedFilters.bySet);
}

validateIfOperational(log, readinessManager, methodName, splitOrSplits);
Expand All @@ -63,7 +63,7 @@ export function clientInputValidationDecorator<TClient extends SplitIO.IClient |
}

function getTreatment(maybeKey: SplitIO.SplitKey, maybeFeatureFlagName: string, maybeAttributes?: SplitIO.Attributes) {
const params = validateEvaluationParams(maybeKey, maybeFeatureFlagName, maybeAttributes, 'getTreatment');
const params = validateEvaluationParams(maybeKey, maybeFeatureFlagName, maybeAttributes, GET_TREATMENT);

if (params.valid) {
return client.getTreatment(params.key as SplitIO.SplitKey, params.splitOrSplits as string, params.attributes as SplitIO.Attributes | undefined);
Expand All @@ -73,7 +73,7 @@ export function clientInputValidationDecorator<TClient extends SplitIO.IClient |
}

function getTreatmentWithConfig(maybeKey: SplitIO.SplitKey, maybeFeatureFlagName: string, maybeAttributes?: SplitIO.Attributes) {
const params = validateEvaluationParams(maybeKey, maybeFeatureFlagName, maybeAttributes, 'getTreatmentWithConfig');
const params = validateEvaluationParams(maybeKey, maybeFeatureFlagName, maybeAttributes, GET_TREATMENT_WITH_CONFIG);

if (params.valid) {
return client.getTreatmentWithConfig(params.key as SplitIO.SplitKey, params.splitOrSplits as string, params.attributes as SplitIO.Attributes | undefined);
Expand All @@ -83,7 +83,7 @@ export function clientInputValidationDecorator<TClient extends SplitIO.IClient |
}

function getTreatments(maybeKey: SplitIO.SplitKey, maybeFeatureFlagNames: string[], maybeAttributes?: SplitIO.Attributes) {
const params = validateEvaluationParams(maybeKey, maybeFeatureFlagNames, maybeAttributes, 'getTreatments');
const params = validateEvaluationParams(maybeKey, maybeFeatureFlagNames, maybeAttributes, GET_TREATMENTS);

if (params.valid) {
return client.getTreatments(params.key as SplitIO.SplitKey, params.splitOrSplits as string[], params.attributes as SplitIO.Attributes | undefined);
Expand All @@ -96,7 +96,7 @@ export function clientInputValidationDecorator<TClient extends SplitIO.IClient |
}

function getTreatmentsWithConfig(maybeKey: SplitIO.SplitKey, maybeFeatureFlagNames: string[], maybeAttributes?: SplitIO.Attributes) {
const params = validateEvaluationParams(maybeKey, maybeFeatureFlagNames, maybeAttributes, 'getTreatmentsWithConfig');
const params = validateEvaluationParams(maybeKey, maybeFeatureFlagNames, maybeAttributes, GET_TREATMENTS_WITH_CONFIG);

if (params.valid) {
return client.getTreatmentsWithConfig(params.key as SplitIO.SplitKey, params.splitOrSplits as string[], params.attributes as SplitIO.Attributes | undefined);
Expand All @@ -109,7 +109,7 @@ export function clientInputValidationDecorator<TClient extends SplitIO.IClient |
}

function getTreatmentsByFlagSets(maybeKey: SplitIO.SplitKey, maybeFlagSets: string[], maybeAttributes?: SplitIO.Attributes) {
const params = validateEvaluationParams(maybeKey, undefined, maybeAttributes, 'getTreatmentsByFlagSets', maybeFlagSets);
const params = validateEvaluationParams(maybeKey, undefined, maybeAttributes, GET_TREATMENTS_BY_FLAG_SETS, maybeFlagSets);

if (params.valid) {
return client.getTreatmentsByFlagSets(params.key as SplitIO.SplitKey, params.flagSetOrFlagSets as string[], params.attributes as SplitIO.Attributes | undefined);
Expand All @@ -119,7 +119,7 @@ export function clientInputValidationDecorator<TClient extends SplitIO.IClient |
}

function getTreatmentsWithConfigByFlagSets(maybeKey: SplitIO.SplitKey, maybeFlagSets: string[], maybeAttributes?: SplitIO.Attributes) {
const params = validateEvaluationParams(maybeKey, undefined, maybeAttributes, 'getTreatmentsWithConfigByFlagSets', maybeFlagSets);
const params = validateEvaluationParams(maybeKey, undefined, maybeAttributes, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SETS, maybeFlagSets);

if (params.valid) {
return client.getTreatmentsWithConfigByFlagSets(params.key as SplitIO.SplitKey, params.flagSetOrFlagSets as string[], params.attributes as SplitIO.Attributes | undefined);
Expand All @@ -129,7 +129,7 @@ export function clientInputValidationDecorator<TClient extends SplitIO.IClient |
}

function getTreatmentsByFlagSet(maybeKey: SplitIO.SplitKey, maybeFlagSet: string, maybeAttributes?: SplitIO.Attributes) {
const params = validateEvaluationParams(maybeKey, undefined, maybeAttributes, 'getTreatmentsByFlagSet', [maybeFlagSet]);
const params = validateEvaluationParams(maybeKey, undefined, maybeAttributes, GET_TREATMENTS_BY_FLAG_SET, [maybeFlagSet]);

if (params.valid) {
return client.getTreatmentsByFlagSet(params.key as SplitIO.SplitKey, params.flagSetOrFlagSets[0] as string, params.attributes as SplitIO.Attributes | undefined);
Expand All @@ -139,7 +139,7 @@ export function clientInputValidationDecorator<TClient extends SplitIO.IClient |
}

function getTreatmentsWithConfigByFlagSet(maybeKey: SplitIO.SplitKey, maybeFlagSet: string, maybeAttributes?: SplitIO.Attributes) {
const params = validateEvaluationParams(maybeKey, undefined, maybeAttributes, 'getTreatmentsWithConfigByFlagSet', [maybeFlagSet]);
const params = validateEvaluationParams(maybeKey, undefined, maybeAttributes, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SET, [maybeFlagSet]);

if (params.valid) {
return client.getTreatmentsWithConfigByFlagSet(params.key as SplitIO.SplitKey, params.flagSetOrFlagSets[0] as string, params.attributes as SplitIO.Attributes | undefined);
Expand All @@ -149,12 +149,12 @@ export function clientInputValidationDecorator<TClient extends SplitIO.IClient |
}

function track(maybeKey: SplitIO.SplitKey, maybeTT: string, maybeEvent: string, maybeEventValue?: number, maybeProperties?: SplitIO.Properties) {
const key = validateKey(log, maybeKey, 'track');
const tt = validateTrafficType(log, maybeTT, 'track');
const event = validateEvent(log, maybeEvent, 'track');
const eventValue = validateEventValue(log, maybeEventValue, 'track');
const { properties, size } = validateEventProperties(log, maybeProperties, 'track');
const isNotDestroyed = validateIfNotDestroyed(log, readinessManager, 'track');
const key = validateKey(log, maybeKey, TRACK_FN_LABEL);
const tt = validateTrafficType(log, maybeTT, TRACK_FN_LABEL);
const event = validateEvent(log, maybeEvent, TRACK_FN_LABEL);
const eventValue = validateEventValue(log, maybeEventValue, TRACK_FN_LABEL);
const { properties, size } = validateEventProperties(log, maybeProperties, TRACK_FN_LABEL);
const isNotDestroyed = validateIfNotDestroyed(log, readinessManager, TRACK_FN_LABEL);

if (isNotDestroyed && key && tt && event && eventValue !== false && properties !== false) { // @ts-expect-error
return client.track(key, tt, event, eventValue, properties, size);
Expand Down
4 changes: 2 additions & 2 deletions src/sdkManager/__tests__/index.asyncCache.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import Redis from 'ioredis';
import splitObject from './mocks/input.json';
import splitView from './mocks/output.json';
import { sdkManagerFactory } from '../index';
Expand All @@ -9,6 +8,7 @@ import { KeyBuilderSS } from '../../storages/KeyBuilderSS';
import { ISdkReadinessManager } from '../../readiness/types';
import { loggerMock } from '../../logger/__tests__/sdkLogger.mock';
import { metadata } from '../../storages/__tests__/KeyBuilder.spec';
import { RedisAdapter } from '../../storages/inRedis/RedisAdapter';
import { SplitIO } from '../../types';

// @ts-expect-error
Expand All @@ -29,7 +29,7 @@ describe('Manager with async cache', () => {
test('returns the expected data from the cache', async () => {

/** Setup: create manager */
const connection = new Redis({});
const connection = new RedisAdapter(loggerMock);
const cache = new SplitsCacheInRedis(loggerMock, keys, connection);
const manager = sdkManagerFactory({ mode: 'consumer', log: loggerMock }, cache, sdkReadinessManagerMock);
await cache.clear();
Expand Down
5 changes: 1 addition & 4 deletions src/sdkManager/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@ import { ISdkReadinessManager } from '../readiness/types';
import { ISplit } from '../dtos/types';
import { ISettings, SplitIO } from '../types';
import { isStorageSync } from '../trackers/impressionObserver/utils';

const SPLIT_FN_LABEL = 'split';
const SPLITS_FN_LABEL = 'splits';
const NAMES_FN_LABEL = 'names';
import { SPLIT_FN_LABEL, SPLITS_FN_LABEL, NAMES_FN_LABEL } from '../utils/constants';

function collectTreatments(splitObject: ISplit) {
const conditions = splitObject.conditions;
Expand Down
Loading

0 comments on commit f5861d8

Please sign in to comment.