From 9fc64abde8a4cc56f5b557de13f54c3780b9383d Mon Sep 17 00:00:00 2001 From: John Salis Date: Wed, 11 Oct 2017 14:58:34 -0400 Subject: [PATCH 1/5] Fix issue with video bid validation --- src/video.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/video.js b/src/video.js index 386b6b692e9..5b94fdc8a7b 100644 --- a/src/video.js +++ b/src/video.js @@ -1,5 +1,6 @@ import { videoAdapters } from './adaptermanager'; import { getBidRequest, deepAccess } from './utils'; +import { auctionManager } from './auctionManager'; const VIDEO_MEDIA_TYPE = 'video'; const OUTSTREAM = 'outstream'; @@ -23,7 +24,7 @@ export const hasNonVideoBidder = adUnit => * @return {boolean} If object is valid */ export function isValidVideoBid(bid) { - const bidRequest = getBidRequest(bid.adId); + const bidRequest = getBidRequest(bid.adId, auctionManager.getBidsRequested()); const videoMediaType = bidRequest && deepAccess(bidRequest, 'mediaTypes.video'); From 3f9f4552bca356ab417196538f053dded63e640b Mon Sep 17 00:00:00 2001 From: John Salis Date: Wed, 11 Oct 2017 18:19:50 -0400 Subject: [PATCH 2/5] Modified tests to stub `auctionManager.getBidsRequested` instead of `getBidRequest` --- test/spec/video_spec.js | 82 +++++++++++++++++++++++++++-------------- 1 file changed, 54 insertions(+), 28 deletions(-) diff --git a/test/spec/video_spec.js b/test/spec/video_spec.js index 57a7f7a127e..747162816df 100644 --- a/test/spec/video_spec.js +++ b/test/spec/video_spec.js @@ -1,20 +1,26 @@ import { isValidVideoBid } from 'src/video'; -const utils = require('src/utils'); +import { auctionManager } from 'src/auctionManager'; describe('video.js', () => { afterEach(() => { - utils.getBidRequest.restore(); + auctionManager.getBidsRequested.restore(); }); it('validates valid instream bids', () => { - sinon.stub(utils, 'getBidRequest', () => ({ - bidder: 'appnexusAst', - mediaTypes: { - video: { context: 'instream' }, - }, - })); + sinon.stub(auctionManager, 'getBidsRequested', () => [ + { + bids: [{ + bidId: '123abc', + bidder: 'appnexusAst', + mediaTypes: { + video: { context: 'instream' } + } + }] + } + ]); const valid = isValidVideoBid({ + adId: '123abc', vastUrl: 'http://www.example.com/vastUrl' }); @@ -22,27 +28,40 @@ describe('video.js', () => { }); it('catches invalid instream bids', () => { - sinon.stub(utils, 'getBidRequest', () => ({ - bidder: 'appnexusAst', - mediaTypes: { - video: { context: 'instream' }, - }, - })); + sinon.stub(auctionManager, 'getBidsRequested', () => [ + { + bids: [{ + bidId: '123abc', + bidder: 'appnexusAst', + mediaTypes: { + video: { context: 'instream' } + } + }] + } + ]); - const valid = isValidVideoBid({}); + const valid = isValidVideoBid({ + adId: '123abc' + }); expect(valid).to.be(false); }); it('validates valid outstream bids', () => { - sinon.stub(utils, 'getBidRequest', () => ({ - bidder: 'appnexusAst', - mediaTypes: { - video: { context: 'outstream' }, - }, - })); + sinon.stub(auctionManager, 'getBidsRequested', () => [ + { + bids: [{ + bidId: '123abc', + bidder: 'appnexusAst', + mediaTypes: { + video: { context: 'outstream' } + } + }] + } + ]); const valid = isValidVideoBid({ + adId: '123abc', renderer: { url: 'render.url', render: () => true, @@ -53,14 +72,21 @@ describe('video.js', () => { }); it('catches invalid outstream bids', () => { - sinon.stub(utils, 'getBidRequest', () => ({ - bidder: 'appnexusAst', - mediaTypes: { - video: { context: 'outstream' }, - }, - })); + sinon.stub(auctionManager, 'getBidsRequested', () => [ + { + bids: [{ + bidId: '123abc', + bidder: 'appnexusAst', + mediaTypes: { + video: { context: 'outstream' } + } + }] + } + ]); - const valid = isValidVideoBid({}); + const valid = isValidVideoBid({ + adId: '123abc' + }); expect(valid).to.be(false); }); From e5ca9ea1a3ce2d8d0435b67770fd0dcc9f9c27ae Mon Sep 17 00:00:00 2001 From: John Salis Date: Mon, 16 Oct 2017 12:06:21 -0400 Subject: [PATCH 3/5] Move stub to beforeEach hook --- test/spec/video_spec.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/spec/video_spec.js b/test/spec/video_spec.js index 747162816df..20e9da4dd2a 100644 --- a/test/spec/video_spec.js +++ b/test/spec/video_spec.js @@ -2,12 +2,16 @@ import { isValidVideoBid } from 'src/video'; import { auctionManager } from 'src/auctionManager'; describe('video.js', () => { + beforeEach(() => { + sinon.stub(auctionManager, 'getBidsRequested'); + }); + afterEach(() => { auctionManager.getBidsRequested.restore(); }); it('validates valid instream bids', () => { - sinon.stub(auctionManager, 'getBidsRequested', () => [ + auctionManager.getBidsRequested.returns([ { bids: [{ bidId: '123abc', @@ -28,7 +32,7 @@ describe('video.js', () => { }); it('catches invalid instream bids', () => { - sinon.stub(auctionManager, 'getBidsRequested', () => [ + auctionManager.getBidsRequested.returns([ { bids: [{ bidId: '123abc', @@ -48,7 +52,7 @@ describe('video.js', () => { }); it('validates valid outstream bids', () => { - sinon.stub(auctionManager, 'getBidsRequested', () => [ + auctionManager.getBidsRequested.returns([ { bids: [{ bidId: '123abc', @@ -72,7 +76,7 @@ describe('video.js', () => { }); it('catches invalid outstream bids', () => { - sinon.stub(auctionManager, 'getBidsRequested', () => [ + auctionManager.getBidsRequested.returns([ { bids: [{ bidId: '123abc', From 6877b895d2eba825e09b90338553fa902176b55a Mon Sep 17 00:00:00 2001 From: John Salis Date: Mon, 16 Oct 2017 12:08:04 -0400 Subject: [PATCH 4/5] Fix lint errors --- test/spec/video_spec.js | 104 ++++++++++++++++++++-------------------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/test/spec/video_spec.js b/test/spec/video_spec.js index 20e9da4dd2a..7fdbdcbae9f 100644 --- a/test/spec/video_spec.js +++ b/test/spec/video_spec.js @@ -2,29 +2,29 @@ import { isValidVideoBid } from 'src/video'; import { auctionManager } from 'src/auctionManager'; describe('video.js', () => { - beforeEach(() => { - sinon.stub(auctionManager, 'getBidsRequested'); - }); + beforeEach(() => { + sinon.stub(auctionManager, 'getBidsRequested'); + }); afterEach(() => { - auctionManager.getBidsRequested.restore(); + auctionManager.getBidsRequested.restore(); }); it('validates valid instream bids', () => { - auctionManager.getBidsRequested.returns([ - { - bids: [{ - bidId: '123abc', - bidder: 'appnexusAst', - mediaTypes: { - video: { context: 'instream' } - } - }] - } - ]); + auctionManager.getBidsRequested.returns([ + { + bids: [{ + bidId: '123abc', + bidder: 'appnexusAst', + mediaTypes: { + video: { context: 'instream' } + } + }] + } + ]); const valid = isValidVideoBid({ - adId: '123abc', + adId: '123abc', vastUrl: 'http://www.example.com/vastUrl' }); @@ -32,40 +32,40 @@ describe('video.js', () => { }); it('catches invalid instream bids', () => { - auctionManager.getBidsRequested.returns([ - { - bids: [{ - bidId: '123abc', - bidder: 'appnexusAst', - mediaTypes: { - video: { context: 'instream' } - } - }] - } - ]); + auctionManager.getBidsRequested.returns([ + { + bids: [{ + bidId: '123abc', + bidder: 'appnexusAst', + mediaTypes: { + video: { context: 'instream' } + } + }] + } + ]); const valid = isValidVideoBid({ - adId: '123abc' + adId: '123abc' }); expect(valid).to.be(false); }); it('validates valid outstream bids', () => { - auctionManager.getBidsRequested.returns([ - { - bids: [{ - bidId: '123abc', - bidder: 'appnexusAst', - mediaTypes: { - video: { context: 'outstream' } - } - }] - } - ]); + auctionManager.getBidsRequested.returns([ + { + bids: [{ + bidId: '123abc', + bidder: 'appnexusAst', + mediaTypes: { + video: { context: 'outstream' } + } + }] + } + ]); const valid = isValidVideoBid({ - adId: '123abc', + adId: '123abc', renderer: { url: 'render.url', render: () => true, @@ -76,20 +76,20 @@ describe('video.js', () => { }); it('catches invalid outstream bids', () => { - auctionManager.getBidsRequested.returns([ - { - bids: [{ - bidId: '123abc', - bidder: 'appnexusAst', - mediaTypes: { - video: { context: 'outstream' } - } - }] - } - ]); + auctionManager.getBidsRequested.returns([ + { + bids: [{ + bidId: '123abc', + bidder: 'appnexusAst', + mediaTypes: { + video: { context: 'outstream' } + } + }] + } + ]); const valid = isValidVideoBid({ - adId: '123abc' + adId: '123abc' }); expect(valid).to.be(false); From a29ece54848c1d5ce94c14353df637fae42499ed Mon Sep 17 00:00:00 2001 From: John Salis Date: Tue, 17 Oct 2017 15:23:08 -0400 Subject: [PATCH 5/5] Add bidRequests param to bid validation --- src/auction.js | 4 +- src/native.js | 10 ++-- src/video.js | 10 ++-- test/spec/video_spec.js | 117 +++++++++++++++++----------------------- 4 files changed, 61 insertions(+), 80 deletions(-) diff --git a/src/auction.js b/src/auction.js index 9a782b71549..f327e5746f6 100644 --- a/src/auction.js +++ b/src/auction.js @@ -202,11 +202,11 @@ function newAuction({adUnits, adUnitCodes}) { utils.logWarn(`Some adapter tried to add an undefined bid for ${adUnitCode}.`); return false; } - if (bid.mediaType === 'native' && !nativeBidIsValid(bid)) { + if (bid.mediaType === 'native' && !nativeBidIsValid(bid, _bidderRequests)) { utils.logError(errorMessage('Native bid missing some required properties.')); return false; } - if (bid.mediaType === 'video' && !isValidVideoBid(bid)) { + if (bid.mediaType === 'video' && !isValidVideoBid(bid, _bidderRequests)) { utils.logError(errorMessage(`Video bid does not have required vastUrl or renderer property`)); return false; } diff --git a/src/native.js b/src/native.js index 4486f2241bf..4ca970773d8 100644 --- a/src/native.js +++ b/src/native.js @@ -1,5 +1,4 @@ import { getBidRequest, logError, triggerPixel } from './utils'; -import { auctionManager } from './auctionManager'; export const nativeAdapters = []; @@ -65,12 +64,15 @@ export const nativeBidder = bid => nativeAdapters.includes(bid.bidder); export const hasNonNativeBidder = adUnit => adUnit.bids.filter(bid => !nativeBidder(bid)).length; -/* +/** * Validate that the native assets on this bid contain all assets that were * marked as required in the adUnit configuration. + * @param {Bid} bid Native bid to validate + * @param {BidRequest[]} bidRequests All bid requests for an auction + * @return {Boolean} If object is valid */ -export function nativeBidIsValid(bid) { - const bidRequest = getBidRequest(bid.adId, auctionManager.getBidsRequested()); +export function nativeBidIsValid(bid, bidRequests) { + const bidRequest = getBidRequest(bid.adId, bidRequests); if (!bidRequest) { return false; } const requestedAssets = bidRequest.nativeParams; diff --git a/src/video.js b/src/video.js index 5b94fdc8a7b..b28820c0167 100644 --- a/src/video.js +++ b/src/video.js @@ -1,6 +1,5 @@ import { videoAdapters } from './adaptermanager'; import { getBidRequest, deepAccess } from './utils'; -import { auctionManager } from './auctionManager'; const VIDEO_MEDIA_TYPE = 'video'; const OUTSTREAM = 'outstream'; @@ -20,11 +19,12 @@ export const hasNonVideoBidder = adUnit => /** * 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 + * @param {VideoBid} bid Video bid to validate + * @param {BidRequest[]} bidRequests All bid requests for an auction + * @return {Boolean} If object is valid */ -export function isValidVideoBid(bid) { - const bidRequest = getBidRequest(bid.adId, auctionManager.getBidsRequested()); +export function isValidVideoBid(bid, bidRequests) { + const bidRequest = getBidRequest(bid.adId, bidRequests); const videoMediaType = bidRequest && deepAccess(bidRequest, 'mediaTypes.video'); diff --git a/test/spec/video_spec.js b/test/spec/video_spec.js index 7fdbdcbae9f..e9118f653dc 100644 --- a/test/spec/video_spec.js +++ b/test/spec/video_spec.js @@ -1,97 +1,76 @@ import { isValidVideoBid } from 'src/video'; -import { auctionManager } from 'src/auctionManager'; describe('video.js', () => { - beforeEach(() => { - sinon.stub(auctionManager, 'getBidsRequested'); - }); - - afterEach(() => { - auctionManager.getBidsRequested.restore(); - }); - it('validates valid instream bids', () => { - auctionManager.getBidsRequested.returns([ - { - bids: [{ - bidId: '123abc', - bidder: 'appnexusAst', - mediaTypes: { - video: { context: 'instream' } - } - }] - } - ]); - - const valid = isValidVideoBid({ + const bid = { adId: '123abc', vastUrl: 'http://www.example.com/vastUrl' - }); - + }; + const bidRequests = [{ + bids: [{ + bidId: '123abc', + bidder: 'appnexusAst', + mediaTypes: { + video: { context: 'instream' } + } + }] + }]; + const valid = isValidVideoBid(bid, bidRequests); expect(valid).to.be(true); }); it('catches invalid instream bids', () => { - auctionManager.getBidsRequested.returns([ - { - bids: [{ - bidId: '123abc', - bidder: 'appnexusAst', - mediaTypes: { - video: { context: 'instream' } - } - }] - } - ]); - - const valid = isValidVideoBid({ + const bid = { adId: '123abc' - }); - + }; + const bidRequests = [{ + bids: [{ + bidId: '123abc', + bidder: 'appnexusAst', + mediaTypes: { + video: { context: 'instream' } + } + }] + }]; + const valid = isValidVideoBid(bid, bidRequests); expect(valid).to.be(false); }); it('validates valid outstream bids', () => { - auctionManager.getBidsRequested.returns([ - { - bids: [{ - bidId: '123abc', - bidder: 'appnexusAst', - mediaTypes: { - video: { context: 'outstream' } - } - }] - } - ]); - - const valid = isValidVideoBid({ + const bid = { adId: '123abc', renderer: { url: 'render.url', render: () => true, } - }); - + }; + const bidRequests = [{ + bids: [{ + bidId: '123abc', + bidder: 'appnexusAst', + mediaTypes: { + video: { context: 'outstream' } + } + }] + }]; + const valid = isValidVideoBid(bid, bidRequests); expect(valid).to.be(true); }); it('catches invalid outstream bids', () => { - auctionManager.getBidsRequested.returns([ - { - bids: [{ - bidId: '123abc', - bidder: 'appnexusAst', - mediaTypes: { - video: { context: 'outstream' } - } - }] - } - ]); - - const valid = isValidVideoBid({ + const bid = { adId: '123abc' - }); - + }; + const bidRequests = [{ + bids: [{ + bidId: '123abc', + bidder: 'appnexusAst', + mediaTypes: { + video: { context: 'outstream' } + } + }] + }]; + const valid = isValidVideoBid(bid, bidRequests); expect(valid).to.be(false); }); });