From 065192c228e272ad77f14e7870d161f0562a3b6b Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 14 Sep 2021 18:19:20 -0300 Subject: [PATCH 1/7] implementation and tests --- src/__tests__/offline/browser.spec.js | 32 +++++++++++++++---- src/factory/offline.js | 4 +-- .../updater/SplitChangesFromObject.js | 14 +++++--- src/utils/__tests__/settings/index.spec.js | 1 + src/utils/settings/storage/browser.js | 7 ---- 5 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/__tests__/offline/browser.spec.js b/src/__tests__/offline/browser.spec.js index 186510c7e..d2f806b7f 100644 --- a/src/__tests__/offline/browser.spec.js +++ b/src/__tests__/offline/browser.spec.js @@ -71,8 +71,8 @@ tape('Browser offline mode', function (assert) { assert.false(client.track({}, [], 'invalid_stuff')); assert.true(sharedClient.track('a_tt', 'another_ev_id', 10)); - assert.equal(client.getTreatment('testing_split'), 'control'); - assert.equal(sharedClient.getTreatment('testing_split'), 'control'); + assert.equal(client.getTreatment('testing_split'), 'control', 'control due to not ready'); + assert.equal(sharedClient.getTreatment('testing_split'), 'control', 'control due to not ready'); assert.equal(manager.splits().length, 0); // SDK events on shared client @@ -90,12 +90,29 @@ tape('Browser offline mode', function (assert) { const factories = [ SplitFactory(config), SplitFactory({ ...config }), - SplitFactory({ ...config, features: { ...config.features } }) + SplitFactory({ ...config, features: { ...config.features } }), + SplitFactory({ ...config, storage: { type: 'LOCALSTORAGE' } }) ]; - let readyCount = 0, updateCount = 0; + let readyCount = 0, updateCount = 0, readyFromCacheCount = 0; for (let i = 0; i < factories.length; i++) { - let client = factories[i].client(), manager = factories[i].manager(); + const factory = factories[i], client = factory.client(), manager = factory.manager(); + + client.on(client.Event.SDK_READY_FROM_CACHE, () => { + assert.equal(client.__context.get(client.__context.constants.READY_FROM_CACHE, true), true, 'READY_FROM_CACHE status must be true'); + assert.equal(client.__context.get(client.__context.constants.READY, true), undefined, 'READY status must not be set before READY_FROM_CACHE'); + assert.equal(client.getTreatment('testing_split_with_config'), 'off'); + readyFromCacheCount++; + + const sharedClient = factory.client('other'); + assert.equal(sharedClient.getTreatment('testing_split'), 'on', 'If ready from cache, shared clients should evaluate from cache'); + sharedClient.on(sharedClient.Event.SDK_READY_FROM_CACHE, () => { + assert.fail('Shared clients should not emit a SDK_READY_FROM_CACHE event. Only the main client does'); + }); + sharedClient.on(sharedClient.Event.SDK_READY, () => { + readyCount++; + }); + }); client.on(client.Event.SDK_READY, () => { assert.deepEqual(manager.names(), ['testing_split', 'testing_split_with_config']); @@ -103,8 +120,8 @@ tape('Browser offline mode', function (assert) { readyCount++; }); client.on(client.Event.SDK_UPDATE, () => { - assert.equal(client.getTreatment('testing_split_with_config'), 'nope'); assert.deepEqual(manager.names(), ['testing_split', 'testing_split_2', 'testing_split_3', 'testing_split_with_config']); + assert.equal(client.getTreatment('testing_split_with_config'), 'nope'); updateCount++; }); } @@ -308,8 +325,9 @@ tape('Browser offline mode', function (assert) { assert.equal(sharedUpdateCount, 1, 'Shared client should have emitted SDK_UPDATE event once'); // SDK events on other factory clients - assert.equal(readyCount, factories.length, 'Each factory client should have emitted SDK_READY event once'); + assert.equal(readyCount, factories.length + 1, 'Each factory client should have emitted SDK_READY event once plus one shared client'); assert.equal(updateCount, factories.length - 1, 'Each factory client except one should have emitted SDK_UPDATE event once'); + assert.equal(readyFromCacheCount, 1, 'The main client of the factory with LOCALSTORAGE should have emitted SDK_READY_FROM_CACHE event'); assert.end(); }); diff --git a/src/factory/offline.js b/src/factory/offline.js index 36528ab00..70de58a87 100644 --- a/src/factory/offline.js +++ b/src/factory/offline.js @@ -12,8 +12,8 @@ function SplitFactoryOffline(context, sharedTrackers) { const storage = context.get(context.constants.STORAGE); const statusManager = context.get(context.constants.STATUS_MANAGER); - // In LOCALHOST mode, shared clients are ready in the next event-loop cycle than created - // and then updated on each SDK_SPLITS_ARRIVED event + // In LOCALHOST mode, shared clients are ready in the next event-loop cycle than created, and then updated on + // each SDK_SPLITS_ARRIVED event. As in online mode, SDK_READY_FROM_CACHE is only emitted by the main client. if (sharedInstance) setTimeout(() => { readiness.splits.on(readiness.splits.SDK_SPLITS_ARRIVED, () => { readiness.gate.emit(readiness.gate.SDK_UPDATE); diff --git a/src/producer/updater/SplitChangesFromObject.js b/src/producer/updater/SplitChangesFromObject.js index f61e2cc4e..2bb549e7a 100644 --- a/src/producer/updater/SplitChangesFromObject.js +++ b/src/producer/updater/SplitChangesFromObject.js @@ -24,7 +24,7 @@ function FromObjectUpdaterFactory(Fetcher, context) { [context.constants.STORAGE]: storage } = context.getAll(); - let firstTime = true; + let startingUp = true; return function ObjectUpdater() { const splits = []; @@ -58,13 +58,17 @@ function FromObjectUpdaterFactory(Fetcher, context) { }); return Promise.all([ - storage.splits.flush(), + storage.splits.flush(), // required to sync removed splits from mock + storage.splits.setChangeNumber(Date.now()), storage.splits.addSplits(splits) ]).then(() => { readiness.splits.emit(readiness.splits.SDK_SPLITS_ARRIVED); - // Only emits SDK_SEGMENTS_ARRIVED the first time for SDK_READY - if (firstTime) { - firstTime = false; + + if (startingUp) { + startingUp = false; + // If we have cached splits after starting up, let's notify that before the sdk gets ready. + if (storage.splits.checkCache()) readiness.gate.emit(readiness.gate.SDK_READY_FROM_CACHE); + // Only emits SDK_SEGMENTS_ARRIVED the first time for SDK_READY readiness.segments.emit(readiness.segments.SDK_SEGMENTS_ARRIVED); } }); diff --git a/src/utils/__tests__/settings/index.spec.js b/src/utils/__tests__/settings/index.spec.js index 18129c8c4..83447a9be 100644 --- a/src/utils/__tests__/settings/index.spec.js +++ b/src/utils/__tests__/settings/index.spec.js @@ -35,6 +35,7 @@ tape('SETTINGS / check defaults', assert => { }); assert.equal(settings.sync.impressionsMode, OPTIMIZED); assert.equal(settings.version, `${language}-${packageJson.version}`); + assert.true(packageJson.version.length <= 16, 'SDK version must not exceed 16 chars length'); assert.end(); }); diff --git a/src/utils/settings/storage/browser.js b/src/utils/settings/storage/browser.js index 2618e784b..0b90d5b05 100644 --- a/src/utils/settings/storage/browser.js +++ b/src/utils/settings/storage/browser.js @@ -18,14 +18,12 @@ import logFactory from '../../../utils/logger'; const log = logFactory('splitio-settings'); import isLocalStorageAvailable from '../../../utils/localstorage/isAvailable'; import { - LOCALHOST_MODE, STORAGE_MEMORY, STORAGE_LOCALSTORAGE } from '../../../utils/constants'; const ParseStorageSettings = settings => { let { - mode, storage: { type = STORAGE_MEMORY, options = {}, @@ -39,11 +37,6 @@ const ParseStorageSettings = settings => { prefix = 'SPLITIO'; } - if (mode === LOCALHOST_MODE) return { - type: STORAGE_MEMORY, - prefix - }; - // If an invalid storage type is provided OR we want to use LOCALSTORAGE and // it's not available, fallback into MEMORY if (type !== STORAGE_MEMORY && type !== STORAGE_LOCALSTORAGE || From be0087b61d87365885853afd151c61c3986f9f55 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 14 Sep 2021 18:49:35 -0300 Subject: [PATCH 2/7] fixed TT in localhost mode --- src/__tests__/offline/browser.spec.js | 8 ++++---- src/__tests__/offline/node.spec.js | 12 ++++++------ src/services/splitChanges/offline/browser.js | 1 + src/services/splitChanges/offline/node.js | 2 +- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/__tests__/offline/browser.spec.js b/src/__tests__/offline/browser.spec.js index d2f806b7f..f43c8b481 100644 --- a/src/__tests__/offline/browser.spec.js +++ b/src/__tests__/offline/browser.spec.js @@ -107,7 +107,7 @@ tape('Browser offline mode', function (assert) { const sharedClient = factory.client('other'); assert.equal(sharedClient.getTreatment('testing_split'), 'on', 'If ready from cache, shared clients should evaluate from cache'); sharedClient.on(sharedClient.Event.SDK_READY_FROM_CACHE, () => { - assert.fail('Shared clients should not emit a SDK_READY_FROM_CACHE event. Only the main client does'); + assert.fail('Shared clients don\'t emit SDK_READY_FROM_CACHE event. Only the main client does'); }); sharedClient.on(sharedClient.Event.SDK_READY, () => { readyCount++; @@ -157,10 +157,10 @@ tape('Browser offline mode', function (assert) { // Manager tests const expectedSplitView1 = { - name: 'testing_split', trafficType: null, killed: false, changeNumber: 0, treatments: ['on'], configs: {} + name: 'testing_split', trafficType: 'localhost', killed: false, changeNumber: 0, treatments: ['on'], configs: {} }; const expectedSplitView2 = { - name: 'testing_split_with_config', trafficType: null, killed: false, changeNumber: 0, treatments: ['off'], configs: { off: '{ "color": "blue" }' } + name: 'testing_split_with_config', trafficType: 'localhost', killed: false, changeNumber: 0, treatments: ['off'], configs: { off: '{ "color": "blue" }' } }; assert.deepEqual(manager.names(), ['testing_split', 'testing_split_with_config']); assert.deepEqual(manager.split('testing_split'), expectedSplitView1); @@ -258,7 +258,7 @@ tape('Browser offline mode', function (assert) { // Manager tests const expectedSplitView3 = { - name: 'testing_split_with_config', trafficType: null, killed: false, changeNumber: 0, treatments: ['nope'], configs: {} + name: 'testing_split_with_config', trafficType: 'localhost', killed: false, changeNumber: 0, treatments: ['nope'], configs: {} }; assert.deepEqual(manager.names(), ['testing_split', 'testing_split_2', 'testing_split_3', 'testing_split_with_config']); assert.deepEqual(manager.split('testing_split'), expectedSplitView1); diff --git a/src/__tests__/offline/node.spec.js b/src/__tests__/offline/node.spec.js index ac2b8ad79..ed3453292 100644 --- a/src/__tests__/offline/node.spec.js +++ b/src/__tests__/offline/node.spec.js @@ -251,15 +251,15 @@ function ManagerDotSplitTests(assert) { assert.deepEqual(manager.names(), ['testing_split', 'testing_split2', 'testing_split3']); const expectedView1 = { - name: 'testing_split', changeNumber: 0, killed: false, trafficType: null, + name: 'testing_split', changeNumber: 0, killed: false, trafficType: 'localhost', treatments: ['on'], configs: {} }; const expectedView2 = { - name: 'testing_split2', changeNumber: 0, killed: false, trafficType: null, + name: 'testing_split2', changeNumber: 0, killed: false, trafficType: 'localhost', treatments: ['off'], configs: {} }; const expectedView3 = { - name: 'testing_split3', changeNumber: 0, killed: false, trafficType: null, + name: 'testing_split3', changeNumber: 0, killed: false, trafficType: 'localhost', treatments: ['custom_treatment'], configs: {} }; @@ -397,15 +397,15 @@ function MultipleInstancesTests(assert) { assert.deepEqual(manager.names(), ['testing_split', 'testing_split2', 'testing_split3']); const expectedView1 = { - name: 'testing_split', changeNumber: 0, killed: false, trafficType: null, + name: 'testing_split', changeNumber: 0, killed: false, trafficType: 'localhost', treatments: ['on'], configs: {} }; const expectedView2 = { - name: 'testing_split2', changeNumber: 0, killed: false, trafficType: null, + name: 'testing_split2', changeNumber: 0, killed: false, trafficType: 'localhost', treatments: ['off'], configs: {} }; const expectedView3 = { - name: 'testing_split3', changeNumber: 0, killed: false, trafficType: null, + name: 'testing_split3', changeNumber: 0, killed: false, trafficType: 'localhost', treatments: ['custom_treatment'], configs: {} }; diff --git a/src/services/splitChanges/offline/browser.js b/src/services/splitChanges/offline/browser.js index 3273764a5..bc6673e52 100644 --- a/src/services/splitChanges/offline/browser.js +++ b/src/services/splitChanges/offline/browser.js @@ -54,6 +54,7 @@ export default function createGetConfigurationFromSettings() { if (config !== null) configurations[treatment] = config; splitObjects[splitName] = { + trafficTypeName: 'localhost', conditions: [parseCondition({ treatment })], configurations }; diff --git a/src/services/splitChanges/offline/node.js b/src/services/splitChanges/offline/node.js index 1386285fd..1037adfe8 100644 --- a/src/services/splitChanges/offline/node.js +++ b/src/services/splitChanges/offline/node.js @@ -111,7 +111,7 @@ export default function createGetSplitConfigForFile() { } else { const splitName = tuple[SPLIT_POSITION]; const condition = parseCondition({ treatment: tuple[TREATMENT_POSITION] }); - accum[splitName] = { conditions: [condition], configurations: {} }; + accum[splitName] = { conditions: [condition], configurations: {}, trafficTypeName: 'localhost' }; } } From 0705729193b96e07fecf99b41a2cf60b53e37cae Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 15 Sep 2021 13:26:57 -0300 Subject: [PATCH 3/7] fixed issue with shared clients created inmediatelly --- src/__tests__/offline/browser.spec.js | 33 ++++++++++++++++----------- src/index.js | 9 +++++--- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/__tests__/offline/browser.spec.js b/src/__tests__/offline/browser.spec.js index f43c8b481..1d3b6918f 100644 --- a/src/__tests__/offline/browser.spec.js +++ b/src/__tests__/offline/browser.spec.js @@ -96,22 +96,29 @@ tape('Browser offline mode', function (assert) { let readyCount = 0, updateCount = 0, readyFromCacheCount = 0; for (let i = 0; i < factories.length; i++) { - const factory = factories[i], client = factory.client(), manager = factory.manager(); + const factory = factories[i], client = factory.client(), manager = factory.manager(), sClient1 = factory.client('other'); - client.on(client.Event.SDK_READY_FROM_CACHE, () => { - assert.equal(client.__context.get(client.__context.constants.READY_FROM_CACHE, true), true, 'READY_FROM_CACHE status must be true'); - assert.equal(client.__context.get(client.__context.constants.READY, true), undefined, 'READY status must not be set before READY_FROM_CACHE'); - assert.equal(client.getTreatment('testing_split_with_config'), 'off'); - readyFromCacheCount++; + client.once(client.Event.SDK_READY_FROM_CACHE, () => { + const sClient2 = factory.client('another'); - const sharedClient = factory.client('other'); - assert.equal(sharedClient.getTreatment('testing_split'), 'on', 'If ready from cache, shared clients should evaluate from cache'); - sharedClient.on(sharedClient.Event.SDK_READY_FROM_CACHE, () => { - assert.fail('Shared clients don\'t emit SDK_READY_FROM_CACHE event. Only the main client does'); + [client, sClient1, sClient2].forEach(client => { + assert.equal(client.__context.get(client.__context.constants.READY_FROM_CACHE, true), true, 'If ready from cache, READY_FROM_CACHE status must be true'); + assert.equal(client.__context.get(client.__context.constants.READY, true), undefined, 'READY status must not be set before READY_FROM_CACHE'); + assert.equal(client.getTreatment('testing_split_with_config'), 'off', 'If ready from cache, clients should evaluate from cache'); + + client.on(client.Event.SDK_READY_FROM_CACHE, () => { + assert.fail('Shared clients don\'t emit SDK_READY_FROM_CACHE event. Only the main client does'); + }); }); - sharedClient.on(sharedClient.Event.SDK_READY, () => { - readyCount++; + + [sClient1, sClient2].forEach(client => { + client.on(client.Event.SDK_READY, () => { + assert.equal(client.__context.get(client.__context.constants.READY, true), true, 'READY status must be set once SDK_READY is emitted'); + readyCount++; + }); }); + + readyFromCacheCount++; }); client.on(client.Event.SDK_READY, () => { @@ -325,7 +332,7 @@ tape('Browser offline mode', function (assert) { assert.equal(sharedUpdateCount, 1, 'Shared client should have emitted SDK_UPDATE event once'); // SDK events on other factory clients - assert.equal(readyCount, factories.length + 1, 'Each factory client should have emitted SDK_READY event once plus one shared client'); + assert.equal(readyCount, factories.length + 2, 'Each factory client should have emitted SDK_READY event once plus two shared clients'); assert.equal(updateCount, factories.length - 1, 'Each factory client except one should have emitted SDK_UPDATE event once'); assert.equal(readyFromCacheCount, 1, 'The main client of the factory with LOCALSTORAGE should have emitted SDK_READY_FROM_CACHE event'); diff --git a/src/index.js b/src/index.js index 3efa70238..05b513813 100644 --- a/src/index.js +++ b/src/index.js @@ -105,9 +105,12 @@ export function SplitFactory(config) { const sharedSettings = settings.overrideKeyAndTT(validKey, validTrafficType); const sharedContext = new Context(); - const readiness = gateFactory(sharedSettings.startup.readyTimeout); - sharedContext.put(context.constants.READY_FROM_CACHE, context.get(context.constants.READY_FROM_CACHE, true)); - sharedContext.put(context.constants.READINESS, readiness); + const setReadyFromCache = () => sharedContext.put(context.constants.READY_FROM_CACHE, true); + if (context.get(context.constants.READY_FROM_CACHE, true)) setReadyFromCache(); + else readiness.gate.once(readiness.gate.SDK_READY_FROM_CACHE, setReadyFromCache); + + const sharedReadiness = gateFactory(sharedSettings.startup.readyTimeout); + sharedContext.put(context.constants.READINESS, sharedReadiness); // for shared clients, the internal offset of added/removed SDK_READY callbacks is -1 sharedContext.put(context.constants.STATUS_MANAGER, sdkStatusManager(sharedContext, -1)); sharedContext.put(context.constants.SETTINGS, sharedSettings); From 0ad4350f0e2df3de6ea363fc117e4579de8acb70 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 15 Sep 2021 15:38:00 -0300 Subject: [PATCH 4/7] shared clients might also emit SDK_READY_FROM_CACHE --- .../browserSuites/ready-from-cache.spec.js | 26 ++++++---- src/__tests__/offline/browser.spec.js | 51 +++++++++---------- src/index.js | 9 ++-- .../updater/SplitChangesFromObject.js | 4 +- src/readiness/index.js | 9 ++-- 5 files changed, 53 insertions(+), 46 deletions(-) diff --git a/src/__tests__/browserSuites/ready-from-cache.spec.js b/src/__tests__/browserSuites/ready-from-cache.spec.js index 8f556e170..647ce05b2 100644 --- a/src/__tests__/browserSuites/ready-from-cache.spec.js +++ b/src/__tests__/browserSuites/ready-from-cache.spec.js @@ -183,20 +183,20 @@ export default function (fetchMock, assert) { t.equal(client.getTreatment('always_on'), 'control', 'It should evaluate control treatments if not ready neither by cache nor the cloud'); t.equal(client3.getTreatment('always_on'), 'control', 'It should evaluate control treatments if not ready neither by cache nor the cloud'); - client.once(client.Event.SDK_READY_TIMED_OUT, () => { + client.on(client.Event.SDK_READY_TIMED_OUT, () => { t.fail('It should not timeout in this scenario.'); t.end(); }); - client.once(client.Event.SDK_READY_FROM_CACHE, () => { + client.on(client.Event.SDK_READY_FROM_CACHE, () => { t.true(Date.now() - startTime < 400, 'It should emit SDK_READY_FROM_CACHE on every client if there was data in the cache and we subscribe on time. Should be considerably faster than actual readiness from the cloud.'); t.equal(client.getTreatment('always_on'), 'off', 'It should evaluate treatments with data from cache instead of control due to Input Validation'); }); - client2.once(client2.Event.SDK_READY_FROM_CACHE, () => { + client2.on(client2.Event.SDK_READY_FROM_CACHE, () => { t.true(Date.now() - startTime < 400, 'It should emit SDK_READY_FROM_CACHE on every client if there was data in the cache and we subscribe on time. Should be considerably faster than actual readiness from the cloud.'); t.equal(client2.getTreatment('always_on'), 'off', 'It should evaluate treatments with data from cache instead of control due to Input Validation'); }); - client3.once(client3.Event.SDK_READY_FROM_CACHE, () => { + client3.on(client3.Event.SDK_READY_FROM_CACHE, () => { t.true(Date.now() - startTime < 400, 'It should emit SDK_READY_FROM_CACHE on every client if there was data in the cache and we subscribe on time. Should be considerably faster than actual readiness from the cloud.'); t.equal(client3.getTreatment('always_on'), 'off', 'It should evaluate treatments with data from cache instead of control due to Input Validation'); }); @@ -248,7 +248,7 @@ export default function (fetchMock, assert) { events: 'https://events.baseurl/readyFromCacheWithData3' }; localStorage.clear(); - t.plan(12 * 2 + 4); + t.plan(12 * 2 + 5); fetchMock.get(testUrls.sdk + '/splitChanges?since=25', function () { t.equal(localStorage.getItem('readyFromCache_3.SPLITIO.split.always_on'), alwaysOnSplitInverted, 'splits must not be cleaned from cache'); @@ -264,6 +264,7 @@ export default function (fetchMock, assert) { fetchMock.get(testUrls.sdk + '/mySegments/nicolas3%40split.io', function () { return new Promise(res => { setTimeout(() => res({ status: 200, body: { 'mySegments': [] }, headers: {} }), 1000); }); // Third client mySegments will come after 1s }); + fetchMock.get(testUrls.sdk + '/mySegments/nicolas4%40split.io', { 'mySegments': [] }); fetchMock.postOnce(testUrls.events + '/testImpressions/bulk', 200); fetchMock.postOnce(testUrls.events + '/testImpressions/count', 200); @@ -292,20 +293,27 @@ export default function (fetchMock, assert) { t.equal(client.getTreatment('always_on'), 'control', 'It should evaluate control treatments if not ready neither by cache nor the cloud'); t.equal(client3.getTreatment('always_on'), 'control', 'It should evaluate control treatments if not ready neither by cache nor the cloud'); - client.once(client.Event.SDK_READY_TIMED_OUT, () => { + client.on(client.Event.SDK_READY_TIMED_OUT, () => { t.fail('It should not timeout in this scenario.'); t.end(); }); - client.once(client.Event.SDK_READY_FROM_CACHE, () => { + client.on(client.Event.SDK_READY_FROM_CACHE, () => { t.true(Date.now() - startTime < 400, 'It should emit SDK_READY_FROM_CACHE on every client if there was data in the cache and we subscribe on time. Should be considerably faster than actual readiness from the cloud.'); t.equal(client.getTreatment('always_on'), 'off', 'It should evaluate treatments with data from cache instead of control due to Input Validation'); + + const client4 = splitio.client('nicolas4@split.io'); + t.equal(client4.getTreatment('always_on'), 'off', 'It should evaluate treatments with data from cache instead of control'); + + client4.on(client4.Event.SDK_READY_FROM_CACHE, () => { + t.fail('It should not emit SDK_READY_FROM_CACHE if already done.'); + }); }); - client2.once(client2.Event.SDK_READY_FROM_CACHE, () => { + client2.on(client2.Event.SDK_READY_FROM_CACHE, () => { t.true(Date.now() - startTime < 400, 'It should emit SDK_READY_FROM_CACHE on every client if there was data in the cache and we subscribe on time. Should be considerably faster than actual readiness from the cloud.'); t.equal(client2.getTreatment('always_on'), 'off', 'It should evaluate treatments with data from cache instead of control due to Input Validation'); }); - client3.once(client3.Event.SDK_READY_FROM_CACHE, () => { + client3.on(client3.Event.SDK_READY_FROM_CACHE, () => { t.true(Date.now() - startTime < 400, 'It should emit SDK_READY_FROM_CACHE on every client if there was data in the cache and we subscribe on time. Should be considerably faster than actual readiness from the cloud.'); t.equal(client3.getTreatment('always_on'), 'off', 'It should evaluate treatments with data from cache instead of control due to Input Validation'); }); diff --git a/src/__tests__/offline/browser.spec.js b/src/__tests__/offline/browser.spec.js index 1d3b6918f..68702dce0 100644 --- a/src/__tests__/offline/browser.spec.js +++ b/src/__tests__/offline/browser.spec.js @@ -96,30 +96,7 @@ tape('Browser offline mode', function (assert) { let readyCount = 0, updateCount = 0, readyFromCacheCount = 0; for (let i = 0; i < factories.length; i++) { - const factory = factories[i], client = factory.client(), manager = factory.manager(), sClient1 = factory.client('other'); - - client.once(client.Event.SDK_READY_FROM_CACHE, () => { - const sClient2 = factory.client('another'); - - [client, sClient1, sClient2].forEach(client => { - assert.equal(client.__context.get(client.__context.constants.READY_FROM_CACHE, true), true, 'If ready from cache, READY_FROM_CACHE status must be true'); - assert.equal(client.__context.get(client.__context.constants.READY, true), undefined, 'READY status must not be set before READY_FROM_CACHE'); - assert.equal(client.getTreatment('testing_split_with_config'), 'off', 'If ready from cache, clients should evaluate from cache'); - - client.on(client.Event.SDK_READY_FROM_CACHE, () => { - assert.fail('Shared clients don\'t emit SDK_READY_FROM_CACHE event. Only the main client does'); - }); - }); - - [sClient1, sClient2].forEach(client => { - client.on(client.Event.SDK_READY, () => { - assert.equal(client.__context.get(client.__context.constants.READY, true), true, 'READY status must be set once SDK_READY is emitted'); - readyCount++; - }); - }); - - readyFromCacheCount++; - }); + const factory = factories[i], client = factory.client(), manager = factory.manager(), client2 = factory.client('other'); client.on(client.Event.SDK_READY, () => { assert.deepEqual(manager.names(), ['testing_split', 'testing_split_with_config']); @@ -131,6 +108,28 @@ tape('Browser offline mode', function (assert) { assert.equal(client.getTreatment('testing_split_with_config'), 'nope'); updateCount++; }); + + const sdkReadyFromCache = (client) => () => { + assert.equal(client.__context.get(client.__context.constants.READY_FROM_CACHE, true), true, 'If ready from cache, READY_FROM_CACHE status must be true'); + assert.equal(client.__context.get(client.__context.constants.READY, true), undefined, 'READY status must not be set before READY_FROM_CACHE'); + + assert.deepEqual(manager.names(), ['testing_split', 'testing_split_with_config']); + assert.equal(client.getTreatment('testing_split_with_config'), 'off'); + readyFromCacheCount++; + + client.on(client.Event.SDK_READY_FROM_CACHE, () => { + assert.fail('It should not emit SDK_READY_FROM_CACHE again'); + }); + + const newClient = factory.client('another'); + assert.equal(newClient.getTreatment('testing_split_with_config'), 'off', 'It should evaluate treatments with data from cache instead of control'); + newClient.on(newClient.Event.SDK_READY_FROM_CACHE, () => { + assert.fail('It should not emit SDK_READY_FROM_CACHE if already done.'); + }); + }; + + client.on(client.Event.SDK_READY_FROM_CACHE, sdkReadyFromCache(client)); + client2.on(client2.Event.SDK_READY_FROM_CACHE, sdkReadyFromCache(client2)); } client.once(client.Event.SDK_READY, function () { @@ -332,9 +331,9 @@ tape('Browser offline mode', function (assert) { assert.equal(sharedUpdateCount, 1, 'Shared client should have emitted SDK_UPDATE event once'); // SDK events on other factory clients - assert.equal(readyCount, factories.length + 2, 'Each factory client should have emitted SDK_READY event once plus two shared clients'); + assert.equal(readyCount, factories.length, 'Each factory client should have emitted SDK_READY event once plus two shared clients'); assert.equal(updateCount, factories.length - 1, 'Each factory client except one should have emitted SDK_UPDATE event once'); - assert.equal(readyFromCacheCount, 1, 'The main client of the factory with LOCALSTORAGE should have emitted SDK_READY_FROM_CACHE event'); + assert.equal(readyFromCacheCount, 2, 'The main and shared client of the factory with LOCALSTORAGE should have emitted SDK_READY_FROM_CACHE event'); assert.end(); }); diff --git a/src/index.js b/src/index.js index 05b513813..3efa70238 100644 --- a/src/index.js +++ b/src/index.js @@ -105,12 +105,9 @@ export function SplitFactory(config) { const sharedSettings = settings.overrideKeyAndTT(validKey, validTrafficType); const sharedContext = new Context(); - const setReadyFromCache = () => sharedContext.put(context.constants.READY_FROM_CACHE, true); - if (context.get(context.constants.READY_FROM_CACHE, true)) setReadyFromCache(); - else readiness.gate.once(readiness.gate.SDK_READY_FROM_CACHE, setReadyFromCache); - - const sharedReadiness = gateFactory(sharedSettings.startup.readyTimeout); - sharedContext.put(context.constants.READINESS, sharedReadiness); + const readiness = gateFactory(sharedSettings.startup.readyTimeout); + sharedContext.put(context.constants.READY_FROM_CACHE, context.get(context.constants.READY_FROM_CACHE, true)); + sharedContext.put(context.constants.READINESS, readiness); // for shared clients, the internal offset of added/removed SDK_READY callbacks is -1 sharedContext.put(context.constants.STATUS_MANAGER, sdkStatusManager(sharedContext, -1)); sharedContext.put(context.constants.SETTINGS, sharedSettings); diff --git a/src/producer/updater/SplitChangesFromObject.js b/src/producer/updater/SplitChangesFromObject.js index 2bb549e7a..7a356b78d 100644 --- a/src/producer/updater/SplitChangesFromObject.js +++ b/src/producer/updater/SplitChangesFromObject.js @@ -66,8 +66,8 @@ function FromObjectUpdaterFactory(Fetcher, context) { if (startingUp) { startingUp = false; - // If we have cached splits after starting up, let's notify that before the sdk gets ready. - if (storage.splits.checkCache()) readiness.gate.emit(readiness.gate.SDK_READY_FROM_CACHE); + // Emits SDK_READY_FROM_CACHE + if (storage.splits.checkCache()) readiness.splits.emit(readiness.splits.SDK_SPLITS_CACHE_LOADED, true); // Only emits SDK_SEGMENTS_ARRIVED the first time for SDK_READY readiness.segments.emit(readiness.segments.SDK_SEGMENTS_ARRIVED); } diff --git a/src/readiness/index.js b/src/readiness/index.js index 773a1f41e..5e76d2290 100644 --- a/src/readiness/index.js +++ b/src/readiness/index.js @@ -53,9 +53,12 @@ function GateContext() { gate.emit(Events.READINESS_GATE_CHECK_STATE); }); - splits.once(Events.SDK_SPLITS_CACHE_LOADED, () => { - // Make it async - setTimeout(() => gate.emit(Events.SDK_READY_FROM_CACHE), 0); + splits.once(Events.SDK_SPLITS_CACHE_LOADED, (localhost) => { + if (localhost) { + gate.emit(Events.SDK_READY_FROM_CACHE); + } else { // Make async for online mode, to let attach a cb + setTimeout(() => gate.emit(Events.SDK_READY_FROM_CACHE), 0); + } }); segments.on(Events.SDK_SEGMENTS_ARRIVED, () => { From e7db8d76fd87366d242341308ccba87d44462852 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 15 Sep 2021 15:52:31 -0300 Subject: [PATCH 5/7] polishing --- src/__tests__/offline/browser.spec.js | 2 +- src/factory/offline.js | 4 ++-- src/producer/updater/SplitChanges.js | 7 +++++-- src/producer/updater/SplitChangesFromObject.js | 2 +- src/readiness/index.js | 8 ++------ 5 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/__tests__/offline/browser.spec.js b/src/__tests__/offline/browser.spec.js index 68702dce0..721d98dec 100644 --- a/src/__tests__/offline/browser.spec.js +++ b/src/__tests__/offline/browser.spec.js @@ -331,7 +331,7 @@ tape('Browser offline mode', function (assert) { assert.equal(sharedUpdateCount, 1, 'Shared client should have emitted SDK_UPDATE event once'); // SDK events on other factory clients - assert.equal(readyCount, factories.length, 'Each factory client should have emitted SDK_READY event once plus two shared clients'); + assert.equal(readyCount, factories.length, 'Each factory client should have emitted SDK_READY event once'); assert.equal(updateCount, factories.length - 1, 'Each factory client except one should have emitted SDK_UPDATE event once'); assert.equal(readyFromCacheCount, 2, 'The main and shared client of the factory with LOCALSTORAGE should have emitted SDK_READY_FROM_CACHE event'); diff --git a/src/factory/offline.js b/src/factory/offline.js index 70de58a87..36528ab00 100644 --- a/src/factory/offline.js +++ b/src/factory/offline.js @@ -12,8 +12,8 @@ function SplitFactoryOffline(context, sharedTrackers) { const storage = context.get(context.constants.STORAGE); const statusManager = context.get(context.constants.STATUS_MANAGER); - // In LOCALHOST mode, shared clients are ready in the next event-loop cycle than created, and then updated on - // each SDK_SPLITS_ARRIVED event. As in online mode, SDK_READY_FROM_CACHE is only emitted by the main client. + // In LOCALHOST mode, shared clients are ready in the next event-loop cycle than created + // and then updated on each SDK_SPLITS_ARRIVED event if (sharedInstance) setTimeout(() => { readiness.splits.on(readiness.splits.SDK_SPLITS_ARRIVED, () => { readiness.gate.emit(readiness.gate.SDK_UPDATE); diff --git a/src/producer/updater/SplitChanges.js b/src/producer/updater/SplitChanges.js index 909d3e6ab..6acc512b4 100644 --- a/src/producer/updater/SplitChanges.js +++ b/src/producer/updater/SplitChanges.js @@ -120,8 +120,11 @@ export default function SplitChangesUpdaterFactory(context, isNode = false) { return false; }); - // After triggering the requests, if we have cached splits information let's notify that. - if (startingUp && storage.splits.checkCache()) splitsEventEmitter.emit(splitsEventEmitter.SDK_SPLITS_CACHE_LOADED); + // After triggering the requests, if we have cached splits information let's notify + // that asynchronously, to let attach a listener for SDK_READY_FROM_CACHE + if (startingUp && storage.splits.checkCache()) { + setTimeout(splitsEventEmitter.emit(splitsEventEmitter.SDK_SPLITS_CACHE_LOADED), 0); + } return fetcherPromise; } diff --git a/src/producer/updater/SplitChangesFromObject.js b/src/producer/updater/SplitChangesFromObject.js index 7a356b78d..e11c6b9b4 100644 --- a/src/producer/updater/SplitChangesFromObject.js +++ b/src/producer/updater/SplitChangesFromObject.js @@ -67,7 +67,7 @@ function FromObjectUpdaterFactory(Fetcher, context) { if (startingUp) { startingUp = false; // Emits SDK_READY_FROM_CACHE - if (storage.splits.checkCache()) readiness.splits.emit(readiness.splits.SDK_SPLITS_CACHE_LOADED, true); + if (storage.splits.checkCache()) readiness.splits.emit(readiness.splits.SDK_SPLITS_CACHE_LOADED); // Only emits SDK_SEGMENTS_ARRIVED the first time for SDK_READY readiness.segments.emit(readiness.segments.SDK_SEGMENTS_ARRIVED); } diff --git a/src/readiness/index.js b/src/readiness/index.js index 5e76d2290..c31c8ed57 100644 --- a/src/readiness/index.js +++ b/src/readiness/index.js @@ -53,12 +53,8 @@ function GateContext() { gate.emit(Events.READINESS_GATE_CHECK_STATE); }); - splits.once(Events.SDK_SPLITS_CACHE_LOADED, (localhost) => { - if (localhost) { - gate.emit(Events.SDK_READY_FROM_CACHE); - } else { // Make async for online mode, to let attach a cb - setTimeout(() => gate.emit(Events.SDK_READY_FROM_CACHE), 0); - } + splits.once(Events.SDK_SPLITS_CACHE_LOADED, () => { + gate.emit(Events.SDK_READY_FROM_CACHE); }); segments.on(Events.SDK_SEGMENTS_ARRIVED, () => { From 292b1ca73ba38f5428f32815ebe69dfa5a1b60b8 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 15 Sep 2021 16:22:11 -0300 Subject: [PATCH 6/7] updated log level --- src/metrics/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/metrics/index.js b/src/metrics/index.js index 2147c37ba..908b881ba 100644 --- a/src/metrics/index.js +++ b/src/metrics/index.js @@ -49,7 +49,7 @@ const MetricsFactory = context => { const pushMetrics = () => { if (storage.metrics.isEmpty() && storage.count.isEmpty()) return Promise.resolve(); - log.info('Pushing metrics'); + log.debug('Pushing metrics'); const latencyTrackerStop = tracker.start(tracker.TaskNames.METRICS_PUSH); // POST latencies From 9fe8a09d0f601bf1ce74284b1db6e5cf6989f7a7 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 16 Sep 2021 15:09:49 -0300 Subject: [PATCH 7/7] polishing --- package-lock.json | 2 +- package.json | 2 +- .../hasher/__tests__/buildKey.spec.js | 17 +++++++++++++++++ src/impressions/hasher/hashImpression32.js | 2 +- src/sync/PushManager/index.js | 4 +--- src/sync/SSEHandler/index.js | 1 - src/utils/settings/index.js | 2 +- 7 files changed, 22 insertions(+), 8 deletions(-) create mode 100644 src/impressions/hasher/__tests__/buildKey.spec.js diff --git a/package-lock.json b/package-lock.json index ea3c1e993..45142afd5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio", - "version": "10.15.10-rc.0", + "version": "10.15.10-rc.1", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index fb775ca82..67a59e76a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio", - "version": "10.15.10-rc.0", + "version": "10.15.10-rc.1", "description": "Split SDK", "files": [ "README.md", diff --git a/src/impressions/hasher/__tests__/buildKey.spec.js b/src/impressions/hasher/__tests__/buildKey.spec.js new file mode 100644 index 000000000..8c93e8439 --- /dev/null +++ b/src/impressions/hasher/__tests__/buildKey.spec.js @@ -0,0 +1,17 @@ +import tape from 'tape-catch'; +import { buildKey } from '../buildKey'; + +tape('buildKey', assert => { + assert.equal(buildKey({}), 'undefined:undefined:undefined:undefined:undefined'); + + const imp = { + feature: 'feature_0', + keyName: 'key_0', + changeNumber: 0, + label: 'in segment all', + treatment: 'someTreatment', + }; + assert.equal(buildKey(imp), 'key_0:feature_0:someTreatment:in segment all:0'); + + assert.end(); +}); diff --git a/src/impressions/hasher/hashImpression32.js b/src/impressions/hasher/hashImpression32.js index a424bef1d..7b0833135 100644 --- a/src/impressions/hasher/hashImpression32.js +++ b/src/impressions/hasher/hashImpression32.js @@ -2,5 +2,5 @@ import murmur from '../../engine/engine/murmur3/murmur3'; import { buildKey } from './buildKey'; export function hashImpression32(impression) { - return murmur.hash(buildKey(impression)).toString(); + return murmur.hash(buildKey(impression)); } diff --git a/src/sync/PushManager/index.js b/src/sync/PushManager/index.js index 9e58fb608..e7e9ed66e 100644 --- a/src/sync/PushManager/index.js +++ b/src/sync/PushManager/index.js @@ -206,7 +206,6 @@ export default function PushManagerFactory(context, clientContexts /* undefined pushEmitter.on(SPLIT_UPDATE, splitUpdateWorker.put); if (clientContexts) { // browser - // @TODO remove pushEmitter.on(MY_SEGMENTS_UPDATE, function handleMySegmentsUpdate(parsedData, channel) { const userKeyHash = channel.split('_')[2]; const userKey = userKeyHashes[userKeyHash]; @@ -231,8 +230,7 @@ export default function PushManagerFactory(context, clientContexts /* undefined forOwn(clients, ({ hash64, worker }) => { if (isInBitmap(bitmap, hash64.hex)) { - // fetch mySegments - worker.put(parsedData.changeNumber); + worker.put(parsedData.changeNumber); // fetch mySegments } }); return; diff --git a/src/sync/SSEHandler/index.js b/src/sync/SSEHandler/index.js index 993e5916c..fededd0cd 100644 --- a/src/sync/SSEHandler/index.js +++ b/src/sync/SSEHandler/index.js @@ -77,7 +77,6 @@ export default function SSEHandlerFactory(pushEmitter) { parsedData.changeNumber, parsedData.segmentName); break; - // @TODO remove case MY_SEGMENTS_UPDATE: pushEmitter.emit(MY_SEGMENTS_UPDATE, parsedData, diff --git a/src/utils/settings/index.js b/src/utils/settings/index.js index bac1c8aed..b8828c052 100644 --- a/src/utils/settings/index.js +++ b/src/utils/settings/index.js @@ -27,7 +27,7 @@ import { API } from '../../utils/logger'; import { STANDALONE_MODE, STORAGE_MEMORY, CONSUMER_MODE, OPTIMIZED } from '../../utils/constants'; import validImpressionsMode from './impressionsMode'; -const version = '10.15.10-rc.0'; +const version = '10.15.10-rc.1'; const eventsEndpointMatcher = /^\/(testImpressions|metrics|events)/; const authEndpointMatcher = /^\/v2\/auth/; const streamingEndpointMatcher = /^\/(sse|event-stream)/;