Skip to content

Commit 9ddadd6

Browse files
refactor: mode rollout plan validation
1 parent c65b3d0 commit 9ddadd6

File tree

6 files changed

+63
-37
lines changed

6 files changed

+63
-37
lines changed

src/sdkClient/sdkClientMethodCS.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import { RETRIEVE_CLIENT_DEFAULT, NEW_SHARED_CLIENT, RETRIEVE_CLIENT_EXISTING, L
99
import { SDK_SEGMENTS_ARRIVED } from '../readiness/constants';
1010
import { ISdkFactoryContext } from '../sdkFactory/types';
1111
import { buildInstanceId } from './identity';
12-
import { isConsumerMode } from '../utils/settingsValidation/mode';
1312
import { setRolloutPlan } from '../storages/dataLoader';
1413
import { ISegmentsCacheSync } from '../storages/types';
1514

@@ -18,7 +17,7 @@ import { ISegmentsCacheSync } from '../storages/types';
1817
* Therefore, clients don't have a bound TT for the track method.
1918
*/
2019
export function sdkClientMethodCSFactory(params: ISdkFactoryContext): (key?: SplitIO.SplitKey) => SplitIO.IBrowserClient {
21-
const { clients, storage, syncManager, sdkReadinessManager, settings: { core: { key }, log, initialRolloutPlan, mode } } = params;
20+
const { clients, storage, syncManager, sdkReadinessManager, settings: { core: { key }, log, initialRolloutPlan } } = params;
2221

2322
const mainClientInstance = clientCSDecorator(
2423
log,
@@ -59,7 +58,7 @@ export function sdkClientMethodCSFactory(params: ISdkFactoryContext): (key?: Spl
5958
sharedSdkReadiness.readinessManager.segments.emit(SDK_SEGMENTS_ARRIVED);
6059
});
6160

62-
if (sharedStorage && initialRolloutPlan && !isConsumerMode(mode)) {
61+
if (sharedStorage && initialRolloutPlan) {
6362
setRolloutPlan(log, initialRolloutPlan, { segments: sharedStorage.segments as ISegmentsCacheSync, largeSegments: sharedStorage.largeSegments as ISegmentsCacheSync }, matchingKey);
6463
}
6564

src/sdkFactory/index.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import { strategyNoneFactory } from '../trackers/strategy/strategyNone';
1515
import { uniqueKeysTrackerFactory } from '../trackers/uniqueKeysTracker';
1616
import { DEBUG, OPTIMIZED } from '../utils/constants';
1717
import { setRolloutPlan } from '../storages/dataLoader';
18-
import { isConsumerMode } from '../utils/settingsValidation/mode';
1918
import { IStorageSync } from '../storages/types';
2019
import { getMatching } from '../utils/key';
2120

@@ -28,7 +27,7 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA
2827
syncManagerFactory, SignalListener, impressionsObserverFactory,
2928
integrationsManagerFactory, sdkManagerFactory, sdkClientMethodFactory,
3029
filterAdapterFactory, lazyInit } = params;
31-
const { log, sync: { impressionsMode }, initialRolloutPlan, mode, core: { key } } = settings;
30+
const { log, sync: { impressionsMode }, initialRolloutPlan, core: { key } } = settings;
3231

3332
// @TODO handle non-recoverable errors, such as, global `fetch` not available, invalid SDK Key, etc.
3433
// On non-recoverable errors, we should mark the SDK as destroyed and not start synchronization.
@@ -61,7 +60,7 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA
6160
}
6261
});
6362

