From 2896f90c01636fcadfb9eeb3324047c16d03374f Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Wed, 30 Jan 2019 15:52:49 -0500 Subject: [PATCH 01/21] initial version of adpod module --- modules/adpod.js | 256 +++++++++++++ test/spec/modules/adpod_spec.js | 645 ++++++++++++++++++++++++++++++++ 2 files changed, 901 insertions(+) create mode 100644 modules/adpod.js create mode 100644 test/spec/modules/adpod_spec.js diff --git a/modules/adpod.js b/modules/adpod.js new file mode 100644 index 00000000000..235efbfaab1 --- /dev/null +++ b/modules/adpod.js @@ -0,0 +1,256 @@ +import * as utils from '../src/utils'; +import { addBidToAuction, doCallbacksIfTimedout } from '../src/auction'; +import { store } from '../src/videoCache'; +import { hooks } from '../src/hook'; +import { config } from '../src/config'; +import find from 'core-js/library/fn/array/find'; +import Set from 'core-js/library/fn/set'; + +const from = require('core-js/library/fn/array/from'); +export const ADPOD = 'adpod'; + +// NOTE - can these global vars be handled differently??? +// NOTE - are these good defaults? +let queueTimeDelay = 50; +let queueSizeLimit = 5; +let bidCacheRegistry = createBidCacheRegistry(); + +// NEED TO FIX THE SET.... failures in IE/Safari even with polyfill/package +function createBidCacheRegistry() { + let registry = {}; + return { + addBid: function (bid) { + // create parent level object based on auction ID (in case there are concurrent auctions running) to store objects for that auction + if (!registry[bid.auctionId]) { + registry[bid.auctionId] = {}; + registry[bid.auctionId].bidStorage = new Set(); + registry[bid.auctionId].queueDispatcher = createDispatcher(queueTimeDelay); + registry[bid.auctionId].initialCacheKey = utils.generateUUID(); + } + registry[bid.auctionId].bidStorage.add(bid); + }, + removeBid: function (bid) { + registry[bid.auctionId].bidStorage.delete(bid); + }, + getBids: function (bid) { + return registry[bid.auctionId] && registry[bid.auctionId].bidStorage.values(); + }, + getQueueDispatcher: function(bid) { + return registry[bid.auctionId] && registry[bid.auctionId].queueDispatcher; + }, + getInitialCacheKey: function(bid) { + return registry[bid.auctionId] && registry[bid.auctionId].initialCacheKey; + } + } +} + +function createDispatcher(timeoutDuration) { + let timeout; + let counter = 1; + + return function() { + const context = this; + const args = arguments; + + var callbackFn = function() { + firePrebidCacheCall.apply(context, args); + }; + + clearTimeout(timeout); + + // want to fire off the queue if either: size limit is reached or time has passed + if (counter === queueSizeLimit) { + counter = 1; + callbackFn(); + } else { + counter++; + timeout = setTimeout(callbackFn, timeoutDuration); + } + }; +} + +function updateBidQueue(auctionInstance, bidResponse, afterBidAdded, key) { + let bidListIter = bidCacheRegistry.getBids(bidResponse); + + if (bidListIter) { + let bidListArr = from(bidListIter); + let callDispatcher = bidCacheRegistry.getQueueDispatcher(bidResponse); + callDispatcher(auctionInstance, bidListArr, afterBidAdded, key); + } else { + utils.logWarn('Attempted to cache a bid from an unkonwn auction. Bid:', bidResponse); + } +} + +function removeBidsFromStorage(bidResponses) { + for (let i = 0; i < bidResponses.length; i++) { + bidCacheRegistry.removeBid(bidResponses[i]); + } +} + +function firePrebidCacheCall(auctionInstance, bidList, afterBidAdded, key) { + let bidListCopy = bidList.slice(); // is making a copy really needed? + + // remove entries now so other incoming bids won't accidentally have a stale version of the list while PBC is processing the current submitted list + removeBidsFromStorage(bidListCopy); + + store(bidListCopy, function (error, cacheIds) { + if (error) { + utils.logWarn(`Failed to save to the video cache: ${error}. Video bid(s) must be discarded.`); + for (let i = 0; i < bidList.length; i++) { + doCallbacksIfTimedout(auctionInstance, bidList[i]); + } + } else { + for (let i = 0; i < cacheIds.length; i++) { + // when uuid in response is empty string then the key already existed, so this bid wasn't cached + // TODO - should we throw warning here? + // TODO - verify the cacheKey is one of the expected values? + if (cacheIds[i].uuid !== '') { + // bidListCopy[i].videoCacheKey = cacheIds[i].uuid; // remove later + addBidToAuction(auctionInstance, bidListCopy[i]); + afterBidAdded(); + } + } + } + }); +} + +function attachCustomCacheKeyToBid(bid) { + let cpmFixed = bid.cpm.toFixed(2); + // TODO - check internally on field name for FW category + // TODO - remove backup values later (once adapter code is worked in for testing) + let category = (bid.meta && bid.meta.primaryCatId) || 'testCategory'; + let duration = (bid.video && bid.video.durationSeconds) || randomGen(60); + let pid = `${cpmFixed}_${category}_${duration}s`; + let initialCacheKey = bidCacheRegistry.getInitialCacheKey(bid); + + if (!bid.adserverTargeting) { + bid.adserverTargeting = {}; + } + bid.adserverTargeting.hb_uuid = initialCacheKey; + bid.adserverTargeting.hb_price_industry_duration = pid; + bid.customCacheKey = `${pid}_${initialCacheKey}`; +} + +export function callPrebidCacheHook(fn, auctionInstance, bidResponse, afterBidAdded, bidderRequest) { + let videoConfig = bidderRequest.mediaTypes && bidderRequest.mediaTypes.video; + // if (videoConfig && videoConfig.context === 'instream') { //remove later + if (videoConfig && videoConfig.context === ADPOD) { + let allowBidToCache = true; + // TODO - move this code and the custom cache key to a separate hooked function in Jaimin's FW PR; this stuff is FW specific and doesn't belong here + if (videoConfig.requireExactDuration) { + let bidDuration = bidResponse.video.durationSeconds; + let ranges = videoConfig.durationRangeSec; + + ranges.sort((a, b) => a - b); // ensure the ranges are sorted in numeric order + + // if bidDuration is not an exact match to a listed range value, round bidDuration to closest highest range + if (!ranges.some(range => range === bidDuration)) { + let nextHighestRange = find(ranges, (value) => value > bidDuration); + + if (nextHighestRange) { + bidResponse.video.durationSeconds = nextHighestRange; + } else { + allowBidToCache = false; + utils.logWarn(`Detected a bid with a duration value higher than any accepted range found in mediaTypes.video.durationRangeSec. Rejecting bid: `, bidResponse); + afterBidAdded(); + } + } + } + + if (allowBidToCache) { + bidCacheRegistry.addBid(bidResponse); + attachCustomCacheKeyToBid(bidResponse); + + updateBidQueue(auctionInstance, bidResponse, afterBidAdded, bidResponse.customCacheKey); + } + } else { + fn.call(this, auctionInstance, bidResponse, afterBidAdded, bidderRequest); + } +} + +// TODO remove later - just for dev work +function randomGen(max) { + return Math.floor(Math.random() * Math.floor(max)); +} + +export function checkAdUnitSetupHook(fn, adUnits) { + let goodAdUnits = adUnits.filter(adUnit => { + let videoConfig = adUnit.mediaTypes && adUnit.mediaTypes.video; + if (videoConfig && videoConfig.context === ADPOD) { + let errMsg = `Detected missing or incorrectly setup fields for an adpod adUnit. Please review the following fields of adUnitCode: ${adUnit.code}. This adUnit will be removed from the auction.`; + + let playerSize = !!(videoConfig.playerSize && utils.isArrayOfNums(videoConfig.playerSize)); + let adPodDurationSec = !!(videoConfig.adPodDurationSec && utils.isNumber(videoConfig.adPodDurationSec)); + let durationRangeSec = !!(videoConfig.durationRangeSec && utils.isArrayOfNums(videoConfig.durationRangeSec)); + + if (!playerSize || !adPodDurationSec || !durationRangeSec) { + errMsg += (!playerSize) ? '\nmediaTypes.video.playerSize' : ''; + errMsg += (!adPodDurationSec) ? '\nmediaTypes.video.adPodDurationSec' : ''; + errMsg += (!durationRangeSec) ? '\nmediaTypes.video.durationRangeSec' : ''; + utils.logWarn(errMsg); + return false; + } + } + return true; + }); + adUnits = goodAdUnits; + return fn.apply(this, adUnits); +} + +export function checkVideoBidSetupHook(fn, bid, bidRequest, videoMediaType, context) { + if (context === ADPOD) { + // TODO - check internally on field name for FW category + if (!utils.deepAccess(bid, 'meta.primaryCatId')) { + return false; + } + + if (utils.deepAccess(bid, 'video')) { + if (!utils.deepAccess(bid, 'video.context') || bid.video.context !== ADPOD) { + return false; + } + + if (!utils.deepAccess(bid, 'video.durationSeconds')) { + return false; + } + } + + if (!config.getConfig('cache.url') && bid.vastXml && !bid.vastUrl) { + utils.logError(` + This bid contains only vastXml and will not work when a prebid cache url is not specified. + Try enabling prebid cache with pbjs.setConfig({ cache: {url: "..."} }); + `); + return false; + }; + + return true; + } else { + return fn.call(this, bid, bidRequest, videoMediaType, context); + } +} + +export function adpodSetConfig(config) { + if (config.bidQueueTimeDelay !== undefined) { + if (typeof config.bidQueueTimeDelay === 'number' && config.bidQueueTimeDelay > 0) { + // add check to see if config setting is too high? + queueTimeDelay = config.bidQueueTimeDelay; + } else { + utils.logWarn(`Detected invalid value for adpod.bidQueueTimeDelay in setConfig; must be a positive number. Using default: ${queueTimeDelay}`) + } + } + + if (config.bidQueueSizeLimit !== undefined) { + if (typeof config.bidQueueSizeLimit === 'number' && config.bidQueueSizeLimit > 0) { + // add check to see if config setting is too high? too low? + queueSizeLimit = config.bidQueueSizeLimit; + } else { + utils.logWarn(`Detected invalid value for adpod.bidQueueSizeLimit in setConfig; must be a positive number. Using default: ${queueSizeLimit}`) + } + } +} +config.getConfig('adpod', config => adpodSetConfig(config.adpod)); + +export function initAdpod() { + hooks['callPrebidCache'].before(callPrebidCacheHook, 15); + hooks['checkAdUnitSetup'].before(checkAdUnitSetupHook, 15); + hooks['checkVideoBidSetup'].before(checkVideoBidSetupHook, 15); +} diff --git a/test/spec/modules/adpod_spec.js b/test/spec/modules/adpod_spec.js new file mode 100644 index 00000000000..bf162026a9f --- /dev/null +++ b/test/spec/modules/adpod_spec.js @@ -0,0 +1,645 @@ +// import * as utils from 'src/utils'; +// import { config } from 'src/config'; +// import * as videoCache from 'src/videoCache'; +// import * as auction from 'src/auction'; + +// import { callPrebidCacheHook, checkAdUnitSetupHook, checkVideoBidSetupHook, adpodSetConfig, ADPOD } from 'modules/adpod'; + +// let expect = require('chai').expect; + +// describe('adpod.js', function () { +// let logErrorStub; +// let logWarnStub; + +// describe('callPrebidCacheHook', function () { +// let callbackResult; +// let clock; +// let addBidToAuctionStub; +// let doCallbacksIfTimedoutStub; +// let storeStub; +// let afterBidAddedSpy; +// let auctionBids = []; + +// let callbackFn = function() { +// callbackResult = true; +// } + +// const fakeStoreFn = function(bids, callback) { +// let payload = []; +// bids.forEach(bid => payload.push({uuid: bid.customCacheKey})); +// callback(null, payload); +// } + +// beforeEach(function() { +// callbackResult = null; +// afterBidAddedSpy = sinon.spy(); +// storeStub = sinon.stub(videoCache, 'store'); +// logWarnStub = sinon.stub(utils, 'logWarn'); +// addBidToAuctionStub = sinon.stub(auction, 'addBidToAuction').callsFake(function (auctionInstance, bid) { +// auctionBids.push(bid); +// }); +// doCallbacksIfTimedoutStub = sinon.stub(auction, 'doCallbacksIfTimedout'); +// clock = sinon.useFakeTimers(); +// config.setConfig({ +// cache: { +// url: 'https://prebid.adnxs.com/pbc/v1/cache' +// } +// }); +// }); + +// afterEach(function() { +// storeStub.restore(); +// logWarnStub.restore(); +// addBidToAuctionStub.restore(); +// doCallbacksIfTimedoutStub.restore(); +// clock.restore(); +// config.resetConfig(); +// auctionBids = []; +// }) + +// it('should redirect back to the original function if bid is not an adpod video', function () { +// let bid = { +// adId: 'testAdId_123', +// mediaType: 'video' +// }; + +// let bidderRequest = { +// adUnitCode: 'adUnit_123', +// mediaTypes: { +// video: { +// context: 'outstream' +// } +// } +// } + +// callPrebidCacheHook(this, bid, function () {}, bidderRequest, callbackFn); +// expect(callbackResult).to.equal(true); +// }); + +// it('should reject a bid if the duration is too high', function () { +// let bidResponse = { +// adId: 'badAd890', +// auctionId: 'bad123', +// mediaType: 'video', +// cpm: 9, +// meta: { +// primaryCatId: 'food' +// }, +// video: { +// context: ADPOD, +// durationSeconds: 600 +// } +// }; + +// let bidderRequest = { +// adUnitCode: 'adpod_0', +// auctionId: 'bad123', +// mediaTypes: { +// video: { +// context: ADPOD, +// playerSize: [300, 300], +// adPodDurationSec: 600, +// durationRangeSec: [15, 30, 60], +// requireExactDuration: true +// } +// } +// }; + +// callPrebidCacheHook(this, bidResponse, afterBidAddedSpy, bidderRequest, callbackFn); + +// expect(callbackResult).to.be.null; +// expect(auctionBids.length).to.equal(0); +// expect(logWarnStub.calledOnce).to.equal(true); +// expect(storeStub.called).to.equal(false); +// expect(afterBidAddedSpy.called).to.equal(true); +// }); + +// it('should send prebid cache call once bid queue is full', function () { +// storeStub.callsFake(fakeStoreFn); + +// config.setConfig({ +// adpod: { +// bidQueueSizeLimit: 2 +// } +// }); + +// let bidResponse1 = { +// adId: 'adId123', +// auctionId: 'full_abc123', +// mediaType: 'video', +// cpm: 10, +// meta: { +// primaryCatId: 'airline' +// }, +// video: { +// context: ADPOD, +// durationSeconds: 30 +// } +// }; +// let bidResponse2 = { +// adId: 'adId234', +// auctionId: 'full_abc123', +// mediaType: 'video', +// cpm: 15, +// meta: { +// primaryCatId: 'airline' +// }, +// video: { +// context: ADPOD, +// durationSeconds: 30 +// } +// }; +// let bidderRequest = { +// adUnitCode: 'adpod_1', +// auctionId: 'full_abc123', +// mediaTypes: { +// video: { +// context: ADPOD, +// playerSize: [300, 300], +// adPodDurationSec: 120, +// durationRangeSec: [15, 30], +// requireExactDuration: false +// } +// } +// }; + +// callPrebidCacheHook(this, bidResponse1, afterBidAddedSpy, bidderRequest, callbackFn); +// callPrebidCacheHook(this, bidResponse2, afterBidAddedSpy, bidderRequest, callbackFn); + +// expect(callbackResult).to.be.null; +// expect(afterBidAddedSpy.calledTwice).to.equal(true); +// expect(auctionBids.length).to.equal(2); +// expect(auctionBids[0].adId).to.equal('adId123'); +// expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^10\.00_airline_30s_.*/); +// expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('10.00_airline_30s'); +// expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; +// expect(auctionBids[1].adId).to.equal('adId234'); +// expect(auctionBids[1].customCacheKey).to.exist.and.to.match(/^15\.00_airline_30s_.*/); +// expect(auctionBids[1].adserverTargeting.hb_price_industry_duration).to.equal('15.00_airline_30s'); +// expect(auctionBids[1].adserverTargeting.hb_uuid).to.exist; +// }); + +// it('should send prebid cache call after set period of time (even if queue is not full)', function () { +// storeStub.callsFake(fakeStoreFn); + +// config.setConfig({ +// adpod: { +// bidQueueSizeLimit: 2, +// bidQueueTimeDelay: 30 +// } +// }); + +// let bidResponse = { +// adId: 'adId234', +// auctionId: 'timer_abc234', +// mediaType: 'video', +// cpm: 15, +// meta: { +// primaryCatId: 'airline' +// }, +// video: { +// context: ADPOD, +// durationSeconds: 25 +// } +// }; +// let bidderRequest = { +// adUnitCode: 'adpod_2', +// auctionId: 'timer_abc234', +// mediaTypes: { +// video: { +// context: ADPOD, +// playerSize: [300, 300], +// adPodDurationSec: 120, +// durationRangeSec: [15, 30], +// requireExactDuration: true +// } +// } +// }; + +// callPrebidCacheHook(this, bidResponse, afterBidAddedSpy, bidderRequest, callbackFn); +// clock.tick(31); + +// expect(callbackResult).to.be.null; +// expect(afterBidAddedSpy.calledOnce).to.equal(true); +// expect(auctionBids.length).to.equal(1); +// expect(auctionBids[0].adId).to.equal('adId234'); +// expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^15\.00_airline_30s_.*/); +// expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('15.00_airline_30s'); +// expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; +// }); + +// it('should execute multiple prebid cache calls when number of bids exceeds queue size', function () { +// storeStub.callsFake(fakeStoreFn); + +// config.setConfig({ +// adpod: { +// bidQueueSizeLimit: 2, +// bidQueueTimeDelay: 30 +// } +// }); + +// let bidResponse1 = { +// adId: 'multi_ad1', +// auctionId: 'multi_call_abc345', +// mediaType: 'video', +// cpm: 15, +// meta: { +// primaryCatId: 'airline' +// }, +// video: { +// context: ADPOD, +// durationSeconds: 15 +// } +// }; +// let bidResponse2 = { +// adId: 'multi_ad2', +// auctionId: 'multi_call_abc345', +// mediaType: 'video', +// cpm: 15, +// meta: { +// primaryCatId: 'news' +// }, +// video: { +// context: ADPOD, +// durationSeconds: 15 +// } +// }; +// let bidResponse3 = { +// adId: 'multi_ad3', +// auctionId: 'multi_call_abc345', +// mediaType: 'video', +// cpm: 10, +// meta: { +// primaryCatId: 'sports' +// }, +// video: { +// context: ADPOD, +// durationSeconds: 15 +// } +// }; + +// let bidderRequest = { +// adUnitCode: 'adpod_3', +// auctionId: 'multi_call_abc345', +// mediaTypes: { +// video: { +// context: ADPOD, +// playerSize: [300, 300], +// adPodDurationSec: 45, +// durationRangeSec: [15, 30], +// requireExactDuration: false +// } +// } +// }; + +// callPrebidCacheHook(this, bidResponse1, afterBidAddedSpy, bidderRequest, callbackFn); +// callPrebidCacheHook(this, bidResponse2, afterBidAddedSpy, bidderRequest, callbackFn); +// callPrebidCacheHook(this, bidResponse3, afterBidAddedSpy, bidderRequest, callbackFn); +// clock.next(); + +// expect(callbackResult).to.be.null; +// expect(afterBidAddedSpy.calledThrice).to.equal(true); +// expect(storeStub.calledTwice).to.equal(true); +// expect(auctionBids.length).to.equal(3); +// expect(auctionBids[0].adId).to.equal('multi_ad1'); +// expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^15\.00_airline_15s_.*/); +// expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('15.00_airline_15s'); +// expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; +// expect(auctionBids[1].adId).to.equal('multi_ad2'); +// expect(auctionBids[1].customCacheKey).to.exist.and.to.match(/^15\.00_news_15s_.*/); +// expect(auctionBids[1].adserverTargeting.hb_price_industry_duration).to.equal('15.00_news_15s'); +// expect(auctionBids[1].adserverTargeting.hb_uuid).to.exist.and.to.equal(auctionBids[0].adserverTargeting.hb_uuid); +// expect(auctionBids[2].adId).to.equal('multi_ad3'); +// expect(auctionBids[2].customCacheKey).to.exist.and.to.match(/^10\.00_sports_15s_.*/); +// expect(auctionBids[2].adserverTargeting.hb_price_industry_duration).to.equal('10.00_sports_15s'); +// expect(auctionBids[2].adserverTargeting.hb_uuid).to.exist.and.to.equal(auctionBids[0].adserverTargeting.hb_uuid); +// }); + +// it('should not add bid to auction when Prebid Cache detects an existing key', function () { +// storeStub.callsFake(function(bids, callback) { +// let payload = []; +// bids.forEach(bid => payload.push({uuid: bid.customCacheKey})); + +// // fake a duplicate bid response from PBC (sets an empty string for the uuid) +// payload[1].uuid = ''; +// callback(null, payload); +// }); + +// config.setConfig({ +// adpod: { +// bidQueueSizeLimit: 2 +// } +// }); + +// let bidResponse1 = { +// adId: 'dup_ad_1', +// auctionId: 'duplicate_def123', +// mediaType: 'video', +// cpm: 5, +// meta: { +// primaryCatId: 'tech' +// }, +// video: { +// context: ADPOD, +// durationSeconds: 45 +// } +// }; +// let bidResponse2 = { +// adId: 'dup_ad_2', +// auctionId: 'duplicate_def123', +// mediaType: 'video', +// cpm: 5, +// meta: { +// primaryCatId: 'tech' +// }, +// video: { +// context: ADPOD, +// durationSeconds: 45 +// } +// }; +// let bidderRequest = { +// adUnitCode: 'adpod_4', +// auctionId: 'duplicate_def123', +// mediaTypes: { +// video: { +// context: ADPOD, +// playerSize: [300, 300], +// adPodDurationSec: 120, +// durationRangeSec: [15, 30, 45], +// requireExactDuration: false +// } +// } +// }; + +// callPrebidCacheHook(this, bidResponse1, afterBidAddedSpy, bidderRequest, callbackFn); +// callPrebidCacheHook(this, bidResponse2, afterBidAddedSpy, bidderRequest, callbackFn); + +// expect(callbackResult).to.be.null; +// expect(afterBidAddedSpy.calledOnce).to.equal(true); +// expect(storeStub.calledOnce).to.equal(true); +// expect(auctionBids.length).to.equal(1); +// expect(auctionBids[0].adId).to.equal('dup_ad_1'); +// expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^5\.00_tech_45s_.*/); +// expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('5.00_tech_45s'); +// expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; +// }); + +// it('should not add bids to auction if PBC returns an error', function() { +// storeStub.callsFake(function(bids, callback) { +// let payload = []; +// let errmsg = 'invalid json'; + +// callback(errmsg, payload); +// }); + +// config.setConfig({ +// adpod: { +// bidQueueSizeLimit: 2 +// } +// }); + +// let bidResponse1 = { +// adId: 'err_ad_1', +// auctionId: 'error_xyz123', +// mediaType: 'video', +// cpm: 5, +// meta: { +// primaryCatId: 'tech' +// }, +// video: { +// context: ADPOD, +// durationSeconds: 30 +// } +// }; +// let bidResponse2 = { +// adId: 'err_ad_2', +// auctionId: 'error_xyz123', +// mediaType: 'video', +// cpm: 5, +// meta: { +// primaryCatId: 'tech' +// }, +// video: { +// context: ADPOD, +// durationSeconds: 30 +// } +// }; +// let bidderRequest = { +// adUnitCode: 'adpod_5', +// auctionId: 'error_xyz123', +// mediaTypes: { +// video: { +// context: ADPOD, +// playerSize: [300, 300], +// adPodDurationSec: 120, +// durationRangeSec: [15, 30, 45], +// requireExactDuration: false +// } +// } +// }; + +// callPrebidCacheHook(this, bidResponse1, afterBidAddedSpy, bidderRequest, callbackFn); +// callPrebidCacheHook(this, bidResponse2, afterBidAddedSpy, bidderRequest, callbackFn); + +// expect(doCallbacksIfTimedoutStub.calledTwice).to.equal(true); +// expect(logWarnStub.calledOnce).to.equal(true); +// expect(auctionBids.length).to.equal(0); +// }); +// }); + +// describe('checkAdUnitSetupHook', function () { +// beforeEach(function () { +// logWarnStub = sinon.stub(utils, 'logWarn'); +// }); + +// afterEach(function() { +// utils.logWarn.restore(); +// }); + +// it('removes an incorrectly setup adpod adunit', function() { +// let results; +// let adUnits = [{ +// code: 'test1', +// mediaTypes: { +// video: { +// context: ADPOD +// } +// } +// }, { +// code: 'test2', +// mediaTypes: { +// video: { +// context: 'instream' +// } +// } +// }]; + +// checkAdUnitSetupHook(adUnits, function (adUnits) { +// results = adUnits; +// }); + +// expect(results).to.deep.equal([{ +// code: 'test2', +// mediaTypes: { +// video: { +// context: 'instream' +// } +// } +// }]); +// expect(logWarnStub.calledOnce).to.equal(true); +// }); + +// it('accepts mixed set of adunits', function() { +// let results; +// let adUnits = [{ +// code: 'test3', +// mediaTypes: { +// video: { +// context: ADPOD, +// playerSize: [300, 300], +// adPodDurationSec: 360, +// durationRangeSec: [15, 30, 45] +// } +// } +// }, { +// code: 'test4', +// mediaTypes: { +// banner: { +// sizes: [[300, 250]] +// } +// } +// }]; + +// checkAdUnitSetupHook(adUnits, function (adUnits) { +// results = adUnits; +// }); + +// expect(results).to.deep.equal(adUnits); +// expect(logWarnStub.called).to.equal(false); +// }); +// }); + +// describe('checkVideoBidSetupHook', function () { +// let callbackResult; +// const adpodTestBid = { +// video: { +// context: ADPOD, +// durationSeconds: 300 +// }, +// meta: { +// primaryCatId: 'testCategory_123' +// }, +// vastXml: 'test XML here' +// }; + +// beforeEach(function() { +// callbackResult = null; +// logErrorStub = sinon.stub(utils, 'logError'); +// }); + +// afterEach(function() { +// config.resetConfig(); +// utils.logError.restore(); +// }) + +// it('redirects to original function for non-adpod type video bids', function() { +// let bannerTestBid = { +// mediaType: 'video' +// }; +// let hookReturnValue = checkVideoBidSetupHook(bannerTestBid, {}, {}, 'instream', function (bid) { +// callbackResult = bid; +// }); +// expect(callbackResult).to.deep.equal(bannerTestBid); +// expect(hookReturnValue).to.be.undefined; +// expect(logErrorStub.called).to.equal(false); +// }); + +// it('returns true when adpod bid is properly setup', function() { +// config.setConfig({ +// cache: { +// url: 'http://test.cache.url/endpoint' +// } +// }); + +// let goodBid = utils.deepClone(adpodTestBid); +// let hookReturnValue = checkVideoBidSetupHook(goodBid, {}, {}, ADPOD, function (bid) { +// callbackResult = bid; +// }); +// expect(callbackResult).to.be.null; +// expect(hookReturnValue).to.equal(true); +// expect(logErrorStub.called).to.equal(false); +// }); + +// it('returns false when a required property from an adpod bid is missing', function() { +// function testInvalidAdpodBid(badTestBid, shouldErrorBeLogged) { +// let hookReturnValue = checkVideoBidSetupHook(badTestBid, {}, {}, ADPOD, function(bid) { +// callbackResult = bid; +// }); +// expect(callbackResult).to.be.null; +// expect(hookReturnValue).to.equal(false); +// expect(logErrorStub.called).to.equal(shouldErrorBeLogged); +// } + +// let noCatBid = utils.deepClone(adpodTestBid); +// delete noCatBid.meta; +// testInvalidAdpodBid(noCatBid, false); + +// let noContextBid = utils.deepClone(adpodTestBid); +// delete noContextBid.video.context; +// testInvalidAdpodBid(noContextBid, false); + +// let wrongContextBid = utils.deepClone(adpodTestBid); +// wrongContextBid.video.context = 'instream'; +// testInvalidAdpodBid(wrongContextBid, false); + +// let noDurationBid = utils.deepClone(adpodTestBid); +// delete noDurationBid.video.durationSeconds; +// testInvalidAdpodBid(noDurationBid, false); + +// let noCacheUrlBid = utils.deepClone(adpodTestBid); +// testInvalidAdpodBid(noCacheUrlBid, true); +// }); +// }); + +// describe('adpodSetConfig', function () { +// let logWarnStub; +// beforeEach(function() { +// logWarnStub = sinon.stub(utils, 'logWarn'); +// }); + +// afterEach(function () { +// logWarnStub.restore(); +// }); + +// it('should log a warning when values other than numbers are used in setConfig', function() { +// adpodSetConfig({ +// bidQueueSizeLimit: '2', +// bidQueueTimeDelay: '50' +// }); +// expect(logWarnStub.calledTwice).to.equal(true); +// }); + +// it('should log a warning when numbers less than or equal to zero are used in setConfig', function() { +// adpodSetConfig({ +// bidQueueSizeLimit: 0, +// bidQueueTimeDelay: -2 +// }); +// expect(logWarnStub.calledTwice).to.equal(true); +// }); + +// // TODO add unit test if there should be a ceiling for acceptable values + +// it('should not log any warning when using a valid config', function() { +// adpodSetConfig({ +// bidQueueSizeLimit: 10 +// }); +// expect(logWarnStub.called).to.equal(false); + +// adpodSetConfig({ +// bidQueueTimeDelay: 100, +// bidQueueSizeLimit: 20 +// }); +// expect(logWarnStub.called).to.equal(false); +// }) +// }); +// }); From 01369b585287ed0f54ac78528541c64ba9b511cf Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Thu, 31 Jan 2019 10:37:51 -0500 Subject: [PATCH 02/21] fix hooks, clean-up and uncomment unit tests --- modules/adpod.js | 75 +- test/spec/modules/adpod_spec.js | 1254 +++++++++++++++---------------- 2 files changed, 623 insertions(+), 706 deletions(-) diff --git a/modules/adpod.js b/modules/adpod.js index 235efbfaab1..f76cf4504d1 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -3,7 +3,6 @@ import { addBidToAuction, doCallbacksIfTimedout } from '../src/auction'; import { store } from '../src/videoCache'; import { hooks } from '../src/hook'; import { config } from '../src/config'; -import find from 'core-js/library/fn/array/find'; import Set from 'core-js/library/fn/set'; const from = require('core-js/library/fn/array/from'); @@ -15,7 +14,6 @@ let queueTimeDelay = 50; let queueSizeLimit = 5; let bidCacheRegistry = createBidCacheRegistry(); -// NEED TO FIX THE SET.... failures in IE/Safari even with polyfill/package function createBidCacheRegistry() { let registry = {}; return { @@ -58,7 +56,7 @@ function createDispatcher(timeoutDuration) { clearTimeout(timeout); - // want to fire off the queue if either: size limit is reached or time has passed + // want to fire off the queue if either: size limit is reached or time has passed since last call to dispatcher if (counter === queueSizeLimit) { counter = 1; callbackFn(); @@ -69,13 +67,13 @@ function createDispatcher(timeoutDuration) { }; } -function updateBidQueue(auctionInstance, bidResponse, afterBidAdded, key) { +function updateBidQueue(auctionInstance, bidResponse, afterBidAdded) { let bidListIter = bidCacheRegistry.getBids(bidResponse); if (bidListIter) { let bidListArr = from(bidListIter); let callDispatcher = bidCacheRegistry.getQueueDispatcher(bidResponse); - callDispatcher(auctionInstance, bidListArr, afterBidAdded, key); + callDispatcher(auctionInstance, bidListArr, afterBidAdded); } else { utils.logWarn('Attempted to cache a bid from an unkonwn auction. Bid:', bidResponse); } @@ -87,8 +85,11 @@ function removeBidsFromStorage(bidResponses) { } } -function firePrebidCacheCall(auctionInstance, bidList, afterBidAdded, key) { - let bidListCopy = bidList.slice(); // is making a copy really needed? +function firePrebidCacheCall(auctionInstance, bidList, afterBidAdded) { + // note to reviewers - doing this to ensure the variable wouldn't be malformed by the additional incoming bids + // while the current list is being processed by PBC and the store callback + // ...but is this copy really needed? + let bidListCopy = bidList.slice(); // remove entries now so other incoming bids won't accidentally have a stale version of the list while PBC is processing the current submitted list removeBidsFromStorage(bidListCopy); @@ -102,8 +103,8 @@ function firePrebidCacheCall(auctionInstance, bidList, afterBidAdded, key) { } else { for (let i = 0; i < cacheIds.length; i++) { // when uuid in response is empty string then the key already existed, so this bid wasn't cached - // TODO - should we throw warning here? - // TODO - verify the cacheKey is one of the expected values? + // TODO - should we throw warning here? should we call the afterBidAdded() anyway to decrement the internal bid counter? + // TODO - verify the cacheKey is one of the expected values (eg in case PBC returns generic UUID)? if (cacheIds[i].uuid !== '') { // bidListCopy[i].videoCacheKey = cacheIds[i].uuid; // remove later addBidToAuction(auctionInstance, bidListCopy[i]); @@ -114,65 +115,17 @@ function firePrebidCacheCall(auctionInstance, bidList, afterBidAdded, key) { }); } -function attachCustomCacheKeyToBid(bid) { - let cpmFixed = bid.cpm.toFixed(2); - // TODO - check internally on field name for FW category - // TODO - remove backup values later (once adapter code is worked in for testing) - let category = (bid.meta && bid.meta.primaryCatId) || 'testCategory'; - let duration = (bid.video && bid.video.durationSeconds) || randomGen(60); - let pid = `${cpmFixed}_${category}_${duration}s`; - let initialCacheKey = bidCacheRegistry.getInitialCacheKey(bid); - - if (!bid.adserverTargeting) { - bid.adserverTargeting = {}; - } - bid.adserverTargeting.hb_uuid = initialCacheKey; - bid.adserverTargeting.hb_price_industry_duration = pid; - bid.customCacheKey = `${pid}_${initialCacheKey}`; -} - export function callPrebidCacheHook(fn, auctionInstance, bidResponse, afterBidAdded, bidderRequest) { let videoConfig = bidderRequest.mediaTypes && bidderRequest.mediaTypes.video; // if (videoConfig && videoConfig.context === 'instream') { //remove later if (videoConfig && videoConfig.context === ADPOD) { - let allowBidToCache = true; - // TODO - move this code and the custom cache key to a separate hooked function in Jaimin's FW PR; this stuff is FW specific and doesn't belong here - if (videoConfig.requireExactDuration) { - let bidDuration = bidResponse.video.durationSeconds; - let ranges = videoConfig.durationRangeSec; - - ranges.sort((a, b) => a - b); // ensure the ranges are sorted in numeric order - - // if bidDuration is not an exact match to a listed range value, round bidDuration to closest highest range - if (!ranges.some(range => range === bidDuration)) { - let nextHighestRange = find(ranges, (value) => value > bidDuration); - - if (nextHighestRange) { - bidResponse.video.durationSeconds = nextHighestRange; - } else { - allowBidToCache = false; - utils.logWarn(`Detected a bid with a duration value higher than any accepted range found in mediaTypes.video.durationRangeSec. Rejecting bid: `, bidResponse); - afterBidAdded(); - } - } - } - - if (allowBidToCache) { - bidCacheRegistry.addBid(bidResponse); - attachCustomCacheKeyToBid(bidResponse); - - updateBidQueue(auctionInstance, bidResponse, afterBidAdded, bidResponse.customCacheKey); - } + bidCacheRegistry.addBid(bidResponse); + updateBidQueue(auctionInstance, bidResponse, afterBidAdded); } else { fn.call(this, auctionInstance, bidResponse, afterBidAdded, bidderRequest); } } -// TODO remove later - just for dev work -function randomGen(max) { - return Math.floor(Math.random() * Math.floor(max)); -} - export function checkAdUnitSetupHook(fn, adUnits) { let goodAdUnits = adUnits.filter(adUnit => { let videoConfig = adUnit.mediaTypes && adUnit.mediaTypes.video; @@ -194,7 +147,7 @@ export function checkAdUnitSetupHook(fn, adUnits) { return true; }); adUnits = goodAdUnits; - return fn.apply(this, adUnits); + fn.call(this, adUnits); } export function checkVideoBidSetupHook(fn, bid, bidRequest, videoMediaType, context) { @@ -224,7 +177,7 @@ export function checkVideoBidSetupHook(fn, bid, bidRequest, videoMediaType, cont return true; } else { - return fn.call(this, bid, bidRequest, videoMediaType, context); + fn.call(this, bid, bidRequest, videoMediaType, context); } } diff --git a/test/spec/modules/adpod_spec.js b/test/spec/modules/adpod_spec.js index bf162026a9f..7ae201ad26c 100644 --- a/test/spec/modules/adpod_spec.js +++ b/test/spec/modules/adpod_spec.js @@ -1,645 +1,609 @@ -// import * as utils from 'src/utils'; -// import { config } from 'src/config'; -// import * as videoCache from 'src/videoCache'; -// import * as auction from 'src/auction'; - -// import { callPrebidCacheHook, checkAdUnitSetupHook, checkVideoBidSetupHook, adpodSetConfig, ADPOD } from 'modules/adpod'; - -// let expect = require('chai').expect; - -// describe('adpod.js', function () { -// let logErrorStub; -// let logWarnStub; - -// describe('callPrebidCacheHook', function () { -// let callbackResult; -// let clock; -// let addBidToAuctionStub; -// let doCallbacksIfTimedoutStub; -// let storeStub; -// let afterBidAddedSpy; -// let auctionBids = []; - -// let callbackFn = function() { -// callbackResult = true; -// } - -// const fakeStoreFn = function(bids, callback) { -// let payload = []; -// bids.forEach(bid => payload.push({uuid: bid.customCacheKey})); -// callback(null, payload); -// } - -// beforeEach(function() { -// callbackResult = null; -// afterBidAddedSpy = sinon.spy(); -// storeStub = sinon.stub(videoCache, 'store'); -// logWarnStub = sinon.stub(utils, 'logWarn'); -// addBidToAuctionStub = sinon.stub(auction, 'addBidToAuction').callsFake(function (auctionInstance, bid) { -// auctionBids.push(bid); -// }); -// doCallbacksIfTimedoutStub = sinon.stub(auction, 'doCallbacksIfTimedout'); -// clock = sinon.useFakeTimers(); -// config.setConfig({ -// cache: { -// url: 'https://prebid.adnxs.com/pbc/v1/cache' -// } -// }); -// }); - -// afterEach(function() { -// storeStub.restore(); -// logWarnStub.restore(); -// addBidToAuctionStub.restore(); -// doCallbacksIfTimedoutStub.restore(); -// clock.restore(); -// config.resetConfig(); -// auctionBids = []; -// }) - -// it('should redirect back to the original function if bid is not an adpod video', function () { -// let bid = { -// adId: 'testAdId_123', -// mediaType: 'video' -// }; - -// let bidderRequest = { -// adUnitCode: 'adUnit_123', -// mediaTypes: { -// video: { -// context: 'outstream' -// } -// } -// } - -// callPrebidCacheHook(this, bid, function () {}, bidderRequest, callbackFn); -// expect(callbackResult).to.equal(true); -// }); - -// it('should reject a bid if the duration is too high', function () { -// let bidResponse = { -// adId: 'badAd890', -// auctionId: 'bad123', -// mediaType: 'video', -// cpm: 9, -// meta: { -// primaryCatId: 'food' -// }, -// video: { -// context: ADPOD, -// durationSeconds: 600 -// } -// }; - -// let bidderRequest = { -// adUnitCode: 'adpod_0', -// auctionId: 'bad123', -// mediaTypes: { -// video: { -// context: ADPOD, -// playerSize: [300, 300], -// adPodDurationSec: 600, -// durationRangeSec: [15, 30, 60], -// requireExactDuration: true -// } -// } -// }; - -// callPrebidCacheHook(this, bidResponse, afterBidAddedSpy, bidderRequest, callbackFn); - -// expect(callbackResult).to.be.null; -// expect(auctionBids.length).to.equal(0); -// expect(logWarnStub.calledOnce).to.equal(true); -// expect(storeStub.called).to.equal(false); -// expect(afterBidAddedSpy.called).to.equal(true); -// }); - -// it('should send prebid cache call once bid queue is full', function () { -// storeStub.callsFake(fakeStoreFn); - -// config.setConfig({ -// adpod: { -// bidQueueSizeLimit: 2 -// } -// }); - -// let bidResponse1 = { -// adId: 'adId123', -// auctionId: 'full_abc123', -// mediaType: 'video', -// cpm: 10, -// meta: { -// primaryCatId: 'airline' -// }, -// video: { -// context: ADPOD, -// durationSeconds: 30 -// } -// }; -// let bidResponse2 = { -// adId: 'adId234', -// auctionId: 'full_abc123', -// mediaType: 'video', -// cpm: 15, -// meta: { -// primaryCatId: 'airline' -// }, -// video: { -// context: ADPOD, -// durationSeconds: 30 -// } -// }; -// let bidderRequest = { -// adUnitCode: 'adpod_1', -// auctionId: 'full_abc123', -// mediaTypes: { -// video: { -// context: ADPOD, -// playerSize: [300, 300], -// adPodDurationSec: 120, -// durationRangeSec: [15, 30], -// requireExactDuration: false -// } -// } -// }; - -// callPrebidCacheHook(this, bidResponse1, afterBidAddedSpy, bidderRequest, callbackFn); -// callPrebidCacheHook(this, bidResponse2, afterBidAddedSpy, bidderRequest, callbackFn); - -// expect(callbackResult).to.be.null; -// expect(afterBidAddedSpy.calledTwice).to.equal(true); -// expect(auctionBids.length).to.equal(2); -// expect(auctionBids[0].adId).to.equal('adId123'); -// expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^10\.00_airline_30s_.*/); -// expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('10.00_airline_30s'); -// expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; -// expect(auctionBids[1].adId).to.equal('adId234'); -// expect(auctionBids[1].customCacheKey).to.exist.and.to.match(/^15\.00_airline_30s_.*/); -// expect(auctionBids[1].adserverTargeting.hb_price_industry_duration).to.equal('15.00_airline_30s'); -// expect(auctionBids[1].adserverTargeting.hb_uuid).to.exist; -// }); - -// it('should send prebid cache call after set period of time (even if queue is not full)', function () { -// storeStub.callsFake(fakeStoreFn); - -// config.setConfig({ -// adpod: { -// bidQueueSizeLimit: 2, -// bidQueueTimeDelay: 30 -// } -// }); - -// let bidResponse = { -// adId: 'adId234', -// auctionId: 'timer_abc234', -// mediaType: 'video', -// cpm: 15, -// meta: { -// primaryCatId: 'airline' -// }, -// video: { -// context: ADPOD, -// durationSeconds: 25 -// } -// }; -// let bidderRequest = { -// adUnitCode: 'adpod_2', -// auctionId: 'timer_abc234', -// mediaTypes: { -// video: { -// context: ADPOD, -// playerSize: [300, 300], -// adPodDurationSec: 120, -// durationRangeSec: [15, 30], -// requireExactDuration: true -// } -// } -// }; - -// callPrebidCacheHook(this, bidResponse, afterBidAddedSpy, bidderRequest, callbackFn); -// clock.tick(31); - -// expect(callbackResult).to.be.null; -// expect(afterBidAddedSpy.calledOnce).to.equal(true); -// expect(auctionBids.length).to.equal(1); -// expect(auctionBids[0].adId).to.equal('adId234'); -// expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^15\.00_airline_30s_.*/); -// expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('15.00_airline_30s'); -// expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; -// }); - -// it('should execute multiple prebid cache calls when number of bids exceeds queue size', function () { -// storeStub.callsFake(fakeStoreFn); - -// config.setConfig({ -// adpod: { -// bidQueueSizeLimit: 2, -// bidQueueTimeDelay: 30 -// } -// }); - -// let bidResponse1 = { -// adId: 'multi_ad1', -// auctionId: 'multi_call_abc345', -// mediaType: 'video', -// cpm: 15, -// meta: { -// primaryCatId: 'airline' -// }, -// video: { -// context: ADPOD, -// durationSeconds: 15 -// } -// }; -// let bidResponse2 = { -// adId: 'multi_ad2', -// auctionId: 'multi_call_abc345', -// mediaType: 'video', -// cpm: 15, -// meta: { -// primaryCatId: 'news' -// }, -// video: { -// context: ADPOD, -// durationSeconds: 15 -// } -// }; -// let bidResponse3 = { -// adId: 'multi_ad3', -// auctionId: 'multi_call_abc345', -// mediaType: 'video', -// cpm: 10, -// meta: { -// primaryCatId: 'sports' -// }, -// video: { -// context: ADPOD, -// durationSeconds: 15 -// } -// }; - -// let bidderRequest = { -// adUnitCode: 'adpod_3', -// auctionId: 'multi_call_abc345', -// mediaTypes: { -// video: { -// context: ADPOD, -// playerSize: [300, 300], -// adPodDurationSec: 45, -// durationRangeSec: [15, 30], -// requireExactDuration: false -// } -// } -// }; - -// callPrebidCacheHook(this, bidResponse1, afterBidAddedSpy, bidderRequest, callbackFn); -// callPrebidCacheHook(this, bidResponse2, afterBidAddedSpy, bidderRequest, callbackFn); -// callPrebidCacheHook(this, bidResponse3, afterBidAddedSpy, bidderRequest, callbackFn); -// clock.next(); - -// expect(callbackResult).to.be.null; -// expect(afterBidAddedSpy.calledThrice).to.equal(true); -// expect(storeStub.calledTwice).to.equal(true); -// expect(auctionBids.length).to.equal(3); -// expect(auctionBids[0].adId).to.equal('multi_ad1'); -// expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^15\.00_airline_15s_.*/); -// expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('15.00_airline_15s'); -// expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; -// expect(auctionBids[1].adId).to.equal('multi_ad2'); -// expect(auctionBids[1].customCacheKey).to.exist.and.to.match(/^15\.00_news_15s_.*/); -// expect(auctionBids[1].adserverTargeting.hb_price_industry_duration).to.equal('15.00_news_15s'); -// expect(auctionBids[1].adserverTargeting.hb_uuid).to.exist.and.to.equal(auctionBids[0].adserverTargeting.hb_uuid); -// expect(auctionBids[2].adId).to.equal('multi_ad3'); -// expect(auctionBids[2].customCacheKey).to.exist.and.to.match(/^10\.00_sports_15s_.*/); -// expect(auctionBids[2].adserverTargeting.hb_price_industry_duration).to.equal('10.00_sports_15s'); -// expect(auctionBids[2].adserverTargeting.hb_uuid).to.exist.and.to.equal(auctionBids[0].adserverTargeting.hb_uuid); -// }); - -// it('should not add bid to auction when Prebid Cache detects an existing key', function () { -// storeStub.callsFake(function(bids, callback) { -// let payload = []; -// bids.forEach(bid => payload.push({uuid: bid.customCacheKey})); - -// // fake a duplicate bid response from PBC (sets an empty string for the uuid) -// payload[1].uuid = ''; -// callback(null, payload); -// }); - -// config.setConfig({ -// adpod: { -// bidQueueSizeLimit: 2 -// } -// }); - -// let bidResponse1 = { -// adId: 'dup_ad_1', -// auctionId: 'duplicate_def123', -// mediaType: 'video', -// cpm: 5, -// meta: { -// primaryCatId: 'tech' -// }, -// video: { -// context: ADPOD, -// durationSeconds: 45 -// } -// }; -// let bidResponse2 = { -// adId: 'dup_ad_2', -// auctionId: 'duplicate_def123', -// mediaType: 'video', -// cpm: 5, -// meta: { -// primaryCatId: 'tech' -// }, -// video: { -// context: ADPOD, -// durationSeconds: 45 -// } -// }; -// let bidderRequest = { -// adUnitCode: 'adpod_4', -// auctionId: 'duplicate_def123', -// mediaTypes: { -// video: { -// context: ADPOD, -// playerSize: [300, 300], -// adPodDurationSec: 120, -// durationRangeSec: [15, 30, 45], -// requireExactDuration: false -// } -// } -// }; - -// callPrebidCacheHook(this, bidResponse1, afterBidAddedSpy, bidderRequest, callbackFn); -// callPrebidCacheHook(this, bidResponse2, afterBidAddedSpy, bidderRequest, callbackFn); - -// expect(callbackResult).to.be.null; -// expect(afterBidAddedSpy.calledOnce).to.equal(true); -// expect(storeStub.calledOnce).to.equal(true); -// expect(auctionBids.length).to.equal(1); -// expect(auctionBids[0].adId).to.equal('dup_ad_1'); -// expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^5\.00_tech_45s_.*/); -// expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('5.00_tech_45s'); -// expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; -// }); - -// it('should not add bids to auction if PBC returns an error', function() { -// storeStub.callsFake(function(bids, callback) { -// let payload = []; -// let errmsg = 'invalid json'; - -// callback(errmsg, payload); -// }); - -// config.setConfig({ -// adpod: { -// bidQueueSizeLimit: 2 -// } -// }); - -// let bidResponse1 = { -// adId: 'err_ad_1', -// auctionId: 'error_xyz123', -// mediaType: 'video', -// cpm: 5, -// meta: { -// primaryCatId: 'tech' -// }, -// video: { -// context: ADPOD, -// durationSeconds: 30 -// } -// }; -// let bidResponse2 = { -// adId: 'err_ad_2', -// auctionId: 'error_xyz123', -// mediaType: 'video', -// cpm: 5, -// meta: { -// primaryCatId: 'tech' -// }, -// video: { -// context: ADPOD, -// durationSeconds: 30 -// } -// }; -// let bidderRequest = { -// adUnitCode: 'adpod_5', -// auctionId: 'error_xyz123', -// mediaTypes: { -// video: { -// context: ADPOD, -// playerSize: [300, 300], -// adPodDurationSec: 120, -// durationRangeSec: [15, 30, 45], -// requireExactDuration: false -// } -// } -// }; - -// callPrebidCacheHook(this, bidResponse1, afterBidAddedSpy, bidderRequest, callbackFn); -// callPrebidCacheHook(this, bidResponse2, afterBidAddedSpy, bidderRequest, callbackFn); - -// expect(doCallbacksIfTimedoutStub.calledTwice).to.equal(true); -// expect(logWarnStub.calledOnce).to.equal(true); -// expect(auctionBids.length).to.equal(0); -// }); -// }); - -// describe('checkAdUnitSetupHook', function () { -// beforeEach(function () { -// logWarnStub = sinon.stub(utils, 'logWarn'); -// }); - -// afterEach(function() { -// utils.logWarn.restore(); -// }); - -// it('removes an incorrectly setup adpod adunit', function() { -// let results; -// let adUnits = [{ -// code: 'test1', -// mediaTypes: { -// video: { -// context: ADPOD -// } -// } -// }, { -// code: 'test2', -// mediaTypes: { -// video: { -// context: 'instream' -// } -// } -// }]; - -// checkAdUnitSetupHook(adUnits, function (adUnits) { -// results = adUnits; -// }); - -// expect(results).to.deep.equal([{ -// code: 'test2', -// mediaTypes: { -// video: { -// context: 'instream' -// } -// } -// }]); -// expect(logWarnStub.calledOnce).to.equal(true); -// }); - -// it('accepts mixed set of adunits', function() { -// let results; -// let adUnits = [{ -// code: 'test3', -// mediaTypes: { -// video: { -// context: ADPOD, -// playerSize: [300, 300], -// adPodDurationSec: 360, -// durationRangeSec: [15, 30, 45] -// } -// } -// }, { -// code: 'test4', -// mediaTypes: { -// banner: { -// sizes: [[300, 250]] -// } -// } -// }]; - -// checkAdUnitSetupHook(adUnits, function (adUnits) { -// results = adUnits; -// }); - -// expect(results).to.deep.equal(adUnits); -// expect(logWarnStub.called).to.equal(false); -// }); -// }); - -// describe('checkVideoBidSetupHook', function () { -// let callbackResult; -// const adpodTestBid = { -// video: { -// context: ADPOD, -// durationSeconds: 300 -// }, -// meta: { -// primaryCatId: 'testCategory_123' -// }, -// vastXml: 'test XML here' -// }; - -// beforeEach(function() { -// callbackResult = null; -// logErrorStub = sinon.stub(utils, 'logError'); -// }); - -// afterEach(function() { -// config.resetConfig(); -// utils.logError.restore(); -// }) - -// it('redirects to original function for non-adpod type video bids', function() { -// let bannerTestBid = { -// mediaType: 'video' -// }; -// let hookReturnValue = checkVideoBidSetupHook(bannerTestBid, {}, {}, 'instream', function (bid) { -// callbackResult = bid; -// }); -// expect(callbackResult).to.deep.equal(bannerTestBid); -// expect(hookReturnValue).to.be.undefined; -// expect(logErrorStub.called).to.equal(false); -// }); - -// it('returns true when adpod bid is properly setup', function() { -// config.setConfig({ -// cache: { -// url: 'http://test.cache.url/endpoint' -// } -// }); - -// let goodBid = utils.deepClone(adpodTestBid); -// let hookReturnValue = checkVideoBidSetupHook(goodBid, {}, {}, ADPOD, function (bid) { -// callbackResult = bid; -// }); -// expect(callbackResult).to.be.null; -// expect(hookReturnValue).to.equal(true); -// expect(logErrorStub.called).to.equal(false); -// }); - -// it('returns false when a required property from an adpod bid is missing', function() { -// function testInvalidAdpodBid(badTestBid, shouldErrorBeLogged) { -// let hookReturnValue = checkVideoBidSetupHook(badTestBid, {}, {}, ADPOD, function(bid) { -// callbackResult = bid; -// }); -// expect(callbackResult).to.be.null; -// expect(hookReturnValue).to.equal(false); -// expect(logErrorStub.called).to.equal(shouldErrorBeLogged); -// } - -// let noCatBid = utils.deepClone(adpodTestBid); -// delete noCatBid.meta; -// testInvalidAdpodBid(noCatBid, false); - -// let noContextBid = utils.deepClone(adpodTestBid); -// delete noContextBid.video.context; -// testInvalidAdpodBid(noContextBid, false); - -// let wrongContextBid = utils.deepClone(adpodTestBid); -// wrongContextBid.video.context = 'instream'; -// testInvalidAdpodBid(wrongContextBid, false); - -// let noDurationBid = utils.deepClone(adpodTestBid); -// delete noDurationBid.video.durationSeconds; -// testInvalidAdpodBid(noDurationBid, false); - -// let noCacheUrlBid = utils.deepClone(adpodTestBid); -// testInvalidAdpodBid(noCacheUrlBid, true); -// }); -// }); - -// describe('adpodSetConfig', function () { -// let logWarnStub; -// beforeEach(function() { -// logWarnStub = sinon.stub(utils, 'logWarn'); -// }); - -// afterEach(function () { -// logWarnStub.restore(); -// }); - -// it('should log a warning when values other than numbers are used in setConfig', function() { -// adpodSetConfig({ -// bidQueueSizeLimit: '2', -// bidQueueTimeDelay: '50' -// }); -// expect(logWarnStub.calledTwice).to.equal(true); -// }); - -// it('should log a warning when numbers less than or equal to zero are used in setConfig', function() { -// adpodSetConfig({ -// bidQueueSizeLimit: 0, -// bidQueueTimeDelay: -2 -// }); -// expect(logWarnStub.calledTwice).to.equal(true); -// }); - -// // TODO add unit test if there should be a ceiling for acceptable values - -// it('should not log any warning when using a valid config', function() { -// adpodSetConfig({ -// bidQueueSizeLimit: 10 -// }); -// expect(logWarnStub.called).to.equal(false); - -// adpodSetConfig({ -// bidQueueTimeDelay: 100, -// bidQueueSizeLimit: 20 -// }); -// expect(logWarnStub.called).to.equal(false); -// }) -// }); -// }); +import * as utils from 'src/utils'; +import { config } from 'src/config'; +import * as videoCache from 'src/videoCache'; +import * as auction from 'src/auction'; + +import { callPrebidCacheHook, checkAdUnitSetupHook, checkVideoBidSetupHook, adpodSetConfig, ADPOD } from 'modules/adpod'; + +let expect = require('chai').expect; + +// TODO - need to update the bidResponses to include the targting keys/UUID; update store stubs? + +describe('adpod.js', function () { + let logErrorStub; + let logWarnStub; + + describe('callPrebidCacheHook', function () { + let callbackResult; + let clock; + let addBidToAuctionStub; + let doCallbacksIfTimedoutStub; + let storeStub; + let afterBidAddedSpy; + let auctionBids = []; + + let callbackFn = function() { + callbackResult = true; + } + + const fakeStoreFn = function(bids, callback) { + let payload = []; + bids.forEach(bid => payload.push({uuid: bid.customCacheKey})); + callback(null, payload); + } + + beforeEach(function() { + callbackResult = null; + afterBidAddedSpy = sinon.spy(); + storeStub = sinon.stub(videoCache, 'store'); + logWarnStub = sinon.stub(utils, 'logWarn'); + addBidToAuctionStub = sinon.stub(auction, 'addBidToAuction').callsFake(function (auctionInstance, bid) { + auctionBids.push(bid); + }); + doCallbacksIfTimedoutStub = sinon.stub(auction, 'doCallbacksIfTimedout'); + clock = sinon.useFakeTimers(); + config.setConfig({ + cache: { + url: 'https://prebid.adnxs.com/pbc/v1/cache' + } + }); + }); + + afterEach(function() { + storeStub.restore(); + logWarnStub.restore(); + addBidToAuctionStub.restore(); + doCallbacksIfTimedoutStub.restore(); + clock.restore(); + config.resetConfig(); + auctionBids = []; + }) + + it('should redirect back to the original function if bid is not an adpod video', function () { + let bid = { + adId: 'testAdId_123', + mediaType: 'video' + }; + + let bidderRequest = { + adUnitCode: 'adUnit_123', + mediaTypes: { + video: { + context: 'outstream' + } + } + } + + callPrebidCacheHook(this, bid, function () {}, bidderRequest, callbackFn); + expect(callbackResult).to.equal(true); + }); + + it('should send prebid cache call once bid queue is full', function () { + storeStub.callsFake(fakeStoreFn); + + config.setConfig({ + adpod: { + bidQueueSizeLimit: 2 + } + }); + + let bidResponse1 = { + adId: 'adId123', + auctionId: 'full_abc123', + mediaType: 'video', + cpm: 10, + meta: { + primaryCatId: 'airline' + }, + video: { + context: ADPOD, + durationSeconds: 30 + } + }; + let bidResponse2 = { + adId: 'adId234', + auctionId: 'full_abc123', + mediaType: 'video', + cpm: 15, + meta: { + primaryCatId: 'airline' + }, + video: { + context: ADPOD, + durationSeconds: 30 + } + }; + let bidderRequest = { + adUnitCode: 'adpod_1', + auctionId: 'full_abc123', + mediaTypes: { + video: { + context: ADPOD, + playerSize: [300, 300], + adPodDurationSec: 120, + durationRangeSec: [15, 30], + requireExactDuration: false + } + } + }; + + callPrebidCacheHook(this, bidResponse1, afterBidAddedSpy, bidderRequest, callbackFn); + callPrebidCacheHook(this, bidResponse2, afterBidAddedSpy, bidderRequest, callbackFn); + + expect(callbackResult).to.be.null; + expect(afterBidAddedSpy.calledTwice).to.equal(true); + expect(auctionBids.length).to.equal(2); + expect(auctionBids[0].adId).to.equal('adId123'); + expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^10\.00_airline_30s_.*/); + expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('10.00_airline_30s'); + expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; + expect(auctionBids[1].adId).to.equal('adId234'); + expect(auctionBids[1].customCacheKey).to.exist.and.to.match(/^15\.00_airline_30s_.*/); + expect(auctionBids[1].adserverTargeting.hb_price_industry_duration).to.equal('15.00_airline_30s'); + expect(auctionBids[1].adserverTargeting.hb_uuid).to.exist; + }); + + it('should send prebid cache call after set period of time (even if queue is not full)', function () { + storeStub.callsFake(fakeStoreFn); + + config.setConfig({ + adpod: { + bidQueueSizeLimit: 2, + bidQueueTimeDelay: 30 + } + }); + + let bidResponse = { + adId: 'adId234', + auctionId: 'timer_abc234', + mediaType: 'video', + cpm: 15, + meta: { + primaryCatId: 'airline' + }, + video: { + context: ADPOD, + durationSeconds: 30 + } + }; + let bidderRequest = { + adUnitCode: 'adpod_2', + auctionId: 'timer_abc234', + mediaTypes: { + video: { + context: ADPOD, + playerSize: [300, 300], + adPodDurationSec: 120, + durationRangeSec: [15, 30], + requireExactDuration: true + } + } + }; + + callPrebidCacheHook(this, bidResponse, afterBidAddedSpy, bidderRequest, callbackFn); + clock.tick(31); + + expect(callbackResult).to.be.null; + expect(afterBidAddedSpy.calledOnce).to.equal(true); + expect(auctionBids.length).to.equal(1); + expect(auctionBids[0].adId).to.equal('adId234'); + expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^15\.00_airline_30s_.*/); + expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('15.00_airline_30s'); + expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; + }); + + it('should execute multiple prebid cache calls when number of bids exceeds queue size', function () { + storeStub.callsFake(fakeStoreFn); + + config.setConfig({ + adpod: { + bidQueueSizeLimit: 2, + bidQueueTimeDelay: 30 + } + }); + + let bidResponse1 = { + adId: 'multi_ad1', + auctionId: 'multi_call_abc345', + mediaType: 'video', + cpm: 15, + meta: { + primaryCatId: 'airline' + }, + video: { + context: ADPOD, + durationSeconds: 15 + } + }; + let bidResponse2 = { + adId: 'multi_ad2', + auctionId: 'multi_call_abc345', + mediaType: 'video', + cpm: 15, + meta: { + primaryCatId: 'news' + }, + video: { + context: ADPOD, + durationSeconds: 15 + } + }; + let bidResponse3 = { + adId: 'multi_ad3', + auctionId: 'multi_call_abc345', + mediaType: 'video', + cpm: 10, + meta: { + primaryCatId: 'sports' + }, + video: { + context: ADPOD, + durationSeconds: 15 + } + }; + + let bidderRequest = { + adUnitCode: 'adpod_3', + auctionId: 'multi_call_abc345', + mediaTypes: { + video: { + context: ADPOD, + playerSize: [300, 300], + adPodDurationSec: 45, + durationRangeSec: [15, 30], + requireExactDuration: false + } + } + }; + + callPrebidCacheHook(this, bidResponse1, afterBidAddedSpy, bidderRequest, callbackFn); + callPrebidCacheHook(this, bidResponse2, afterBidAddedSpy, bidderRequest, callbackFn); + callPrebidCacheHook(this, bidResponse3, afterBidAddedSpy, bidderRequest, callbackFn); + clock.next(); + + expect(callbackResult).to.be.null; + expect(afterBidAddedSpy.calledThrice).to.equal(true); + expect(storeStub.calledTwice).to.equal(true); + expect(auctionBids.length).to.equal(3); + expect(auctionBids[0].adId).to.equal('multi_ad1'); + expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^15\.00_airline_15s_.*/); + expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('15.00_airline_15s'); + expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; + expect(auctionBids[1].adId).to.equal('multi_ad2'); + expect(auctionBids[1].customCacheKey).to.exist.and.to.match(/^15\.00_news_15s_.*/); + expect(auctionBids[1].adserverTargeting.hb_price_industry_duration).to.equal('15.00_news_15s'); + expect(auctionBids[1].adserverTargeting.hb_uuid).to.exist.and.to.equal(auctionBids[0].adserverTargeting.hb_uuid); + expect(auctionBids[2].adId).to.equal('multi_ad3'); + expect(auctionBids[2].customCacheKey).to.exist.and.to.match(/^10\.00_sports_15s_.*/); + expect(auctionBids[2].adserverTargeting.hb_price_industry_duration).to.equal('10.00_sports_15s'); + expect(auctionBids[2].adserverTargeting.hb_uuid).to.exist.and.to.equal(auctionBids[0].adserverTargeting.hb_uuid); + }); + + it('should not add bid to auction when Prebid Cache detects an existing key', function () { + storeStub.callsFake(function(bids, callback) { + let payload = []; + bids.forEach(bid => payload.push({uuid: bid.customCacheKey})); + + // fake a duplicate bid response from PBC (sets an empty string for the uuid) + payload[1].uuid = ''; + callback(null, payload); + }); + + config.setConfig({ + adpod: { + bidQueueSizeLimit: 2 + } + }); + + let bidResponse1 = { + adId: 'dup_ad_1', + auctionId: 'duplicate_def123', + mediaType: 'video', + cpm: 5, + meta: { + primaryCatId: 'tech' + }, + video: { + context: ADPOD, + durationSeconds: 45 + } + }; + let bidResponse2 = { + adId: 'dup_ad_2', + auctionId: 'duplicate_def123', + mediaType: 'video', + cpm: 5, + meta: { + primaryCatId: 'tech' + }, + video: { + context: ADPOD, + durationSeconds: 45 + } + }; + let bidderRequest = { + adUnitCode: 'adpod_4', + auctionId: 'duplicate_def123', + mediaTypes: { + video: { + context: ADPOD, + playerSize: [300, 300], + adPodDurationSec: 120, + durationRangeSec: [15, 30, 45], + requireExactDuration: false + } + } + }; + + callPrebidCacheHook(this, bidResponse1, afterBidAddedSpy, bidderRequest, callbackFn); + callPrebidCacheHook(this, bidResponse2, afterBidAddedSpy, bidderRequest, callbackFn); + + expect(callbackResult).to.be.null; + expect(afterBidAddedSpy.calledOnce).to.equal(true); + expect(storeStub.calledOnce).to.equal(true); + expect(auctionBids.length).to.equal(1); + expect(auctionBids[0].adId).to.equal('dup_ad_1'); + expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^5\.00_tech_45s_.*/); + expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('5.00_tech_45s'); + expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; + }); + + it('should not add bids to auction if PBC returns an error', function() { + storeStub.callsFake(function(bids, callback) { + let payload = []; + let errmsg = 'invalid json'; + + callback(errmsg, payload); + }); + + config.setConfig({ + adpod: { + bidQueueSizeLimit: 2 + } + }); + + let bidResponse1 = { + adId: 'err_ad_1', + auctionId: 'error_xyz123', + mediaType: 'video', + cpm: 5, + meta: { + primaryCatId: 'tech' + }, + video: { + context: ADPOD, + durationSeconds: 30 + } + }; + let bidResponse2 = { + adId: 'err_ad_2', + auctionId: 'error_xyz123', + mediaType: 'video', + cpm: 5, + meta: { + primaryCatId: 'tech' + }, + video: { + context: ADPOD, + durationSeconds: 30 + } + }; + let bidderRequest = { + adUnitCode: 'adpod_5', + auctionId: 'error_xyz123', + mediaTypes: { + video: { + context: ADPOD, + playerSize: [300, 300], + adPodDurationSec: 120, + durationRangeSec: [15, 30, 45], + requireExactDuration: false + } + } + }; + + callPrebidCacheHook(this, bidResponse1, afterBidAddedSpy, bidderRequest, callbackFn); + callPrebidCacheHook(this, bidResponse2, afterBidAddedSpy, bidderRequest, callbackFn); + + expect(doCallbacksIfTimedoutStub.calledTwice).to.equal(true); + expect(logWarnStub.calledOnce).to.equal(true); + expect(auctionBids.length).to.equal(0); + }); + }); + + describe('checkAdUnitSetupHook', function () { + beforeEach(function () { + logWarnStub = sinon.stub(utils, 'logWarn'); + }); + + afterEach(function() { + utils.logWarn.restore(); + }); + + it('removes an incorrectly setup adpod adunit', function() { + let results; + let adUnits = [{ + code: 'test1', + mediaTypes: { + video: { + context: ADPOD + } + } + }, { + code: 'test2', + mediaTypes: { + video: { + context: 'instream' + } + } + }]; + + checkAdUnitSetupHook(adUnits, function (adUnits) { + results = adUnits; + }); + + expect(results).to.deep.equal([{ + code: 'test2', + mediaTypes: { + video: { + context: 'instream' + } + } + }]); + expect(logWarnStub.calledOnce).to.equal(true); + }); + + it('accepts mixed set of adunits', function() { + let results; + let adUnits = [{ + code: 'test3', + mediaTypes: { + video: { + context: ADPOD, + playerSize: [300, 300], + adPodDurationSec: 360, + durationRangeSec: [15, 30, 45] + } + } + }, { + code: 'test4', + mediaTypes: { + banner: { + sizes: [[300, 250]] + } + } + }]; + + checkAdUnitSetupHook(adUnits, function (adUnits) { + results = adUnits; + }); + + expect(results).to.deep.equal(adUnits); + expect(logWarnStub.called).to.equal(false); + }); + }); + + describe('checkVideoBidSetupHook', function () { + let callbackResult; + const adpodTestBid = { + video: { + context: ADPOD, + durationSeconds: 300 + }, + meta: { + primaryCatId: 'testCategory_123' + }, + vastXml: 'test XML here' + }; + + beforeEach(function() { + callbackResult = null; + logErrorStub = sinon.stub(utils, 'logError'); + }); + + afterEach(function() { + config.resetConfig(); + utils.logError.restore(); + }) + + it('redirects to original function for non-adpod type video bids', function() { + let bannerTestBid = { + mediaType: 'video' + }; + let hookReturnValue = checkVideoBidSetupHook(bannerTestBid, {}, {}, 'instream', function (bid) { + callbackResult = bid; + }); + expect(callbackResult).to.deep.equal(bannerTestBid); + expect(hookReturnValue).to.be.undefined; + expect(logErrorStub.called).to.equal(false); + }); + + it('returns true when adpod bid is properly setup', function() { + config.setConfig({ + cache: { + url: 'http://test.cache.url/endpoint' + } + }); + + let goodBid = utils.deepClone(adpodTestBid); + let hookReturnValue = checkVideoBidSetupHook(goodBid, {}, {}, ADPOD, function (bid) { + callbackResult = bid; + }); + expect(callbackResult).to.be.null; + expect(hookReturnValue).to.equal(true); + expect(logErrorStub.called).to.equal(false); + }); + + it('returns false when a required property from an adpod bid is missing', function() { + function testInvalidAdpodBid(badTestBid, shouldErrorBeLogged) { + let hookReturnValue = checkVideoBidSetupHook(badTestBid, {}, {}, ADPOD, function(bid) { + callbackResult = bid; + }); + expect(callbackResult).to.be.null; + expect(hookReturnValue).to.equal(false); + expect(logErrorStub.called).to.equal(shouldErrorBeLogged); + } + + let noCatBid = utils.deepClone(adpodTestBid); + delete noCatBid.meta; + testInvalidAdpodBid(noCatBid, false); + + let noContextBid = utils.deepClone(adpodTestBid); + delete noContextBid.video.context; + testInvalidAdpodBid(noContextBid, false); + + let wrongContextBid = utils.deepClone(adpodTestBid); + wrongContextBid.video.context = 'instream'; + testInvalidAdpodBid(wrongContextBid, false); + + let noDurationBid = utils.deepClone(adpodTestBid); + delete noDurationBid.video.durationSeconds; + testInvalidAdpodBid(noDurationBid, false); + + let noCacheUrlBid = utils.deepClone(adpodTestBid); + testInvalidAdpodBid(noCacheUrlBid, true); + }); + }); + + describe('adpodSetConfig', function () { + let logWarnStub; + beforeEach(function() { + logWarnStub = sinon.stub(utils, 'logWarn'); + }); + + afterEach(function () { + logWarnStub.restore(); + }); + + it('should log a warning when values other than numbers are used in setConfig', function() { + adpodSetConfig({ + bidQueueSizeLimit: '2', + bidQueueTimeDelay: '50' + }); + expect(logWarnStub.calledTwice).to.equal(true); + }); + + it('should log a warning when numbers less than or equal to zero are used in setConfig', function() { + adpodSetConfig({ + bidQueueSizeLimit: 0, + bidQueueTimeDelay: -2 + }); + expect(logWarnStub.calledTwice).to.equal(true); + }); + + // TODO add unit test if there should be a ceiling for acceptable values + + it('should not log any warning when using a valid config', function() { + adpodSetConfig({ + bidQueueSizeLimit: 10 + }); + expect(logWarnStub.called).to.equal(false); + + adpodSetConfig({ + bidQueueTimeDelay: 100, + bidQueueSizeLimit: 20 + }); + expect(logWarnStub.called).to.equal(false); + }) + }); +}); From 99fdaae1d4e3a9344890ca30527ef3141563d4df Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Thu, 31 Jan 2019 10:56:28 -0500 Subject: [PATCH 03/21] correct issues in unit tests --- test/spec/modules/adpod_spec.js | 53 ++++++++++++++++----------------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/test/spec/modules/adpod_spec.js b/test/spec/modules/adpod_spec.js index 7ae201ad26c..35c114d4270 100644 --- a/test/spec/modules/adpod_spec.js +++ b/test/spec/modules/adpod_spec.js @@ -74,7 +74,7 @@ describe('adpod.js', function () { } } - callPrebidCacheHook(this, bid, function () {}, bidderRequest, callbackFn); + callPrebidCacheHook(callbackFn, this, bid, function () {}, bidderRequest); expect(callbackResult).to.equal(true); }); @@ -127,8 +127,8 @@ describe('adpod.js', function () { } }; - callPrebidCacheHook(this, bidResponse1, afterBidAddedSpy, bidderRequest, callbackFn); - callPrebidCacheHook(this, bidResponse2, afterBidAddedSpy, bidderRequest, callbackFn); + callPrebidCacheHook(callbackFn, this, bidResponse1, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, this, bidResponse2, afterBidAddedSpy, bidderRequest); expect(callbackResult).to.be.null; expect(afterBidAddedSpy.calledTwice).to.equal(true); @@ -180,7 +180,7 @@ describe('adpod.js', function () { } }; - callPrebidCacheHook(this, bidResponse, afterBidAddedSpy, bidderRequest, callbackFn); + callPrebidCacheHook(callbackFn, this, bidResponse, afterBidAddedSpy, bidderRequest); clock.tick(31); expect(callbackResult).to.be.null; @@ -256,9 +256,9 @@ describe('adpod.js', function () { } }; - callPrebidCacheHook(this, bidResponse1, afterBidAddedSpy, bidderRequest, callbackFn); - callPrebidCacheHook(this, bidResponse2, afterBidAddedSpy, bidderRequest, callbackFn); - callPrebidCacheHook(this, bidResponse3, afterBidAddedSpy, bidderRequest, callbackFn); + callPrebidCacheHook(callbackFn, this, bidResponse1, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, this, bidResponse2, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, this, bidResponse3, afterBidAddedSpy, bidderRequest); clock.next(); expect(callbackResult).to.be.null; @@ -335,8 +335,8 @@ describe('adpod.js', function () { } }; - callPrebidCacheHook(this, bidResponse1, afterBidAddedSpy, bidderRequest, callbackFn); - callPrebidCacheHook(this, bidResponse2, afterBidAddedSpy, bidderRequest, callbackFn); + callPrebidCacheHook(callbackFn, this, bidResponse1, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, this, bidResponse2, afterBidAddedSpy, bidderRequest); expect(callbackResult).to.be.null; expect(afterBidAddedSpy.calledOnce).to.equal(true); @@ -402,8 +402,8 @@ describe('adpod.js', function () { } }; - callPrebidCacheHook(this, bidResponse1, afterBidAddedSpy, bidderRequest, callbackFn); - callPrebidCacheHook(this, bidResponse2, afterBidAddedSpy, bidderRequest, callbackFn); + callPrebidCacheHook(callbackFn, this, bidResponse1, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, this, bidResponse2, afterBidAddedSpy, bidderRequest); expect(doCallbacksIfTimedoutStub.calledTwice).to.equal(true); expect(logWarnStub.calledOnce).to.equal(true); @@ -412,8 +412,14 @@ describe('adpod.js', function () { }); describe('checkAdUnitSetupHook', function () { + let results; + let callbackFn = function (adUnits) { + results = adUnits; + }; + beforeEach(function () { logWarnStub = sinon.stub(utils, 'logWarn'); + results = null; }); afterEach(function() { @@ -421,7 +427,6 @@ describe('adpod.js', function () { }); it('removes an incorrectly setup adpod adunit', function() { - let results; let adUnits = [{ code: 'test1', mediaTypes: { @@ -438,9 +443,7 @@ describe('adpod.js', function () { } }]; - checkAdUnitSetupHook(adUnits, function (adUnits) { - results = adUnits; - }); + checkAdUnitSetupHook(callbackFn, adUnits); expect(results).to.deep.equal([{ code: 'test2', @@ -454,7 +457,6 @@ describe('adpod.js', function () { }); it('accepts mixed set of adunits', function() { - let results; let adUnits = [{ code: 'test3', mediaTypes: { @@ -474,9 +476,7 @@ describe('adpod.js', function () { } }]; - checkAdUnitSetupHook(adUnits, function (adUnits) { - results = adUnits; - }); + checkAdUnitSetupHook(callbackFn, adUnits); expect(results).to.deep.equal(adUnits); expect(logWarnStub.called).to.equal(false); @@ -485,6 +485,9 @@ describe('adpod.js', function () { describe('checkVideoBidSetupHook', function () { let callbackResult; + let callbackFn = function (bid) { + callbackResult = bid; + }; const adpodTestBid = { video: { context: ADPOD, @@ -510,9 +513,7 @@ describe('adpod.js', function () { let bannerTestBid = { mediaType: 'video' }; - let hookReturnValue = checkVideoBidSetupHook(bannerTestBid, {}, {}, 'instream', function (bid) { - callbackResult = bid; - }); + let hookReturnValue = checkVideoBidSetupHook(callbackFn, bannerTestBid, {}, {}, 'instream'); expect(callbackResult).to.deep.equal(bannerTestBid); expect(hookReturnValue).to.be.undefined; expect(logErrorStub.called).to.equal(false); @@ -526,9 +527,7 @@ describe('adpod.js', function () { }); let goodBid = utils.deepClone(adpodTestBid); - let hookReturnValue = checkVideoBidSetupHook(goodBid, {}, {}, ADPOD, function (bid) { - callbackResult = bid; - }); + let hookReturnValue = checkVideoBidSetupHook(callbackFn, goodBid, {}, {}, ADPOD); expect(callbackResult).to.be.null; expect(hookReturnValue).to.equal(true); expect(logErrorStub.called).to.equal(false); @@ -536,9 +535,7 @@ describe('adpod.js', function () { it('returns false when a required property from an adpod bid is missing', function() { function testInvalidAdpodBid(badTestBid, shouldErrorBeLogged) { - let hookReturnValue = checkVideoBidSetupHook(badTestBid, {}, {}, ADPOD, function(bid) { - callbackResult = bid; - }); + let hookReturnValue = checkVideoBidSetupHook(callbackFn, badTestBid, {}, {}, ADPOD); expect(callbackResult).to.be.null; expect(hookReturnValue).to.equal(false); expect(logErrorStub.called).to.equal(shouldErrorBeLogged); From 0d7d2af5f3cce764820811d635bcda7d8adbc560 Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Thu, 31 Jan 2019 11:48:41 -0500 Subject: [PATCH 04/21] add missing custom key fields in fake bids for unit tests --- test/spec/modules/adpod_spec.js | 76 +++++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 13 deletions(-) diff --git a/test/spec/modules/adpod_spec.js b/test/spec/modules/adpod_spec.js index 35c114d4270..91704c9f9a4 100644 --- a/test/spec/modules/adpod_spec.js +++ b/test/spec/modules/adpod_spec.js @@ -7,8 +7,6 @@ import { callPrebidCacheHook, checkAdUnitSetupHook, checkVideoBidSetupHook, adpo let expect = require('chai').expect; -// TODO - need to update the bidResponses to include the targting keys/UUID; update store stubs? - describe('adpod.js', function () { let logErrorStub; let logWarnStub; @@ -21,6 +19,7 @@ describe('adpod.js', function () { let storeStub; let afterBidAddedSpy; let auctionBids = []; + let uuid; let callbackFn = function() { callbackResult = true; @@ -34,6 +33,7 @@ describe('adpod.js', function () { beforeEach(function() { callbackResult = null; + uuid = utils.generateUUID(); afterBidAddedSpy = sinon.spy(); storeStub = sinon.stub(videoCache, 'store'); logWarnStub = sinon.stub(utils, 'logWarn'); @@ -98,7 +98,12 @@ describe('adpod.js', function () { video: { context: ADPOD, durationSeconds: 30 - } + }, + adserverTargeting: { + hb_price_industry_duration: '10.00_airline_30s', + hb_uuid: uuid + }, + customCacheKey: `10.00_airline_30s_${uuid}` }; let bidResponse2 = { adId: 'adId234', @@ -111,7 +116,12 @@ describe('adpod.js', function () { video: { context: ADPOD, durationSeconds: 30 - } + }, + adserverTargeting: { + hb_price_industry_duration: '15.00_airline_30s', + hb_uuid: uuid + }, + customCacheKey: `15.00_airline_30s_${uuid}` }; let bidderRequest = { adUnitCode: 'adpod_1', @@ -136,7 +146,7 @@ describe('adpod.js', function () { expect(auctionBids[0].adId).to.equal('adId123'); expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^10\.00_airline_30s_.*/); expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('10.00_airline_30s'); - expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; + expect(auctionBids[0].adserverTargeting.hb_uuid).to.equal(uuid); expect(auctionBids[1].adId).to.equal('adId234'); expect(auctionBids[1].customCacheKey).to.exist.and.to.match(/^15\.00_airline_30s_.*/); expect(auctionBids[1].adserverTargeting.hb_price_industry_duration).to.equal('15.00_airline_30s'); @@ -164,7 +174,12 @@ describe('adpod.js', function () { video: { context: ADPOD, durationSeconds: 30 - } + }, + adserverTargeting: { + hb_price_industry_duration: '15.00_airline_30s', + hb_uuid: uuid + }, + customCacheKey: `15.00_airline_30s_${uuid}` }; let bidderRequest = { adUnitCode: 'adpod_2', @@ -213,7 +228,12 @@ describe('adpod.js', function () { video: { context: ADPOD, durationSeconds: 15 - } + }, + adserverTargeting: { + hb_price_industry_duration: '15.00_airline_15s', + hb_uuid: uuid + }, + customCacheKey: `15.00_airline_15s_${uuid}` }; let bidResponse2 = { adId: 'multi_ad2', @@ -226,7 +246,12 @@ describe('adpod.js', function () { video: { context: ADPOD, durationSeconds: 15 - } + }, + adserverTargeting: { + hb_price_industry_duration: '15.00_news_15s', + hb_uuid: uuid + }, + customCacheKey: `15.00_news_15s_${uuid}` }; let bidResponse3 = { adId: 'multi_ad3', @@ -239,7 +264,12 @@ describe('adpod.js', function () { video: { context: ADPOD, durationSeconds: 15 - } + }, + adserverTargeting: { + hb_price_industry_duration: '10.00_sports_15s', + hb_uuid: uuid + }, + customCacheKey: `10.00_sports_15s_${uuid}` }; let bidderRequest = { @@ -306,7 +336,12 @@ describe('adpod.js', function () { video: { context: ADPOD, durationSeconds: 45 - } + }, + adserverTargeting: { + hb_price_industry_duration: '5.00_tech_45s', + hb_uuid: uuid + }, + customCacheKey: `5.00_tech_45s_${uuid}` }; let bidResponse2 = { adId: 'dup_ad_2', @@ -319,7 +354,12 @@ describe('adpod.js', function () { video: { context: ADPOD, durationSeconds: 45 - } + }, + adserverTargeting: { + hb_price_industry_duration: '5.00_tech_45s', + hb_uuid: uuid + }, + customCacheKey: `5.00_tech_45s_${uuid}` }; let bidderRequest = { adUnitCode: 'adpod_4', @@ -373,7 +413,12 @@ describe('adpod.js', function () { video: { context: ADPOD, durationSeconds: 30 - } + }, + adserverTargeting: { + hb_price_industry_duration: '5.00_tech_30s', + hb_uuid: uuid + }, + customCacheKey: `5.00_tech_30s_${uuid}` }; let bidResponse2 = { adId: 'err_ad_2', @@ -386,7 +431,12 @@ describe('adpod.js', function () { video: { context: ADPOD, durationSeconds: 30 - } + }, + adserverTargeting: { + hb_price_industry_duration: '5.00_tech_30s', + hb_uuid: uuid + }, + customCacheKey: `5.00_tech_30s_${uuid}` }; let bidderRequest = { adUnitCode: 'adpod_5', From dcd4056c2f29a2c5e2ec4f649b35cb6008797622 Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Mon, 4 Feb 2019 14:30:42 -0500 Subject: [PATCH 05/21] several updates to module and unit tests --- modules/adpod.js | 167 +++++++++++++++++++--- test/spec/modules/adpod_spec.js | 245 +++++++++++++++++++------------- 2 files changed, 292 insertions(+), 120 deletions(-) diff --git a/modules/adpod.js b/modules/adpod.js index f76cf4504d1..778a9cc035b 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -1,19 +1,23 @@ import * as utils from '../src/utils'; -import { addBidToAuction, doCallbacksIfTimedout } from '../src/auction'; +import { addBidToAuction, doCallbacksIfTimedout, AUCTION_IN_PROGRESS } from '../src/auction'; import { store } from '../src/videoCache'; import { hooks } from '../src/hook'; import { config } from '../src/config'; import Set from 'core-js/library/fn/set'; +import find from 'core-js/library/fn/array/find'; const from = require('core-js/library/fn/array/from'); export const ADPOD = 'adpod'; -// NOTE - can these global vars be handled differently??? // NOTE - are these good defaults? let queueTimeDelay = 50; let queueSizeLimit = 5; let bidCacheRegistry = createBidCacheRegistry(); +/** + * Create a registry object that stores/manages bids while be held in queue for Prebid Cache. + * @returns registry object with defined accessor functions + */ function createBidCacheRegistry() { let registry = {}; return { @@ -42,51 +46,99 @@ function createBidCacheRegistry() { } } +/** + * Creates a function that when called updates the bid queue and extends the running timer (when called subsequently). + * Once the threshold for the queue (defined by queueSizeLimit), the queue will be flushed by calling the `firePrebidCacheCall` function. + * If there is a long enough time between calls (based on timeoutDration), the queue will automatically flush itself. + * @param {Number} timeoutDuration number of milliseconds to pass before timer expires and current bid queue is flushed + * @returns {Function} + */ function createDispatcher(timeoutDuration) { let timeout; let counter = 1; - return function() { + return function(auctionInstance, bidListArr, afterBidAdded, killQueue) { const context = this; - const args = arguments; var callbackFn = function() { - firePrebidCacheCall.apply(context, args); + firePrebidCacheCall.call(context, auctionInstance, bidListArr, afterBidAdded); }; clearTimeout(timeout); - // want to fire off the queue if either: size limit is reached or time has passed since last call to dispatcher - if (counter === queueSizeLimit) { - counter = 1; - callbackFn(); + if (!killQueue) { + // want to fire off the queue if either: size limit is reached or time has passed since last call to dispatcher + if (counter === queueSizeLimit) { + counter = 1; + callbackFn(); + } else { + counter++; + timeout = setTimeout(callbackFn, timeoutDuration); + } } else { - counter++; - timeout = setTimeout(callbackFn, timeoutDuration); + counter = 1; } }; } +/** + * This function reads certain fields from the bid to generate a specific key used for caching the bid in Prebid Cache + * @param {Object} bid bid object to update + */ +function attachPriceIndustryDurationKeyToBid(bid) { + let category = bid.meta && bid.meta.adServerCatId; + let duration = bid.video && bid.video.durationBucket; + let cpmFixed = bid.cpm.toFixed(2); + let initialCacheKey = bidCacheRegistry.getInitialCacheKey(bid); + // TODO? - add check to verify all above values are populated and throw error if not? + let pid = `${cpmFixed}_${category}_${duration}s`; + + if (!bid.adserverTargeting) { + bid.adserverTargeting = {}; + } + bid.adserverTargeting.hb_price_industry_duration = pid; + bid.adserverTargeting.hb_uuid = initialCacheKey; + bid.customCacheKey = `${pid}_${initialCacheKey}`; +} + +/** + * Updates the running queue for the associated auction. + * Does a check to ensure the auction is still running; if it's not - the previously running queue is killed. + * @param {*} auctionInstance running context of the auction + * @param {Object} bidResponse bid object being added to queue + * @param {Function} afterBidAdded callback function used when Prebid Cache responds + */ function updateBidQueue(auctionInstance, bidResponse, afterBidAdded) { let bidListIter = bidCacheRegistry.getBids(bidResponse); if (bidListIter) { let bidListArr = from(bidListIter); let callDispatcher = bidCacheRegistry.getQueueDispatcher(bidResponse); - callDispatcher(auctionInstance, bidListArr, afterBidAdded); + let killQueue = !!(auctionInstance.getAuctionStatus() !== AUCTION_IN_PROGRESS); + callDispatcher(auctionInstance, bidListArr, afterBidAdded, killQueue); } else { utils.logWarn('Attempted to cache a bid from an unkonwn auction. Bid:', bidResponse); } } +/** + * Small helper function to remove bids from internal storage; normally b/c they're about to sent to Prebid Cache for processing. + * @param {Array[Object]} bidResponses list of bids to remove + */ function removeBidsFromStorage(bidResponses) { for (let i = 0; i < bidResponses.length; i++) { bidCacheRegistry.removeBid(bidResponses[i]); } } +/** + * + * @param {*} auctionInstance running context of the auction + * @param {Array[Object]} bidList list of bid objects that need to be sent to Prebid Cache + * @param {Function} afterBidAdded callback function used when Prebid Cache responds + */ function firePrebidCacheCall(auctionInstance, bidList, afterBidAdded) { - // note to reviewers - doing this to ensure the variable wouldn't be malformed by the additional incoming bids + // note to reviewers - was doing this to ensure the variable wouldn't be malformed by the additional incoming bids // while the current list is being processed by PBC and the store callback // ...but is this copy really needed? let bidListCopy = bidList.slice(); @@ -103,10 +155,9 @@ function firePrebidCacheCall(auctionInstance, bidList, afterBidAdded) { } else { for (let i = 0; i < cacheIds.length; i++) { // when uuid in response is empty string then the key already existed, so this bid wasn't cached - // TODO - should we throw warning here? should we call the afterBidAdded() anyway to decrement the internal bid counter? - // TODO - verify the cacheKey is one of the expected values (eg in case PBC returns generic UUID)? + // TODO? - should we throw warning here? should we call the afterBidAdded() anyway to decrement the internal bid counter? + // TODO? - verify the cacheKey is one of the expected values (eg in case PBC returns generic UUID)? if (cacheIds[i].uuid !== '') { - // bidListCopy[i].videoCacheKey = cacheIds[i].uuid; // remove later addBidToAuction(auctionInstance, bidListCopy[i]); afterBidAdded(); } @@ -115,17 +166,33 @@ function firePrebidCacheCall(auctionInstance, bidList, afterBidAdded) { }); } +/** + * This is the main hook function to handle adpod bids; maintains the logic to temporarily hold bids in a queue in order to send bulk requests to Prebid Cache. + * @param {Function} fn reference to original function (used by hook logic) + * @param {*} auctionInstance running context of the auction + * @param {Object} bidResponse incoming bid; if adpod, will be processed through hook function. If not adpod, returns to original function. + * @param {Function} afterBidAdded callback function used when Prebid Cache responds + * @param {Object} bidderRequest copy of bid's associated bidderRequest object + */ export function callPrebidCacheHook(fn, auctionInstance, bidResponse, afterBidAdded, bidderRequest) { let videoConfig = bidderRequest.mediaTypes && bidderRequest.mediaTypes.video; - // if (videoConfig && videoConfig.context === 'instream') { //remove later if (videoConfig && videoConfig.context === ADPOD) { bidCacheRegistry.addBid(bidResponse); + attachPriceIndustryDurationKeyToBid(bidResponse); + updateBidQueue(auctionInstance, bidResponse, afterBidAdded); } else { fn.call(this, auctionInstance, bidResponse, afterBidAdded, bidderRequest); } } +/** + * This hook function will review the adUnit setup and verify certain required are present in any adpod adUnits. + * If the fields are missing or incorrectly setup, the adUnit is removed from the list. + * @param {Function} fn reference to original function (used by hook logic) + * @param {Array[Object]} adUnits list of adUnits to be evaluated + * @returns {Array[Object]} list of adUnits that passed the check + */ export function checkAdUnitSetupHook(fn, adUnits) { let goodAdUnits = adUnits.filter(adUnit => { let videoConfig = adUnit.mediaTypes && adUnit.mediaTypes.video; @@ -150,10 +217,58 @@ export function checkAdUnitSetupHook(fn, adUnits) { fn.call(this, adUnits); } +/** + * This check evaluates the incoming bid's `video.durationSeconds` field and tests it against specific logic depending on adUnit config. Summary of logic below: + * when adUnit.mediaTypes.video.requireExactDuration is true + * - only bids that exactly match those listed values are accepted (don't round at all). + * - populate the `bid.video.durationBucket` field with the matching duration value + * when adUnit.mediaTypes.video.requireExactDuration is false + * - round the duration to the next highest specified duration value based on adunit (eg if range was [5, 15, 30] -> 3s is rounded to 5s; 16s is rounded to 30s) + * - if the bid is above the range of thje listed durations, reject the bid + * - set the rounded duration value in the `bid.video.durationBucket` field for accepted bids + * @param {Object} bidderRequest copy of the bidderRequest object associated to bidResponse + * @param {Object} bidResponse incoming bidResponse being evaluated by bidderFactory + * @returns {boolean} return false if bid duration is deemed invalid as per adUnit configuration; return true if fine +*/ +function checkBidDuration(bidderRequest, bidResponse) { + let bidDuration = bidResponse.video && bidResponse.video.durationSeconds; + let videoConfig = bidderRequest.mediaTypes && bidderRequest.mediaTypes.video; + let adUnitRanges = videoConfig.durationRangeSec; + adUnitRanges.sort((a, b) => a - b); // ensure the ranges are sorted in numeric order + + if (!videoConfig.requireExactDuration) { + let max = Math.max(...adUnitRanges); + if (bidDuration <= max) { + let nextHighestRange = find(adUnitRanges, range => range >= bidDuration); + bidResponse.video.durationBucket = nextHighestRange; + } else { + utils.logWarn(`Detected a bid with a duration value outside the accepted ranges specified in adUnit.mediaTypes.video.durationRangeSec. Rejecting bid: `, bidResponse); + return false; + } + } else { + if (find(adUnitRanges, range => range === bidDuration)) { + bidResponse.video.durationBucket = bidDuration; + } else { + utils.logWarn(`Detected a bid with a duration value not part of the list of accepted ranges specified in adUnit.mediaTypes.video.durationRangeSec. Exact match durations must be used for this adUnit. Rejecting bid: `, bidResponse); + return false; + } + } + return true; +} + +/** + * This hooked function evaluates an adpod bid and determines if the required fields are present. + * If it's found to not be an adpod bid, it return to original function via hook logic + * @param {Function} fn reference to original function (used by hook logic) + * @param {Object} bid incoming bid object + * @param {Object} bidRequest bidRequest object of associated bid + * @param {Object} videoMediaType copy of the `bidRequest.mediaTypes.video` object; used in original function + * @param {String} context value of the `bidRequest.mediaTypes.video.context` field; used in original function + * @returns {boolean} this return is only used for adpod bids + */ export function checkVideoBidSetupHook(fn, bid, bidRequest, videoMediaType, context) { if (context === ADPOD) { - // TODO - check internally on field name for FW category - if (!utils.deepAccess(bid, 'meta.primaryCatId')) { + if (!utils.deepAccess(bid, 'meta.iabSubCatId')) { return false; } @@ -162,8 +277,11 @@ export function checkVideoBidSetupHook(fn, bid, bidRequest, videoMediaType, cont return false; } - if (!utils.deepAccess(bid, 'video.durationSeconds')) { + if (!utils.deepAccess(bid, 'video.durationSeconds') || bid.video.durationSeconds <= 0) { return false; + } else { + let isBidGood = checkBidDuration(bidRequest, bid); + if (!isBidGood) return false; } } @@ -181,6 +299,10 @@ export function checkVideoBidSetupHook(fn, bid, bidRequest, videoMediaType, cont } } +/** + * This function reads the (optional) settings for the adpod as set from the setConfig() + * @param {Object} config contains the config settings for adpod module + */ export function adpodSetConfig(config) { if (config.bidQueueTimeDelay !== undefined) { if (typeof config.bidQueueTimeDelay === 'number' && config.bidQueueTimeDelay > 0) { @@ -202,7 +324,10 @@ export function adpodSetConfig(config) { } config.getConfig('adpod', config => adpodSetConfig(config.adpod)); -export function initAdpod() { +/** + * This function initializes the adpod module's hooks. This is called by the corresponding adserver video module. + */ +export function initAdpodHooks() { hooks['callPrebidCache'].before(callPrebidCacheHook, 15); hooks['checkAdUnitSetup'].before(checkAdUnitSetupHook, 15); hooks['checkVideoBidSetup'].before(checkVideoBidSetupHook, 15); diff --git a/test/spec/modules/adpod_spec.js b/test/spec/modules/adpod_spec.js index 91704c9f9a4..aea9ded0506 100644 --- a/test/spec/modules/adpod_spec.js +++ b/test/spec/modules/adpod_spec.js @@ -19,7 +19,6 @@ describe('adpod.js', function () { let storeStub; let afterBidAddedSpy; let auctionBids = []; - let uuid; let callbackFn = function() { callbackResult = true; @@ -33,7 +32,6 @@ describe('adpod.js', function () { beforeEach(function() { callbackResult = null; - uuid = utils.generateUUID(); afterBidAddedSpy = sinon.spy(); storeStub = sinon.stub(videoCache, 'store'); logWarnStub = sinon.stub(utils, 'logWarn'); @@ -93,17 +91,13 @@ describe('adpod.js', function () { mediaType: 'video', cpm: 10, meta: { - primaryCatId: 'airline' + adServerCatId: 'airline' }, video: { context: ADPOD, - durationSeconds: 30 - }, - adserverTargeting: { - hb_price_industry_duration: '10.00_airline_30s', - hb_uuid: uuid - }, - customCacheKey: `10.00_airline_30s_${uuid}` + durationSeconds: 20, + durationBucket: 30 + } }; let bidResponse2 = { adId: 'adId234', @@ -111,17 +105,13 @@ describe('adpod.js', function () { mediaType: 'video', cpm: 15, meta: { - primaryCatId: 'airline' + adServerCatId: 'airline' }, video: { context: ADPOD, - durationSeconds: 30 - }, - adserverTargeting: { - hb_price_industry_duration: '15.00_airline_30s', - hb_uuid: uuid - }, - customCacheKey: `15.00_airline_30s_${uuid}` + durationSeconds: 25, + durationBucket: 30 + } }; let bidderRequest = { adUnitCode: 'adpod_1', @@ -146,7 +136,7 @@ describe('adpod.js', function () { expect(auctionBids[0].adId).to.equal('adId123'); expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^10\.00_airline_30s_.*/); expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('10.00_airline_30s'); - expect(auctionBids[0].adserverTargeting.hb_uuid).to.equal(uuid); + expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; expect(auctionBids[1].adId).to.equal('adId234'); expect(auctionBids[1].customCacheKey).to.exist.and.to.match(/^15\.00_airline_30s_.*/); expect(auctionBids[1].adserverTargeting.hb_price_industry_duration).to.equal('15.00_airline_30s'); @@ -169,17 +159,13 @@ describe('adpod.js', function () { mediaType: 'video', cpm: 15, meta: { - primaryCatId: 'airline' + adServerCatId: 'airline' }, video: { context: ADPOD, - durationSeconds: 30 - }, - adserverTargeting: { - hb_price_industry_duration: '15.00_airline_30s', - hb_uuid: uuid - }, - customCacheKey: `15.00_airline_30s_${uuid}` + durationSeconds: 30, + durationBucket: 30 + } }; let bidderRequest = { adUnitCode: 'adpod_2', @@ -223,17 +209,13 @@ describe('adpod.js', function () { mediaType: 'video', cpm: 15, meta: { - primaryCatId: 'airline' + adServerCatId: 'airline' }, video: { context: ADPOD, - durationSeconds: 15 - }, - adserverTargeting: { - hb_price_industry_duration: '15.00_airline_15s', - hb_uuid: uuid - }, - customCacheKey: `15.00_airline_15s_${uuid}` + durationSeconds: 15, + durationBucket: 15 + } }; let bidResponse2 = { adId: 'multi_ad2', @@ -241,17 +223,13 @@ describe('adpod.js', function () { mediaType: 'video', cpm: 15, meta: { - primaryCatId: 'news' + adServerCatId: 'news' }, video: { context: ADPOD, - durationSeconds: 15 - }, - adserverTargeting: { - hb_price_industry_duration: '15.00_news_15s', - hb_uuid: uuid - }, - customCacheKey: `15.00_news_15s_${uuid}` + durationSeconds: 15, + durationBucket: 15 + } }; let bidResponse3 = { adId: 'multi_ad3', @@ -259,17 +237,13 @@ describe('adpod.js', function () { mediaType: 'video', cpm: 10, meta: { - primaryCatId: 'sports' + adServerCatId: 'sports' }, video: { context: ADPOD, - durationSeconds: 15 - }, - adserverTargeting: { - hb_price_industry_duration: '10.00_sports_15s', - hb_uuid: uuid - }, - customCacheKey: `10.00_sports_15s_${uuid}` + durationSeconds: 15, + durationBucket: 15 + } }; let bidderRequest = { @@ -331,17 +305,13 @@ describe('adpod.js', function () { mediaType: 'video', cpm: 5, meta: { - primaryCatId: 'tech' + adServerCatId: 'tech' }, video: { context: ADPOD, - durationSeconds: 45 - }, - adserverTargeting: { - hb_price_industry_duration: '5.00_tech_45s', - hb_uuid: uuid - }, - customCacheKey: `5.00_tech_45s_${uuid}` + durationSeconds: 45, + durationBucket: 45 + } }; let bidResponse2 = { adId: 'dup_ad_2', @@ -349,17 +319,13 @@ describe('adpod.js', function () { mediaType: 'video', cpm: 5, meta: { - primaryCatId: 'tech' + adServerCatId: 'tech' }, video: { context: ADPOD, - durationSeconds: 45 - }, - adserverTargeting: { - hb_price_industry_duration: '5.00_tech_45s', - hb_uuid: uuid - }, - customCacheKey: `5.00_tech_45s_${uuid}` + durationSeconds: 45, + durationBucket: 45 + } }; let bidderRequest = { adUnitCode: 'adpod_4', @@ -408,17 +374,13 @@ describe('adpod.js', function () { mediaType: 'video', cpm: 5, meta: { - primaryCatId: 'tech' + adServerCatId: 'tech' }, video: { context: ADPOD, - durationSeconds: 30 - }, - adserverTargeting: { - hb_price_industry_duration: '5.00_tech_30s', - hb_uuid: uuid - }, - customCacheKey: `5.00_tech_30s_${uuid}` + durationSeconds: 30, + durationBucket: 30 + } }; let bidResponse2 = { adId: 'err_ad_2', @@ -426,17 +388,13 @@ describe('adpod.js', function () { mediaType: 'video', cpm: 5, meta: { - primaryCatId: 'tech' + adServerCatId: 'tech' }, video: { context: ADPOD, - durationSeconds: 30 - }, - adserverTargeting: { - hb_price_industry_duration: '5.00_tech_30s', - hb_uuid: uuid - }, - customCacheKey: `5.00_tech_30s_${uuid}` + durationSeconds: 30, + durationBucket: 30 + } }; let bidderRequest = { adUnitCode: 'adpod_5', @@ -476,7 +434,7 @@ describe('adpod.js', function () { utils.logWarn.restore(); }); - it('removes an incorrectly setup adpod adunit', function() { + it('removes an incorrectly setup adpod adunit - required fields are missing', function() { let adUnits = [{ code: 'test1', mediaTypes: { @@ -514,7 +472,8 @@ describe('adpod.js', function () { context: ADPOD, playerSize: [300, 300], adPodDurationSec: 360, - durationRangeSec: [15, 30, 45] + durationRangeSec: [15, 30, 45], + requireExactDuration: true } } }, { @@ -535,28 +494,58 @@ describe('adpod.js', function () { describe('checkVideoBidSetupHook', function () { let callbackResult; - let callbackFn = function (bid) { + const callbackFn = function (bid) { callbackResult = bid; }; const adpodTestBid = { video: { context: ADPOD, - durationSeconds: 300 + durationSeconds: 15, + durationBucket: 15 }, meta: { - primaryCatId: 'testCategory_123' + iabSubCatId: 'testCategory_123' }, vastXml: 'test XML here' }; + const bidderRequestNoExact = { + mediaTypes: { + video: { + context: ADPOD, + playerSize: [300, 400], + durationRangeSec: [15, 45], + requireExactDuration: false, + adPodDurationSec: 300 + } + } + }; + const bidderRequestWithExact = { + mediaTypes: { + video: { + context: ADPOD, + playerSize: [300, 400], + durationRangeSec: [15, 30, 45, 60], + requireExactDuration: true, + adPodDurationSec: 300 + } + } + }; beforeEach(function() { callbackResult = null; + config.setConfig({ + cache: { + url: 'http://test.cache.url/endpoint' + } + }); + logWarnStub = sinon.stub(utils, 'logWarn'); logErrorStub = sinon.stub(utils, 'logError'); }); afterEach(function() { config.resetConfig(); - utils.logError.restore(); + logWarnStub.restore(); + logErrorStub.restore(); }) it('redirects to original function for non-adpod type video bids', function() { @@ -570,14 +559,8 @@ describe('adpod.js', function () { }); it('returns true when adpod bid is properly setup', function() { - config.setConfig({ - cache: { - url: 'http://test.cache.url/endpoint' - } - }); - let goodBid = utils.deepClone(adpodTestBid); - let hookReturnValue = checkVideoBidSetupHook(callbackFn, goodBid, {}, {}, ADPOD); + let hookReturnValue = checkVideoBidSetupHook(callbackFn, goodBid, bidderRequestNoExact, {}, ADPOD); expect(callbackResult).to.be.null; expect(hookReturnValue).to.equal(true); expect(logErrorStub.called).to.equal(false); @@ -585,11 +568,12 @@ describe('adpod.js', function () { it('returns false when a required property from an adpod bid is missing', function() { function testInvalidAdpodBid(badTestBid, shouldErrorBeLogged) { - let hookReturnValue = checkVideoBidSetupHook(callbackFn, badTestBid, {}, {}, ADPOD); + let hookReturnValue = checkVideoBidSetupHook(callbackFn, badTestBid, bidderRequestNoExact, {}, ADPOD); expect(callbackResult).to.be.null; expect(hookReturnValue).to.equal(false); expect(logErrorStub.called).to.equal(shouldErrorBeLogged); } + config.resetConfig(); let noCatBid = utils.deepClone(adpodTestBid); delete noCatBid.meta; @@ -610,6 +594,71 @@ describe('adpod.js', function () { let noCacheUrlBid = utils.deepClone(adpodTestBid); testInvalidAdpodBid(noCacheUrlBid, true); }); + + describe('checkBidDuration', function() { + const basicBid = { + video: { + context: ADPOD, + durationSeconds: 30 + }, + meta: { + iabSubCatId: 'testCategory_123' + }, + vastXml: '' + }; + + it('when requireExactDuration is true', function() { + let goodBid = utils.deepClone(basicBid); + let hookReturnValue = checkVideoBidSetupHook(callbackFn, goodBid, bidderRequestWithExact, {}, ADPOD); + + expect(callbackResult).to.be.null; + expect(goodBid.video.durationBucket).to.equal(30); + expect(hookReturnValue).to.equal(true); + expect(logWarnStub.called).to.equal(false); + + let badBid = utils.deepClone(basicBid); + badBid.video.durationSeconds = 14; + hookReturnValue = checkVideoBidSetupHook(callbackFn, badBid, bidderRequestWithExact, {}, ADPOD); + + expect(callbackResult).to.be.null; + expect(badBid.video.durationBucket).to.be.undefined; + expect(hookReturnValue).to.equal(false); + expect(logWarnStub.calledOnce).to.equal(true); + }); + + it('when requireExactDuration is false and bids are bucketed properly', function() { + function testRoundingForGoodBId(bid, bucketValue) { + let hookReturnValue = checkVideoBidSetupHook(callbackFn, bid, bidderRequestNoExact, {}, ADPOD); + expect(callbackResult).to.be.null; + expect(bid.video.durationBucket).to.equal(bucketValue); + expect(hookReturnValue).to.equal(true); + expect(logWarnStub.called).to.equal(false); + } + + let goodBid45 = utils.deepClone(basicBid); + goodBid45.video.durationSeconds = 45; + testRoundingForGoodBId(goodBid45, 45); + + let goodBid30 = utils.deepClone(basicBid); + goodBid30.video.durationSeconds = 30; + testRoundingForGoodBId(goodBid30, 45); + + let goodBid14 = utils.deepClone(basicBid); + goodBid14.video.durationSeconds = 14; + testRoundingForGoodBId(goodBid14, 15); + }); + + it('when requireExactDuration is false and bid duration exceeds listed buckets', function() { + let badBid100 = utils.deepClone(basicBid); + badBid100.video.durationSeconds = 100; + + let hookReturnValue = checkVideoBidSetupHook(callbackFn, badBid100, bidderRequestNoExact, {}, ADPOD); + expect(callbackResult).to.be.null; + expect(badBid100.video.durationBucket).to.be.undefined; + expect(hookReturnValue).to.equal(false); + expect(logWarnStub.called).to.equal(true); + }); + }); }); describe('adpodSetConfig', function () { @@ -638,8 +687,6 @@ describe('adpod.js', function () { expect(logWarnStub.calledTwice).to.equal(true); }); - // TODO add unit test if there should be a ceiling for acceptable values - it('should not log any warning when using a valid config', function() { adpodSetConfig({ bidQueueSizeLimit: 10 From c661a7ac8e2929bbd90c771b20386c7f692953ca Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Mon, 4 Feb 2019 15:17:06 -0500 Subject: [PATCH 06/21] update logic to only add hooks once --- modules/adpod.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/modules/adpod.js b/modules/adpod.js index 778a9cc035b..6f55686de17 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -328,7 +328,14 @@ config.getConfig('adpod', config => adpodSetConfig(config.adpod)); * This function initializes the adpod module's hooks. This is called by the corresponding adserver video module. */ export function initAdpodHooks() { - hooks['callPrebidCache'].before(callPrebidCacheHook, 15); - hooks['checkAdUnitSetup'].before(checkAdUnitSetupHook, 15); - hooks['checkVideoBidSetup'].before(checkVideoBidSetupHook, 15); + function setupHookFnOnce(hookId, hookFn, priority = 15) { + let result = hooks[hookId].getHooks({hook: hookFn}); + if (result.length === 0) { + hooks[hookId].before(hookFn, priority); + } + } + + setupHookFnOnce('callPrebidCache', callPrebidCacheHook); + setupHookFnOnce('checkAdUnitSetup', checkAdUnitSetupHook); + setupHookFnOnce('checkVideoBidSetup', checkVideoBidSetupHook); } From 9c7d7e0e9646e69d47393199d71286ede56673db Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Tue, 5 Feb 2019 08:44:31 -0500 Subject: [PATCH 07/21] update targeting keys to new name --- modules/adpod.js | 11 +++++++---- test/spec/modules/adpod_spec.js | 28 ++++++++++++++-------------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/modules/adpod.js b/modules/adpod.js index 6f55686de17..952ef75e12b 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -9,6 +9,9 @@ import find from 'core-js/library/fn/array/find'; const from = require('core-js/library/fn/array/from'); export const ADPOD = 'adpod'; +const TARGETING_KEY_PB_CAT_DUR = 'hb_pb_cat_dur'; +const TARGETING_KEY_CACHE_ID = 'hb_cache_id' + // NOTE - are these good defaults? let queueTimeDelay = 50; let queueSizeLimit = 5; @@ -91,14 +94,14 @@ function attachPriceIndustryDurationKeyToBid(bid) { let cpmFixed = bid.cpm.toFixed(2); let initialCacheKey = bidCacheRegistry.getInitialCacheKey(bid); // TODO? - add check to verify all above values are populated and throw error if not? - let pid = `${cpmFixed}_${category}_${duration}s`; + let pcd = `${cpmFixed}_${category}_${duration}s`; if (!bid.adserverTargeting) { bid.adserverTargeting = {}; } - bid.adserverTargeting.hb_price_industry_duration = pid; - bid.adserverTargeting.hb_uuid = initialCacheKey; - bid.customCacheKey = `${pid}_${initialCacheKey}`; + bid.adserverTargeting[TARGETING_KEY_PB_CAT_DUR] = pcd; + bid.adserverTargeting[TARGETING_KEY_CACHE_ID] = initialCacheKey; + bid.customCacheKey = `${pcd}_${initialCacheKey}`; } /** diff --git a/test/spec/modules/adpod_spec.js b/test/spec/modules/adpod_spec.js index aea9ded0506..9944248b5ea 100644 --- a/test/spec/modules/adpod_spec.js +++ b/test/spec/modules/adpod_spec.js @@ -135,12 +135,12 @@ describe('adpod.js', function () { expect(auctionBids.length).to.equal(2); expect(auctionBids[0].adId).to.equal('adId123'); expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^10\.00_airline_30s_.*/); - expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('10.00_airline_30s'); - expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; + expect(auctionBids[0].adserverTargeting.hb_pb_cat_dur).to.equal('10.00_airline_30s'); + expect(auctionBids[0].adserverTargeting.hb_cache_id).to.exist; expect(auctionBids[1].adId).to.equal('adId234'); expect(auctionBids[1].customCacheKey).to.exist.and.to.match(/^15\.00_airline_30s_.*/); - expect(auctionBids[1].adserverTargeting.hb_price_industry_duration).to.equal('15.00_airline_30s'); - expect(auctionBids[1].adserverTargeting.hb_uuid).to.exist; + expect(auctionBids[1].adserverTargeting.hb_pb_cat_dur).to.equal('15.00_airline_30s'); + expect(auctionBids[1].adserverTargeting.hb_cache_id).to.exist; }); it('should send prebid cache call after set period of time (even if queue is not full)', function () { @@ -189,8 +189,8 @@ describe('adpod.js', function () { expect(auctionBids.length).to.equal(1); expect(auctionBids[0].adId).to.equal('adId234'); expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^15\.00_airline_30s_.*/); - expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('15.00_airline_30s'); - expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; + expect(auctionBids[0].adserverTargeting.hb_pb_cat_dur).to.equal('15.00_airline_30s'); + expect(auctionBids[0].adserverTargeting.hb_cache_id).to.exist; }); it('should execute multiple prebid cache calls when number of bids exceeds queue size', function () { @@ -271,16 +271,16 @@ describe('adpod.js', function () { expect(auctionBids.length).to.equal(3); expect(auctionBids[0].adId).to.equal('multi_ad1'); expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^15\.00_airline_15s_.*/); - expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('15.00_airline_15s'); - expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; + expect(auctionBids[0].adserverTargeting.hb_pb_cat_dur).to.equal('15.00_airline_15s'); + expect(auctionBids[0].adserverTargeting.hb_cache_id).to.exist; expect(auctionBids[1].adId).to.equal('multi_ad2'); expect(auctionBids[1].customCacheKey).to.exist.and.to.match(/^15\.00_news_15s_.*/); - expect(auctionBids[1].adserverTargeting.hb_price_industry_duration).to.equal('15.00_news_15s'); - expect(auctionBids[1].adserverTargeting.hb_uuid).to.exist.and.to.equal(auctionBids[0].adserverTargeting.hb_uuid); + expect(auctionBids[1].adserverTargeting.hb_pb_cat_dur).to.equal('15.00_news_15s'); + expect(auctionBids[1].adserverTargeting.hb_cache_id).to.exist.and.to.equal(auctionBids[0].adserverTargeting.hb_uuid); expect(auctionBids[2].adId).to.equal('multi_ad3'); expect(auctionBids[2].customCacheKey).to.exist.and.to.match(/^10\.00_sports_15s_.*/); - expect(auctionBids[2].adserverTargeting.hb_price_industry_duration).to.equal('10.00_sports_15s'); - expect(auctionBids[2].adserverTargeting.hb_uuid).to.exist.and.to.equal(auctionBids[0].adserverTargeting.hb_uuid); + expect(auctionBids[2].adserverTargeting.hb_pb_cat_dur).to.equal('10.00_sports_15s'); + expect(auctionBids[2].adserverTargeting.hb_cache_id).to.exist.and.to.equal(auctionBids[0].adserverTargeting.hb_uuid); }); it('should not add bid to auction when Prebid Cache detects an existing key', function () { @@ -350,8 +350,8 @@ describe('adpod.js', function () { expect(auctionBids.length).to.equal(1); expect(auctionBids[0].adId).to.equal('dup_ad_1'); expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^5\.00_tech_45s_.*/); - expect(auctionBids[0].adserverTargeting.hb_price_industry_duration).to.equal('5.00_tech_45s'); - expect(auctionBids[0].adserverTargeting.hb_uuid).to.exist; + expect(auctionBids[0].adserverTargeting.hb_pb_cat_dur).to.equal('5.00_tech_45s'); + expect(auctionBids[0].adserverTargeting.hb_cache_id).to.exist; }); it('should not add bids to auction if PBC returns an error', function() { From 1a6cd392c0681e8f177c72409f3b6b0c223fdb5e Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Tue, 5 Feb 2019 09:52:22 -0500 Subject: [PATCH 08/21] add module description --- modules/adpod.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/modules/adpod.js b/modules/adpod.js index 952ef75e12b..dce8e5784a0 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -1,3 +1,17 @@ +/** + * This module houses the functionality to evaluate and process adpod adunits/bids. Specifically there are several hooked functions, + * that either supplement the base function (ie to check something additional unique to adpod objects) or to replace the base funtion + * entirely when appropriate. + * + * Brief outline of each hook: + * - `callPrebidCacheHook` - for any adpod bids, this function will temporarily hold them in a queue in order to send the bids to Prebid Cache in bulk + * - `checkAdUnitSetupHook` - evaluates the adUnits to ensure that required fields for adpod adUnits are present. Invalid adpod adUntis are removed from the array. + * - `checkVideoBidSetupHook` - evaluates the adpod bid returned from an adaptor/bidder to ensure required fields are populated; also initializes duration bucket field. + * + * To initialize the module, there is an `initAdpodHooks()` function that should be imported and executed by a corresponding `...AdServerVideo` + * module that designed to support adpod video type ads. This import process allows this module to effectively act as a sub-module. + */ + import * as utils from '../src/utils'; import { addBidToAuction, doCallbacksIfTimedout, AUCTION_IN_PROGRESS } from '../src/auction'; import { store } from '../src/videoCache'; From 2b7158f63fdffe3cb248405fc124fdef5ef86f17 Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Tue, 5 Feb 2019 11:17:55 -0500 Subject: [PATCH 09/21] fix typo --- modules/adpod.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/adpod.js b/modules/adpod.js index dce8e5784a0..2f6f4766657 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -134,7 +134,7 @@ function updateBidQueue(auctionInstance, bidResponse, afterBidAdded) { let killQueue = !!(auctionInstance.getAuctionStatus() !== AUCTION_IN_PROGRESS); callDispatcher(auctionInstance, bidListArr, afterBidAdded, killQueue); } else { - utils.logWarn('Attempted to cache a bid from an unkonwn auction. Bid:', bidResponse); + utils.logWarn('Attempted to cache a bid from an unknown auction. Bid:', bidResponse); } } From 44eb4eccb571146193f168ddcfcf5e425dbba1a2 Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Tue, 5 Feb 2019 13:56:15 -0500 Subject: [PATCH 10/21] add adunit check in case of multi-format --- modules/adpod.js | 9 ++++++++- test/spec/modules/adpod_spec.js | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/modules/adpod.js b/modules/adpod.js index 2f6f4766657..037ecd33c19 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -212,8 +212,15 @@ export function callPrebidCacheHook(fn, auctionInstance, bidResponse, afterBidAd */ export function checkAdUnitSetupHook(fn, adUnits) { let goodAdUnits = adUnits.filter(adUnit => { - let videoConfig = adUnit.mediaTypes && adUnit.mediaTypes.video; + let mediaTypes = adUnit.mediaTypes; + let videoConfig = mediaTypes && mediaTypes.video; if (videoConfig && videoConfig.context === ADPOD) { + // run check to see if other mediaTypes are defined (ie multi-format); reject adUnit if so + if (Object.keys(mediaTypes).length > 1) { + utils.logWarn(`Detected more than one mediaType in adUnitCode: ${adUnit.code} while attempting to define an 'adpod' video adUnit. 'adpod' adUnits cannot be mixed with other mediaTypes. This adUnit will be removed from the auction.`); + return false; + } + let errMsg = `Detected missing or incorrectly setup fields for an adpod adUnit. Please review the following fields of adUnitCode: ${adUnit.code}. This adUnit will be removed from the auction.`; let playerSize = !!(videoConfig.playerSize && utils.isArrayOfNums(videoConfig.playerSize)); diff --git a/test/spec/modules/adpod_spec.js b/test/spec/modules/adpod_spec.js index 9944248b5ea..3076d50eaaa 100644 --- a/test/spec/modules/adpod_spec.js +++ b/test/spec/modules/adpod_spec.js @@ -464,6 +464,28 @@ describe('adpod.js', function () { expect(logWarnStub.calledOnce).to.equal(true); }); + it('removes an incorrectly setup adpod adunit - attempting to use multi-format adUnit', function() { + let adUnits = [{ + code: 'multi_test1', + mediaTypes: { + banner: { + sizes: [[300, 250], [300, 600]] + }, + video: { + context: 'adpod', + playerSize: [300, 250], + durationRangeSec: [15, 30, 45], + adPodDurationSec: 300 + } + } + }]; + + checkAdUnitSetupHook(callbackFn, adUnits); + + expect(results).to.deep.equal([]); + expect(logWarnStub.calledOnce).to.equal(true); + }); + it('accepts mixed set of adunits', function() { let adUnits = [{ code: 'test3', From 7380f08503e2fe81a13a34a5c63424da2e74cd5e Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Tue, 5 Feb 2019 15:51:52 -0500 Subject: [PATCH 11/21] add exports for adpod targeting keys --- modules/adpod.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/modules/adpod.js b/modules/adpod.js index 037ecd33c19..607325fa790 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -19,12 +19,11 @@ import { hooks } from '../src/hook'; import { config } from '../src/config'; import Set from 'core-js/library/fn/set'; import find from 'core-js/library/fn/array/find'; - const from = require('core-js/library/fn/array/from'); -export const ADPOD = 'adpod'; -const TARGETING_KEY_PB_CAT_DUR = 'hb_pb_cat_dur'; -const TARGETING_KEY_CACHE_ID = 'hb_cache_id' +export const ADPOD = 'adpod'; +export const TARGETING_KEY_PB_CAT_DUR = 'hb_pb_cat_dur'; +export const TARGETING_KEY_CACHE_ID = 'hb_cache_id' // NOTE - are these good defaults? let queueTimeDelay = 50; From db14fc6f5e6114f0339925f538cfce5b15d2ff30 Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Wed, 6 Feb 2019 10:33:01 -0500 Subject: [PATCH 12/21] move ADPOD constant to mediaTypes.js --- modules/adpod.js | 2 +- src/mediaTypes.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/adpod.js b/modules/adpod.js index 607325fa790..2224d357092 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -17,11 +17,11 @@ import { addBidToAuction, doCallbacksIfTimedout, AUCTION_IN_PROGRESS } from '../ import { store } from '../src/videoCache'; import { hooks } from '../src/hook'; import { config } from '../src/config'; +import { ADPOD } from '../src/mediaTypes'; import Set from 'core-js/library/fn/set'; import find from 'core-js/library/fn/array/find'; const from = require('core-js/library/fn/array/from'); -export const ADPOD = 'adpod'; export const TARGETING_KEY_PB_CAT_DUR = 'hb_pb_cat_dur'; export const TARGETING_KEY_CACHE_ID = 'hb_cache_id' diff --git a/src/mediaTypes.js b/src/mediaTypes.js index 7a40030e4e2..fd787c98021 100644 --- a/src/mediaTypes.js +++ b/src/mediaTypes.js @@ -15,3 +15,5 @@ export const NATIVE = 'native'; export const VIDEO = 'video'; /** @type MediaType */ export const BANNER = 'banner'; +/** @type MediaType */ +export const ADPOD = 'adpod'; From 69bc093e5a3ace7b76b36431c9c67c428c253a7a Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Wed, 6 Feb 2019 10:46:17 -0500 Subject: [PATCH 13/21] undo mediaTypes change --- src/mediaTypes.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/mediaTypes.js b/src/mediaTypes.js index fd787c98021..7a40030e4e2 100644 --- a/src/mediaTypes.js +++ b/src/mediaTypes.js @@ -15,5 +15,3 @@ export const NATIVE = 'native'; export const VIDEO = 'video'; /** @type MediaType */ export const BANNER = 'banner'; -/** @type MediaType */ -export const ADPOD = 'adpod'; From f655c5c6b324ef6c36f11c1b1f95682e8b237bf3 Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Wed, 6 Feb 2019 14:25:15 -0500 Subject: [PATCH 14/21] fix unit tests --- test/spec/modules/adpod_spec.js | 37 ++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/test/spec/modules/adpod_spec.js b/test/spec/modules/adpod_spec.js index 3076d50eaaa..6c052a48a7a 100644 --- a/test/spec/modules/adpod_spec.js +++ b/test/spec/modules/adpod_spec.js @@ -2,8 +2,9 @@ import * as utils from 'src/utils'; import { config } from 'src/config'; import * as videoCache from 'src/videoCache'; import * as auction from 'src/auction'; +import { ADPOD } from 'src/mediaTypes'; -import { callPrebidCacheHook, checkAdUnitSetupHook, checkVideoBidSetupHook, adpodSetConfig, ADPOD } from 'modules/adpod'; +import { callPrebidCacheHook, checkAdUnitSetupHook, checkVideoBidSetupHook, adpodSetConfig } from 'modules/adpod'; let expect = require('chai').expect; @@ -22,13 +23,19 @@ describe('adpod.js', function () { let callbackFn = function() { callbackResult = true; + }; + + let auctionInstance = { + getAuctionStatus: function() { + return auction.AUCTION_IN_PROGRESS; + } } const fakeStoreFn = function(bids, callback) { let payload = []; bids.forEach(bid => payload.push({uuid: bid.customCacheKey})); callback(null, payload); - } + }; beforeEach(function() { callbackResult = null; @@ -72,7 +79,7 @@ describe('adpod.js', function () { } } - callPrebidCacheHook(callbackFn, this, bid, function () {}, bidderRequest); + callPrebidCacheHook(callbackFn, auctionInstance, bid, function () {}, bidderRequest); expect(callbackResult).to.equal(true); }); @@ -127,8 +134,8 @@ describe('adpod.js', function () { } }; - callPrebidCacheHook(callbackFn, this, bidResponse1, afterBidAddedSpy, bidderRequest); - callPrebidCacheHook(callbackFn, this, bidResponse2, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, auctionInstance, bidResponse1, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, auctionInstance, bidResponse2, afterBidAddedSpy, bidderRequest); expect(callbackResult).to.be.null; expect(afterBidAddedSpy.calledTwice).to.equal(true); @@ -181,7 +188,7 @@ describe('adpod.js', function () { } }; - callPrebidCacheHook(callbackFn, this, bidResponse, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, auctionInstance, bidResponse, afterBidAddedSpy, bidderRequest); clock.tick(31); expect(callbackResult).to.be.null; @@ -260,9 +267,9 @@ describe('adpod.js', function () { } }; - callPrebidCacheHook(callbackFn, this, bidResponse1, afterBidAddedSpy, bidderRequest); - callPrebidCacheHook(callbackFn, this, bidResponse2, afterBidAddedSpy, bidderRequest); - callPrebidCacheHook(callbackFn, this, bidResponse3, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, auctionInstance, bidResponse1, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, auctionInstance, bidResponse2, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, auctionInstance, bidResponse3, afterBidAddedSpy, bidderRequest); clock.next(); expect(callbackResult).to.be.null; @@ -276,11 +283,11 @@ describe('adpod.js', function () { expect(auctionBids[1].adId).to.equal('multi_ad2'); expect(auctionBids[1].customCacheKey).to.exist.and.to.match(/^15\.00_news_15s_.*/); expect(auctionBids[1].adserverTargeting.hb_pb_cat_dur).to.equal('15.00_news_15s'); - expect(auctionBids[1].adserverTargeting.hb_cache_id).to.exist.and.to.equal(auctionBids[0].adserverTargeting.hb_uuid); + expect(auctionBids[1].adserverTargeting.hb_cache_id).to.exist.and.to.equal(auctionBids[0].adserverTargeting.hb_cache_id); expect(auctionBids[2].adId).to.equal('multi_ad3'); expect(auctionBids[2].customCacheKey).to.exist.and.to.match(/^10\.00_sports_15s_.*/); expect(auctionBids[2].adserverTargeting.hb_pb_cat_dur).to.equal('10.00_sports_15s'); - expect(auctionBids[2].adserverTargeting.hb_cache_id).to.exist.and.to.equal(auctionBids[0].adserverTargeting.hb_uuid); + expect(auctionBids[2].adserverTargeting.hb_cache_id).to.exist.and.to.equal(auctionBids[0].adserverTargeting.hb_cache_id); }); it('should not add bid to auction when Prebid Cache detects an existing key', function () { @@ -341,8 +348,8 @@ describe('adpod.js', function () { } }; - callPrebidCacheHook(callbackFn, this, bidResponse1, afterBidAddedSpy, bidderRequest); - callPrebidCacheHook(callbackFn, this, bidResponse2, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, auctionInstance, bidResponse1, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, auctionInstance, bidResponse2, afterBidAddedSpy, bidderRequest); expect(callbackResult).to.be.null; expect(afterBidAddedSpy.calledOnce).to.equal(true); @@ -410,8 +417,8 @@ describe('adpod.js', function () { } }; - callPrebidCacheHook(callbackFn, this, bidResponse1, afterBidAddedSpy, bidderRequest); - callPrebidCacheHook(callbackFn, this, bidResponse2, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, auctionInstance, bidResponse1, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, auctionInstance, bidResponse2, afterBidAddedSpy, bidderRequest); expect(doCallbacksIfTimedoutStub.calledTwice).to.equal(true); expect(logWarnStub.calledOnce).to.equal(true); From 92823ccda5595306ddb2fd649472ae0320bdca40 Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Wed, 6 Feb 2019 15:38:24 -0500 Subject: [PATCH 15/21] updates based on initial feedback --- modules/adpod.js | 33 +++++++++++---------------------- test/spec/modules/adpod_spec.js | 2 +- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/modules/adpod.js b/modules/adpod.js index 2224d357092..9d6b7f657bd 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -25,7 +25,6 @@ const from = require('core-js/library/fn/array/from'); export const TARGETING_KEY_PB_CAT_DUR = 'hb_pb_cat_dur'; export const TARGETING_KEY_CACHE_ID = 'hb_cache_id' -// NOTE - are these good defaults? let queueTimeDelay = 50; let queueSizeLimit = 5; let bidCacheRegistry = createBidCacheRegistry(); @@ -102,11 +101,10 @@ function createDispatcher(timeoutDuration) { * @param {Object} bid bid object to update */ function attachPriceIndustryDurationKeyToBid(bid) { - let category = bid.meta && bid.meta.adServerCatId; - let duration = bid.video && bid.video.durationBucket; + let category = utils.deepAccess(bid, 'meta.adServerCatId'); + let duration = utils.deepAccess(bid, 'video.durationBucket'); let cpmFixed = bid.cpm.toFixed(2); let initialCacheKey = bidCacheRegistry.getInitialCacheKey(bid); - // TODO? - add check to verify all above values are populated and throw error if not? let pcd = `${cpmFixed}_${category}_${duration}s`; if (!bid.adserverTargeting) { @@ -154,15 +152,10 @@ function removeBidsFromStorage(bidResponses) { * @param {Function} afterBidAdded callback function used when Prebid Cache responds */ function firePrebidCacheCall(auctionInstance, bidList, afterBidAdded) { - // note to reviewers - was doing this to ensure the variable wouldn't be malformed by the additional incoming bids - // while the current list is being processed by PBC and the store callback - // ...but is this copy really needed? - let bidListCopy = bidList.slice(); - // remove entries now so other incoming bids won't accidentally have a stale version of the list while PBC is processing the current submitted list - removeBidsFromStorage(bidListCopy); + removeBidsFromStorage(bidList); - store(bidListCopy, function (error, cacheIds) { + store(bidList, function (error, cacheIds) { if (error) { utils.logWarn(`Failed to save to the video cache: ${error}. Video bid(s) must be discarded.`); for (let i = 0; i < bidList.length; i++) { @@ -171,12 +164,10 @@ function firePrebidCacheCall(auctionInstance, bidList, afterBidAdded) { } else { for (let i = 0; i < cacheIds.length; i++) { // when uuid in response is empty string then the key already existed, so this bid wasn't cached - // TODO? - should we throw warning here? should we call the afterBidAdded() anyway to decrement the internal bid counter? - // TODO? - verify the cacheKey is one of the expected values (eg in case PBC returns generic UUID)? if (cacheIds[i].uuid !== '') { - addBidToAuction(auctionInstance, bidListCopy[i]); - afterBidAdded(); + addBidToAuction(auctionInstance, bidList[i]); } + afterBidAdded(); } } }); @@ -191,7 +182,7 @@ function firePrebidCacheCall(auctionInstance, bidList, afterBidAdded) { * @param {Object} bidderRequest copy of bid's associated bidderRequest object */ export function callPrebidCacheHook(fn, auctionInstance, bidResponse, afterBidAdded, bidderRequest) { - let videoConfig = bidderRequest.mediaTypes && bidderRequest.mediaTypes.video; + let videoConfig = utils.deepAccess(bidderRequest, 'mediaTypes.video'); if (videoConfig && videoConfig.context === ADPOD) { bidCacheRegistry.addBid(bidResponse); attachPriceIndustryDurationKeyToBid(bidResponse); @@ -211,8 +202,8 @@ export function callPrebidCacheHook(fn, auctionInstance, bidResponse, afterBidAd */ export function checkAdUnitSetupHook(fn, adUnits) { let goodAdUnits = adUnits.filter(adUnit => { - let mediaTypes = adUnit.mediaTypes; - let videoConfig = mediaTypes && mediaTypes.video; + let mediaTypes = utils.deepAccess(adUnit, 'mediaTypes'); + let videoConfig = utils.deepAccess(mediaTypes, 'video'); if (videoConfig && videoConfig.context === ADPOD) { // run check to see if other mediaTypes are defined (ie multi-format); reject adUnit if so if (Object.keys(mediaTypes).length > 1) { @@ -254,8 +245,8 @@ export function checkAdUnitSetupHook(fn, adUnits) { * @returns {boolean} return false if bid duration is deemed invalid as per adUnit configuration; return true if fine */ function checkBidDuration(bidderRequest, bidResponse) { - let bidDuration = bidResponse.video && bidResponse.video.durationSeconds; - let videoConfig = bidderRequest.mediaTypes && bidderRequest.mediaTypes.video; + let bidDuration = utils.deepAccess(bidResponse, 'video.durationSeconds'); + let videoConfig = utils.deepAccess(bidderRequest, 'mediaTypes.video'); let adUnitRanges = videoConfig.durationRangeSec; adUnitRanges.sort((a, b) => a - b); // ensure the ranges are sorted in numeric order @@ -329,7 +320,6 @@ export function checkVideoBidSetupHook(fn, bid, bidRequest, videoMediaType, cont export function adpodSetConfig(config) { if (config.bidQueueTimeDelay !== undefined) { if (typeof config.bidQueueTimeDelay === 'number' && config.bidQueueTimeDelay > 0) { - // add check to see if config setting is too high? queueTimeDelay = config.bidQueueTimeDelay; } else { utils.logWarn(`Detected invalid value for adpod.bidQueueTimeDelay in setConfig; must be a positive number. Using default: ${queueTimeDelay}`) @@ -338,7 +328,6 @@ export function adpodSetConfig(config) { if (config.bidQueueSizeLimit !== undefined) { if (typeof config.bidQueueSizeLimit === 'number' && config.bidQueueSizeLimit > 0) { - // add check to see if config setting is too high? too low? queueSizeLimit = config.bidQueueSizeLimit; } else { utils.logWarn(`Detected invalid value for adpod.bidQueueSizeLimit in setConfig; must be a positive number. Using default: ${queueSizeLimit}`) diff --git a/test/spec/modules/adpod_spec.js b/test/spec/modules/adpod_spec.js index 6c052a48a7a..6a3a107fafb 100644 --- a/test/spec/modules/adpod_spec.js +++ b/test/spec/modules/adpod_spec.js @@ -352,7 +352,7 @@ describe('adpod.js', function () { callPrebidCacheHook(callbackFn, auctionInstance, bidResponse2, afterBidAddedSpy, bidderRequest); expect(callbackResult).to.be.null; - expect(afterBidAddedSpy.calledOnce).to.equal(true); + expect(afterBidAddedSpy.calledTwice).to.equal(true); expect(storeStub.calledOnce).to.equal(true); expect(auctionBids.length).to.equal(1); expect(auctionBids[0].adId).to.equal('dup_ad_1'); From b74d7a37ed3b46f2a3577b8d646b5d1370e40b84 Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Thu, 7 Feb 2019 09:15:18 -0500 Subject: [PATCH 16/21] add msg when PBC rejects bid due to duplicate key --- modules/adpod.js | 2 ++ test/spec/modules/adpod_spec.js | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/modules/adpod.js b/modules/adpod.js index 9d6b7f657bd..c180bfac2e9 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -166,6 +166,8 @@ function firePrebidCacheCall(auctionInstance, bidList, afterBidAdded) { // when uuid in response is empty string then the key already existed, so this bid wasn't cached if (cacheIds[i].uuid !== '') { addBidToAuction(auctionInstance, bidList[i]); + } else { + utils.logInfo(`Detected a bid was not cached because the custom key was already registered. Attempted to use key: ${bidList[i].customCacheKey}. Bid was: `, bidList[i]); } afterBidAdded(); } diff --git a/test/spec/modules/adpod_spec.js b/test/spec/modules/adpod_spec.js index 6a3a107fafb..7c89a9c57e8 100644 --- a/test/spec/modules/adpod_spec.js +++ b/test/spec/modules/adpod_spec.js @@ -11,6 +11,7 @@ let expect = require('chai').expect; describe('adpod.js', function () { let logErrorStub; let logWarnStub; + let logInfoStub; describe('callPrebidCacheHook', function () { let callbackResult; @@ -42,6 +43,7 @@ describe('adpod.js', function () { afterBidAddedSpy = sinon.spy(); storeStub = sinon.stub(videoCache, 'store'); logWarnStub = sinon.stub(utils, 'logWarn'); + logInfoStub = sinon.stub(utils, 'logInfo'); addBidToAuctionStub = sinon.stub(auction, 'addBidToAuction').callsFake(function (auctionInstance, bid) { auctionBids.push(bid); }); @@ -57,6 +59,7 @@ describe('adpod.js', function () { afterEach(function() { storeStub.restore(); logWarnStub.restore(); + logInfoStub.restore(); addBidToAuctionStub.restore(); doCallbacksIfTimedoutStub.restore(); clock.restore(); @@ -354,6 +357,7 @@ describe('adpod.js', function () { expect(callbackResult).to.be.null; expect(afterBidAddedSpy.calledTwice).to.equal(true); expect(storeStub.calledOnce).to.equal(true); + expect(logInfoStub.calledOnce).to.equal(true); expect(auctionBids.length).to.equal(1); expect(auctionBids[0].adId).to.equal('dup_ad_1'); expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^5\.00_tech_45s_.*/); From de58a2bdfa9d3bf781c3c44e24928855d81c0f66 Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Fri, 8 Feb 2019 15:54:25 -0500 Subject: [PATCH 17/21] fix issue in videoBidCheck hook --- modules/adpod.js | 13 +++++----- test/spec/modules/adpod_spec.js | 43 +++++++++++++++++++-------------- 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/modules/adpod.js b/modules/adpod.js index c180bfac2e9..e6e3bfbabae 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -284,20 +284,21 @@ function checkBidDuration(bidderRequest, bidResponse) { */ export function checkVideoBidSetupHook(fn, bid, bidRequest, videoMediaType, context) { if (context === ADPOD) { + let result = true; if (!utils.deepAccess(bid, 'meta.iabSubCatId')) { - return false; + result = false; } if (utils.deepAccess(bid, 'video')) { if (!utils.deepAccess(bid, 'video.context') || bid.video.context !== ADPOD) { - return false; + result = false; } if (!utils.deepAccess(bid, 'video.durationSeconds') || bid.video.durationSeconds <= 0) { - return false; + result = false; } else { let isBidGood = checkBidDuration(bidRequest, bid); - if (!isBidGood) return false; + if (!isBidGood) result = false; } } @@ -306,10 +307,10 @@ export function checkVideoBidSetupHook(fn, bid, bidRequest, videoMediaType, cont This bid contains only vastXml and will not work when a prebid cache url is not specified. Try enabling prebid cache with pbjs.setConfig({ cache: {url: "..."} }); `); - return false; + result = false; }; - return true; + fn.bail(result); } else { fn.call(this, bid, bidRequest, videoMediaType, context); } diff --git a/test/spec/modules/adpod_spec.js b/test/spec/modules/adpod_spec.js index 7c89a9c57e8..e2fc171bbd4 100644 --- a/test/spec/modules/adpod_spec.js +++ b/test/spec/modules/adpod_spec.js @@ -527,9 +527,15 @@ describe('adpod.js', function () { describe('checkVideoBidSetupHook', function () { let callbackResult; - const callbackFn = function (bid) { - callbackResult = bid; - }; + let bailResult; + const callbackFn = { + call: function(context, bid) { + callbackResult = bid; + }, + bail: function(result) { + bailResult = result; + } + } const adpodTestBid = { video: { context: ADPOD, @@ -566,6 +572,7 @@ describe('adpod.js', function () { beforeEach(function() { callbackResult = null; + bailResult = null; config.setConfig({ cache: { url: 'http://test.cache.url/endpoint' @@ -585,28 +592,27 @@ describe('adpod.js', function () { let bannerTestBid = { mediaType: 'video' }; - let hookReturnValue = checkVideoBidSetupHook(callbackFn, bannerTestBid, {}, {}, 'instream'); + checkVideoBidSetupHook(callbackFn, bannerTestBid, {}, {}, 'instream'); expect(callbackResult).to.deep.equal(bannerTestBid); - expect(hookReturnValue).to.be.undefined; + expect(bailResult).to.be.null; expect(logErrorStub.called).to.equal(false); }); it('returns true when adpod bid is properly setup', function() { let goodBid = utils.deepClone(adpodTestBid); - let hookReturnValue = checkVideoBidSetupHook(callbackFn, goodBid, bidderRequestNoExact, {}, ADPOD); + checkVideoBidSetupHook(callbackFn, goodBid, bidderRequestNoExact, {}, ADPOD); expect(callbackResult).to.be.null; - expect(hookReturnValue).to.equal(true); + expect(bailResult).to.equal(true); expect(logErrorStub.called).to.equal(false); }); it('returns false when a required property from an adpod bid is missing', function() { function testInvalidAdpodBid(badTestBid, shouldErrorBeLogged) { - let hookReturnValue = checkVideoBidSetupHook(callbackFn, badTestBid, bidderRequestNoExact, {}, ADPOD); + checkVideoBidSetupHook(callbackFn, badTestBid, bidderRequestNoExact, {}, ADPOD); expect(callbackResult).to.be.null; - expect(hookReturnValue).to.equal(false); + expect(bailResult).to.equal(false); expect(logErrorStub.called).to.equal(shouldErrorBeLogged); } - config.resetConfig(); let noCatBid = utils.deepClone(adpodTestBid); delete noCatBid.meta; @@ -624,6 +630,7 @@ describe('adpod.js', function () { delete noDurationBid.video.durationSeconds; testInvalidAdpodBid(noDurationBid, false); + config.resetConfig(); let noCacheUrlBid = utils.deepClone(adpodTestBid); testInvalidAdpodBid(noCacheUrlBid, true); }); @@ -642,29 +649,29 @@ describe('adpod.js', function () { it('when requireExactDuration is true', function() { let goodBid = utils.deepClone(basicBid); - let hookReturnValue = checkVideoBidSetupHook(callbackFn, goodBid, bidderRequestWithExact, {}, ADPOD); + checkVideoBidSetupHook(callbackFn, goodBid, bidderRequestWithExact, {}, ADPOD); expect(callbackResult).to.be.null; expect(goodBid.video.durationBucket).to.equal(30); - expect(hookReturnValue).to.equal(true); + expect(bailResult).to.equal(true); expect(logWarnStub.called).to.equal(false); let badBid = utils.deepClone(basicBid); badBid.video.durationSeconds = 14; - hookReturnValue = checkVideoBidSetupHook(callbackFn, badBid, bidderRequestWithExact, {}, ADPOD); + checkVideoBidSetupHook(callbackFn, badBid, bidderRequestWithExact, {}, ADPOD); expect(callbackResult).to.be.null; expect(badBid.video.durationBucket).to.be.undefined; - expect(hookReturnValue).to.equal(false); + expect(bailResult).to.equal(false); expect(logWarnStub.calledOnce).to.equal(true); }); it('when requireExactDuration is false and bids are bucketed properly', function() { function testRoundingForGoodBId(bid, bucketValue) { - let hookReturnValue = checkVideoBidSetupHook(callbackFn, bid, bidderRequestNoExact, {}, ADPOD); + checkVideoBidSetupHook(callbackFn, bid, bidderRequestNoExact, {}, ADPOD); expect(callbackResult).to.be.null; expect(bid.video.durationBucket).to.equal(bucketValue); - expect(hookReturnValue).to.equal(true); + expect(bailResult).to.equal(true); expect(logWarnStub.called).to.equal(false); } @@ -685,10 +692,10 @@ describe('adpod.js', function () { let badBid100 = utils.deepClone(basicBid); badBid100.video.durationSeconds = 100; - let hookReturnValue = checkVideoBidSetupHook(callbackFn, badBid100, bidderRequestNoExact, {}, ADPOD); + checkVideoBidSetupHook(callbackFn, badBid100, bidderRequestNoExact, {}, ADPOD); expect(callbackResult).to.be.null; expect(badBid100.video.durationBucket).to.be.undefined; - expect(hookReturnValue).to.equal(false); + expect(bailResult).to.equal(false); expect(logWarnStub.called).to.equal(true); }); }); From ad1c678e5ff9120e20b0d92855e218d241f15432 Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Wed, 20 Feb 2019 16:12:07 -0500 Subject: [PATCH 18/21] add buffer logic when rounding bidDuration --- modules/adpod.js | 22 +++++++++++++--------- test/spec/modules/adpod_spec.js | 31 +++++++++++++++++++++++-------- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/modules/adpod.js b/modules/adpod.js index e6e3bfbabae..b8b9fbbc6e2 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -1,6 +1,6 @@ /** * This module houses the functionality to evaluate and process adpod adunits/bids. Specifically there are several hooked functions, - * that either supplement the base function (ie to check something additional unique to adpod objects) or to replace the base funtion + * that either supplement the base function (ie to check something additional or unique to adpod objects) or to replace the base funtion * entirely when appropriate. * * Brief outline of each hook: @@ -63,7 +63,7 @@ function createBidCacheRegistry() { /** * Creates a function that when called updates the bid queue and extends the running timer (when called subsequently). - * Once the threshold for the queue (defined by queueSizeLimit), the queue will be flushed by calling the `firePrebidCacheCall` function. + * Once the time threshold for the queue (defined by queueSizeLimit) is reached, the queue will be flushed by calling the `firePrebidCacheCall` function. * If there is a long enough time between calls (based on timeoutDration), the queue will automatically flush itself. * @param {Number} timeoutDuration number of milliseconds to pass before timer expires and current bid queue is flushed * @returns {Function} @@ -146,7 +146,9 @@ function removeBidsFromStorage(bidResponses) { } /** - * + * This function will send a list of bids to Prebid Cache. It also removes the same bids from the internal bidCacheRegistry + * to maintain which bids are in queue. + * If the bids are successfully cached, they will be added to the respective auction. * @param {*} auctionInstance running context of the auction * @param {Array[Object]} bidList list of bid objects that need to be sent to Prebid Cache * @param {Function} afterBidAdded callback function used when Prebid Cache responds @@ -196,7 +198,7 @@ export function callPrebidCacheHook(fn, auctionInstance, bidResponse, afterBidAd } /** - * This hook function will review the adUnit setup and verify certain required are present in any adpod adUnits. + * This hook function will review the adUnit setup and verify certain required values are present in any adpod adUnits. * If the fields are missing or incorrectly setup, the adUnit is removed from the list. * @param {Function} fn reference to original function (used by hook logic) * @param {Array[Object]} adUnits list of adUnits to be evaluated @@ -239,14 +241,16 @@ export function checkAdUnitSetupHook(fn, adUnits) { * - only bids that exactly match those listed values are accepted (don't round at all). * - populate the `bid.video.durationBucket` field with the matching duration value * when adUnit.mediaTypes.video.requireExactDuration is false - * - round the duration to the next highest specified duration value based on adunit (eg if range was [5, 15, 30] -> 3s is rounded to 5s; 16s is rounded to 30s) - * - if the bid is above the range of thje listed durations, reject the bid + * - round the duration to the next highest specified duration value based on adunit. If the duration is above a range within a set buffer, that bid falls down into that bucket. + * (eg if range was [5, 15, 30] -> 2s is rounded to 5s; 17s is rounded back to 15s; 18s is rounded up to 30s) + * - if the bid is above the range of the listed durations (and outside the buffer), reject the bid * - set the rounded duration value in the `bid.video.durationBucket` field for accepted bids * @param {Object} bidderRequest copy of the bidderRequest object associated to bidResponse * @param {Object} bidResponse incoming bidResponse being evaluated by bidderFactory * @returns {boolean} return false if bid duration is deemed invalid as per adUnit configuration; return true if fine */ function checkBidDuration(bidderRequest, bidResponse) { + const buffer = 2; let bidDuration = utils.deepAccess(bidResponse, 'video.durationSeconds'); let videoConfig = utils.deepAccess(bidderRequest, 'mediaTypes.video'); let adUnitRanges = videoConfig.durationRangeSec; @@ -254,8 +258,8 @@ function checkBidDuration(bidderRequest, bidResponse) { if (!videoConfig.requireExactDuration) { let max = Math.max(...adUnitRanges); - if (bidDuration <= max) { - let nextHighestRange = find(adUnitRanges, range => range >= bidDuration); + if (bidDuration <= (max + buffer)) { + let nextHighestRange = find(adUnitRanges, range => (range + buffer) >= bidDuration); bidResponse.video.durationBucket = nextHighestRange; } else { utils.logWarn(`Detected a bid with a duration value outside the accepted ranges specified in adUnit.mediaTypes.video.durationRangeSec. Rejecting bid: `, bidResponse); @@ -274,7 +278,7 @@ function checkBidDuration(bidderRequest, bidResponse) { /** * This hooked function evaluates an adpod bid and determines if the required fields are present. - * If it's found to not be an adpod bid, it return to original function via hook logic + * If it's found to not be an adpod bid, it will return to original function via hook logic * @param {Function} fn reference to original function (used by hook logic) * @param {Object} bid incoming bid object * @param {Object} bidRequest bidRequest object of associated bid diff --git a/test/spec/modules/adpod_spec.js b/test/spec/modules/adpod_spec.js index e2fc171bbd4..06f0b98ebf9 100644 --- a/test/spec/modules/adpod_spec.js +++ b/test/spec/modules/adpod_spec.js @@ -683,20 +683,35 @@ describe('adpod.js', function () { goodBid30.video.durationSeconds = 30; testRoundingForGoodBId(goodBid30, 45); - let goodBid14 = utils.deepClone(basicBid); - goodBid14.video.durationSeconds = 14; - testRoundingForGoodBId(goodBid14, 15); + let goodBid10 = utils.deepClone(basicBid); + goodBid10.video.durationSeconds = 10; + testRoundingForGoodBId(goodBid10, 15); + + let goodBid16 = utils.deepClone(basicBid); + goodBid16.video.durationSeconds = 16; + testRoundingForGoodBId(goodBid16, 15); + + let goodBid47 = utils.deepClone(basicBid); + goodBid47.video.durationSeconds = 47; + testRoundingForGoodBId(goodBid47, 45); }); it('when requireExactDuration is false and bid duration exceeds listed buckets', function() { + function testRoundingForBadBid(bid) { + checkVideoBidSetupHook(callbackFn, bid, bidderRequestNoExact, {}, ADPOD); + expect(callbackResult).to.be.null; + expect(bid.video.durationBucket).to.be.undefined; + expect(bailResult).to.equal(false); + expect(logWarnStub.called).to.equal(true); + } + let badBid100 = utils.deepClone(basicBid); badBid100.video.durationSeconds = 100; + testRoundingForBadBid(badBid100); - checkVideoBidSetupHook(callbackFn, badBid100, bidderRequestNoExact, {}, ADPOD); - expect(callbackResult).to.be.null; - expect(badBid100.video.durationBucket).to.be.undefined; - expect(bailResult).to.equal(false); - expect(logWarnStub.called).to.equal(true); + let badBid48 = utils.deepClone(basicBid); + badBid48.video.durationSeconds = 48; + testRoundingForBadBid(badBid48); }); }); }); From 1e1e49b7bc3643f2a62d8fea0d4f5fcf8ce7b554 Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Mon, 25 Feb 2019 14:25:03 -0500 Subject: [PATCH 19/21] add logic for deferCaching --- modules/adpod.js | 58 +++++++++++++++++++--- test/spec/modules/adpod_spec.js | 85 +++++++++++++++++++++++++++++++-- 2 files changed, 131 insertions(+), 12 deletions(-) diff --git a/modules/adpod.js b/modules/adpod.js index b8b9fbbc6e2..e2b1d0e3a42 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -35,14 +35,19 @@ let bidCacheRegistry = createBidCacheRegistry(); */ function createBidCacheRegistry() { let registry = {}; + + function setupRegistrySlot(auctionId) { + registry[auctionId] = {}; + registry[auctionId].bidStorage = new Set(); + registry[auctionId].queueDispatcher = createDispatcher(queueTimeDelay); + registry[auctionId].initialCacheKey = utils.generateUUID(); + } + return { addBid: function (bid) { // create parent level object based on auction ID (in case there are concurrent auctions running) to store objects for that auction if (!registry[bid.auctionId]) { - registry[bid.auctionId] = {}; - registry[bid.auctionId].bidStorage = new Set(); - registry[bid.auctionId].queueDispatcher = createDispatcher(queueTimeDelay); - registry[bid.auctionId].initialCacheKey = utils.generateUUID(); + setupRegistrySlot(bid.auctionId); } registry[bid.auctionId].bidStorage.add(bid); }, @@ -55,6 +60,12 @@ function createBidCacheRegistry() { getQueueDispatcher: function(bid) { return registry[bid.auctionId] && registry[bid.auctionId].queueDispatcher; }, + setupInitialCacheKey: function(bid) { + if (!registry[bid.auctionId]) { + registry[bid.auctionId] = {}; + registry[bid.auctionId].initialCacheKey = utils.generateUUID(); + } + }, getInitialCacheKey: function(bid) { return registry[bid.auctionId] && registry[bid.auctionId].initialCacheKey; } @@ -188,10 +199,20 @@ function firePrebidCacheCall(auctionInstance, bidList, afterBidAdded) { export function callPrebidCacheHook(fn, auctionInstance, bidResponse, afterBidAdded, bidderRequest) { let videoConfig = utils.deepAccess(bidderRequest, 'mediaTypes.video'); if (videoConfig && videoConfig.context === ADPOD) { - bidCacheRegistry.addBid(bidResponse); - attachPriceIndustryDurationKeyToBid(bidResponse); + if (config.getConfig('adpod.deferCaching') === false) { + bidCacheRegistry.addBid(bidResponse); + attachPriceIndustryDurationKeyToBid(bidResponse); + + updateBidQueue(auctionInstance, bidResponse, afterBidAdded); + } else { + // generate targeting keys for bid + bidCacheRegistry.setupInitialCacheKey(bidResponse); + attachPriceIndustryDurationKeyToBid(bidResponse); - updateBidQueue(auctionInstance, bidResponse, afterBidAdded); + // add bid to auction + addBidToAuction(auctionInstance, bidResponse); + afterBidAdded(); + } } else { fn.call(this, auctionInstance, bidResponse, afterBidAdded, bidderRequest); } @@ -358,3 +379,26 @@ export function initAdpodHooks() { setupHookFnOnce('checkAdUnitSetup', checkAdUnitSetupHook); setupHookFnOnce('checkVideoBidSetup', checkVideoBidSetupHook); } + +/** + * + * @param {Array[Object]} bids list of 'winning' bids that need to be cached + * @param {Function} callback send the cached bids (or error) back to adserverVideoModule for further processing + }} + */ +export function callPrebidCacheAfterAuction(bids, callback) { + // will call PBC here and execute cb param to initialize player code + store(bids, function(error, cacheIds) { + if (error) { + callback(error, null); + } else { + let successfulCachedBids = []; + for (let i = 0; i < cacheIds.length; i++) { + if (cacheIds[i] !== '') { + successfulCachedBids.push(bids[i]); + } + } + callback(null, successfulCachedBids); + } + }) +} diff --git a/test/spec/modules/adpod_spec.js b/test/spec/modules/adpod_spec.js index 06f0b98ebf9..98909fd11b0 100644 --- a/test/spec/modules/adpod_spec.js +++ b/test/spec/modules/adpod_spec.js @@ -86,12 +86,83 @@ describe('adpod.js', function () { expect(callbackResult).to.equal(true); }); + it('should immediately add the adpod bid to auction if adpod.deferCaching in config is true', function() { + config.setConfig({ + adpod: { + deferCaching: true + } + }); + + let bidResponse1 = { + adId: 'adId01277', + auctionId: 'no_defer_123', + mediaType: 'video', + cpm: 5, + meta: { + adServerCatId: 'test' + }, + video: { + context: ADPOD, + durationSeconds: 15, + durationBucket: 15 + } + }; + + let bidResponse2 = { + adId: 'adId46547', + auctionId: 'no_defer_123', + mediaType: 'video', + cpm: 12, + meta: { + adServerCatId: 'value' + }, + video: { + context: ADPOD, + durationSeconds: 15, + durationBucket: 15 + } + }; + + let bidderRequest = { + adUnitCode: 'adpod_1', + auctionId: 'no_defer_123', + mediaTypes: { + video: { + context: ADPOD, + playerSize: [300, 300], + adPodDurationSec: 300, + durationRangeSec: [15, 30, 45], + requireExactDuration: false + } + }, + }; + + callPrebidCacheHook(callbackFn, auctionInstance, bidResponse1, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, auctionInstance, bidResponse2, afterBidAddedSpy, bidderRequest); + + // check if bid adsereverTargeting is setup + expect(callbackResult).to.be.null; + expect(storeStub.called).to.equal(false); + expect(afterBidAddedSpy.calledTwice).to.equal(true); + expect(auctionBids.length).to.equal(2); + expect(auctionBids[0].adId).to.equal(bidResponse1.adId); + expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^5\.00_test_15s_.*/); + expect(auctionBids[0].adserverTargeting.hb_pb_cat_dur).to.equal('5.00_test_15s'); + expect(auctionBids[0].adserverTargeting.hb_cache_id).to.exist; + expect(auctionBids[1].adId).to.equal(bidResponse2.adId); + expect(auctionBids[1].customCacheKey).to.exist.and.to.match(/^12\.00_value_15s_.*/); + expect(auctionBids[1].adserverTargeting.hb_pb_cat_dur).to.equal('12.00_value_15s'); + expect(auctionBids[1].adserverTargeting.hb_cache_id).to.exist; + expect(auctionBids[1].adserverTargeting.hb_cache_id).to.equal(auctionBids[0].adserverTargeting.hb_cache_id); + }); + it('should send prebid cache call once bid queue is full', function () { storeStub.callsFake(fakeStoreFn); config.setConfig({ adpod: { - bidQueueSizeLimit: 2 + bidQueueSizeLimit: 2, + deferCaching: false } }); @@ -159,7 +230,8 @@ describe('adpod.js', function () { config.setConfig({ adpod: { bidQueueSizeLimit: 2, - bidQueueTimeDelay: 30 + bidQueueTimeDelay: 30, + deferCaching: false } }); @@ -209,7 +281,8 @@ describe('adpod.js', function () { config.setConfig({ adpod: { bidQueueSizeLimit: 2, - bidQueueTimeDelay: 30 + bidQueueTimeDelay: 30, + deferCaching: false } }); @@ -305,7 +378,8 @@ describe('adpod.js', function () { config.setConfig({ adpod: { - bidQueueSizeLimit: 2 + bidQueueSizeLimit: 2, + deferCaching: false } }); @@ -375,7 +449,8 @@ describe('adpod.js', function () { config.setConfig({ adpod: { - bidQueueSizeLimit: 2 + bidQueueSizeLimit: 2, + deferCaching: false } }); From b60cea77ae5d4d289025bfccfb1498e786fe4cf9 Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Tue, 26 Feb 2019 11:37:11 -0500 Subject: [PATCH 20/21] update logic around brandCategoryExclusion --- modules/adpod.js | 26 ++++-- test/spec/modules/adpod_spec.js | 140 ++++++++++++++++++++++++++++++-- 2 files changed, 154 insertions(+), 12 deletions(-) diff --git a/modules/adpod.js b/modules/adpod.js index e2b1d0e3a42..cc5bb9729e4 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -110,13 +110,20 @@ function createDispatcher(timeoutDuration) { /** * This function reads certain fields from the bid to generate a specific key used for caching the bid in Prebid Cache * @param {Object} bid bid object to update + * @param {Boolean} brandCategoryExclusion value read from setConfig; influences whether category is required or not */ -function attachPriceIndustryDurationKeyToBid(bid) { - let category = utils.deepAccess(bid, 'meta.adServerCatId'); +function attachPriceIndustryDurationKeyToBid(bid, brandCategoryExclusion) { + let initialCacheKey = bidCacheRegistry.getInitialCacheKey(bid); let duration = utils.deepAccess(bid, 'video.durationBucket'); let cpmFixed = bid.cpm.toFixed(2); - let initialCacheKey = bidCacheRegistry.getInitialCacheKey(bid); - let pcd = `${cpmFixed}_${category}_${duration}s`; + let pcd; + + if (brandCategoryExclusion) { + let category = utils.deepAccess(bid, 'meta.adServerCatId'); + pcd = `${cpmFixed}_${category}_${duration}s`; + } else { + pcd = `${cpmFixed}_${duration}s`; + } if (!bid.adserverTargeting) { bid.adserverTargeting = {}; @@ -199,15 +206,22 @@ function firePrebidCacheCall(auctionInstance, bidList, afterBidAdded) { export function callPrebidCacheHook(fn, auctionInstance, bidResponse, afterBidAdded, bidderRequest) { let videoConfig = utils.deepAccess(bidderRequest, 'mediaTypes.video'); if (videoConfig && videoConfig.context === ADPOD) { + let brandCategoryExclusion = config.getConfig('adpod.brandCategoryExclusion'); + let adServerCatId = utils.deepAccess(bidResponse, 'meta.adServerCatId'); + if (!adServerCatId && brandCategoryExclusion) { + utils.logWarn('Detected a bid without meta.adServerCatId while setConfig({adpod.brandCategoryExclusion}) was enabled. This bid has been rejected:', bidResponse) + afterBidAdded(); + } + if (config.getConfig('adpod.deferCaching') === false) { bidCacheRegistry.addBid(bidResponse); - attachPriceIndustryDurationKeyToBid(bidResponse); + attachPriceIndustryDurationKeyToBid(bidResponse, brandCategoryExclusion); updateBidQueue(auctionInstance, bidResponse, afterBidAdded); } else { // generate targeting keys for bid bidCacheRegistry.setupInitialCacheKey(bidResponse); - attachPriceIndustryDurationKeyToBid(bidResponse); + attachPriceIndustryDurationKeyToBid(bidResponse, brandCategoryExclusion); // add bid to auction addBidToAuction(auctionInstance, bidResponse); diff --git a/test/spec/modules/adpod_spec.js b/test/spec/modules/adpod_spec.js index 98909fd11b0..fd8abea310a 100644 --- a/test/spec/modules/adpod_spec.js +++ b/test/spec/modules/adpod_spec.js @@ -89,7 +89,8 @@ describe('adpod.js', function () { it('should immediately add the adpod bid to auction if adpod.deferCaching in config is true', function() { config.setConfig({ adpod: { - deferCaching: true + deferCaching: true, + brandCategoryExclusion: true } }); @@ -162,7 +163,8 @@ describe('adpod.js', function () { config.setConfig({ adpod: { bidQueueSizeLimit: 2, - deferCaching: false + deferCaching: false, + brandCategoryExclusion: true } }); @@ -231,7 +233,8 @@ describe('adpod.js', function () { adpod: { bidQueueSizeLimit: 2, bidQueueTimeDelay: 30, - deferCaching: false + deferCaching: false, + brandCategoryExclusion: true } }); @@ -282,7 +285,8 @@ describe('adpod.js', function () { adpod: { bidQueueSizeLimit: 2, bidQueueTimeDelay: 30, - deferCaching: false + deferCaching: false, + brandCategoryExclusion: true } }); @@ -366,6 +370,128 @@ describe('adpod.js', function () { expect(auctionBids[2].adserverTargeting.hb_cache_id).to.exist.and.to.equal(auctionBids[0].adserverTargeting.hb_cache_id); }); + it('should cache the bids with a shortened custom key when adpod.brandCategoryExclusion is false', function() { + storeStub.callsFake(fakeStoreFn); + + config.setConfig({ + adpod: { + bidQueueSizeLimit: 2, + bidQueueTimeDelay: 30, + deferCaching: false, + brandCategoryExclusion: false + } + }); + + let bidResponse1 = { + adId: 'nocat_ad1', + auctionId: 'no_category_abc345', + mediaType: 'video', + cpm: 10, + meta: { + adServerCatId: undefined + }, + video: { + context: ADPOD, + durationSeconds: 15, + durationBucket: 15 + } + }; + let bidResponse2 = { + adId: 'nocat_ad2', + auctionId: 'no_category_abc345', + mediaType: 'video', + cpm: 15, + meta: { + adServerCatId: undefined + }, + video: { + context: ADPOD, + durationSeconds: 15, + durationBucket: 15 + } + }; + + let bidderRequest = { + adUnitCode: 'adpod_4', + auctionId: 'no_category_abc345', + mediaTypes: { + video: { + context: ADPOD, + playerSize: [300, 300], + adPodDurationSec: 45, + durationRangeSec: [15, 30], + requireExactDuration: false + } + } + }; + + callPrebidCacheHook(callbackFn, auctionInstance, bidResponse1, afterBidAddedSpy, bidderRequest); + callPrebidCacheHook(callbackFn, auctionInstance, bidResponse2, afterBidAddedSpy, bidderRequest); + + expect(callbackResult).to.be.null; + expect(afterBidAddedSpy.calledTwice).to.equal(true); + expect(storeStub.calledOnce).to.equal(true); + expect(auctionBids.length).to.equal(2); + expect(auctionBids[0].adId).to.equal('nocat_ad1'); + expect(auctionBids[0].customCacheKey).to.exist.and.to.match(/^10\.00_15s_.*/); + expect(auctionBids[0].adserverTargeting.hb_pb_cat_dur).to.equal('10.00_15s'); + expect(auctionBids[0].adserverTargeting.hb_cache_id).to.exist; + expect(auctionBids[1].adId).to.equal('nocat_ad2'); + expect(auctionBids[1].customCacheKey).to.exist.and.to.match(/^15\.00_15s_.*/); + expect(auctionBids[1].adserverTargeting.hb_pb_cat_dur).to.equal('15.00_15s'); + expect(auctionBids[1].adserverTargeting.hb_cache_id).to.exist.and.to.equal(auctionBids[0].adserverTargeting.hb_cache_id); + }); + + it('should not add bid to auction when config adpod.brandCategoryExclusion is true but bid is missing adServerCatId', function() { + storeStub.callsFake(fakeStoreFn); + + config.setConfig({ + adpod: { + bidQueueSizeLimit: 2, + bidQueueTimeDelay: 30, + deferCaching: false, + brandCategoryExclusion: true + } + }); + + let bidResponse1 = { + adId: 'missingCat_ad1', + auctionId: 'missing_category_abc345', + mediaType: 'video', + cpm: 10, + meta: { + adServerCatId: undefined + }, + video: { + context: ADPOD, + durationSeconds: 15, + durationBucket: 15 + } + }; + + let bidderRequest = { + adUnitCode: 'adpod_5', + auctionId: 'missing_category_abc345', + mediaTypes: { + video: { + context: ADPOD, + playerSize: [300, 300], + adPodDurationSec: 45, + durationRangeSec: [15, 30], + requireExactDuration: false + } + } + }; + + callPrebidCacheHook(callbackFn, auctionInstance, bidResponse1, afterBidAddedSpy, bidderRequest); + + expect(callbackResult).to.be.null; + expect(afterBidAddedSpy.calledOnce).to.equal(true); + expect(storeStub.called).to.equal(false); + expect(logWarnStub.calledOnce).to.equal(true); + expect(auctionBids.length).to.equal(0); + }); + it('should not add bid to auction when Prebid Cache detects an existing key', function () { storeStub.callsFake(function(bids, callback) { let payload = []; @@ -379,7 +505,8 @@ describe('adpod.js', function () { config.setConfig({ adpod: { bidQueueSizeLimit: 2, - deferCaching: false + deferCaching: false, + brandCategoryExclusion: true } }); @@ -450,7 +577,8 @@ describe('adpod.js', function () { config.setConfig({ adpod: { bidQueueSizeLimit: 2, - deferCaching: false + deferCaching: false, + brandCategoryExclusion: true } }); From 9728f2f659de9bbe86a4fe5ad2673512e2acb633 Mon Sep 17 00:00:00 2001 From: Jason Snellbaker Date: Wed, 27 Feb 2019 11:46:25 -0500 Subject: [PATCH 21/21] update support to new hook api and update logic when checking iab category field --- modules/adpod.js | 22 +++++++++------------- src/auction.js | 2 +- src/hook.js | 7 +++++++ src/prebid.js | 2 +- src/video.js | 2 +- test/spec/modules/adpod_spec.js | 23 ++++++++++++++++++++++- 6 files changed, 41 insertions(+), 17 deletions(-) diff --git a/modules/adpod.js b/modules/adpod.js index cc5bb9729e4..87e5869e17b 100644 --- a/modules/adpod.js +++ b/modules/adpod.js @@ -13,9 +13,11 @@ */ import * as utils from '../src/utils'; -import { addBidToAuction, doCallbacksIfTimedout, AUCTION_IN_PROGRESS } from '../src/auction'; +import { addBidToAuction, doCallbacksIfTimedout, AUCTION_IN_PROGRESS, callPrebidCache } from '../src/auction'; +import { checkAdUnitSetup } from '../src/prebid'; +import { checkVideoBidSetup } from '../src/video'; +import { setupBeforeHookFnOnce } from '../src/hook'; import { store } from '../src/videoCache'; -import { hooks } from '../src/hook'; import { config } from '../src/config'; import { ADPOD } from '../src/mediaTypes'; import Set from 'core-js/library/fn/set'; @@ -324,7 +326,8 @@ function checkBidDuration(bidderRequest, bidResponse) { export function checkVideoBidSetupHook(fn, bid, bidRequest, videoMediaType, context) { if (context === ADPOD) { let result = true; - if (!utils.deepAccess(bid, 'meta.iabSubCatId')) { + let brandCategoryExclusion = config.getConfig('adpod.brandCategoryExclusion'); + if (brandCategoryExclusion && !utils.deepAccess(bid, 'meta.iabSubCatId')) { result = false; } @@ -382,16 +385,9 @@ config.getConfig('adpod', config => adpodSetConfig(config.adpod)); * This function initializes the adpod module's hooks. This is called by the corresponding adserver video module. */ export function initAdpodHooks() { - function setupHookFnOnce(hookId, hookFn, priority = 15) { - let result = hooks[hookId].getHooks({hook: hookFn}); - if (result.length === 0) { - hooks[hookId].before(hookFn, priority); - } - } - - setupHookFnOnce('callPrebidCache', callPrebidCacheHook); - setupHookFnOnce('checkAdUnitSetup', checkAdUnitSetupHook); - setupHookFnOnce('checkVideoBidSetup', checkVideoBidSetupHook); + setupBeforeHookFnOnce(callPrebidCache, callPrebidCacheHook); + setupBeforeHookFnOnce(checkAdUnitSetup, checkAdUnitSetupHook); + setupBeforeHookFnOnce(checkVideoBidSetup, checkVideoBidSetupHook); } /** diff --git a/src/auction.js b/src/auction.js index fc8c42023f7..97f482d6fc3 100644 --- a/src/auction.js +++ b/src/auction.js @@ -416,7 +416,7 @@ function tryAddVideoBid(auctionInstance, bidResponse, bidRequests, afterBidAdded } } -const callPrebidCache = hook('async', function(auctionInstance, bidResponse, afterBidAdded, bidderRequest) { +export const callPrebidCache = hook('async', function(auctionInstance, bidResponse, afterBidAdded, bidderRequest) { store([bidResponse], function (error, cacheIds) { if (error) { utils.logWarn(`Failed to save to the video cache: ${error}. Video bid must be discarded.`); diff --git a/src/hook.js b/src/hook.js index c4e450bf5f2..6d25a0a7430 100644 --- a/src/hook.js +++ b/src/hook.js @@ -10,3 +10,10 @@ export let hook = funHooks({ * @type {{}} */ export const hooks = hook.hooks; + +export function setupBeforeHookFnOnce(baseFn, hookFn, priority = 15) { + let result = baseFn.getHooks({hook: hookFn}); + if (result.length === 0) { + baseFn.before(hookFn, priority); + } +} diff --git a/src/prebid.js b/src/prebid.js index 92f6d819ecf..2a832c26d2a 100644 --- a/src/prebid.js +++ b/src/prebid.js @@ -69,7 +69,7 @@ function setRenderSize(doc, width, height) { } } -const checkAdUnitSetup = hook('sync', function (adUnits) { +export const checkAdUnitSetup = hook('sync', function (adUnits) { adUnits.forEach((adUnit) => { const mediaTypes = adUnit.mediaTypes; const normalizedSize = utils.getAdUnitSizes(adUnit); diff --git a/src/video.js b/src/video.js index d5fcadd1f38..9cf25016d46 100644 --- a/src/video.js +++ b/src/video.js @@ -42,7 +42,7 @@ export function isValidVideoBid(bid, bidRequests) { return checkVideoBidSetup(bid, bidRequest, videoMediaType, context); } -const checkVideoBidSetup = hook('sync', function(bid, bidRequest, videoMediaType, context) { +export const checkVideoBidSetup = hook('sync', function(bid, bidRequest, videoMediaType, context) { if (!bidRequest || (videoMediaType && context !== OUTSTREAM)) { // xml-only video bids require a prebid cache url if (!config.getConfig('cache.url') && bid.vastXml && !bid.vastUrl) { diff --git a/test/spec/modules/adpod_spec.js b/test/spec/modules/adpod_spec.js index fd8abea310a..16e94fd569f 100644 --- a/test/spec/modules/adpod_spec.js +++ b/test/spec/modules/adpod_spec.js @@ -779,6 +779,9 @@ describe('adpod.js', function () { config.setConfig({ cache: { url: 'http://test.cache.url/endpoint' + }, + adpod: { + brandCategoryExclusion: true } }); logWarnStub = sinon.stub(utils, 'logWarn'); @@ -802,6 +805,24 @@ describe('adpod.js', function () { }); it('returns true when adpod bid is properly setup', function() { + config.setConfig({ + cache: { + url: 'http://test.cache.url/endpoint' + }, + adpod: { + brandCategoryExclusion: false + } + }); + + let goodBid = utils.deepClone(adpodTestBid); + goodBid.meta.iabSubCatId = undefined; + checkVideoBidSetupHook(callbackFn, goodBid, bidderRequestNoExact, {}, ADPOD); + expect(callbackResult).to.be.null; + expect(bailResult).to.equal(true); + expect(logErrorStub.called).to.equal(false); + }); + + it('returns true when adpod bid is missing iab category while brandCategoryExclusion in config is false', function() { let goodBid = utils.deepClone(adpodTestBid); checkVideoBidSetupHook(callbackFn, goodBid, bidderRequestNoExact, {}, ADPOD); expect(callbackResult).to.be.null; @@ -818,7 +839,7 @@ describe('adpod.js', function () { } let noCatBid = utils.deepClone(adpodTestBid); - delete noCatBid.meta; + noCatBid.meta.iabSubCatId = undefined; testInvalidAdpodBid(noCatBid, false); let noContextBid = utils.deepClone(adpodTestBid);