Merge pull request #252 from splitio/sdks-7561

[SDKS-7561] Input validation for evaluations

src/logger/constants.ts

@@ -99,6 +99,7 @@ export const STREAMING_PARSING_MY_SEGMENTS_UPDATE_V2 = 223;
 export const STREAMING_PARSING_SPLIT_UPDATE = 224;
 export const WARN_SPLITS_FILTER_INVALID_SET = 225;
 export const WARN_SPLITS_FILTER_LOWERCASE_SET = 226;
+export const WARN_FLAGSET_NOT_CONFIGURED = 227;
 
 export const ERROR_ENGINE_COMBINER_IFELSEIF = 300;
 export const ERROR_LOGLEVEL_INVALID = 301;

src/logger/messages/warn.ts

@@ -24,6 +24,7 @@ export const codesWarn: [number, string][] = codesError.concat([
   [c.WARN_NOT_EXISTENT_SPLIT, '%s: feature flag "%s" does not exist in this environment. Please double check what feature flags exist in the Split user interface.'],
   [c.WARN_LOWERCASE_TRAFFIC_TYPE, '%s: traffic_type_name should be all lowercase - converting string to lowercase.'],
   [c.WARN_NOT_EXISTENT_TT, '%s: traffic type "%s" does not have any corresponding feature flag in this environment, make sure you\'re tracking your events to a valid traffic type defined in the Split user interface.'],
+  [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.'],

src/sdkClient/clientInputValidation.ts

@@ -17,6 +17,7 @@ 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';
 
 /**
  * Decorator that validates the input before actually executing the client methods.
@@ -30,21 +31,22 @@ 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 | string[] | undefined) {
+  function validateEvaluationParams(maybeKey: SplitIO.SplitKey, maybeFeatureFlagNameOrNames: string | string[] | undefined, maybeAttributes: SplitIO.Attributes | undefined, methodName: string, maybeFlagSetNameOrNames?: string[]) {
     const multi = startsWith(methodName, 'getTreatments');
     const key = validateKey(log, maybeKey, methodName);
     let splitOrSplits: string | string[] | false = false;
+    let flagSetOrFlagSets: string[] = [];
     if (maybeFeatureFlagNameOrNames) {
       splitOrSplits = multi ? validateSplits(log, maybeFeatureFlagNameOrNames, methodName) : validateSplit(log, maybeFeatureFlagNameOrNames, methodName);
     }
     const attributes = validateAttributes(log, maybeAttributes, methodName);
     const isNotDestroyed = validateIfNotDestroyed(log, readinessManager, methodName);
-    // @todo maybeFlagSets validation method
-    const flagSetOrFlagSets = maybeFlagSetNameOrNames;
-
+    if (maybeFlagSetNameOrNames) {
+      flagSetOrFlagSets = flagSetsAreValid(log, methodName, maybeFlagSetNameOrNames, settings.sync.__splitFiltersValidation.groupedFilters.bySet);
+    }
     validateIfOperational(log, readinessManager, methodName);
 
-    const valid = isNotDestroyed && key && (splitOrSplits || flagSetOrFlagSets) && attributes !== false;
+    const valid = isNotDestroyed && key && (splitOrSplits || flagSetOrFlagSets.length > 0) && attributes !== false;
 
     return {
       valid,
@@ -126,20 +128,20 @@ 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, 'getTreatmentsByFlagSet', [maybeFlagSet]);
 
     if (params.valid) {
-      return client.getTreatmentsByFlagSet(params.key as SplitIO.SplitKey, params.flagSetOrFlagSets as string, params.attributes as SplitIO.Attributes | undefined);
+      return client.getTreatmentsByFlagSet(params.key as SplitIO.SplitKey, params.flagSetOrFlagSets[0] as string, params.attributes as SplitIO.Attributes | undefined);
     } else {
       return wrapResult({});
     }
   }
 
   function getTreatmentsWithConfigByFlagSet(maybeKey: SplitIO.SplitKey, maybeFlagSet: string, maybeAttributes?: SplitIO.Attributes) {
-    const params = validateEvaluationParams(maybeKey, undefined, maybeAttributes, 'getTreatmentsWithConfigByFlagSet', maybeFlagSet);
+    const params = validateEvaluationParams(maybeKey, undefined, maybeAttributes, 'getTreatmentsWithConfigByFlagSet', [maybeFlagSet]);
 
     if (params.valid) {
-      return client.getTreatmentsWithConfigByFlagSet(params.key as SplitIO.SplitKey, params.flagSetOrFlagSets as string, params.attributes as SplitIO.Attributes | undefined);
+      return client.getTreatmentsWithConfigByFlagSet(params.key as SplitIO.SplitKey, params.flagSetOrFlagSets[0] as string, params.attributes as SplitIO.Attributes | undefined);
     } else {
       return wrapResult({});
     }

src/utils/settingsValidation/__tests__/splitFilters.spec.ts

@@ -6,8 +6,8 @@ import { STANDALONE_MODE, CONSUMER_MODE } from '../../constants';
 import { splitFilters, queryStrings, groupedFilters } from '../../../__tests__/mocks/fetchSpecificSplits';
 
 // Test target
-import { 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 } from '../../../logger/constants';
+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';
 
 describe('validateSplitFilters', () => {
 
@@ -128,4 +128,73 @@ describe('validateSplitFilters', () => {
     expect(loggerMock.warn.mock.calls.length).toEqual(12);
     expect(loggerMock.error.mock.calls.length).toEqual(3);
   });
+
+  test('flagSetsAreValid - Flag set validation for evaluations', () => {
+
+    let flagSetsFilter = ['set_1', 'set_2'];
+
+    // empty array
+    expect(flagSetsAreValid(loggerMock, 'test_method', [], flagSetsFilter)).toEqual([]);
+
+    // must start with a letter or number
+    expect(flagSetsAreValid(loggerMock, 'test_method', ['_set_1'], flagSetsFilter)).toEqual([]);
+    expect(loggerMock.warn.mock.calls[0]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['_set_1', regexp, '_set_1']]);
+
+    // can contain _a-z0-9
+    expect(flagSetsAreValid(loggerMock, 'test_method', ['set*1'], flagSetsFilter)).toEqual([]);
+    expect(loggerMock.warn.mock.calls[1]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*1', regexp, 'set*1']]);
+
+    // have a max length of 50 characters
+    const longName = '1234567890_1234567890_1234567890_1234567890_1234567890';
+    expect(flagSetsAreValid(loggerMock, 'test_method', [longName], flagSetsFilter)).toEqual([]);
+    expect(loggerMock.warn.mock.calls[2]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, [longName, regexp, longName]]);
+
+    // both set names invalid -> empty list & warn
+    expect(flagSetsAreValid(loggerMock, 'test_method', ['set*1','set*3'], flagSetsFilter)).toEqual([]);
+    expect(loggerMock.warn.mock.calls[3]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*1', regexp, 'set*1']]);
+    expect(loggerMock.warn.mock.calls[4]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*3', regexp, 'set*3']]);
+
+    // only set_1 is valid => [set_1] & warn
+    expect(flagSetsAreValid(loggerMock, 'test_method', ['set_1','set*3'], flagSetsFilter)).toEqual(['set_1']);
+    expect(loggerMock.warn.mock.calls[5]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*3', regexp, 'set*3']]);
+
+    // set_3 not included in configuration but set_1 included => [set_1] & warn
+    expect(flagSetsAreValid(loggerMock, 'test_method', ['set_1','set_3'], flagSetsFilter)).toEqual(['set_1']);
+    expect(loggerMock.warn.mock.calls[6]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, ['test_method', 'set_3']]);
+
+    // set_3 not included in configuration => [] & warn
+    expect(flagSetsAreValid(loggerMock, 'test_method', ['set_3'], flagSetsFilter)).toEqual([]);
+    expect(loggerMock.warn.mock.calls[7]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, ['test_method','set_3']]);
+
+    // empty config
+
+
+    // must start with a letter or number
+    expect(flagSetsAreValid(loggerMock, 'test_method', ['_set_1'], [])).toEqual([]);
+    expect(loggerMock.warn.mock.calls[8]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['_set_1', regexp, '_set_1']]);
+
+    // can contain _a-z0-9
+    expect(flagSetsAreValid(loggerMock, 'test_method', ['set*1'], [])).toEqual([]);
+    expect(loggerMock.warn.mock.calls[9]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*1', regexp, 'set*1']]);
+
+    // have a max length of 50 characters
+    expect(flagSetsAreValid(loggerMock, 'test_method', [longName], [])).toEqual([]);
+    expect(loggerMock.warn.mock.calls[10]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, [longName, regexp, longName]]);
+
+    // both set names invalid -> empty list & warn
+    expect(flagSetsAreValid(loggerMock, 'test_method', ['set*1','set*3'], [])).toEqual([]);
+    expect(loggerMock.warn.mock.calls[11]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*1', regexp, 'set*1']]);
+    expect(loggerMock.warn.mock.calls[12]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*3', regexp, 'set*3']]);
+
+    // only set_1 is valid => [set_1] & warn
+    expect(flagSetsAreValid(loggerMock, 'test_method', ['set_1','set*3'], [])).toEqual(['set_1']);
+    expect(loggerMock.warn.mock.calls[13]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*3', regexp, 'set*3']]);
+
+    // any set should be returned if there isn't flag sets in filter
+    expect(flagSetsAreValid(loggerMock, 'test_method', ['set_1'], [])).toEqual(['set_1']);
+    expect(flagSetsAreValid(loggerMock, 'test_method', ['set_1','set_2'], [])).toEqual(['set_1','set_2']);
+    expect(flagSetsAreValid(loggerMock, 'test_method', ['set_3'], [])).toEqual(['set_3']);
+
+  });
+
 });

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 } from '../../logger/constants';
+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 { objectAssign } from '../lang/objectAssign';
 import { find, uniq } from '../lang';
 
@@ -187,3 +187,19 @@ export function validateSplitFilters(log: ILogger, maybeSplitFilters: any, mode:
 
   return res;
 }
+
+export function flagSetsAreValid(log: ILogger, method: string, flagSets: string[], flagSetsInConfig: string[]): string[] {
+  const sets = validateSplits(log, flagSets, method, 'flag sets', 'flag set');
+  let toReturn = sets ? sanitizeFlagSets(log, sets) : [];
+  if (flagSetsInConfig.length > 0) {
+    toReturn = toReturn.filter(flagSet => {
+      if (flagSetsInConfig.indexOf(flagSet) > -1) {
+        return true;
+      }
+      log.warn(WARN_FLAGSET_NOT_CONFIGURED, [method, flagSet]);
+      return false;
+    });
+  }
+
+  return toReturn;
+}