Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rubicon project improvement/user sync #1549

Merged
merged 51 commits into from
Sep 18, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
89515a0
Add new user sync module
grevory May 15, 2017
3adaee4
Rename user sync file
grevory May 15, 2017
e233465
Reset image pixel queue after firing pixels
grevory May 17, 2017
9d20a9a
Resolved conflicts from master
grevory May 17, 2017
b7d1455
Added unit test for user sync
grevory May 23, 2017
efa23e2
Merge conflict from master
grevory May 24, 2017
9bef360
Conflict with master
grevory May 24, 2017
ca8cb54
Fixed typo
grevory May 24, 2017
b28f15b
Overlooked conflict from master
grevory May 24, 2017
7840f88
Improvements based on peer feedback for user sync
grevory May 25, 2017
7450ee5
Merge branch 'master' of https://github.com/rubicon-project/Prebid.js…
grevory Jun 6, 2017
f981c20
Added global user sync config
grevory Jun 23, 2017
66abfef
Allow auto-syncing override
grevory Jun 24, 2017
f34eb0f
Prevent syncing if there is no cookie support
grevory Jun 27, 2017
bd0cef2
Only trigger sync once per page
grevory Jul 3, 2017
4b13cc2
Limit number of user syncs per bidder
grevory Jul 10, 2017
2174667
User syncs should fire pixels without adding element to DOM
grevory Jul 11, 2017
61dbb60
Merged master into improvement/user-sync-enhancements
grevory Jul 11, 2017
c5bf2a8
Randomize user sync order by type
grevory Jul 27, 2017
94a64c7
Merge branch 'master' into improvement/user-sync-enhancements
grevory Jul 27, 2017
ff44cb7
Comment about shuffling user syncs
grevory Jul 27, 2017
26f1e20
Remove karma.conf.js and fix unused default value
grevory Jul 27, 2017
8fc8ee2
Fix prebid server adapter test to use the right cookie function
grevory Jul 27, 2017
b373c69
Added iframe support to user sync
grevory Jul 28, 2017
3f66618
Fix oversights for user sync
grevory Jul 31, 2017
c414f94
Move try catch block for user sync
grevory Aug 1, 2017
df99a16
Option to disable user sync
grevory Aug 1, 2017
a213c81
Enable specific bidders for user sync
grevory Aug 1, 2017
7cb4c24
Merge branch 'improvement/user-sync' into improvement/user-sync-enhan…
grevory Aug 1, 2017
bb30408
Merge branch 'improvement/user-sync-iframe' of https://github.com/rub…
grevory Aug 3, 2017
c1cca60
Encapsulate the user sync module
grevory Aug 3, 2017
82d1ce6
Remove unused functions for user sync
grevory Aug 6, 2017
9f6d3fd
Merge branch 'master' of https://github.com/rubicon-project/Prebid.js…
grevory Aug 6, 2017
7c71de1
Clean merge markers
grevory Aug 6, 2017
d0185d6
Merge branch 'improvement/user-sync' of https://github.com/rubicon-pr…
Aug 15, 2017
88f8197
Merge branch 'master' into improvement/user-sync
Aug 15, 2017
bd9acb4
fix usersync test problems
Aug 15, 2017
6766102
added setConfig to userSync
Aug 17, 2017
0e67a72
Merge branch 'master' into improvement/user-sync
Aug 17, 2017
2213fa5
fixed lint problem
Aug 17, 2017
e79760b
Change override user sync API and how user sync uses config
grevory Aug 29, 2017
f00a8b5
Fixed registerSync API to avoid spread operator
grevory Aug 29, 2017
f49a1e4
Merged from upstream
grevory Aug 29, 2017
9a197ab
Fixed issues from last conflict resolution
grevory Aug 30, 2017
cd5b6f5
Merge branch 'improvement/user-sync' of https://github.com/rubicon-pr…
mkendall07 Aug 31, 2017
9718d06
Small refactoring to inject dependencies better.
dbemiller Aug 31, 2017
a7a2945
Added a test for when user syncs are disabled.
dbemiller Aug 31, 2017
20ce33a
Merge branch 'master' of https://github.com/prebid/Prebid.js into rub…
dbemiller Sep 15, 2017
b555778
Implemented usersync in the base adapter.
dbemiller Sep 15, 2017
ecff1a7
user-sync updates
Sep 15, 2017
cd7206a
not tracking client bidder syncs
Sep 18, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/prebid.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ var bidfactory = require('./bidfactory');
var events = require('./events');
var adserver = require('./adserver.js');
var targeting = require('./targeting.js');
const { syncUsers, syncUsersOverride } = userSync;
const { syncUsers, triggerUserSyncs } = userSync;

