Skip to content

Commit

Permalink
Fix duplicated /memberships fetch of non-default clients when using l…
Browse files Browse the repository at this point in the history
…azy init
  • Loading branch information
EmilianoSanchez committed Oct 5, 2024
1 parent fcdeb1b commit bdf69e8
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 40 deletions.
15 changes: 11 additions & 4 deletions src/readiness/readinessManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ function splitsEventEmitterFactory(EventEmitter: new () => IEventEmitter): ISpli
const splitsEventEmitter = objectAssign(new EventEmitter(), {
splitsArrived: false,
splitsCacheLoaded: false,
initialized: false,
initCallbacks: []
});

// `isSplitKill` condition avoids an edge-case of wrongly emitting SDK_READY if:
Expand Down Expand Up @@ -55,7 +57,6 @@ export function readinessManagerFactory(

// emit SDK_READY_TIMED_OUT
let hasTimedout = false;
let readyTimeoutId: ReturnType<typeof setTimeout>;

function timeout() { // eslint-disable-next-line no-use-before-define
if (hasTimedout || isReady) return;
Expand All @@ -64,6 +65,12 @@ export function readinessManagerFactory(
gate.emit(SDK_READY_TIMED_OUT, 'Split SDK emitted SDK_READY_TIMED_OUT event.');
}

let readyTimeoutId: ReturnType<typeof setTimeout>;
if (readyTimeout > 0) {
if (splits.initialized) readyTimeoutId = setTimeout(timeout, readyTimeout);
else splits.initCallbacks.push(() => { readyTimeoutId = setTimeout(timeout, readyTimeout); });
}

// emit SDK_READY and SDK_UPDATE
let isReady = false;
splits.on(SDK_SPLITS_ARRIVED, checkIsReadyOrUpdate);
Expand Down Expand Up @@ -129,9 +136,9 @@ export function readinessManagerFactory(
setDestroyed() { isDestroyed = true; },

init() {
if (readyTimeout > 0) {
readyTimeoutId = setTimeout(timeout, readyTimeout);
}
if (splits.initialized) return;
splits.initialized = true;
splits.initCallbacks.forEach(cb => cb());
},

destroy() {
Expand Down
2 changes: 2 additions & 0 deletions src/readiness/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ export interface ISplitsEventEmitter extends IEventEmitter {
once(event: ISplitsEvent, listener: (...args: any[]) => void): this;
splitsArrived: boolean
splitsCacheLoaded: boolean
initialized: boolean,
initCallbacks: (() => void)[]
}

/** Segments data emitter */
Expand Down
7 changes: 1 addition & 6 deletions src/sdkClient/sdkClientMethodCS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { buildInstanceId } from './identity';
* Therefore, clients don't have a bound TT for the track method.
*/
export function sdkClientMethodCSFactory(params: ISdkFactoryContext): (key?: SplitIO.SplitKey) => SplitIO.ICsClient {
const { clients, storage, syncManager, sdkReadinessManager, settings: { core: { key }, log }, whenInit } = params;
const { clients, storage, syncManager, sdkReadinessManager, settings: { core: { key }, log } } = params;

const mainClientInstance = clientCSDecorator(
log,
Expand Down Expand Up @@ -75,11 +75,6 @@ export function sdkClientMethodCSFactory(params: ISdkFactoryContext): (key?: Spl
validKey
);

whenInit(() => {
sharedSdkReadiness.readinessManager.init();
sharedSyncManager && sharedSyncManager.start();
});

log.info(NEW_SHARED_CLIENT);
} else {
log.debug(RETRIEVE_CLIENT_EXISTING);
Expand Down
7 changes: 1 addition & 6 deletions src/sdkClient/sdkClientMethodCSWithTT.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { buildInstanceId } from './identity';
* (default client) or the client method (shared clients).
*/
export function sdkClientMethodCSFactory(params: ISdkFactoryContext): (key?: SplitIO.SplitKey, trafficType?: string) => SplitIO.ICsClient {
const { clients, storage, syncManager, sdkReadinessManager, settings: { core: { key, trafficType }, log }, whenInit } = params;
const { clients, storage, syncManager, sdkReadinessManager, settings: { core: { key, trafficType }, log } } = params;

const mainClientInstance = clientCSDecorator(
log,
Expand Down Expand Up @@ -86,11 +86,6 @@ export function sdkClientMethodCSFactory(params: ISdkFactoryContext): (key?: Spl
validTrafficType
);

whenInit(() => {
sharedSdkReadiness.readinessManager.init();
sharedSyncManager && sharedSyncManager.start();
});

log.info(NEW_SHARED_CLIENT);
} else {
log.debug(RETRIEVE_CLIENT_EXISTING);
Expand Down
6 changes: 0 additions & 6 deletions src/sync/__tests__/syncManagerOnline.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,9 @@ test('syncManagerOnline should syncAll a single time when sync is disabled', ()

if (!pollingSyncManagerShared) throw new Error('pollingSyncManagerShared should exist');

pollingSyncManagerShared.start();

expect(pollingManagerMock.start).not.toBeCalled();

pollingSyncManagerShared.stop();
pollingSyncManagerShared.start();

expect(pollingManagerMock.start).not.toBeCalled();

Expand All @@ -153,12 +150,9 @@ test('syncManagerOnline should syncAll a single time when sync is disabled', ()

if (!pushingSyncManagerShared) throw new Error('pushingSyncManagerShared should exist');

pushingSyncManagerShared.start();

expect(pollingManagerMock.start).not.toBeCalled();

pushingSyncManagerShared.stop();
pushingSyncManagerShared.start();

expect(pollingManagerMock.start).not.toBeCalled();

Expand Down
36 changes: 19 additions & 17 deletions src/sync/syncManagerOnline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,27 +143,29 @@ export function syncManagerOnlineFactory(

const mySegmentsSyncTask = (pollingManager as IPollingManagerCS).add(matchingKey, readinessManager, storage);

return {
isRunning: mySegmentsSyncTask.isRunning,
start() {
if (syncEnabled) {
if (pushManager) {
if (pollingManager!.isRunning()) {
// if doing polling, we must start the periodic fetch of data
if (storage.splits.usesSegments()) mySegmentsSyncTask.start();
} else {
// if not polling, we must execute the sync task for the initial fetch
// of segments since `syncAll` was already executed when starting the main client
mySegmentsSyncTask.execute();
}
pushManager.add(matchingKey, mySegmentsSyncTask);
} else {
if (running) {
if (syncEnabled) {
if (pushManager) {
if (pollingManager!.isRunning()) {
// if doing polling, we must start the periodic fetch of data
if (storage.splits.usesSegments()) mySegmentsSyncTask.start();
} else {
// if not polling, we must execute the sync task for the initial fetch
// of segments since `syncAll` was already executed when starting the main client
mySegmentsSyncTask.execute();
}
pushManager.add(matchingKey, mySegmentsSyncTask);
} else {
if (!readinessManager.isReady()) mySegmentsSyncTask.execute();
if (storage.splits.usesSegments()) mySegmentsSyncTask.start();
}
},
} else {
if (!readinessManager.isReady()) mySegmentsSyncTask.execute();
}
}

return {
isRunning: mySegmentsSyncTask.isRunning,

stop() {
// check in case `client.destroy()` has been invoked more than once for the same client
const mySegmentsSyncTask = (pollingManager as IPollingManagerCS).get(matchingKey);
Expand Down
2 changes: 1 addition & 1 deletion src/sync/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,5 @@ export interface ISyncManager extends ITask {
}

export interface ISyncManagerCS extends ISyncManager {
shared(matchingKey: string, readinessManager: IReadinessManager, storage: IStorageSync): ISyncManager | undefined
shared(matchingKey: string, readinessManager: IReadinessManager, storage: IStorageSync): Pick<ISyncManager, 'stop' | 'flush'> | undefined
}

0 comments on commit bdf69e8

Please sign in to comment.