Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Prebid 1.0 Fix issue with video bid validation #1680

Merged
merged 6 commits into from
Oct 18, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/auction.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,11 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout}) {
utils.logWarn(`Some adapter tried to add an undefined bid for ${adUnitCode}.`);
return false;
}
if (bid.mediaType === 'native' && !nativeBidIsValid(bid)) {
if (bid.mediaType === 'native' && !nativeBidIsValid(bid, _bidderRequests)) {
utils.logError(errorMessage('Native bid missing some required properties.'));
return false;
}
if (bid.mediaType === 'video' && !isValidVideoBid(bid)) {
if (bid.mediaType === 'video' && !isValidVideoBid(bid, _bidderRequests)) {
utils.logError(errorMessage(`Video bid does not have required vastUrl or renderer property`));
return false;
}
Expand Down
10 changes: 6 additions & 4 deletions src/native.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { auctionManager } from './auctionManager';
import { deepAccess, getBidRequest, logError, triggerPixel } from './utils';

export const nativeAdapters = [];
Expand Down Expand Up @@ -69,12 +68,15 @@ export const nativeBidder = bid => nativeAdapters.includes(bid.bidder);
export const hasNonNativeBidder = adUnit =>
adUnit.bids.filter(bid => !nativeBidder(bid)).length;

/*
/**
* Validate that the native assets on this bid contain all assets that were
* marked as required in the adUnit configuration.
* @param {Bid} bid Native bid to validate
* @param {BidRequest[]} bidRequests All bid requests for an auction
* @return {Boolean} If object is valid
*/
export function nativeBidIsValid(bid) {
const bidRequest = getBidRequest(bid.adId, auctionManager.getBidsRequested());
export function nativeBidIsValid(bid, bidRequests) {
const bidRequest = getBidRequest(bid.adId, bidRequests);
if (!bidRequest) { return false; }

const requestedAssets = bidRequest.nativeParams;
Expand Down
9 changes: 5 additions & 4 deletions src/video.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ export const hasNonVideoBidder = adUnit =>

/**
* Validate that the assets required for video context are present on the bid
* @param {VideoBid} bid video bid to validate
* @return {boolean} If object is valid
* @param {VideoBid} bid Video bid to validate
* @param {BidRequest[]} bidRequests All bid requests for an auction
* @return {Boolean} If object is valid
*/
export function isValidVideoBid(bid) {
const bidRequest = getBidRequest(bid.adId);
export function isValidVideoBid(bid, bidRequests) {
const bidRequest = getBidRequest(bid.adId, bidRequests);

const videoMediaType =
bidRequest && deepAccess(bidRequest, 'mediaTypes.video');
Expand Down
95 changes: 52 additions & 43 deletions test/spec/video_spec.js
Original file line number Diff line number Diff line change
@@ -1,67 +1,76 @@
import { isValidVideoBid } from 'src/video';
const utils = require('src/utils');

describe('video.js', () => {
afterEach(() => {
utils.getBidRequest.restore();
});

it('validates valid instream bids', () => {
sinon.stub(utils, 'getBidRequest', () => ({
bidder: 'appnexusAst',
mediaTypes: {
video: { context: 'instream' },
},
}));

const valid = isValidVideoBid({
const bid = {
adId: '123abc',
vastUrl: 'http://www.example.com/vastUrl'
});

};
const bidRequests = [{
bids: [{
bidId: '123abc',
bidder: 'appnexusAst',
mediaTypes: {
video: { context: 'instream' }
}
}]
}];
const valid = isValidVideoBid(bid, bidRequests);
expect(valid).to.be(true);
});

it('catches invalid instream bids', () => {
sinon.stub(utils, 'getBidRequest', () => ({
bidder: 'appnexusAst',
mediaTypes: {
video: { context: 'instream' },
},
}));

const valid = isValidVideoBid({});

const bid = {
adId: '123abc'
};
const bidRequests = [{
bids: [{
bidId: '123abc',
bidder: 'appnexusAst',
mediaTypes: {
video: { context: 'instream' }
}
}]
}];
const valid = isValidVideoBid(bid, bidRequests);
expect(valid).to.be(false);
});

it('validates valid outstream bids', () => {
sinon.stub(utils, 'getBidRequest', () => ({
bidder: 'appnexusAst',
mediaTypes: {
video: { context: 'outstream' },
},
}));

const valid = isValidVideoBid({
const bid = {
adId: '123abc',
renderer: {
url: 'render.url',
render: () => true,
}
});

};
const bidRequests = [{
bids: [{
bidId: '123abc',
bidder: 'appnexusAst',
mediaTypes: {
video: { context: 'outstream' }
}
}]
}];
const valid = isValidVideoBid(bid, bidRequests);
expect(valid).to.be(true);
});

it('catches invalid outstream bids', () => {
sinon.stub(utils, 'getBidRequest', () => ({
bidder: 'appnexusAst',
mediaTypes: {
video: { context: 'outstream' },
},
}));

const valid = isValidVideoBid({});

const bid = {
adId: '123abc'
};
const bidRequests = [{
bids: [{
bidId: '123abc',
bidder: 'appnexusAst',
mediaTypes: {
video: { context: 'outstream' }
}
}]
}];
const valid = isValidVideoBid(bid, bidRequests);
expect(valid).to.be(false);
});
});