Skip to content

Commit c88d787

Browse files
ZamoraEmmanuelZamoraEmmanuel
authored
Merge pull request #444 from splitio/fme-10567-refactor
[FME-10567] Add fallbackTreatmentCalculator to client
2 parents 381b458 + d9c4cff commit c88d787

File tree

13 files changed

+206
-85
lines changed

13 files changed

+206
-85
lines changed

‎src/evaluator/fallbackTreatmentsCalculator/__tests__/fallback-calculator.spec.ts‎

Lines changed: 95 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,99 @@
11
import { FallbackTreatmentsCalculator } from '../';
2-
import type { FallbackTreatmentConfiguration } from '../../../../types/splitio'; // adjust path if needed
2+
import type { FallbackTreatmentConfiguration } from '../../../../types/splitio';
3+
import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock';
4+
import { CONTROL } from '../../../utils/constants';
35

4-
describe('FallbackTreatmentsCalculator', () => {
6+
describe('FallbackTreatmentsCalculator' , () => {
7+
const longName = 'a'.repeat(101);
8+
9+
test('logs an error if flag name is invalid - by Flag', () => {
10+
let config: FallbackTreatmentConfiguration = {
11+
byFlag: {
12+
'feature A': { treatment: 'TREATMENT_A', config: '{ value: 1 }' },
13+
},
14+
};
15+
new FallbackTreatmentsCalculator(loggerMock, config);
16+
expect(loggerMock.error.mock.calls[0][0]).toBe(
17+
'Fallback treatments - Discarded flag \'feature A\': Invalid flag name (max 100 chars, no spaces)'
18+
);
19+
config = {
20+
byFlag: {
21+
[longName]: { treatment: 'TREATMENT_A', config: '{ value: 1 }' },
22+
},
23+
};
24+
new FallbackTreatmentsCalculator(loggerMock, config);
25+
expect(loggerMock.error.mock.calls[1][0]).toBe(
26+
`Fallback treatments - Discarded flag '${longName}': Invalid flag name (max 100 chars, no spaces)`
27+
);
28+
29+
config = {
30+
byFlag: {
31+
'featureB': { treatment: longName, config: '{ value: 1 }' },
32+
},
33+
};
34+
new FallbackTreatmentsCalculator(loggerMock, config);
35+
expect(loggerMock.error.mock.calls[2][0]).toBe(
36+
'Fallback treatments - Discarded treatment for flag \'featureB\': Invalid treatment (max 100 chars and must match pattern)'
37+
);
38+
39+
config = {
40+
byFlag: {
41+
// @ts-ignore
42+
'featureC': { config: '{ global: true }' },
43+
},
44+
};
45+
new FallbackTreatmentsCalculator(loggerMock, config);
46+
expect(loggerMock.error.mock.calls[3][0]).toBe(
47+
'Fallback treatments - Discarded treatment for flag \'featureC\': Invalid treatment (max 100 chars and must match pattern)'
48+
);
49+
50+
config = {
51+
byFlag: {
52+
// @ts-ignore
53+
'featureC': { treatment: 'invalid treatment!', config: '{ global: true }' },
54+
},
55+
};
56+
new FallbackTreatmentsCalculator(loggerMock, config);
57+
expect(loggerMock.error.mock.calls[4][0]).toBe(
58+
'Fallback treatments - Discarded treatment for flag \'featureC\': Invalid treatment (max 100 chars and must match pattern)'
59+
);
60+
});
61+
62+
test('logs an error if flag name is invalid - global', () => {
63+
let config: FallbackTreatmentConfiguration = {
64+
global: { treatment: longName, config: '{ value: 1 }' },
65+
};
66+
new FallbackTreatmentsCalculator(loggerMock, config);
67+
expect(loggerMock.error.mock.calls[2][0]).toBe(
68+
'Fallback treatments - Discarded treatment for flag \'featureB\': Invalid treatment (max 100 chars and must match pattern)'
69+
);
70+
71+
config = {
72+
// @ts-ignore
73+
global: { config: '{ global: true }' },
74+
};
75+
new FallbackTreatmentsCalculator(loggerMock, config);
76+
expect(loggerMock.error.mock.calls[3][0]).toBe(
77+
'Fallback treatments - Discarded treatment for flag \'featureC\': Invalid treatment (max 100 chars and must match pattern)'
78+
);
79+
80+
config = {
81+
// @ts-ignore
82+
global: { treatment: 'invalid treatment!', config: '{ global: true }' },
83+
};
84+
new FallbackTreatmentsCalculator(loggerMock, config);
85+
expect(loggerMock.error.mock.calls[4][0]).toBe(
86+
'Fallback treatments - Discarded treatment for flag \'featureC\': Invalid treatment (max 100 chars and must match pattern)'
87+
);
88+
});
589

690
test('returns specific fallback if flag exists', () => {
791
const config: FallbackTreatmentConfiguration = {
892
byFlag: {
993
'featureA': { treatment: 'TREATMENT_A', config: '{ value: 1 }' },
1094
},
1195
};
12-
const calculator = new FallbackTreatmentsCalculator(config);
96+
const calculator = new FallbackTreatmentsCalculator(loggerMock, config);
1397
const result = calculator.resolve('featureA', 'label by flag');
1498

1599
expect(result).toEqual({
@@ -24,7 +108,7 @@ describe('FallbackTreatmentsCalculator', () => {
24108
byFlag: {},
25109
global: { treatment: 'GLOBAL_TREATMENT', config: '{ global: true }' },
26110
};
27-
const calculator = new FallbackTreatmentsCalculator(config);
111+
const calculator = new FallbackTreatmentsCalculator(loggerMock, config);
28112
const result = calculator.resolve('missingFlag', 'label by global');
29113

30114
expect(result).toEqual({
@@ -38,29 +122,29 @@ describe('FallbackTreatmentsCalculator', () => {
38122
const config: FallbackTreatmentConfiguration = {
39123
byFlag: {},
40124
};
41-
const calculator = new FallbackTreatmentsCalculator(config);
125+
const calculator = new FallbackTreatmentsCalculator(loggerMock, config);
42126
const result = calculator.resolve('missingFlag', 'label by noFallback');
43127

44128
expect(result).toEqual({
45-
treatment: 'CONTROL',
129+
treatment: CONTROL,
46130
config: null,
47-
label: 'fallback - label by noFallback',
131+
label: 'label by noFallback',
48132
});
49133
});
50134

51135
test('returns undefined label if no label provided', () => {
52136
const config: FallbackTreatmentConfiguration = {
53137
byFlag: {
54-
'featureB': { treatment: 'TREATMENT_B' },
138+
'featureB': { treatment: 'TREATMENT_B', config: '{ value: 1 }' },
55139
},
56140
};
57-
const calculator = new FallbackTreatmentsCalculator(config);
141+
const calculator = new FallbackTreatmentsCalculator(loggerMock, config);
58142
const result = calculator.resolve('featureB');
59143

60144
expect(result).toEqual({
61145
treatment: 'TREATMENT_B',
62-
config: undefined,
63-
label: undefined,
146+
config: '{ value: 1 }',
147+
label: '',
64148
});
65149
});
66150
});

‎src/evaluator/fallbackTreatmentsCalculator/__tests__/fallback-sanitizer.spec.ts‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { FallbacksSanitizer } from '../fallbackSanitizer';
22
import { TreatmentWithConfig } from '../../../../types/splitio';
3+
import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock';
34

45
describe('FallbacksSanitizer', () => {
56
const validTreatment: TreatmentWithConfig = { treatment: 'on', config: '{"color":"blue"}' };
@@ -10,7 +11,7 @@ describe('FallbacksSanitizer', () => {
1011
});
1112

1213
afterEach(() => {
13-
(console.error as jest.Mock).mockRestore();
14+
(loggerMock.error as jest.Mock).mockRestore();
1415
});
1516

1617
describe('isValidFlagName', () => {
@@ -52,14 +53,14 @@ describe('FallbacksSanitizer', () => {
5253

5354
describe('sanitizeGlobal', () => {
5455
test('returns the treatment if valid', () => {
55-
expect(FallbacksSanitizer.sanitizeGlobal(validTreatment)).toEqual(validTreatment);
56-
expect(console.error).not.toHaveBeenCalled();
56+
expect(FallbacksSanitizer.sanitizeGlobal(loggerMock, validTreatment)).toEqual(validTreatment);
57+
expect(loggerMock.error).not.toHaveBeenCalled();
5758
});
5859

5960
test('returns undefined and logs error if invalid', () => {
60-
const result = FallbacksSanitizer.sanitizeGlobal(invalidTreatment);
61+
const result = FallbacksSanitizer.sanitizeGlobal(loggerMock, invalidTreatment);
6162
expect(result).toBeUndefined();
62-
expect(console.error).toHaveBeenCalledWith(
63+
expect(loggerMock.error).toHaveBeenCalledWith(
6364
expect.stringContaining('Fallback treatments - Discarded fallback')
6465
);
6566
});
@@ -73,20 +74,20 @@ describe('FallbacksSanitizer', () => {
7374
bad_treatment: invalidTreatment,
7475
};
7576

76-
const result = FallbacksSanitizer.sanitizeByFlag(input);
77+
const result = FallbacksSanitizer.sanitizeByFlag(loggerMock, input);
7778

7879
expect(result).toEqual({ valid_flag: validTreatment });
79-
expect(console.error).toHaveBeenCalledTimes(2); // invalid flag + bad_treatment
80+
expect(loggerMock.error).toHaveBeenCalledTimes(2); // invalid flag + bad_treatment
8081
});
8182

8283
test('returns empty object if all invalid', () => {
8384
const input = {
8485
'invalid flag': invalidTreatment,
8586
};
8687

87-
const result = FallbacksSanitizer.sanitizeByFlag(input);
88+
const result = FallbacksSanitizer.sanitizeByFlag(loggerMock, input);
8889
expect(result).toEqual({});
89-
expect(console.error).toHaveBeenCalled();
90+
expect(loggerMock.error).toHaveBeenCalled();
9091
});
9192

9293
test('returns same object if all valid', () => {
@@ -95,9 +96,9 @@ describe('FallbacksSanitizer', () => {
9596
flag_two: { treatment: 'valid_2', config: null },
9697
};
9798

98-
const result = FallbacksSanitizer.sanitizeByFlag(input);
99+
const result = FallbacksSanitizer.sanitizeByFlag(loggerMock, input);
99100
expect(result).toEqual(input);
100-
expect(console.error).not.toHaveBeenCalled();
101+
expect(loggerMock.error).not.toHaveBeenCalled();
101102
});
102103
});
103104
});

‎src/evaluator/fallbackTreatmentsCalculator/fallbackSanitizer/index.ts‎

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
import { FallbackTreatment } from '../../../../types/splitio';
1+
import { FallbackTreatment, Treatment, TreatmentWithConfig } from '../../../../types/splitio';
2+
import { ILogger } from '../../../logger/types';
3+
import { isObject, isString } from '../../../utils/lang';
24
import { FallbackDiscardReason } from '../constants';
35

46

@@ -10,51 +12,43 @@ export class FallbacksSanitizer {
1012
return name.length <= 100 && !name.includes(' ');
1113
}
1214

13-
private static isValidTreatment(t?: FallbackTreatment): boolean {
14-
if (!t) {
15-
return false;
16-
}
17-
18-
if (typeof t === 'string') {
19-
if (t.length > 100) {
20-
return false;
21-
}
22-
return FallbacksSanitizer.pattern.test(t);
23-
}
15+
private static isValidTreatment(t?: Treatment | FallbackTreatment): boolean {
16+
const treatment = isObject(t) ? (t as TreatmentWithConfig).treatment : t;
2417

25-
const { treatment } = t;
26-
if (!treatment || treatment.length > 100) {
18+
if (!isString(treatment) || treatment.length > 100) {
2719
return false;
2820
}
2921
return FallbacksSanitizer.pattern.test(treatment);
3022
}
3123

32-
static sanitizeGlobal(treatment?: FallbackTreatment): FallbackTreatment | undefined {
24+
static sanitizeGlobal(logger: ILogger, treatment?: string | FallbackTreatment): string | FallbackTreatment | undefined {
3325
if (!this.isValidTreatment(treatment)) {
34-
console.error(
26+
logger.error(
3527
`Fallback treatments - Discarded fallback: ${FallbackDiscardReason.Treatment}`
3628
);
3729
return undefined;
3830
}
39-
return treatment!;
31+
return treatment;
4032
}
4133

4234
static sanitizeByFlag(
43-
byFlagFallbacks: Record<string, FallbackTreatment>
44-
): Record<string, FallbackTreatment> {
45-
const sanitizedByFlag: Record<string, FallbackTreatment> = {};
46-
47-
const entries = Object.entries(byFlagFallbacks);
48-
entries.forEach(([flag, t]) => {
35+
logger: ILogger,
36+
byFlagFallbacks: Record<string, string | FallbackTreatment>
37+
): Record<string, string | FallbackTreatment> {
38+
const sanitizedByFlag: Record<string, string | FallbackTreatment> = {};
39+
40+
const entries = Object.keys(byFlagFallbacks);
41+
entries.forEach((flag) => {
42+
const t = byFlagFallbacks[flag];
4943
if (!this.isValidFlagName(flag)) {
50-
console.error(
44+
logger.error(
5145
`Fallback treatments - Discarded flag '${flag}': ${FallbackDiscardReason.FlagName}`
5246
);
5347
return;
5448
}
5549

5650
if (!this.isValidTreatment(t)) {
57-
console.error(
51+
logger.error(
5852
`Fallback treatments - Discarded treatment for flag '${flag}': ${FallbackDiscardReason.Treatment}`
5953
);
6054
return;

‎src/evaluator/fallbackTreatmentsCalculator/index.ts‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,28 @@
1-
import { FallbackTreatmentConfiguration, FallbackTreatment, IFallbackTreatmentsCalculator} from '../../../types/splitio';
1+
import { FallbackTreatmentConfiguration, FallbackTreatment } from '../../../types/splitio';
2+
import { FallbacksSanitizer } from './fallbackSanitizer';
3+
import { CONTROL } from '../../utils/constants';
4+
import { isString } from '../../utils/lang';
5+
import { ILogger } from '../../logger/types';
6+
7+
export type IFallbackTreatmentsCalculator = {
8+
resolve(flagName: string, label?: string): FallbackTreatment & { label?: string };
9+
}
210

311
export class FallbackTreatmentsCalculator implements IFallbackTreatmentsCalculator {
412
private readonly labelPrefix = 'fallback - ';
5-
private readonly control = 'CONTROL';
613
private readonly fallbacks: FallbackTreatmentConfiguration;
714

8-
constructor(fallbacks: FallbackTreatmentConfiguration) {
9-
this.fallbacks = fallbacks;
15+
constructor(logger: ILogger, fallbacks?: FallbackTreatmentConfiguration) {
16+
const sanitizedGlobal = fallbacks?.global ? FallbacksSanitizer.sanitizeGlobal(logger, fallbacks.global) : undefined;
17+
const sanitizedByFlag = fallbacks?.byFlag ? FallbacksSanitizer.sanitizeByFlag(logger, fallbacks.byFlag) : {};
18+
this.fallbacks = {
19+
global: sanitizedGlobal,
20+
byFlag: sanitizedByFlag
21+
};
1022
}
1123

12-
resolve(flagName: string, label?: string | undefined): FallbackTreatment {
13-
const treatment = this.fallbacks.byFlag[flagName];
24+
resolve(flagName: string, label?: string): FallbackTreatment & { label?: string } {
25+
const treatment = this.fallbacks.byFlag?.[flagName];
1426
if (treatment) {
1527
return this.copyWithLabel(treatment, label);
1628
}
@@ -20,14 +32,14 @@ export class FallbackTreatmentsCalculator implements IFallbackTreatmentsCalculat
2032
}
2133

2234
return {
23-
treatment: this.control,
35+
treatment: CONTROL,
2436
config: null,
25-
label: this.resolveLabel(label),
37+
label,
2638
};
2739
}
2840

29-
private copyWithLabel(fallback: FallbackTreatment, label: string | undefined): FallbackTreatment {
30-
if (typeof fallback === 'string') {
41+
private copyWithLabel(fallback: string | FallbackTreatment, label?: string): FallbackTreatment & { label: string } {
42+
if (isString(fallback)) {
3143
return {
3244
treatment: fallback,
3345
config: null,
@@ -42,8 +54,8 @@ export class FallbackTreatmentsCalculator implements IFallbackTreatmentsCalculat
4254
};
4355
}
4456

45-
private resolveLabel(label?: string | undefined): string | undefined {
46-
return label ? `${this.labelPrefix}${label}` : undefined;
57+
private resolveLabel(label?: string): string {
58+
return label ? `${this.labelPrefix}${label}` : '';
4759
}
4860

4961
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { clientInputValidationDecorator } from '../clientInputValidation';
44
// Mocks
55
import { DebugLogger } from '../../logger/browser/DebugLogger';
66
import { createClientMock } from './testUtils';
7+
import { FallbackTreatmentsCalculator, IFallbackTreatmentsCalculator } from '../../evaluator/fallbackTreatmentsCalculator';
78

89
const settings: any = {
910
log: DebugLogger(),
@@ -13,13 +14,15 @@ const settings: any = {
1314
const EVALUATION_RESULT = 'on';
1415
const client: any = createClientMock(EVALUATION_RESULT);
1516

17+
const fallbackTreatmentsCalculator: IFallbackTreatmentsCalculator = new FallbackTreatmentsCalculator(settings);
18+
1619
const readinessManager: any = {
1720
isReady: () => true,
1821
isDestroyed: () => false
1922
};
2023

2124
describe('clientInputValidationDecorator', () => {
22-
const clientWithValidation = clientInputValidationDecorator(settings, client, readinessManager);
25+
const clientWithValidation = clientInputValidationDecorator(settings, client, readinessManager, fallbackTreatmentsCalculator);
2326
const logSpy = jest.spyOn(console, 'log');
2427

2528
beforeEach(() => {

0 commit comments

Comments
 (0)