Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rubicon Bidder Factory #1587

Merged
merged 3 commits into from
Sep 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions modules/appnexusAstBidAdapter.js
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
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,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 @@ -37,14 +37,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 @@ -56,7 +56,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 @@ -76,6 +75,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 @@ -172,7 +172,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) {
fillNoBids();
return;
Expand Down Expand Up @@ -205,7 +205,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 @@ -236,57 +236,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
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,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 @@ -63,57 +63,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 @@ -125,7 +125,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 @@ -148,7 +148,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 @@ -170,7 +170,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 @@ -206,7 +206,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 @@ -217,12 +217,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 @@ -252,7 +257,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 @@ -272,7 +277,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 @@ -302,7 +307,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 @@ -317,7 +322,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 @@ -337,7 +342,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 @@ -369,7 +374,7 @@ describe('registerBidder', () => {
function newEmptySpec() {
return {
code: CODE,
areParamsValid: function() { },
isBidRequestValid: function() { },
buildRequests: function() { },
interpretResponse: function() { },
};
Expand Down