Merge pull request #119 from splitio/update_shared_clients_status

Update shared client status to store `isReadyFromCache` property

Author: EmilianoSanchez

src/__tests__/asyncActions.browser.test.ts

@@ -293,14 +293,13 @@ describe('getTreatments', () => {
         expect(store.getActions().length).toBe(4);
 
         // getting the evaluation result and validating it matches the results from SDK calls
-        const treatments = action.payload.treatments;
         expect(splitSdk.factory.client().getTreatmentsWithConfig).toHaveBeenNthCalledWith(3, ['split1'], undefined);
         expect(splitSdk.factory.client().getTreatmentsWithConfig).toHaveBeenNthCalledWith(4, ['split2', 'split3'], attributes);
         const expectedTreatments = {
           ...(splitSdk.factory.client().getTreatmentsWithConfig as jest.Mock).mock.results[2].value,
           ...(splitSdk.factory.client().getTreatmentsWithConfig as jest.Mock).mock.results[3].value,
         };
-        expect(treatments).toEqual(expectedTreatments);
+        expect(action.payload.treatments).toEqual(expectedTreatments);
 
         expect(splitSdk.factory.client().getTreatmentsWithConfig).toBeCalledTimes(4); // control assertion - getTreatmentsWithConfig calls
         expect(getClient(splitSdk).evalOnUpdate).toEqual({}); // control assertion - cbs scheduled for update
@@ -524,41 +523,42 @@ describe('getTreatments', () => {
     // Init SDK and set ready
     const store = mockStore(STATE_INITIAL);
     const actionResult = store.dispatch<any>(initSplitSdk({ config: sdkBrowserConfig }));
+    (splitSdk.factory as any).client().__emitter__.emit(Event.SDK_READY_FROM_CACHE);
     (splitSdk.factory as any).client().__emitter__.emit(Event.SDK_READY);
 
     actionResult.then(() => {
-      // SPLIT_READY should have been dispatched
-      expect(store.getActions().length).toBe(1);
-      let action = store.getActions()[0];
-      expect(action).toEqual({
-        type: SPLIT_READY,
-        payload: {
-          timestamp: expect.any(Number)
-        }
-      });
-
-      // If SDK is ready for the main key and a getTreatment is dispatched for a different user key:
-      // the item is added to the 'evalOnReady' list of the new client,
+      // SDK_READY_FROM_CACHE & SPLIT_READY should have been dispatched
+      expect(store.getActions()).toEqual([{
+        type: SPLIT_READY_FROM_CACHE, payload: { timestamp: expect.any(Number) }
+      }, {
+        type: SPLIT_READY, payload: { timestamp: expect.any(Number) }
+      }]);
+
+      // If getTreatment is dispatched for a different user key, the item is added to the 'evalOnReady' list of the new client
       store.dispatch<any>(getTreatments({ splitNames: 'split2', key: 'other-user-key' }));
       expect(getClient(splitSdk).evalOnReady.length).toEqual(0); // control assertion - no evaluations were registered for SDK_READY on main client
       expect(getClient(splitSdk, 'other-user-key').evalOnReady.length).toEqual(1); // control assertion - 1 evaluation was registered for SDK_READY on the new client
       expect(getClient(splitSdk).evalOnUpdate).toEqual({});
 
-      // and an ADD_TREATMENTS action is dispatched with control treatments without calling SDK client.
-      action = store.getActions()[1];
+      // If SDK was ready from cache, the SPLIT_READY_FROM_CACHE_WITH_EVALUATIONS action is dispatched for the new clients, calling SDK client to evaluate from cache
+      let action = store.getActions()[2];
       expect(action).toEqual({
-        type: ADD_TREATMENTS,
+        type: SPLIT_READY_FROM_CACHE_WITH_EVALUATIONS,
         payload: {
           key: 'other-user-key',
-          treatments: getControlTreatmentsWithConfig(['split2'])
+          timestamp: 0,
+          treatments: expect.any(Object),
+          nonDefaultKey: true
         }
       });
-      expect(splitSdk.factory.client('other-user-key').getTreatmentsWithConfig).toBeCalledTimes(0);
+
+      expect(splitSdk.factory.client('other-user-key').getTreatmentsWithConfig).lastCalledWith(['split2'], undefined);
+      expect(splitSdk.factory.client('other-user-key').getTreatmentsWithConfig).toHaveLastReturnedWith(action.payload.treatments);
 
       (splitSdk.factory as any).client('other-user-key').__emitter__.emit(Event.SDK_READY, 'other-user-key');
 
       // The SPLIT_READY_WITH_EVALUATIONS action is dispatched synchronously once the SDK is ready for the new user key
-      action = store.getActions()[2];
+      action = store.getActions()[3];
       expect(action).toEqual({
         type: SPLIT_READY_WITH_EVALUATIONS,
         payload: {
@@ -570,16 +570,15 @@ describe('getTreatments', () => {
       });
 
       // getting the evaluation result and validating it matches the results from SDK
-      const treatments = action.payload.treatments;
       expect(splitSdk.factory.client('other-user-key').getTreatmentsWithConfig).lastCalledWith(['split2'], undefined);
-      expect(splitSdk.factory.client('other-user-key').getTreatmentsWithConfig).toHaveLastReturnedWith(treatments);
+      expect(splitSdk.factory.client('other-user-key').getTreatmentsWithConfig).toHaveLastReturnedWith(action.payload.treatments);
 
       expect(getClient(splitSdk).evalOnUpdate).toEqual({}); // control assertion
 
       // The getTreatments is dispatched again, but this time is evaluated with attributes and registered for 'evalOnUpdate'
       const attributes = { att1: 'att1' };
       store.dispatch<any>(getTreatments({ splitNames: 'split2', attributes, key: 'other-user-key', evalOnUpdate: true }));
-      action = store.getActions()[3];
+      action = store.getActions()[4];
       expect(action).toEqual({
         type: ADD_TREATMENTS,
         payload: {
@@ -592,7 +591,7 @@ describe('getTreatments', () => {
 
       // The SPLIT_UPDATE_WITH_EVALUATIONS action is dispatched when the SDK is updated for the new user key
       (splitSdk.factory as any).client('other-user-key').__emitter__.emit(Event.SDK_UPDATE);
-      action = store.getActions()[4];
+      action = store.getActions()[5];
       expect(action).toEqual({
         type: SPLIT_UPDATE_WITH_EVALUATIONS,
         payload: {
@@ -608,7 +607,7 @@ describe('getTreatments', () => {
 
       // We deregister the item from evalOnUpdate.
       store.dispatch<any>(getTreatments({ splitNames: 'split2', key: 'other-user-key', evalOnUpdate: false }));
-      action = store.getActions()[5];
+      action = store.getActions()[6];
       expect(action).toEqual({
         type: ADD_TREATMENTS,
         payload: {
@@ -622,7 +621,7 @@ describe('getTreatments', () => {
 
       // Now, SDK_UPDATE events do not trigger SPLIT_UPDATE_WITH_EVALUATIONS but SPLIT_UPDATE instead
       (splitSdk.factory as any).client('other-user-key').__emitter__.emit(Event.SDK_UPDATE);
-      action = store.getActions()[6];
+      action = store.getActions()[7];
       expect(action).toEqual({
         type: SPLIT_UPDATE,
         payload: {
@@ -631,8 +630,8 @@ describe('getTreatments', () => {
         }
       });
 
-      expect(store.getActions().length).toBe(7); // control assertion - no more actions after the update.
-      expect(splitSdk.factory.client('other-user-key').getTreatmentsWithConfig).toBeCalledTimes(4); // control assertion - called 4 times, in actions SPLIT_READY_FROM_CACHE_WITH_EVALUATIONS, SPLIT_READY_WITH_EVALUATIONS, SPLIT_UPDATE_WITH_EVALUATIONS and ADD_TREATMENTS.
+      expect(store.getActions().length).toBe(8); // control assertion - no more actions after the update.
+      expect(splitSdk.factory.client('other-user-key').getTreatmentsWithConfig).toBeCalledTimes(5); // control assertion - called 5 times, in actions SPLIT_READY_FROM_CACHE_WITH_EVALUATIONS, SPLIT_READY_WITH_EVALUATIONS, ADD_TREATMENTS, SPLIT_UPDATE_WITH_EVALUATIONS and ADD_TREATMENTS.
 
       done();
     });

src/__tests__/helpers.browser.test.ts

@@ -192,7 +192,7 @@ describe('getStatus', () => {
     (splitSdk.factory as any).client('user_2').__emitter__.emit(Event.SDK_READY_FROM_CACHE);
 
     // Main client
-    const MAIN_CLIENT_STATUS = { ...STATUS_INITIAL, isReady: true, isOperational: true, lastUpdate: (splitSdk.factory.client() as any).__getStatus().lastUpdate };
+    const MAIN_CLIENT_STATUS = { ...STATUS_INITIAL, isReadyFromCache: true, isReady: true, isOperational: true, lastUpdate: (splitSdk.factory.client() as any).__getStatus().lastUpdate };
     expect(getStatus()).toEqual(MAIN_CLIENT_STATUS);
     expect(getStatus(sdkBrowserConfig.core.key)).toEqual(MAIN_CLIENT_STATUS);
     expect(getStatus({ matchingKey: sdkBrowserConfig.core.key as string, bucketingKey: '' })).toEqual(MAIN_CLIENT_STATUS);

src/__tests__/utils/mockBrowserSplitSdk.ts

@@ -30,10 +30,12 @@ export function mockSdk() {
 
   return jest.fn((config: SplitIO.IBrowserSettings, __updateModules?: (modules: { settings: { version: string } }) => void) => {
 
+    // ATM, isReadyFromCache is shared among clients
+    let isReadyFromCache = false;
+
     function mockClient(key?: SplitIO.SplitKey) {
       // Readiness
       let isReady = false;
-      let isReadyFromCache = false;
       let hasTimedout = false;
       let isDestroyed = false;
       let lastUpdate = 0;

src/asyncActions.ts

@@ -148,6 +148,7 @@ export function getTreatments(params: IGetTreatmentsParams): Action | (() => voi
       client.evalOnReady.push(params);
     }
 
+    // @TODO breaking: consider removing `evalOnReadyFromCache` config option, since `false` value has no effect on shared clients (they are ready from cache immediately) and on the main client if its ready from cache when `getTreatments` is called
     // If the SDK is not ready from cache and flag `evalOnReadyFromCache`, it stores the action to execute when ready from cache
     if (!status.isReadyFromCache && params.evalOnReadyFromCache) {
       client.evalOnReadyFromCache.push(params);
@@ -156,7 +157,12 @@ export function getTreatments(params: IGetTreatmentsParams): Action | (() => voi
     if (status.isOperational) {
       // If the SDK is operational (i.e., it is ready or ready from cache), it evaluates and adds treatments to the store
       const treatments = __getTreatments(client, [params]);
-      return addTreatments(params.key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments);
+
+      // Shared clients might be ready from cache immediately, so we need to dispatch a single action that updates treatments and `isReadyFromCache` status atomically
+      // @TODO handle this corner case by refactoring actions into a single action that includes both the client status and optional evaluation/s, to minimize state changes and avoid edge cases
+      return status.isReadyFromCache && !status.isReady && !isMainClient(params.key) ?
+        splitReadyFromCacheWithEvaluations(params.key, treatments, status.lastUpdate, true) :
+        addTreatments(params.key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments);
     } else {
       // Otherwise, it adds control treatments to the store, without calling the SDK (no impressions sent)
       // With flag sets, an empty object is passed since we don't know their feature flag names
@@ -241,7 +247,7 @@ export function getClient(splitSdk: ISplitSdk, key?: SplitIO.SplitKey): IClientN
     if (splitSdk.dispatch) splitSdk.dispatch(splitTimedout(__getStatus(client).lastUpdate, key));
   });
 
-  // On SDK timed out, dispatch `splitReadyFromCache` action
+  // On SDK ready from cache, dispatch `splitReadyFromCache` action
   client.once(client.Event.SDK_READY_FROM_CACHE, function onReadyFromCache() {
     if (!splitSdk.dispatch) return;