From 9bc0faade205ec09b775d3c7fa3290513ce5c4ff Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Sun, 13 Aug 2017 23:00:02 -0700 Subject: [PATCH 01/14] Add support for video stream context --- src/adaptermanager.js | 10 +++++++--- src/bidmanager.js | 5 +++-- src/utils.js | 10 ++++++++++ src/video.js | 26 +++++++++++++++++++++++++- test/spec/bidmanager_spec.js | 23 +++++++++++++++++++++++ test/spec/utils_spec.js | 19 +++++++++++++++++++ 6 files changed, 87 insertions(+), 6 deletions(-) diff --git a/src/adaptermanager.js b/src/adaptermanager.js index d41ce0b3cef..63ce98ebbcd 100644 --- a/src/adaptermanager.js +++ b/src/adaptermanager.js @@ -1,6 +1,6 @@ /** @module adaptermanger */ -import { flatten, getBidderCodes, shuffle } from './utils'; +import { flatten, getBidderCodes, getDefinedParams, shuffle } from './utils'; import { mapSizes } from './sizeMapping'; import { processNativeAdUnitParams, nativeAdapters } from './native'; import { StorageManager, pbjsSyncsKey } from './storagemanager'; @@ -48,10 +48,14 @@ function getBids({bidderCode, requestId, bidderRequestId, adUnits}) { }); } + bid = Object.assign({}, bid, getDefinedParams(adUnit, [ + 'mediaType', + 'context', + 'renderer' + ])); + return Object.assign({}, bid, { placementCode: adUnit.code, - mediaType: adUnit.mediaType, - renderer: adUnit.renderer, transactionId: adUnit.transactionId, sizes: sizes, bidId: bid.bid_id || utils.getUniqueIdentifierStr(), diff --git a/src/bidmanager.js b/src/bidmanager.js index 386a0b05a08..60b82f601fb 100644 --- a/src/bidmanager.js +++ b/src/bidmanager.js @@ -1,6 +1,7 @@ import { uniques, flatten, adUnitsFilter, getBidderRequest } from './utils'; import { getPriceBucketString } from './cpmBucketManager'; import { NATIVE_KEYS, nativeBidIsValid } from './native'; +import { videoBidIsValid } from './video'; import { getCacheUrl, store } from './videoCache'; import { Renderer } from 'src/Renderer'; import { config } from 'src/config'; @@ -113,8 +114,8 @@ exports.addBidResponse = function (adUnitCode, bid) { utils.logError(errorMessage('Native bid missing some required properties.')); return false; } - if (bid.mediaType === 'video' && !(bid.vastUrl || bid.vastXml)) { - utils.logError(errorMessage(`Video bid has no vastUrl or vastXml property.`)); + if (bid.mediaType === 'video' && !videoBidIsValid(bid)) { + utils.logError(errorMessage(`Video bid does not have required vastUrl or renderer property`)); return false; } if (bid.mediaType === 'banner' && !validBidSize(bid)) { diff --git a/src/utils.js b/src/utils.js index ed249734661..d0684fded84 100644 --- a/src/utils.js +++ b/src/utils.js @@ -720,3 +720,13 @@ export function deepAccess(obj, path) { } return obj; } + +/* + * Build an object consisting of only defined parameters to avoid creating an + * object with defined keys and undefined values. + */ +export function getDefinedParams(object, params) { + return params + .filter(param => object[param]) + .reduce((bid, param) => Object.assign(bid, { [param]: object[param] }), {}); +} diff --git a/src/video.js b/src/video.js index eee1a9dbbd8..9c922958dbe 100644 --- a/src/video.js +++ b/src/video.js @@ -1,8 +1,32 @@ import { videoAdapters } from './adaptermanager'; +import { getBidRequest } from './utils'; + +const VIDEO_MEDIA_TYPE = 'video' +const INSTREAM = 'instream'; +const OUTSTREAM = 'outstream'; /** * Helper functions for working with video-enabled adUnits */ -export const videoAdUnit = adUnit => adUnit.mediaType === 'video'; +export const videoAdUnit = adUnit => adUnit.mediaType === VIDEO_MEDIA_TYPE; const nonVideoBidder = bid => !videoAdapters.includes(bid.bidder); export const hasNonVideoBidder = adUnit => adUnit.bids.filter(nonVideoBidder).length; + +/* + * Validate that the assets required for video context are present on the bid + */ +export function videoBidIsValid(bid) { + const bidRequest = getBidRequest(bid.adId); + + // if context not defined assume default 'instream' for video bids + if (!bidRequest || !bidRequest.context || bidRequest.context === INSTREAM) { + return !!(bid.vastUrl || bid.vastPayload); + } + + // outstream bids require a renderer on the bid or pub-defined on adunit + if (bidRequest.context === OUTSTREAM) { + return !!(bid.renderer || bidRequest.renderer); + } + + return true; +} diff --git a/test/spec/bidmanager_spec.js b/test/spec/bidmanager_spec.js index 150163aa43b..ee20ecd36d1 100644 --- a/test/spec/bidmanager_spec.js +++ b/test/spec/bidmanager_spec.js @@ -597,5 +597,28 @@ describe('bidmanager.js', function () { utils.getBidderRequest.restore(); }); + + it('requires a renderer on outstream bids', () => { + sinon.stub(utils, 'getBidRequest', () => ({ + bidder: 'appnexusAst', + mediaType: 'video', + context: 'outstream', + })); + + const bid = Object.assign({}, + bidfactory.createBid(1), + { + bidderCode: 'appnexusAst', + mediaType: 'video', + renderer: {render: () => true, url: 'render.js'}, + } + ); + + const bidsRecCount = $$PREBID_GLOBAL$$._bidsReceived.length; + bidmanager.addBidResponse('adUnit-code', bid); + assert.equal(bidsRecCount + 1, $$PREBID_GLOBAL$$._bidsReceived.length); + + utils.getBidRequest.restore(); + }); }); }); diff --git a/test/spec/utils_spec.js b/test/spec/utils_spec.js index b1837123449..e5a0b72d9b2 100755 --- a/test/spec/utils_spec.js +++ b/test/spec/utils_spec.js @@ -568,4 +568,23 @@ describe('Utils', function () { assert.equal(value, undefined); }); }); + + describe('getDefinedParams', () => { + it('builds an object consisting of defined params', () => { + const adUnit = { + mediaType: 'video', + comeWithMe: 'ifuwant2live', + notNeeded: 'do not include', + }; + + const builtObject = utils.getDefinedParams(adUnit, [ + 'mediaType', 'comeWithMe' + ]); + + assert.deepEqual(builtObject, { + mediaType: 'video', + comeWithMe: 'ifuwant2live', + }); + }); + }); }); From 7b5d7886ef384457067acdf08a99a999321dafc7 Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Mon, 14 Aug 2017 11:20:29 -0700 Subject: [PATCH 02/14] Define adapter as supporting video --- modules/unrulyBidAdapter.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/unrulyBidAdapter.js b/modules/unrulyBidAdapter.js index 997272cf9cc..5012ce7d821 100644 --- a/modules/unrulyBidAdapter.js +++ b/modules/unrulyBidAdapter.js @@ -106,6 +106,8 @@ function UnrulyAdapter() { return adapter } -adaptermanager.registerBidAdapter(new UnrulyAdapter(), 'unruly') +adaptermanager.registerBidAdapter(new UnrulyAdapter(), 'unruly', { + supportedMediaTypes: ['video'] +}); module.exports = UnrulyAdapter From 2bff8748b7e5dc0dc6bc670d6a61a2a38c8df5db Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Wed, 30 Aug 2017 10:06:32 -0700 Subject: [PATCH 03/14] Use mediaTypes param to specify context --- src/adaptermanager.js | 2 +- src/video.js | 11 ++++++++--- test/spec/bidmanager_spec.js | 5 +++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/adaptermanager.js b/src/adaptermanager.js index 63ce98ebbcd..9af18411f1d 100644 --- a/src/adaptermanager.js +++ b/src/adaptermanager.js @@ -50,7 +50,7 @@ function getBids({bidderCode, requestId, bidderRequestId, adUnits}) { bid = Object.assign({}, bid, getDefinedParams(adUnit, [ 'mediaType', - 'context', + 'mediaTypes', 'renderer' ])); diff --git a/src/video.js b/src/video.js index 9c922958dbe..91e65ef116f 100644 --- a/src/video.js +++ b/src/video.js @@ -1,7 +1,7 @@ import { videoAdapters } from './adaptermanager'; import { getBidRequest } from './utils'; -const VIDEO_MEDIA_TYPE = 'video' +const VIDEO_MEDIA_TYPE = 'video'; const INSTREAM = 'instream'; const OUTSTREAM = 'outstream'; @@ -17,14 +17,19 @@ export const hasNonVideoBidder = adUnit => adUnit.bids.filter(nonVideoBidder).le */ export function videoBidIsValid(bid) { const bidRequest = getBidRequest(bid.adId); + const context = + bidRequest && + bidRequest.mediaTypes && + bidRequest.mediaTypes.video && + bidRequest.mediaTypes.video.context; // if context not defined assume default 'instream' for video bids - if (!bidRequest || !bidRequest.context || bidRequest.context === INSTREAM) { + if (!bidRequest || !context || context === INSTREAM) { return !!(bid.vastUrl || bid.vastPayload); } // outstream bids require a renderer on the bid or pub-defined on adunit - if (bidRequest.context === OUTSTREAM) { + if (context === OUTSTREAM) { return !!(bid.renderer || bidRequest.renderer); } diff --git a/test/spec/bidmanager_spec.js b/test/spec/bidmanager_spec.js index ee20ecd36d1..f736a0b1bb0 100644 --- a/test/spec/bidmanager_spec.js +++ b/test/spec/bidmanager_spec.js @@ -601,8 +601,9 @@ describe('bidmanager.js', function () { it('requires a renderer on outstream bids', () => { sinon.stub(utils, 'getBidRequest', () => ({ bidder: 'appnexusAst', - mediaType: 'video', - context: 'outstream', + mediaTypes: { + video: {context: 'outstream'} + }, })); const bid = Object.assign({}, From ac8fea92918e724aeab63514ba1fde906e0fd548 Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Wed, 30 Aug 2017 15:05:44 -0700 Subject: [PATCH 04/14] Use utils.deepAccess --- src/video.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/video.js b/src/video.js index 91e65ef116f..da1d9eb0356 100644 --- a/src/video.js +++ b/src/video.js @@ -1,5 +1,5 @@ import { videoAdapters } from './adaptermanager'; -import { getBidRequest } from './utils'; +import { getBidRequest, deepAccess } from './utils'; const VIDEO_MEDIA_TYPE = 'video'; const INSTREAM = 'instream'; @@ -10,7 +10,8 @@ const OUTSTREAM = 'outstream'; */ export const videoAdUnit = adUnit => adUnit.mediaType === VIDEO_MEDIA_TYPE; const nonVideoBidder = bid => !videoAdapters.includes(bid.bidder); -export const hasNonVideoBidder = adUnit => adUnit.bids.filter(nonVideoBidder).length; +export const hasNonVideoBidder = adUnit => + adUnit.bids.filter(nonVideoBidder).length; /* * Validate that the assets required for video context are present on the bid @@ -18,10 +19,7 @@ export const hasNonVideoBidder = adUnit => adUnit.bids.filter(nonVideoBidder).le export function videoBidIsValid(bid) { const bidRequest = getBidRequest(bid.adId); const context = - bidRequest && - bidRequest.mediaTypes && - bidRequest.mediaTypes.video && - bidRequest.mediaTypes.video.context; + bidRequest && deepAccess(bidRequest, 'mediaTypes.video.context'); // if context not defined assume default 'instream' for video bids if (!bidRequest || !context || context === INSTREAM) { From c581d4a84bc098e08b1078ec8c7409b40a07a0a7 Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Wed, 30 Aug 2017 15:14:45 -0700 Subject: [PATCH 05/14] Check for outstream bids --- modules/unrulyBidAdapter.js | 5 +++++ test/spec/modules/unrulyBidAdapter_spec.js | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/modules/unrulyBidAdapter.js b/modules/unrulyBidAdapter.js index 5012ce7d821..7e55adc85c2 100644 --- a/modules/unrulyBidAdapter.js +++ b/modules/unrulyBidAdapter.js @@ -85,6 +85,11 @@ function UnrulyAdapter() { return } + const context = utils.deepAccess(bidRequestBids[0], 'mediaTypes.video.context') + if (context !== 'outstream') { + return + } + const payload = { bidRequests: bidRequestBids } diff --git a/test/spec/modules/unrulyBidAdapter_spec.js b/test/spec/modules/unrulyBidAdapter_spec.js index dfa7a72b8ad..067f3ea46d0 100644 --- a/test/spec/modules/unrulyBidAdapter_spec.js +++ b/test/spec/modules/unrulyBidAdapter_spec.js @@ -17,7 +17,7 @@ describe('UnrulyAdapter', () => { 'placementId': '5768085' }, 'placementCode': placementCode, - 'mediaType': 'video', + 'mediaTypes': { video: { context: 'outstream' } }, 'transactionId': '62890707-3770-497c-a3b8-d905a2d0cb98', 'sizes': [ 640, From 81297c19c276fd98904aa647bd415f3d7e4942ec Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Wed, 30 Aug 2017 16:32:30 -0700 Subject: [PATCH 06/14] Add JSDoc and validation --- src/adaptermanager.js | 9 ++++++++- src/utils.js | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/adaptermanager.js b/src/adaptermanager.js index 9af18411f1d..fc5ca111546 100644 --- a/src/adaptermanager.js +++ b/src/adaptermanager.js @@ -48,9 +48,16 @@ function getBids({bidderCode, requestId, bidderRequestId, adUnits}) { }); } + if (adUnit.mediaTypes) { + if (utils.mediaTypesValid(adUnit.mediaTypes)) { + bid = Object.assign({}, bid, { mediaTypes: adUnit.mediaTypes }); + } else { + utils.logError(`mediaTypes is not correctly configured`); + } + } + bid = Object.assign({}, bid, getDefinedParams(adUnit, [ 'mediaType', - 'mediaTypes', 'renderer' ])); diff --git a/src/utils.js b/src/utils.js index d0684fded84..c456aece8dc 100644 --- a/src/utils.js +++ b/src/utils.js @@ -721,12 +721,44 @@ export function deepAccess(obj, path) { return obj; } -/* +/** * Build an object consisting of only defined parameters to avoid creating an * object with defined keys and undefined values. + * @param {object} object The object to pick defined params out of + * @param {string[]} params An array of strings representing properties to look for in the object + * @returns {object} An object containing all the specified values that are defined */ export function getDefinedParams(object, params) { return params .filter(param => object[param]) .reduce((bid, param) => Object.assign(bid, { [param]: object[param] }), {}); } + +/** + * @typedef {Object} MediaTypes + * @property {Object} banner banner configuration + * @property {Object} native native configuration + * @property {Object} video video configuration + */ + +/** + * Validates an adunit's `mediaTypes` parameter + * @param{MediaTypes} mediaTypes mediaTypes parameter to validate + * @returns{boolean} If object is valie + */ +export function mediaTypesValid(mediaTypes) { + const SUPPORTED_MEDIA_TYPES = ['banner', 'native', 'video']; + const SUPPORTED_STREAM_TYPES = ['instream', 'outstream']; + + const types = Object.keys(mediaTypes); + + if (!types.every(type => SUPPORTED_MEDIA_TYPES.includes(type))) { + return false; + } + + if (mediaTypes.video && mediaTypes.video.context) { + return SUPPORTED_STREAM_TYPES.includes(mediaTypes.video.context); + } + + return true; +} From 3633165e9ebdb9bd4a91eca20d938596641f3074 Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Tue, 5 Sep 2017 12:06:35 -0700 Subject: [PATCH 07/14] Rename functions and add unit test --- src/adaptermanager.js | 2 +- src/bidmanager.js | 4 +-- src/utils.js | 6 ++-- src/video.js | 11 +++++-- test/spec/video_spec.js | 70 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 85 insertions(+), 8 deletions(-) create mode 100644 test/spec/video_spec.js diff --git a/src/adaptermanager.js b/src/adaptermanager.js index fc5ca111546..eeffe9a994f 100644 --- a/src/adaptermanager.js +++ b/src/adaptermanager.js @@ -49,7 +49,7 @@ function getBids({bidderCode, requestId, bidderRequestId, adUnits}) { } if (adUnit.mediaTypes) { - if (utils.mediaTypesValid(adUnit.mediaTypes)) { + if (utils.isValidMediaTypes(adUnit.mediaTypes)) { bid = Object.assign({}, bid, { mediaTypes: adUnit.mediaTypes }); } else { utils.logError(`mediaTypes is not correctly configured`); diff --git a/src/bidmanager.js b/src/bidmanager.js index 60b82f601fb..9b9f565c345 100644 --- a/src/bidmanager.js +++ b/src/bidmanager.js @@ -1,7 +1,7 @@ import { uniques, flatten, adUnitsFilter, getBidderRequest } from './utils'; import { getPriceBucketString } from './cpmBucketManager'; import { NATIVE_KEYS, nativeBidIsValid } from './native'; -import { videoBidIsValid } from './video'; +import { isValidVideoBid } from './video'; import { getCacheUrl, store } from './videoCache'; import { Renderer } from 'src/Renderer'; import { config } from 'src/config'; @@ -114,7 +114,7 @@ exports.addBidResponse = function (adUnitCode, bid) { utils.logError(errorMessage('Native bid missing some required properties.')); return false; } - if (bid.mediaType === 'video' && !videoBidIsValid(bid)) { + if (bid.mediaType === 'video' && !isValidVideoBid(bid)) { utils.logError(errorMessage(`Video bid does not have required vastUrl or renderer property`)); return false; } diff --git a/src/utils.js b/src/utils.js index c456aece8dc..7b3ad3cda21 100644 --- a/src/utils.js +++ b/src/utils.js @@ -743,10 +743,10 @@ export function getDefinedParams(object, params) { /** * Validates an adunit's `mediaTypes` parameter - * @param{MediaTypes} mediaTypes mediaTypes parameter to validate - * @returns{boolean} If object is valie + * @param {MediaTypes} mediaTypes mediaTypes parameter to validate + * @return {boolean} If object is valid */ -export function mediaTypesValid(mediaTypes) { +export function isValidMediaTypes(mediaTypes) { const SUPPORTED_MEDIA_TYPES = ['banner', 'native', 'video']; const SUPPORTED_STREAM_TYPES = ['instream', 'outstream']; diff --git a/src/video.js b/src/video.js index da1d9eb0356..3cc899c2a74 100644 --- a/src/video.js +++ b/src/video.js @@ -13,10 +13,17 @@ const nonVideoBidder = bid => !videoAdapters.includes(bid.bidder); export const hasNonVideoBidder = adUnit => adUnit.bids.filter(nonVideoBidder).length; -/* +/** + * @typedef {object} VideoBid + * @property {string} adId id of the bid + */ + +/** * Validate that the assets required for video context are present on the bid + * @param {VideoBid} bid video bid to validate + * @return {boolean} If object is valid */ -export function videoBidIsValid(bid) { +export function isValidVideoBid(bid) { const bidRequest = getBidRequest(bid.adId); const context = bidRequest && deepAccess(bidRequest, 'mediaTypes.video.context'); diff --git a/test/spec/video_spec.js b/test/spec/video_spec.js new file mode 100644 index 00000000000..49326a4b439 --- /dev/null +++ b/test/spec/video_spec.js @@ -0,0 +1,70 @@ +import { isValidVideoBid } from 'src/video'; +const utils = require('src/utils'); + +describe('video.js', () => { + it('validates valid instream bids', () => { + sinon.stub(utils, 'getBidRequest', () => ({ + bidder: 'appnexusAst', + mediaTypes: { + video: { context: 'instream' }, + }, + })); + + const valid = isValidVideoBid({ + vastUrl: 'http://www.example.com/vastUrl' + }); + + expect(valid).to.be(true); + + utils.getBidRequest.restore(); + }); + + it('catches invalid instream bids', () => { + sinon.stub(utils, 'getBidRequest', () => ({ + bidder: 'appnexusAst', + mediaTypes: { + video: { context: 'instream' }, + }, + })); + + const valid = isValidVideoBid({}); + + expect(valid).to.be(false); + + utils.getBidRequest.restore(); + }); + + it('validates valid outstream bids', () => { + sinon.stub(utils, 'getBidRequest', () => ({ + bidder: 'appnexusAst', + mediaTypes: { + video: { context: 'outstream' }, + }, + renderer: { + url: 'render.url', + render: () => true, + } + })); + + const valid = isValidVideoBid({}); + + expect(valid).to.be(true); + + utils.getBidRequest.restore(); + }); + + it('catches invalid outstream bids', () => { + sinon.stub(utils, 'getBidRequest', () => ({ + bidder: 'appnexusAst', + mediaTypes: { + video: { context: 'outstream' }, + }, + })); + + const valid = isValidVideoBid({}); + + expect(valid).to.be(false); + + utils.getBidRequest.restore(); + }); +}); From 140675a353573098ea115e09a5fa2e58c65920bd Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Tue, 5 Sep 2017 12:09:49 -0700 Subject: [PATCH 08/14] Update property name --- src/video.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/video.js b/src/video.js index 3cc899c2a74..4a1cb01ef4d 100644 --- a/src/video.js +++ b/src/video.js @@ -30,7 +30,7 @@ export function isValidVideoBid(bid) { // if context not defined assume default 'instream' for video bids if (!bidRequest || !context || context === INSTREAM) { - return !!(bid.vastUrl || bid.vastPayload); + return !!(bid.vastUrl || bid.vastXml); } // outstream bids require a renderer on the bid or pub-defined on adunit From c9c366e537749eb062009cee1362ef67c2ff5bab Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Tue, 5 Sep 2017 12:24:44 -0700 Subject: [PATCH 09/14] Update stubs to new sinon stub syntax --- test/spec/bidmanager_spec.js | 4 +--- test/spec/video_spec.js | 26 +++++++++++--------------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/test/spec/bidmanager_spec.js b/test/spec/bidmanager_spec.js index f736a0b1bb0..8099de73b51 100644 --- a/test/spec/bidmanager_spec.js +++ b/test/spec/bidmanager_spec.js @@ -599,7 +599,7 @@ describe('bidmanager.js', function () { }); it('requires a renderer on outstream bids', () => { - sinon.stub(utils, 'getBidRequest', () => ({ + getSandbox().stub(utils, 'getBidRequest').callsFake(() => ({ bidder: 'appnexusAst', mediaTypes: { video: {context: 'outstream'} @@ -618,8 +618,6 @@ describe('bidmanager.js', function () { const bidsRecCount = $$PREBID_GLOBAL$$._bidsReceived.length; bidmanager.addBidResponse('adUnit-code', bid); assert.equal(bidsRecCount + 1, $$PREBID_GLOBAL$$._bidsReceived.length); - - utils.getBidRequest.restore(); }); }); }); diff --git a/test/spec/video_spec.js b/test/spec/video_spec.js index 49326a4b439..cede9239f9c 100644 --- a/test/spec/video_spec.js +++ b/test/spec/video_spec.js @@ -1,9 +1,12 @@ +import useSandbox from 'test/mocks/sandbox'; import { isValidVideoBid } from 'src/video'; const utils = require('src/utils'); describe('video.js', () => { + const getSandbox = useSandbox() + it('validates valid instream bids', () => { - sinon.stub(utils, 'getBidRequest', () => ({ + getSandbox().stub(utils, 'getBidRequest').callsFake(() => ({ bidder: 'appnexusAst', mediaTypes: { video: { context: 'instream' }, @@ -15,12 +18,10 @@ describe('video.js', () => { }); expect(valid).to.be(true); - - utils.getBidRequest.restore(); }); it('catches invalid instream bids', () => { - sinon.stub(utils, 'getBidRequest', () => ({ + getSandbox().stub(utils, 'getBidRequest').callsFake(() => ({ bidder: 'appnexusAst', mediaTypes: { video: { context: 'instream' }, @@ -30,31 +31,28 @@ describe('video.js', () => { const valid = isValidVideoBid({}); expect(valid).to.be(false); - - utils.getBidRequest.restore(); }); it('validates valid outstream bids', () => { - sinon.stub(utils, 'getBidRequest', () => ({ + getSandbox().stub(utils, 'getBidRequest').callsFake(() => ({ bidder: 'appnexusAst', mediaTypes: { video: { context: 'outstream' }, }, + })); + + const valid = isValidVideoBid({ renderer: { url: 'render.url', render: () => true, } - })); - - const valid = isValidVideoBid({}); + }); expect(valid).to.be(true); - - utils.getBidRequest.restore(); }); it('catches invalid outstream bids', () => { - sinon.stub(utils, 'getBidRequest', () => ({ + getSandbox().stub(utils, 'getBidRequest').callsFake(() => ({ bidder: 'appnexusAst', mediaTypes: { video: { context: 'outstream' }, @@ -64,7 +62,5 @@ describe('video.js', () => { const valid = isValidVideoBid({}); expect(valid).to.be(false); - - utils.getBidRequest.restore(); }); }); From 97cd71f66ac714f4b394d46302ead995dd43e75f Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Tue, 5 Sep 2017 13:09:42 -0700 Subject: [PATCH 10/14] Only check context when mediaTypes.video was defined --- modules/appnexusAstBidAdapter.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/modules/appnexusAstBidAdapter.js b/modules/appnexusAstBidAdapter.js index 7da6bf0285b..c56113164dc 100644 --- a/modules/appnexusAstBidAdapter.js +++ b/modules/appnexusAstBidAdapter.js @@ -289,7 +289,13 @@ function bidToTag(bid) { } } - if (bid.mediaType === 'video') { tag.require_asset_url = true; } + const videoMediaType = utils.deepAccess(bid, 'mediaTypes.video'); + const context = utils.deepAccess(bid, 'mediaTypes.video.context'); + + if (bid.mediaType === 'video' || (videoMediaType && context !== 'outstream')) { + tag.require_asset_url = true; + } + if (bid.params.video) { tag.video = {}; // place any valid video params on the tag From ccfc7eee70e1f0334b864351b5e47ffe87d4b36b Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Thu, 7 Sep 2017 15:15:11 -0700 Subject: [PATCH 11/14] Retain video-outstream compatibility --- modules/unrulyBidAdapter.js | 3 ++- src/video.js | 10 ++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/modules/unrulyBidAdapter.js b/modules/unrulyBidAdapter.js index 7e55adc85c2..fd9e94859c9 100644 --- a/modules/unrulyBidAdapter.js +++ b/modules/unrulyBidAdapter.js @@ -85,8 +85,9 @@ function UnrulyAdapter() { return } + const videoMediaType = utils.deepAccess(bidRequestBids[0], 'mediaTypes.video') const context = utils.deepAccess(bidRequestBids[0], 'mediaTypes.video.context') - if (context !== 'outstream') { + if (videoMediaType && context !== 'outstream') { return } diff --git a/src/video.js b/src/video.js index 4a1cb01ef4d..386b6b692e9 100644 --- a/src/video.js +++ b/src/video.js @@ -2,7 +2,6 @@ import { videoAdapters } from './adaptermanager'; import { getBidRequest, deepAccess } from './utils'; const VIDEO_MEDIA_TYPE = 'video'; -const INSTREAM = 'instream'; const OUTSTREAM = 'outstream'; /** @@ -25,11 +24,14 @@ export const hasNonVideoBidder = adUnit => */ export function isValidVideoBid(bid) { const bidRequest = getBidRequest(bid.adId); - const context = - bidRequest && deepAccess(bidRequest, 'mediaTypes.video.context'); + + const videoMediaType = + bidRequest && deepAccess(bidRequest, 'mediaTypes.video'); + const context = videoMediaType && deepAccess(videoMediaType, 'context'); // if context not defined assume default 'instream' for video bids - if (!bidRequest || !context || context === INSTREAM) { + // instream bids require a vast url or vast xml content + if (!bidRequest || (videoMediaType && context !== OUTSTREAM)) { return !!(bid.vastUrl || bid.vastXml); } From 0c8c9f1e5c14bcc25b539ba12000dacda7dbb8a4 Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Fri, 8 Sep 2017 12:04:10 -0700 Subject: [PATCH 12/14] Revert to Sinon 1 syntax --- test/spec/bidmanager_spec.js | 4 +++- test/spec/video_spec.js | 19 ++++++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/test/spec/bidmanager_spec.js b/test/spec/bidmanager_spec.js index 8099de73b51..f736a0b1bb0 100644 --- a/test/spec/bidmanager_spec.js +++ b/test/spec/bidmanager_spec.js @@ -599,7 +599,7 @@ describe('bidmanager.js', function () { }); it('requires a renderer on outstream bids', () => { - getSandbox().stub(utils, 'getBidRequest').callsFake(() => ({ + sinon.stub(utils, 'getBidRequest', () => ({ bidder: 'appnexusAst', mediaTypes: { video: {context: 'outstream'} @@ -618,6 +618,8 @@ describe('bidmanager.js', function () { const bidsRecCount = $$PREBID_GLOBAL$$._bidsReceived.length; bidmanager.addBidResponse('adUnit-code', bid); assert.equal(bidsRecCount + 1, $$PREBID_GLOBAL$$._bidsReceived.length); + + utils.getBidRequest.restore(); }); }); }); diff --git a/test/spec/video_spec.js b/test/spec/video_spec.js index cede9239f9c..9c9503e0010 100644 --- a/test/spec/video_spec.js +++ b/test/spec/video_spec.js @@ -1,12 +1,9 @@ -import useSandbox from 'test/mocks/sandbox'; import { isValidVideoBid } from 'src/video'; const utils = require('src/utils'); describe('video.js', () => { - const getSandbox = useSandbox() - it('validates valid instream bids', () => { - getSandbox().stub(utils, 'getBidRequest').callsFake(() => ({ + sinon.stub(utils, 'getBidRequest', () => ({ bidder: 'appnexusAst', mediaTypes: { video: { context: 'instream' }, @@ -18,10 +15,12 @@ describe('video.js', () => { }); expect(valid).to.be(true); + + utils.getBidRequest.restore(); }); it('catches invalid instream bids', () => { - getSandbox().stub(utils, 'getBidRequest').callsFake(() => ({ + sinon.stub(utils, 'getBidRequest', () => ({ bidder: 'appnexusAst', mediaTypes: { video: { context: 'instream' }, @@ -31,10 +30,12 @@ describe('video.js', () => { const valid = isValidVideoBid({}); expect(valid).to.be(false); + + utils.getBidRequest.restore(); }); it('validates valid outstream bids', () => { - getSandbox().stub(utils, 'getBidRequest').callsFake(() => ({ + sinon.stub(utils, 'getBidRequest', () => ({ bidder: 'appnexusAst', mediaTypes: { video: { context: 'outstream' }, @@ -49,10 +50,12 @@ describe('video.js', () => { }); expect(valid).to.be(true); + + utils.getBidRequest.restore(); }); it('catches invalid outstream bids', () => { - getSandbox().stub(utils, 'getBidRequest').callsFake(() => ({ + sinon.stub(utils, 'getBidRequest', () => ({ bidder: 'appnexusAst', mediaTypes: { video: { context: 'outstream' }, @@ -62,5 +65,7 @@ describe('video.js', () => { const valid = isValidVideoBid({}); expect(valid).to.be(false); + + utils.getBidRequest.restore(); }); }); From 94bd7f215ed303f1126c87fbb33f8fdcb9a95cd7 Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Wed, 13 Sep 2017 15:36:13 -0700 Subject: [PATCH 13/14] Server and bid response ad type for any stream type is always 'video' --- modules/appnexusAstBidAdapter.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/modules/appnexusAstBidAdapter.js b/modules/appnexusAstBidAdapter.js index c56113164dc..9a7776a8bb5 100644 --- a/modules/appnexusAstBidAdapter.js +++ b/modules/appnexusAstBidAdapter.js @@ -5,7 +5,7 @@ import { NATIVE, VIDEO } from 'src/mediaTypes'; const BIDDER_CODE = 'appnexusAst'; const URL = '//ib.adnxs.com/ut/v3/prebid'; -const SUPPORTED_AD_TYPES = ['banner', 'video', 'video-outstream', 'native']; +const SUPPORTED_AD_TYPES = ['banner', 'video', 'native']; const VIDEO_TARGETING = ['id', 'mimes', 'minduration', 'maxduration', 'startdelay', 'skippable', 'playback_method', 'frameworks']; const USER_PARAMS = ['age', 'external_uid', 'segments', 'gender', 'dnt', 'language']; @@ -218,6 +218,7 @@ function newBid(serverBid, rtbBid) { return bid; } + function bidToTag(bid) { const tag = {}; tag.sizes = transformSizes(bid.sizes); @@ -362,9 +363,7 @@ function handleOutstreamRendererEvents(bid, id, eventName) { function parseMediaType(rtbBid) { const adType = rtbBid.ad_type; - if (rtbBid.renderer_url) { - return 'video-outstream'; - } else if (adType === 'video') { + if (adType === 'video') { return 'video'; } else if (adType === 'native') { return 'native'; From 673dfef4bc4cb78707b4256b71e030cc41df6bdc Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Fri, 15 Sep 2017 09:00:35 -0700 Subject: [PATCH 14/14] Update to address code review --- src/adaptermanager.js | 4 +++- test/spec/video_spec.js | 12 ++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/adaptermanager.js b/src/adaptermanager.js index eeffe9a994f..38b7073478a 100644 --- a/src/adaptermanager.js +++ b/src/adaptermanager.js @@ -52,7 +52,9 @@ function getBids({bidderCode, requestId, bidderRequestId, adUnits}) { if (utils.isValidMediaTypes(adUnit.mediaTypes)) { bid = Object.assign({}, bid, { mediaTypes: adUnit.mediaTypes }); } else { - utils.logError(`mediaTypes is not correctly configured`); + utils.logError( + `mediaTypes is not correctly configured for adunit ${adUnit.code}` + ); } } diff --git a/test/spec/video_spec.js b/test/spec/video_spec.js index 9c9503e0010..57a7f7a127e 100644 --- a/test/spec/video_spec.js +++ b/test/spec/video_spec.js @@ -2,6 +2,10 @@ import { isValidVideoBid } from 'src/video'; const utils = require('src/utils'); describe('video.js', () => { + afterEach(() => { + utils.getBidRequest.restore(); + }); + it('validates valid instream bids', () => { sinon.stub(utils, 'getBidRequest', () => ({ bidder: 'appnexusAst', @@ -15,8 +19,6 @@ describe('video.js', () => { }); expect(valid).to.be(true); - - utils.getBidRequest.restore(); }); it('catches invalid instream bids', () => { @@ -30,8 +32,6 @@ describe('video.js', () => { const valid = isValidVideoBid({}); expect(valid).to.be(false); - - utils.getBidRequest.restore(); }); it('validates valid outstream bids', () => { @@ -50,8 +50,6 @@ describe('video.js', () => { }); expect(valid).to.be(true); - - utils.getBidRequest.restore(); }); it('catches invalid outstream bids', () => { @@ -65,7 +63,5 @@ describe('video.js', () => { const valid = isValidVideoBid({}); expect(valid).to.be(false); - - utils.getBidRequest.restore(); }); });