Skip to content

Commit e5eca00

Browse files
EmilianoSanchezEmilianoSanchez
authored
Merge pull request #313 from splitio/input_validation_update
Input validation update: log error when an invalid value is provided as feature flag and flag set name
2 parents fe27372 + 244c954 commit e5eca00

File tree

7 files changed

+99
-44
lines changed

7 files changed

+99
-44
lines changed

‎CHANGES.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
1.15.1 (May 28, 2024)
2+
- Bugfixing - Restored some input validation error logs that were removed in version 1.12.0. The logs inform the user when the `getTreatment(s)` methods are called with an invalid value as feature flag name or flag set name.
3+
14
1.15.0 (May 13, 2024)
25
- Added an optional settings validation parameter to let overwrite the default flag spec version, used by the JS Synchronizer.
36

‎package-lock.json‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@splitsoftware/splitio-commons",
3-
"version": "1.15.0",
3+
"version": "1.15.1-rc.0",
44
"description": "Split JavaScript SDK common components",
55
"main": "cjs/index.js",
66
"module": "esm/index.js",

‎src/sdkClient/__tests__/clientInputValidation.spec.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// Test target
2+
import { clientInputValidationDecorator } from '../clientInputValidation';
3+
4+
// Mocks
5+
import { DebugLogger } from '../../logger/browser/DebugLogger';
6+
7+
const settings: any = {
8+
log: DebugLogger(),
9+
sync: { __splitFiltersValidation: { groupedFilters: { bySet: [] } } }
10+
};
11+
12+
const client: any = {};
13+
14+
const readinessManager: any = {
15+
isReady: () => true,
16+
isDestroyed: () => false
17+
};
18+
19+
describe('clientInputValidationDecorator', () => {
20+
const clientWithValidation = clientInputValidationDecorator(settings, client, readinessManager);
21+
const logSpy = jest.spyOn(console, 'log');
22+
23+
beforeEach(() => {
24+
logSpy.mockClear();
25+
});
26+
27+
test('should return control and log an error if the passed 2nd argument (feature flag(s) or flag set(s)) is invalid', () => {
28+
expect(clientWithValidation.getTreatment('key')).toEqual('control');
29+
expect(logSpy).toHaveBeenLastCalledWith('[ERROR] splitio => getTreatment: you passed a null or undefined feature flag name. It must be a non-empty string.');
30+
31+
expect(clientWithValidation.getTreatmentWithConfig('key', [])).toEqual({ treatment: 'control', config: null });
32+
expect(logSpy).toHaveBeenLastCalledWith('[ERROR] splitio => getTreatmentWithConfig: you passed an invalid feature flag name. It must be a non-empty string.');
33+
34+
expect(clientWithValidation.getTreatments('key')).toEqual({});
35+
expect(logSpy).toHaveBeenLastCalledWith('[ERROR] splitio => getTreatments: feature flag names must be a non-empty array.');
36+
37+
expect(clientWithValidation.getTreatmentsWithConfig('key', [])).toEqual({});
38+
expect(logSpy).toHaveBeenLastCalledWith('[ERROR] splitio => getTreatmentsWithConfig: feature flag names must be a non-empty array.');
39+
40+
expect(clientWithValidation.getTreatmentsByFlagSet('key')).toEqual({});
41+
expect(logSpy).toBeCalledWith('[ERROR] splitio => getTreatmentsByFlagSet: you passed a null or undefined flag set. It must be a non-empty string.');
42+
43+
expect(clientWithValidation.getTreatmentsWithConfigByFlagSet('key', [])).toEqual({});
44+
expect(logSpy).toBeCalledWith('[ERROR] splitio => getTreatmentsWithConfigByFlagSet: you passed an invalid flag set. It must be a non-empty string.');
45+
46+
expect(clientWithValidation.getTreatmentsByFlagSets('key')).toEqual({});
47+
expect(logSpy).toHaveBeenLastCalledWith('[ERROR] splitio => getTreatmentsByFlagSets: flag sets must be a non-empty array.');
48+
49+
expect(clientWithValidation.getTreatmentsWithConfigByFlagSets('key', [])).toEqual({});
50+
expect(logSpy).toHaveBeenLastCalledWith('[ERROR] splitio => getTreatmentsWithConfigByFlagSets: flag sets must be a non-empty array.');
51+
52+
// @TODO should be 8, but there is an additional log from `getTreatmentsByFlagSet` and `getTreatmentsWithConfigByFlagSet` that should be removed
53+
expect(logSpy).toBeCalledTimes(10);
54+
});
55+
});

