From 9bcac185594089f104ae18dd622122a3442d55f5 Mon Sep 17 00:00:00 2001 From: Robert Ing Date: Mon, 6 Jan 2025 12:09:33 -0500 Subject: [PATCH] refactor: Simplify cookie sync manager --- src/cookieSyncManager.ts | 139 ++++++++----------- src/identity.js | 6 +- src/utils.ts | 11 ++ test/jest/cookieSyncManager.spec.ts | 207 +++++++--------------------- test/jest/utils.spec.ts | 18 +++ test/src/tests-cookie-syncing.ts | 16 +-- 6 files changed, 144 insertions(+), 253 deletions(-) diff --git a/src/cookieSyncManager.ts b/src/cookieSyncManager.ts index 9822a577..2be4c973 100644 --- a/src/cookieSyncManager.ts +++ b/src/cookieSyncManager.ts @@ -1,4 +1,9 @@ -import { Dictionary, isEmpty, replaceAmpWithAmpersand, replaceMPID } from './utils'; +import { + Dictionary, + isEmpty, + replaceAmpWithAmpersand, + combineUrlWithRedirect, +} from './utils'; import Constants from './constants'; import { MParticleWebSDK } from './sdkRuntimeModels'; import { MPID } from '@mparticle/web-sdk'; @@ -25,7 +30,6 @@ export interface IPixelConfiguration { } export interface ICookieSyncManager { attemptCookieSync: ( - previousMPID: MPID, mpid: MPID, mpidIsNotInCookies?: boolean ) => void; @@ -34,7 +38,6 @@ export interface ICookieSyncManager { moduleId: string, mpid: MPID, cookieSyncDates: CookieSyncDates, - filteringConsentRuleValues: IConsentRules, mpidIsNotInCookies: boolean, requiresConsent: boolean ) => void; @@ -47,17 +50,19 @@ export interface ICookieSyncManager { const hasFrequencyCapExpired = ( frequencyCap: number, - lastSyncDate?: number, + lastSyncDate?: number ): boolean => { + // If there is no lastSyncDate, then there is no previous cookie sync, so we should sync the cookie if (!lastSyncDate) { return true; } + // Otherwise, compare the last sync date to determine if it should do a cookie sync again return ( new Date().getTime() > new Date(lastSyncDate).getTime() + frequencyCap * DAYS_IN_MILLISECONDS ); -} +}; export default function CookieSyncManager( this: ICookieSyncManager, @@ -67,7 +72,6 @@ export default function CookieSyncManager( // Public this.attemptCookieSync = ( - previousMPID: MPID, mpid: MPID, mpidIsNotInCookies?: boolean ): void => { @@ -75,9 +79,13 @@ export default function CookieSyncManager( return; } - const { pixelConfigurations } = mpInstance._Store; const persistence = mpInstance._Persistence.getPersistence(); + if (isEmpty(persistence)) { + return; + } + + const { pixelConfigurations } = mpInstance._Store; pixelConfigurations.forEach((pixelSettings: IPixelConfiguration) => { // set requiresConsent to false to start each additional pixel configuration // set to true only if filteringConsenRuleValues.values.length exists @@ -91,6 +99,17 @@ export default function CookieSyncManager( requiresConsent = true; } + // if MPID is new to cookies, we should not try to perform the cookie sync + // because a cookie sync can only occur once a user either consents or doesn't + // we should not check if its enabled if the user has a blank consent + if (requiresConsent && mpidIsNotInCookies) { + return; + } + + if (isEmpty(pixelSettings.pixelUrl) && isEmpty(pixelSettings.redirectUrl)) { + return; + } + // Kit Module ID const moduleId = pixelSettings.moduleId.toString(); @@ -104,63 +123,39 @@ export default function CookieSyncManager( ? replaceAmpWithAmpersand(pixelSettings.redirectUrl) : null; - const urlWithRedirect = this.combineUrlWithRedirect( + const urlWithRedirect = combineUrlWithRedirect( mpid, pixelUrl, redirectUrl ); - if (previousMPID && previousMPID !== mpid) { - if (persistence && persistence[mpid]) { - if (!persistence[mpid].csd) { - persistence[mpid].csd = {}; - } - self.performCookieSync( - urlWithRedirect, - moduleId, - mpid, - persistence[mpid].csd, - filteringConsentRuleValues, - mpidIsNotInCookies, - requiresConsent - ); - } - return; - } else { - if (!persistence || !persistence[mpid]) { - return; - } - + // set up csd object if it doesn't exist + if (persistence && persistence[mpid]) { if (!persistence[mpid].csd) { persistence[mpid].csd = {}; } + } - const lastSyncDateForModule = persistence[mpid].csd[moduleId] || null; - - // Check to see if we need to refresh cookieSync - if (hasFrequencyCapExpired(frequencyCap, lastSyncDateForModule)) { - self.performCookieSync( - urlWithRedirect, - moduleId, - mpid, - persistence[mpid].csd, - filteringConsentRuleValues, - mpidIsNotInCookies, - requiresConsent - ); - } + const lastSyncDateForModule = persistence[mpid].csd[moduleId] || null; + + const { isEnabledForUserConsent } = mpInstance._Consent; + if (!isEnabledForUserConsent(filteringConsentRuleValues, mpInstance.Identity.getCurrentUser())) { + return; } - }); - }; - this.combineUrlWithRedirect = ( - mpid: MPID, - pixelUrl: string, - redirectUrl: string, - ): string => { - const url = replaceMPID(pixelUrl, mpid); - const redirect = redirectUrl ? replaceMPID(redirectUrl, mpid) : ''; - return url + encodeURIComponent(redirect); + if (!hasFrequencyCapExpired(frequencyCap, lastSyncDateForModule)) { + return; + } + + self.performCookieSync( + urlWithRedirect, + moduleId, + mpid, + persistence[mpid].csd, + mpidIsNotInCookies, + requiresConsent + ); + }); }; // Private @@ -169,35 +164,17 @@ export default function CookieSyncManager( moduleId: string, mpid: MPID, cookieSyncDates: CookieSyncDates, - filteringConsentRuleValues: IConsentRules, - mpidIsNotInCookies: boolean, - requiresConsent: boolean ): void => { - // if MPID is new to cookies, we should not try to perform the cookie sync - // because a cookie sync can only occur once a user either consents or doesn't - // we should not check if its enabled if the user has a blank consent - if (requiresConsent && mpidIsNotInCookies) { - return; - } + const img = document.createElement('img'); - if ( - // https://go.mparticle.com/work/SQDSDKS-5009 - mpInstance._Consent.isEnabledForUserConsent( - filteringConsentRuleValues, - mpInstance.Identity.getCurrentUser() - ) - ) { - const img = document.createElement('img'); - - mpInstance.Logger.verbose(InformationMessages.CookieSync); - img.onload = function() { - cookieSyncDates[moduleId] = new Date().getTime(); - mpInstance._Persistence.saveUserCookieSyncDatesToPersistence( - mpid, - cookieSyncDates - ); - }; - img.src = url; - } + mpInstance.Logger.verbose(InformationMessages.CookieSync); + img.onload = function() { + cookieSyncDates[moduleId] = new Date().getTime(); + mpInstance._Persistence.saveUserCookieSyncDatesToPersistence( + mpid, + cookieSyncDates + ); + }; + img.src = url; }; } diff --git a/src/identity.js b/src/identity.js index a551c7cb..686e574d 100644 --- a/src/identity.js +++ b/src/identity.js @@ -1230,10 +1230,7 @@ export default function Identity(mpInstance) { this.getUserIdentities().userIdentities, mpInstance._APIClient.prepareForwardingStats ); - mpInstance._CookieSyncManager.attemptCookieSync( - null, - this.getMPID() - ); + mpInstance._CookieSyncManager.attemptCookieSync(this.getMPID()); }, isLoggedIn: function() { return isLoggedIn; @@ -1562,7 +1559,6 @@ export default function Identity(mpInstance) { ); mpInstance._CookieSyncManager.attemptCookieSync( - previousMPID, identityApiResult.mpid, mpidIsNotInCookies ); diff --git a/src/utils.ts b/src/utils.ts index 389179d0..7dfba459 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -180,6 +180,16 @@ const replaceMPID = (value: string, mpid: MPID): string => value.replace('%%mpid const replaceAmpWithAmpersand = (value: string): string => value.replace(/&/g, '&'); +const combineUrlWithRedirect = ( + mpid: MPID, + pixelUrl: string, + redirectUrl: string, +): string => { + const url = replaceMPID(pixelUrl, mpid); + const redirect = redirectUrl ? replaceMPID(redirectUrl, mpid) : ''; + return url + encodeURIComponent(redirect); +}; + // FIXME: REFACTOR for V3 // only used in store.js to sanitize server-side formatting of // booleans when checking for `isDevelopmentMode` @@ -373,4 +383,5 @@ export { getHref, replaceMPID, replaceAmpWithAmpersand, + combineUrlWithRedirect, }; diff --git a/test/jest/cookieSyncManager.spec.ts b/test/jest/cookieSyncManager.spec.ts index 01467a92..f7053d8d 100644 --- a/test/jest/cookieSyncManager.spec.ts +++ b/test/jest/cookieSyncManager.spec.ts @@ -13,10 +13,12 @@ const pixelSettings: IPixelConfiguration = { isProduction: true, settings: {}, frequencyCap: 14, - pixelUrl: '', - redirectUrl: '', + pixelUrl: 'https://test.com', + redirectUrl: '?redirect=https://redirect.com&mpid=%%mpid%%', }; +const pixelUrlAndReidrectUrl = 'https://test.com%3Fredirect%3Dhttps%3A%2F%2Fredirect.com%26mpid%3DtestMPID'; + describe('CookieSyncManager', () => { describe('#attemptCookieSync', () => { // https://go.mparticle.com/work/SQDSDKS-6915 @@ -36,14 +38,13 @@ describe('CookieSyncManager', () => { const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); - cookieSyncManager.attemptCookieSync(null, testMPID, true); + cookieSyncManager.attemptCookieSync(testMPID, true); expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( - '', + pixelUrlAndReidrectUrl, '5', testMPID, {}, - undefined, true, false, ); @@ -65,7 +66,7 @@ describe('CookieSyncManager', () => { const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); - cookieSyncManager.attemptCookieSync(null, null, true); + cookieSyncManager.attemptCookieSync(null, true); expect(cookieSyncManager.performCookieSync).not.toHaveBeenCalled(); }); @@ -86,13 +87,15 @@ describe('CookieSyncManager', () => { const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); - cookieSyncManager.attemptCookieSync(null, testMPID, true); + cookieSyncManager.attemptCookieSync(testMPID, true); expect(cookieSyncManager.performCookieSync).not.toHaveBeenCalled(); }); it('should toggle requiresConsent value if filtering filteringConsentRuleValues are defined', () => { const myPixelSettings: IPixelConfiguration = { + pixelUrl: 'https://test.com', + redirectUrl: '?redirect=https://redirect.com&mpid=%%mpid%%', filteringConsentRuleValues: { values: ['test'], }, @@ -113,14 +116,13 @@ describe('CookieSyncManager', () => { const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); - cookieSyncManager.attemptCookieSync(null, testMPID, true); + cookieSyncManager.attemptCookieSync(testMPID, true); expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( - '', + pixelUrlAndReidrectUrl, '5', testMPID, {}, - { values: ['test'] }, true, true, ); @@ -147,107 +149,18 @@ describe('CookieSyncManager', () => { const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); - cookieSyncManager.attemptCookieSync(null, testMPID, true); + cookieSyncManager.attemptCookieSync(testMPID, true); expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( - 'https://test.com%3Fredirect%3Dhttps%3A%2F%2Fredirect.com%26mpid%3DtestMPID', + pixelUrlAndReidrectUrl, '5', testMPID, {}, - undefined, - true, - false, - ); - }); - - // https://go.mparticle.com/work/SQDSDKS-6915 - it('should call performCookieSync with mpid if previousMpid and mpid do not match', () => { - const mockMPInstance = ({ - _Store: { - webviewBridgeEnabled: false, - pixelConfigurations: [pixelSettings], - }, - _Persistence: { - getPersistence: () => ({testMPID: { - }}), - }, - } as unknown) as MParticleWebSDK; - - const cookieSyncManager = new CookieSyncManager(mockMPInstance); - cookieSyncManager.performCookieSync = jest.fn(); - - cookieSyncManager.attemptCookieSync('other-mpid', testMPID, true); - - expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( - '', - '5', - testMPID, - {}, - undefined, - true, - false, - ); - }); - - it('should include mpid AND csd when calling performCookieSync if previousMpid and mpid do not match', () => { - const mockMPInstance = ({ - _Store: { - webviewBridgeEnabled: false, - pixelConfigurations: [pixelSettings], - }, - _Persistence: { - getPersistence: () => ({testMPID: { - csd: { 5: 1234567890 } - }}), - }, - } as unknown) as MParticleWebSDK; - - const cookieSyncManager = new CookieSyncManager(mockMPInstance); - cookieSyncManager.performCookieSync = jest.fn(); - - cookieSyncManager.attemptCookieSync('other-mpid', testMPID, true); - - expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( - '', - '5', - testMPID, - { - 5: 1234567890 - }, - undefined, true, false, ); }); - it('should call performCookieSync with mpid if previousMpid and mpid match', () => { - const mockMPInstance = ({ - _Store: { - webviewBridgeEnabled: false, - pixelConfigurations: [pixelSettings], - }, - _Persistence: { - getPersistence: () => ({testMPID: { - }}), - }, - } as unknown) as MParticleWebSDK; - - const cookieSyncManager = new CookieSyncManager(mockMPInstance); - cookieSyncManager.performCookieSync = jest.fn(); - - cookieSyncManager.attemptCookieSync(testMPID, testMPID, true); - - expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( - '', - '5', - testMPID, - {}, - undefined, - true, - false, - ); - }); - it('should perform a cookie sync if lastSyncDateForModule is null', () => { const mockMPInstance = ({ _Store: { @@ -262,14 +175,13 @@ describe('CookieSyncManager', () => { const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); - cookieSyncManager.attemptCookieSync(null, testMPID, true); + cookieSyncManager.attemptCookieSync(testMPID, true); expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( - '', + pixelUrlAndReidrectUrl, '5', testMPID, {}, - undefined, true, false, ); @@ -296,16 +208,15 @@ describe('CookieSyncManager', () => { const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); - cookieSyncManager.attemptCookieSync(null, testMPID, true); + cookieSyncManager.attemptCookieSync(testMPID, true); expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( - '', + pixelUrlAndReidrectUrl, '5', testMPID, { 5: cookieSyncDateInPast }, - undefined, true, false, ); @@ -329,35 +240,57 @@ describe('CookieSyncManager', () => { const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); - cookieSyncManager.attemptCookieSync(null, testMPID, true); + cookieSyncManager.attemptCookieSync(testMPID, true); expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( - '', + pixelUrlAndReidrectUrl, '5', testMPID, {}, - undefined, true, false, ); }); - it('should sync cookies when there was not a previous cookie-sync', () => { + it.only('should sync cookies when there was not a previous cookie-sync', () => { const mockMPInstance = ({ _Store: { webviewBridgeEnabled: false, pixelConfigurations: [pixelSettings], }, _Persistence: { - getPersistence: () => ({}), + setCookieSyncDates: jest.fn(), + getPersistence: () => ({testMPID: {}}), + saveUserCookieSyncDatesToPersistence: jest.fn(), + }, + _Consent: { + isEnabledForUserConsent: jest.fn().mockReturnValue(true), + }, + Identity: { + getCurrentUser: jest.fn().mockReturnValue({ + getMPID: () => testMPID, + }), + }, + Logger: { + verbose: jest.fn(), }, } as unknown) as MParticleWebSDK; const cookieSyncManager = new CookieSyncManager(mockMPInstance); + cookieSyncManager.performCookieSync = jest.fn(); - cookieSyncManager.attemptCookieSync(null, '456', true); + cookieSyncManager.attemptCookieSync(testMPID, true); expect(mockMPInstance._Store.pixelConfigurations.length).toBe(1); expect(mockMPInstance._Store.pixelConfigurations[0].moduleId).toBe(5); + + expect(cookieSyncManager.performCookieSync).toHaveBeenCalledWith( + pixelUrlAndReidrectUrl, + '5', + testMPID, + {}, + true, + false, + ); }); it('should not perform a cookie sync if persistence is empty', () => { @@ -374,7 +307,7 @@ describe('CookieSyncManager', () => { const cookieSyncManager = new CookieSyncManager(mockMPInstance); cookieSyncManager.performCookieSync = jest.fn(); - cookieSyncManager.attemptCookieSync(null, testMPID, true); + cookieSyncManager.attemptCookieSync(testMPID, true); expect(cookieSyncManager.performCookieSync).not.toHaveBeenCalled(); }); @@ -396,6 +329,7 @@ describe('CookieSyncManager', () => { pixelConfigurations: [pixelSettings], }, _Persistence: { + // FIXME: why is this here? it doesn't exist in the SDK (it is used elsewhere too) setCookieSyncDates: jest.fn(), getPersistence: jest.fn(), saveUserCookieSyncDatesToPersistence: jest.fn(), @@ -421,7 +355,6 @@ describe('CookieSyncManager', () => { 42, '1234', cookieSyncDates, - null, false, false ); @@ -476,7 +409,6 @@ describe('CookieSyncManager', () => { 42, '1234', cookieSyncDates, - null, false, false ); @@ -583,7 +515,6 @@ describe('CookieSyncManager', () => { 42, '1234', cookieSyncDates, - null, true, true, ); @@ -595,46 +526,4 @@ describe('CookieSyncManager', () => { expect(cookieSyncDates[42]).toBeUndefined(); }); }); - - describe('#combineUrlWithRedirect', () => { - it('should combine a pixelUrl with a redirectUrl', () => { - const mockMPInstance = ({ - _Store: { - webviewBridgeEnabled: false, - pixelConfigurations: [pixelSettings], - }, - } as unknown) as MParticleWebSDK; - - const cookieSyncManager = new CookieSyncManager(mockMPInstance); - - // Note: We expect that the input of the pixelUrl will include a `?` - // (ideally at the end of the string) - // so that the redirectUrl can be processed by the backend correctly - const result = cookieSyncManager.combineUrlWithRedirect( - '1234', - 'https://test.com/some/path?', - 'https://redirect.mparticle.com/v1/sync?esid=1234&MPID=%%mpid%%&ID=$UID&Key=testMPID&env=2' - ); - - expect(result).toBe('https://test.com/some/path?https%3A%2F%2Fredirect.mparticle.com%2Fv1%2Fsync%3Fesid%3D1234%26amp%3BMPID%3D1234%26amp%3BID%3D%24UID%26amp%3BKey%3DtestMPID%26amp%3Benv%3D2'); - }); - - it('should return the pixelUrl if no redirectUrl is defined', () => { - const mockMPInstance = ({ - _Store: { - webviewBridgeEnabled: false, - pixelConfigurations: [pixelSettings], - }, - } as unknown) as MParticleWebSDK; - - const cookieSyncManager = new CookieSyncManager(mockMPInstance); - - const result = cookieSyncManager.combineUrlWithRedirect( - '1234', - 'https://test.com', - ); - - expect(result).toBe('https://test.com'); - }); - }); }); diff --git a/test/jest/utils.spec.ts b/test/jest/utils.spec.ts index f9ac67dc..b0987a16 100644 --- a/test/jest/utils.spec.ts +++ b/test/jest/utils.spec.ts @@ -4,6 +4,7 @@ import { getHref, replaceMPID, replaceAmpWithAmpersand, + combineUrlWithRedirect, } from '../../src/utils'; import { deleteAllCookies } from './utils'; @@ -183,4 +184,21 @@ describe('Utils', () => { ); }); }); + + describe('#combineUrlWithRedirect', () => { + const pixelUrl: string = "https://abc.abcdex.net/ibs:exampleid=12345&exampleuuid=%%mpid%%&redir="; + const redirectUrl: string = "https://cookiesync.mparticle.com/v1/sync?esid=123456&MPID=%%mpid%%&ID=${DD_UUID}&Key=mpApiKey&env=2" + + it('should properly combine data from a pixelUrl, redirectUrl, and mpid when everything is defined', () => { + expect(combineUrlWithRedirect('testMPID', pixelUrl, redirectUrl)).toEqual( + 'https://abc.abcdex.net/ibs:exampleid=12345&exampleuuid=testMPID&redir=https%3A%2F%2Fcookiesync.mparticle.com%2Fv1%2Fsync%3Fesid%3D123456%26amp%3BMPID%3DtestMPID%26amp%3BID%3D%24%7BDD_UUID%7D%26amp%3BKey%3DmpApiKey%26amp%3Benv%3D2' + ); + }); + + it('should return the pixelUrl with MPID if no redirectUrl is defined', () => { + expect(combineUrlWithRedirect('testMPID', pixelUrl, '')).toEqual( + 'https://abc.abcdex.net/ibs:exampleid=12345&exampleuuid=testMPID&redir=' + ); + }); + }); }); diff --git a/test/src/tests-cookie-syncing.ts b/test/src/tests-cookie-syncing.ts index 6eae3050..01fa9535 100644 --- a/test/src/tests-cookie-syncing.ts +++ b/test/src/tests-cookie-syncing.ts @@ -18,8 +18,8 @@ const pixelSettings: IPixelConfiguration = { isProduction: true, settings: {}, frequencyCap: 14, - pixelUrl: '', - redirectUrl: '', + pixelUrl: 'https://test.com', + redirectUrl: '?redirect=https://redirect.com&mpid=%%mpid%%', }; declare global { @@ -277,8 +277,8 @@ describe('cookie syncing', function() { isProduction: true, settings: {}, frequencyCap: 14, - pixelUrl: '', - redirectUrl: '', + pixelUrl: 'https://test.com', + redirectUrl: '?redirect=https://redirect.com&mpid=%%mpid%%', }, ], }; @@ -1222,8 +1222,8 @@ describe('cookie syncing', function() { isProduction: true, settings: {}, frequencyCap: 14, - pixelUrl: '', - redirectUrl: '', + pixelUrl: 'https://test.com', + redirectUrl: '?redirect=https://redirect.com&mpid=%%mpid%%', }; pixelSettings1.filteringConsentRuleValues = { @@ -1247,8 +1247,8 @@ describe('cookie syncing', function() { isProduction: true, settings: {}, frequencyCap: 14, - pixelUrl: '', - redirectUrl: '', + pixelUrl: 'https://test2.com', + redirectUrl: '?redirect=https://redirect2.com&mpid=%%mpid%%', }; window.mParticle.config.pixelConfigs = [pixelSettings1, pixelSettings2];