Skip to content

Commit

Permalink
Deprecate user sync (#4241)
Browse files Browse the repository at this point in the history
* deprecate some properties of userSync object

* change bid adapter to Appnexus

* fix 3 failing test cases

* make all unit test cases pass

* remove debugger
  • Loading branch information
Fawke authored Dec 3, 2019
1 parent dfdde06 commit f561c71
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 42 deletions.
4 changes: 2 additions & 2 deletions src/adapters/bidderFactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,8 @@ export function newBidder(spec) {
if (spec.getUserSyncs) {
let filterConfig = config.getConfig('userSync.filterSettings');
let syncs = spec.getUserSyncs({
iframeEnabled: !!(config.getConfig('userSync.iframeEnabled') || (filterConfig && (filterConfig.iframe || filterConfig.all))),
pixelEnabled: !!(config.getConfig('userSync.pixelEnabled') || (filterConfig && (filterConfig.image || filterConfig.all))),
iframeEnabled: !!(filterConfig && (filterConfig.iframe || filterConfig.all)),
pixelEnabled: !!(filterConfig && (filterConfig.image || filterConfig.all)),
}, responses, gdprConsent);
if (syncs) {
if (!Array.isArray(syncs)) {
Expand Down
22 changes: 9 additions & 13 deletions src/userSync.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@ import includes from 'core-js/library/fn/array/includes';
config.setDefaults({
'userSync': {
syncEnabled: true,
pixelEnabled: true,
filterSettings: {
image: {
bidders: '*',
filter: 'include'
}
},
syncsPerBidder: 5,
syncDelay: 3000,
auctionDelay: 0
Expand All @@ -32,7 +37,7 @@ export function newUserSync(userSyncDependencies) {

// for now - default both to false in case filterSettings config is absent/misconfigured
let permittedPixels = {
image: false,
image: true,
iframe: false
};

Expand Down Expand Up @@ -94,7 +99,7 @@ export function newUserSync(userSyncDependencies) {
* @private
*/
function fireImagePixels() {
if (!(usConfig.pixelEnabled || permittedPixels.image)) {
if (!permittedPixels.image) {
return;
}
forEachFire(queue.image, (sync) => {
Expand All @@ -111,7 +116,7 @@ export function newUserSync(userSyncDependencies) {
* @private
*/
function loadIframes() {
if (!(usConfig.iframeEnabled || permittedPixels.iframe)) {
if (!(permittedPixels.iframe)) {
return;
}
forEachFire(queue.iframe, (sync) => {
Expand Down Expand Up @@ -272,13 +277,6 @@ export function newUserSync(userSyncDependencies) {
if (shouldBidderBeBlocked(type, bidder)) {
return false;
}
// TODO remove this else if code that supports deprecated fields (sometime in 2.x); for now - only run if filterSettings config is not present
} else if (usConfig.enabledBidders && usConfig.enabledBidders.length && usConfig.enabledBidders.indexOf(bidder) < 0) {
return false
} else if (type === 'iframe' && !(usConfig.iframeEnabled || permittedPixels.iframe)) {
return false;
} else if (type === 'image' && !(usConfig.pixelEnabled || permittedPixels.image)) {
return false;
}
return true;
}
Expand All @@ -304,8 +302,6 @@ export const userSync = newUserSync({
*
* @property {boolean} enableOverride
* @property {boolean} syncEnabled
* @property {boolean} pixelEnabled
* @property {boolean} iframeEnabled
* @property {int} syncsPerBidder
* @property {string[]} enabledBidders
* @property {Object} filterSettings
Expand Down
82 changes: 55 additions & 27 deletions test/spec/userSync_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,21 +85,31 @@ describe('user sync', function () {
});

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

it('should register and load an iframe', function () {
const userSync = newTestUserSync({iframeEnabled: true});
const userSync = newTestUserSync({filterSettings: {
iframe: {
bidders: '*',
filter: 'include'
}
}});
userSync.registerSync('iframe', 'testBidder', 'http://example.com/iframe');
userSync.syncUsers();
expect(insertUserSyncIframeStub.getCall(0).args[0]).to.equal('http://example.com/iframe');
});

it('should only trigger syncs once per page per bidder', function () {
const userSync = newTestUserSync({pixelEnabled: true});
const userSync = newTestUserSync({ pixelEnabled: true });
userSync.registerSync('image', 'testBidder', 'http://example.com/1');
userSync.syncUsers();
userSync.registerSync('image', 'testBidder', 'http://example.com/2');
Expand All @@ -113,7 +123,7 @@ describe('user sync', function () {
});

it('should not fire syncs if cookies are not supported', function () {
const userSync = newTestUserSync({pixelEnabled: true}, true);
const userSync = newTestUserSync({ pixelEnabled: true }, true);
userSync.registerSync('image', 'testBidder', 'http://example.com');
userSync.syncUsers();
expect(triggerPixelStub.getCall(0)).to.be.null;
Expand All @@ -132,14 +142,14 @@ describe('user sync', function () {
userSync.triggerUserSyncs();
expect(syncUsersSpy.notCalled).to.be.true;
// triggerUserSyncs should trigger syncUsers if enableOverride is on
userSync = newTestUserSync({enableOverride: true});
userSync = newTestUserSync({ enableOverride: true });
syncUsersSpy = sinon.spy(userSync, 'syncUsers');
userSync.triggerUserSyncs();
expect(syncUsersSpy.called).to.be.true;
});

it('should limit the number of syncs per bidder', function () {
const userSync = newTestUserSync({syncsPerBidder: 2});
const userSync = newTestUserSync({ syncsPerBidder: 2 });
userSync.registerSync('image', 'testBidder', 'http://example.com/1');
userSync.registerSync('image', 'testBidder', 'http://example.com/2');
userSync.registerSync('image', 'testBidder', 'http://example.com/3');
Expand All @@ -151,8 +161,8 @@ describe('user sync', function () {
expect(triggerPixelStub.getCall(2)).to.be.null;
});

it('should not limit the number of syncs per bidder when set to 0', function() {
const userSync = newTestUserSync({syncsPerBidder: 0});
it('should not limit the number of syncs per bidder when set to 0', function () {
const userSync = newTestUserSync({ syncsPerBidder: 0 });
userSync.registerSync('image', 'testBidder', 'http://example.com/1');
userSync.registerSync('image', 'testBidder', 'http://example.com/2');
userSync.registerSync('image', 'testBidder', 'http://example.com/3');
Expand Down Expand Up @@ -182,15 +192,20 @@ describe('user sync', function () {
});

it('should disable user sync', function () {
const userSync = newTestUserSync({syncEnabled: false});
const userSync = newTestUserSync({ syncEnabled: false });
userSync.registerSync('pixel', 'testBidder', 'http://example.com');
expect(logWarnStub.getCall(0).args[0]).to.exist;
userSync.syncUsers();
expect(triggerPixelStub.getCall(0)).to.be.null;
});

it('should only sync enabled bidders', function () {
const userSync = newTestUserSync({enabledBidders: ['testBidderA']});
const userSync = newTestUserSync({filterSettings: {
image: {
bidders: ['testBidderA'],
filter: 'include'
}
}});
userSync.registerSync('image', 'testBidderA', 'http://example.com/1');
userSync.registerSync('image', 'testBidderB', 'http://example.com/2');
userSync.syncUsers();
Expand All @@ -201,9 +216,9 @@ describe('user sync', function () {

it('should register config set after instantiation', function () {
// start with userSync off
const userSync = newTestUserSync({syncEnabled: false});
const userSync = newTestUserSync({ syncEnabled: false });
// turn it on with setConfig()
config.setConfig({userSync: {syncEnabled: true}});
config.setConfig({ userSync: { syncEnabled: true } });
userSync.registerSync('image', 'testBidder', 'http://example.com');
userSync.syncUsers();
expect(triggerPixelStub.getCall(0)).to.not.be.null;
Expand Down Expand Up @@ -360,8 +375,8 @@ describe('user sync', function () {
});

describe('publicAPI', function () {
describe('canBidderRegisterSync', function() {
describe('with filterSettings', function() {
describe('canBidderRegisterSync', function () {
describe('with filterSettings', function () {
it('should return false if filter settings does not allow it', function () {
const userSync = newUserSync({
config: {
Expand Down Expand Up @@ -397,14 +412,17 @@ describe('user sync', function () {
expect(userSync.canBidderRegisterSync('iframe', 'testBidder')).to.equal(true);
});
});
describe('almost deprecated - without filterSettings', function() {
describe('enabledBidders contains testBidder', function() {
describe('almost deprecated - without filterSettings', function () {
describe('enabledBidders contains testBidder', function () {
it('should return false if type is iframe and iframeEnabled is false', function () {
const userSync = newUserSync({
config: {
pixelEnabled: true,
iframeEnabled: false,
enabledBidders: ['testBidder'],
filterSettings: {
iframe: {
bidders: ['testBidder'],
filter: 'exclude'
}
}
}
});
expect(userSync.canBidderRegisterSync('iframe', 'testBidder')).to.equal(false);
Expand All @@ -424,9 +442,12 @@ describe('user sync', function () {
it('should return false if type is image and pixelEnabled is false', function () {
const userSync = newUserSync({
config: {
pixelEnabled: false,
iframeEnabled: true,
enabledBidders: ['testBidder'],
filterSettings: {
image: {
bidders: ['testBidder'],
filter: 'exclude'
}
}
}
});
expect(userSync.canBidderRegisterSync('image', 'testBidder')).to.equal(false);
Expand All @@ -444,13 +465,20 @@ describe('user sync', function () {
});
});

describe('enabledBidders does not container testBidder', function() {
it('should return false since testBidder is not in enabledBidders', function() {
describe('enabledBidders does not container testBidder', function () {
it('should return false since testBidder is not in enabledBidders', function () {
const userSync = newUserSync({
config: {
pixelEnabled: true,
iframeEnabled: true,
enabledBidders: ['otherTestBidder'],
filterSettings: {
image: {
bidders: ['otherTestBidder'],
filter: 'include'
},
iframe: {
bidders: ['otherTestBidder'],
filter: 'include'
}
}
}
});
expect(userSync.canBidderRegisterSync('iframe', 'testBidder')).to.equal(false);
Expand Down

0 comments on commit f561c71

Please sign in to comment.