From c1080ddd9588e04c5684a20f39d246707c3bd901 Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Thu, 17 Aug 2017 13:27:28 -0400 Subject: [PATCH 01/17] Added a base adapter for single-request adapters, and ported the appnexusAst adapter onto it --- modules/appnexusAstBidAdapter.js | 606 +++++++++--------- src/Renderer.js | 8 + src/adapters/singleRequestBidder.js | 208 ++++++ src/ajax.js | 7 +- .../modules/appnexusAstBidAdapter_spec.js | 6 +- 5 files changed, 508 insertions(+), 327 deletions(-) create mode 100644 src/adapters/singleRequestBidder.js diff --git a/modules/appnexusAstBidAdapter.js b/modules/appnexusAstBidAdapter.js index fd0fe99d19e..d3c2abb09f5 100644 --- a/modules/appnexusAstBidAdapter.js +++ b/modules/appnexusAstBidAdapter.js @@ -1,12 +1,10 @@ -import Adapter from 'src/adapter'; import { Renderer } from 'src/Renderer'; -import bidfactory from 'src/bidfactory'; -import bidmanager from 'src/bidmanager'; import * as utils from 'src/utils'; -import { ajax } from 'src/ajax'; -import { STATUS } from 'src/constants'; import adaptermanager from 'src/adaptermanager'; +import { newBidder } from 'src/adapters/singleRequestBidder'; +import { POST } from '../src/ajax'; +const BIDDER_CODE = 'appnexusAst'; const ENDPOINT = '//ib.adnxs.com/ut/v3/prebid'; const SUPPORTED_AD_TYPES = ['banner', 'video', 'video-outstream', 'native']; const VIDEO_TARGETING = ['id', 'mimes', 'minduration', 'maxduration', @@ -25,356 +23,324 @@ const NATIVE_MAPPING = { sponsoredBy: 'sponsored_by' }; -/** - * Bidder adapter for /ut endpoint. Given the list of all ad unit tag IDs, - * sends out a bid request. When a bid response is back, registers the bid - * to Prebid.js. This adapter supports alias bidding. - */ -function AppnexusAstAdapter() { - let baseAdapter = new Adapter('appnexusAst'); - let bidRequests = {}; - let usersync = false; - - /* Prebid executes this function when the page asks to send out bid requests */ - baseAdapter.callBids = function(bidRequest) { - bidRequests = {}; +const spec = { + code: BIDDER_CODE, + + /** + * Determines whether or not the given bid request is valid. + * + * @param {object} bidParams The params to validate. + * @return boolean True if this is a valid bid, and false otherwise. + */ + areParamsValid: function(bidParams) { + return !!(bidParams.placementId || (bidParams.member && bidParams.invCode)); + }, - const bids = bidRequest.bids || []; - var member = 0; + /** + * Make a server request from the list of BidRequests. + * + * @param {BidRequest[]} bidRequests A non-empty list of bid requests which should be sent to the Server. + * @return ServerRequest Info describing the request to the server. + */ + buildRequest: function(bidRequests) { + const tags = bidRequests.map(bidToTag); + const userObjBid = bidRequests.find(hasUserInfo); let userObj; - const tags = bids - .filter(bid => valid(bid)) - .map(bid => { - // map request id to bid object to retrieve adUnit code in callback - bidRequests[bid.bidId] = bid; - - let tag = {}; - tag.sizes = getSizes(bid.sizes); - tag.primary_size = tag.sizes[0]; - tag.uuid = bid.bidId; - if (bid.params.placementId) { - tag.id = parseInt(bid.params.placementId, 10); - } else { - tag.code = bid.params.invCode; - } - tag.allow_smaller_sizes = bid.params.allowSmallerSizes || false; - tag.prebid = true; - tag.disable_psa = true; - member = parseInt(bid.params.member, 10); - if (bid.params.reserve) { - tag.reserve = bid.params.reserve; - } - if (bid.params.position) { - tag.position = {'above': 1, 'below': 2}[bid.params.position] || 0; - } - if (bid.params.trafficSourceCode) { - tag.traffic_source_code = bid.params.trafficSourceCode; - } - if (bid.params.privateSizes) { - tag.private_sizes = getSizes(bid.params.privateSizes); - } - if (bid.params.supplyType) { - tag.supply_type = bid.params.supplyType; - } - if (bid.params.pubClick) { - tag.pubclick = bid.params.pubClick; - } - if (bid.params.extInvCode) { - tag.ext_inv_code = bid.params.extInvCode; - } - if (bid.params.externalImpId) { - tag.external_imp_id = bid.params.externalImpId; - } - if (!utils.isEmpty(bid.params.keywords)) { - tag.keywords = getKeywords(bid.params.keywords); - } + if (userObjBid) { + userObj = {}; + Object.keys(userObjBid.params.user) + .filter(param => USER_PARAMS.includes(param)) + .forEach(param => userObj[param] = userObjBid.params.user[param]); + } - if (bid.mediaType === 'native') { - tag.ad_types = ['native']; - - if (bid.nativeParams) { - const nativeRequest = {}; - - // map standard prebid native asset identifier to /ut parameters - // e.g., tag specifies `body` but /ut only knows `description` - // mapping may be in form {tag: ''} or - // {tag: {serverName: '', serverParams: {...}}} - Object.keys(bid.nativeParams).forEach(key => { - // check if one of the forms is used, otherwise - // a mapping wasn't specified so pass the key straight through - const requestKey = - (NATIVE_MAPPING[key] && NATIVE_MAPPING[key].serverName) || - NATIVE_MAPPING[key] || - key; - - // if the mapping for this identifier specifies required server - // params via the `serverParams` object, merge that in - const params = Object.assign({}, - bid.nativeParams[key], - NATIVE_MAPPING[key] && NATIVE_MAPPING[key].serverParams - ); - - nativeRequest[requestKey] = params; - }); - - tag.native = {layouts: [nativeRequest]}; - } - } + const memberIdBid = bidRequests.find(hasMemberId); + const member = memberIdBid ? parseInt(memberIdBid.params.member, 10) : 0; - if (bid.mediaType === 'video') { tag.require_asset_url = true; } - if (bid.params.video) { - tag.video = {}; - // place any valid video params on the tag - Object.keys(bid.params.video) - .filter(param => VIDEO_TARGETING.includes(param)) - .forEach(param => tag.video[param] = bid.params.video[param]); - } + const payload = {tags: [...tags], user: userObj}; + if (member > 0) { + payload.member_id = member; + } + const payloadString = JSON.stringify(payload); + return { + type: POST, + endpoint: ENDPOINT, + data: payloadString, + }; + }, - if (bid.params.user) { - userObj = {}; - Object.keys(bid.params.user) - .filter(param => USER_PARAMS.includes(param)) - .forEach(param => userObj[param] = bid.params.user[param]); + /** + * Unpack the response from the server into a list of bids. + * + * @param {*} serverResponse A successful response from the server. + * @return {Bid[]} An array of bids which were nested inside the server. + */ + interpretResponse: function(serverResponse) { + const parsed = JSON.parse(serverResponse); + const bids = []; + parsed.tags.forEach(serverBid => { + const rtbBid = getRtbBid(serverBid); + if (rtbBid) { + if (rtbBid.cpm !== 0 && SUPPORTED_AD_TYPES.includes(rtbBid.ad_type)) { + const bid = newBid(serverBid, rtbBid); + bid.mediaType = parseMediaType(rtbBid); + bids.push(bid); } - - return tag; - }); - - if (!utils.isEmpty(tags)) { - const payloadJson = {tags: [...tags], user: userObj}; - if (member > 0) { - payloadJson.member_id = member; } - const payload = JSON.stringify(payloadJson); - ajax(ENDPOINT, handleResponse, payload, { - contentType: 'text/plain', - withCredentials: true - }); - } - }; - - /* Notify Prebid of bid responses so bids can get in the auction */ - function handleResponse(response) { - let parsed; - - try { - parsed = JSON.parse(response); - } catch (error) { - utils.logError(error); - } + }); + return bids; + } +} - if (!parsed || parsed.error) { - let errorMessage = `in response for ${baseAdapter.getBidderCode()} adapter`; - if (parsed && parsed.error) { errorMessage += `: ${parsed.error}`; } - utils.logError(errorMessage); - - // signal this response is complete - Object.keys(bidRequests) - .map(bidId => bidRequests[bidId].placementCode) - .forEach(placementCode => { - bidmanager.addBidResponse(placementCode, createBid(STATUS.NO_BID)); - }); - return; +// TODO: Patch UserSync into the spec, once the UserSync PR has been merged. +// function userSyncCode() { +// const iframe = utils.createInvisibleIframe(); +// iframe.src = '//acdn.adnxs.com/ib/static/usersync/v3/async_usersync.html'; +// try { +// document.body.appendChild(iframe); +// } catch (error) { +// utils.logError(error); +// } +// } + +function newRenderer(adUnitCode, rtbBid) { + const renderer = Renderer.install({ + id: rtbBid.renderer_id, + url: rtbBid.renderer_url, + config: { adText: `AppNexus Outstream Video Ad via Prebid.js` }, + loaded: false, + }); + renderer.setRender(outstreamRender); + + renderer.setEventHandlers({ + impression: () => utils.logMessage('AppNexus outstream video impression event'), + loaded: () => utils.logMessage('AppNexus outstream video loaded event'), + ended: () => { + utils.logMessage('AppNexus outstream renderer video event'); + document.querySelector(`#${adUnitCode}`).style.display = 'none'; } + }); + return renderer; +} - parsed.tags.forEach(tag => { - const ad = getRtbBid(tag); - const cpm = ad && ad.cpm; - const type = ad && ad.ad_type; +/* Turn keywords parameter into ut-compatible format */ +function getKeywords(keywords) { + let arrs = []; - let status; - if (cpm !== 0 && (SUPPORTED_AD_TYPES.includes(type))) { - status = STATUS.GOOD; + utils._each(keywords, (v, k) => { + if (utils.isArray(v)) { + let values = []; + utils._each(v, (val) => { + val = utils.getValueString('keywords.' + k, val); + if (val) { values.push(val); } + }); + v = values; + } else { + v = utils.getValueString('keywords.' + k, v); + if (utils.isStr(v)) { + v = [v]; } else { - status = STATUS.NO_BID; - } + return; + } // unsuported types - don't send a key + } + arrs.push({key: k, value: v}); + }); - if (type && (!SUPPORTED_AD_TYPES.includes(type))) { - utils.logError(`${type} ad type not supported`); - } + return arrs; +} - tag.bidId = tag.uuid; // bidfactory looks for bidId on requested bid - const bid = createBid(status, tag); - if (type === 'native') bid.mediaType = 'native'; - if (type === 'video') bid.mediaType = 'video'; - if (ad && ad.renderer_url) bid.mediaType = 'video-outstream'; +/** + * Unpack the Server's Bid into a Prebid-compatible one. + * @param serverBid + * @param rtbBid + * @return Bid + */ +function newBid(serverBid, rtbBid) { + const bid = { + requestId: serverBid.uuid, + cpm: rtbBid.cpm, + creative_id: rtbBid.creative_id, + dealId: rtbBid.deal_id, + }; - if (bid.adId in bidRequests) { - const placement = bidRequests[bid.adId].placementCode; - bidmanager.addBidResponse(placement, bid); - } + 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 }); - - if (!usersync) { - const iframe = utils.createInvisibleIframe(); - iframe.src = '//acdn.adnxs.com/ib/static/usersync/v3/async_usersync.html'; - try { - document.body.appendChild(iframe); - } catch (error) { - utils.logError(error); - } - usersync = true; + // This supports Outstream Video + if (rtbBid.renderer_url) { + Object.assign(bid, { + adResponse: serverBid, + renderer: newRenderer(bid.adUnitCode, rtbBid) + }); + bid.adResponse.ad = bid.adResponse.ads[0]; + bid.adResponse.ad.video = bid.adResponse.ad.rtb.video; + } + } else if (rtbBid.rtb.native) { + const native = rtbBid.rtb.native; + bid.native = { + title: native.title, + body: native.desc, + sponsoredBy: native.sponsored, + image: native.main_img && native.main_img.url, + icon: native.icon && native.icon.url, + clickUrl: native.link.url, + impressionTrackers: native.impression_trackers, + }; + } else { + Object.assign(bid, { + width: rtbBid.rtb.banner.width, + height: rtbBid.rtb.banner.height, + ad: rtbBid.rtb.banner.content + }); + try { + const url = rtbBid.rtb.trackers[0].impression_urls[0]; + const tracker = utils.createTrackPixelHtml(url); + bid.ad += tracker; + } catch (error) { + utils.logError('Error appending tracking pixel', error); } } - /* Check that a bid has required paramters */ - function valid(bid) { - if (bid.params.placementId || (bid.params.member && bid.params.invCode)) { - return bid; - } else { - utils.logError('bid requires placementId or (member and invCode) params'); - } + return bid; +} +function bidToTag(bid) { + const tag = {}; + tag.sizes = transformSizes(bid.sizes); + tag.primary_size = tag.sizes[0]; + tag.uuid = bid.bidId; + if (bid.params.placementId) { + tag.id = parseInt(bid.params.placementId, 10); + } else { + tag.code = bid.params.invCode; + } + tag.allow_smaller_sizes = bid.params.allowSmallerSizes || false; + tag.prebid = true; + tag.disable_psa = true; + if (bid.params.reserve) { + tag.reserve = bid.params.reserve; + } + if (bid.params.position) { + tag.position = {'above': 1, 'below': 2}[bid.params.position] || 0; + } + if (bid.params.trafficSourceCode) { + tag.traffic_source_code = bid.params.trafficSourceCode; + } + if (bid.params.privateSizes) { + tag.private_sizes = getSizes(bid.params.privateSizes); + } + if (bid.params.supplyType) { + tag.supply_type = bid.params.supplyType; + } + if (bid.params.pubClick) { + tag.pubclick = bid.params.pubClick; + } + if (bid.params.extInvCode) { + tag.ext_inv_code = bid.params.extInvCode; + } + if (bid.params.externalImpId) { + tag.external_imp_id = bid.params.externalImpId; + } + if (!utils.isEmpty(bid.params.keywords)) { + tag.keywords = getKeywords(bid.params.keywords); } - /* Turn keywords parameter into ut-compatible format */ - function getKeywords(keywords) { - let arrs = []; - - utils._each(keywords, (v, k) => { - if (utils.isArray(v)) { - let values = []; - utils._each(v, (val) => { - val = utils.getValueString('keywords.' + k, val); - if (val) { values.push(val); } - }); - v = values; - } else { - v = utils.getValueString('keywords.' + k, v); - if (utils.isStr(v)) { v = [v]; } else { return; } // unsuported types - don't send a key - } - arrs.push({key: k, value: v}); - }); + if (bid.mediaType === 'native') { + tag.ad_types = ['native']; + + if (bid.nativeParams) { + const nativeRequest = {}; + + // map standard prebid native asset identifier to /ut parameters + // e.g., tag specifies `body` but /ut only knows `description` + // mapping may be in form {tag: ''} or + // {tag: {serverName: '', serverParams: {...}}} + Object.keys(bid.nativeParams).forEach(key => { + // check if one of the forms is used, otherwise + // a mapping wasn't specified so pass the key straight through + const requestKey = + (NATIVE_MAPPING[key] && NATIVE_MAPPING[key].serverName) || + NATIVE_MAPPING[key] || + key; + + // if the mapping for this identifier specifies required server + // params via the `serverParams` object, merge that in + const params = Object.assign({}, + bid.nativeParams[key], + NATIVE_MAPPING[key] && NATIVE_MAPPING[key].serverParams + ); + + nativeRequest[requestKey] = params; + }); - return arrs; + tag.native = {layouts: [nativeRequest]}; + } + } + + if (bid.mediaType === 'video') { tag.require_asset_url = true; } + if (bid.params.video) { + tag.video = {}; + // place any valid video params on the tag + Object.keys(bid.params.video) + .filter(param => VIDEO_TARGETING.includes(param)) + .forEach(param => tag.video[param] = bid.params.video[param]); } - /* Turn bid request sizes into ut-compatible format */ - function getSizes(requestSizes) { - let sizes = []; - let sizeObj = {}; + return tag; +} - if (utils.isArray(requestSizes) && requestSizes.length === 2 && - !utils.isArray(requestSizes[0])) { - sizeObj.width = parseInt(requestSizes[0], 10); - sizeObj.height = parseInt(requestSizes[1], 10); +/* Turn bid request sizes into ut-compatible format */ +function transformSizes(requestSizes) { + let sizes = []; + let sizeObj = {}; + + if (utils.isArray(requestSizes) && requestSizes.length === 2 && + !utils.isArray(requestSizes[0])) { + sizeObj.width = parseInt(requestSizes[0], 10); + sizeObj.height = parseInt(requestSizes[1], 10); + sizes.push(sizeObj); + } else if (typeof requestSizes === 'object') { + for (let i = 0; i < requestSizes.length; i++) { + let size = requestSizes[i]; + sizeObj = {}; + sizeObj.width = parseInt(size[0], 10); + sizeObj.height = parseInt(size[1], 10); sizes.push(sizeObj); - } else if (typeof requestSizes === 'object') { - for (let i = 0; i < requestSizes.length; i++) { - let size = requestSizes[i]; - sizeObj = {}; - sizeObj.width = parseInt(size[0], 10); - sizeObj.height = parseInt(size[1], 10); - sizes.push(sizeObj); - } } - - return sizes; } - function getRtbBid(tag) { - return tag && tag.ads && tag.ads.length && tag.ads.find(ad => ad.rtb); - } + return sizes; +} - function outstreamRender(bid) { - // push to render queue because ANOutstreamVideo may not be loaded yet - bid.renderer.push(() => { - window.ANOutstreamVideo.renderAd({ - tagId: bid.adResponse.tag_id, - sizes: [bid.getSize().split('x')], - targetId: bid.adUnitCode, // target div id to render video - uuid: bid.adResponse.uuid, - adResponse: bid.adResponse, - rendererOptions: bid.renderer.getConfig() - }, handleOutstreamRendererEvents.bind(bid)); - }); - } +function hasUserInfo(bid) { + return !!bid.params.user; +} - function handleOutstreamRendererEvents(id, eventName) { - const bid = this; - bid.renderer.handleVideoEvent({ id, eventName }); - } +function hasMemberId(bid) { + return !!parseInt(bid.params.member, 10); +} - /* Create and return a bid object based on status and tag */ - function createBid(status, tag) { - const ad = getRtbBid(tag); - let bid = bidfactory.createBid(status, tag); - bid.code = baseAdapter.getBidderCode(); - bid.bidderCode = baseAdapter.getBidderCode(); - - if (ad && status === STATUS.GOOD) { - bid.cpm = ad.cpm; - bid.creative_id = ad.creative_id; - bid.dealId = ad.deal_id; - - if (ad.rtb.video) { - bid.width = ad.rtb.video.player_width; - bid.height = ad.rtb.video.player_height; - bid.vastUrl = ad.rtb.video.asset_url; - bid.descriptionUrl = ad.rtb.video.asset_url; - if (ad.renderer_url) { - // outstream video - - bid.adResponse = tag; - bid.renderer = Renderer.install({ - id: ad.renderer_id, - url: ad.renderer_url, - config: { adText: `AppNexus Outstream Video Ad via Prebid.js` }, - loaded: false, - }); - try { - bid.renderer.setRender(outstreamRender); - } catch (err) { - utils.logWarning('Prebid Error calling setRender on renderer', err); - } - - bid.renderer.setEventHandlers({ - impression: () => utils.logMessage('AppNexus outstream video impression event'), - loaded: () => utils.logMessage('AppNexus outstream video loaded event'), - ended: () => { - utils.logMessage('AppNexus outstream renderer video event'); - document.querySelector(`#${bid.adUnitCode}`).style.display = 'none'; - } - }); - - bid.adResponse.ad = bid.adResponse.ads[0]; - bid.adResponse.ad.video = bid.adResponse.ad.rtb.video; - } - } else if (ad.rtb.native) { - const native = ad.rtb.native; - bid.native = { - title: native.title, - body: native.desc, - sponsoredBy: native.sponsored, - image: native.main_img && native.main_img.url, - icon: native.icon && native.icon.url, - clickUrl: native.link.url, - impressionTrackers: native.impression_trackers, - }; - } else { - bid.width = ad.rtb.banner.width; - bid.height = ad.rtb.banner.height; - bid.ad = ad.rtb.banner.content; - try { - const url = ad.rtb.trackers[0].impression_urls[0]; - const tracker = utils.createTrackPixelHtml(url); - bid.ad += tracker; - } catch (error) { - utils.logError('Error appending tracking pixel', error); - } - } - } +function getRtbBid(tag) { + return tag && tag.ads && tag.ads.length && tag.ads.find(ad => ad.rtb); +} - return bid; +function parseMediaType(rtbBid) { + const adType = rtbBid.ad_type; + if (rtbBid.renderer_url) { + return 'video-outstream'; + } else if (adType === 'video') { + return 'video'; + } else if (adType === 'native') { + return 'native'; + } else { + return 'banner'; } - - return Object.assign(this, { - callBids: baseAdapter.callBids, - setBidderCode: baseAdapter.setBidderCode, - }); } -adaptermanager.registerBidAdapter(new AppnexusAstAdapter(), 'appnexusAst', { +export const adapter = newBidder(spec); + +adaptermanager.registerBidAdapter(adapter, 'appnexusAst', { supportedMediaTypes: ['video', 'native'] }); - -module.exports = AppnexusAstAdapter; diff --git a/src/Renderer.js b/src/Renderer.js index c532b164fea..3ef0be5ae4d 100644 --- a/src/Renderer.js +++ b/src/Renderer.js @@ -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; diff --git a/src/adapters/singleRequestBidder.js b/src/adapters/singleRequestBidder.js new file mode 100644 index 00000000000..77f99ab3ea0 --- /dev/null +++ b/src/adapters/singleRequestBidder.js @@ -0,0 +1,208 @@ +import Adapter from 'src/adapter'; +import bidmanager from 'src/bidmanager'; +import bidfactory from 'src/bidfactory'; +import { STATUS } from 'src/constants'; + +import { ajax, GET, POST } from 'src/ajax'; +import { logWarn, logError, parseQueryStringParameters } from 'src/utils'; + +/** + * This file supports bidders which follow a "single request architecture." + * That is, they take _all_ the bid request data, make a single request to the server, and receive a + * response containing _all_ the bids. + * + * Typical usage looks something like: + * + * const adapter = newBidder({ + * code: 'myBidderCode', + * areParamsValid: function(paramsObject) { return true/false }, + * buildRequest: function(bidRequests) { return some ServerRequest }, + * interpretResponse: function(anything) { return some Bids, or throw an error. } + * }); + */ + +/** + * This function aims to minimize the Adapter-specific code for "single request" Bidders. That is, if your adapter + * only makes a single request to the server for bids, regardless of how many bid requests or ad units are in + * the Auction, then this function is for you. + * + * @param {SingleRequestBidder} spec An object containing the bare-bones functions we need to make a Bidder. + */ +export function newBidder(spec) { + return Object.assign(new Adapter(spec.code), { + callBids: function(bidsRequest) { + if (!bidsRequest.bids || !bidsRequest.bids.filter) { + return; + } + const bidRequests = bidsRequest.bids.filter(filterAndWarn); + if (bidRequests.length === 0) { + return; + } + const request = spec.buildRequest(bidRequests); + + function onSuccess(response) { + const bidRequestMap = {}; + bidRequests.forEach(bid => { + bidRequestMap[bid.bidId] = bid; + }) + + 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); + addEmptyBids(bidRequests); + return; + } + + bids.forEach((bid) => { + const copy = copyWithDefaults(bid, bidRequestMap[bid.requestId]); + bidmanager.addBidResponse(bidRequestMap[bid.requestId].placementCode, copy); + }); + if (bids.length === 0) { + addEmptyBids(bidRequests); + } + } + + function onError(err) { + logError(`Server call for ${spec.code} failed. Continuing without bids.`, null, err); + addEmptyBids(bidRequests); + } + + switch (request.type) { + case GET: + ajax( + `${request.endpoint}?${parseQueryStringParameters(request.data)}`, + { + success: onSuccess, + error: onError + }, + undefined, + { + method: GET, + withCredentials: true + } + ); + break; + case POST: + ajax( + request.endpoint, + { + success: onSuccess, + error: onError + }, + typeof request.data === 'string' ? request.data : JSON.stringify(request.data), + { + method: POST, + contentType: 'text/plain', + withCredentials: true + } + ); + break; + default: + throw new Error(`buildRequest() must return a bid whose type is either "get" or "post". Got ${request.type}`); + } + } + }); + + function filterAndWarn(bid) { + if (!spec.areParamsValid(bid.params)) { + logWarn(`Invalid bid sent to bidder ${spec.code}: ${JSON.stringify(bid)}`); + return false; + } + return true; + } + + function addEmptyBids(bidRequests) { + Object.keys(bidRequests) + .map(bidRequest => bidRequest.placementCode) + .forEach(placementCode => { + bidmanager.addBidResponse(placementCode, newEmptyBid()); + }); + } + + function newEmptyBid() { + const bid = bidfactory.createBid(STATUS.NO_BID); + bid.code = spec.code; + bid.bidderCode = spec.code; + return bid; + } +} + +function copyWithDefaults(bid, bidRequest) { + return Object.assign({ + getStatusCode: function() { + return STATUS.GOOD; + }, + getSize: function() { + return bid.width + 'x' + bid.height; + }, + bidderCode: bidRequest.bidder, + code: bidRequest.bidder, + mediaType: 'banner', + adId: bidRequest.bidId, + width: 0, + height: 0, + statusMessage: 'Bid available', + }, bid); +} + +/** + * @typedef {object} ServerRequest + * + * @param {('GET'|'POST')} type The type of request which this is. + * @param {string} endpoint The endpoint for the request. For example, "//bids.example.com". + * @param {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} BidRequest + * + * @param {string} bidId A string which uniquely identifies this BidRequest in the current Auction. + * @param {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} Bid + * + * @param {string} requestId The specific BidRequest which this bid is aimed at. + * This should correspond to one of the + * @param {string} ad A URL which can be used to load this ad, if it's chosen by the publisher. + * @param {number} cpm The bid price, in US cents per thousand impressions. + * @param {number} height The height of the ad, in pixels. + * @param {number} width The width of the ad, in pixels. + * + * @param [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 bids. + * @param [UserSyncInfo] userSyncs An object with the data needed to run a user sync, for this bid. + */ + +/** + * @typedef {object} SingleRequestBidder 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 {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} buildRequest 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(): UserSyncInfo[]} fetchUserSyncs + */ + +/** + * TODO: Move this to the UserSync module after that PR is merged. + * + * @typedef {object} UserSyncInfo + * + * @param {('image'|'iframe')} type The type of user sync to be done. + * @param {string} url The URL which makes the sync happen. + */ diff --git a/src/ajax.js b/src/ajax.js index 02b699d7e9a..fb47b5d23b8 100644 --- a/src/ajax.js +++ b/src/ajax.js @@ -5,6 +5,9 @@ var utils = require('./utils'); const XHR_DONE = 4; let _timeout = 3000; +export const GET = 'GET'; +export const POST = 'POST'; + /** * Simple IE9+ and cross-browser ajax request function * Note: x-domain requests in IE9 do not support the use of cookies @@ -22,7 +25,7 @@ export function ajax(url, callback, data, options = {}) { try { let x; let useXDomainRequest = false; - let method = options.method || (data ? 'POST' : 'GET'); + let method = options.method || (data ? POST : GET); let callbacks = typeof callback === 'object' ? callback : { success: function() { @@ -97,7 +100,7 @@ export function ajax(url, callback, data, options = {}) { } x.setRequestHeader('Content-Type', options.contentType || 'text/plain'); } - x.send(method === 'POST' && data); + x.send(method === POST && data); } catch (error) { utils.logError('xhr construction', error); } diff --git a/test/spec/modules/appnexusAstBidAdapter_spec.js b/test/spec/modules/appnexusAstBidAdapter_spec.js index 6ed1b4d8565..c94d4eaf098 100644 --- a/test/spec/modules/appnexusAstBidAdapter_spec.js +++ b/test/spec/modules/appnexusAstBidAdapter_spec.js @@ -1,5 +1,5 @@ import { expect } from 'chai'; -import Adapter from 'modules/appnexusAstBidAdapter'; +import { adapter } from 'modules/appnexusAstBidAdapter'; import bidmanager from 'src/bidmanager'; const ENDPOINT = '//ib.adnxs.com/ut/v3/prebid'; @@ -61,10 +61,6 @@ const RESPONSE = { }; describe('AppNexusAdapter', () => { - let adapter; - - beforeEach(() => adapter = new Adapter()); - describe('request function', () => { let xhr; let requests; From 70fcf5173515b99a1ea115d64bb1585c7bc28030 Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Fri, 18 Aug 2017 12:33:16 -0400 Subject: [PATCH 02/17] Renamed the SingleRequestBidder to BidderFactory. Updated it to handle multi-request architecture adapters. --- modules/appnexusAstBidAdapter.js | 4 +- ...ingleRequestBidder.js => bidderFactory.js} | 179 ++++++++++-------- src/utils.js | 25 +++ 3 files changed, 126 insertions(+), 82 deletions(-) rename src/adapters/{singleRequestBidder.js => bidderFactory.js} (50%) diff --git a/modules/appnexusAstBidAdapter.js b/modules/appnexusAstBidAdapter.js index d3c2abb09f5..13247dddd8a 100644 --- a/modules/appnexusAstBidAdapter.js +++ b/modules/appnexusAstBidAdapter.js @@ -1,7 +1,7 @@ import { Renderer } from 'src/Renderer'; import * as utils from 'src/utils'; import adaptermanager from 'src/adaptermanager'; -import { newBidder } from 'src/adapters/singleRequestBidder'; +import { newBidder } from 'src/adapters/bidderFactory'; import { POST } from '../src/ajax'; const BIDDER_CODE = 'appnexusAst'; @@ -42,7 +42,7 @@ const spec = { * @param {BidRequest[]} bidRequests A non-empty list of bid requests which should be sent to the Server. * @return ServerRequest Info describing the request to the server. */ - buildRequest: function(bidRequests) { + buildRequests: function(bidRequests) { const tags = bidRequests.map(bidToTag); const userObjBid = bidRequests.find(hasUserInfo); let userObj; diff --git a/src/adapters/singleRequestBidder.js b/src/adapters/bidderFactory.js similarity index 50% rename from src/adapters/singleRequestBidder.js rename to src/adapters/bidderFactory.js index 77f99ab3ea0..27568ca7df2 100644 --- a/src/adapters/singleRequestBidder.js +++ b/src/adapters/bidderFactory.js @@ -4,20 +4,23 @@ import bidfactory from 'src/bidfactory'; import { STATUS } from 'src/constants'; import { ajax, GET, POST } from 'src/ajax'; -import { logWarn, logError, parseQueryStringParameters } from 'src/utils'; +import { logWarn, logError, parseQueryStringParameters, delayExecution } from 'src/utils'; /** - * This file supports bidders which follow a "single request architecture." - * That is, they take _all_ the bid request data, make a single request to the server, and receive a - * response containing _all_ the bids. + * 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 = newBidder({ * code: 'myBidderCode', * areParamsValid: function(paramsObject) { return true/false }, - * buildRequest: function(bidRequests) { return some ServerRequest }, - * interpretResponse: function(anything) { return some Bids, or throw an error. } + * buildRequests: function(bidRequests) { return some ServerRequest(s) }, + * interpretResponse: function(oneServerResponse) { return some Bids, or throw an error. } * }); */ @@ -26,7 +29,7 @@ import { logWarn, logError, parseQueryStringParameters } from 'src/utils'; * only makes a single request to the server for bids, regardless of how many bid requests or ad units are in * the Auction, then this function is for you. * - * @param {SingleRequestBidder} spec An object containing the bare-bones functions we need to make a Bidder. + * @param {BidderSpec} spec An object containing the bare-bones functions we need to make a Bidder. */ export function newBidder(spec) { return Object.assign(new Adapter(spec.code), { @@ -34,12 +37,89 @@ export function newBidder(spec) { if (!bidsRequest.bids || !bidsRequest.bids.filter) { 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) { + placementCodesHandled[placementCode] = true; + bidmanager.addBidResponse(placementCode, bid); + } + function fillNoBids() { + bidsRequest.bids + .map(bidRequest => bidRequest.placementCode) + .forEach(placementCode => { + if (placementCode && !placementCodesHandled[placementCode]) { + bidmanager.addBidResponse(placementCode, newEmptyBid()); + } + }); + } + const bidRequests = bidsRequest.bids.filter(filterAndWarn); if (bidRequests.length === 0) { + fillNoBids(); return; } - const request = spec.buildRequest(bidRequests); + let requests = spec.buildRequests(bidRequests); + if (!requests || requests.length === 0) { + fillNoBids(); + return; + } + if (!Array.isArray(requests)) { + requests = [requests]; + } + + // 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(fillNoBids, requests.length) + requests.forEach(processRequest); + + function processRequest(request) { + switch (request.type) { + case GET: + ajax( + `${request.endpoint}?${parseQueryStringParameters(request.data)}`, + { + success: onSuccess, + error: onError + }, + undefined, + { + method: GET, + withCredentials: true + } + ); + break; + case POST: + ajax( + request.endpoint, + { + success: onSuccess, + error: onError + }, + typeof request.data === 'string' ? request.data : JSON.stringify(request.data), + { + method: POST, + contentType: 'text/plain', + withCredentials: true + } + ); + break; + default: + throw new Error(`buildRequest() must return a bid whose type is either "get" or "post". Got ${request.type}`); + } + } + + // 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) { const bidRequestMap = {}; bidRequests.forEach(bid => { @@ -51,56 +131,22 @@ export function newBidder(spec) { bids = spec.interpretResponse(response); } catch (err) { logError(`Bidder ${spec.code} failed to interpret the server's response. Continuing without bids`, null, err); - addEmptyBids(bidRequests); + onResponse(); return; } bids.forEach((bid) => { - const copy = copyWithDefaults(bid, bidRequestMap[bid.requestId]); - bidmanager.addBidResponse(bidRequestMap[bid.requestId].placementCode, copy); + const prebidBid = Object.assign(bidfactory.createBid(STATUS.GOOD, bidRequestMap[bid.requestId]), bid); + addBidWithCode(bidRequestMap[bid.requestId].placementCode, prebidBid); }); - if (bids.length === 0) { - addEmptyBids(bidRequests); - } + onResponse(); } - function onError(err) { + // 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 onError() { logError(`Server call for ${spec.code} failed. Continuing without bids.`, null, err); - addEmptyBids(bidRequests); - } - - switch (request.type) { - case GET: - ajax( - `${request.endpoint}?${parseQueryStringParameters(request.data)}`, - { - success: onSuccess, - error: onError - }, - undefined, - { - method: GET, - withCredentials: true - } - ); - break; - case POST: - ajax( - request.endpoint, - { - success: onSuccess, - error: onError - }, - typeof request.data === 'string' ? request.data : JSON.stringify(request.data), - { - method: POST, - contentType: 'text/plain', - withCredentials: true - } - ); - break; - default: - throw new Error(`buildRequest() must return a bid whose type is either "get" or "post". Got ${request.type}`); + onResponse(); } } }); @@ -113,14 +159,6 @@ export function newBidder(spec) { return true; } - function addEmptyBids(bidRequests) { - Object.keys(bidRequests) - .map(bidRequest => bidRequest.placementCode) - .forEach(placementCode => { - bidmanager.addBidResponse(placementCode, newEmptyBid()); - }); - } - function newEmptyBid() { const bid = bidfactory.createBid(STATUS.NO_BID); bid.code = spec.code; @@ -129,24 +167,6 @@ export function newBidder(spec) { } } -function copyWithDefaults(bid, bidRequest) { - return Object.assign({ - getStatusCode: function() { - return STATUS.GOOD; - }, - getSize: function() { - return bid.width + 'x' + bid.height; - }, - bidderCode: bidRequest.bidder, - code: bidRequest.bidder, - mediaType: 'banner', - adId: bidRequest.bidId, - width: 0, - height: 0, - statusMessage: 'Bid available', - }, bid); -} - /** * @typedef {object} ServerRequest * @@ -177,19 +197,18 @@ function copyWithDefaults(bid, bidRequest) { * @param {number} width The width of the ad, in pixels. * * @param [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 bids. - * @param [UserSyncInfo] userSyncs An object with the data needed to run a user sync, for this bid. + * if the publisher doesn't override it. This is only relevant for Outstream Video bids. */ /** - * @typedef {object} SingleRequestBidder An object containing the adapter-specific functions needed to + * @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 {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} buildRequest Build the request to the Server which + * @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 diff --git a/src/utils.js b/src/utils.js index 15a40edf1c7..5441db39e01 100644 --- a/src/utils.js +++ b/src/utils.js @@ -662,3 +662,28 @@ export function getBidderRequest(bidder, adUnitCode) { .filter(bid => bid.bidder === bidder && bid.placementCode === adUnitCode).length > 0; }) || { start: null, requestId: null }; } + +/** + * 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); + } + } +} From 211e7fc32fbb7b1eb9ba10fb92139ba131f3fd7d Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Fri, 18 Aug 2017 12:49:09 -0400 Subject: [PATCH 03/17] Added a unit test for the delayExecution function. --- test/spec/utils_spec.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/spec/utils_spec.js b/test/spec/utils_spec.js index 2bbdcbaf6e6..d3055cc455c 100755 --- a/test/spec/utils_spec.js +++ b/test/spec/utils_spec.js @@ -524,4 +524,18 @@ describe('Utils', function () { assert.equal(arr.length, count, 'Polyfill test fails') }); }); + + describe('delayExecution', function () { + it('should execute the core function after the correct number of calls', function () { + const callback = sinon.spy(); + const delayed = utils.delayExecution(callback, 5); + for (let i = 0; i < 4; i++) { + delayed(); + } + assert(callback.notCalled); + delayed(3); + assert(callback.called) + assert.equal(callback.firstCall.args[0], 3); + }); + }); }); From d509150e47a8b187b2e5326e378d6706e9b50aca Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Fri, 18 Aug 2017 16:59:17 -0400 Subject: [PATCH 04/17] Made newBidder a default import. Added some unit tests. --- modules/appnexusAstBidAdapter.js | 2 +- src/adapters/bidderFactory.js | 2 +- test/spec/unit/core/bidderFactory_spec.js | 127 ++++++++++++++++++++++ 3 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 test/spec/unit/core/bidderFactory_spec.js diff --git a/modules/appnexusAstBidAdapter.js b/modules/appnexusAstBidAdapter.js index 13247dddd8a..1b5970916ac 100644 --- a/modules/appnexusAstBidAdapter.js +++ b/modules/appnexusAstBidAdapter.js @@ -1,7 +1,7 @@ import { Renderer } from 'src/Renderer'; import * as utils from 'src/utils'; import adaptermanager from 'src/adaptermanager'; -import { newBidder } from 'src/adapters/bidderFactory'; +import newBidder from 'src/adapters/bidderFactory'; import { POST } from '../src/ajax'; const BIDDER_CODE = 'appnexusAst'; diff --git a/src/adapters/bidderFactory.js b/src/adapters/bidderFactory.js index 27568ca7df2..d1b95c94ba6 100644 --- a/src/adapters/bidderFactory.js +++ b/src/adapters/bidderFactory.js @@ -31,7 +31,7 @@ import { logWarn, logError, parseQueryStringParameters, delayExecution } from 's * * @param {BidderSpec} spec An object containing the bare-bones functions we need to make a Bidder. */ -export function newBidder(spec) { +export default function newBidder(spec) { return Object.assign(new Adapter(spec.code), { callBids: function(bidsRequest) { if (!bidsRequest.bids || !bidsRequest.bids.filter) { diff --git a/test/spec/unit/core/bidderFactory_spec.js b/test/spec/unit/core/bidderFactory_spec.js new file mode 100644 index 00000000000..9059f271222 --- /dev/null +++ b/test/spec/unit/core/bidderFactory_spec.js @@ -0,0 +1,127 @@ +import newBidder from 'src/adapters/bidderFactory'; +import bidmanager from 'src/bidmanager'; +import * as ajax from 'src/ajax'; +import { expect } from 'chai'; + +const CODE = 'sampleBidder'; +const MOCK_BIDS_REQUEST = { + bids: [ + { + placementCode: 'mock/placement', + params: { + param: 5 + } + }, + { + placementCode: 'mock/placement2', + params: { + badParam: 6 + } + } + ] +} + +describe('The bidder factory', () => { + let spec; + let bidder; + let addBidRequestStub; + let ajaxMock; + + beforeEach(() => { + spec = { + code: CODE, + areParamsValid: sinon.stub(), + buildRequests: sinon.stub(), + interpretResponse: sinon.stub() + }; + bidder = newBidder(spec); + addBidRequestStub = sinon.stub(bidmanager, 'addBidResponse'); + ajaxMock = sinon.mock(ajax); + }); + + afterEach(() => { + addBidRequestStub.restore(); + ajaxMock.restore(); + }) + + it('should handle bad bid requests gracefully', () => { + ajaxMock.expects('ajax').never(); + + bidder.callBids({}); + bidder.callBids({ bids: 'nothing useful' }); + expect(spec.areParamsValid.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.', () => { + spec.areParamsValid.returns(true); + spec.buildRequests.returns([]); + ajaxMock.expects('ajax').never(); + + bidder.callBids(MOCK_BIDS_REQUEST); + + expect(spec.areParamsValid.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.', () => { + spec.areParamsValid.returns(false); + spec.buildRequests.returns([]); + ajaxMock.expects('ajax').never(); + + bidder.callBids(MOCK_BIDS_REQUEST); + + expect(spec.areParamsValid.calledTwice).to.equal(true); + expect(spec.buildRequests.called).to.equal(false); + }); + + it('should filter out invalid bids before calling buildRequests.', () => { + spec.areParamsValid.onFirstCall().returns(true); + spec.areParamsValid.onSecondCall().returns(false); + spec.buildRequests.returns([]); + ajaxMock.expects('ajax').never(); + + bidder.callBids(MOCK_BIDS_REQUEST); + + expect(spec.areParamsValid.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 the appropriate POST requests.', () => { + const url = 'test.url.com'; + const data = { arg: 2 }; + spec.areParamsValid.returns(true); + spec.buildRequests.returns({ + type: 'POST', + endpoint: url, + data: data + }); + ajaxMock.expects('ajax').once().withArgs(url, sinon.match.object, JSON.stringify(data), { + method: 'POST', + contentType: 'text/plain', + withCredentials: true + }); + + bidder.callBids(MOCK_BIDS_REQUEST); + }); + + it('should make the appropriate GET requests.', () => { + const url = 'test.url.com'; + const data = { arg: 2 }; + spec.areParamsValid.returns(true); + spec.buildRequests.returns({ + type: 'GET', + endpoint: url, + data: data + }); + ajaxMock.expects('ajax').once().withArgs(`${url}?arg=2&`, sinon.match.object, undefined, { + method: 'GET', + withCredentials: true + }); + + bidder.callBids(MOCK_BIDS_REQUEST); + }); +}); From 8d4ebc1ebdc1aabae98f593a0939c4673424d637 Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Mon, 21 Aug 2017 11:31:36 -0400 Subject: [PATCH 05/17] Added more tests. --- test/spec/unit/core/bidderFactory_spec.js | 55 ++++++++++++++++++++--- 1 file changed, 50 insertions(+), 5 deletions(-) diff --git a/test/spec/unit/core/bidderFactory_spec.js b/test/spec/unit/core/bidderFactory_spec.js index 9059f271222..424f3a4adbf 100644 --- a/test/spec/unit/core/bidderFactory_spec.js +++ b/test/spec/unit/core/bidderFactory_spec.js @@ -49,35 +49,39 @@ describe('The bidder factory', () => { bidder.callBids({}); bidder.callBids({ bids: 'nothing useful' }); + + ajaxMock.verify(); expect(spec.areParamsValid.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.', () => { + it('should call buildRequests(bidRequest) the params are valid', () => { spec.areParamsValid.returns(true); spec.buildRequests.returns([]); ajaxMock.expects('ajax').never(); bidder.callBids(MOCK_BIDS_REQUEST); + ajaxMock.verify(); expect(spec.areParamsValid.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.', () => { + it('should not call buildRequests the params are invalid', () => { spec.areParamsValid.returns(false); spec.buildRequests.returns([]); ajaxMock.expects('ajax').never(); bidder.callBids(MOCK_BIDS_REQUEST); + ajaxMock.verify(); expect(spec.areParamsValid.calledTwice).to.equal(true); expect(spec.buildRequests.called).to.equal(false); }); - it('should filter out invalid bids before calling buildRequests.', () => { + it('should filter out invalid bids before calling buildRequests', () => { spec.areParamsValid.onFirstCall().returns(true); spec.areParamsValid.onSecondCall().returns(false); spec.buildRequests.returns([]); @@ -85,12 +89,23 @@ describe('The bidder factory', () => { bidder.callBids(MOCK_BIDS_REQUEST); + ajaxMock.verify(); expect(spec.areParamsValid.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 the appropriate POST requests.', () => { + it("should make no server requests if the spec doesn't return any", () => { + spec.areParamsValid.returns(true); + spec.buildRequests.returns([]); + ajaxMock.expects('ajax').never(); + + bidder.callBids(MOCK_BIDS_REQUEST); + + ajaxMock.verify(); + }); + + it('should make the appropriate POST request', () => { const url = 'test.url.com'; const data = { arg: 2 }; spec.areParamsValid.returns(true); @@ -106,9 +121,11 @@ describe('The bidder factory', () => { }); bidder.callBids(MOCK_BIDS_REQUEST); + + ajaxMock.verify(); }); - it('should make the appropriate GET requests.', () => { + it('should make the appropriate GET request', () => { const url = 'test.url.com'; const data = { arg: 2 }; spec.areParamsValid.returns(true); @@ -123,5 +140,33 @@ describe('The bidder factory', () => { }); bidder.callBids(MOCK_BIDS_REQUEST); + + ajaxMock.verify(); + }); + + it('should make multiple calls if the spec returns them', () => { + const url = 'test.url.com'; + const data = { arg: 2 }; + spec.areParamsValid.returns(true); + spec.buildRequests.returns([ + { + type: 'POST', + endpoint: url, + data: data + }, + { + type: 'GET', + endpoint: url, + data: data + } + ]); + + ajaxMock.expects('ajax').twice(); + + bidder.callBids(MOCK_BIDS_REQUEST); + + ajaxMock.verify(); }); + + // TODO: Pending the sinon 3.0 upgrade, mock the ajax calls and make sure it calls interpretResponse on each. }); From 1f23044328e350f0b9393751b6f33dc87ccebf07 Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Tue, 22 Aug 2017 10:16:23 -0400 Subject: [PATCH 06/17] Added more tests, and fixed a few bugs. --- src/adapters/bidderFactory.js | 30 +- test/spec/unit/core/bidderFactory_spec.js | 319 +++++++++++++++------- 2 files changed, 243 insertions(+), 106 deletions(-) diff --git a/src/adapters/bidderFactory.js b/src/adapters/bidderFactory.js index d1b95c94ba6..a12525e14c3 100644 --- a/src/adapters/bidderFactory.js +++ b/src/adapters/bidderFactory.js @@ -88,7 +88,7 @@ export default function newBidder(spec) { `${request.endpoint}?${parseQueryStringParameters(request.data)}`, { success: onSuccess, - error: onError + error: onFailure }, undefined, { @@ -102,7 +102,7 @@ export default function newBidder(spec) { request.endpoint, { success: onSuccess, - error: onError + error: onFailure }, typeof request.data === 'string' ? request.data : JSON.stringify(request.data), { @@ -124,7 +124,16 @@ export default function newBidder(spec) { const bidRequestMap = {}; bidRequests.forEach(bid => { bidRequestMap[bid.bidId] = bid; - }) + }); + 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.`); + } + } let bids; try { @@ -135,17 +144,20 @@ export default function newBidder(spec) { return; } - bids.forEach((bid) => { - const prebidBid = Object.assign(bidfactory.createBid(STATUS.GOOD, bidRequestMap[bid.requestId]), bid); - addBidWithCode(bidRequestMap[bid.requestId].placementCode, prebidBid); - }); + if (bids) { + if (bids.forEach) { + bids.forEach(addBidUsingRequestMap); + } else { + addBidUsingRequestMap(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 onError() { - logError(`Server call for ${spec.code} failed. Continuing without bids.`, null, err); + function onFailure(err) { + logError(`Server call for ${spec.code} failed: ${err}. Continuing without bids.`); onResponse(); } } diff --git a/test/spec/unit/core/bidderFactory_spec.js b/test/spec/unit/core/bidderFactory_spec.js index 424f3a4adbf..aacb5046045 100644 --- a/test/spec/unit/core/bidderFactory_spec.js +++ b/test/spec/unit/core/bidderFactory_spec.js @@ -2,17 +2,20 @@ import newBidder from 'src/adapters/bidderFactory'; import bidmanager from 'src/bidmanager'; import * as ajax from 'src/ajax'; import { expect } from 'chai'; +import { STATUS } from 'src/constants'; const CODE = 'sampleBidder'; const MOCK_BIDS_REQUEST = { bids: [ { + requestId: 'first-bid-id', placementCode: 'mock/placement', params: { param: 5 } }, { + requestId: 'second-bid-id', placementCode: 'mock/placement2', params: { badParam: 6 @@ -25,7 +28,6 @@ describe('The bidder factory', () => { let spec; let bidder; let addBidRequestStub; - let ajaxMock; beforeEach(() => { spec = { @@ -36,137 +38,260 @@ describe('The bidder factory', () => { }; bidder = newBidder(spec); addBidRequestStub = sinon.stub(bidmanager, 'addBidResponse'); - ajaxMock = sinon.mock(ajax); }); afterEach(() => { addBidRequestStub.restore(); - ajaxMock.restore(); - }) + }); - it('should handle bad bid requests gracefully', () => { - ajaxMock.expects('ajax').never(); + describe('when the ajax response is irrelevant', () => { + let ajaxStub; - bidder.callBids({}); - bidder.callBids({ bids: 'nothing useful' }); + beforeEach(() => { + ajaxStub = sinon.stub(ajax, 'ajax'); + }); - ajaxMock.verify(); - expect(spec.areParamsValid.called).to.equal(false); - expect(spec.buildRequests.called).to.equal(false); - expect(spec.interpretResponse.called).to.equal(false); - }); + afterEach(() => { + ajaxStub.restore(); + }); - it('should call buildRequests(bidRequest) the params are valid', () => { - spec.areParamsValid.returns(true); - spec.buildRequests.returns([]); - ajaxMock.expects('ajax').never(); + it('should handle bad bid requests gracefully', () => { + bidder.callBids({}); + bidder.callBids({ bids: 'nothing useful' }); - bidder.callBids(MOCK_BIDS_REQUEST); + expect(ajaxStub.called).to.equal(false); + expect(spec.areParamsValid.called).to.equal(false); + expect(spec.buildRequests.called).to.equal(false); + expect(spec.interpretResponse.called).to.equal(false); + }); - ajaxMock.verify(); - expect(spec.areParamsValid.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 call buildRequests(bidRequest) the params are valid', () => { + spec.areParamsValid.returns(true); + spec.buildRequests.returns([]); - it('should not call buildRequests the params are invalid', () => { - spec.areParamsValid.returns(false); - spec.buildRequests.returns([]); - ajaxMock.expects('ajax').never(); + bidder.callBids(MOCK_BIDS_REQUEST); - bidder.callBids(MOCK_BIDS_REQUEST); + expect(ajaxStub.called).to.equal(false); + expect(spec.areParamsValid.calledTwice).to.equal(true); + expect(spec.buildRequests.calledOnce).to.equal(true); + expect(spec.buildRequests.firstCall.args[0]).to.deep.equal(MOCK_BIDS_REQUEST.bids); + }); - ajaxMock.verify(); - expect(spec.areParamsValid.calledTwice).to.equal(true); - expect(spec.buildRequests.called).to.equal(false); - }); + it('should not call buildRequests the params are invalid', () => { + spec.areParamsValid.returns(false); + spec.buildRequests.returns([]); - it('should filter out invalid bids before calling buildRequests', () => { - spec.areParamsValid.onFirstCall().returns(true); - spec.areParamsValid.onSecondCall().returns(false); - spec.buildRequests.returns([]); - ajaxMock.expects('ajax').never(); + bidder.callBids(MOCK_BIDS_REQUEST); - bidder.callBids(MOCK_BIDS_REQUEST); + expect(ajaxStub.called).to.equal(false); + expect(spec.areParamsValid.calledTwice).to.equal(true); + expect(spec.buildRequests.called).to.equal(false); + }); - ajaxMock.verify(); - expect(spec.areParamsValid.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 filter out invalid bids before calling buildRequests', () => { + spec.areParamsValid.onFirstCall().returns(true); + spec.areParamsValid.onSecondCall().returns(false); + spec.buildRequests.returns([]); - it("should make no server requests if the spec doesn't return any", () => { - spec.areParamsValid.returns(true); - spec.buildRequests.returns([]); - ajaxMock.expects('ajax').never(); + bidder.callBids(MOCK_BIDS_REQUEST); - bidder.callBids(MOCK_BIDS_REQUEST); + expect(ajaxStub.called).to.equal(false); + expect(spec.areParamsValid.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]]); + }); - ajaxMock.verify(); - }); + it("should make no server requests if the spec doesn't return any", () => { + spec.areParamsValid.returns(true); + spec.buildRequests.returns([]); + + bidder.callBids(MOCK_BIDS_REQUEST); + + expect(ajaxStub.called).to.equal(false); + }); - it('should make the appropriate POST request', () => { - const url = 'test.url.com'; - const data = { arg: 2 }; - spec.areParamsValid.returns(true); - spec.buildRequests.returns({ - type: 'POST', - endpoint: url, - data: data + it('should make the appropriate POST request', () => { + const url = 'test.url.com'; + const data = { arg: 2 }; + spec.areParamsValid.returns(true); + spec.buildRequests.returns({ + type: 'POST', + endpoint: url, + data: data + }); + + bidder.callBids(MOCK_BIDS_REQUEST); + + expect(ajaxStub.calledOnce).to.equal(true); + expect(ajaxStub.firstCall.args[0]).to.equal(url); + expect(ajaxStub.firstCall.args[2]).to.equal(JSON.stringify(data)); + expect(ajaxStub.firstCall.args[3]).to.deep.equal({ + method: 'POST', + contentType: 'text/plain', + withCredentials: true + }); }); - ajaxMock.expects('ajax').once().withArgs(url, sinon.match.object, JSON.stringify(data), { - method: 'POST', - contentType: 'text/plain', - withCredentials: true + + it('should make the appropriate GET request', () => { + const url = 'test.url.com'; + const data = { arg: 2 }; + spec.areParamsValid.returns(true); + spec.buildRequests.returns({ + type: 'GET', + endpoint: url, + data: data + }); + + bidder.callBids(MOCK_BIDS_REQUEST); + + expect(ajaxStub.calledOnce).to.equal(true); + expect(ajaxStub.firstCall.args[0]).to.equal(`${url}?arg=2&`); + expect(ajaxStub.firstCall.args[2]).to.be.undefined; + expect(ajaxStub.firstCall.args[3]).to.deep.equal({ + method: 'GET', + withCredentials: true + }); }); - bidder.callBids(MOCK_BIDS_REQUEST); + it('should make multiple calls if the spec returns them', () => { + const url = 'test.url.com'; + const data = { arg: 2 }; + spec.areParamsValid.returns(true); + spec.buildRequests.returns([ + { + type: 'POST', + endpoint: url, + data: data + }, + { + type: 'GET', + endpoint: url, + data: data + } + ]); + + bidder.callBids(MOCK_BIDS_REQUEST); - ajaxMock.verify(); + expect(ajaxStub.calledTwice).to.equal(true); + }); }); - it('should make the appropriate GET request', () => { - const url = 'test.url.com'; - const data = { arg: 2 }; - spec.areParamsValid.returns(true); - spec.buildRequests.returns({ - type: 'GET', - endpoint: url, - data: data + describe('when the ajax call succeeds', () => { + let ajaxStub; + + beforeEach(() => { + ajaxStub = sinon.stub(ajax, 'ajax', function(url, callbacks) { + callbacks.success('response body'); + }); }); - ajaxMock.expects('ajax').once().withArgs(`${url}?arg=2&`, sinon.match.object, undefined, { - method: 'GET', - withCredentials: true + + afterEach(() => { + ajaxStub.restore(); }); - bidder.callBids(MOCK_BIDS_REQUEST); + it('should call spec.interpretResponse() with the response body content', () => { + spec.areParamsValid.returns(true); + spec.buildRequests.returns({ + type: 'POST', + endpoint: 'test.url.com', + data: {} + }); + + bidder.callBids(MOCK_BIDS_REQUEST); + + expect(spec.interpretResponse.calledOnce).to.equal(true); + expect(spec.interpretResponse.firstCall.args[0]).to.equal('response body'); + }); + + it('should call spec.interpretResponse() once for each request made', () => { + spec.areParamsValid.returns(true); + spec.buildRequests.returns([ + { + type: 'POST', + endpoint: 'test.url.com', + data: {} + }, + { + type: 'POST', + endpoint: 'test.url.com', + data: {} + }, + ]); + + bidder.callBids(MOCK_BIDS_REQUEST); + + expect(spec.interpretResponse.calledTwice).to.equal(true); + }); - ajaxMock.verify(); + it("should add bids for each placement code into the bidmanager, even if the bidder doesn't bid on all of them", () => { + const bid = { + requestId: 'some-id', + ad: 'ad-url.com', + cpm: 0.5, + height: 200, + width: 300, + placementCode: 'mock/placement' + }; + spec.areParamsValid.returns(true); + spec.buildRequests.returns({ + type: 'POST', + endpoint: 'test.url.com', + data: {} + }); + spec.interpretResponse.returns(bid); + + bidder.callBids(MOCK_BIDS_REQUEST); + + expect(bidmanager.addBidResponse.calledTwice).to.equal(true); + const placementsWithBids = + [bidmanager.addBidResponse.firstCall.args[0], bidmanager.addBidResponse.secondCall.args[0]]; + expect(placementsWithBids).to.contain('mock/placement'); + expect(placementsWithBids).to.contain('mock/placement2'); + }); }); - it('should make multiple calls if the spec returns them', () => { - const url = 'test.url.com'; - const data = { arg: 2 }; - spec.areParamsValid.returns(true); - spec.buildRequests.returns([ - { + describe('when the ajax call fails', () => { + let ajaxStub; + + beforeEach(() => { + ajaxStub = sinon.stub(ajax, 'ajax', function(url, callbacks) { + callbacks.error('ajax call failed.'); + }); + }); + + afterEach(() => { + ajaxStub.restore(); + }); + + it('should not spec.interpretResponse()', () => { + spec.areParamsValid.returns(true); + spec.buildRequests.returns({ type: 'POST', - endpoint: url, - data: data - }, - { - type: 'GET', - endpoint: url, - data: data - } - ]); + endpoint: 'test.url.com', + data: {} + }); - ajaxMock.expects('ajax').twice(); + bidder.callBids(MOCK_BIDS_REQUEST); - bidder.callBids(MOCK_BIDS_REQUEST); + expect(spec.interpretResponse.called).to.equal(false); + }); - ajaxMock.verify(); - }); + it('should add bids for each placement code into the bidmanager', () => { + spec.areParamsValid.returns(true); + spec.buildRequests.returns({ + type: 'POST', + endpoint: 'test.url.com', + data: {} + }); + spec.interpretResponse.returns([]); - // TODO: Pending the sinon 3.0 upgrade, mock the ajax calls and make sure it calls interpretResponse on each. + bidder.callBids(MOCK_BIDS_REQUEST); + + expect(bidmanager.addBidResponse.calledTwice).to.equal(true); + const placementsWithBids = + [bidmanager.addBidResponse.firstCall.args[0], bidmanager.addBidResponse.secondCall.args[0]]; + expect(placementsWithBids).to.contain('mock/placement'); + expect(placementsWithBids).to.contain('mock/placement2'); + }); + }); }); From a627a4334a720e0402f727d8f0537c4d3bee344d Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Wed, 23 Aug 2017 12:12:35 -0400 Subject: [PATCH 07/17] Changed an error to a log message. Fixed a small bug. --- src/adapters/bidderFactory.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/adapters/bidderFactory.js b/src/adapters/bidderFactory.js index a12525e14c3..25bbf3be266 100644 --- a/src/adapters/bidderFactory.js +++ b/src/adapters/bidderFactory.js @@ -113,7 +113,8 @@ export default function newBidder(spec) { ); break; default: - throw new Error(`buildRequest() must return a bid whose type is either "get" or "post". Got ${request.type}`); + logWarn(`Skipping invalid request from ${spec.code}. Request type ${request.type} must be GET or POST`); + onResponse(); } } From c246aa64fcd52a3b9434176a46c3fc04eec06d96 Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Thu, 31 Aug 2017 15:02:17 -0400 Subject: [PATCH 08/17] Did the no-brainer improvements from PR comments. --- src/adapters/bidderFactory.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/adapters/bidderFactory.js b/src/adapters/bidderFactory.js index 25bbf3be266..2d271a9008e 100644 --- a/src/adapters/bidderFactory.js +++ b/src/adapters/bidderFactory.js @@ -33,8 +33,8 @@ import { logWarn, logError, parseQueryStringParameters, delayExecution } from 's */ export default function newBidder(spec) { return Object.assign(new Adapter(spec.code), { - callBids: function(bidsRequest) { - if (!bidsRequest.bids || !bidsRequest.bids.filter) { + callBids: function(bidderRequest) { + if (!Array.isArray(bidderRequest.bids)) { return; } @@ -51,7 +51,7 @@ export default function newBidder(spec) { bidmanager.addBidResponse(placementCode, bid); } function fillNoBids() { - bidsRequest.bids + bidderRequest.bids .map(bidRequest => bidRequest.placementCode) .forEach(placementCode => { if (placementCode && !placementCodesHandled[placementCode]) { @@ -60,11 +60,15 @@ export default function newBidder(spec) { }); } - const bidRequests = bidsRequest.bids.filter(filterAndWarn); + 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) { @@ -122,10 +126,6 @@ export default function newBidder(spec) { // 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) { - const bidRequestMap = {}; - bidRequests.forEach(bid => { - bidRequestMap[bid.bidId] = bid; - }); function addBidUsingRequestMap(bid) { const bidRequest = bidRequestMap[bid.requestId]; if (bidRequest) { From 6c80c3d368f3b3a7a444382a9a6d002e690e0e8d Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Thu, 31 Aug 2017 15:51:31 -0400 Subject: [PATCH 09/17] Added spec-level support for aliases and mediaTypes. Aliases may still be broken. --- modules/appnexusAstBidAdapter.js | 10 +++++----- src/adapters/bidderFactory.js | 22 +++++++++++++++++++++- src/mediaTypes.js | 15 +++++++++++++++ 3 files changed, 41 insertions(+), 6 deletions(-) create mode 100644 src/mediaTypes.js diff --git a/modules/appnexusAstBidAdapter.js b/modules/appnexusAstBidAdapter.js index 4f869c65042..18cc7626a76 100644 --- a/modules/appnexusAstBidAdapter.js +++ b/modules/appnexusAstBidAdapter.js @@ -1,8 +1,8 @@ import { Renderer } from 'src/Renderer'; import * as utils from 'src/utils'; -import adaptermanager from 'src/adaptermanager'; import newBidder from 'src/adapters/bidderFactory'; import { POST } from '../src/ajax'; +import { NATIVE, VIDEO } from 'src/mediaTypes'; const BIDDER_CODE = 'appnexusAst'; const ENDPOINT = '//ib.adnxs.com/ut/v3/prebid'; @@ -26,6 +26,7 @@ const NATIVE_MAPPING = { const spec = { code: BIDDER_CODE, + supportedMediaTypes: [VIDEO, NATIVE], /** * Determines whether or not the given bid request is valid. @@ -339,8 +340,7 @@ function parseMediaType(rtbBid) { } } +// TODO: Before this PR merges, export the "spec" for testing and make this the "registerAdapter" call. +// We'll have to simplify the unit tests first, though... and right now it's valuable to know that they +// still work. export const adapter = newBidder(spec); - -adaptermanager.registerBidAdapter(adapter, 'appnexusAst', { - supportedMediaTypes: ['video', 'native'] -}); diff --git a/src/adapters/bidderFactory.js b/src/adapters/bidderFactory.js index 2d271a9008e..94f6b08dd72 100644 --- a/src/adapters/bidderFactory.js +++ b/src/adapters/bidderFactory.js @@ -1,4 +1,5 @@ import Adapter from 'src/adapter'; +import adaptermanager from 'src/adaptermanager'; import bidmanager from 'src/bidmanager'; import bidfactory from 'src/bidfactory'; import { STATUS } from 'src/constants'; @@ -32,7 +33,7 @@ import { logWarn, logError, parseQueryStringParameters, delayExecution } from 's * @param {BidderSpec} spec An object containing the bare-bones functions we need to make a Bidder. */ export default function newBidder(spec) { - return Object.assign(new Adapter(spec.code), { + const bidder = Object.assign(new Adapter(spec.code), { callBids: function(bidderRequest) { if (!Array.isArray(bidderRequest.bids)) { return; @@ -178,6 +179,23 @@ export default function newBidder(spec) { bid.bidderCode = spec.code; return bid; } + + const mediaTypes = Array.isArray(spec.supportedMediaTypes) + ? { supportedMediaTypes: spec.supportedMediaTypes } + : undefined; + adaptermanager.registerBidAdapter(bidder, spec.code, mediaTypes); + + if (Array.isArray(spec.aliases)) { + spec.aliases.forEach(alias => { + adaptermanager.aliasBidAdapter(spec.code, alias); + }); + } + + // TODO: Before this PR merges, rename this function and stop returning the bidder. + // I'm returning it for now to prove that the appnexusAstAdapter_spec tests still pass. + // Once that's established, those tests should be simplified so that they only test the + // "spec" methods. + return bidder; } /** @@ -219,6 +237,8 @@ export default function newBidder(spec) { * * @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 diff --git a/src/mediaTypes.js b/src/mediaTypes.js new file mode 100644 index 00000000000..e6b0015c5d3 --- /dev/null +++ b/src/mediaTypes.js @@ -0,0 +1,15 @@ +/** + * 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 + */ + +/** @ype MediaType */ +export const NATIVE = 'native'; +/** @ype MediaType */ +export const VIDEO = 'video'; From 37399c7e493cba982314c6fd43037b56b5125d9a Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Fri, 1 Sep 2017 11:36:00 -0400 Subject: [PATCH 10/17] Added support for aliases. Added more tests --- modules/appnexusAstBidAdapter.js | 6 +- src/adapters/bidderFactory.js | 56 ++++++++------ test/spec/unit/core/bidderFactory_spec.js | 89 ++++++++++++++++++++++- 3 files changed, 122 insertions(+), 29 deletions(-) diff --git a/modules/appnexusAstBidAdapter.js b/modules/appnexusAstBidAdapter.js index 18cc7626a76..7a329ca455a 100644 --- a/modules/appnexusAstBidAdapter.js +++ b/modules/appnexusAstBidAdapter.js @@ -1,6 +1,6 @@ import { Renderer } from 'src/Renderer'; import * as utils from 'src/utils'; -import newBidder from 'src/adapters/bidderFactory'; +import { registerBidder, newBidder } from 'src/adapters/bidderFactory'; import { POST } from '../src/ajax'; import { NATIVE, VIDEO } from 'src/mediaTypes'; @@ -340,7 +340,9 @@ function parseMediaType(rtbBid) { } } -// TODO: Before this PR merges, export the "spec" for testing and make this the "registerAdapter" call. +// TODO: Before this PR merges, export the "spec" for testing and replace this with the "registerAdapter" call. // We'll have to simplify the unit tests first, though... and right now it's valuable to know that they // still work. export const adapter = newBidder(spec); + +registerBidder(spec); diff --git a/src/adapters/bidderFactory.js b/src/adapters/bidderFactory.js index 94f6b08dd72..3d00daf7e6b 100644 --- a/src/adapters/bidderFactory.js +++ b/src/adapters/bidderFactory.js @@ -17,23 +17,50 @@ import { logWarn, logError, parseQueryStringParameters, delayExecution } from 's * * Typical usage looks something like: * - * const adapter = newBidder({ + * const adapter = registerBidder({ * code: 'myBidderCode', + * 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. */ /** - * This function aims to minimize the Adapter-specific code for "single request" Bidders. That is, if your adapter - * only makes a single request to the server for bids, regardless of how many bid requests or ad units are in - * the Auction, then this function is for you. + * 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 default function newBidder(spec) { - const bidder = Object.assign(new Adapter(spec.code), { +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), { callBids: function(bidderRequest) { if (!Array.isArray(bidderRequest.bids)) { return; @@ -179,23 +206,6 @@ export default function newBidder(spec) { bid.bidderCode = spec.code; return bid; } - - const mediaTypes = Array.isArray(spec.supportedMediaTypes) - ? { supportedMediaTypes: spec.supportedMediaTypes } - : undefined; - adaptermanager.registerBidAdapter(bidder, spec.code, mediaTypes); - - if (Array.isArray(spec.aliases)) { - spec.aliases.forEach(alias => { - adaptermanager.aliasBidAdapter(spec.code, alias); - }); - } - - // TODO: Before this PR merges, rename this function and stop returning the bidder. - // I'm returning it for now to prove that the appnexusAstAdapter_spec tests still pass. - // Once that's established, those tests should be simplified so that they only test the - // "spec" methods. - return bidder; } /** diff --git a/test/spec/unit/core/bidderFactory_spec.js b/test/spec/unit/core/bidderFactory_spec.js index aacb5046045..333e016d7bf 100644 --- a/test/spec/unit/core/bidderFactory_spec.js +++ b/test/spec/unit/core/bidderFactory_spec.js @@ -1,5 +1,6 @@ -import newBidder from 'src/adapters/bidderFactory'; +import { newBidder, registerBidder } from 'src/adapters/bidderFactory'; import bidmanager from 'src/bidmanager'; +import adaptermanager from 'src/adaptermanager'; import * as ajax from 'src/ajax'; import { expect } from 'chai'; import { STATUS } from 'src/constants'; @@ -24,10 +25,10 @@ const MOCK_BIDS_REQUEST = { ] } -describe('The bidder factory', () => { +describe('bidders created by newBidder', () => { let spec; - let bidder; let addBidRequestStub; + let bidder; beforeEach(() => { spec = { @@ -36,7 +37,6 @@ describe('The bidder factory', () => { buildRequests: sinon.stub(), interpretResponse: sinon.stub() }; - bidder = newBidder(spec); addBidRequestStub = sinon.stub(bidmanager, 'addBidResponse'); }); @@ -56,6 +56,8 @@ describe('The bidder factory', () => { }); it('should handle bad bid requests gracefully', () => { + const bidder = newBidder(spec); + bidder.callBids({}); bidder.callBids({ bids: 'nothing useful' }); @@ -66,6 +68,8 @@ describe('The bidder factory', () => { }); it('should call buildRequests(bidRequest) the params are valid', () => { + const bidder = newBidder(spec); + spec.areParamsValid.returns(true); spec.buildRequests.returns([]); @@ -78,6 +82,8 @@ describe('The bidder factory', () => { }); it('should not call buildRequests the params are invalid', () => { + const bidder = newBidder(spec); + spec.areParamsValid.returns(false); spec.buildRequests.returns([]); @@ -89,6 +95,8 @@ describe('The bidder factory', () => { }); it('should filter out invalid bids before calling buildRequests', () => { + const bidder = newBidder(spec); + spec.areParamsValid.onFirstCall().returns(true); spec.areParamsValid.onSecondCall().returns(false); spec.buildRequests.returns([]); @@ -102,6 +110,8 @@ describe('The bidder factory', () => { }); it("should make no server requests if the spec doesn't return any", () => { + const bidder = newBidder(spec); + spec.areParamsValid.returns(true); spec.buildRequests.returns([]); @@ -111,6 +121,7 @@ describe('The bidder factory', () => { }); it('should make the appropriate POST request', () => { + const bidder = newBidder(spec); const url = 'test.url.com'; const data = { arg: 2 }; spec.areParamsValid.returns(true); @@ -133,6 +144,7 @@ describe('The bidder factory', () => { }); it('should make the appropriate GET request', () => { + const bidder = newBidder(spec); const url = 'test.url.com'; const data = { arg: 2 }; spec.areParamsValid.returns(true); @@ -154,6 +166,7 @@ describe('The bidder factory', () => { }); it('should make multiple calls if the spec returns them', () => { + const bidder = newBidder(spec); const url = 'test.url.com'; const data = { arg: 2 }; spec.areParamsValid.returns(true); @@ -190,6 +203,8 @@ describe('The bidder factory', () => { }); it('should call spec.interpretResponse() with the response body content', () => { + const bidder = newBidder(spec); + spec.areParamsValid.returns(true); spec.buildRequests.returns({ type: 'POST', @@ -204,6 +219,8 @@ describe('The bidder factory', () => { }); it('should call spec.interpretResponse() once for each request made', () => { + const bidder = newBidder(spec); + spec.areParamsValid.returns(true); spec.buildRequests.returns([ { @@ -224,6 +241,8 @@ describe('The bidder factory', () => { }); 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 = { requestId: 'some-id', ad: 'ad-url.com', @@ -264,6 +283,8 @@ describe('The bidder factory', () => { }); it('should not spec.interpretResponse()', () => { + const bidder = newBidder(spec); + spec.areParamsValid.returns(true); spec.buildRequests.returns({ type: 'POST', @@ -277,6 +298,8 @@ describe('The bidder factory', () => { }); it('should add bids for each placement code into the bidmanager', () => { + const bidder = newBidder(spec); + spec.areParamsValid.returns(true); spec.buildRequests.returns({ type: 'POST', @@ -295,3 +318,61 @@ describe('The bidder factory', () => { }); }); }); + +describe('registerBidder', () => { + let registerBidAdapterStub; + let aliasBidAdapterStub; + + beforeEach(() => { + registerBidAdapterStub = sinon.stub(adaptermanager, 'registerBidAdapter'); + aliasBidAdapterStub = sinon.stub(adaptermanager, 'aliasBidAdapter'); + }); + + afterEach(() => { + registerBidAdapterStub.restore(); + aliasBidAdapterStub.restore(); + }); + + function newEmptySpec() { + return { + code: CODE, + areParamsValid: function() { }, + buildRequests: function() { }, + interpretResponse: function() { }, + fetchUserSyncs: function() { } + }; + } + + it('should register a bidder with the adapterManager', () => { + registerBidder(newEmptySpec()); + expect(registerBidAdapterStub.calledOnce).to.equal(true); + expect(registerBidAdapterStub.firstCall.args[0]).to.have.property('callBids'); + expect(registerBidAdapterStub.firstCall.args[0].callBids).to.be.a('function'); + + expect(registerBidAdapterStub.firstCall.args[1]).to.equal(CODE); + expect(registerBidAdapterStub.firstCall.args[2]).to.be.undefined; + }); + + it('should register a bidder with the appropriate mediaTypes', () => { + const thisSpec = Object.assign(newEmptySpec(), { supportedMediaTypes: ['video'] }); + registerBidder(thisSpec); + expect(registerBidAdapterStub.calledOnce).to.equal(true); + expect(registerBidAdapterStub.firstCall.args[2]).to.deep.equal({supportedMediaTypes: ['video']}); + }); + + it('should register bidders with the appropriate aliases', () => { + const thisSpec = Object.assign(newEmptySpec(), { aliases: ['foo', 'bar'] }); + registerBidder(thisSpec); + + expect(registerBidAdapterStub.calledThrice).to.equal(true); + + // Make sure our later calls don't override the bidder code from previous calls. + expect(registerBidAdapterStub.firstCall.args[0].getBidderCode()).to.equal(CODE); + expect(registerBidAdapterStub.secondCall.args[0].getBidderCode()).to.equal('foo') + expect(registerBidAdapterStub.thirdCall.args[0].getBidderCode()).to.equal('bar') + + expect(registerBidAdapterStub.firstCall.args[1]).to.equal(CODE); + expect(registerBidAdapterStub.secondCall.args[1]).to.equal('foo') + expect(registerBidAdapterStub.thirdCall.args[1]).to.equal('bar') + }); +}) From 1b42995f8ab5aa615cb64c87cc64c85c05158cc3 Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Fri, 1 Sep 2017 11:40:59 -0400 Subject: [PATCH 11/17] Cleaned up some unnecessary code. --- modules/appnexusAstBidAdapter.js | 9 ++------- src/adapters/bidderFactory.js | 2 +- test/spec/modules/appnexusAstBidAdapter_spec.js | 5 ++++- test/spec/unit/core/bidderFactory_spec.js | 1 - 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/modules/appnexusAstBidAdapter.js b/modules/appnexusAstBidAdapter.js index 7a329ca455a..3ab1244e389 100644 --- a/modules/appnexusAstBidAdapter.js +++ b/modules/appnexusAstBidAdapter.js @@ -1,6 +1,6 @@ import { Renderer } from 'src/Renderer'; import * as utils from 'src/utils'; -import { registerBidder, newBidder } from 'src/adapters/bidderFactory'; +import { registerBidder } from 'src/adapters/bidderFactory'; import { POST } from '../src/ajax'; import { NATIVE, VIDEO } from 'src/mediaTypes'; @@ -24,7 +24,7 @@ const NATIVE_MAPPING = { sponsoredBy: 'sponsored_by' }; -const spec = { +export const spec = { code: BIDDER_CODE, supportedMediaTypes: [VIDEO, NATIVE], @@ -340,9 +340,4 @@ function parseMediaType(rtbBid) { } } -// TODO: Before this PR merges, export the "spec" for testing and replace this with the "registerAdapter" call. -// We'll have to simplify the unit tests first, though... and right now it's valuable to know that they -// still work. -export const adapter = newBidder(spec); - registerBidder(spec); diff --git a/src/adapters/bidderFactory.js b/src/adapters/bidderFactory.js index 3d00daf7e6b..382a9e087fd 100644 --- a/src/adapters/bidderFactory.js +++ b/src/adapters/bidderFactory.js @@ -257,7 +257,7 @@ export function newBidder(spec) { * @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(): UserSyncInfo[]} fetchUserSyncs + * @property {function(): UserSyncInfo[]} [fetchUserSyncs] */ /** diff --git a/test/spec/modules/appnexusAstBidAdapter_spec.js b/test/spec/modules/appnexusAstBidAdapter_spec.js index 540c054eee9..a438ca9f6a9 100644 --- a/test/spec/modules/appnexusAstBidAdapter_spec.js +++ b/test/spec/modules/appnexusAstBidAdapter_spec.js @@ -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'; @@ -61,6 +62,8 @@ const RESPONSE = { }; describe('AppNexusAdapter', () => { + const adapter = newBidder(spec); + describe('request function', () => { let xhr; let requests; diff --git a/test/spec/unit/core/bidderFactory_spec.js b/test/spec/unit/core/bidderFactory_spec.js index 333e016d7bf..8264cae749a 100644 --- a/test/spec/unit/core/bidderFactory_spec.js +++ b/test/spec/unit/core/bidderFactory_spec.js @@ -339,7 +339,6 @@ describe('registerBidder', () => { areParamsValid: function() { }, buildRequests: function() { }, interpretResponse: function() { }, - fetchUserSyncs: function() { } }; } From 5c3d645d31f480c8207c56dee98b5bb3c3f96563 Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Fri, 1 Sep 2017 13:38:05 -0400 Subject: [PATCH 12/17] Removed the GET/POST constants. Fixed some typos, and renamed some Request-related properties for consistency with the native object. --- modules/appnexusAstBidAdapter.js | 28 ++++--- src/adapters/bidderFactory.js | 94 +++++++++++++++-------- src/ajax.js | 7 +- src/mediaTypes.js | 6 +- test/spec/unit/core/bidderFactory_spec.js | 75 +++++++++++++----- 5 files changed, 134 insertions(+), 76 deletions(-) diff --git a/modules/appnexusAstBidAdapter.js b/modules/appnexusAstBidAdapter.js index 3ab1244e389..4bb8c26aa39 100644 --- a/modules/appnexusAstBidAdapter.js +++ b/modules/appnexusAstBidAdapter.js @@ -1,11 +1,10 @@ import { Renderer } from 'src/Renderer'; import * as utils from 'src/utils'; import { registerBidder } from 'src/adapters/bidderFactory'; -import { POST } from '../src/ajax'; import { NATIVE, VIDEO } from 'src/mediaTypes'; const BIDDER_CODE = 'appnexusAst'; -const ENDPOINT = '//ib.adnxs.com/ut/v3/prebid'; +const URL = '//ib.adnxs.com/ut/v3/prebid'; const SUPPORTED_AD_TYPES = ['banner', 'video', 'video-outstream', 'native']; const VIDEO_TARGETING = ['id', 'mimes', 'minduration', 'maxduration', 'startdelay', 'skippable', 'playback_method', 'frameworks']; @@ -27,6 +26,7 @@ const NATIVE_MAPPING = { export const spec = { code: BIDDER_CODE, supportedMediaTypes: [VIDEO, NATIVE], + aliases: ['dummy'], /** * Determines whether or not the given bid request is valid. @@ -64,8 +64,8 @@ export const spec = { } const payloadString = JSON.stringify(payload); return { - type: POST, - endpoint: ENDPOINT, + method: 'POST', + url: URL, data: payloadString, }; }, @@ -90,20 +90,18 @@ export const spec = { } }); return bids; + }, + + getUserSyncs: function(syncOptions) { + if (syncOptions.iframeEnabled) { + return [{ + type: 'iframe', + url: '//acdn.adnxs.com/ib/static/usersync/v3/async_usersync.html' + }]; + } } } -// TODO: Patch UserSync into the spec, once the UserSync PR has been merged. -// function userSyncCode() { -// const iframe = utils.createInvisibleIframe(); -// iframe.src = '//acdn.adnxs.com/ib/static/usersync/v3/async_usersync.html'; -// try { -// document.body.appendChild(iframe); -// } catch (error) { -// utils.logError(error); -// } -// } - function newRenderer(adUnitCode, rtbBid) { const renderer = Renderer.install({ id: rtbBid.renderer_id, diff --git a/src/adapters/bidderFactory.js b/src/adapters/bidderFactory.js index 382a9e087fd..1b839976718 100644 --- a/src/adapters/bidderFactory.js +++ b/src/adapters/bidderFactory.js @@ -1,10 +1,10 @@ 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 { ajax, GET, POST } from 'src/ajax'; import { logWarn, logError, parseQueryStringParameters, delayExecution } from 'src/utils'; /** @@ -107,38 +107,52 @@ export function newBidder(spec) { 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(fillNoBids, requests.length) + const onResponse = delayExecution(afterAllResponses, requests.length) requests.forEach(processRequest); function processRequest(request) { - switch (request.type) { - case GET: + switch (request.method) { + case 'GET': ajax( - `${request.endpoint}?${parseQueryStringParameters(request.data)}`, + `${request.url}?${parseQueryStringParameters(request.data)}`, { success: onSuccess, error: onFailure }, undefined, { - method: GET, + method: 'GET', withCredentials: true } ); break; - case POST: + case 'POST': ajax( - request.endpoint, + request.url, { success: onSuccess, error: onFailure }, typeof request.data === 'string' ? request.data : JSON.stringify(request.data), { - method: POST, + method: 'POST', contentType: 'text/plain', withCredentials: true } @@ -154,15 +168,7 @@ export function newBidder(spec) { // 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) { - 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.`); - } - } + responses.push(response); let bids; try { @@ -181,6 +187,16 @@ 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); + } 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 @@ -211,9 +227,9 @@ export function newBidder(spec) { /** * @typedef {object} ServerRequest * - * @param {('GET'|'POST')} type The type of request which this is. - * @param {string} endpoint The endpoint for the request. For example, "//bids.example.com". - * @param {string|object} data Data to be sent in the request. + * @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. @@ -222,22 +238,22 @@ export function newBidder(spec) { /** * @typedef {object} BidRequest * - * @param {string} bidId A string which uniquely identifies this BidRequest in the current Auction. - * @param {object} params Any bidder-specific params which the publisher used in their bid request. + * @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} Bid * - * @param {string} requestId The specific BidRequest which this bid is aimed at. + * @property {string} requestId The specific BidRequest which this bid is aimed at. * This should correspond to one of the - * @param {string} ad A URL which can be used to load this ad, if it's chosen by the publisher. - * @param {number} cpm The bid price, in US cents per thousand impressions. - * @param {number} height The height of the ad, in pixels. - * @param {number} width The width of the ad, in pixels. + * @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. * - * @param [Renderer] renderer A Renderer which can be used as a default for this bid, + * @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. */ @@ -257,14 +273,26 @@ export function newBidder(spec) { * @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(): UserSyncInfo[]} [fetchUserSyncs] + * @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} 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} UserSyncInfo + * @typedef {object} UserSync * - * @param {('image'|'iframe')} type The type of user sync to be done. - * @param {string} url The URL which makes the sync happen. + * @property {('image'|'iframe')} type The type of user sync to be done. + * @property {string} url The URL which makes the sync happen. */ diff --git a/src/ajax.js b/src/ajax.js index fb47b5d23b8..02b699d7e9a 100644 --- a/src/ajax.js +++ b/src/ajax.js @@ -5,9 +5,6 @@ var utils = require('./utils'); const XHR_DONE = 4; let _timeout = 3000; -export const GET = 'GET'; -export const POST = 'POST'; - /** * Simple IE9+ and cross-browser ajax request function * Note: x-domain requests in IE9 do not support the use of cookies @@ -25,7 +22,7 @@ export function ajax(url, callback, data, options = {}) { try { let x; let useXDomainRequest = false; - let method = options.method || (data ? POST : GET); + let method = options.method || (data ? 'POST' : 'GET'); let callbacks = typeof callback === 'object' ? callback : { success: function() { @@ -100,7 +97,7 @@ export function ajax(url, callback, data, options = {}) { } x.setRequestHeader('Content-Type', options.contentType || 'text/plain'); } - x.send(method === POST && data); + x.send(method === 'POST' && data); } catch (error) { utils.logError('xhr construction', error); } diff --git a/src/mediaTypes.js b/src/mediaTypes.js index e6b0015c5d3..7a40030e4e2 100644 --- a/src/mediaTypes.js +++ b/src/mediaTypes.js @@ -9,7 +9,9 @@ * @typedef {('native'|'video'|'banner')} MediaType */ -/** @ype MediaType */ +/** @type MediaType */ export const NATIVE = 'native'; -/** @ype MediaType */ +/** @type MediaType */ export const VIDEO = 'video'; +/** @type MediaType */ +export const BANNER = 'banner'; diff --git a/test/spec/unit/core/bidderFactory_spec.js b/test/spec/unit/core/bidderFactory_spec.js index 8264cae749a..6a819d53d41 100644 --- a/test/spec/unit/core/bidderFactory_spec.js +++ b/test/spec/unit/core/bidderFactory_spec.js @@ -35,7 +35,8 @@ describe('bidders created by newBidder', () => { code: CODE, areParamsValid: sinon.stub(), buildRequests: sinon.stub(), - interpretResponse: sinon.stub() + interpretResponse: sinon.stub(), + getUserSyncs: sinon.stub() }; addBidRequestStub = sinon.stub(bidmanager, 'addBidResponse'); }); @@ -126,8 +127,8 @@ describe('bidders created by newBidder', () => { const data = { arg: 2 }; spec.areParamsValid.returns(true); spec.buildRequests.returns({ - type: 'POST', - endpoint: url, + method: 'POST', + url: url, data: data }); @@ -149,8 +150,8 @@ describe('bidders created by newBidder', () => { const data = { arg: 2 }; spec.areParamsValid.returns(true); spec.buildRequests.returns({ - type: 'GET', - endpoint: url, + method: 'GET', + url: url, data: data }); @@ -172,13 +173,13 @@ describe('bidders created by newBidder', () => { spec.areParamsValid.returns(true); spec.buildRequests.returns([ { - type: 'POST', - endpoint: url, + method: 'POST', + url: url, data: data }, { - type: 'GET', - endpoint: url, + method: 'GET', + url: url, data: data } ]); @@ -207,8 +208,8 @@ describe('bidders created by newBidder', () => { spec.areParamsValid.returns(true); spec.buildRequests.returns({ - type: 'POST', - endpoint: 'test.url.com', + method: 'POST', + url: 'test.url.com', data: {} }); @@ -224,13 +225,13 @@ describe('bidders created by newBidder', () => { spec.areParamsValid.returns(true); spec.buildRequests.returns([ { - type: 'POST', - endpoint: 'test.url.com', + method: 'POST', + url: 'test.url.com', data: {} }, { - type: 'POST', - endpoint: 'test.url.com', + method: 'POST', + url: 'test.url.com', data: {} }, ]); @@ -253,8 +254,8 @@ describe('bidders created by newBidder', () => { }; spec.areParamsValid.returns(true); spec.buildRequests.returns({ - type: 'POST', - endpoint: 'test.url.com', + method: 'POST', + url: 'test.url.com', data: {} }); spec.interpretResponse.returns(bid); @@ -267,6 +268,22 @@ describe('bidders created by newBidder', () => { expect(placementsWithBids).to.contain('mock/placement'); expect(placementsWithBids).to.contain('mock/placement2'); }); + + it('should call spec.getUserSyncs() with the response', () => { + const bidder = newBidder(spec); + + spec.areParamsValid.returns(true); + spec.buildRequests.returns({ + method: 'POST', + url: 'test.url.com', + data: {} + }); + + bidder.callBids(MOCK_BIDS_REQUEST); + + expect(spec.getUserSyncs.calledOnce).to.equal(true); + expect(spec.getUserSyncs.firstCall.args[1]).to.deep.equal(['response body']); + }); }); describe('when the ajax call fails', () => { @@ -287,8 +304,8 @@ describe('bidders created by newBidder', () => { spec.areParamsValid.returns(true); spec.buildRequests.returns({ - type: 'POST', - endpoint: 'test.url.com', + method: 'POST', + url: 'test.url.com', data: {} }); @@ -302,8 +319,8 @@ describe('bidders created by newBidder', () => { spec.areParamsValid.returns(true); spec.buildRequests.returns({ - type: 'POST', - endpoint: 'test.url.com', + method: 'POST', + url: 'test.url.com', data: {} }); spec.interpretResponse.returns([]); @@ -316,6 +333,22 @@ describe('bidders created by newBidder', () => { expect(placementsWithBids).to.contain('mock/placement'); expect(placementsWithBids).to.contain('mock/placement2'); }); + + it('should call spec.getUserSyncs() with no responses', () => { + const bidder = newBidder(spec); + + spec.areParamsValid.returns(true); + spec.buildRequests.returns({ + method: 'POST', + url: 'test.url.com', + data: {} + }); + + bidder.callBids(MOCK_BIDS_REQUEST); + + expect(spec.getUserSyncs.calledOnce).to.equal(true); + expect(spec.getUserSyncs.firstCall.args[1]).to.deep.equal([]); + }); }); }); From 735f2bcc240ccf3a597b127664b5ccad5749c10c Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Wed, 6 Sep 2017 12:18:03 -0400 Subject: [PATCH 13/17] Re-added some code for outstream rendering, which was apparently lost in the merges somewhere. --- modules/appnexusAstBidAdapter.js | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/modules/appnexusAstBidAdapter.js b/modules/appnexusAstBidAdapter.js index cdcd8afafb4..e53da37318f 100644 --- a/modules/appnexusAstBidAdapter.js +++ b/modules/appnexusAstBidAdapter.js @@ -117,7 +117,12 @@ function newRenderer(adUnitCode, rtbBid) { config: { adText: `AppNexus Outstream Video Ad via Prebid.js` }, loaded: false, }); - renderer.setRender(outstreamRender); + + try { + renderer.setRender(outstreamRender); + } catch (err) { + utils.logWarning('Prebid Error calling setRender on renderer', err); + } renderer.setEventHandlers({ impression: () => utils.logMessage('AppNexus outstream video impression event'), @@ -333,6 +338,26 @@ function getRtbBid(tag) { return tag && tag.ads && tag.ads.length && tag.ads.find(ad => ad.rtb); } + +function outstreamRender(bid) { + // push to render queue because ANOutstreamVideo may not be loaded yet + bid.renderer.push(() => { + window.ANOutstreamVideo.renderAd({ + tagId: bid.adResponse.tag_id, + sizes: [bid.getSize().split('x')], + targetId: bid.adUnitCode, // target div id to render video + uuid: bid.adResponse.uuid, + adResponse: bid.adResponse, + rendererOptions: bid.renderer.getConfig() + }, handleOutstreamRendererEvents.bind(bid)); + }); +} + +function handleOutstreamRendererEvents(id, eventName) { + const bid = this; + bid.renderer.handleVideoEvent({ id, eventName }); +} + function parseMediaType(rtbBid) { const adType = rtbBid.ad_type; if (rtbBid.renderer_url) { From dd4e5953bff47dd0315c330da446f56acee41e0e Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Wed, 6 Sep 2017 12:21:55 -0400 Subject: [PATCH 14/17] Removed confusing use of this --- modules/appnexusAstBidAdapter.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/modules/appnexusAstBidAdapter.js b/modules/appnexusAstBidAdapter.js index e53da37318f..211b0eb439d 100644 --- a/modules/appnexusAstBidAdapter.js +++ b/modules/appnexusAstBidAdapter.js @@ -349,12 +349,11 @@ function outstreamRender(bid) { uuid: bid.adResponse.uuid, adResponse: bid.adResponse, rendererOptions: bid.renderer.getConfig() - }, handleOutstreamRendererEvents.bind(bid)); + }, handleOutstreamRendererEvents.bind(null, bid)); }); } -function handleOutstreamRendererEvents(id, eventName) { - const bid = this; +function handleOutstreamRendererEvents(bid, id, eventName) { bid.renderer.handleVideoEvent({ id, eventName }); } From 25f822afb321cec3d8961234f5744ab9d68b5604 Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Wed, 6 Sep 2017 12:31:00 -0400 Subject: [PATCH 15/17] Fixed lint error --- modules/appnexusAstBidAdapter.js | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/appnexusAstBidAdapter.js b/modules/appnexusAstBidAdapter.js index 211b0eb439d..ba10538b5a9 100644 --- a/modules/appnexusAstBidAdapter.js +++ b/modules/appnexusAstBidAdapter.js @@ -338,7 +338,6 @@ function getRtbBid(tag) { return tag && tag.ads && tag.ads.length && tag.ads.find(ad => ad.rtb); } - function outstreamRender(bid) { // push to render queue because ANOutstreamVideo may not be loaded yet bid.renderer.push(() => { From e63f2d6ba65ba22a5d88bf12142102914eb5502f Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Wed, 6 Sep 2017 15:43:52 -0400 Subject: [PATCH 16/17] Moved JSON parsing into the bidderFactory, and moved the JSDocs to the top. --- modules/appnexusAstBidAdapter.js | 4 +- src/adapters/bidderFactory.js | 149 ++++++++++++++++--------------- 2 files changed, 77 insertions(+), 76 deletions(-) diff --git a/modules/appnexusAstBidAdapter.js b/modules/appnexusAstBidAdapter.js index ba10538b5a9..7da6bf0285b 100644 --- a/modules/appnexusAstBidAdapter.js +++ b/modules/appnexusAstBidAdapter.js @@ -27,7 +27,6 @@ const SOURCE = 'pbjs'; export const spec = { code: BIDDER_CODE, supportedMediaTypes: [VIDEO, NATIVE], - aliases: ['dummy'], /** * Determines whether or not the given bid request is valid. @@ -85,9 +84,8 @@ export const spec = { * @return {Bid[]} An array of bids which were nested inside the server. */ interpretResponse: function(serverResponse) { - const parsed = JSON.parse(serverResponse); const bids = []; - parsed.tags.forEach(serverBid => { + serverResponse.tags.forEach(serverBid => { const rtbBid = getRtbBid(serverBid); if (rtbBid) { if (rtbBid.cpm !== 0 && SUPPORTED_AD_TYPES.includes(rtbBid.ad_type)) { diff --git a/src/adapters/bidderFactory.js b/src/adapters/bidderFactory.js index 1b839976718..f68847240cb 100644 --- a/src/adapters/bidderFactory.js +++ b/src/adapters/bidderFactory.js @@ -29,6 +29,79 @@ import { logWarn, logError, parseQueryStringParameters, delayExecution } from 's * @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. * @@ -168,6 +241,9 @@ export function newBidder(spec) { // 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; @@ -223,76 +299,3 @@ export function newBidder(spec) { return bid; } } - -/** - * @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} 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} 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} 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} 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. - */ From 13d3911bb5bfc67c833608c7ba4542db8d7d022f Mon Sep 17 00:00:00 2001 From: Dave Bemiller Date: Thu, 7 Sep 2017 16:43:29 -0400 Subject: [PATCH 17/17] Removed placementCode from everywhere I could. --- src/adapters/bidderFactory.js | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/adapters/bidderFactory.js b/src/adapters/bidderFactory.js index f68847240cb..7c9860095d9 100644 --- a/src/adapters/bidderFactory.js +++ b/src/adapters/bidderFactory.js @@ -139,24 +139,25 @@ export function newBidder(spec) { return; } - // callBids must add a NO_BID response for _every_ placementCode, in order for the auction to + // callBids must add a NO_BID response for _every_ AdUnit code, 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. + // As we add _real_ bids to the bidmanager, we'll log the ad unit codes here too. Once all the real + // bids have been added, fillNoBids() can be called to add NO_BID bids for any extra ad units, which + // will end the auction. // // In Prebid 1.0, this will be simplified to use the `addBidResponse` and `done` callbacks. - const placementCodesHandled = {}; - function addBidWithCode(placementCode, bid) { - placementCodesHandled[placementCode] = true; - bidmanager.addBidResponse(placementCode, bid); + const adUnitCodesHandled = {}; + function addBidWithCode(adUnitCode, bid) { + adUnitCodesHandled[adUnitCode] = true; + bidmanager.addBidResponse(adUnitCode, bid); } function fillNoBids() { bidderRequest.bids .map(bidRequest => bidRequest.placementCode) - .forEach(placementCode => { - if (placementCode && !placementCodesHandled[placementCode]) { - bidmanager.addBidResponse(placementCode, newEmptyBid()); + .forEach(adUnitCode => { + if (adUnitCode && !adUnitCodesHandled[adUnitCode]) { + bidmanager.addBidResponse(adUnitCode, newEmptyBid()); } }); }