Skip to content

Commit

Permalink
Rubicon Adapter - Multiple media types bug fix (#2347)
Browse files Browse the repository at this point in the history
* Removed validation for video mediaTypes; unsupported/malformed video mediaTypes fallback to banner type

* Updated warning text for mediaTypes actions

* Linting fixes

* Added validation for mediaTypes requiring a banner property if video is malformed to fallback without invalidating

* fixed typo and some spacing

* Added invalidation case for invalid legacy video definition

* Linting fix for space after comma in spec file
  • Loading branch information
idettman authored Apr 10, 2018
1 parent c738ab5 commit d8a81fc
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 14 deletions.
32 changes: 24 additions & 8 deletions modules/rubiconBidAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,31 @@ export const spec = {
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 (spec.hasVideoMediaType(bid)) {
// support instream only
if ((utils.deepAccess(bid, `mediaTypes.${VIDEO}`) && utils.deepAccess(bid, `mediaTypes.${VIDEO}.context`) !== 'instream') || typeof params.video !== 'object' || !params.video.size_id) {
return false;
}
}
return true;
},
/**
Expand Down Expand Up @@ -246,7 +260,8 @@ export const spec = {
* @returns {boolean}
*/
hasVideoMediaType: function(bidRequest) {
return bidRequest.mediaType === VIDEO || typeof utils.deepAccess(bidRequest, `mediaTypes.${VIDEO}`) !== 'undefined';
return (typeof utils.deepAccess(bidRequest, 'params.video.size_id') !== 'undefined' &&
(bidRequest.mediaType === VIDEO || utils.deepAccess(bidRequest, `mediaTypes.${VIDEO}.context`) === 'instream'));
},
/**
* @param {*} responseObj
Expand Down Expand Up @@ -373,7 +388,7 @@ function parseSizes(bid) {
let params = bid.params;
if (spec.hasVideoMediaType(bid)) {
let size = [];
if (typeof params.video === 'object' && params.video.playerWidth && params.video.playerHeight) {
if (params.video && params.video.playerWidth && params.video.playerHeight) {
size = [
params.video.playerWidth,
params.video.playerHeight
Expand All @@ -384,6 +399,7 @@ function parseSizes(bid) {
return size;
}

// deprecated: temp legacy support
let sizes = Array.isArray(params.sizes) ? params.sizes : mapSizes(bid.sizes)

return masSizeOrdering(sizes);
Expand Down
61 changes: 55 additions & 6 deletions test/spec/modules/rubiconBidAdapter_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,29 @@ describe('the rubicon adapter', () => {
expect(floor).to.equal(3.25);
});

it('should not validate bid request when a invalid video object is passed in', () => {
it('should validate bid request with invalid video if a mediaTypes banner property is defined', () => {
const bidRequest = {
mediaTypes: {
video: {
context: 'instream'
},
banner: {
sizes: [[300, 250]]
}
},
params: {
accountId: 1001,
video: {}
},
sizes: [[300, 250]]
}
sandbox.stub(Date, 'now').callsFake(() =>
bidderRequest.auctionStart + 100
);
expect(spec.isBidRequestValid(bidRequest)).to.equal(true);
});

it('should not validate bid request when a invalid video object and no banner object is passed in', () => {
createVideoBidderRequestNoVideo();
sandbox.stub(Date, 'now').callsFake(() =>
bidderRequest.auctionStart + 100
Expand All @@ -730,7 +752,7 @@ describe('the rubicon adapter', () => {
bidRequestCopy.params.video = 123;
expect(spec.isBidRequestValid(bidRequestCopy)).to.equal(false);

bidRequestCopy.params.video = { size_id: '' };
bidRequestCopy.params.video = { size_id: undefined };
expect(spec.isBidRequestValid(bidRequestCopy)).to.equal(false);

delete bidRequestCopy.params.video;
Expand Down Expand Up @@ -798,12 +820,22 @@ describe('the rubicon adapter', () => {
});

describe('hasVideoMediaType', () => {
it('should return true if mediaType is true', () => {
it('should return true if mediaType is video and size_id is set', () => {
createVideoBidderRequest();
const legacyVideoTypeBidRequest = spec.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({
bid: 99,
mediaType: 'video',
params: {
video: {}
}
})).is.equal(false);
});

it('should return false if bidRequest.mediaType is not equal to video', () => {
expect(spec.hasVideoMediaType({
mediaType: 'banner'
Expand All @@ -814,16 +846,33 @@ describe('the rubicon adapter', () => {
expect(spec.hasVideoMediaType({})).is.equal(false);
});

it('should return true if bidRequest.mediaTypes.video object exists', () => {
it('should return true if bidRequest.mediaTypes.video.context is instream and size_id is defined', () => {
expect(spec.hasVideoMediaType({
mediaTypes: {
video: {
context: 'outstream',
playerSize: [300, 250]
context: 'instream'
}
},
params: {
video: {
size_id: 7
}
}
})).is.equal(true);
});

it('should return false if bidRequest.mediaTypes.video.context is instream but size_id is not defined', () => {
expect(spec.hasVideoMediaType({
mediaTypes: {
video: {
context: 'instream'
}
},
params: {
video: {}
}
})).is.equal(false);
});
});
});

Expand Down

0 comments on commit d8a81fc

Please sign in to comment.