From 68bdba2b83ac4d3fcce16e6c6ed39dc7714d6e9e Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Fri, 26 Apr 2024 13:49:59 -0400 Subject: [PATCH 1/2] refactor: Remove usages of Persent First/Last Seen Times --- src/identity.js | 12 +-- src/persistence.interfaces.ts | 4 - src/persistence.js | 72 ------------- src/store.ts | 12 ++- test/src/tests-identity.js | 42 ++++++++ test/src/tests-persistence.ts | 192 ---------------------------------- test/src/tests-store.ts | 95 ++++++++++++++--- 7 files changed, 140 insertions(+), 289 deletions(-) diff --git a/src/identity.js b/src/identity.js index dc008650f..46acf9ecb 100644 --- a/src/identity.js +++ b/src/identity.js @@ -1238,10 +1238,10 @@ export default function Identity(mpInstance) { return isLoggedIn; }, getLastSeenTime: function() { - return mpInstance._Persistence.getLastSeenTime(mpid); + return mpInstance._Store.getLastSeenTime(mpid); }, getFirstSeenTime: function() { - return mpInstance._Persistence.getFirstSeenTime(mpid); + return mpInstance._Store.getFirstSeenTime(mpid); }, /** * Get user audiences @@ -1494,15 +1494,15 @@ export default function Identity(mpInstance) { if (prevUser) { // https://go.mparticle.com/work/SQDSDKS-6329 - mpInstance._Persistence.setLastSeenTime(previousMPID); + mpInstance._Store.setLastSeenTime(previousMPID); } - const mpidIsNotInCookies = !mpInstance._Persistence.getFirstSeenTime( + const mpidIsNotInCookies = !mpInstance._Store.getFirstSeenTime( identityApiResult.mpid ); // https://go.mparticle.com/work/SQDSDKS-6329 - mpInstance._Persistence.setFirstSeenTime(identityApiResult.mpid); + mpInstance._Store.setFirstSeenTime(identityApiResult.mpid); if (xhr.status === 200) { if (getFeatureFlag(CacheIdentity)) { @@ -1543,7 +1543,7 @@ export default function Identity(mpInstance) { prevUser && identityApiResult.mpid === prevUserMPID ) { - mpInstance._Persistence.setFirstSeenTime( + mpInstance._Store.setFirstSeenTime( identityApiResult.mpid ); } diff --git a/src/persistence.interfaces.ts b/src/persistence.interfaces.ts index 520d46707..24ddeeb28 100644 --- a/src/persistence.interfaces.ts +++ b/src/persistence.interfaces.ts @@ -133,10 +133,6 @@ export interface IPersistence { savePersistence(persistance: IPersistenceMinified): void; getPersistence(): IPersistenceMinified; getConsentState(mpid: MPID): ConsentState | null; - getFirstSeenTime(mpid: MPID): string | null; - setFirstSeenTime(mpid: MPID, time: number): void; - getLastSeenTime(mpid: MPID): number | null; - setLastSeenTime(mpid: MPID, time: number): void; getDeviceId(): string; setDeviceId(guid: string): void; resetPersistence(): void; diff --git a/src/persistence.js b/src/persistence.js index fce96621f..86e8d1185 100644 --- a/src/persistence.js +++ b/src/persistence.js @@ -981,78 +981,6 @@ export default function _Persistence(mpInstance) { } }; - this.getFirstSeenTime = function(mpid) { - if (!mpid) { - return null; - } - var persistence = self.getPersistence(); - if (persistence && persistence[mpid] && persistence[mpid].fst) { - return persistence[mpid].fst; - } else { - return null; - } - }; - - /** - * set the "first seen" time for a user. the time will only be set once for a given - * mpid after which subsequent calls will be ignored - */ - this.setFirstSeenTime = function(mpid, time) { - if (!mpid) { - return; - } - // https://go.mparticle.com/work/SQDSDKS-6329 - if (!time) { - time = new Date().getTime(); - } - var persistence = self.getPersistence(); - if (persistence) { - if (!persistence[mpid]) { - persistence[mpid] = {}; - } - if (!persistence[mpid].fst) { - persistence[mpid].fst = time; - self.savePersistence(persistence); - } - } - }; - - /** - * returns the "last seen" time for a user. If the mpid represents the current user, the - * return value will always be the current time, otherwise it will be to stored "last seen" - * time - */ - this.getLastSeenTime = function(mpid) { - if (!mpid) { - return null; - } - if (mpid === mpInstance.Identity.getCurrentUser().getMPID()) { - //if the mpid is the current user, its last seen time is the current time - return new Date().getTime(); - } else { - var persistence = self.getPersistence(); - if (persistence && persistence[mpid] && persistence[mpid].lst) { - return persistence[mpid].lst; - } - return null; - } - }; - - this.setLastSeenTime = function(mpid, time) { - if (!mpid) { - return; - } - // https://go.mparticle.com/work/SQDSDKS-6329 - if (!time) { - time = new Date().getTime(); - } - var persistence = self.getPersistence(); - if (persistence && persistence[mpid]) { - persistence[mpid].lst = time; - self.savePersistence(persistence); - } - }; - this.getDeviceId = function() { return mpInstance._Store.deviceId; }; diff --git a/src/store.ts b/src/store.ts index d909a0a59..5d3d70677 100644 --- a/src/store.ts +++ b/src/store.ts @@ -604,6 +604,8 @@ export default function Store( this.getFirstSeenTime = (mpid: MPID) => this._getFromPersistence(mpid, 'fst'); + // set the "first seen" time for a user. the time will only be set once for a given + // mpid after which subsequent calls will be ignored this.setFirstSeenTime = (mpid: MPID, _time?: number) => { if (!mpid) { return; @@ -611,7 +613,15 @@ export default function Store( const time = _time || new Date().getTime(); - this._setPersistence(mpid, 'fst', time); + this.syncPersistenceData(); + + if(!this.persistenceData[mpid]) { + this.persistenceData[mpid] = {}; + } + if(!this.persistenceData[mpid].fst) { + this.persistenceData[mpid].fst = time; + this._setPersistence(mpid, 'fst', time); + } }; this.getLastSeenTime = (mpid: MPID): number => { diff --git a/test/src/tests-identity.js b/test/src/tests-identity.js index fd70be8d0..155cc623e 100644 --- a/test/src/tests-identity.js +++ b/test/src/tests-identity.js @@ -1,11 +1,13 @@ import Constants from '../../src/constants'; import Utils from './config/utils'; import sinon from 'sinon'; +import { expect } from 'chai'; import fetchMock from 'fetch-mock/esm/client'; import { urls, apiKey, testMPID, MPConfig, workspaceCookieName, + mParticle, } from './config/constants'; const getLocalStorage = Utils.getLocalStorage, @@ -3084,6 +3086,46 @@ describe('identity', function() { done(); }); + describe('#identify', function() { + it("should set fst when the user does not change, after an identify request", function(done) { + mParticle._resetForTests(MPConfig); + + const cookies = JSON.stringify({ + gs: { + sid: 'fst Test', + les: new Date().getTime(), + }, + current: {}, + cu: 'current', + }); + + mockServer.respondWith(urls.identify, [ + 200, + {}, + JSON.stringify({ mpid: 'current', is_logged_in: false }), + ]); + + // We set the cookies because the init process will load cookies into + // our in-memory store, which we will check before and after the identify request + setCookie(workspaceCookieName, cookies, true); + mParticle.config.useCookieStorage = true; + + mParticle.init(apiKey, mParticle.config); + + expect( + mParticle.getInstance()._Store.getFirstSeenTime('current') + ).to.equal(null); + + mParticle.Identity.identify(); + + expect( + mParticle.getInstance()._Store.getFirstSeenTime('current') + ).to.not.equal(null); + + done(); + }); + }); + describe('identity caching', function() { afterEach(function() { sinon.restore(); diff --git a/test/src/tests-persistence.ts b/test/src/tests-persistence.ts index 996a56c17..2c55a4214 100644 --- a/test/src/tests-persistence.ts +++ b/test/src/tests-persistence.ts @@ -1794,198 +1794,6 @@ describe('persistence', () => { done(); }); - it('should only set setFirstSeenTime() once', done => { - mParticle._resetForTests(MPConfig); - - const cookies = JSON.stringify({ - gs: { - sid: 'fst Test', - les: new Date().getTime(), - }, - previous: {}, - previous_set: { - fst: 100, - }, - cu: 'current', - }); - - setCookie(workspaceCookieName, cookies); - - // TODO: Refactor this into setup/teardown - mParticle.config.useCookieStorage = true; - - mParticle.init(apiKey, mParticle.config); - - mParticle.getInstance()._Persistence.setFirstSeenTime('current', 10000); - const currentFirstSeenTime = mParticle - .getInstance() - ._Persistence.getFirstSeenTime('current'); - mParticle.getInstance()._Persistence.setFirstSeenTime('current', 2); - mParticle - .getInstance() - ._Persistence.getFirstSeenTime('current') - .should.equal(currentFirstSeenTime); - - mParticle.getInstance()._Persistence.setFirstSeenTime('previous', 10); - mParticle - .getInstance() - ._Persistence.getFirstSeenTime('previous') - .should.equal(10); - mParticle.getInstance()._Persistence.setFirstSeenTime('previous', 20); - mParticle - .getInstance() - ._Persistence.getFirstSeenTime('previous') - .should.equal(10); - - mParticle - .getInstance() - ._Persistence.getFirstSeenTime('previous_set') - .should.equal(100); - mParticle - .getInstance() - ._Persistence.setFirstSeenTime('previous_set', 200); - mParticle - .getInstance() - ._Persistence.getFirstSeenTime('previous_set') - .should.equal(100); - done(); - }); - - it('should properly set setLastSeenTime()', done => { - mParticle._resetForTests(MPConfig); - - const cookies = JSON.stringify({ - gs: { - sid: 'lst Test', - les: new Date().getTime(), - }, - previous: {}, - previous_set: { - lst: 10, - }, - cu: 'current', - }); - - setCookie(workspaceCookieName, cookies, true); - - mParticle.init(apiKey, mParticle.config); - - const clock = sinon.useFakeTimers(); - clock.tick(100); - - const persistence: IPersistence = mParticle.getInstance()._Persistence; - - persistence.setLastSeenTime('previous', 1); - expect(persistence.getLastSeenTime('previous')).to.equal(1); - - persistence.setLastSeenTime('previous', 2); - expect(persistence.getLastSeenTime('previous')).to.equal(2); - - expect(persistence.getLastSeenTime('previous_set')).to.equal(10); - persistence.setLastSeenTime('previous_set', 20); - expect(persistence.getLastSeenTime('previous_set')).to.equal(20); - - expect(persistence.getLastSeenTime('current')).to.equal(100); - persistence.setLastSeenTime('current', 200); - //lastSeenTime for the current user should always reflect the current time, - //even if was set - expect(persistence.getLastSeenTime('current')).to.equal(100); - clock.tick(50); - expect(persistence.getLastSeenTime('current')).to.equal(150); - - clock.restore(); - done(); - }); - - it("should set firstSeenTime() for a user that doesn't have storage yet", done => { - mParticle._resetForTests(MPConfig); - - const cookies = JSON.stringify({ - gs: { - sid: 'fst Test', - les: new Date().getTime(), - }, - cu: 'test', - }); - - setCookie(workspaceCookieName, cookies, true); - // FIXME: Should this be in configs or global? - mParticle.config.useCookieStorage = true; - - mParticle.init(apiKey, mParticle.config); - - mParticle.getInstance()._Persistence.setFirstSeenTime('1', 1); - mParticle - .getInstance() - ._Persistence.getFirstSeenTime('1') - .should.equal(1); - //firstSeenTime should ignore subsiquent calls after it has been set - mParticle.getInstance()._Persistence.setFirstSeenTime('2', 2); - mParticle - .getInstance() - ._Persistence.getFirstSeenTime('1') - .should.equal(1); - - done(); - }); - - it('fst should be set when the user does not change, after an identify request', done => { - mParticle._resetForTests(MPConfig); - - const cookies = JSON.stringify({ - gs: { - sid: 'fst Test', - les: new Date().getTime(), - }, - current: {}, - cu: 'current', - }); - - mockServer.respondWith(urls.identify, [ - 200, - {}, - JSON.stringify({ mpid: 'current', is_logged_in: false }), - ]); - - setCookie(workspaceCookieName, cookies, true); - // FIXME: Should this be in configs or global? - mParticle.config.useCookieStorage = true; - - mParticle.init(apiKey, mParticle.config); - expect( - mParticle.getInstance()._Persistence.getFirstSeenTime('current') - ).to.equal(null); - - mParticle.Identity.identify(); - - expect( - mParticle.getInstance()._Persistence.getFirstSeenTime('current') - ).to.not.equal(null); - - done(); - }); - - it('lastSeenTime should be null for users in storage without an lst value', done => { - const cookies = JSON.stringify({ - gs: { - sid: 'lst Test', - les: new Date().getTime(), - }, - previous: {}, - cu: 'current', - }); - setCookie(workspaceCookieName, cookies, true); - // FIXME: config or global? - mParticle.config.useCookieStorage = true; - - mParticle.init(apiKey, mParticle.config); - expect( - mParticle.getInstance()._Persistence.getFirstSeenTime('previous') - ).to.equal(null); - - done(); - }); - it('should save to persistence a device id set with setDeviceId', done => { mParticle._resetForTests(MPConfig); diff --git a/test/src/tests-store.ts b/test/src/tests-store.ts index 1320341e0..9817b0d15 100644 --- a/test/src/tests-store.ts +++ b/test/src/tests-store.ts @@ -20,8 +20,9 @@ import { IGlobalStoreV2MinifiedKeys } from '../../src/persistence.interfaces'; import { IMinifiedConsentJSONObject } from '../../src/consent'; const MockSideloadedKit = Utils.MockSideloadedKit; +const { setCookie } = Utils; + describe('Store', () => { - const now = new Date(); let sandbox; let clock; @@ -105,7 +106,7 @@ describe('Store', () => { beforeEach(function() { sandbox = sinon.createSandbox(); - clock = sinon.useFakeTimers(now.getTime()); + clock = sinon.useFakeTimers(); // MP Instance is just used because Store requires an mParticle instance window.mParticle.init(apiKey, window.mParticle.config); }); @@ -997,15 +998,6 @@ describe('Store', () => { expect(store.persistenceData[testMPID].fst).to.equal(12345); }); - it('should return undefined if mpid is null', () => { - const store: IStore = new Store( - sampleConfig, - window.mParticle.getInstance() - ); - - expect(store.setFirstSeenTime(null, 12345)).to.equal(undefined); - }); - it('should set the firstSeenTime in persistence', () => { const store: IStore = new Store( sampleConfig, @@ -1022,7 +1014,7 @@ describe('Store', () => { expect(fromPersistence[testMPID].fst).to.equal(12345); }); - it('should override persistence with store values', () => { + it('should override store with persistence values', () => { const persistenceValue = JSON.stringify({ testMPID: { fst: 12345, @@ -1037,13 +1029,39 @@ describe('Store', () => { ); store.setFirstSeenTime(testMPID, 54321); + + expect(store.getFirstSeenTime(testMPID)).to.equal(12345); + const fromPersistence = window.mParticle .getInstance() ._Persistence.getPersistence(); expect(fromPersistence[testMPID]).to.be.ok; expect(fromPersistence[testMPID].fst).to.be.ok; - expect(fromPersistence[testMPID].fst).to.equal(54321); + expect(fromPersistence[testMPID].fst).to.equal(12345); + }); + + it('should set firstSeenTime once', () => { + const store: IStore = new Store( + sampleConfig, + window.mParticle.getInstance() + ); + + store.persistenceData['previous-set-mpid'] = { + fst: 100 + } + + store.setFirstSeenTime('current-mpid', 10000); + store.setFirstSeenTime('current-mpid', 2); + expect(store.getFirstSeenTime('current-mpid')).to.equal(10000); + + store.setFirstSeenTime('previous-mpid', 10); + store.setFirstSeenTime('previous-mpid', 20); + expect(store.getFirstSeenTime('previous-mpid')).to.equal(10); + + store.setFirstSeenTime('previous-set-mpid', 200); + expect(store.getFirstSeenTime('previous-set-mpid')).to.equal(100); + }); }); @@ -1093,7 +1111,8 @@ describe('Store', () => { window.mParticle.getInstance() ); - expect(store.getLastSeenTime(testMPID)).to.equal(now.getTime()); + clock.tick(100) + expect(store.getLastSeenTime(testMPID)).to.equal(100); }); }); @@ -1147,6 +1166,29 @@ describe('Store', () => { expect(fromPersistence[testMPID].lst).to.be.ok; expect(fromPersistence[testMPID].lst).to.equal(54321); }); + + it('returns current time for the current user', () => { + const userSpy = sinon.stub( + window.mParticle.getInstance().Identity, + 'getCurrentUser' + ); + userSpy.returns({ + getMPID: () => 'testMPID', + }); + + const store: IStore = new Store( + sampleConfig, + window.mParticle.getInstance() + ); + + // Simulates current time + clock.tick(100); + store.setLastSeenTime(testMPID, 12345); + expect(store.getLastSeenTime(testMPID)).to.equal(100); + + clock.tick(50); + expect(store.getLastSeenTime(testMPID)).to.equal(150); + }); }); describe('#getUserIdentities', () => { @@ -1814,4 +1856,29 @@ describe('Store', () => { }); }); }); + + describe('integration tests', () => { + describe('#firstSeenTime', () => { + it('should be null for users in storage without an lst value', done => { + const cookies = JSON.stringify({ + gs: { + sid: 'lst Test', + les: new Date().getTime(), + }, + previous: {}, + cu: 'current', + }); + setCookie(workspaceCookieName, cookies, true); + window.mParticle.config.useCookieStorage = true; + + window.mParticle.init(apiKey, window.mParticle.config); + expect( + window.mParticle.getInstance()._Store.getFirstSeenTime('previous') + ).to.equal(null); + + done(); + }); + + }); + }); }); From 6661f929ca7c4ee509acc6c25dd2e55e6e55e0b9 Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Fri, 26 Apr 2024 15:35:35 -0400 Subject: [PATCH 2/2] Address PR Comments --- test/src/tests-identity.js | 8 +++++++- test/src/tests-store.ts | 14 +++++++------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/test/src/tests-identity.js b/test/src/tests-identity.js index 155cc623e..0d5ac7bcf 100644 --- a/test/src/tests-identity.js +++ b/test/src/tests-identity.js @@ -3110,7 +3110,13 @@ describe('identity', function() { setCookie(workspaceCookieName, cookies, true); mParticle.config.useCookieStorage = true; - mParticle.init(apiKey, mParticle.config); + + // As part of init, there is a call to Identity.Identify. However, in this test case, + // we are starting with the assumption that there is a current user, and that user has + // both a sessionId (sid) and lastEventSent (les), which causes that initial identify call + // to be skipped. By manually forcing an identify call below, we can test that + // identify correctly calls setFirstSeenTime to set a value; + mParticle.init(apiKey, mParticle.config); expect( mParticle.getInstance()._Store.getFirstSeenTime('current') diff --git a/test/src/tests-store.ts b/test/src/tests-store.ts index 9817b0d15..155d5abf6 100644 --- a/test/src/tests-store.ts +++ b/test/src/tests-store.ts @@ -1051,6 +1051,9 @@ describe('Store', () => { fst: 100 } + store.setFirstSeenTime('previous-set-mpid', 200); + expect(store.getFirstSeenTime('previous-set-mpid')).to.equal(100); + store.setFirstSeenTime('current-mpid', 10000); store.setFirstSeenTime('current-mpid', 2); expect(store.getFirstSeenTime('current-mpid')).to.equal(10000); @@ -1059,8 +1062,6 @@ describe('Store', () => { store.setFirstSeenTime('previous-mpid', 20); expect(store.getFirstSeenTime('previous-mpid')).to.equal(10); - store.setFirstSeenTime('previous-set-mpid', 200); - expect(store.getFirstSeenTime('previous-set-mpid')).to.equal(100); }); }); @@ -1167,7 +1168,7 @@ describe('Store', () => { expect(fromPersistence[testMPID].lst).to.equal(54321); }); - it('returns current time for the current user', () => { + it('returns current time for the current user as the lastSeenTime', () => { const userSpy = sinon.stub( window.mParticle.getInstance().Identity, 'getCurrentUser' @@ -1865,20 +1866,19 @@ describe('Store', () => { sid: 'lst Test', les: new Date().getTime(), }, - previous: {}, - cu: 'current', + previousMPID: {}, + cu: 'current-mpid', }); setCookie(workspaceCookieName, cookies, true); window.mParticle.config.useCookieStorage = true; window.mParticle.init(apiKey, window.mParticle.config); expect( - window.mParticle.getInstance()._Store.getFirstSeenTime('previous') + window.mParticle.getInstance()._Store.getFirstSeenTime('previousMPID') ).to.equal(null); done(); }); - }); }); });