From 357e515ebc8e0353786982a4e2986a8baf2f2583 Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Wed, 8 Nov 2017 14:00:34 -0800 Subject: [PATCH 1/3] Check mediaTypes when validating video bidders --- src/video.js | 6 +++++- test/spec/unit/pbjs_api_spec.js | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/video.js b/src/video.js index f5203e4b198..ff890c5e24a 100644 --- a/src/video.js +++ b/src/video.js @@ -8,7 +8,11 @@ const OUTSTREAM = 'outstream'; /** * Helper functions for working with video-enabled adUnits */ -export const videoAdUnit = adUnit => adUnit.mediaType === VIDEO_MEDIA_TYPE; +export const videoAdUnit = adUnit => { + const mediaTypes = deepAccess(adUnit, 'mediaTypes.video'); + return !!mediaTypes || adUnit.mediaType === VIDEO_MEDIA_TYPE; +}; + const nonVideoBidder = bid => !videoAdapters.includes(bid.bidder); export const hasNonVideoBidder = adUnit => adUnit.bids.filter(nonVideoBidder).length; diff --git a/test/spec/unit/pbjs_api_spec.js b/test/spec/unit/pbjs_api_spec.js index d6f1451b65c..37f6ef7ba34 100644 --- a/test/spec/unit/pbjs_api_spec.js +++ b/test/spec/unit/pbjs_api_spec.js @@ -799,6 +799,26 @@ describe('Unit: Prebid Module', function () { adaptermanager.videoAdapters = videoAdaptersBackup; }); + it('should not callBids if a video adUnit defined with mediaTypes has non-video bidders', () => { + sinon.spy(adaptermanager, 'callBids'); + const videoAdaptersBackup = adaptermanager.videoAdapters; + adaptermanager.videoAdapters = ['appnexusAst']; + const adUnits = [{ + code: 'adUnit-code', + mediaTypes: {video: {context: 'instream'}}, + bids: [ + {bidder: 'appnexus', params: {placementId: 'id'}}, + {bidder: 'appnexusAst', params: {placementId: 'id'}} + ] + }]; + + $$PREBID_GLOBAL$$.requestBids({adUnits}); + sinon.assert.notCalled(adaptermanager.callBids); + + adaptermanager.callBids.restore(); + adaptermanager.videoAdapters = videoAdaptersBackup; + }); + it('should callBids if a video adUnit has all video bidders', () => { sinon.spy(adaptermanager, 'callBids'); const videoAdaptersBackup = adaptermanager.videoAdapters; From 2aad787371dd934ad708ad09dd8d5de170f283d9 Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Wed, 8 Nov 2017 14:07:30 -0800 Subject: [PATCH 2/3] Update error message --- src/prebid.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/prebid.js b/src/prebid.js index 3d25eff6761..889abb50977 100644 --- a/src/prebid.js +++ b/src/prebid.js @@ -383,7 +383,7 @@ $$PREBID_GLOBAL$$.requestBids = function ({ bidsBackHandler, timeout, adUnits, a // for video-enabled adUnits, only request bids if all bidders support video const invalidVideoAdUnits = adUnits.filter(videoAdUnit).filter(hasNonVideoBidder); invalidVideoAdUnits.forEach(adUnit => { - utils.logError(`adUnit ${adUnit.code} has 'mediaType' set to 'video' but contains a bidder that doesn't support video. No Prebid demand requests will be triggered for this adUnit.`); + utils.logError(`adUnit ${adUnit.code} is of type 'video' but contains a bidder that doesn't support video. No Prebid demand requests will be triggered for this adUnit.`); for (let i = 0; i < adUnits.length; i++) { if (adUnits[i].code === adUnit.code) { adUnits.splice(i, 1); } } From 515faa30a530073369a7ae211b6cdb0934cbc6b6 Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Wed, 8 Nov 2017 16:52:03 -0800 Subject: [PATCH 3/3] Drop non-compatible bidders rather than entire ad unit --- src/prebid.js | 26 +++++++++++++++++--------- src/video.js | 8 ++++---- test/spec/unit/pbjs_api_spec.js | 16 ++++++++++++---- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/prebid.js b/src/prebid.js index 889abb50977..48862b7eb2b 100644 --- a/src/prebid.js +++ b/src/prebid.js @@ -2,7 +2,7 @@ import { getGlobal } from './prebidGlobal'; import { flatten, uniques, isGptPubadsDefined, adUnitsFilter } from './utils'; -import { videoAdUnit, hasNonVideoBidder } from './video'; +import { videoAdUnit, videoBidder, hasNonVideoBidder } from './video'; import { nativeAdUnit, nativeBidder, hasNonNativeBidder } from './native'; import './polyfill'; import { parse as parseURL, format as formatURL } from './url'; @@ -380,13 +380,18 @@ $$PREBID_GLOBAL$$.requestBids = function ({ bidsBackHandler, timeout, adUnits, a adUnitCodes = adUnits && adUnits.map(unit => unit.code); } - // for video-enabled adUnits, only request bids if all bidders support video - const invalidVideoAdUnits = adUnits.filter(videoAdUnit).filter(hasNonVideoBidder); - invalidVideoAdUnits.forEach(adUnit => { - utils.logError(`adUnit ${adUnit.code} is of type 'video' but contains a bidder that doesn't support video. No Prebid demand requests will be triggered for this adUnit.`); - for (let i = 0; i < adUnits.length; i++) { - if (adUnits[i].code === adUnit.code) { adUnits.splice(i, 1); } - } + // for video-enabled adUnits, only request bids for bidders that support video + adUnits.filter(videoAdUnit).filter(hasNonVideoBidder).forEach(adUnit => { + const nonVideoBidders = adUnit.bids + .filter(bid => !videoBidder(bid)) + .map(bid => bid.bidder) + .join(', '); + + utils.logError(` + ${adUnit.code} is a 'video' ad unit but contains non-video bidder(s) ${nonVideoBidders}. + No Prebid demand requests will be triggered for those bidders. + `); + adUnit.bids = adUnit.bids.filter(videoBidder); }); // for native-enabled adUnits, only request bids for bidders that support native @@ -396,7 +401,10 @@ $$PREBID_GLOBAL$$.requestBids = function ({ bidsBackHandler, timeout, adUnits, a .map(bid => bid.bidder) .join(', '); - utils.logError(`adUnit ${adUnit.code} has 'mediaType' set to 'native' but contains non-native bidder(s) ${nonNativeBidders}. No Prebid demand requests will be triggered for those bidders.`); + utils.logError(` + ${adUnit.code} is a 'native' ad unit but contains non-native bidder(s) ${nonNativeBidders}. + No Prebid demand requests will be triggered for those bidders. + `); adUnit.bids = adUnit.bids.filter(nativeBidder); }); diff --git a/src/video.js b/src/video.js index ff890c5e24a..22255068cc0 100644 --- a/src/video.js +++ b/src/video.js @@ -9,13 +9,13 @@ const OUTSTREAM = 'outstream'; * Helper functions for working with video-enabled adUnits */ export const videoAdUnit = adUnit => { + const mediaType = adUnit.mediaType === VIDEO_MEDIA_TYPE; const mediaTypes = deepAccess(adUnit, 'mediaTypes.video'); - return !!mediaTypes || adUnit.mediaType === VIDEO_MEDIA_TYPE; + return mediaType || mediaTypes; }; - -const nonVideoBidder = bid => !videoAdapters.includes(bid.bidder); +export const videoBidder = bid => videoAdapters.includes(bid.bidder); export const hasNonVideoBidder = adUnit => - adUnit.bids.filter(nonVideoBidder).length; + adUnit.bids.filter(bid => !videoBidder(bid)).length; /** * @typedef {object} VideoBid diff --git a/test/spec/unit/pbjs_api_spec.js b/test/spec/unit/pbjs_api_spec.js index 37f6ef7ba34..5a91fe218ef 100644 --- a/test/spec/unit/pbjs_api_spec.js +++ b/test/spec/unit/pbjs_api_spec.js @@ -779,7 +779,7 @@ describe('Unit: Prebid Module', function () { adaptermanager.callBids.restore(); }); - it('should not callBids if a video adUnit has non-video bidders', () => { + it('should only request video bidders on video adunits', () => { sinon.spy(adaptermanager, 'callBids'); const videoAdaptersBackup = adaptermanager.videoAdapters; adaptermanager.videoAdapters = ['appnexusAst']; @@ -793,13 +793,17 @@ describe('Unit: Prebid Module', function () { }]; $$PREBID_GLOBAL$$.requestBids({adUnits}); - sinon.assert.notCalled(adaptermanager.callBids); + sinon.assert.calledOnce(adaptermanager.callBids); + + const spyArgs = adaptermanager.callBids.getCall(0); + const biddersCalled = spyArgs.args[0].adUnits[0].bids; + expect(biddersCalled.length).to.equal(1); adaptermanager.callBids.restore(); adaptermanager.videoAdapters = videoAdaptersBackup; }); - it('should not callBids if a video adUnit defined with mediaTypes has non-video bidders', () => { + it('should only request video bidders on video adunits configured with mediaTypes', () => { sinon.spy(adaptermanager, 'callBids'); const videoAdaptersBackup = adaptermanager.videoAdapters; adaptermanager.videoAdapters = ['appnexusAst']; @@ -813,7 +817,11 @@ describe('Unit: Prebid Module', function () { }]; $$PREBID_GLOBAL$$.requestBids({adUnits}); - sinon.assert.notCalled(adaptermanager.callBids); + sinon.assert.calledOnce(adaptermanager.callBids); + + const spyArgs = adaptermanager.callBids.getCall(0); + const biddersCalled = spyArgs.args[0].adUnits[0].bids; + expect(biddersCalled.length).to.equal(1); adaptermanager.callBids.restore(); adaptermanager.videoAdapters = videoAdaptersBackup;