Validating bid response params (prebid#1738)

* validating bid response params

* added all required params

* replaced id with correct param requestid

* keeping only common keys to validate

modules/appnexusAstBidAdapter.js

@@ -182,16 +182,20 @@ function newBid(serverBid, rtbBid) {
   const bid = {
     requestId: serverBid.uuid,
     cpm: rtbBid.cpm,
-    creative_id: rtbBid.creative_id,
+    creativeId: rtbBid.creative_id,
     dealId: rtbBid.deal_id,
+    currency: 'USD',
+    netRevenue: true,
+    ttl: 300
   };
 
   if (rtbBid.rtb.video) {
     Object.assign(bid, {
       width: rtbBid.rtb.video.player_width,
       height: rtbBid.rtb.video.player_height,
       vastUrl: rtbBid.rtb.video.asset_url,
-      descriptionUrl: rtbBid.rtb.video.asset_url
+      descriptionUrl: rtbBid.rtb.video.asset_url,
+      ttl: 3600
     });
     // This supports Outstream Video
     if (rtbBid.renderer_url) {

src/adapters/bidderFactory.js

@@ -116,6 +116,9 @@ import { logWarn, logError, parseQueryStringParameters, delayExecution } from 's
  * @property {string} url The URL which makes the sync happen.
  */
 
+// common params for all mediaTypes
+const COMMON_BID_RESPONSE_KEYS = ['requestId', 'cpm', 'ttl', 'creativeId', 'netRevenue', 'currency'];
+
 /**
  * Register a bidder with prebid, using the given spec.
  *
@@ -303,12 +306,17 @@ export function newBidder(spec) {
           onResponse();
 
           function addBidUsingRequestMap(bid) {
-            const bidRequest = bidRequestMap[bid.requestId];
-            if (bidRequest) {
-              const prebidBid = Object.assign(bidfactory.createBid(STATUS.GOOD, bidRequest), bid);
-              addBidWithCode(bidRequest.placementCode, prebidBid);
+            // In Prebid 1.0 all the validation logic from bidmanager will move here, as of now we are only validating new params so that adapters dont miss adding them.
+            if (hasValidKeys(bid)) {
+              const bidRequest = bidRequestMap[bid.requestId];
+              if (bidRequest) {
+                const prebidBid = Object.assign(bidfactory.createBid(STATUS.GOOD, bidRequest), bid);
+                addBidWithCode(bidRequest.placementCode, prebidBid);
+              } else {
+                logWarn(`Bidder ${spec.code} made bid for unknown request ID: ${bid.requestId}. Ignoring.`);
+              }
             } else {
-              logWarn(`Bidder ${spec.code} made bid for unknown request ID: ${bid.requestId}. Ignoring.`);
+              logError(`Bidder ${spec.code} is missing required params. Check http://prebid.org/dev-docs/bidder-adapter-1.html for list of params.`);
             }
           }
 
@@ -337,6 +345,11 @@ export function newBidder(spec) {
     return true;
   }
 
+  function hasValidKeys(bid) {
+    let bidKeys = Object.keys(bid);
+    return COMMON_BID_RESPONSE_KEYS.every(key => bidKeys.includes(key));
+  }
+
   function newEmptyBid() {
     const bid = bidfactory.createBid(STATUS.NO_BID);
     bid.code = spec.code;

test/spec/modules/appnexusAstBidAdapter_spec.js

@@ -296,18 +296,20 @@ describe('AppNexusAdapter', () => {
         {
           'requestId': '3db3773286ee59',
           'cpm': 0.5,
-          'creative_id': 29681110,
+          'creativeId': 29681110,
           'dealId': undefined,
           'width': 300,
           'height': 250,
           'ad': '<!-- Creative -->',
-          'mediaType': 'banner'
+          'mediaType': 'banner',
+          'currency': 'USD',
+          'ttl': 300,
+          'netRevenue': true
         }
       ];
       let bidderRequest;
-
       let result = spec.interpretResponse({ body: response }, {bidderRequest});
-      expect(Object.keys(result[0])).to.deep.equal(Object.keys(expectedResponse[0]));
+      expect(Object.keys(result[0])).to.have.members(Object.keys(expectedResponse[0]));
     });
 
     it('handles nobid responses', () => {

test/spec/unit/core/bidderFactory_spec.js

@@ -5,6 +5,7 @@ import * as ajax from 'src/ajax';
 import { expect } from 'chai';
 import { STATUS } from 'src/constants';
 import { userSync } from 'src/userSync'
+import * as utils from 'src/utils';
 
 const CODE = 'sampleBidder';
 const MOCK_BIDS_REQUEST = {
@@ -113,7 +114,7 @@ describe('bidders created by newBidder', () => {
       expect(spec.buildRequests.firstCall.args[0]).to.deep.equal([MOCK_BIDS_REQUEST.bids[0]]);
     });
 
-    it("should make no server requests if the spec doesn't return any", () => {
+    it('should make no server requests if the spec doesn\'t return any', () => {
       const bidder = newBidder(spec);
 
       spec.isBidRequestValid.returns(true);
@@ -262,6 +263,7 @@ describe('bidders created by newBidder', () => {
   describe('when the ajax call succeeds', () => {
     let ajaxStub;
     let userSyncStub;
+    let logErrorSpy;
 
     beforeEach(() => {
       ajaxStub = sinon.stub(ajax, 'ajax', function(url, callbacks) {
@@ -270,11 +272,13 @@ describe('bidders created by newBidder', () => {
         callbacks.success('response body', { getResponseHeader: fakeResponse });
       });
       userSyncStub = sinon.stub(userSync, 'registerSync')
+      logErrorSpy = sinon.spy(utils, 'logError');
     });
 
     afterEach(() => {
       ajaxStub.restore();
       userSyncStub.restore();
+      utils.logError.restore();
     });
 
     it('should call spec.interpretResponse() with the response content', () => {
@@ -324,16 +328,21 @@ describe('bidders created by newBidder', () => {
       expect(spec.interpretResponse.calledTwice).to.equal(true);
     });
 
-    it("should add bids for each placement code into the bidmanager, even if the bidder doesn't bid on all of them", () => {
+    it('should add bids for each placement code into the bidmanager, even if the bidder doesn\'t bid on all of them', () => {
       const bidder = newBidder(spec);
 
       const bid = {
+        creativeId: 'creative-id',
+        bidderCode: 'code',
         requestId: 'some-id',
         ad: 'ad-url.com',
         cpm: 0.5,
         height: 200,
         width: 300,
-        placementCode: 'mock/placement'
+        placementCode: 'mock/placement',
+        currency: 'USD',
+        netRevenue: true,
+        ttl: 300
       };
       spec.isBidRequestValid.returns(true);
       spec.buildRequests.returns({
@@ -352,6 +361,7 @@ describe('bidders created by newBidder', () => {
         [bidmanager.addBidResponse.firstCall.args[0], bidmanager.addBidResponse.secondCall.args[0]];
       expect(placementsWithBids).to.contain('mock/placement');
       expect(placementsWithBids).to.contain('mock/placement2');
+      expect(logErrorSpy.callCount).to.equal(0);
     });
 
     it('should call spec.getUserSyncs() with the response', () => {
@@ -391,6 +401,32 @@ describe('bidders created by newBidder', () => {
       expect(userSyncStub.firstCall.args[1]).to.equal(spec.code);
       expect(userSyncStub.firstCall.args[2]).to.equal('usersync.com');
     });
+
+    it('should logError when required bid response params are missing', () => {
+      const bidder = newBidder(spec);
+
+      const bid = {
+        requestId: 'some-id',
+        ad: 'ad-url.com',
+        cpm: 0.5,
+        height: 200,
+        width: 300,
+        placementCode: 'mock/placement'
+      };
+      spec.isBidRequestValid.returns(true);
+      spec.buildRequests.returns({
+        method: 'POST',
+        url: 'test.url.com',
+        data: {}
+      });
+      spec.getUserSyncs.returns([]);
+
+      spec.interpretResponse.returns(bid);
+
+      bidder.callBids(MOCK_BIDS_REQUEST);
+
+      expect(logErrorSpy.calledOnce).to.equal(true);
+    });
   });
 
   describe('when the ajax call fails', () => {