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

BaseAdapter for the Prebid 0.x -> 1.x transition #1494

Merged
merged 22 commits into from
Sep 13, 2017
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
c1080dd
Added a base adapter for single-request adapters, and ported the appn…
dbemiller Aug 17, 2017
70fcf51
Renamed the SingleRequestBidder to BidderFactory. Updated it to handl…
dbemiller Aug 18, 2017
211e7fc
Added a unit test for the delayExecution function.
dbemiller Aug 18, 2017
6246eaf
Merged from master. Fixed conflicts.
dbemiller Aug 18, 2017
d509150
Made newBidder a default import. Added some unit tests.
dbemiller Aug 18, 2017
8d4ebc1
Added more tests.
dbemiller Aug 21, 2017
7ffc16f
Merge branch 'master' of https://github.com/prebid/Prebid.js into sin…
dbemiller Aug 21, 2017
1f23044
Added more tests, and fixed a few bugs.
dbemiller Aug 22, 2017
8d1efa6
Merged from master. Fixed a conflict.
dbemiller Aug 23, 2017
a627a43
Changed an error to a log message. Fixed a small bug.
dbemiller Aug 23, 2017
ff99ff6
Merged from master, and fixed conflicts.
dbemiller Aug 31, 2017
c246aa6
Did the no-brainer improvements from PR comments.
dbemiller Aug 31, 2017
6c80c3d
Added spec-level support for aliases and mediaTypes. Aliases may stil…
dbemiller Aug 31, 2017
37399c7
Added support for aliases. Added more tests
dbemiller Sep 1, 2017
1b42995
Cleaned up some unnecessary code.
dbemiller Sep 1, 2017
5c3d645
Removed the GET/POST constants. Fixed some typos, and renamed some Re…
dbemiller Sep 1, 2017
31899f4
Merged from master. Fixed conflicts.
dbemiller Sep 6, 2017
735f2bc
Re-added some code for outstream rendering, which was apparently lost…
dbemiller Sep 6, 2017
dd4e595
Removed confusing use of this
dbemiller Sep 6, 2017
25f822a
Fixed lint error
dbemiller Sep 6, 2017
e63f2d6
Moved JSON parsing into the bidderFactory, and moved the JSDocs to th…
dbemiller Sep 6, 2017
13d3911
Removed placementCode from everywhere I could.
dbemiller Sep 7, 2017
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
636 changes: 308 additions & 328 deletions modules/appnexusAstBidAdapter.js

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions src/Renderer.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import { loadScript } from './adloader';
import * as utils from './utils';

/**
* @typedef {object} Renderer
*
* A Renderer stores some functions which are used to render a particular Bid.
* These are used in Outstream Video Bids, returned on the Bid by the adapter, and will
* be used to render that bid unless the Publisher overrides them.
*/

