Skip to content

Commit

Permalink
Merge pull request #262 from splitio/update
Browse files Browse the repository at this point in the history
Remove condition to ignore splitFilters if not in standalone mode
  • Loading branch information
emmaz90 authored Oct 24, 2023
2 parents 40750b5 + 5e243f5 commit 54dc3ab
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 33 deletions.
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.10.0",
"version": "1.10.1-rc.0",
"description": "Split Javascript SDK common components",
"main": "cjs/index.js",
"module": "esm/index.js",
Expand Down
1 change: 0 additions & 1 deletion src/logger/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ export const WARN_NOT_EXISTENT_SPLIT = 215;
export const WARN_LOWERCASE_TRAFFIC_TYPE = 216;
export const WARN_NOT_EXISTENT_TT = 217;
export const WARN_INTEGRATION_INVALID = 218;
export const WARN_SPLITS_FILTER_IGNORED = 219;
export const WARN_SPLITS_FILTER_INVALID = 220;
export const WARN_SPLITS_FILTER_EMPTY = 221;
export const WARN_SDK_KEY = 222;
Expand Down
1 change: 0 additions & 1 deletion src/logger/messages/warn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ export const codesWarn: [number, string][] = codesError.concat([
[c.WARN_FLAGSET_NOT_CONFIGURED, '%s: : you passed %s wich is not part of the configured FlagSetsFilter, ignoring Flag Set.'],
// initialization / settings validation
[c.WARN_INTEGRATION_INVALID, c.LOG_PREFIX_SETTINGS+': %s integration item(s) at settings is invalid. %s'],
[c.WARN_SPLITS_FILTER_IGNORED, c.LOG_PREFIX_SETTINGS+': feature flag filters have been configured but will have no effect if mode is not "%s", since synchronization is being deferred to an external tool.'],
[c.WARN_SPLITS_FILTER_INVALID, c.LOG_PREFIX_SETTINGS+': feature flag filter at position %s is invalid. It must be an object with a valid filter type ("bySet", "byName" or "byPrefix") and a list of "values".'],
[c.WARN_SPLITS_FILTER_EMPTY, c.LOG_PREFIX_SETTINGS+': feature flag filter configuration must be a non-empty array of filter objects.'],
[c.WARN_SDK_KEY, c.LOG_PREFIX_SETTINGS+': You already have %s. We recommend keeping only one instance of the factory at all times (Singleton pattern) and reusing it throughout your application'],
Expand Down
33 changes: 15 additions & 18 deletions src/utils/settingsValidation/__tests__/splitFilters.spec.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock';

import { STANDALONE_MODE, CONSUMER_MODE } from '../../constants';

// Split filter and QueryStrings examples
import { splitFilters, queryStrings, groupedFilters } from '../../../__tests__/mocks/fetchSpecificSplits';

// Test target
import { flagSetsAreValid, validateSplitFilters } from '../splitFilters';
import { SETTINGS_SPLITS_FILTER, ERROR_INVALID, ERROR_EMPTY_ARRAY, ERROR_SETS_FILTER_EXCLUSIVE, WARN_SPLITS_FILTER_IGNORED, WARN_SPLITS_FILTER_INVALID, WARN_SPLITS_FILTER_EMPTY, WARN_TRIMMING, WARN_SPLITS_FILTER_INVALID_SET, WARN_SPLITS_FILTER_LOWERCASE_SET, WARN_FLAGSET_NOT_CONFIGURED } from '../../../logger/constants';
import { SETTINGS_SPLITS_FILTER, ERROR_INVALID, ERROR_EMPTY_ARRAY, ERROR_SETS_FILTER_EXCLUSIVE, WARN_SPLITS_FILTER_INVALID, WARN_SPLITS_FILTER_EMPTY, WARN_TRIMMING, WARN_SPLITS_FILTER_INVALID_SET, WARN_SPLITS_FILTER_LOWERCASE_SET, WARN_FLAGSET_NOT_CONFIGURED } from '../../../logger/constants';

describe('validateSplitFilters', () => {

Expand All @@ -32,16 +30,15 @@ describe('validateSplitFilters', () => {

test('Returns default output with empty values if `splitFilters` is an invalid object or `mode` is not \'standalone\'', () => {

expect(validateSplitFilters(loggerMock, undefined, STANDALONE_MODE)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array
expect(validateSplitFilters(loggerMock, null, STANDALONE_MODE)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array
expect(validateSplitFilters(loggerMock, undefined)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array
expect(validateSplitFilters(loggerMock, null)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array
expect(loggerMock.warn).not.toBeCalled();

expect(validateSplitFilters(loggerMock, true, STANDALONE_MODE)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array
expect(validateSplitFilters(loggerMock, 15, STANDALONE_MODE)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array
expect(validateSplitFilters(loggerMock, 'string', STANDALONE_MODE)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array
expect(validateSplitFilters(loggerMock, [], STANDALONE_MODE)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array
expect(validateSplitFilters(loggerMock, [{ type: 'byName', values: ['split_1'] }], CONSUMER_MODE)).toEqual(defaultOutput); // splitFilters ignored if not in 'standalone' mode
expect(loggerMock.warn.mock.calls).toEqual([[WARN_SPLITS_FILTER_EMPTY], [WARN_SPLITS_FILTER_EMPTY], [WARN_SPLITS_FILTER_EMPTY], [WARN_SPLITS_FILTER_EMPTY], [WARN_SPLITS_FILTER_IGNORED, ['standalone']]]);
expect(validateSplitFilters(loggerMock, true)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array
expect(validateSplitFilters(loggerMock, 15)).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array
expect(validateSplitFilters(loggerMock, 'string')).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array
expect(validateSplitFilters(loggerMock, [])).toEqual(defaultOutput); // splitFilters ignored if not a non-empty array
expect(loggerMock.warn.mock.calls).toEqual([[WARN_SPLITS_FILTER_EMPTY], [WARN_SPLITS_FILTER_EMPTY], [WARN_SPLITS_FILTER_EMPTY], [WARN_SPLITS_FILTER_EMPTY]]);

expect(loggerMock.debug).not.toBeCalled();
expect(loggerMock.error).not.toBeCalled();
Expand All @@ -59,7 +56,7 @@ describe('validateSplitFilters', () => {
queryString: null,
groupedFilters: { bySet: [], byName: [], byPrefix: [] },
};
expect(validateSplitFilters(loggerMock, splitFilters, STANDALONE_MODE)).toEqual(output); // filters without values
expect(validateSplitFilters(loggerMock, splitFilters)).toEqual(output); // filters without values
expect(loggerMock.debug).toBeCalledWith(SETTINGS_SPLITS_FILTER, [null]);
loggerMock.debug.mockClear();

Expand All @@ -69,7 +66,7 @@ describe('validateSplitFilters', () => {
{ type: null, values: [] },
{ type: 'byName', values: [13] });
output.validFilters.push({ type: 'byName', values: [13] });
expect(validateSplitFilters(loggerMock, splitFilters, STANDALONE_MODE)).toEqual(output); // some filters are invalid
expect(validateSplitFilters(loggerMock, splitFilters)).toEqual(output); // some filters are invalid
expect(loggerMock.debug.mock.calls).toEqual([[SETTINGS_SPLITS_FILTER, [null]]]);
expect(loggerMock.warn.mock.calls).toEqual([
[WARN_SPLITS_FILTER_INVALID, [4]], // invalid value of `type` property
Expand All @@ -93,24 +90,24 @@ describe('validateSplitFilters', () => {
queryString: queryStrings[i],
groupedFilters: groupedFilters[i],
};
expect(validateSplitFilters(loggerMock, splitFilters[i], STANDALONE_MODE)).toEqual(output); // splitFilters #${i}
expect(validateSplitFilters(loggerMock, splitFilters[i])).toEqual(output); // splitFilters #${i}
expect(loggerMock.debug).lastCalledWith(SETTINGS_SPLITS_FILTER, [queryStrings[i]]);

} else { // tests where validateSplitFilters throws an exception
expect(() => validateSplitFilters(loggerMock, splitFilters[i], STANDALONE_MODE)).toThrow(queryStrings[i]);
expect(() => validateSplitFilters(loggerMock, splitFilters[i])).toThrow(queryStrings[i]);
}
}
});

test('Validates flag set filters', () => {
// extra spaces trimmed and sorted query output
expect(validateSplitFilters(loggerMock, splitFilters[6], STANDALONE_MODE)).toEqual(getOutput(6)); // trim & sort
expect(validateSplitFilters(loggerMock, splitFilters[6])).toEqual(getOutput(6)); // trim & sort
expect(loggerMock.warn.mock.calls[0]).toEqual([WARN_TRIMMING, ['settings', 'bySet filter value', ' set_1']]);
expect(loggerMock.warn.mock.calls[1]).toEqual([WARN_TRIMMING, ['settings', 'bySet filter value', 'set_3 ']]);
expect(loggerMock.warn.mock.calls[2]).toEqual([WARN_TRIMMING, ['settings', 'bySet filter value', ' set_a ']]);
expect(loggerMock.error.mock.calls[0]).toEqual([ERROR_SETS_FILTER_EXCLUSIVE]);

expect(validateSplitFilters(loggerMock, splitFilters[7], STANDALONE_MODE)).toEqual(getOutput(7)); // lowercase and regexp
expect(validateSplitFilters(loggerMock, splitFilters[7])).toEqual(getOutput(7)); // lowercase and regexp
expect(loggerMock.warn.mock.calls[3]).toEqual([WARN_SPLITS_FILTER_LOWERCASE_SET, ['seT_c']]); // lowercase
expect(loggerMock.warn.mock.calls[4]).toEqual([WARN_SPLITS_FILTER_LOWERCASE_SET, ['set_B']]); // lowercase
expect(loggerMock.warn.mock.calls[5]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set_ 1', regexp, 'set_ 1']]); // empty spaces
Expand All @@ -119,7 +116,7 @@ describe('validateSplitFilters', () => {
expect(loggerMock.warn.mock.calls[8]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set_1234567890_1234567890_234567890_1234567890_1234567890', regexp, 'set_1234567890_1234567890_234567890_1234567890_1234567890']]); // max of 50 characters
expect(loggerMock.error.mock.calls[1]).toEqual([ERROR_SETS_FILTER_EXCLUSIVE]);

expect(validateSplitFilters(loggerMock, splitFilters[8], STANDALONE_MODE)).toEqual(getOutput(8)); // lowercase and dedupe
expect(validateSplitFilters(loggerMock, splitFilters[8])).toEqual(getOutput(8)); // lowercase and dedupe
expect(loggerMock.warn.mock.calls[9]).toEqual([WARN_SPLITS_FILTER_LOWERCASE_SET, ['SET_2']]); // lowercase
expect(loggerMock.warn.mock.calls[10]).toEqual([WARN_SPLITS_FILTER_LOWERCASE_SET, ['set_B']]); // lowercase
expect(loggerMock.warn.mock.calls[11]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set_3!', regexp, 'set_3!']]); // special character
Expand Down
2 changes: 1 addition & 1 deletion src/utils/settingsValidation/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ export function settingsValidation(config: unknown, validationParams: ISettingsV
}

// validate the `splitFilters` settings and parse splits query
const splitFiltersValidation = validateSplitFilters(log, withDefaults.sync.splitFilters, withDefaults.mode);
const splitFiltersValidation = validateSplitFilters(log, withDefaults.sync.splitFilters);
withDefaults.sync.splitFilters = splitFiltersValidation.validFilters;
withDefaults.sync.__splitFiltersValidation = splitFiltersValidation;

Expand Down
11 changes: 2 additions & 9 deletions src/utils/settingsValidation/splitFilters.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { STANDALONE_MODE } from '../constants';
import { validateSplits } from '../inputValidation/splits';
import { ISplitFiltersValidation } from '../../dtos/types';
import { SplitIO } from '../../types';
import { ILogger } from '../../logger/types';
import { WARN_SPLITS_FILTER_IGNORED, WARN_SPLITS_FILTER_EMPTY, WARN_SPLITS_FILTER_INVALID, SETTINGS_SPLITS_FILTER, LOG_PREFIX_SETTINGS, ERROR_SETS_FILTER_EXCLUSIVE, WARN_SPLITS_FILTER_LOWERCASE_SET, WARN_SPLITS_FILTER_INVALID_SET, WARN_FLAGSET_NOT_CONFIGURED } from '../../logger/constants';
import { WARN_SPLITS_FILTER_EMPTY, WARN_SPLITS_FILTER_INVALID, SETTINGS_SPLITS_FILTER, LOG_PREFIX_SETTINGS, ERROR_SETS_FILTER_EXCLUSIVE, WARN_SPLITS_FILTER_LOWERCASE_SET, WARN_SPLITS_FILTER_INVALID_SET, WARN_FLAGSET_NOT_CONFIGURED } from '../../logger/constants';
import { objectAssign } from '../lang/objectAssign';
import { find, uniq } from '../lang';

Expand Down Expand Up @@ -129,15 +128,14 @@ function configuredFilter(validFilters: SplitIO.SplitFilter[], filterType: Split
*
* @param {ILogger} log logger
* @param {any} maybeSplitFilters split filters configuration param provided by the user
* @param {string} mode settings mode
* @returns it returns an object with the following properties:
* - `validFilters`: the validated `splitFilters` configuration object defined by the user.
* - `queryString`: the parsed split filter query. it is null if all filters are invalid or all values in filters are invalid.
* - `groupedFilters`: list of values grouped by filter type.
*
* @throws Error if the some of the grouped list of values per filter exceeds the max allowed length
*/
export function validateSplitFilters(log: ILogger, maybeSplitFilters: any, mode: string): ISplitFiltersValidation {
export function validateSplitFilters(log: ILogger, maybeSplitFilters: any): ISplitFiltersValidation {
// Validation result schema
const res = {
validFilters: [],
Expand All @@ -147,11 +145,6 @@ export function validateSplitFilters(log: ILogger, maybeSplitFilters: any, mode:

// do nothing if `splitFilters` param is not a non-empty array or mode is not STANDALONE
if (!maybeSplitFilters) return res;
// Warn depending on the mode
if (mode !== STANDALONE_MODE) {
log.warn(WARN_SPLITS_FILTER_IGNORED, [STANDALONE_MODE]);
return res;
}
// Check collection type
if (!Array.isArray(maybeSplitFilters) || maybeSplitFilters.length === 0) {
log.warn(WARN_SPLITS_FILTER_EMPTY);
Expand Down

0 comments on commit 54dc3ab

Please sign in to comment.