Skip to content

Commit

Permalink
Rubicon Bidder Factory (prebid#1587)
Browse files Browse the repository at this point in the history 
* Renamed areParamsValid to isBidRequestValid. Added the bidrequest to interpretResponse.

* Changed areParamsValid into isBidRequestValid.

* Updated the unit tests.
  • Loading branch information
@dbemiller
dbemiller authored Sep 19, 2017
1 parent ea97915 commit 5fc4022
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 74 deletions.
8 changes: 4 additions & 4 deletions modules/appnexusAstBidAdapter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ export const spec = {
/**
* Determines whether or not the given bid request is valid.
*
* @param {object} bidParams The params to validate.
* @param {object} bid The bid to validate.
* @return boolean True if this is a valid bid, and false otherwise.
*/
areParamsValid: function(bidParams) {
return !!(bidParams.placementId || (bidParams.member && bidParams.invCode));
isBidRequestValid: function(bid) {
return !!(bid.params.placementId || (bid.params.member && bid.params.invCode));
},

/**
Expand Down Expand Up @@ -119,7 +119,7 @@ function newRenderer(adUnitCode, rtbBid) {
try {
renderer.setRender(outstreamRender);
} catch (err) {
utils.logWarning('Prebid Error calling setRender on renderer', err);
utils.logWarn('Prebid Error calling setRender on renderer', err);
}

renderer.setEventHandlers({
Expand Down
98 changes: 49 additions & 49 deletions src/adapters/bidderFactory.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { logWarn, logError, parseQueryStringParameters, delayExecution } from 's
* code: 'myBidderCode',
* aliases: ['alias1', 'alias2'],
* supportedMediaTypes: ['video', 'native'],
* areParamsValid: function(paramsObject) { return true/false },
* isBidRequestValid: function(paramsObject) { return true/false },
* buildRequests: function(bidRequests) { return some ServerRequest(s) },
* interpretResponse: function(oneServerResponse) { return some Bids, or throw an error. }
* });
Expand All @@ -39,14 +39,14 @@ import { logWarn, logError, parseQueryStringParameters, delayExecution } from 's
* one as is used in the call to registerBidAdapter
* @property {string[]} [aliases] A list of aliases which should also resolve to this bidder.
* @property {MediaType[]} [supportedMediaTypes]: A list of Media Types which the adapter supports.
* @property {function(object): boolean} areParamsValid Determines whether or not the given object has all the params
* @property {function(object): boolean} isBidRequestValid Determines whether or not the given bid has all the params
* needed to make a valid request.
* @property {function(BidRequest[]): ServerRequest|ServerRequest[]} buildRequests Build the request to the Server which
* requests Bids for the given array of Requests. Each BidRequest in the argument array is guaranteed to have
* a "params" property which has passed the areParamsValid() test
* @property {function(*): Bid[]} interpretResponse Given a successful response from the Server, interpret it
* and return the Bid objects. This function will be run inside a try/catch. If it throws any errors, your
* bids will be discarded.
* @property {function(BidRequest[]): ServerRequest|ServerRequest[]} buildRequests Build the request to the Server
* which requests Bids for the given array of Requests. Each BidRequest in the argument array is guaranteed to have
* passed the isBidRequestValid() test.
* @property {function(*, BidRequest): Bid[]} interpretResponse Given a successful response from the Server,
* interpret it and return the Bid objects. This function will be run inside a try/catch.
* If it throws any errors, your bids will be discarded.
* @property {function(SyncOptions, Array): UserSync[]} [getUserSyncs] Given an array of all the responses
* from the server, determine which user syncs should occur. The argument array will contain every element
* which has been sent through to interpretResponse. The order of syncs in this array matters. The most
Expand All @@ -58,7 +58,6 @@ import { logWarn, logError, parseQueryStringParameters, delayExecution } from 's
*
* @property {string} bidId A string which uniquely identifies this BidRequest in the current Auction.
* @property {object} params Any bidder-specific params which the publisher used in their bid request.
* This is guaranteed to have passed the spec.areParamsValid() test.
*/

/**
Expand All @@ -78,6 +77,7 @@ import { logWarn, logError, parseQueryStringParameters, delayExecution } from 's
* @property {string} requestId The specific BidRequest which this bid is aimed at.
* This should correspond to one of the
* @property {string} ad A URL which can be used to load this ad, if it's chosen by the publisher.
* @property {string} currency The currency code for the cpm value
* @property {number} cpm The bid price, in US cents per thousand impressions.
* @property {number} height The height of the ad, in pixels.
* @property {number} width The width of the ad, in pixels.
Expand Down Expand Up @@ -195,7 +195,7 @@ export function newBidder(spec) {
bidRequestMap[bid.bidId] = bid;
});

let requests = spec.buildRequests(bidRequests);
let requests = spec.buildRequests(bidRequests, bidderRequest);
if (!requests || requests.length === 0) {
afterAllResponses();
return;
Expand All @@ -214,7 +214,7 @@ export function newBidder(spec) {
switch (request.method) {
case 'GET':
ajax(
`${request.url}?${parseQueryStringParameters(request.data)}`,
`${request.url}?${typeof request.data === 'object' ? parseQueryStringParameters(request.data) : request.data}`,
{
success: onSuccess,
error: onFailure
Expand Down Expand Up @@ -245,57 +245,57 @@ export function newBidder(spec) {
logWarn(`Skipping invalid request from ${spec.code}. Request type ${request.type} must be GET or POST`);
onResponse();
}
}

// If the server responds successfully, use the adapter code to unpack the Bids from it.
// If the adapter code fails, no bids should be added. After all the bids have been added, make
// sure to call the `onResponse` function so that we're one step closer to calling fillNoBids().
function onSuccess(response) {
try {
response = JSON.parse(response);
} catch (e) { /* response might not be JSON... that's ok. */ }
responses.push(response);
// If the server responds successfully, use the adapter code to unpack the Bids from it.
// If the adapter code fails, no bids should be added. After all the bids have been added, make
// sure to call the `onResponse` function so that we're one step closer to calling fillNoBids().
function onSuccess(response) {
try {
response = JSON.parse(response);
} catch (e) { /* response might not be JSON... that's ok. */ }
responses.push(response);

let bids;
try {
bids = spec.interpretResponse(response);
} catch (err) {
logError(`Bidder ${spec.code} failed to interpret the server's response. Continuing without bids`, null, err);
onResponse();
return;
}
let bids;
try {
bids = spec.interpretResponse(response, request);
} catch (err) {
logError(`Bidder ${spec.code} failed to interpret the server's response. Continuing without bids`, null, err);
onResponse();
return;
}

if (bids) {
if (bids.forEach) {
bids.forEach(addBidUsingRequestMap);
} else {
addBidUsingRequestMap(bids);
if (bids) {
if (bids.forEach) {
bids.forEach(addBidUsingRequestMap);
} else {
addBidUsingRequestMap(bids);
}
}
}
onResponse();
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);
} else {
logWarn(`Bidder ${spec.code} made bid for unknown request ID: ${bid.requestId}. Ignoring.`);
function addBidUsingRequestMap(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.`);
}
}
}
}

// If the server responds with an error, there's not much we can do. Log it, and make sure to
// call onResponse() so that we're one step closer to calling fillNoBids().
function onFailure(err) {
logError(`Server call for ${spec.code} failed: ${err}. Continuing without bids.`);
onResponse();
// If the server responds with an error, there's not much we can do. Log it, and make sure to
// call onResponse() so that we're one step closer to calling fillNoBids().
function onFailure(err) {
logError(`Server call for ${spec.code} failed: ${err}. Continuing without bids.`);
onResponse();
}
}
}
});

function filterAndWarn(bid) {
if (!spec.areParamsValid(bid.params)) {
if (!spec.isBidRequestValid(bid)) {
logWarn(`Invalid bid sent to bidder ${spec.code}: ${JSON.stringify(bid)}`);
return false;
}
Expand Down
47 changes: 26 additions & 21 deletions test/spec/unit/core/bidderFactory_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('bidders created by newBidder', () => {
beforeEach(() => {
spec = {
code: CODE,
areParamsValid: sinon.stub(),
isBidRequestValid: sinon.stub(),
buildRequests: sinon.stub(),
interpretResponse: sinon.stub(),
getUserSyncs: sinon.stub()
Expand Down Expand Up @@ -66,57 +66,57 @@ describe('bidders created by newBidder', () => {
bidder.callBids({ bids: 'nothing useful' });

expect(ajaxStub.called).to.equal(false);
expect(spec.areParamsValid.called).to.equal(false);
expect(spec.isBidRequestValid.called).to.equal(false);
expect(spec.buildRequests.called).to.equal(false);
expect(spec.interpretResponse.called).to.equal(false);
});

it('should call buildRequests(bidRequest) the params are valid', () => {
const bidder = newBidder(spec);

spec.areParamsValid.returns(true);
spec.isBidRequestValid.returns(true);
spec.buildRequests.returns([]);

bidder.callBids(MOCK_BIDS_REQUEST);

expect(ajaxStub.called).to.equal(false);
expect(spec.areParamsValid.calledTwice).to.equal(true);
expect(spec.isBidRequestValid.calledTwice).to.equal(true);
expect(spec.buildRequests.calledOnce).to.equal(true);
expect(spec.buildRequests.firstCall.args[0]).to.deep.equal(MOCK_BIDS_REQUEST.bids);
});

it('should not call buildRequests the params are invalid', () => {
const bidder = newBidder(spec);

spec.areParamsValid.returns(false);
spec.isBidRequestValid.returns(false);
spec.buildRequests.returns([]);

bidder.callBids(MOCK_BIDS_REQUEST);

expect(ajaxStub.called).to.equal(false);
expect(spec.areParamsValid.calledTwice).to.equal(true);
expect(spec.isBidRequestValid.calledTwice).to.equal(true);
expect(spec.buildRequests.called).to.equal(false);
});

it('should filter out invalid bids before calling buildRequests', () => {
const bidder = newBidder(spec);

spec.areParamsValid.onFirstCall().returns(true);
spec.areParamsValid.onSecondCall().returns(false);
spec.isBidRequestValid.onFirstCall().returns(true);
spec.isBidRequestValid.onSecondCall().returns(false);
spec.buildRequests.returns([]);

bidder.callBids(MOCK_BIDS_REQUEST);

expect(ajaxStub.called).to.equal(false);
expect(spec.areParamsValid.calledTwice).to.equal(true);
expect(spec.isBidRequestValid.calledTwice).to.equal(true);
expect(spec.buildRequests.calledOnce).to.equal(true);
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", () => {
const bidder = newBidder(spec);

spec.areParamsValid.returns(true);
spec.isBidRequestValid.returns(true);
spec.buildRequests.returns([]);

bidder.callBids(MOCK_BIDS_REQUEST);
Expand All @@ -128,7 +128,7 @@ describe('bidders created by newBidder', () => {
const bidder = newBidder(spec);
const url = 'test.url.com';
const data = { arg: 2 };
spec.areParamsValid.returns(true);
spec.isBidRequestValid.returns(true);
spec.buildRequests.returns({
method: 'POST',
url: url,
Expand All @@ -151,7 +151,7 @@ describe('bidders created by newBidder', () => {
const bidder = newBidder(spec);
const url = 'test.url.com';
const data = { arg: 2 };
spec.areParamsValid.returns(true);
spec.isBidRequestValid.returns(true);
spec.buildRequests.returns({
method: 'GET',
url: url,
Expand All @@ -173,7 +173,7 @@ describe('bidders created by newBidder', () => {
const bidder = newBidder(spec);
const url = 'test.url.com';
const data = { arg: 2 };
spec.areParamsValid.returns(true);
spec.isBidRequestValid.returns(true);
spec.buildRequests.returns([
{
method: 'POST',
Expand Down Expand Up @@ -229,7 +229,7 @@ describe('bidders created by newBidder', () => {
it('should call spec.interpretResponse() with the response body content', () => {
const bidder = newBidder(spec);

spec.areParamsValid.returns(true);
spec.isBidRequestValid.returns(true);
spec.buildRequests.returns({
method: 'POST',
url: 'test.url.com',
Expand All @@ -241,12 +241,17 @@ describe('bidders created by newBidder', () => {

expect(spec.interpretResponse.calledOnce).to.equal(true);
expect(spec.interpretResponse.firstCall.args[0]).to.equal('response body');
expect(spec.interpretResponse.firstCall.args[1]).to.deep.equal({
method: 'POST',
url: 'test.url.com',
data: {}
});
});

it('should call spec.interpretResponse() once for each request made', () => {
const bidder = newBidder(spec);

spec.areParamsValid.returns(true);
spec.isBidRequestValid.returns(true);
spec.buildRequests.returns([
{
method: 'POST',
Expand Down Expand Up @@ -277,7 +282,7 @@ describe('bidders created by newBidder', () => {
width: 300,
placementCode: 'mock/placement'
};
spec.areParamsValid.returns(true);
spec.isBidRequestValid.returns(true);
spec.buildRequests.returns({
method: 'POST',
url: 'test.url.com',
Expand All @@ -299,7 +304,7 @@ describe('bidders created by newBidder', () => {
it('should call spec.getUserSyncs() with the response', () => {
const bidder = newBidder(spec);

spec.areParamsValid.returns(true);
spec.isBidRequestValid.returns(true);
spec.buildRequests.returns({
method: 'POST',
url: 'test.url.com',
Expand Down Expand Up @@ -348,7 +353,7 @@ describe('bidders created by newBidder', () => {
it('should not spec.interpretResponse()', () => {
const bidder = newBidder(spec);

spec.areParamsValid.returns(true);
spec.isBidRequestValid.returns(true);
spec.buildRequests.returns({
method: 'POST',
url: 'test.url.com',
Expand All @@ -364,7 +369,7 @@ describe('bidders created by newBidder', () => {
it('should add bids for each placement code into the bidmanager', () => {
const bidder = newBidder(spec);

spec.areParamsValid.returns(true);
spec.isBidRequestValid.returns(true);
spec.buildRequests.returns({
method: 'POST',
url: 'test.url.com',
Expand All @@ -385,7 +390,7 @@ describe('bidders created by newBidder', () => {
it('should call spec.getUserSyncs() with no responses', () => {
const bidder = newBidder(spec);

spec.areParamsValid.returns(true);
spec.isBidRequestValid.returns(true);
spec.buildRequests.returns({
method: 'POST',
url: 'test.url.com',
Expand Down Expand Up @@ -418,7 +423,7 @@ describe('registerBidder', () => {
function newEmptySpec() {
return {
code: CODE,
areParamsValid: function() { },
isBidRequestValid: function() { },
buildRequests: function() { },
interpretResponse: function() { },
};
Expand Down

0 comments on commit 5fc4022

Please sign in to comment.