From 2dfedcbfe12961122e1cddfe2763764b3bde7c0e Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Mon, 22 Jul 2019 09:15:29 -0600 Subject: [PATCH] update user sync to allow for multiple auction syncs (#3984) --- src/userSync.js | 30 +++++++++++++++++++----------- test/spec/userSync_spec.js | 7 +++++-- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/userSync.js b/src/userSync.js index 3cbdc58a075..2d5dfbd9a28 100644 --- a/src/userSync.js +++ b/src/userSync.js @@ -24,8 +24,8 @@ export function newUserSync(userSyncDependencies) { // Let getDefaultQueue() set the defaults let queue = getDefaultQueue(); - // Whether or not user syncs have been trigger on this page load - let hasFired = false; + // Whether or not user syncs have been trigger on this page load for a specific bidder + let hasFiredBidder = new Set(); // How many bids for each adapter let numAdapterBids = {}; @@ -33,7 +33,7 @@ export function newUserSync(userSyncDependencies) { let permittedPixels = { image: false, iframe: false - } + }; // Use what is in config by default let usConfig = userSyncDependencies.config; @@ -61,7 +61,7 @@ export function newUserSync(userSyncDependencies) { * @private */ function fireSyncs() { - if (!usConfig.syncEnabled || !userSyncDependencies.browserSupportsCookies || (!usConfig.enableOverride && hasFired)) { + if (!usConfig.syncEnabled || !userSyncDependencies.browserSupportsCookies) { return; } @@ -75,7 +75,16 @@ export function newUserSync(userSyncDependencies) { } // Reset the user sync queue queue = getDefaultQueue(); - hasFired = true; + } + + function forEachFire(queue, fn) { + // Randomize the order of the pixels before firing + // This is to avoid giving any bidder who has registered multiple syncs + // any preferential treatment and balancing them out + utils.shuffle(queue).forEach((sync) => { + fn(sync); + hasFiredBidder.add(sync[0]); + }); } /** @@ -87,10 +96,7 @@ export function newUserSync(userSyncDependencies) { if (!(usConfig.pixelEnabled || permittedPixels.image)) { return; } - // Randomize the order of the pixels before firing - // This is to avoid giving any bidder who has registered multiple syncs - // any preferential treatment and balancing them out - utils.shuffle(queue.image).forEach((sync) => { + forEachFire(queue.image, (sync) => { let [bidderName, trackingPixelUrl] = sync; utils.logMessage(`Invoking image pixel user sync for bidder: ${bidderName}`); // Create image object and add the src url @@ -107,8 +113,7 @@ export function newUserSync(userSyncDependencies) { if (!(usConfig.iframeEnabled || permittedPixels.iframe)) { return; } - // Randomize the order of these syncs just like the pixels above - utils.shuffle(queue.iframe).forEach((sync) => { + forEachFire(queue.iframe, (sync) => { let [bidderName, iframeUrl] = sync; utils.logMessage(`Invoking iframe user sync for bidder: ${bidderName}`); // Insert iframe into DOM @@ -146,6 +151,9 @@ export function newUserSync(userSyncDependencies) { * userSync.registerSync('image', 'rubicon', 'http://example.com/pixel') */ publicApi.registerSync = (type, bidder, url) => { + if (hasFiredBidder.has(bidder)) { + return utils.logWarn(`already registered syncs for "${bidder}"`); + } if (!usConfig.syncEnabled || !utils.isArray(queue[type])) { return utils.logWarn(`User sync type "${type}" not supported`); } diff --git a/test/spec/userSync_spec.js b/test/spec/userSync_spec.js index f330be4c2f4..f55fe13c528 100644 --- a/test/spec/userSync_spec.js +++ b/test/spec/userSync_spec.js @@ -98,15 +98,18 @@ describe('user sync', function () { expect(insertUserSyncIframeStub.getCall(0).args[0]).to.equal('http://example.com/iframe'); }); - it('should only trigger syncs once per page', function () { + it('should only trigger syncs once per page per bidder', function () { const userSync = newTestUserSync({pixelEnabled: true}); userSync.registerSync('image', 'testBidder', 'http://example.com/1'); userSync.syncUsers(); userSync.registerSync('image', 'testBidder', 'http://example.com/2'); + userSync.registerSync('image', 'testBidder2', 'http://example.com/3'); userSync.syncUsers(); + expect(triggerPixelStub.callCount).to.equal(2); expect(triggerPixelStub.getCall(0)).to.not.be.null; expect(triggerPixelStub.getCall(0).args[0]).to.exist.and.to.equal('http://example.com/1'); - expect(triggerPixelStub.getCall(1)).to.be.null; + expect(triggerPixelStub.getCall(1)).to.not.be.null; + expect(triggerPixelStub.getCall(1).args[0]).to.exist.and.to.equal('http://example.com/3'); }); it('should not fire syncs if cookies are not supported', function () {