From d3a90a30d8aad05c144219a648dfb1be7f4530b1 Mon Sep 17 00:00:00 2001 From: Emmanuel Zamora Date: Thu, 28 Sep 2023 11:52:08 -0300 Subject: [PATCH] Update logic & error log when using sets with other filters --- package-lock.json | 4 ++-- package.json | 2 +- src/logger/constants.ts | 2 +- src/logger/messages/error.ts | 2 +- src/sync/polling/updaters/splitChangesUpdater.ts | 14 +++++++++++--- .../__tests__/splitFilters.spec.ts | 8 ++++---- src/utils/settingsValidation/splitFilters.ts | 4 ++-- 7 files changed, 22 insertions(+), 14 deletions(-) diff --git a/package-lock.json b/package-lock.json index b345dd22..7b73fa71 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.9.1-rc.0", + "version": "1.9.2-rc.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-commons", - "version": "1.9.1-rc.0", + "version": "1.9.2-rc.0", "license": "Apache-2.0", "dependencies": { "tslib": "^2.3.1" diff --git a/package.json b/package.json index 543c6582..b3f4afa1 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/src/logger/constants.ts b/src/logger/constants.ts index 4ac3d64a..961a1a98 100644 --- a/src/logger/constants.ts +++ b/src/logger/constants.ts @@ -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'; diff --git a/src/logger/messages/error.ts b/src/logger/messages/error.ts index 91f2481f..a75dbccb 100644 --- a/src/logger/messages/error.ts +++ b/src/logger/messages/error.ts @@ -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.'], ]; diff --git a/src/sync/polling/updaters/splitChangesUpdater.ts b/src/sync/polling/updaters/splitChangesUpdater.ts index ccbab176..3a7e616a 100644 --- a/src/sync/polling/updaters/splitChangesUpdater.ts +++ b/src/sync/polling/updaters/splitChangesUpdater.ts @@ -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 @@ -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; } /** diff --git a/src/utils/settingsValidation/__tests__/splitFilters.spec.ts b/src/utils/settingsValidation/__tests__/splitFilters.spec.ts index 8d6b80ca..0e042faa 100644 --- a/src/utils/settingsValidation/__tests__/splitFilters.spec.ts +++ b/src/utils/settingsValidation/__tests__/splitFilters.spec.ts @@ -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', () => { @@ -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 @@ -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); diff --git a/src/utils/settingsValidation/splitFilters.ts b/src/utils/settingsValidation/splitFilters.ts index 1df96f1b..06c9de8b 100644 --- a/src/utils/settingsValidation/splitFilters.ts +++ b/src/utils/settingsValidation/splitFilters.ts @@ -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'; @@ -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: [] }); }