update user sync to allow for multiple auction syncs (prebid#3984)

gamoshi · Nov 28, 2019 · 9fe0837 · 9fe0837
1 parent 552d160
commit 9fe0837
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 13 deletions.
diff --git a/src/userSync.js b/src/userSync.js
@@ -24,16 +24,16 @@ 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 = {};
 
   // for now - default both to false in case filterSettings config is absent/misconfigured
   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
@@ -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 () {