‎src/sdkClient/clientInputValidation.ts‎

Lines changed: 25 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -31,29 +31,26 @@ export function clientInputValidationDecorator<TClient extends SplitIO.IClient |
3131
/**
3232
* Avoid repeating this validations code
3333
*/
34-
function validateEvaluationParams(maybeKey: SplitIO.SplitKey, maybeFeatureFlagNameOrNames: string | string[] | undefined, maybeAttributes: SplitIO.Attributes | undefined, methodName: string, maybeFlagSetNameOrNames?: string[]) {
35-
const multi = startsWith(methodName, GET_TREATMENTS);
34+
function validateEvaluationParams(maybeKey: SplitIO.SplitKey, maybeNameOrNames: string | string[], maybeAttributes: SplitIO.Attributes | undefined, methodName: string) {
3635
const key = validateKey(log, maybeKey, methodName);
37-
let splitOrSplits: string | string[] | false = false;
38-
let flagSetOrFlagSets: string[] = [];
39-
if (maybeFeatureFlagNameOrNames) {
40-
splitOrSplits = multi ? validateSplits(log, maybeFeatureFlagNameOrNames, methodName) : validateSplit(log, maybeFeatureFlagNameOrNames, methodName);
41-
}
36+
37+
const nameOrNames = methodName.indexOf('ByFlagSet') > -1 ?
38+
validateFlagSets(log, methodName, maybeNameOrNames as string[], settings.sync.__splitFiltersValidation.groupedFilters.bySet) :
39+
startsWith(methodName, GET_TREATMENTS) ?
40+
validateSplits(log, maybeNameOrNames, methodName) :
41+
validateSplit(log, maybeNameOrNames, methodName);
42+
4243
const attributes = validateAttributes(log, maybeAttributes, methodName);
4344
const isNotDestroyed = validateIfNotDestroyed(log, readinessManager, methodName);
44-
if (maybeFlagSetNameOrNames) {
45-
flagSetOrFlagSets = validateFlagSets(log, methodName, maybeFlagSetNameOrNames, settings.sync.__splitFiltersValidation.groupedFilters.bySet);
46-
}
4745

48-
validateIfOperational(log, readinessManager, methodName, splitOrSplits);
46+
validateIfOperational(log, readinessManager, methodName, nameOrNames);
4947

50-
const valid = isNotDestroyed && key && (splitOrSplits || flagSetOrFlagSets.length > 0) && attributes !== false;
48+
const valid = isNotDestroyed && key && nameOrNames && attributes !== false;
5149

5250
return {
5351
valid,
5452
key,
55-
splitOrSplits,
56-
flagSetOrFlagSets,
53+
nameOrNames,
5754
attributes
5855
};
5956
}
@@ -66,7 +63,7 @@ export function clientInputValidationDecorator<TClient extends SplitIO.IClient |
6663
const params = validateEvaluationParams(maybeKey, maybeFeatureFlagName, maybeAttributes, GET_TREATMENT);
6764

6865
if (params.valid) {
69-
return client.getTreatment(params.key as SplitIO.SplitKey, params.splitOrSplits as string, params.attributes as SplitIO.Attributes | undefined);
66+
return client.getTreatment(params.key as SplitIO.SplitKey, params.nameOrNames as string, params.attributes as SplitIO.Attributes | undefined);
7067
} else {
7168
return wrapResult(CONTROL);
7269
}
@@ -76,7 +73,7 @@ export function clientInputValidationDecorator<TClient extends SplitIO.IClient |
7673
const params = validateEvaluationParams(maybeKey, maybeFeatureFlagName, maybeAttributes, GET_TREATMENT_WITH_CONFIG);
7774

7875
if (params.valid) {
79-
return client.getTreatmentWithConfig(params.key as SplitIO.SplitKey, params.splitOrSplits as string, params.attributes as SplitIO.Attributes | undefined);
76+
return client.getTreatmentWithConfig(params.key as SplitIO.SplitKey, params.nameOrNames as string, params.attributes as SplitIO.Attributes | undefined);
8077
} else {
8178
return wrapResult(objectAssign({}, CONTROL_WITH_CONFIG));
8279
}
@@ -86,10 +83,10 @@ export function clientInputValidationDecorator<TClient extends SplitIO.IClient |
8683
const params = validateEvaluationParams(maybeKey, maybeFeatureFlagNames, maybeAttributes, GET_TREATMENTS);
8784

8885
if (params.valid) {
89-
return client.getTreatments(params.key as SplitIO.SplitKey, params.splitOrSplits as string[], params.attributes as SplitIO.Attributes | undefined);
86+
return client.getTreatments(params.key as SplitIO.SplitKey, params.nameOrNames as string[], params.attributes as SplitIO.Attributes | undefined);
9087
} else {
9188
const res: SplitIO.Treatments = {};
92-
if (params.splitOrSplits) (params.splitOrSplits as string[]).forEach((split: string) => res[split] = CONTROL);
89+
if (params.nameOrNames) (params.nameOrNames as string[]).forEach((split: string) => res[split] = CONTROL);
9390

9491
return wrapResult(res);
9592
}
@@ -99,50 +96,50 @@ export function clientInputValidationDecorator<TClient extends SplitIO.IClient |
9996
const params = validateEvaluationParams(maybeKey, maybeFeatureFlagNames, maybeAttributes, GET_TREATMENTS_WITH_CONFIG);
10097

10198
if (params.valid) {
102-
return client.getTreatmentsWithConfig(params.key as SplitIO.SplitKey, params.splitOrSplits as string[], params.attributes as SplitIO.Attributes | undefined);
99+
return client.getTreatmentsWithConfig(params.key as SplitIO.SplitKey, params.nameOrNames as string[], params.attributes as SplitIO.Attributes | undefined);
103100
} else {
104101
const res: SplitIO.TreatmentsWithConfig = {};
105-
if (params.splitOrSplits) (params.splitOrSplits as string[]).forEach(split => res[split] = objectAssign({}, CONTROL_WITH_CONFIG));
102+
if (params.nameOrNames) (params.nameOrNames as string[]).forEach(split => res[split] = objectAssign({}, CONTROL_WITH_CONFIG));
106103

107104
return wrapResult(res);
108105
}
109106
}
110107

