Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Update logic & error log when using sets with other filters #253

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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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, 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, 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