Skip to content

Commit 781d8ce

Browse files
EmilianoSanchezEmilianoSanchez
committed
Refactor usesSegments method and SplitsUpdateWorker
1 parent b6f5514 commit 781d8ce

File tree

8 files changed

+32
-37
lines changed

8 files changed

+32
-37
lines changed

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ Split has built and maintains SDKs for:
2727
* .NET [Github](https://github.com/splitio/dotnet-client) [Docs](https://help.split.io/hc/en-us/articles/360020240172--NET-SDK)
2828
* Android [Github](https://github.com/splitio/android-client) [Docs](https://help.split.io/hc/en-us/articles/360020343291-Android-SDK)
2929
* Angular [Github](https://github.com/splitio/angular-sdk-plugin) [Docs](https://help.split.io/hc/en-us/articles/6495326064397-Angular-utilities)
30+
* Elixir thin-client [Github](https://github.com/splitio/elixir-thin-client) [Docs](https://help.split.io/hc/en-us/articles/26988707417869-Elixir-Thin-Client-SDK)
3031
* Flutter [Github](https://github.com/splitio/flutter-sdk-plugin) [Docs](https://help.split.io/hc/en-us/articles/8096158017165-Flutter-plugin)
3132
* GO [Github](https://github.com/splitio/go-client) [Docs](https://help.split.io/hc/en-us/articles/360020093652-Go-SDK)
3233
* iOS [Github](https://github.com/splitio/ios-client) [Docs](https://help.split.io/hc/en-us/articles/360020401491-iOS-SDK)

‎src/storages/__tests__/KeyBuilder.spec.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,16 +106,16 @@ test('KEYS / latency and exception keys (telemetry)', () => {
106106
test('getStorageHash', () => {
107107
expect(getStorageHash({
108108
core: { authorizationKey: '<fake-token-rfc>' },
109-
sync: { __splitFiltersValidation: { queryString: '&names=p1__split,p2__split' }, flagSpecVersion: '1.2' }
110-
} as ISettings)).toBe('7ccd6b31');
109+
sync: { __splitFiltersValidation: { queryString: '&names=p1__split,p2__split' }, flagSpecVersion: '1.3' }
110+
} as ISettings)).toBe('2ce5cc38');
111111

112112
expect(getStorageHash({
113113
core: { authorizationKey: '<fake-token-rfc>' },
114-
sync: { __splitFiltersValidation: { queryString: '&names=p2__split,p3__split' }, flagSpecVersion: '1.2' }
115-
} as ISettings)).toBe('2a25d0e1');
114+
sync: { __splitFiltersValidation: { queryString: '&names=p2__split,p3__split' }, flagSpecVersion: '1.3' }
115+
} as ISettings)).toBe('e65079c6');
116116

117117
expect(getStorageHash({
118118
core: { authorizationKey: '<fake-token-rfc>' },
119-
sync: { __splitFiltersValidation: { queryString: null }, flagSpecVersion: '1.2' }
120-
} as ISettings)).toBe('db8943b4');
119+
sync: { __splitFiltersValidation: { queryString: null }, flagSpecVersion: '1.3' }
120+
} as ISettings)).toBe('193e6f3f');
121121
});

‎src/storages/__tests__/RBSegmentsCacheSync.spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ describe.each([cacheInMemory, cacheInLocal])('Rule-based segments cache sync (Me
6161
});
6262

6363
test('usesSegments should track segments usage correctly', () => {
64-
expect(cache.usesSegments()).toBe(true); // Initially true when changeNumber is -1
64+
expect(cache.usesSegments()).toBe(false); // No rbSegments, so false
6565

6666
cache.update([rbSegment], [], 1); // rbSegment doesn't have IN_SEGMENT matcher
6767
expect(cache.usesSegments()).toBe(false);
@@ -70,6 +70,6 @@ describe.each([cacheInMemory, cacheInLocal])('Rule-based segments cache sync (Me
7070
expect(cache.usesSegments()).toBe(true);
7171

7272
cache.clear();
73-
expect(cache.usesSegments()).toBe(true); // True after clear since changeNumber is -1
73+
expect(cache.usesSegments()).toBe(false); // False after clear since there are no rbSegments
7474
});
7575
});