111108
function getTreatmentsByFlagSets(maybeKey: SplitIO.SplitKey, maybeFlagSets: string[], maybeAttributes?: SplitIO.Attributes) {
112-
const params = validateEvaluationParams(maybeKey, undefined, maybeAttributes, GET_TREATMENTS_BY_FLAG_SETS, maybeFlagSets);
109+
const params = validateEvaluationParams(maybeKey, maybeFlagSets, maybeAttributes, GET_TREATMENTS_BY_FLAG_SETS);
113110

114111
if (params.valid) {
115-
return client.getTreatmentsByFlagSets(params.key as SplitIO.SplitKey, params.flagSetOrFlagSets as string[], params.attributes as SplitIO.Attributes | undefined);
112+
return client.getTreatmentsByFlagSets(params.key as SplitIO.SplitKey, params.nameOrNames as string[], params.attributes as SplitIO.Attributes | undefined);
116113
} else {
117114
return wrapResult({});
118115
}
119116
}
120117

121118
function getTreatmentsWithConfigByFlagSets(maybeKey: SplitIO.SplitKey, maybeFlagSets: string[], maybeAttributes?: SplitIO.Attributes) {
122-
const params = validateEvaluationParams(maybeKey, undefined, maybeAttributes, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SETS, maybeFlagSets);
119+
const params = validateEvaluationParams(maybeKey, maybeFlagSets, maybeAttributes, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SETS);
123120

124121
if (params.valid) {
125-
return client.getTreatmentsWithConfigByFlagSets(params.key as SplitIO.SplitKey, params.flagSetOrFlagSets as string[], params.attributes as SplitIO.Attributes | undefined);
122+
return client.getTreatmentsWithConfigByFlagSets(params.key as SplitIO.SplitKey, params.nameOrNames as string[], params.attributes as SplitIO.Attributes | undefined);
126123
} else {
127124
return wrapResult({});
128125
}
129126
}
130127