export function Renderer(options) {
const { url, config, id, callback, loaded } = options;
this.url = url;
Expand Down
301 changes: 301 additions & 0 deletions src/adapters/bidderFactory.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,301 @@
import Adapter from 'src/adapter';
import adaptermanager from 'src/adaptermanager';
import { ajax } from 'src/ajax';
import bidmanager from 'src/bidmanager';
import bidfactory from 'src/bidfactory';
import { STATUS } from 'src/constants';

import { logWarn, logError, parseQueryStringParameters, delayExecution } from 'src/utils';

/**
* This file aims to support Adapters during the Prebid 0.x -> 1.x transition.
*
* Prebid 1.x and Prebid 0.x will be in separate branches--perhaps for a long time.
* This function defines an API for adapter construction which is compatible with both versions.
* Adapters which use it can maintain their code in master, and only this file will need to change
* in the 1.x branch.
*
* Typical usage looks something like:
*
* const adapter = registerBidder({
* code: 'myBidderCode',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice and easy for implementers if we allowed aliases to be defined here somehow

e.g.

const adapter = newBidder({
  code: 'rubicon',
  aliases: ['rubiconLite', 'roooobicon']

* aliases: ['alias1', 'alias2'],
* supportedMediaTypes: ['video', 'native'],
* areParamsValid: function(paramsObject) { return true/false },
* buildRequests: function(bidRequests) { return some ServerRequest(s) },
* interpretResponse: function(oneServerResponse) { return some Bids, or throw an error. }
* });
*
* @see BidderSpec for the full API and more thorough descriptions.
*/

/**
* @typedef {object} BidderSpec An object containing the adapter-specific functions needed to
* make a Bidder.
*
* @property {string} code A code which will be used to uniquely identify this bidder. This should be the same
* 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
* 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(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
* important ones should come first, since publishers may limit how many are dropped on their page.
*/

/**
* @typedef {object} BidRequest
*
* @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.
*/

/**
* @typedef {object} ServerRequest
*
* @property {('GET'|'POST')} method The type of request which this is.
* @property {string} url The endpoint for the request. For example, "//bids.example.com".
* @property {string|object} data Data to be sent in the request.
* If this is a GET request, they'll become query params. If it's a POST request, they'll be added to the body.
* Strings will be added as-is. Objects will be unpacked into query params based on key/value mappings, or
* JSON-serialized into the Request body.
*/

/**
* @typedef {object} Bid
*
* @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 {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.
*
* @property [Renderer] renderer A Renderer which can be used as a default for this bid,
* if the publisher doesn't override it. This is only relevant for Outstream Video bids.
*/

/**
* @typedef {Object} SyncOptions
*
* An object containing information about usersyncs which the adapter should obey.
*
* @property {boolean} iframeEnabled True if iframe usersyncs are allowed, and false otherwise
* @property {boolean} pixelEnabled True if image usersyncs are allowed, and false otherwise
*/

/**
* TODO: Move this to the UserSync module after that PR is merged.
*
* @typedef {object} UserSync
*
* @property {('image'|'iframe')} type The type of user sync to be done.
* @property {string} url The URL which makes the sync happen.
*/

/**
* Register a bidder with prebid, using the given spec.
*
* If possible, Adapter modules should use this function instead of adaptermanager.registerBidAdapter().
*
* @param {BidderSpec} spec An object containing the bare-bones functions we need to make a Bidder.
*/
export function registerBidder(spec) {
const mediaTypes = Array.isArray(spec.supportedMediaTypes)
? { supportedMediaTypes: spec.supportedMediaTypes }
: undefined;
function putBidder(spec) {
const bidder = newBidder(spec);
adaptermanager.registerBidAdapter(bidder, spec.code, mediaTypes);
}

putBidder(spec);
if (Array.isArray(spec.aliases)) {
spec.aliases.forEach(alias => {
putBidder(Object.assign({}, spec, { code: alias }));
});
}
}

/**
* Make a new bidder from the given spec. This is exported mainly for testing.
* Adapters will probably find it more convenient to use registerBidder instead.
*
* @param {BidderSpec} spec
*/
export function newBidder(spec) {
return Object.assign(new Adapter(spec.code), {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to figure out how aliasing is going to work with this new pattern. Currently I have it calling new on the .constructor link of an existing instance. In this case it would be an instance of just plain Adapter, which means it wouldn't work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added support for these. I'm not sure this is the best way, though... so let me know if you have better ideas.

If we think this API is general enough to be the only base class, I might re-organize the code a bit so that "bidder creation" is in a separate file from "bidder registration".

I still haven't heard anyone weigh in on whether or not this handles all the 1.0 use-cases, though, or whether i'm overly-constraining adapters with it.

callBids: function(bidderRequest) {
if (!Array.isArray(bidderRequest.bids)) {
return;
}

// callBids must add a NO_BID response for _every_ placementCode, in order for the auction to
// end properly. This map stores placement codes which we've made _real_ bids on.
//
// As we add _real_ bids to the bidmanager, we'll log the placementCodes here too. Once all the real
// bids have been added, fillNoBids() can be called to end the auction.
//
// In Prebid 1.0, this will be simplified to use the `addBidResponse` and `done` callbacks.
const placementCodesHandled = {};
function addBidWithCode(placementCode, bid) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using two names for same thing in our codebase i.e placementCode and adUnitCode. From 1.0 we should just use code or adUnitCode
Gist for adUnit changes https://gist.github.com/mkendall07/51ee5f6b9f2df01a89162cf6de7fe5b6#adunit-obj

Copy link
Contributor Author

@dbemiller dbemiller Sep 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not know that... I removed placementCode from all the comments and local variable names.

I think I still have to use bidRequest.placementCode in Prebid 0.x, though, right? Or is there somewhere else I should be getting it from?

placementCodesHandled[placementCode] = true;
bidmanager.addBidResponse(placementCode, bid);
}
function fillNoBids() {
bidderRequest.bids
.map(bidRequest => bidRequest.placementCode)
.forEach(placementCode => {
if (placementCode && !placementCodesHandled[placementCode]) {
bidmanager.addBidResponse(placementCode, newEmptyBid());
}
});
}

const bidRequests = bidderRequest.bids.filter(filterAndWarn);
if (bidRequests.length === 0) {
fillNoBids();
return;
}
const bidRequestMap = {};
bidRequests.forEach(bid => {
bidRequestMap[bid.bidId] = bid;
});

let requests = spec.buildRequests(bidRequests);
if (!requests || requests.length === 0) {
fillNoBids();
return;
}
if (!Array.isArray(requests)) {
requests = [requests];
}

// After all the responses have come back, fill up the "no bid" bids and
// register any required usersync pixels.
const responses = [];
function afterAllResponses() {
fillNoBids();

if (spec.getUserSyncs) {
// TODO: Before merge, replace this empty object with the real config values.
// Then register them with the UserSync pool. This is waiting on the UserSync PR
// to be merged first, though.
spec.getUserSyncs({ }, responses);
}
}

// Callbacks don't compose as nicely as Promises. We should call fillNoBids() once _all_ the
// Server requests have returned and been processed. Since `ajax` accepts a single callback,
// we need to rig up a function which only executes after all the requests have been responded.
const onResponse = delayExecution(afterAllResponses, requests.length)
requests.forEach(processRequest);

function processRequest(request) {
switch (request.method) {
case 'GET':
ajax(
`${request.url}?${parseQueryStringParameters(request.data)}`,
{
success: onSuccess,
error: onFailure
},
undefined,
{
method: 'GET',
withCredentials: true
}
);
break;
case 'POST':
ajax(
request.url,
{
success: onSuccess,
error: onFailure
},
typeof request.data === 'string' ? request.data : JSON.stringify(request.data),
{
method: 'POST',
contentType: 'text/plain',
withCredentials: true
}
);
break;
default:
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);

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();
Copy link
Collaborator

@snapwich snapwich Aug 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the calling of onResponse here is a good use case for a finally clause so you don't have to scatter as many calls around.

I'd wrap all this code in a try block and put the onResponse in a finally so it's obvious that it will always be called.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the goal here... but I'm really trying not to catch unexpected errors either.

For example, while writing this I originally had a bug in my addBidUsingRequestMap function where something was unexpectedly undefined.

I easily may never have caught that if it'd been inside the try block.

return;
}

if (bids) {
if (bids.forEach) {
bids.forEach(addBidUsingRequestMap);
} else {
addBidUsingRequestMap(bids);
}
}
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.`);
}
}
}

// 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)) {
logWarn(`Invalid bid sent to bidder ${spec.code}: ${JSON.stringify(bid)}`);
return false;
}
return true;
}

function newEmptyBid() {
const bid = bidfactory.createBid(STATUS.NO_BID);
bid.code = spec.code;
bid.bidderCode = spec.code;
return bid;
}
}
17 changes: 17 additions & 0 deletions src/mediaTypes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* This file contains the valid Media Types in Prebid.
*
* All adapters are assumed to support banner ads. Other media types are specified by Adapters when they
* register themselves with prebid-core.
*/

/**
* @typedef {('native'|'video'|'banner')} MediaType
*/

/** @type MediaType */
export const NATIVE = 'native';
/** @type MediaType */
export const VIDEO = 'video';
/** @type MediaType */
export const BANNER = 'banner';
24 changes: 24 additions & 0 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,31 @@ export function getBidderRequest(bidder, adUnitCode) {
}

/**
* Given a function, return a function which only executes the original after
* it's been called numRequiredCalls times.
*
* Note that the arguments from the previous calls will *not* be forwarded to the original function.
* Only the final call's arguments matter.
*
* @param {function} func The function which should be executed, once the returned function has been executed
* numRequiredCalls times.
* @param {int} numRequiredCalls The number of times which the returned function needs to be called before
* func is.
*/
export function delayExecution(func, numRequiredCalls) {
if (numRequiredCalls < 1) {
throw new Error(`numRequiredCalls must be a positive number. Got ${numRequiredCalls}`);
}
let numCalls = 0;
return function () {
numCalls++;
if (numCalls === numRequiredCalls) {
func.apply(null, arguments);
}
}
}

/**
* https://stackoverflow.com/a/34890276/428704
* @export
* @param {array} xs
Expand Down
7 changes: 3 additions & 4 deletions test/spec/modules/appnexusAstBidAdapter_spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { expect } from 'chai';
import Adapter from 'modules/appnexusAstBidAdapter';
import { spec } from 'modules/appnexusAstBidAdapter';
import { newBidder } from 'src/adapters/bidderFactory';
import bidmanager from 'src/bidmanager';

const ENDPOINT = '//ib.adnxs.com/ut/v3/prebid';
Expand Down Expand Up @@ -61,9 +62,7 @@ const RESPONSE = {
};

describe('AppNexusAdapter', () => {
let adapter;

beforeEach(() => adapter = new Adapter());
const adapter = newBidder(spec);

describe('request function', () => {
let xhr;
Expand Down
Loading