From 24b76b2f1e5588932ee129bcad970fd8438fb2f9 Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Thu, 14 Dec 2017 13:53:05 -0500 Subject: [PATCH 1/3] initial commit for multiformat size validation checks --- src/adaptermanager.js | 62 +++++++++++++++++++++++++++++++++++++++++++ src/native.js | 12 +++++++++ 2 files changed, 74 insertions(+) diff --git a/src/adaptermanager.js b/src/adaptermanager.js index 670b329ef72..2efe99dfd49 100644 --- a/src/adaptermanager.js +++ b/src/adaptermanager.js @@ -149,6 +149,9 @@ function getAdUnitCopyForClientAdapters(adUnits) { exports.makeBidRequests = function(adUnits, auctionStart, auctionId, cbTimeout, labels) { let bidRequests = []; + + adUnits = isValidBidRequest(adUnits); + let bidderCodes = getBidderCodes(adUnits); if (config.getConfig('bidderSequence') === RANDOM) { bidderCodes = shuffle(bidderCodes); @@ -211,6 +214,65 @@ exports.makeBidRequests = function(adUnits, auctionStart, auctionId, cbTimeout, return bidRequests; }; +function isValidBidRequest(adUnits) { + adUnits.forEach(adUnit => { + if (adUnit.sizes) { + utils.logWarn('Usage of adUnits.sizes will eventually be deprecated. Please define size dimensions within the corresponding area of the mediaTypes. (eg mediaTypes.banner.sizes).'); + } + + if (adUnit.mediaTypes) { + if (adUnit.mediaTypes.banner) { + if (adUnit.mediaTypes.banner.sizes) { + adUnit.sizes = adUnit.mediaTypes.banner.sizes; + } else { + utils.logError('Detected an adUnits.mediaTypes.banner object did not include sizes. Removing adUnits.mediaTypes.banner object from bid request'); + delete adUnit.mediaTypes.banner; + } + } + + if (adUnit.mediaTypes.video) { + if (adUnit.mediaTypes.video.playerSize) { + if ( + Array.isArray(adUnit.mediaTypes.video.playerSize) && + adUnit.mediaTypes.video.playerSize.length === 2 && + !isNaN(adUnit.mediaTypes.video.playerSize[0])) { + adUnit.sizes = adUnit.mediaTypes.video.playerSize; + } else { + utils.logError('Detected incorrect configuration of mediaTypes.video.playerSize. Please specify only one set of dimensions in a format like: [640, 480]'); + delete adUnit.mediaTypes.video; + } + } + } + + if (adUnit.mediaTypes.native) { + if (adUnit.mediaTypes.native.image) { + if (adUnit.mediaTypes.native.image.sizes) { + if (!Array.isArray(adUnit.mediaTypes.native.image.sizes)) { + utils.logWarn('Please use an array of sizes for native.image.sizes field.'); + delete adUnit.mediaTypes.native.image; + } + } + if (adUnit.mediaTypes.native.image.aspect_ratios) { + if (!Array.isArray(adUnit.mediaTypes.native.image.aspect_ratios)) { + utils.logWarn('Please use an array of sizes for native.image.aspect_ratios field.'); + delete adUnit.mediaTypes.native.image; + } + } + } + if (adUnit.mediaTypes.native.icon) { + if (adUnit.mediaTypes.native.icon.sizes) { + if (!Array.isArray(adUnit.mediaTypes.native.image.sizes)) { + utils.logError('Please use an array of sizes for native.icon.sizes field.'); + delete adUnit.mediaTypes.native.icon; + } + } + } + } + } + }); + return adUnits; +} + exports.callBids = (adUnits, bidRequests, addBidResponse, doneCb) => { if (!bidRequests.length) { utils.logWarn('callBids executed with no bidRequests. Were they filtered by labels or sizing?'); diff --git a/src/native.js b/src/native.js index bfe4b64405d..6c8ac266471 100644 --- a/src/native.js +++ b/src/native.js @@ -85,6 +85,18 @@ export function nativeBidIsValid(bid, bidRequests) { return false; } + if (deepAccess(bid, 'native.image')) { + if (!deepAccess(bid, 'native.image.height') || !deepAccess(bid, 'native.image.width')) { + return false; + } + } + + if (deepAccess(bid, 'native.icon')) { + if (!deepAccess(bid, 'native.icon.height') || !deepAccess(bid, 'native.icon.width')) { + return false; + } + } + const requestedAssets = bidRequest.nativeParams; if (!requestedAssets) { return true; From 5cde8695d40476c0f1d22ac31df5d8d411552c87 Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Fri, 15 Dec 2017 19:33:18 -0500 Subject: [PATCH 2/3] adding unit tests and changes to checkBidRequestSizes function --- src/adaptermanager.js | 79 ++++----- test/spec/native_spec.js | 112 +++++++++++- test/spec/unit/core/adapterManager_spec.js | 188 +++++++++++++++++++++ 3 files changed, 333 insertions(+), 46 deletions(-) diff --git a/src/adaptermanager.js b/src/adaptermanager.js index 2efe99dfd49..12a15f7c3c8 100644 --- a/src/adaptermanager.js +++ b/src/adaptermanager.js @@ -150,7 +150,7 @@ function getAdUnitCopyForClientAdapters(adUnits) { exports.makeBidRequests = function(adUnits, auctionStart, auctionId, cbTimeout, labels) { let bidRequests = []; - adUnits = isValidBidRequest(adUnits); + adUnits = exports.checkBidRequestSizes(adUnits); let bidderCodes = getBidderCodes(adUnits); if (config.getConfig('bidderSequence') === RANDOM) { @@ -214,59 +214,48 @@ exports.makeBidRequests = function(adUnits, auctionStart, auctionId, cbTimeout, return bidRequests; }; -function isValidBidRequest(adUnits) { - adUnits.forEach(adUnit => { +exports.checkBidRequestSizes = (adUnits) => { + Array.prototype.forEach.call(adUnits, adUnit => { if (adUnit.sizes) { utils.logWarn('Usage of adUnits.sizes will eventually be deprecated. Please define size dimensions within the corresponding area of the mediaTypes. (eg mediaTypes.banner.sizes).'); } - if (adUnit.mediaTypes) { - if (adUnit.mediaTypes.banner) { - if (adUnit.mediaTypes.banner.sizes) { - adUnit.sizes = adUnit.mediaTypes.banner.sizes; - } else { - utils.logError('Detected an adUnits.mediaTypes.banner object did not include sizes. Removing adUnits.mediaTypes.banner object from bid request'); - delete adUnit.mediaTypes.banner; - } + const mediaTypes = adUnit.mediaTypes; + if (mediaTypes && mediaTypes.banner) { + const banner = mediaTypes.banner; + if (banner.sizes) { + adUnit.sizes = banner.sizes; + } else { + utils.logError('Detected a mediaTypes.banner object did not include sizes. This is a required field for the mediaTypes.banner object. Removing invalid mediaTypes.banner object from request.'); + delete adUnit.mediaTypes.banner; } + } - if (adUnit.mediaTypes.video) { - if (adUnit.mediaTypes.video.playerSize) { - if ( - Array.isArray(adUnit.mediaTypes.video.playerSize) && - adUnit.mediaTypes.video.playerSize.length === 2 && - !isNaN(adUnit.mediaTypes.video.playerSize[0])) { - adUnit.sizes = adUnit.mediaTypes.video.playerSize; - } else { - utils.logError('Detected incorrect configuration of mediaTypes.video.playerSize. Please specify only one set of dimensions in a format like: [640, 480]'); - delete adUnit.mediaTypes.video; - } + if (mediaTypes && mediaTypes.video) { + const video = mediaTypes.video; + if (video.playerSize) { + if (Array.isArray(video.playerSize) && video.playerSize.length === 2 && Number.isInteger(video.playerSize[0]) && Number.isInteger(video.playerSize[1])) { + adUnit.sizes = video.playerSize; + } else { + utils.logError('Detected incorrect configuration of mediaTypes.video.playerSize. Please specify only one set of dimensions in a format like: [640, 480]. Removing invalid mediaTypes.video.playerSize property from request.'); + delete adUnit.mediaTypes.video.playerSize; } } + } - if (adUnit.mediaTypes.native) { - if (adUnit.mediaTypes.native.image) { - if (adUnit.mediaTypes.native.image.sizes) { - if (!Array.isArray(adUnit.mediaTypes.native.image.sizes)) { - utils.logWarn('Please use an array of sizes for native.image.sizes field.'); - delete adUnit.mediaTypes.native.image; - } - } - if (adUnit.mediaTypes.native.image.aspect_ratios) { - if (!Array.isArray(adUnit.mediaTypes.native.image.aspect_ratios)) { - utils.logWarn('Please use an array of sizes for native.image.aspect_ratios field.'); - delete adUnit.mediaTypes.native.image; - } - } - } - if (adUnit.mediaTypes.native.icon) { - if (adUnit.mediaTypes.native.icon.sizes) { - if (!Array.isArray(adUnit.mediaTypes.native.image.sizes)) { - utils.logError('Please use an array of sizes for native.icon.sizes field.'); - delete adUnit.mediaTypes.native.icon; - } - } - } + if (mediaTypes && mediaTypes.native) { + const native = mediaTypes.native; + if (native.image && native.image.sizes && !Array.isArray(native.image.sizes)) { + utils.logError('Please use an array of sizes for native.image.sizes field. Removing invalid mediaTypes.native.image.sizes property from request.'); + delete adUnit.mediaTypes.native.image.sizes; + } + if (native.image && native.image.aspect_ratios && !Array.isArray(native.image.aspect_ratios)) { + utils.logError('Please use an array of sizes for native.image.aspect_ratios field. Removing invalid mediaTypes.native.image.aspect_ratios property from request.'); + delete adUnit.mediaTypes.native.image.aspect_ratios; + } + if (native.icon && native.icon.sizes && !Array.isArray(native.icon.sizes)) { + utils.logError('Please use an array of sizes for native.icon.sizes field. Removing invalid mediaTypes.native.icon.sizes property from request.'); + delete adUnit.mediaTypes.native.icon.sizes; } } }); diff --git a/test/spec/native_spec.js b/test/spec/native_spec.js index 977575a4d19..fa57ceed5f5 100644 --- a/test/spec/native_spec.js +++ b/test/spec/native_spec.js @@ -1,5 +1,5 @@ import { expect } from 'chai'; -import { fireNativeTrackers, getNativeTargeting } from 'src/native'; +import { fireNativeTrackers, getNativeTargeting, nativeBidIsValid } from 'src/native'; const utils = require('src/utils'); const bid = { @@ -44,3 +44,113 @@ describe('native.js', () => { sinon.assert.calledWith(triggerPixelStub, bid.native.clickTrackers[0]); }); }); + +describe('validate native', () => { + let bidReq = [{ + bids: [{ + bidderCode: 'test_bidder', + bidId: 'test_bid_id', + mediaTypes: { + native: { + title: { + required: true, + }, + body: { + required: true, + }, + image: { + required: true, + sizes: [150, 50], + aspect_ratios: [150, 50] + }, + icon: { + required: true, + sizes: [50, 50] + }, + } + } + }] + }]; + + let validBid = { + adId: 'test_bid_id', + adUnitCode: '123/prebid_native_adunit', + bidder: 'test_bidder', + native: { + body: 'This is a Prebid Native Creative. There are many like it, but this one is mine.', + clickTrackers: ['http://my.click.tracker/url'], + icon: { + url: 'http://my.image.file/ad_image.jpg', + height: 75, + width: 75 + }, + image: { + url: 'http://my.icon.file/ad_icon.jpg', + height: 2250, + width: 3000 + }, + clickUrl: 'http://prebid.org/dev-docs/show-native-ads.html', + impressionTrackers: ['http://my.imp.tracker/url'], + title: 'This is an example Prebid Native creative' + } + }; + + let noIconDimBid = { + adId: 'test_bid_id', + adUnitCode: '123/prebid_native_adunit', + bidder: 'test_bidder', + native: { + body: 'This is a Prebid Native Creative. There are many like it, but this one is mine.', + clickTrackers: ['http://my.click.tracker/url'], + icon: { + url: 'http://my.image.file/ad_image.jpg', + height: 0, + width: 0 + }, + image: { + url: 'http://my.icon.file/ad_icon.jpg', + height: 2250, + width: 3000 + }, + clickUrl: 'http://prebid.org/dev-docs/show-native-ads.html', + impressionTrackers: ['http://my.imp.tracker/url'], + title: 'This is an example Prebid Native creative' + } + }; + + let noImgDimBid = { + adId: 'test_bid_id', + adUnitCode: '123/prebid_native_adunit', + bidder: 'test_bidder', + native: { + body: 'This is a Prebid Native Creative. There are many like it, but this one is mine.', + clickTrackers: ['http://my.click.tracker/url'], + icon: { + url: 'http://my.image.file/ad_image.jpg', + height: 75, + width: 75 + }, + image: { + url: 'http://my.icon.file/ad_icon.jpg', + height: 0, + width: 0 + }, + clickUrl: 'http://prebid.org/dev-docs/show-native-ads.html', + impressionTrackers: ['http://my.imp.tracker/url'], + title: 'This is an example Prebid Native creative' + } + }; + + beforeEach(() => {}); + + afterEach(() => {}); + + it('should reject bid if no image sizes are defined', () => { + let result = nativeBidIsValid(validBid, bidReq); + expect(result).to.be.true; + result = nativeBidIsValid(noIconDimBid, bidReq); + expect(result).to.be.false; + result = nativeBidIsValid(noImgDimBid, bidReq); + expect(result).to.be.false; + }); +}); diff --git a/test/spec/unit/core/adapterManager_spec.js b/test/spec/unit/core/adapterManager_spec.js index 95a249263b5..d349dc2a6da 100644 --- a/test/spec/unit/core/adapterManager_spec.js +++ b/test/spec/unit/core/adapterManager_spec.js @@ -1,5 +1,6 @@ import { expect } from 'chai'; import AdapterManager from 'src/adaptermanager'; +import { checkBidRequestSizes } from 'src/adaptermanager'; import { getAdUnits } from 'test/fixtures/fixtures'; import CONSTANTS from 'src/constants.json'; import * as utils from 'src/utils'; @@ -816,4 +817,191 @@ describe('adapterManager tests', () => { }) }); }); + + describe('isValidBidRequest', () => { + describe('positive tests for validating bid request', () => { + beforeEach(() => {}); + + afterEach(() => {}); + it('should main adUnit structure and adUnits.sizes is replaced', () => { + let fullAdUnit = [{ + sizes: [[300, 250], [300, 600]], + mediaTypes: { + banner: { + sizes: [[300, 250]] + }, + video: { + playerSize: [640, 480] + }, + native: { + image: { + sizes: [150, 150], + aspect_ratios: [140, 140] + }, + icon: { + sizes: [75, 75] + } + } + } + }]; + let result = checkBidRequestSizes(fullAdUnit); + expect(result[0].sizes).to.deep.equal([640, 480]); + expect(result[0].mediaTypes.video.playerSize).to.deep.equal([640, 480]); + expect(result[0].mediaTypes.native.image.sizes).to.deep.equal([150, 150]); + expect(result[0].mediaTypes.native.icon.sizes).to.deep.equal([75, 75]); + expect(result[0].mediaTypes.native.image.aspect_ratios).to.deep.equal([140, 140]); + + let noOptnlFieldAdUnit = [{ + sizes: [[300, 250], [300, 600]], + mediaTypes: { + banner: { + sizes: [[300, 250]] + }, + video: { + context: 'outstream' + }, + native: { + image: { + required: true + }, + icon: { + required: true + } + } + } + }]; + result = checkBidRequestSizes(noOptnlFieldAdUnit); + expect(result[0].sizes).to.deep.equal([[300, 250]]); + expect(result[0].mediaTypes.video).to.exist; + + let mixedAdUnit = [{ + sizes: [[300, 250], [300, 600]], + mediaTypes: { + video: { + context: 'outstream', + playerSize: [400, 350] + }, + native: { + image: { + aspect_ratios: [200, 150], + required: true + } + } + } + }]; + result = checkBidRequestSizes(mixedAdUnit); + expect(result[0].sizes).to.deep.equal([400, 350]); + expect(result[0].mediaTypes.video).to.exist; + }); + }); + + describe('negative tests for validating bid requests', () => { + beforeEach(() => { + sinon.stub(utils, 'logError'); + }); + + afterEach(() => { + utils.logError.restore(); + }); + + it('should throw error message and delete an object/property', () => { + let badBanner = [{ + sizes: [[300, 250], [300, 600]], + mediaTypes: { + banner: { + name: 'test' + } + } + }]; + let result = checkBidRequestSizes(badBanner); + expect(result[0].sizes).to.deep.equal([[300, 250], [300, 600]]); + expect(result[0].mediaTypes.banner).to.be.undefined; + sinon.assert.called(utils.logError); + + let badVideo1 = [{ + sizes: [[600, 600]], + mediaTypes: { + video: { + playerSize: '600x400' + } + } + }]; + result = checkBidRequestSizes(badVideo1); + expect(result[0].sizes).to.deep.equal([[600, 600]]); + expect(result[0].mediaTypes.video.playerSize).to.be.undefined; + expect(result[0].mediaTypes.video).to.exist; + sinon.assert.called(utils.logError); + + let badVideo2 = [{ + sizes: [[600, 600]], + mediaTypes: { + video: { + playerSize: ['300', '200'] + } + } + }]; + result = checkBidRequestSizes(badVideo2); + expect(result[0].sizes).to.deep.equal([[600, 600]]); + expect(result[0].mediaTypes.video.playerSize).to.be.undefined; + expect(result[0].mediaTypes.video).to.exist; + sinon.assert.called(utils.logError); + + let badVideo3 = [{ + sizes: [[600, 600]], + mediaTypes: { + video: { + playerSize: [[640, 480]] + } + } + }]; + result = checkBidRequestSizes(badVideo3); + expect(result[0].sizes).to.deep.equal([[600, 600]]); + expect(result[0].mediaTypes.video.playerSize).to.be.undefined; + expect(result[0].mediaTypes.video).to.exist; + sinon.assert.called(utils.logError); + + let badNativeImgSize = [{ + mediaTypes: { + native: { + image: { + sizes: '300x250' + } + } + } + }]; + result = checkBidRequestSizes(badNativeImgSize); + expect(result[0].mediaTypes.native.image.sizes).to.be.undefined; + expect(result[0].mediaTypes.native.image).to.exist; + sinon.assert.called(utils.logError); + + let badNativeImgAspRat = [{ + mediaTypes: { + native: { + image: { + aspect_ratios: '300x250' + } + } + } + }]; + result = checkBidRequestSizes(badNativeImgAspRat); + expect(result[0].mediaTypes.native.image.aspect_ratios).to.be.undefined; + expect(result[0].mediaTypes.native.image).to.exist; + sinon.assert.called(utils.logError); + + let badNativeIcon = [{ + mediaTypes: { + native: { + icon: { + sizes: '300x250' + } + } + } + }]; + result = checkBidRequestSizes(badNativeIcon); + expect(result[0].mediaTypes.native.icon.sizes).to.be.undefined; + expect(result[0].mediaTypes.native.icon).to.exist; + sinon.assert.called(utils.logError); + }); + }); + }); }); From aaeba52be18656fbeadfeb4654a875ab5d9f6e09 Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Thu, 4 Jan 2018 09:59:32 -0500 Subject: [PATCH 3/3] updates to appnexusBidAdapter --- modules/appnexusBidAdapter.js | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/modules/appnexusBidAdapter.js b/modules/appnexusBidAdapter.js index d46be776b59..7db2b9aab2f 100644 --- a/modules/appnexusBidAdapter.js +++ b/modules/appnexusBidAdapter.js @@ -216,20 +216,24 @@ function newBid(serverBid, rtbBid) { body: nativeAd.desc, cta: nativeAd.ctatext, sponsoredBy: nativeAd.sponsored, - image: { - url: nativeAd.main_img && nativeAd.main_img.url, - height: nativeAd.main_img && nativeAd.main_img.height, - width: nativeAd.main_img && nativeAd.main_img.width, - }, - icon: { - url: nativeAd.icon && nativeAd.icon.url, - height: nativeAd.icon && nativeAd.icon.height, - width: nativeAd.icon && nativeAd.icon.width, - }, clickUrl: nativeAd.link.url, clickTrackers: nativeAd.link.click_trackers, impressionTrackers: nativeAd.impression_trackers, }; + if (nativeAd.main_img) { + bid['native'].image = { + url: nativeAd.main_img.url, + height: nativeAd.main_img.height, + width: nativeAd.main_img.width, + }; + } + if (nativeAd.icon) { + bid['native'].icon = { + url: nativeAd.icon.url, + height: nativeAd.icon.height, + width: nativeAd.icon.width, + }; + } } else { Object.assign(bid, { width: rtbBid.rtb.banner.width,