131128
function getTreatmentsByFlagSet(maybeKey: SplitIO.SplitKey, maybeFlagSet: string, maybeAttributes?: SplitIO.Attributes) {
132-
const params = validateEvaluationParams(maybeKey, undefined, maybeAttributes, GET_TREATMENTS_BY_FLAG_SET, [maybeFlagSet]);
129+
const params = validateEvaluationParams(maybeKey, [maybeFlagSet], maybeAttributes, GET_TREATMENTS_BY_FLAG_SET);
133130

134131
if (params.valid) {
135-
return client.getTreatmentsByFlagSet(params.key as SplitIO.SplitKey, params.flagSetOrFlagSets[0] as string, params.attributes as SplitIO.Attributes | undefined);
132+
return client.getTreatmentsByFlagSet(params.key as SplitIO.SplitKey, (params.nameOrNames as string[])[0], params.attributes as SplitIO.Attributes | undefined);
136133
} else {
137134
return wrapResult({});
138135
}
139136
}
140137

141138
function getTreatmentsWithConfigByFlagSet(maybeKey: SplitIO.SplitKey, maybeFlagSet: string, maybeAttributes?: SplitIO.Attributes) {
142-
const params = validateEvaluationParams(maybeKey, undefined, maybeAttributes, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SET, [maybeFlagSet]);
139+
const params = validateEvaluationParams(maybeKey, [maybeFlagSet], maybeAttributes, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SET);
143140

144141
if (params.valid) {
145-
return client.getTreatmentsWithConfigByFlagSet(params.key as SplitIO.SplitKey, params.flagSetOrFlagSets[0] as string, params.attributes as SplitIO.Attributes | undefined);
142+
return client.getTreatmentsWithConfigByFlagSet(params.key as SplitIO.SplitKey, (params.nameOrNames as string[])[0], params.attributes as SplitIO.Attributes | undefined);
146143
} else {
147144
return wrapResult({});
148145
}

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -136,24 +136,24 @@ describe('validateSplitFilters', () => {
136136
const flagSetsFilter = ['set_1', 'set_2'];
137137
const METHOD_NAME = 'test_method';
138138

139-
// empty array
140-
expect(validateFlagSets(loggerMock, METHOD_NAME, [], flagSetsFilter)).toEqual([]);
139+
// false
140+
expect(validateFlagSets(loggerMock, METHOD_NAME, [], flagSetsFilter)).toEqual(false);
141141

142142
// must start with a letter or number
143-
expect(validateFlagSets(loggerMock, METHOD_NAME, ['_set_1'], flagSetsFilter)).toEqual([]);
143+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['_set_1'], flagSetsFilter)).toEqual(false);
144144
expect(loggerMock.warn.mock.calls[0]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, '_set_1', regexp, '_set_1']]);
145145

