Skip to content

Commit f6dcac3

Browse files
ZamoraEmmanuelZamoraEmmanuel
committed
review changes
1 parent 70f4a23 commit f6dcac3

File tree

9 files changed

+71
-78
lines changed

9 files changed

+71
-78
lines changed

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

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { FallbackTreatmentsCalculator } from '../';
22
import type { FallbackTreatmentConfiguration } from '../../../../types/splitio';
3+
import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock';
34
import { CONTROL } from '../../../utils/constants';
45

5-
describe('FallbackTreatmentsCalculator', () => {
6-
const spy = jest.spyOn(console, 'error').mockImplementation(() => {});
6+
describe('FallbackTreatmentsCalculator' , () => {
77
const longName = 'a'.repeat(101);
88

99
test('logs an error if flag name is invalid - by Flag', () => {
@@ -12,17 +12,17 @@ describe('FallbackTreatmentsCalculator', () => {
1212
'feature A': { treatment: 'TREATMENT_A', config: '{ value: 1 }' },
1313
},
1414
};
15-
new FallbackTreatmentsCalculator(config);
16-
expect(spy.mock.calls[0][0]).toBe(
15+
new FallbackTreatmentsCalculator(loggerMock, config);
16+
expect(loggerMock.error.mock.calls[0][0]).toBe(
1717
'Fallback treatments - Discarded flag \'feature A\': Invalid flag name (max 100 chars, no spaces)'
1818
);
1919
config = {
2020
byFlag: {
2121
[longName]: { treatment: 'TREATMENT_A', config: '{ value: 1 }' },
2222
},
2323
};
24-
new FallbackTreatmentsCalculator(config);
25-
expect(spy.mock.calls[1][0]).toBe(
24+
new FallbackTreatmentsCalculator(loggerMock, config);
25+
expect(loggerMock.error.mock.calls[1][0]).toBe(
2626
`Fallback treatments - Discarded flag '${longName}': Invalid flag name (max 100 chars, no spaces)`
2727
);
2828

@@ -31,8 +31,8 @@ describe('FallbackTreatmentsCalculator', () => {
3131
'featureB': { treatment: longName, config: '{ value: 1 }' },
3232
},
3333
};
34-
new FallbackTreatmentsCalculator(config);
35-
expect(spy.mock.calls[2][0]).toBe(
34+
new FallbackTreatmentsCalculator(loggerMock, config);
35+
expect(loggerMock.error.mock.calls[2][0]).toBe(
3636
'Fallback treatments - Discarded treatment for flag \'featureB\': Invalid treatment (max 100 chars and must match pattern)'
3737
);
3838

@@ -42,8 +42,8 @@ describe('FallbackTreatmentsCalculator', () => {
4242
'featureC': { config: '{ global: true }' },
4343
},
4444
};
45-
new FallbackTreatmentsCalculator(config);
46-
expect(spy.mock.calls[3][0]).toBe(
45+
new FallbackTreatmentsCalculator(loggerMock, config);
46+
expect(loggerMock.error.mock.calls[3][0]).toBe(
4747
'Fallback treatments - Discarded treatment for flag \'featureC\': Invalid treatment (max 100 chars and must match pattern)'
4848
);
4949

@@ -53,8 +53,8 @@ describe('FallbackTreatmentsCalculator', () => {
5353
'featureC': { treatment: 'invalid treatment!', config: '{ global: true }' },
5454
},
5555
};
56-
new FallbackTreatmentsCalculator(config);
57-
expect(spy.mock.calls[4][0]).toBe(
56+
new FallbackTreatmentsCalculator(loggerMock, config);
57+
expect(loggerMock.error.mock.calls[4][0]).toBe(
5858
'Fallback treatments - Discarded treatment for flag \'featureC\': Invalid treatment (max 100 chars and must match pattern)'
5959
);
6060
});
@@ -63,26 +63,26 @@ describe('FallbackTreatmentsCalculator', () => {
6363
let config: FallbackTreatmentConfiguration = {
6464
global: { treatment: longName, config: '{ value: 1 }' },
6565
};
66-
new FallbackTreatmentsCalculator(config);
67-
expect(spy.mock.calls[2][0]).toBe(
66+
new FallbackTreatmentsCalculator(loggerMock, config);
67+
expect(loggerMock.error.mock.calls[2][0]).toBe(
6868
'Fallback treatments - Discarded treatment for flag \'featureB\': Invalid treatment (max 100 chars and must match pattern)'
6969
);
7070

7171
config = {
7272
// @ts-ignore
7373
global: { config: '{ global: true }' },
7474
};
75-
new FallbackTreatmentsCalculator(config);
76-
expect(spy.mock.calls[3][0]).toBe(
75+
new FallbackTreatmentsCalculator(loggerMock, config);
76+
expect(loggerMock.error.mock.calls[3][0]).toBe(
7777
'Fallback treatments - Discarded treatment for flag \'featureC\': Invalid treatment (max 100 chars and must match pattern)'
7878
);
7979

8080
config = {
8181
// @ts-ignore
8282
global: { treatment: 'invalid treatment!', config: '{ global: true }' },
8383
};
84-
new FallbackTreatmentsCalculator(config);
85-
expect(spy.mock.calls[4][0]).toBe(
84+
new FallbackTreatmentsCalculator(loggerMock, config);
85+
expect(loggerMock.error.mock.calls[4][0]).toBe(
8686
'Fallback treatments - Discarded treatment for flag \'featureC\': Invalid treatment (max 100 chars and must match pattern)'
8787
);
8888
});
@@ -93,7 +93,7 @@ describe('FallbackTreatmentsCalculator', () => {
9393
'featureA': { treatment: 'TREATMENT_A', config: '{ value: 1 }' },
9494
},
9595
};
96-
const calculator = new FallbackTreatmentsCalculator(config);
96+
const calculator = new FallbackTreatmentsCalculator(loggerMock, config);
9797
const result = calculator.resolve('featureA', 'label by flag');
9898

9999
expect(result).toEqual({
@@ -108,7 +108,7 @@ describe('FallbackTreatmentsCalculator', () => {
108108
byFlag: {},
109109
global: { treatment: 'GLOBAL_TREATMENT', config: '{ global: true }' },
110110
};
111-
const calculator = new FallbackTreatmentsCalculator(config);
111+
const calculator = new FallbackTreatmentsCalculator(loggerMock, config);
112112
const result = calculator.resolve('missingFlag', 'label by global');
113113

114114
expect(result).toEqual({
@@ -122,7 +122,7 @@ describe('FallbackTreatmentsCalculator', () => {
122122
const config: FallbackTreatmentConfiguration = {
123123
byFlag: {},
124124
};
125-
const calculator = new FallbackTreatmentsCalculator(config);
125+
const calculator = new FallbackTreatmentsCalculator(loggerMock, config);
126126
const result = calculator.resolve('missingFlag', 'label by noFallback');
127127

128128
expect(result).toEqual({
@@ -135,15 +135,15 @@ describe('FallbackTreatmentsCalculator', () => {
135135
test('returns undefined label if no label provided', () => {
136136
const config: FallbackTreatmentConfiguration = {
137137
byFlag: {
138-
'featureB': { treatment: 'TREATMENT_B' },
138+
'featureB': { treatment: 'TREATMENT_B', config: '{ value: 1 }' },
139139
},
140140
};
141-
const calculator = new FallbackTreatmentsCalculator(config);
141+
const calculator = new FallbackTreatmentsCalculator(loggerMock, config);
142142
const result = calculator.resolve('featureB');
143143

144144
expect(result).toEqual({
145145
treatment: 'TREATMENT_B',
146-
config: undefined,
146+
config: '{ value: 1 }',
147147
label: '',
148148
});
149149
});

‎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: 15 additions & 21 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,28 +12,18 @@ export class FallbacksSanitizer {
1012
return name.length <= 100 && !name.includes(' ');
1113
}
1214

13-
private static isValidTreatment(t?: string | 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 this.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
}
29-
return this.pattern.test(treatment);
21+
return FallbacksSanitizer.pattern.test(treatment);
3022
}
3123

32-
static sanitizeGlobal(treatment?: string | FallbackTreatment): string | 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;
@@ -40,21 +32,23 @@ export class FallbacksSanitizer {
4032
}
4133

4234
static sanitizeByFlag(
35+
logger: ILogger,
4336
byFlagFallbacks: Record<string, string | FallbackTreatment>
4437
): Record<string, string | FallbackTreatment> {
4538
const sanitizedByFlag: Record<string, string | FallbackTreatment> = {};
4639

47-
const entries = Object.entries(byFlagFallbacks);
48-
entries.forEach(([flag, t]) => {
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: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,27 @@
1-
import { FallbackTreatmentConfiguration, FallbackTreatment, IFallbackTreatmentsCalculator} from '../../../types/splitio';
1+
import { FallbackTreatmentConfiguration, FallbackTreatment } from '../../../types/splitio';
22
import { FallbacksSanitizer } from './fallbackSanitizer';
33
import { CONTROL } from '../../utils/constants';
4+
import { isString } from '../../utils/lang';
5+
import { ILogger } from '../../logger/types';
46

7+
type IFallbackTreatmentsCalculator = {
8+
resolve(flagName: string, label?: string): FallbackTreatment;
9+
}
510

611
export class FallbackTreatmentsCalculator implements IFallbackTreatmentsCalculator {
712
private readonly labelPrefix = 'fallback - ';
813
private readonly fallbacks: FallbackTreatmentConfiguration;
914

10-
constructor(fallbacks?: FallbackTreatmentConfiguration) {
11-
const sanitizedGlobal = fallbacks?.global ? FallbacksSanitizer.sanitizeGlobal(fallbacks.global) : undefined;
12-
const sanitizedByFlag = fallbacks?.byFlag ? FallbacksSanitizer.sanitizeByFlag(fallbacks.byFlag) : {};
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) : {};
1318
this.fallbacks = {
1419
global: sanitizedGlobal,
1520
byFlag: sanitizedByFlag
1621
};
1722
}
1823

19-
resolve(flagName: string, label?: string | undefined): FallbackTreatment {
24+
resolve(flagName: string, label?: string): FallbackTreatment & { label?: string } {
2025
const treatment = this.fallbacks.byFlag?.[flagName];
2126
if (treatment) {
2227
return this.copyWithLabel(treatment, label);
@@ -33,8 +38,8 @@ export class FallbackTreatmentsCalculator implements IFallbackTreatmentsCalculat
3338
};
3439
}
3540

36-
private copyWithLabel(fallback: string | FallbackTreatment, label: string | undefined): FallbackTreatment {
37-
if (typeof fallback === 'string') {
41+
private copyWithLabel(fallback: string | FallbackTreatment, label: string | undefined): FallbackTreatment & { label?: string } {
42+
if (isString(fallback)) {
3843
return {
3944
treatment: fallback,
4045
config: null,
@@ -49,7 +54,7 @@ export class FallbackTreatmentsCalculator implements IFallbackTreatmentsCalculat
4954
};
5055
}
5156

52-
private resolveLabel(label?: string | undefined): string {
57+
private resolveLabel(label?: string): string {
5358
return label ? `${this.labelPrefix}${label}` : '';
5459
}
5560

‎src/evaluator/matchers/__tests__/prerequisites.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const mockStorage = {
1717
}
1818
} as IStorageSync;
1919

20-
const fallbackTreatmentsCalculator = new FallbackTreatmentsCalculator();
20+
const fallbackTreatmentsCalculator = new FallbackTreatmentsCalculator(loggerMock);
2121

2222
test('MATCHER PREREQUISITES / should return true when all prerequisites are met', () => {
2323
// A single prerequisite

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ const paramMocks = [
1919
telemetryTracker: telemetryTrackerFactory(),
2020
clients: {},
2121
uniqueKeysTracker: { start: jest.fn(), stop: jest.fn() },
22-
fallbackTreatmentsCalculator: new FallbackTreatmentsCalculator({})
22+
fallbackTreatmentsCalculator: new FallbackTreatmentsCalculator(loggerMock, {})
2323
},
2424
// SyncManager (i.e., Sync SDK) and Signal listener
2525
{
@@ -31,7 +31,7 @@ const paramMocks = [
3131
telemetryTracker: telemetryTrackerFactory(),
3232
clients: {},
3333
uniqueKeysTracker: { start: jest.fn(), stop: jest.fn() },
34-
fallbackTreatmentsCalculator: new FallbackTreatmentsCalculator({})
34+
fallbackTreatmentsCalculator: new FallbackTreatmentsCalculator(loggerMock, {})
3535
}
3636
];
3737

‎src/sdkFactory/__tests__/index.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ const paramsForAsyncSDK = {
3737
platform: {
3838
EventEmitter
3939
},
40-
fallbackTreatmentsCalculator: new FallbackTreatmentsCalculator()
40+
fallbackTreatmentsCalculator: new FallbackTreatmentsCalculator(fullSettings.log)
4141
};
4242

4343
const SignalListenerInstanceMock = { start: jest.fn() };

‎src/sdkFactory/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA
6161
}
6262
});
6363

64-
const fallbackTreatmentsCalculator = new FallbackTreatmentsCalculator(settings.fallbackTreatments);
64+
const fallbackTreatmentsCalculator = new FallbackTreatmentsCalculator(settings.log, settings.fallbackTreatments);
6565

6666
if (initialRolloutPlan) {
6767
setRolloutPlan(log, initialRolloutPlan, storage as IStorageSync, key && getMatching(key));

0 commit comments

Comments
 (0)