Skip to content

Commit

Permalink
Update logic & error log when using sets with other filters
Browse files Browse the repository at this point in the history
  • Loading branch information
Emmanuel Zamora committed Sep 28, 2023
1 parent 925d552 commit d3a90a3
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 14 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.9.1-rc.0",
"version": "1.9.2-rc.0",
"description": "Split Javascript SDK common components",
"main": "cjs/index.js",
"module": "esm/index.js",
Expand Down
2 changes: 1 addition & 1 deletion src/logger/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export const ERROR_STORAGE_INVALID = 324;
export const ERROR_NOT_BOOLEAN = 325;
export const ERROR_MIN_CONFIG_PARAM = 326;
export const ERROR_TOO_MANY_SETS = 327;
export const ERROR_SPLITS_FILTER_NAME_AND_SET = 328;
export const ERROR_SETS_FILTER_EXCLUSIVE = 328;

// Log prefixes (a.k.a. tags or categories)
export const LOG_PREFIX_SETTINGS = 'settings';
Expand Down
2 changes: 1 addition & 1 deletion src/logger/messages/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@ export const codesError: [number, string][] = [
[c.ERROR_STORAGE_INVALID, c.LOG_PREFIX_SETTINGS+': the provided storage is invalid.%s Falling back into default MEMORY storage'],
[c.ERROR_MIN_CONFIG_PARAM, c.LOG_PREFIX_SETTINGS + ': the provided "%s" config param is lower than allowed. Setting to the minimum value %s seconds'],
[c.ERROR_TOO_MANY_SETS, c.LOG_PREFIX_SETTINGS + ': the amount of flag sets provided are big causing uri length error.'],
[c.ERROR_SPLITS_FILTER_NAME_AND_SET, c.LOG_PREFIX_SETTINGS+': names and sets filter cannot be used at the same time. The sdk will proceed using sets filter.'],
[c.ERROR_SETS_FILTER_EXCLUSIVE, c.LOG_PREFIX_SETTINGS+': the Set filter is exclusive and cannot be used simultaneously with names or prefix filters. Ignoring names and prefixes.'],
];
14 changes: 11 additions & 3 deletions src/sync/polling/updaters/splitChangesUpdater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { timeout } from '../../../utils/promise/timeout';
import { SDK_SPLITS_ARRIVED, SDK_SPLITS_CACHE_LOADED } from '../../../readiness/constants';
import { ILogger } from '../../../logger/types';
import { SYNC_SPLITS_FETCH, SYNC_SPLITS_NEW, SYNC_SPLITS_REMOVED, SYNC_SPLITS_SEGMENTS, SYNC_SPLITS_FETCH_FAILS, SYNC_SPLITS_FETCH_RETRY } from '../../../logger/constants';
import { startsWith } from '../../../utils/lang';

type ISplitChangesUpdater = (noCache?: boolean, till?: number, splitUpdateNotification?: { payload: ISplit, changeNumber: number }) => Promise<boolean>

Expand Down Expand Up @@ -53,10 +54,17 @@ interface ISplitMutations {
* @param filters splitFiltersValidation bySet | byName
*/
function matchFilters(featureFlag: ISplit, filters: ISplitFiltersValidation) {
const { bySet: setsFilter, byName: namesFilter } = filters.groupedFilters;
const { bySet: setsFilter, byName: namesFilter, byPrefix: prefixFilter} = filters.groupedFilters;
if (setsFilter.length > 0) return featureFlag.sets && featureFlag.sets.some((featureFlagSet: string) => setsFilter.indexOf(featureFlagSet) > -1);
if (namesFilter.length > 0) return namesFilter.indexOf(featureFlag.name) > -1;
return true;

const namesFilterConfigured = namesFilter.length > 0;
const prefixFilterConfigured = prefixFilter.length > 0;

if (!namesFilterConfigured && !prefixFilterConfigured) return true;

const matchNames = namesFilterConfigured && namesFilter.indexOf(featureFlag.name) > -1;
const matchPrefix = prefixFilterConfigured && prefixFilter.some(prefix => startsWith(featureFlag.name, prefix));
return matchNames || matchPrefix;
}

/**
Expand Down
8 changes: 4 additions & 4 deletions src/utils/settingsValidation/__tests__/splitFilters.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { splitFilters, queryStrings, groupedFilters } from '../../../__tests__/m

// Test target
import { flagSetsAreValid, validateSplitFilters } from '../splitFilters';
import { SETTINGS_SPLITS_FILTER, ERROR_INVALID, ERROR_EMPTY_ARRAY, WARN_SPLITS_FILTER_IGNORED, WARN_SPLITS_FILTER_INVALID, WARN_SPLITS_FILTER_EMPTY, WARN_TRIMMING, ERROR_SPLITS_FILTER_NAME_AND_SET, 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_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';

describe('validateSplitFilters', () => {

Expand Down Expand Up @@ -108,7 +108,7 @@ describe('validateSplitFilters', () => {
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_SPLITS_FILTER_NAME_AND_SET]);
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(loggerMock.warn.mock.calls[3]).toEqual([WARN_SPLITS_FILTER_LOWERCASE_SET, ['seT_c']]); // lowercase
Expand All @@ -117,13 +117,13 @@ describe('validateSplitFilters', () => {
expect(loggerMock.warn.mock.calls[6]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set _3', regexp, 'set _3']]); // empty spaces
expect(loggerMock.warn.mock.calls[7]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['_set_2', regexp, '_set_2']]); // start with a letter
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_SPLITS_FILTER_NAME_AND_SET]);
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(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
expect(loggerMock.error.mock.calls[2]).toEqual([ERROR_SPLITS_FILTER_NAME_AND_SET]);
expect(loggerMock.error.mock.calls[2]).toEqual([ERROR_SETS_FILTER_EXCLUSIVE]);

expect(loggerMock.warn.mock.calls.length).toEqual(12);
expect(loggerMock.error.mock.calls.length).toEqual(3);
Expand Down
4 changes: 2 additions & 2 deletions src/utils/settingsValidation/splitFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ 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_SPLITS_FILTER_NAME_AND_SET, WARN_SPLITS_FILTER_LOWERCASE_SET, WARN_SPLITS_FILTER_INVALID_SET, ERROR_EMPTY_ARRAY, WARN_FLAGSET_NOT_CONFIGURED } from '../../logger/constants';
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, ERROR_EMPTY_ARRAY, WARN_FLAGSET_NOT_CONFIGURED } from '../../logger/constants';
import { objectAssign } from '../lang/objectAssign';
import { find, uniq } from '../lang';

Expand Down Expand Up @@ -177,7 +177,7 @@ export function validateSplitFilters(log: ILogger, maybeSplitFilters: any, mode:
const setFilter = configuredFilter(res.validFilters, 'bySet');
// Clean all filters if set filter is present
if (setFilter) {
if (configuredFilter(res.validFilters, 'byName')) log.error(ERROR_SPLITS_FILTER_NAME_AND_SET);
if (configuredFilter(res.validFilters, 'byName') || configuredFilter(res.validFilters, 'byPrefix')) log.error(ERROR_SETS_FILTER_EXCLUSIVE);
objectAssign(res.groupedFilters, { byName: [], byPrefix: [] });
}

Expand Down

0 comments on commit d3a90a3

Please sign in to comment.