146146
// can contain _a-z0-9
147-
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set*1'], flagSetsFilter)).toEqual([]);
147+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set*1'], flagSetsFilter)).toEqual(false);
148148
expect(loggerMock.warn.mock.calls[1]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, 'set*1', regexp, 'set*1']]);
149149

150150
// have a max length of 50 characters
151151
const longName = '1234567890_1234567890_1234567890_1234567890_1234567890';
152-
expect(validateFlagSets(loggerMock, METHOD_NAME, [longName], flagSetsFilter)).toEqual([]);
152+
expect(validateFlagSets(loggerMock, METHOD_NAME, [longName], flagSetsFilter)).toEqual(false);
153153
expect(loggerMock.warn.mock.calls[2]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, longName, regexp, longName]]);
154154

155155
// both set names invalid -> empty list & warn
156-
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set*1', 'set*3'], flagSetsFilter)).toEqual([]);
156+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set*1', 'set*3'], flagSetsFilter)).toEqual(false);
157157
expect(loggerMock.warn.mock.calls[3]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, 'set*1', regexp, 'set*1']]);
158158
expect(loggerMock.warn.mock.calls[4]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, 'set*3', regexp, 'set*3']]);
159159

@@ -166,26 +166,26 @@ describe('validateSplitFilters', () => {
166166
expect(loggerMock.warn.mock.calls[6]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, [METHOD_NAME, 'set_3']]);
167167

168168
// set_3 not included in configuration => [] & warn
169-
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set_3'], flagSetsFilter)).toEqual([]);
169+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set_3'], flagSetsFilter)).toEqual(false);
170170
expect(loggerMock.warn.mock.calls[7]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, [METHOD_NAME, 'set_3']]);
171171

172172
// empty config
173173

174174

175175
// must start with a letter or number
176-
expect(validateFlagSets(loggerMock, METHOD_NAME, ['_set_1'], [])).toEqual([]);
176+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['_set_1'], [])).toEqual(false);
177177
expect(loggerMock.warn.mock.calls[8]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, '_set_1', regexp, '_set_1']]);
178178

179179
// can contain _a-z0-9
180-
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set*1'], [])).toEqual([]);
180+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set*1'], [])).toEqual(false);
181181
expect(loggerMock.warn.mock.calls[9]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, 'set*1', regexp, 'set*1']]);
182182

183183
// have a max length of 50 characters
184-
expect(validateFlagSets(loggerMock, METHOD_NAME, [longName], [])).toEqual([]);
184+
expect(validateFlagSets(loggerMock, METHOD_NAME, [longName], [])).toEqual(false);
185185
expect(loggerMock.warn.mock.calls[10]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, longName, regexp, longName]]);
186186

187187
// both set names invalid -> empty list & warn
188-
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set*1', 'set*3'], [])).toEqual([]);
188+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set*1', 'set*3'], [])).toEqual(false);
189189
expect(loggerMock.warn.mock.calls[11]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, 'set*1', regexp, 'set*1']]);
190190
expect(loggerMock.warn.mock.calls[12]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, 'set*3', regexp, 'set*3']]);
191191

‎src/utils/settingsValidation/splitFilters.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ export function validateSplitFilters(log: ILogger, maybeSplitFilters: any, mode:
189189
return res;
190190
}
191191

192-
export function validateFlagSets(log: ILogger, method: string, flagSets: string[], flagSetsInConfig: string[]): string[] {
192+
export function validateFlagSets(log: ILogger, method: string, flagSets: string[], flagSetsInConfig: string[]): string[] | false {
193193
const sets = validateSplits(log, flagSets, method, 'flag sets', 'flag set');
194194
let toReturn = sets ? sanitizeFlagSets(log, sets, method) : [];
195195
if (flagSetsInConfig.length > 0) {
@@ -202,5 +202,5 @@ export function validateFlagSets(log: ILogger, method: string, flagSets: string[
202202
});
203203
}
204204

205-
return toReturn;
205+
return toReturn.length ? toReturn : false;
206206
}

0 commit comments

Comments
 (0)