generate bid.adId uniquely instead of using bidRequest.bidId (#3440)

* generate bid.adId uniquely * remove stray debugger * update adId reference in currency file
prebid · Feb 6, 2019 · 8c210bc · 8c210bc
1 parent bdcd0c2
commit 8c210bc
Show file tree

Hide file tree

Showing 12 changed files with 34 additions and 31 deletions.
diff --git a/modules/currency.js b/modules/currency.js
@@ -222,7 +222,7 @@ function wrapFunction(fn, context, params) {
         utils.logWarn('Returning NO_BID, getCurrencyConversion threw error: ', e);
         params[1] = createBid(STATUS.NO_BID, {
           bidder: bid.bidderCode || bid.bidder,
-          bidId: bid.adId
+          bidId: bid.requestId
         });
       }
     }

diff --git a/src/auction.js b/src/auction.js
@@ -396,7 +396,7 @@ function addBidToAuction(auctionInstance, bidResponse) {
 function tryAddVideoBid(auctionInstance, bidResponse, bidRequests, afterBidAdded) {
   let addBid = true;
 
-  const bidRequest = getBidRequest(bidResponse.adId, [bidRequests]);
+  const bidRequest = getBidRequest(bidResponse.requestId, [bidRequests]);
   const videoMediaType =
     bidRequest && deepAccess(bidRequest, 'mediaTypes.video');
   const context = videoMediaType && deepAccess(videoMediaType, 'context');

diff --git a/src/bidfactory.js b/src/bidfactory.js
@@ -15,15 +15,15 @@ var utils = require('./utils.js');
  priceKeyString;
  */
 function Bid(statusCode, bidRequest) {
-  var _bidId = (bidRequest && bidRequest.bidId) || utils.getUniqueIdentifierStr();
   var _bidSrc = (bidRequest && bidRequest.src) || 'client';
   var _statusCode = statusCode || 0;
 
   this.bidderCode = (bidRequest && bidRequest.bidder) || '';
   this.width = 0;
   this.height = 0;
   this.statusMessage = _getStatus();
-  this.adId = _bidId;
+  this.adId = utils.getUniqueIdentifierStr();
+  this.requestId = bidRequest && bidRequest.bidId;
   this.mediaType = 'banner';
   this.source = _bidSrc;
 

diff --git a/src/native.js b/src/native.js
@@ -69,7 +69,7 @@ export const hasNonNativeBidder = adUnit =>
  * @return {Boolean} If object is valid
  */
 export function nativeBidIsValid(bid, bidRequests) {
-  const bidRequest = getBidRequest(bid.adId, bidRequests);
+  const bidRequest = getBidRequest(bid.requestId, bidRequests);
   if (!bidRequest) { return false; }
 
   // all native bid responses must define a landing page url

diff --git a/src/utils.js b/src/utils.js
@@ -750,6 +750,9 @@ export function flatten(a, b) {
 }
 
 export function getBidRequest(id, bidderRequests) {
+  if (!id) {
+    return;
+  }
   let bidRequest;
   bidderRequests.some(bidderRequest => {
     let result = find(bidderRequest.bids, bid => ['bidId', 'adId', 'bid_id'].some(type => bid[type] === id));

diff --git a/src/video.js b/src/video.js
@@ -30,7 +30,7 @@ export const hasNonVideoBidder = adUnit =>
  * @return {Boolean} If object is valid
  */
 export function isValidVideoBid(bid, bidRequests) {
-  const bidRequest = getBidRequest(bid.adId, bidRequests);
+  const bidRequest = getBidRequest(bid.requestId, bidRequests);
 
   const videoMediaType =
     bidRequest && deepAccess(bidRequest, 'mediaTypes.video');

diff --git a/test/spec/auctionmanager_spec.js b/test/spec/auctionmanager_spec.js
@@ -768,13 +768,13 @@ describe('auctionmanager.js', function () {
       auctionModule.newAuction.restore();
     });
 
-    it('should not alter bid adID', function () {
+    it('should not alter bid requestID', function () {
       auction.callBids();
 
       const addedBid2 = auction.getBidsReceived().pop();
-      assert.equal(addedBid2.adId, bids1[0].requestId);
+      assert.equal(addedBid2.requestId, bids1[0].requestId);
       const addedBid1 = auction.getBidsReceived().pop();
-      assert.equal(addedBid1.adId, bids[0].requestId);
+      assert.equal(addedBid1.requestId, bids[0].requestId);
     });
 
     it('should not add banner bids that have no width or height', function () {

diff --git a/test/spec/modules/prebidServerBidAdapter_spec.js b/test/spec/modules/prebidServerBidAdapter_spec.js
@@ -843,7 +843,6 @@ describe('S2S Adapter', function () {
       const response = addBidResponse.firstCall.args[1];
       expect(response).to.have.property('statusMessage', 'Bid available');
       expect(response).to.have.property('cpm', 0.5);
-      expect(response).to.have.property('adId', '123');
       expect(response).to.have.property('requestId', '123');
       expect(response).to.not.have.property('videoCacheKey');
       expect(response).to.have.property('cache_id', '7654321');
@@ -862,7 +861,6 @@ describe('S2S Adapter', function () {
       const response = addBidResponse.firstCall.args[1];
       expect(response).to.have.property('statusMessage', 'Bid available');
       expect(response).to.have.property('cpm', 0.5);
-      expect(response).to.have.property('adId', '123');
       expect(response).to.have.property('requestId', '123');
       expect(response).to.have.property('videoCacheKey', 'video_cache_id');
       expect(response).to.have.property('cache_id', 'video_cache_id');
@@ -915,7 +913,7 @@ describe('S2S Adapter', function () {
 
       expect(addBidResponse.firstCall.args[0]).to.equal('div-gpt-ad-1460505748561-0');
 
-      expect(addBidResponse.firstCall.args[1]).to.have.property('adId', '123');
+      expect(addBidResponse.firstCall.args[1]).to.have.property('requestId', '123');
 
       expect(addBidResponse.firstCall.args[1])
         .to.have.property('statusMessage', 'Bid available');
@@ -994,7 +992,7 @@ describe('S2S Adapter', function () {
       expect(response).to.have.property('source', 's2s');
 
       const bid_request_passed = addBidResponse.firstCall.args[1];
-      expect(bid_request_passed).to.have.property('adId', '123');
+      expect(bid_request_passed).to.have.property('requestId', '123');
     });
 
     it('handles OpenRTB responses and call BIDDER_DONE', function () {
@@ -1016,7 +1014,6 @@ describe('S2S Adapter', function () {
       const response = addBidResponse.firstCall.args[1];
       expect(response).to.have.property('statusMessage', 'Bid available');
       expect(response).to.have.property('bidderCode', 'appnexus');
-      expect(response).to.have.property('adId', '123');
       expect(response).to.have.property('requestId', '123');
       expect(response).to.have.property('cpm', 0.5);
     });
@@ -1037,7 +1034,6 @@ describe('S2S Adapter', function () {
       expect(response).to.have.property('vastXml', RESPONSE_OPENRTB_VIDEO.seatbid[0].bid[0].adm);
       expect(response).to.have.property('mediaType', 'video');
       expect(response).to.have.property('bidderCode', 'appnexus');
-      expect(response).to.have.property('adId', '123');
       expect(response).to.have.property('requestId', '123');
       expect(response).to.have.property('cpm', 10);
     });

diff --git a/test/spec/modules/serverbidServerBidAdapter_spec.js b/test/spec/modules/serverbidServerBidAdapter_spec.js
@@ -243,7 +243,7 @@ describe('ServerBid S2S Adapter', function () {
       const response = addBidResponse.firstCall.args[1];
       expect(response).to.have.property('statusMessage', 'Bid available');
       expect(response).to.have.property('cpm', 0.5);
-      expect(response).to.have.property('adId', '123');
+      expect(response).to.have.property('requestId', '123');
     });
 
     it('registers no-bid response when ad unit not set', function () {
@@ -261,7 +261,7 @@ describe('ServerBid S2S Adapter', function () {
       expect(response).to.have.property('statusMessage', 'Bid returned empty or error response');
 
       const bid_request_passed = addBidResponse.firstCall.args[1];
-      expect(bid_request_passed).to.have.property('adId', '123');
+      expect(bid_request_passed).to.have.property('requestId', '123');
     });
 
     it('registers no-bid response when ad unit is set', function () {
@@ -291,8 +291,8 @@ describe('ServerBid S2S Adapter', function () {
       expect(addBidResponse.firstCall.args[0]).to.equal('div-gpt-ad-1460505748561-0');
       expect(addBidResponse.secondCall.args[0]).to.equal('div-gpt-ad-1460505748561-1');
 
-      expect(addBidResponse.firstCall.args[1]).to.have.property('adId', '123');
-      expect(addBidResponse.secondCall.args[1]).to.have.property('adId', '101111');
+      expect(addBidResponse.firstCall.args[1]).to.have.property('requestId', '123');
+      expect(addBidResponse.secondCall.args[1]).to.have.property('requestId', '101111');
 
       expect(addBidResponse.firstCall.args[1])
         .to.have.property('statusMessage', 'Bid available');

diff --git a/test/spec/native_spec.js b/test/spec/native_spec.js
@@ -102,7 +102,8 @@ describe('validate native', function () {
   }];
 
   let validBid = {
-    adId: 'test_bid_id',
+    adId: 'abc123',
+    requestId: 'test_bid_id',
     adUnitCode: '123/prebid_native_adunit',
     bidder: 'test_bidder',
     native: {
@@ -126,7 +127,8 @@ describe('validate native', function () {
   };
 
   let noIconDimBid = {
-    adId: 'test_bid_id',
+    adId: 'abc234',
+    requestId: 'test_bid_id',
     adUnitCode: '123/prebid_native_adunit',
     bidder: 'test_bidder',
     native: {
@@ -150,7 +152,8 @@ describe('validate native', function () {
   };
 
   let noImgDimBid = {
-    adId: 'test_bid_id',
+    adId: 'abc345',
+    requestId: 'test_bid_id',
     adUnitCode: '123/prebid_native_adunit',
     bidder: 'test_bidder',
     native: {

diff --git a/test/spec/unit/core/bidderFactory_spec.js b/test/spec/unit/core/bidderFactory_spec.js
@@ -639,7 +639,7 @@ describe('validate bid response: ', function () {
   it('should add native bids that do have required assets', function () {
     let bidRequest = {
       bids: [{
-        bidId: 1,
+        bidId: '1',
         auctionId: 'first-bid-id',
         adUnitCode: 'mock/placement',
         params: {
@@ -676,7 +676,7 @@ describe('validate bid response: ', function () {
   it('should not add native bids that do not have required assets', function () {
     let bidRequest = {
       bids: [{
-        bidId: 1,
+        bidId: '1',
         auctionId: 'first-bid-id',
         adUnitCode: 'mock/placement',
         params: {
@@ -712,7 +712,7 @@ describe('validate bid response: ', function () {
   it('should add bid when renderer is present on outstream bids', function () {
     let bidRequest = {
       bids: [{
-        bidId: 1,
+        bidId: '1',
         auctionId: 'first-bid-id',
         adUnitCode: 'mock/placement',
         params: {
@@ -747,7 +747,7 @@ describe('validate bid response: ', function () {
     let bidRequest = {
       bids: [{
         bidder: CODE,
-        bidId: 1,
+        bidId: '1',
         auctionId: 'first-bid-id',
         adUnitCode: 'mock/placement',
         params: {

diff --git a/test/spec/video_spec.js b/test/spec/video_spec.js
@@ -3,8 +3,9 @@ import { isValidVideoBid } from 'src/video';
 describe('video.js', function () {
   it('validates valid instream bids', function () {
     const bid = {
-      adId: '123abc',
-      vastUrl: 'http://www.example.com/vastUrl'
+      adId: '456xyz',
+      vastUrl: 'http://www.example.com/vastUrl',
+      requestId: '123abc'
     };
     const bidRequests = [{
       bids: [{
@@ -21,7 +22,7 @@ describe('video.js', function () {
 
   it('catches invalid instream bids', function () {
     const bid = {
-      adId: '123abc'
+      requestId: '123abc'
     };
     const bidRequests = [{
       bids: [{
@@ -51,7 +52,7 @@ describe('video.js', function () {
 
   it('validates valid outstream bids', function () {
     const bid = {
-      adId: '123abc',
+      requestId: '123abc',
       renderer: {
         url: 'render.url',
         render: () => true,
@@ -72,7 +73,7 @@ describe('video.js', function () {
 
   it('catches invalid outstream bids', function () {
     const bid = {
-      adId: '123abc'
+      requestId: '123abc'
     };
     const bidRequests = [{
       bids: [{