64-
if (initialRolloutPlan && !isConsumerMode(mode)) {
63+
if (initialRolloutPlan) {
6564
setRolloutPlan(log, initialRolloutPlan, storage as IStorageSync, key && getMatching(key));
6665
if ((storage as IStorageSync).splits.getChangeNumber() > -1) readiness.splits.emit(SDK_SPLITS_CACHE_LOADED);
6766
}

src/storages/__tests__/dataLoader.spec.ts

Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,34 +4,54 @@ import { fullSettings } from '../../utils/settingsValidation/__tests__/settings.
44
import { loggerMock } from '../../logger/__tests__/sdkLogger.mock';
55
import { IRBSegment, ISplit } from '../../dtos/types';
66

7-
import { setRolloutPlan, getRolloutPlan } from '../dataLoader';
7+
import { validateRolloutPlan, setRolloutPlan, getRolloutPlan } from '../dataLoader';
8+
9+
const otherKey = 'otherKey';
10+
const expectedRolloutPlan = {
11+
splitChanges: {
12+
ff: { d: [{ name: 'split1' }], t: 123, s: -1 },
13+
rbs: { d: [{ name: 'rbs1' }], t: 321, s: -1 }
14+
},
15+
memberships: {
16+
[fullSettings.core.key as string]: { ms: { k: [{ n: 'segment1' }] }, ls: { k: [] } },
17+
[otherKey]: { ms: { k: [{ n: 'segment1' }] }, ls: { k: [] } }
18+
},
19+
segmentChanges: [{
20+
name: 'segment1',
21+
added: [fullSettings.core.key as string, otherKey],
22+
removed: [],
23+
till: 123
24+
}]
25+
};
26+
27+
describe('validateRolloutPlan', () => {
28+
afterEach(() => {
29+
loggerMock.mockClear();
30+
});
831

9-
describe('getRolloutPlan & setRolloutPlan (client-side)', () => {
10-
const otherKey = 'otherKey';
32+
test('valid rollout plan and mode', () => {
33+
expect(validateRolloutPlan(loggerMock, { mode: 'standalone', initialRolloutPlan: expectedRolloutPlan } as any)).toEqual(expectedRolloutPlan);
34+
expect(loggerMock.error).not.toHaveBeenCalled();
35+
});
36+
37+
test('invalid rollout plan', () => {
38+
expect(validateRolloutPlan(loggerMock, { mode: 'standalone', initialRolloutPlan: {} } as any)).toBeUndefined();
39+
expect(loggerMock.error).toHaveBeenCalledWith('storage: invalid rollout plan provided');
40+
});
1141

42+
test('invalid mode', () => {
43+
expect(validateRolloutPlan(loggerMock, { mode: 'consumer', initialRolloutPlan: expectedRolloutPlan } as any)).toBeUndefined();
44+
expect(loggerMock.warn).toHaveBeenCalledWith('storage: initial rollout plan is ignored in consumer mode');
45+
});
46+
});
47+
48+
describe('getRolloutPlan & setRolloutPlan (client-side)', () => {
1249
// @ts-expect-error Load server-side storage
1350
const serverStorage = InMemoryStorageFactory({ settings: fullSettings });
1451
serverStorage.splits.update([{ name: 'split1' } as ISplit], [], 123);
1552
serverStorage.rbSegments.update([{ name: 'rbs1' } as IRBSegment], [], 321);
1653
serverStorage.segments.update('segment1', [fullSettings.core.key as string, otherKey], [], 123);
1754

18-
const expectedRolloutPlan = {
19-
splitChanges: {
20-
ff: { d: [{ name: 'split1' }], t: 123, s: -1 },
21-
rbs: { d: [{ name: 'rbs1' }], t: 321, s: -1 }
22-
},
23-
memberships: {
24-
[fullSettings.core.key as string]: { ms: { k: [{ n: 'segment1' }] }, ls: { k: [] } },
25-
[otherKey]: { ms: { k: [{ n: 'segment1' }] }, ls: { k: [] } }
26-
},
27-
segmentChanges: [{
28-
name: 'segment1',
29-
added: [fullSettings.core.key as string, otherKey],
30-
removed: [],
31-
till: 123
32-
}]
33-
};
34-
3555
afterEach(() => {
3656
jest.clearAllMocks();
3757
});

src/storages/dataLoader.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { getMatching } from '../utils/key';
55
import { IMembershipsResponse, IMySegmentsResponse, ISegmentChangesResponse, ISplitChangesResponse } from '../dtos/types';
66
import { ILogger } from '../logger/types';
77
import { isObject } from '../utils/lang';
8+
import { isConsumerMode } from '../utils/settingsValidation/mode';
89

910
export type RolloutPlan = {
1011
/**
@@ -27,10 +28,18 @@ export type RolloutPlan = {
2728
/**
2829
* Validates if the given rollout plan is valid.
2930
*/
30-
function validateRolloutPlan(rolloutPlan: unknown): rolloutPlan is RolloutPlan {
31-
if (isObject(rolloutPlan) && isObject((rolloutPlan as any).splitChanges)) return true;
31+
export function validateRolloutPlan(log: ILogger, settings: SplitIO.ISettings): RolloutPlan | undefined {
32+
const { mode, initialRolloutPlan } = settings;
3233

33-
return false;
34+
if (isConsumerMode(mode)) {
35+
log.warn('storage: initial rollout plan is ignored in consumer mode');
36+
return;
37+
}
38+
39+
if (isObject(initialRolloutPlan) && isObject((initialRolloutPlan as any).splitChanges)) return initialRolloutPlan as RolloutPlan;
40+
41+
log.error('storage: invalid rollout plan provided');
42+
return;
3443
}
3544

3645
/**
@@ -39,12 +48,6 @@ function validateRolloutPlan(rolloutPlan: unknown): rolloutPlan is RolloutPlan {
3948
* Otherwise, the storage is handled as a server-side storage (segments is an instance of SegmentsCache).
4049
*/
4150
export function setRolloutPlan(log: ILogger, rolloutPlan: RolloutPlan, storage: { splits?: ISplitsCacheSync, rbSegments?: IRBSegmentsCacheSync, segments: ISegmentsCacheSync, largeSegments?: ISegmentsCacheSync }, matchingKey?: string) {
42-
// Do not load data if current rollout plan is empty
43-
if (!validateRolloutPlan(rolloutPlan)) {
44-
log.error('storage: invalid rollout plan provided');
45-
return;
46-
}
47-
4851
const { splits, rbSegments, segments, largeSegments } = storage;
4952
const { splitChanges: { ff, rbs } } = rolloutPlan;
5053

@@ -79,6 +82,7 @@ export function setRolloutPlan(log: ILogger, rolloutPlan: RolloutPlan, storage:
7982
}
8083
} else { // add segments data (server-side)
8184
if (segmentChanges) {
85+
segments.clear();
8286
segmentChanges.forEach(segment => {
8387
segments.update(segment.name, segment.added, segment.removed, segment.till);
8488
});

src/storages/inLocalStorage/validateCache.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const MILLIS_IN_A_DAY = 86400000;
1717
* @returns `true` if cache should be cleared, `false` otherwise
1818
*/
1919
function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS, currentTimestamp: number, isThereCache: boolean) {
20-
const { log } = settings;
20+
const { log, initialRolloutPlan } = settings;
2121

2222
// Check expiration
2323
const lastUpdatedTimestamp = parseInt(localStorage.getItem(keys.buildLastUpdatedKey()) as string, 10);
@@ -41,7 +41,7 @@ function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: IS
4141
} catch (e) {
4242
log.error(LOG_PREFIX + e);
4343
}
44-
if (isThereCache) {
44+
if (isThereCache && !initialRolloutPlan) {
4545
log.info(LOG_PREFIX + 'SDK key, flags filter criteria, or flags spec version has changed. Cleaning up cache');
4646
return true;
4747
}

src/utils/settingsValidation/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { ISettingsValidationParams } from './types';
77
import { ISettings } from '../../types';
88
import { validateKey } from '../inputValidation/key';
99
import { ERROR_MIN_CONFIG_PARAM, LOG_PREFIX_CLIENT_INSTANTIATION } from '../../logger/constants';
10+
import { validateRolloutPlan } from '../../storages/dataLoader';
1011

1112
// Exported for telemetry
1213
export const base = {
@@ -152,6 +153,9 @@ export function settingsValidation(config: unknown, validationParams: ISettingsV
152153
// @ts-ignore, modify readonly prop
153154
if (storage) withDefaults.storage = storage(withDefaults);
154155

156+
// @ts-ignore, modify readonly prop
157+
if (withDefaults.initialRolloutPlan) withDefaults.initialRolloutPlan = validateRolloutPlan(log, withDefaults);
158+
155159
// Validate key and TT (for client-side)
156160
const maybeKey = withDefaults.core.key;
157161
if (validationParams.acceptKey) {

0 commit comments

Comments
 (0)