Prebid 1.0 Fix issue with video bid validation (#1680)

* Fix issue with video bid validation * Modified tests to stub `auctionManager.getBidsRequested` instead of `getBidRequest` * Move stub to beforeEach hook * Fix lint errors * Add bidRequests param to bid validation
prebid · Oct 18, 2017 · feebc17 · feebc17
1 parent e5b703e
commit feebc17
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 53 deletions.
diff --git a/src/auction.js b/src/auction.js
@@ -202,11 +202,11 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout}) {
         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
@@ -1,4 +1,3 @@
-import { auctionManager } from './auctionManager';
 import { deepAccess, getBidRequest, logError, triggerPixel } from './utils';
 
 export const nativeAdapters = [];
@@ -69,12 +68,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
@@ -19,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);
+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
@@ -1,67 +1,76 @@
 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',
-      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', () => {
-    sinon.stub(utils, 'getBidRequest', () => ({
-      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', () => {
-    sinon.stub(utils, 'getBidRequest', () => ({
-      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', () => {
-    sinon.stub(utils, 'getBidRequest', () => ({
-      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);
   });
 });