Skip to content

Commit

Permalink
allow outstream video, remove parsePosition method, simplify code (pr…
Browse files Browse the repository at this point in the history 
…ebid#2683)

* allow outstream video, remove parsePosition method, simplify code

* update media type validation rule

* update media type validation rule

* video objetc bug fix

* cover no instream no outstream case

* add 'Rubicon bid adapter' as the prefix for log messages
  • Loading branch information
@moonshells @dluxemburg
moonshells authored and dluxemburg committed Jul 17, 2018
1 parent 6637c7b commit b3bfb63
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 66 deletions.
98 changes: 43 additions & 55 deletions modules/rubiconBidAdapter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,38 +88,31 @@ export const spec = {
if (typeof bid.params !== 'object') {
return false;
}
let params = bid.params;

if (!/^\d+$/.test(params.accountId)) {
if (!/^\d+$/.test(bid.params.accountId)) {
return false;
}

// Log warning if context is 'outstream', is not currently supported
if (utils.deepAccess(bid, `mediaTypes.${VIDEO}.context`) === 'outstream') {
utils.logWarn('Warning: outstream video for Rubicon Client Adapter is not supported yet');
}

// Log warning if mediaTypes contains both 'banner' and 'video'
if (spec.hasVideoMediaType(bid) && typeof utils.deepAccess(bid, `mediaTypes.${BANNER}`) !== 'undefined') {
utils.logWarn('Warning: instream video and banner requested for same ad unit, continuing with video instream request');
}

// Bid is invalid if legacy video is set but params video is missing size_id
if (bid.mediaType === 'video' && typeof utils.deepAccess(bid, 'params.video.size_id') === 'undefined') {
return false;
}

// Bid is invalid if mediaTypes video is invalid and a mediaTypes banner property is not defined
if (bid.mediaTypes && !spec.hasVideoMediaType(bid) && typeof bid.mediaTypes.banner === 'undefined') {
return false;
}

let parsedSizes = parseSizes(bid);
if (parsedSizes.length < 1) {
return false;
if (hasVideoMediaType(bid)) {
// Log warning if mediaTypes contains both 'banner' and 'video'
if (utils.deepAccess(bid, `mediaTypes.${VIDEO}.context`) === 'instream' || bid.mediaType === VIDEO) {
if (typeof utils.deepAccess(bid, 'params.video.size_id') === 'undefined') {
utils.logError('Rubicon bid adapter Error: size id is missing for instream video request.');
return false;
}
} else if (utils.deepAccess(bid, `mediaTypes.${VIDEO}.context`) === 'outstream') {
if (utils.deepAccess(bid, 'params.video.size_id') !== 203) {
utils.logWarn('Rubicon bid adapter Warning: outstream video is sending invalid size id, converting size id to 203.');
}
} else {
utils.logError('Rubicon bid adapter Error: no instream or outstream context defined in mediaTypes.');
return false;
}
if (typeof utils.deepAccess(bid, `mediaTypes.${BANNER}`) !== 'undefined') {
utils.logWarn('Rubicon bid adapter Warning: video and banner requested for same ad unit, continuing with video request, multi-format request is not supported by rubicon yet.');
}
}

return true;
return parseSizes(bid).length > 0;
},
/**
* @param {BidRequest[]} bidRequests
Expand All @@ -129,7 +122,7 @@ export const spec = {
buildRequests: function (bidRequests, bidderRequest) {
// separate video bids because the requests are structured differently
let requests = [];
const videoRequests = bidRequests.filter(spec.hasVideoMediaType).map(bidRequest => {
const videoRequests = bidRequests.filter(hasVideoMediaType).map(bidRequest => {
bidRequest.startTime = new Date().getTime();

let params = bidRequest.params;
Expand All @@ -143,24 +136,27 @@ export const spec = {
'x_source.tid': bidRequest.transactionId,
timeout: bidderRequest.timeout - (Date.now() - bidderRequest.auctionStart + TIMEOUT_BUFFER),
stash_creatives: true,
ae_pass_through_parameters: params.video.aeParams,
slots: []
};

// Define the slot object
let slotData = {
site_id: params.siteId,
zone_id: params.zoneId,
position: parsePosition(params.position),
position: params.position === 'atf' || params.position === 'btf' ? params.position : 'unknown',
floor: parseFloat(params.floor) > 0.01 ? params.floor : 0.01,
element_id: bidRequest.adUnitCode,
name: bidRequest.adUnitCode,
language: params.video.language,
width: size[0],
height: size[1],
size_id: params.video.size_id
size_id: utils.deepAccess(bidRequest, `mediaTypes.${VIDEO}.context`) === 'outstream' ? 203 : params.video.size_id
};

if (params.video) {
data.ae_pass_through_parameters = params.video.aeParams;
slotData.language = params.video.language;
}

if (params.inventory && typeof params.inventory === 'object') {
slotData.inventory = params.inventory;
}
Expand Down Expand Up @@ -193,7 +189,7 @@ export const spec = {

if (config.getConfig('rubicon.singleRequest') !== true) {
// bids are not grouped if single request mode is not enabled
requests = videoRequests.concat(bidRequests.filter(bidRequest => !spec.hasVideoMediaType(bidRequest)).map(bidRequest => {
requests = videoRequests.concat(bidRequests.filter(bidRequest => !hasVideoMediaType(bidRequest)).map(bidRequest => {
const bidParams = spec.createSlotParams(bidRequest, bidderRequest);
return {
method: 'GET',
Expand All @@ -208,7 +204,7 @@ export const spec = {
} else {
// single request requires bids to be grouped by site id into a single request
// note: utils.groupBy wasn't used because deep property access was needed
const nonVideoRequests = bidRequests.filter(bidRequest => !spec.hasVideoMediaType(bidRequest));
const nonVideoRequests = bidRequests.filter(bidRequest => !hasVideoMediaType(bidRequest));
const groupedBidRequests = nonVideoRequests.reduce((groupedBids, bid) => {
(groupedBids[bid.params['siteId']] = groupedBids[bid.params['siteId']] || []).push(bid);
return groupedBids;
Expand All @@ -219,7 +215,7 @@ export const spec = {

// fastlane SRA has a limit of 10 slots
if (bidsInGroup.length > 10) {
utils.logWarn(`single request mode has a limit of 10 bids: ${bidsInGroup.length - 10} bids were not sent`);
utils.logWarn(`Rubicon bid adapter Warning: single request mode has a limit of 10 bids: ${bidsInGroup.length - 10} bids were not sent`);
bidsInGroup = bidsInGroup.slice(0, 10);
}

Expand Down Expand Up @@ -335,7 +331,7 @@ export const spec = {
'zone_id': params.zoneId,
'size_id': parsedSizes[0],
'alt_size_ids': parsedSizes.slice(1).join(',') || undefined,
'p_pos': parsePosition(params.position),
'p_pos': params.position === 'atf' || params.position === 'btf' ? params.position : 'unknown',
'rp_floor': (params.floor = parseFloat(params.floor)) > 0.01 ? params.floor : 0.01,
'rp_secure': isSecure() ? '1' : '0',
'tk_flint': INTEGRATION,
Expand Down Expand Up @@ -380,17 +376,6 @@ export const spec = {
return data;
},

/**
* Test if bid has mediaType or mediaTypes set for video.
* note: 'mediaType' has been deprecated, however support will remain for a transitional period
* @param {BidRequest} bidRequest
* @returns {boolean}
*/
hasVideoMediaType: function (bidRequest) {
return (typeof utils.deepAccess(bidRequest, 'params.video.size_id') !== 'undefined' &&
(bidRequest.mediaType === VIDEO || utils.deepAccess(bidRequest, `mediaTypes.${VIDEO}.context`) === 'instream'));
},

/**
* @param {*} responseObj
* @param {BidRequest|Object.<string, BidRequest[]>} bidRequest - if request was SRA the bidRequest argument will be a keyed BidRequest array object,
Expand All @@ -408,7 +393,7 @@ export const spec = {
let ads = responseObj.ads;

// video ads array is wrapped in an object
if (typeof bidRequest === 'object' && !Array.isArray(bidRequest) && spec.hasVideoMediaType(bidRequest) && typeof ads === 'object') {
if (typeof bidRequest === 'object' && !Array.isArray(bidRequest) && hasVideoMediaType(bidRequest) && typeof ads === 'object') {
ads = ads[bidRequest.adUnitCode];
}

Expand Down Expand Up @@ -463,7 +448,7 @@ export const spec = {

bids.push(bid);
} else {
utils.logError(`bidRequest undefined at index position:${i}`, bidRequest, responseObj);
utils.logError(`Rubicon bid adapter Error: bidRequest undefined at index position:${i}`, bidRequest, responseObj);
}

return bids;
Expand Down Expand Up @@ -544,7 +529,7 @@ function _renderCreative(script, impId) {

function parseSizes(bid) {
let params = bid.params;
if (spec.hasVideoMediaType(bid)) {
if (hasVideoMediaType(bid)) {
let size = [];
if (typeof utils.deepAccess(bid, 'mediaTypes.video.playerSize') !== 'undefined') {
size = bid.mediaTypes.video.playerSize;
Expand Down Expand Up @@ -586,11 +571,14 @@ function mapSizes(sizes) {
}, []);
}

function parsePosition(position) {
if (position === 'atf' || position === 'btf') {
return position;
}
return 'unknown';
/**
* Test if bid has mediaType or mediaTypes set for video.
* note: 'mediaType' has been deprecated, however support will remain for a transitional period
* @param {BidRequest} bidRequest
* @returns {boolean}
*/
export function hasVideoMediaType(bidRequest) {
return bidRequest.mediaType === VIDEO || typeof utils.deepAccess(bidRequest, `mediaTypes.${VIDEO}`) !== 'undefined';
}

export function masSizeOrdering(sizes) {
Expand Down
28 changes: 17 additions & 11 deletions test/spec/modules/rubiconBidAdapter_spec.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {expect} from 'chai';
import adapterManager from 'src/adaptermanager';
import {spec, masSizeOrdering, resetUserSync} from 'modules/rubiconBidAdapter';
import {spec, masSizeOrdering, resetUserSync, hasVideoMediaType} from 'modules/rubiconBidAdapter';
import {parse as parseQuery} from 'querystring';
import {newBidder} from 'src/adapters/bidderFactory';
import {userSync} from 'src/userSync';
Expand Down Expand Up @@ -215,7 +215,7 @@ describe('the rubicon adapter', () => {
'p_aso.video.ext.skipdelay': 15,
'playerHeight': 320,
'playerWidth': 640,
'size_id': 201,
'size_id': 203,
'aeParams': {
'p_aso.video.ext.skip': '1',
'p_aso.video.ext.skipdelay': '15'
Expand Down Expand Up @@ -1214,7 +1214,9 @@ describe('the rubicon adapter', () => {
},
params: {
accountId: 1001,
video: {}
video: {
size_id: 201
}
},
sizes: [[300, 250]]
}
Expand Down Expand Up @@ -1271,13 +1273,17 @@ describe('the rubicon adapter', () => {
expect(spec.isBidRequestValid(bidderRequestCopy.bids[0])).to.equal(false);
});

it('should not validate bid request when video is outstream', () => {
it('bid request is valid when video context is outstream', () => {
createVideoBidderRequestOutstream();
sandbox.stub(Date, 'now').callsFake(() =>
bidderRequest.auctionStart + 100
);

expect(spec.isBidRequestValid(bidderRequest.bids[0])).to.equal(false);
const bidRequestCopy = clone(bidderRequest);

let [request] = spec.buildRequests(bidRequestCopy.bids, bidRequestCopy);
expect(spec.isBidRequestValid(bidderRequest.bids[0])).to.equal(true);
expect(request.data.slots[0].size_id).to.equal(203);
});

it('should get size from bid.sizes too', () => {
Expand Down Expand Up @@ -1377,12 +1383,12 @@ describe('the rubicon adapter', () => {
describe('hasVideoMediaType', () => {
it('should return true if mediaType is video and size_id is set', () => {
createVideoBidderRequest();
const legacyVideoTypeBidRequest = spec.hasVideoMediaType(bidderRequest.bids[0]);
const legacyVideoTypeBidRequest = hasVideoMediaType(bidderRequest.bids[0]);
expect(legacyVideoTypeBidRequest).is.equal(true);
});

it('should return false if mediaType is video and size_id is not defined', () => {
expect(spec.hasVideoMediaType({
expect(spec.isBidRequestValid({
bid: 99,
mediaType: 'video',
params: {
Expand All @@ -1392,17 +1398,17 @@ describe('the rubicon adapter', () => {
});

it('should return false if bidRequest.mediaType is not equal to video', () => {
expect(spec.hasVideoMediaType({
expect(hasVideoMediaType({
mediaType: 'banner'
})).is.equal(false);
});

it('should return false if bidRequest.mediaType is not defined', () => {
expect(spec.hasVideoMediaType({})).is.equal(false);
expect(hasVideoMediaType({})).is.equal(false);
});

it('should return true if bidRequest.mediaTypes.video.context is instream and size_id is defined', () => {
expect(spec.hasVideoMediaType({
expect(hasVideoMediaType({
mediaTypes: {
video: {
context: 'instream'
Expand All @@ -1417,7 +1423,7 @@ describe('the rubicon adapter', () => {
});

it('should return false if bidRequest.mediaTypes.video.context is instream but size_id is not defined', () => {
expect(spec.hasVideoMediaType({
expect(spec.isBidRequestValid({
mediaTypes: {
video: {
context: 'instream'
Expand Down

0 comments on commit b3bfb63

Please sign in to comment.