‎src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ export class RBSegmentsCacheInLocal implements IRBSegmentsCacheSync {
1212

1313
private readonly keys: KeyBuilderCS;
1414
private readonly log: ILogger;
15-
private hasSync?: boolean;
1615

1716
constructor(settings: ISettings, keys: KeyBuilderCS) {
1817
this.keys = keys;
@@ -22,7 +21,6 @@ export class RBSegmentsCacheInLocal implements IRBSegmentsCacheSync {
2221
clear() {
2322
this.getNames().forEach(name => this.remove(name));
2423
localStorage.removeItem(this.keys.buildRBSegmentsTillKey());
25-
this.hasSync = false;
2624
}
2725

2826
update(toAdd: IRBSegment[], toRemove: IRBSegment[], changeNumber: number): boolean {
@@ -35,7 +33,6 @@ export class RBSegmentsCacheInLocal implements IRBSegmentsCacheSync {
3533
try {
3634
localStorage.setItem(this.keys.buildRBSegmentsTillKey(), changeNumber + '');
3735
localStorage.setItem(this.keys.buildLastUpdatedKey(), Date.now() + '');
38-
this.hasSync = true;
3936
} catch (e) {
4037
this.log.error(LOG_PREFIX + e);
4138
}
@@ -128,9 +125,6 @@ export class RBSegmentsCacheInLocal implements IRBSegmentsCacheSync {
128125
}
129126

130127
usesSegments(): boolean {
131-
// If cache hasn't been synchronized, assume we need segments
132-
if (!this.hasSync) return true;
133-
134128
const storedCount = localStorage.getItem(this.keys.buildSplitsWithSegmentCountKey());
135129
const splitsWithSegmentsCount = storedCount === null ? 0 : toNumber(storedCount);
136130

‎src/storages/inMemory/RBSegmentsCacheInMemory.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export class RBSegmentsCacheInMemory implements IRBSegmentsCacheSync {
6262
}
6363

6464
usesSegments(): boolean {
65-
return this.getChangeNumber() === -1 || this.segmentsCount > 0;
65+
return this.segmentsCount > 0;
6666
}
6767

6868
}

‎src/sync/streaming/UpdateWorkers/SplitsUpdateWorker.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { STREAMING_PARSING_SPLIT_UPDATE } from '../../../logger/constants';
33
import { ILogger } from '../../../logger/types';
44
import { SDK_SPLITS_ARRIVED } from '../../../readiness/constants';
55
import { ISplitsEventEmitter } from '../../../readiness/types';
6-
import { ISplitsCacheSync } from '../../../storages/types';
6+
import { IRBSegmentsCacheSync, ISplitsCacheSync, IStorageSync } from '../../../storages/types';
77
import { ITelemetryTracker } from '../../../trackers/types';
88
import { Backoff } from '../../../utils/Backoff';
99
import { SPLITS } from '../../../utils/constants';
@@ -17,9 +17,9 @@ import { IUpdateWorker } from './types';
1717
/**
1818
* SplitsUpdateWorker factory
1919
*/
20-
export function SplitsUpdateWorker(log: ILogger, splitsCache: ISplitsCacheSync, splitsSyncTask: ISplitsSyncTask, splitsEventEmitter: ISplitsEventEmitter, telemetryTracker: ITelemetryTracker, segmentsSyncTask?: ISegmentsSyncTask): IUpdateWorker<[updateData: ISplitUpdateData]> & { killSplit(event: ISplitKillData): void } {
20+
export function SplitsUpdateWorker(log: ILogger, storage: IStorageSync, splitsSyncTask: ISplitsSyncTask, splitsEventEmitter: ISplitsEventEmitter, telemetryTracker: ITelemetryTracker, segmentsSyncTask?: ISegmentsSyncTask): IUpdateWorker<[updateData: ISplitUpdateData]> & { killSplit(event: ISplitKillData): void } {
2121

22-
function SplitsUpdateWorker() {
22+
function SplitsUpdateWorker(cache: ISplitsCacheSync | IRBSegmentsCacheSync) {
2323
let maxChangeNumber = 0;
2424
let handleNewEvent = false;
2525
let isHandlingEvent: boolean;
@@ -29,7 +29,7 @@ export function SplitsUpdateWorker(log: ILogger, splitsCache: ISplitsCacheSync,
2929

3030
function __handleSplitUpdateCall() {
3131
isHandlingEvent = true;
32-
if (maxChangeNumber > splitsCache.getChangeNumber()) {
32+
if (maxChangeNumber > cache.getChangeNumber()) {
3333
handleNewEvent = false;
3434
const splitUpdateNotification = payload ? { payload, changeNumber: maxChangeNumber } : undefined;
3535
// fetch splits revalidating data if cached
@@ -44,7 +44,7 @@ export function SplitsUpdateWorker(log: ILogger, splitsCache: ISplitsCacheSync,
4444

4545
const attempts = backoff.attempts + 1;
4646

47-
if (maxChangeNumber <= splitsCache.getChangeNumber()) {
47+
if (maxChangeNumber <= cache.getChangeNumber()) {
4848
log.debug(`Refresh completed${cdnBypass ? ' bypassing the CDN' : ''} in ${attempts} attempts.`);
4949
isHandlingEvent = false;
5050
return;
@@ -77,7 +77,7 @@ export function SplitsUpdateWorker(log: ILogger, splitsCache: ISplitsCacheSync,
7777
* @param changeNumber - change number of the notification
7878
*/
7979
put({ changeNumber, pcn }: ISplitUpdateData, _payload?: ISplit | IRBSegment) {
80-
const currentChangeNumber = splitsCache.getChangeNumber();
80+
const currentChangeNumber = cache.getChangeNumber();
8181

8282
if (changeNumber <= currentChangeNumber || changeNumber <= maxChangeNumber) return;
8383

@@ -100,8 +100,8 @@ export function SplitsUpdateWorker(log: ILogger, splitsCache: ISplitsCacheSync,
100100
};
101101
}
102102

103-
const ff = SplitsUpdateWorker();
104-
const rbs = SplitsUpdateWorker();
103+
const ff = SplitsUpdateWorker(storage.splits);
104+
const rbs = SplitsUpdateWorker(storage.rbSegments);
105105

106106
return {
107107
put(parsedData) {
@@ -126,7 +126,7 @@ export function SplitsUpdateWorker(log: ILogger, splitsCache: ISplitsCacheSync,
126126
* @param defaultTreatment - default treatment value
127127
*/
128128
killSplit({ changeNumber, splitName, defaultTreatment }: ISplitKillData) {
129-
if (splitsCache.killLocally(splitName, defaultTreatment, changeNumber)) {
129+
if (storage.splits.killLocally(splitName, defaultTreatment, changeNumber)) {
130130
// trigger an SDK_UPDATE if Split was killed locally
131131
splitsEventEmitter.emit(SDK_SPLITS_ARRIVED, true);
132132
}

‎src/sync/streaming/UpdateWorkers/__tests__/SplitsUpdateWorker.spec.ts‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ describe('SplitsUpdateWorker', () => {
6565
const splitsSyncTask = splitsSyncTaskMock(cache);
6666

6767
Backoff.__TEST__BASE_MILLIS = 1; // retry immediately
68-
const splitUpdateWorker = SplitsUpdateWorker(loggerMock, cache, splitsSyncTask, telemetryTracker);
68+
const splitUpdateWorker = SplitsUpdateWorker(loggerMock, { splits: cache }, splitsSyncTask, telemetryTracker);
6969

7070
// assert calling `splitsSyncTask.execute` if `isExecuting` is false
7171
expect(splitsSyncTask.isExecuting()).toBe(false);
@@ -104,7 +104,7 @@ describe('SplitsUpdateWorker', () => {
104104
Backoff.__TEST__BASE_MILLIS = 50;
105105
const cache = new SplitsCacheInMemory();
106106
const splitsSyncTask = splitsSyncTaskMock(cache, [90, 90, 90]);
107-
const splitUpdateWorker = SplitsUpdateWorker(loggerMock, cache, splitsSyncTask, telemetryTracker);
107+
const splitUpdateWorker = SplitsUpdateWorker(loggerMock, { splits: cache }, splitsSyncTask, telemetryTracker);
108108

109109
// while fetch fails, should retry with backoff
110110
splitUpdateWorker.put({ changeNumber: 100 });
@@ -123,7 +123,7 @@ describe('SplitsUpdateWorker', () => {
123123
Backoff.__TEST__MAX_MILLIS = 60; // 60 millis instead of 1 min
124124
const cache = new SplitsCacheInMemory();
125125
const splitsSyncTask = splitsSyncTaskMock(cache, [...Array(FETCH_BACKOFF_MAX_RETRIES).fill(90), 90, 100]); // 12 executions. Last one is valid
126-
const splitUpdateWorker = SplitsUpdateWorker(loggerMock, cache, splitsSyncTask, telemetryTracker);
126+
const splitUpdateWorker = SplitsUpdateWorker(loggerMock, { splits: cache }, splitsSyncTask, telemetryTracker);
127127

128128
splitUpdateWorker.put({ changeNumber: 100 }); // queued
129129

@@ -148,7 +148,7 @@ describe('SplitsUpdateWorker', () => {
148148
Backoff.__TEST__MAX_MILLIS = 60; // 60 millis instead of 1 min
149149
const cache = new SplitsCacheInMemory();
150150
const splitsSyncTask = splitsSyncTaskMock(cache, Array(FETCH_BACKOFF_MAX_RETRIES * 2).fill(90)); // 20 executions. No one is valid
151-
const splitUpdateWorker = SplitsUpdateWorker(loggerMock, cache, splitsSyncTask, telemetryTracker);
151+
const splitUpdateWorker = SplitsUpdateWorker(loggerMock, { splits: cache }, splitsSyncTask, telemetryTracker);
152152

153153
splitUpdateWorker.put({ changeNumber: 100 }); // queued
154154

@@ -169,11 +169,11 @@ describe('SplitsUpdateWorker', () => {
169169
test('killSplit', async () => {
170170
// setup
171171
const cache = new SplitsCacheInMemory();
172-
cache.addSplit({ name: 'something'});
173-
cache.addSplit({ name: 'something else'});
172+
cache.addSplit({ name: 'something' });
173+
cache.addSplit({ name: 'something else' });
174174

175175
const splitsSyncTask = splitsSyncTaskMock(cache);
176-
const splitUpdateWorker = SplitsUpdateWorker(loggerMock, cache, splitsSyncTask, splitsEventEmitterMock, telemetryTracker);
176+
const splitUpdateWorker = SplitsUpdateWorker(loggerMock, { splits: cache }, splitsSyncTask, splitsEventEmitterMock, telemetryTracker);
177177

178178
// assert killing split locally, emitting SDK_SPLITS_ARRIVED event, and synchronizing splits if changeNumber is new
179179
splitUpdateWorker.killSplit({ changeNumber: 100, splitName: 'something', defaultTreatment: 'off' }); // splitsCache.killLocally is synchronous
@@ -200,7 +200,7 @@ describe('SplitsUpdateWorker', () => {
200200
const cache = new SplitsCacheInMemory();
201201
const splitsSyncTask = splitsSyncTaskMock(cache, [95]);
202202
Backoff.__TEST__BASE_MILLIS = 1;
203-
const splitUpdateWorker = SplitsUpdateWorker(loggerMock, cache, splitsSyncTask, telemetryTracker);
203+
const splitUpdateWorker = SplitsUpdateWorker(loggerMock, { splits: cache }, splitsSyncTask, telemetryTracker);
204204

205205
splitUpdateWorker.put({ changeNumber: 100 });
206206

@@ -216,7 +216,7 @@ describe('SplitsUpdateWorker', () => {
216216
splitNotifications.forEach(notification => {
217217
const pcn = cache.getChangeNumber();
218218
const splitsSyncTask = splitsSyncTaskMock(cache);
219-
const splitUpdateWorker = SplitsUpdateWorker(loggerMock, cache, splitsSyncTask, telemetryTracker);
219+
const splitUpdateWorker = SplitsUpdateWorker(loggerMock, { splits: cache }, splitsSyncTask, telemetryTracker);
220220
const payload = notification.decoded;
221221
const changeNumber = payload.changeNumber;
222222
splitUpdateWorker.put({ changeNumber, pcn, d: notification.data, c: notification.compression }); // queued
@@ -236,7 +236,7 @@ describe('SplitsUpdateWorker', () => {
236236
const notification = splitNotifications[0];
237237

238238
let splitsSyncTask = splitsSyncTaskMock(cache);
239-
let splitUpdateWorker = SplitsUpdateWorker(loggerMock, cache, splitsSyncTask, telemetryTracker);
239+
let splitUpdateWorker = SplitsUpdateWorker(loggerMock, { splits: cache }, splitsSyncTask, telemetryTracker);
240240
splitUpdateWorker.put({ changeNumber, pcn, d: notification.data, c: notification.compression });
241241
expect(splitsSyncTask.execute).toBeCalledTimes(1);
242242
expect(splitsSyncTask.execute.mock.calls[0]).toEqual([true, undefined, undefined]);
@@ -249,7 +249,7 @@ describe('SplitsUpdateWorker', () => {
249249
cache.setChangeNumber(ccn);
250250

251251
splitsSyncTask = splitsSyncTaskMock(cache);
252-
splitUpdateWorker = SplitsUpdateWorker(loggerMock, cache, splitsSyncTask, telemetryTracker);
252+
splitUpdateWorker = SplitsUpdateWorker(loggerMock, { splits: cache }, splitsSyncTask, telemetryTracker);
253253
splitUpdateWorker.put({ changeNumber, pcn, d: notification.data, c: notification.compression });
254254
expect(splitsSyncTask.execute).toBeCalledTimes(1);
255255
expect(splitsSyncTask.execute.mock.calls[0]).toEqual([true, undefined, undefined]);
@@ -262,7 +262,7 @@ describe('SplitsUpdateWorker', () => {
262262
cache.setChangeNumber(ccn);
263263

264264
splitsSyncTask = splitsSyncTaskMock(cache);
265-
splitUpdateWorker = SplitsUpdateWorker(loggerMock, cache, splitsSyncTask, telemetryTracker);
265+
splitUpdateWorker = SplitsUpdateWorker(loggerMock, { splits: cache }, splitsSyncTask, telemetryTracker);
266266
splitUpdateWorker.put({ changeNumber, pcn, d: notification.data, c: notification.compression });
267267
expect(splitsSyncTask.execute).toBeCalledTimes(1);
268268
expect(splitsSyncTask.execute.mock.calls[0]).toEqual([true, undefined, { payload: notification.decoded, changeNumber }]);

‎src/sync/streaming/pushManager.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export function pushManagerFactory(
5656
// MySegmentsUpdateWorker (client-side) are initiated in `add` method
5757
const segmentsUpdateWorker = userKey ? undefined : SegmentsUpdateWorker(log, pollingManager.segmentsSyncTask as ISegmentsSyncTask, storage.segments);
5858
// For server-side we pass the segmentsSyncTask, used by SplitsUpdateWorker to fetch new segments
59-
const splitsUpdateWorker = SplitsUpdateWorker(log, storage.splits, pollingManager.splitsSyncTask, readiness.splits, telemetryTracker, userKey ? undefined : pollingManager.segmentsSyncTask as ISegmentsSyncTask);
59+
const splitsUpdateWorker = SplitsUpdateWorker(log, storage, pollingManager.splitsSyncTask, readiness.splits, telemetryTracker, userKey ? undefined : pollingManager.segmentsSyncTask as ISegmentsSyncTask);
6060

6161
// [Only for client-side] map of hashes to user keys, to dispatch membership update events to the corresponding MySegmentsUpdateWorker
6262
const userKeyHashes: Record<string, string> = {};

0 commit comments

Comments
 (0)