Update destroy logic in client side [WIP]

src/logger/constants.ts

@@ -36,7 +36,6 @@ export const IMPRESSION = 102;
 export const IMPRESSION_QUEUEING = 103;
 export const NEW_SHARED_CLIENT = 104;
 export const NEW_FACTORY = 105;
-export const POLLING_SMART_PAUSING = 106;
 export const POLLING_START = 107;
 export const POLLING_STOP = 108;
 export const SYNC_SPLITS_FETCH_RETRY = 109;

src/logger/messages/info.ts

@@ -19,7 +19,6 @@ export const codesInfo: [number, string][] = codesWarn.concat([
   [c.USER_CONSENT_INITIAL, 'Starting the SDK with %s user consent. No data will be sent.'],
 
   // synchronizer
-  [c.POLLING_SMART_PAUSING, c.LOG_PREFIX_SYNC_POLLING + 'Turning segments data polling %s.'],
   [c.POLLING_START, c.LOG_PREFIX_SYNC_POLLING + 'Starting polling'],
   [c.POLLING_STOP, c.LOG_PREFIX_SYNC_POLLING + 'Stopping polling'],
   [c.SYNC_SPLITS_FETCH_RETRY, c.LOG_PREFIX_SYNC_SPLITS + 'Retrying download of feature flags #%s. Reason: %s'],

src/sdkClient/__tests__/sdkClientMethod.spec.ts

@@ -11,19 +11,19 @@ const paramMocks = [
   {
     storage: { destroy: jest.fn(() => Promise.resolve()) },
     syncManager: undefined,
-    sdkReadinessManager: { sdkStatus: jest.fn(), readinessManager: { destroy: jest.fn() } },
+    sdkReadinessManager: { sdkStatus: { __getStatus: () => ({ isDestroyed: true }) }, readinessManager: { destroy: jest.fn() } },
     signalListener: undefined,
-    settings: { mode: CONSUMER_MODE, log: loggerMock, core: { authorizationKey: 'sdk key '} },
+    settings: { mode: CONSUMER_MODE, log: loggerMock, core: { authorizationKey: 'sdk key ' } },
     telemetryTracker: telemetryTrackerFactory(),
     clients: {}
   },
   // SyncManager (i.e., Sync SDK) and Signal listener
   {
     storage: { destroy: jest.fn() },
     syncManager: { stop: jest.fn(), flush: jest.fn(() => Promise.resolve()) },
-    sdkReadinessManager: { sdkStatus: jest.fn(), readinessManager: { destroy: jest.fn() } },
+    sdkReadinessManager: { sdkStatus: { __getStatus: () => ({ isDestroyed: true }) }, readinessManager: { destroy: jest.fn() } },
     signalListener: { stop: jest.fn() },
-    settings: { mode: STANDALONE_MODE, log: loggerMock, core: { authorizationKey: 'sdk key '} },
+    settings: { mode: STANDALONE_MODE, log: loggerMock, core: { authorizationKey: 'sdk key ' } },
     telemetryTracker: telemetryTrackerFactory(),
     clients: {}
   }

src/sdkClient/__tests__/sdkClientMethodCS.spec.ts

@@ -14,28 +14,33 @@ const storageMock = {
   })
 };
 
-const partialSdkReadinessManagers: { sdkStatus: jest.Mock, readinessManager: { destroy: jest.Mock } }[] = [];
+function readinessManagerMock() {
+  let isDestroyed = false;
+  return {
+    sdkStatus: { __getStatus: () => ({ isDestroyed }), },
+    readinessManager: { destroy: jest.fn(() => { isDestroyed = true; }) },
+    _undestroy: () => { isDestroyed = false; }
+  };
+}
+
+const partialSdkReadinessManagers: { sdkStatus: any, readinessManager: { destroy: jest.Mock } }[] = [];
 
 const sdkReadinessManagerMock = {
-  sdkStatus: jest.fn(),
-  readinessManager: { destroy: jest.fn() },
+  ...readinessManagerMock(),
   shared: jest.fn(() => {
-    partialSdkReadinessManagers.push({
-      sdkStatus: jest.fn(),
-      readinessManager: { destroy: jest.fn() },
-    });
+    partialSdkReadinessManagers.push(readinessManagerMock());
     return partialSdkReadinessManagers[partialSdkReadinessManagers.length - 1];
   })
 };
 
-const partialSyncManagers: { start: jest.Mock, stop: jest.Mock, flush: jest.Mock }[] = [];
+const mySegmentsSyncManagers: { start: jest.Mock, stop: jest.Mock }[] = [];
 
 const syncManagerMock = {
   stop: jest.fn(),
   flush: jest.fn(() => Promise.resolve()),
   shared: jest.fn(() => {
-    partialSyncManagers.push({ start: jest.fn(), stop: jest.fn(), flush: jest.fn(() => Promise.resolve()) });
-    return partialSyncManagers[partialSyncManagers.length - 1];
+    mySegmentsSyncManagers.push({ start: jest.fn(), stop: jest.fn() });
+    return mySegmentsSyncManagers[mySegmentsSyncManagers.length - 1];
   })
 };
 
@@ -70,8 +75,9 @@ describe('sdkClientMethodCSFactory', () => {
   afterEach(() => {
     jest.clearAllMocks();
     partialStorages.length = 0;
+    sdkReadinessManagerMock._undestroy();
     partialSdkReadinessManagers.length = 0;
-    partialSyncManagers.length = 0;
+    mySegmentsSyncManagers.length = 0;
     params.clients = {};
   });
 
@@ -129,25 +135,30 @@ describe('sdkClientMethodCSFactory', () => {
     // shared methods call once per each new client
     expect(params.storage.shared).toBeCalledTimes(newClients.size);
     expect(params.sdkReadinessManager.shared).toBeCalledTimes(newClients.size);
-    expect(params.syncManager.shared).toBeCalledTimes(newClients.size);
+    expect(params.syncManager.shared).toBeCalledTimes(newClients.size + 1);
 
-    // `client.destroy` of partial clients should stop internal partial components
+    // `client.destroy` should flush and stop partial components
     await Promise.all(Array.from(newClients).map(newClient => newClient.destroy()));
 
     partialSdkReadinessManagers.forEach((partialSdkReadinessManager) => expect(partialSdkReadinessManager.readinessManager.destroy).toBeCalledTimes(1));
     partialStorages.forEach((partialStorage) => expect(partialStorage.destroy).toBeCalledTimes(1));
-    partialSyncManagers.forEach((partialSyncManager) => {
-      expect(partialSyncManager.stop).toBeCalledTimes(1);
-      expect(partialSyncManager.flush).toBeCalledTimes(1);
-    });
+    mySegmentsSyncManagers.slice(1).forEach((mySegmentsSyncManager) => expect(mySegmentsSyncManager.stop).toBeCalledTimes(1));
+    expect(params.syncManager.flush).toBeCalledTimes(newClients.size);
 
-    // `client.destroy` of partial clients shouldn't stop internal main components
+    // `client.destroy` shouldn't stop main components
     expect(params.sdkReadinessManager.readinessManager.destroy).not.toBeCalled();
     expect(params.storage.destroy).not.toBeCalled();
     expect(params.syncManager.stop).not.toBeCalled();
-    expect(params.syncManager.flush).not.toBeCalled();
     expect(params.signalListener.stop).not.toBeCalled();
 
+    // Except the last client is destroyed
+    await sdkClientMethod().destroy();
+
+    expect(params.sdkReadinessManager.readinessManager.destroy).toBeCalledTimes(1);
+    expect(params.storage.destroy).toBeCalledTimes(1);
+    expect(params.syncManager.stop).toBeCalledTimes(1);
+    expect(params.syncManager.flush).toBeCalledTimes(newClients.size + 1);
+    expect(params.signalListener.stop).toBeCalledTimes(1);
   });
 
   test.each(testTargets)('return main client instance if called with same key', (sdkClientMethodCSFactory) => {
@@ -160,7 +171,7 @@ describe('sdkClientMethodCSFactory', () => {
 
     expect(params.storage.shared).not.toBeCalled();
     expect(params.sdkReadinessManager.shared).not.toBeCalled();
-    expect(params.syncManager.shared).not.toBeCalled();
+    expect(params.syncManager.shared).toBeCalledTimes(1);
   });
 
   test.each(testTargets)('return main client instance if called with same key and TT', (sdkClientMethodCSFactory) => {
@@ -173,7 +184,7 @@ describe('sdkClientMethodCSFactory', () => {
 
     expect(params.storage.shared).not.toBeCalled();
     expect(params.sdkReadinessManager.shared).not.toBeCalled();
-    expect(params.syncManager.shared).not.toBeCalled();
+    expect(params.syncManager.shared).toBeCalledTimes(1);
   });
 
   test.each(testTargets)('return main client instance if called with same key object', (sdkClientMethodCSFactory) => {
@@ -186,7 +197,7 @@ describe('sdkClientMethodCSFactory', () => {
 
     expect(params.storage.shared).not.toBeCalled();
     expect(params.sdkReadinessManager.shared).not.toBeCalled();
-    expect(params.syncManager.shared).not.toBeCalled();
+    expect(params.syncManager.shared).toBeCalledTimes(1);
   });
 
   test.each(testTargets)('return same client instance if called with same key or traffic type (input validation)', (sdkClientMethodCSFactory, ignoresTT) => {
@@ -201,15 +212,15 @@ describe('sdkClientMethodCSFactory', () => {
 
     expect(params.storage.shared).toBeCalledTimes(1);
     expect(params.sdkReadinessManager.shared).toBeCalledTimes(1);
-    expect(params.syncManager.shared).toBeCalledTimes(1);
+    expect(params.syncManager.shared).toBeCalledTimes(2);
 
     expect(sdkClientMethod('KEY', 'tt')).not.toBe(clientInstance); // New client created: key is case-sensitive
     if (!ignoresTT) expect(sdkClientMethod('key', 'TT ')).not.toBe(clientInstance); // New client created: TT is not trimmed
 
     const clientCount = ignoresTT ? 2 : 3;
     expect(params.storage.shared).toBeCalledTimes(clientCount);
     expect(params.sdkReadinessManager.shared).toBeCalledTimes(clientCount);
-    expect(params.syncManager.shared).toBeCalledTimes(clientCount);
+    expect(params.syncManager.shared).toBeCalledTimes(clientCount + 1);
   });
 
   test.each(testTargets)('invalid calls throw an error', (sdkClientMethodCSFactory, ignoresTT) => {

src/sdkClient/sdkClient.ts

@@ -1,6 +1,6 @@
 import { objectAssign } from '../utils/lang/objectAssign';
 import { IStatusInterface, SplitIO } from '../types';
-import { releaseApiKey } from '../utils/inputValidation/apiKey';
+import { areAllClientDestroyed, releaseApiKey } from '../utils/inputValidation/apiKey';
 import { clientFactory } from './client';
 import { clientInputValidationDecorator } from './clientInputValidation';
 import { ISdkFactoryContext } from '../sdkFactory/types';
@@ -10,9 +10,10 @@ const COOLDOWN_TIME_IN_MILLIS = 1000;
 /**
  * Creates an Sdk client, i.e., a base client with status and destroy interface
  */
-export function sdkClientFactory(params: ISdkFactoryContext, isSharedClient?: boolean): SplitIO.IClient | SplitIO.IAsyncClient {
-  const { sdkReadinessManager, syncManager, storage, signalListener, settings, telemetryTracker, uniqueKeysTracker } = params;
+export function sdkClientFactory(params: ISdkFactoryContext): SplitIO.IClient | SplitIO.IAsyncClient {
+  const { clients, sdkReadinessManager, syncManager, mySegmentsSyncManager, storage, signalListener, settings, telemetryTracker, uniqueKeysTracker } = params;
 
+  let destroyPromise: Promise<void> | undefined;
   let lastActionTime = 0;
 
   function __cooldown(func: Function, time: number) {
@@ -53,21 +54,25 @@ export function sdkClientFactory(params: ISdkFactoryContext, isSharedClient?: bo
         return __cooldown(__flush, COOLDOWN_TIME_IN_MILLIS);
       },
       destroy() {
-        // Mark the SDK as destroyed immediately
+        if (destroyPromise) return destroyPromise;
+
+        // Mark the client as destroyed immediately
         sdkReadinessManager.readinessManager.destroy();
 
-        // For main client, release the SDK Key and record stat before flushing data
-        if (!isSharedClient) {
+        const isLastDestroyCall = areAllClientDestroyed(clients);
+
+        // Only for client-side standalone
+        mySegmentsSyncManager && mySegmentsSyncManager.stop();
+
+        // For last client, release the SDK Key and record stat before flushing data
+        if (isLastDestroyCall) {
           releaseApiKey(settings.core.authorizationKey);
           telemetryTracker.sessionLength();
+          syncManager && syncManager.stop();
         }
 
-        // Stop background jobs
-        syncManager && syncManager.stop();
-
-        return __flush().then(() => {
-          // For main client, cleanup event listeners and scheduled jobs
-          if (!isSharedClient) {
+        return destroyPromise = __flush().then(() => {
+          if (isLastDestroyCall) {
             signalListener && signalListener.stop();
             uniqueKeysTracker && uniqueKeysTracker.stop();
           }

src/sdkClient/sdkClientMethodCS.ts

@@ -9,6 +9,7 @@ import { RETRIEVE_CLIENT_DEFAULT, NEW_SHARED_CLIENT, RETRIEVE_CLIENT_EXISTING, L
 import { SDK_SEGMENTS_ARRIVED } from '../readiness/constants';
 import { ISdkFactoryContext } from '../sdkFactory/types';
 import { buildInstanceId } from './identity';
+import { IStorageSync } from '../storages/types';
 
 /**
  * Factory of client method for the client-side API variant where TT is ignored.
@@ -19,7 +20,9 @@ export function sdkClientMethodCSFactory(params: ISdkFactoryContext): (key?: Spl
 
   const mainClientInstance = clientCSDecorator(
     log,
-    sdkClientFactory(params) as SplitIO.IClient,
+    sdkClientFactory(objectAssign({}, params, {
+      mySegmentsSyncManager: syncManager && storage.shared && (syncManager as ISyncManagerCS).shared(getMatching(key), sdkReadinessManager.readinessManager, storage as IStorageSync),
+    })) as SplitIO.IClient,
     key
   );
 
@@ -57,11 +60,11 @@ export function sdkClientMethodCSFactory(params: ISdkFactoryContext): (key?: Spl
       });
 
       // 3 possibilities:
-      // - Standalone mode: both syncManager and sharedSyncManager are defined
-      // - Consumer mode: both syncManager and sharedSyncManager are undefined
-      // - Consumer partial mode: syncManager is defined (only for submitters) but sharedSyncManager is undefined
+      // - Standalone mode: both syncManager and mySegmentsSyncManager are defined
+      // - Consumer mode: both syncManager and mySegmentsSyncManager are undefined
+      // - Consumer partial mode: syncManager is defined (only for submitters) but mySegmentsSyncManager not
       // @ts-ignore
-      const sharedSyncManager = syncManager && sharedStorage && (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage);
+      const mySegmentsSyncManager = syncManager && sharedStorage && (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage);
 
       // As shared clients reuse all the storage information, we don't need to check here if we
       // will use offline or online mode. We should stick with the original decision.
@@ -70,12 +73,12 @@ export function sdkClientMethodCSFactory(params: ISdkFactoryContext): (key?: Spl
         sdkClientFactory(objectAssign({}, params, {
           sdkReadinessManager: sharedSdkReadiness,
           storage: sharedStorage || storage,
-          syncManager: sharedSyncManager,
-        }), true) as SplitIO.IClient,
+          mySegmentsSyncManager,
+        })) as SplitIO.IClient,
         validKey
       );
 
-      sharedSyncManager && sharedSyncManager.start();
+      mySegmentsSyncManager && mySegmentsSyncManager.start();
 
       log.info(NEW_SHARED_CLIENT);
     } else {

src/sdkClient/sdkClientMethodCSWithTT.ts

@@ -10,6 +10,7 @@ import { RETRIEVE_CLIENT_DEFAULT, NEW_SHARED_CLIENT, RETRIEVE_CLIENT_EXISTING, L
 import { SDK_SEGMENTS_ARRIVED } from '../readiness/constants';
 import { ISdkFactoryContext } from '../sdkFactory/types';
 import { buildInstanceId } from './identity';
+import { IStorageSync } from '../storages/types';
 
 /**
  * Factory of client method for the client-side (browser) variant of the Isomorphic JS SDK,
@@ -21,7 +22,9 @@ export function sdkClientMethodCSFactory(params: ISdkFactoryContext): (key?: Spl
 
   const mainClientInstance = clientCSDecorator(
     log,
-    sdkClientFactory(params) as SplitIO.IClient,
+    sdkClientFactory(objectAssign({}, params, {
+      mySegmentsSyncManager: syncManager && storage.shared && (syncManager as ISyncManagerCS).shared(getMatching(key), sdkReadinessManager.readinessManager, storage as IStorageSync),
+    })) as SplitIO.IClient,
     key,
     trafficType
   );
@@ -67,11 +70,11 @@ export function sdkClientMethodCSFactory(params: ISdkFactoryContext): (key?: Spl
       });
 
       // 3 possibilities:
-      // - Standalone mode: both syncManager and sharedSyncManager are defined
-      // - Consumer mode: both syncManager and sharedSyncManager are undefined
-      // - Consumer partial mode: syncManager is defined (only for submitters) but sharedSyncManager is undefined
+      // - Standalone mode: both syncManager and mySegmentsSyncManager are defined
+      // - Consumer mode: both syncManager and mySegmentsSyncManager are undefined
+      // - Consumer partial mode: syncManager is defined (only for submitters) but mySegmentsSyncManager not
       // @ts-ignore
-      const sharedSyncManager = syncManager && sharedStorage && (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage);
+      const mySegmentsSyncManager = syncManager && sharedStorage && (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage);
 
       // As shared clients reuse all the storage information, we don't need to check here if we
       // will use offline or online mode. We should stick with the original decision.
@@ -80,13 +83,13 @@ export function sdkClientMethodCSFactory(params: ISdkFactoryContext): (key?: Spl
         sdkClientFactory(objectAssign({}, params, {
           sdkReadinessManager: sharedSdkReadiness,
           storage: sharedStorage || storage,
-          syncManager: sharedSyncManager,
-        }), true) as SplitIO.IClient,
+          mySegmentsSyncManager,
+        })) as SplitIO.IClient,
         validKey,
         validTrafficType
       );
 
-      sharedSyncManager && sharedSyncManager.start();
+      mySegmentsSyncManager && mySegmentsSyncManager.start();
 
       log.info(NEW_SHARED_CLIENT);
     } else {

src/sdkFactory/types.ts

@@ -5,7 +5,7 @@ import type { sdkManagerFactory } from '../sdkManager';
 import type { splitApiFactory } from '../services/splitApi';
 import { IFetch, ISplitApi, IEventSourceConstructor } from '../services/types';
 import { IStorageAsync, IStorageSync, IStorageFactoryParams } from '../storages/types';
-import { ISyncManager } from '../sync/types';
+import { ISyncManager, ITask } from '../sync/types';
 import { IImpressionObserver } from '../trackers/impressionObserver/types';
 import { IImpressionsTracker, IEventTracker, ITelemetryTracker, IFilterAdapter, IUniqueKeysTracker } from '../trackers/types';
 import { SplitIO, ISettings, IEventEmitter, IBasicClient } from '../types';
@@ -49,6 +49,7 @@ export interface ISdkFactoryContext {
   signalListener?: ISignalListener
   splitApi?: ISplitApi
   syncManager?: ISyncManager,
+  mySegmentsSyncManager?: ITask,
   clients: Record<string, IBasicClient>,
 }