/* private variables */

Expand Down Expand Up @@ -73,7 +73,7 @@ utils.logInfo('Prebid.js v$prebid.version$ loaded');
$$PREBID_GLOBAL$$.adUnits = $$PREBID_GLOBAL$$.adUnits || [];

// Allow publishers who enable user sync override to trigger their sync
$$PREBID_GLOBAL$$.triggerUserSyncs = syncUsersOverride;
$$PREBID_GLOBAL$$.triggerUserSyncs = triggerUserSyncs;

function checkDefinedPlacement(id) {
var placementCodes = $$PREBID_GLOBAL$$._bidsRequested.map(bidSet => bidSet.bids.map(bid => bid.placementCode))
Expand Down
18 changes: 16 additions & 2 deletions src/userSync.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as utils from 'src/utils';
import { config } from 'src/config';
import { StorageManager, pbjsSyncsKey } from './storagemanager';

/**
* Factory function which creates a new UserSyncPool.
Expand Down Expand Up @@ -74,6 +75,8 @@ export function newUserSync(userSyncDependencies) {
utils.logMessage(`Invoking image pixel user sync for bidder: ${bidderName}`);
// Create image object and add the src url
utils.triggerPixel(trackingPixelUrl);
// track that bidder has been synced
setBidderSynced(bidderName);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harpere for now, let's drop these as we are only storing keys for S2S bidders and not client side. Later on we can integrate them, possibly by appending s2s${biddercode} to differentiate.

});
}

Expand All @@ -92,9 +95,20 @@ export function newUserSync(userSyncDependencies) {
utils.logMessage(`Invoking iframe user sync for bidder: ${bidderName}`);
// Create image object and add the src url
utils.insertUserSyncIframe(iframeUrl);
// track that bidder has been synced
setBidderSynced(bidderName);
});
}

/**
* @function setBidderSynced
* @summary track that bidder has been synced in storagemanager
* @private
*/
function setBidderSynced(bidder) {
StorageManager.add(pbjsSyncsKey, bidder, true);
}

/**
* @function incrementAdapterBids
* @summary Increment the count of user syncs queue for the adapter
Expand Down Expand Up @@ -157,11 +171,11 @@ export function newUserSync(userSyncDependencies) {
};

/**
* @function overrideSyncUsers
* @function triggerUserSyncs
* @summary A `syncUsers` wrapper for determining if enableOverride has been turned on
* @public
*/
publicApi.syncUsersOverride = () => {
publicApi.triggerUserSyncs = () => {
if (config.enableOverride) {
publicApi.syncUsers();
}
Expand Down
11 changes: 7 additions & 4 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -463,8 +463,7 @@ exports.triggerPixel = function (url) {
* @param {string} encodeUri boolean if URL should be encoded before inserted. Defaults to true
*/
exports.insertUserSyncIframe = function(url) {
let iframeHtml = this.createTrackPixelIframeHtml(url, false);
iframeHtml = iframeHtml.replace('<iframe', '<iframe sandbox="allow-scripts"');
let iframeHtml = this.createTrackPixelIframeHtml(url, false, 'allow-scripts');
let div = document.createElement('div');
div.innerHTML = iframeHtml;
let iframe = div.firstChild;
Expand All @@ -491,17 +490,21 @@ exports.createTrackPixelHtml = function (url) {
* Creates a snippet of Iframe HTML that retrieves the specified `url`
* @param {string} url plain URL to be requested
* @param {string} encodeUri boolean if URL should be encoded before inserted. Defaults to true
* @param {string} sandbox string if provided the sandbox attribute will be included with the given value
* @return {string} HTML snippet that contains the iframe src = set to `url`
*/
exports.createTrackPixelIframeHtml = function (url, encodeUri = true) {
exports.createTrackPixelIframeHtml = function (url, encodeUri = true, sandbox = '') {
if (!url) {
return '';
}
if (encodeUri) {
url = encodeURI(url);
}
if (sandbox) {
sandbox = `sandbox="${sandbox}"`;
}

return `<iframe id="${exports.getUniqueIdentifierStr()}"
return `<iframe ${sandbox} id="${exports.getUniqueIdentifierStr()}"
frameborder="0"
allowtransparency="true"
marginheight="0" marginwidth="0"
Expand Down
35 changes: 31 additions & 4 deletions test/spec/userSync_spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { expect } from 'chai';
import { config } from 'src/config';
import { StorageManager, pbjsSyncsKey } from 'src/storagemanager';
// Use require since we need to be able to write to these vars
const utils = require('../../src/utils');
let { newUserSync } = require('../../src/userSync');
Expand All @@ -10,6 +11,7 @@ describe('user sync', () => {
let timeoutStub;
let shuffleStub;
let getUniqueIdentifierStrStub;
let storageManagerAddStub;
let idPrefix = 'test-generated-id-';
let lastId = 0;
let defaultUserSyncConfig = config.getConfig('userSync');
Expand All @@ -30,6 +32,7 @@ describe('user sync', () => {
shuffleStub = sinon.stub(utils, 'shuffle', (array) => array.reverse());
getUniqueIdentifierStrStub = sinon.stub(utils, 'getUniqueIdentifierStr', () => idPrefix + (lastId += 1));
timeoutStub = sinon.stub(window, 'setTimeout', (callbackFunc) => { callbackFunc(); });
storageManagerAddStub = sinon.stub(StorageManager, 'add');
});

afterEach(() => {
Expand All @@ -38,6 +41,7 @@ describe('user sync', () => {
shuffleStub.restore();
getUniqueIdentifierStrStub.restore();
timeoutStub.restore();
storageManagerAddStub.restore();
});

it('should register and fire a pixel URL', () => {
Expand All @@ -46,12 +50,15 @@ describe('user sync', () => {
userSync.syncUsers();
expect(triggerPixelStub.getCall(0)).to.not.be.null;
expect(triggerPixelStub.getCall(0).args[0]).to.exist.and.to.equal('http://example.com');
expect(storageManagerAddStub.getCall(0)).to.not.be.null;
expect(storageManagerAddStub.getCall(0).args).to.eql([pbjsSyncsKey, 'testBidder', true]);
});

it('should clear queue after sync', () => {
const userSync = newTestUserSync();
userSync.syncUsers();
expect(triggerPixelStub.callCount).to.equal(0);
expect(storageManagerAddStub.callCount).to.be.equal(0);
});

it('should delay firing a pixel by the expected amount', () => {
Expand All @@ -72,13 +79,19 @@ describe('user sync', () => {
expect(triggerPixelStub.getCall(1)).to.not.be.null;
expect(triggerPixelStub.getCall(1).args[0]).to.exist.and.to.include('http://example.com/');
expect(triggerPixelStub.getCall(2)).to.be.null;
expect(storageManagerAddStub.getCall(0)).to.not.be.null;
expect(storageManagerAddStub.getCall(0).args).to.eql([pbjsSyncsKey, 'testBidder', true]);
expect(storageManagerAddStub.getCall(1)).to.not.be.null;
expect(storageManagerAddStub.getCall(1).args).to.eql([pbjsSyncsKey, 'testBidder', true]);
expect(storageManagerAddStub.getCall(2)).to.be.null;
});

it('should not register pixel URL since it is not supported', () => {
const userSync = newTestUserSync({pixelEnabled: false});
userSync.registerSync('image', 'testBidder', 'http://example.com');
userSync.syncUsers();
expect(triggerPixelStub.getCall(0)).to.be.null;
expect(storageManagerAddStub.getCall(0)).to.be.null;
});

it('should register and load an iframe', () => {
Expand All @@ -88,6 +101,8 @@ describe('user sync', () => {
let iframe = window.document.getElementById(idPrefix + lastId);
expect(iframe).to.exist;
expect(iframe.src).to.equal('http://example.com/iframe');
expect(storageManagerAddStub.getCall(0)).to.not.be.null;
expect(storageManagerAddStub.getCall(0).args).to.eql([pbjsSyncsKey, 'testBidder', true]);
});

it('should only trigger syncs once per page', () => {
Expand All @@ -99,13 +114,17 @@ describe('user sync', () => {
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(storageManagerAddStub.getCall(0)).to.not.be.null;
expect(storageManagerAddStub.getCall(0).args).to.eql([pbjsSyncsKey, 'testBidder', true]);
expect(storageManagerAddStub.getCall(1)).to.be.null;
});

it('should not fire syncs if cookies are not supported', () => {
const userSync = newTestUserSync({pixelEnabled: true}, true);
userSync.registerSync('image', 'testBidder', 'http://example.com');
userSync.syncUsers();
expect(triggerPixelStub.getCall(0)).to.be.null;
expect(storageManagerAddStub.getCall(0)).to.be.null;
});

it('should prevent registering invalid type', () => {
Expand All @@ -115,15 +134,15 @@ describe('user sync', () => {
});

it('should expose the syncUsers method for the publisher to manually trigger syncs', () => {
// syncUsersOverride should do nothing by default
// triggerUserSyncs should do nothing by default
let userSync = newTestUserSync();
let syncUsersSpy = sinon.spy(userSync, 'syncUsers');
userSync.syncUsersOverride();
userSync.triggerUserSyncs();
expect(syncUsersSpy.notCalled).to.be.true;
// syncUsersOverride should trigger syncUsers if enableOverride is on
// triggerUserSyncs should trigger syncUsers if enableOverride is on
userSync = newTestUserSync({enableOverride: true});
syncUsersSpy = sinon.spy(userSync, 'syncUsers');
userSync.syncUsersOverride();
userSync.triggerUserSyncs();
expect(syncUsersSpy.called).to.be.true;
});

Expand All @@ -138,6 +157,11 @@ describe('user sync', () => {
expect(triggerPixelStub.getCall(1)).to.not.be.null;
expect(triggerPixelStub.getCall(1).args[0]).to.exist.and.to.match(/^http:\/\/example\.com\/[1|2]/);
expect(triggerPixelStub.getCall(2)).to.be.null;
expect(storageManagerAddStub.getCall(0)).to.not.be.null;
expect(storageManagerAddStub.getCall(0).args).to.eql([pbjsSyncsKey, 'testBidder', true]);
expect(storageManagerAddStub.getCall(1)).to.not.be.null;
expect(storageManagerAddStub.getCall(1).args).to.eql([pbjsSyncsKey, 'testBidder', true]);
expect(storageManagerAddStub.getCall(2)).to.be.null;
});

it('should balance out bidder requests', () => {
Expand Down Expand Up @@ -172,5 +196,8 @@ describe('user sync', () => {
expect(triggerPixelStub.getCall(0)).to.not.be.null;
expect(triggerPixelStub.getCall(0).args[0]).to.exist.and.to.include('http://example.com/');
expect(triggerPixelStub.getCall(1)).to.be.null;
expect(storageManagerAddStub.getCall(0)).to.not.be.null;
expect(storageManagerAddStub.getCall(0).args).to.eql([pbjsSyncsKey, 'testBidderA', true]);
expect(storageManagerAddStub.getCall(1)).to.be.null;
});
});