From a0e85c1f75837872255dd4aaaa81f0f7584d6e1a Mon Sep 17 00:00:00 2001 From: Nick Colletti Date: Thu, 24 Sep 2020 13:30:04 -0400 Subject: [PATCH 1/4] feature/auction-timing --- src/auction.js | 11 +- src/config.js | 34 ++++++ test/pages/auctionTiming.html | 197 +++++++++++++++++++++++++++++++ test/spec/auctionmanager_spec.js | 115 ++++++++++++++++++ test/spec/config_spec.js | 33 ++++++ 5 files changed, 388 insertions(+), 2 deletions(-) create mode 100644 test/pages/auctionTiming.html diff --git a/src/auction.js b/src/auction.js index 5858c3edf78..28ecf6be085 100644 --- a/src/auction.js +++ b/src/auction.js @@ -66,6 +66,7 @@ import { config } from './config.js'; import { userSync } from './userSync.js'; import { hook } from './hook.js'; import find from 'core-js-pure/features/array/find.js'; +import includes from 'core-js-pure/features/array/includes.js'; import { OUTSTREAM } from './video.js'; import { VIDEO } from './mediaTypes.js'; @@ -397,10 +398,16 @@ export function auctionCallbacks(auctionDone, auctionInstance) { function adapterDone() { let bidderRequest = this; + let bidderRequests = auctionInstance.getBidRequests(); bidderRequestsDone.add(bidderRequest); - allAdapterCalledDone = auctionInstance.getBidRequests() - .every(bidderRequest => bidderRequestsDone.has(bidderRequest)); + allAdapterCalledDone = bidderRequests.filter(request => { + const auctionTimingConfig = config.getConfig('auctionTiming'); + if (auctionTimingConfig && auctionTimingConfig.secondaryBidders && !bidderRequests.every(bidder => includes(auctionTimingConfig.secondaryBidders, bidder.bidderCode))) { + return !includes(auctionTimingConfig.secondaryBidders, request.bidderCode) + } + return request; + }).every(bidderRequest => bidderRequestsDone.has(bidderRequest)); bidderRequest.bids.forEach(bid => { if (!bidResponseMap[bid.bidId]) { diff --git a/src/config.js b/src/config.js index 3284be52296..94f6402a556 100644 --- a/src/config.js +++ b/src/config.js @@ -199,6 +199,16 @@ export function newConfig() { set disableAjaxTimeout(val) { this._disableAjaxTimeout = val; }, + + _auctionTiming: {}, + get auctionTiming() { + return this._auctionTiming; + }, + set auctionTiming(val) { + if (validateAuctionTiming(val)) { + this._auctionTiming = val; + } + }, }; if (config) { @@ -237,6 +247,30 @@ export function newConfig() { } return true; } + + function validateAuctionTiming(val) { + if (!utils.isPlainObject(val)) { + utils.logWarn('Auction Timing must be an object') + return false + } + + for (let k of Object.keys(val)) { + if (k !== 'secondaryBidders') { + utils.logWarn(`Auction Timing given an incorrect param: ${k}`) + return false + } + if (k === 'secondaryBidders') { + if (!utils.isArray(val[k])) { + utils.logWarn(`Auction Timing ${k} must be of type Array`); + return false + } else if (!val[k].every(utils.isStr)) { + utils.logWarn(`Auction Timing ${k} must be only string`); + return false + } + } + } + return true; + } } /** diff --git a/test/pages/auctionTiming.html b/test/pages/auctionTiming.html new file mode 100644 index 00000000000..aeb97a677cd --- /dev/null +++ b/test/pages/auctionTiming.html @@ -0,0 +1,197 @@ + + + + + + + + + + + + + + +

Prebid.js Test

+
Div-1
+
+ +
+ + \ No newline at end of file diff --git a/test/spec/auctionmanager_spec.js b/test/spec/auctionmanager_spec.js index e35b1406fbf..6d96a890df9 100644 --- a/test/spec/auctionmanager_spec.js +++ b/test/spec/auctionmanager_spec.js @@ -1252,4 +1252,119 @@ describe('auctionmanager.js', function () { assert.equal(doneSpy.callCount, 1); }) }); + + describe('auctionTiming', function() { + let bidRequests; + let doneSpy; + let clock; + let auction = { + getBidRequests: () => bidRequests, + getAuctionId: () => '1', + addBidReceived: () => true, + getTimeout: () => 1000 + } + let requiredBidder = BIDDER_CODE; + let requiredBidder1 = BIDDER_CODE1; + let secondaryBidder = 'doNotWaitForMe'; + + beforeEach(() => { + clock = sinon.useFakeTimers(); + doneSpy = sinon.spy(); + config.setConfig({ + 'auctionTiming': { + secondaryBidders: [ secondaryBidder ] + } + }) + }); + + afterEach(() => { + doneSpy.resetHistory(); + config.resetConfig(); + clock.restore(); + }); + + it('should not wait to call auction done for secondary bidders', function () { + let bids1 = [mockBid({ bidderCode: requiredBidder })]; + let bids2 = [mockBid({ bidderCode: requiredBidder1 })]; + let bids3 = [mockBid({ bidderCode: secondaryBidder })]; + bidRequests = [ + mockBidRequest(bids1[0], { adUnitCode: ADUNIT_CODE1 }), + mockBidRequest(bids2[0], { adUnitCode: ADUNIT_CODE1 }), + mockBidRequest(bids3[0], { adUnitCode: ADUNIT_CODE1 }), + ]; + let cbs = auctionCallbacks(doneSpy, auction); + // required bidder responds immeaditely to auction + cbs.addBidResponse.call(bidRequests[0], ADUNIT_CODE1, bids1[0]); + cbs.adapterDone.call(bidRequests[0]); + assert.equal(doneSpy.callCount, 0); + + // auction waits for second required bidder to respond + clock.tick(100); + cbs.addBidResponse.call(bidRequests[1], ADUNIT_CODE1, bids2[0]); + cbs.adapterDone.call(bidRequests[1]); + + // auction done is reported and does not wait for secondaryBidder request + assert.equal(doneSpy.callCount, 1); + + cbs.addBidResponse.call(bidRequests[2], ADUNIT_CODE1, bids3[0]); + cbs.adapterDone.call(bidRequests[2]); + }); + + it('should wait for all bidders if they are all secondary', function () { + config.setConfig({ + 'auctionTiming': { + secondaryBidders: [requiredBidder, requiredBidder1, secondaryBidder] + } + }) + let bids1 = [mockBid({ bidderCode: requiredBidder })]; + let bids2 = [mockBid({ bidderCode: requiredBidder1 })]; + let bids3 = [mockBid({ bidderCode: secondaryBidder })]; + bidRequests = [ + mockBidRequest(bids1[0], { adUnitCode: ADUNIT_CODE1 }), + mockBidRequest(bids2[0], { adUnitCode: ADUNIT_CODE1 }), + mockBidRequest(bids3[0], { adUnitCode: ADUNIT_CODE1 }), + ]; + let cbs = auctionCallbacks(doneSpy, auction); + cbs.addBidResponse.call(bidRequests[0], ADUNIT_CODE1, bids1[0]); + cbs.adapterDone.call(bidRequests[0]); + clock.tick(100); + assert.equal(doneSpy.callCount, 0) + + cbs.addBidResponse.call(bidRequests[1], ADUNIT_CODE1, bids2[0]); + cbs.adapterDone.call(bidRequests[1]); + clock.tick(100); + assert.equal(doneSpy.callCount, 0); + + cbs.addBidResponse.call(bidRequests[2], ADUNIT_CODE1, bids3[0]); + cbs.adapterDone.call(bidRequests[2]); + assert.equal(doneSpy.callCount, 1); + }); + + it('should allow secondaryBidders to respond in auction before is is done', function () { + let bids1 = [mockBid({ bidderCode: requiredBidder })]; + let bids2 = [mockBid({ bidderCode: requiredBidder1 })]; + let bids3 = [mockBid({ bidderCode: secondaryBidder })]; + bidRequests = [ + mockBidRequest(bids1[0], { adUnitCode: ADUNIT_CODE1 }), + mockBidRequest(bids2[0], { adUnitCode: ADUNIT_CODE1 }), + mockBidRequest(bids3[0], { adUnitCode: ADUNIT_CODE1 }), + ]; + let cbs = auctionCallbacks(doneSpy, auction); + // secondaryBidder is first to respond + cbs.addBidResponse.call(bidRequests[2], ADUNIT_CODE1, bids3[0]); + cbs.adapterDone.call(bidRequests[2]); + clock.tick(100); + assert.equal(doneSpy.callCount, 0); + + cbs.addBidResponse.call(bidRequests[1], ADUNIT_CODE1, bids2[0]); + cbs.adapterDone.call(bidRequests[1]); + clock.tick(100); + assert.equal(doneSpy.callCount, 0); + + // first required bidder takes longest to respond, auction isn't marked as done until this occurs + cbs.addBidResponse.call(bidRequests[0], ADUNIT_CODE1, bids1[0]); + cbs.adapterDone.call(bidRequests[0]); + assert.equal(doneSpy.callCount, 1); + }); + }); }); diff --git a/test/spec/config_spec.js b/test/spec/config_spec.js index be5b4bbb78b..7f6a18c0271 100644 --- a/test/spec/config_spec.js +++ b/test/spec/config_spec.js @@ -211,4 +211,37 @@ describe('config API', function () { setConfig({ bidderSequence: 'random' }); expect(logWarnSpy.called).to.equal(false); }); + + it('sets auctionTiming', function () { + const auctionTimingConfig = { + 'secondaryBidders': ['rubicon', 'appnexus'] + } + setConfig({ auctionTiming: auctionTimingConfig }); + expect(getConfig('auctionTiming')).to.eql(auctionTimingConfig); + }); + + it('should log warning for the wrong value passed to auctionTiming', function () { + setConfig({ auctionTiming: '' }); + expect(logWarnSpy.calledOnce).to.equal(true); + const warning = 'Auction Timing must be an object'; + assert.ok(logWarnSpy.calledWith(warning), 'expected warning was logged'); + }); + + it('should log warning for invalid auctionTiming bidder values', function () { + setConfig({ auctionTiming: { + 'secondaryBidders': 'appnexus, rubicon', + }}); + expect(logWarnSpy.calledOnce).to.equal(true); + const warning = 'Auction Timing secondaryBidders must be of type Array'; + assert.ok(logWarnSpy.calledWith(warning), 'expected warning was logged'); + }); + + it('should log warning for invalid properties to auctionTiming', function () { + setConfig({ auctionTiming: { + 'testing': true + }}); + expect(logWarnSpy.calledOnce).to.equal(true); + const warning = 'Auction Timing given an incorrect param: testing'; + assert.ok(logWarnSpy.calledWith(warning), 'expected warning was logged'); + }); }); From eff184ac31101870a3793477a7b1da293a41a2fa Mon Sep 17 00:00:00 2001 From: Nick Colletti Date: Mon, 28 Sep 2020 15:44:48 -0400 Subject: [PATCH 2/4] rename to auctionOptions --- src/auction.js | 6 ++-- src/config.js | 22 ++++++------ ...auctionTiming.html => auctionOptions.html} | 34 ++++++++++--------- test/spec/auctionmanager_spec.js | 6 ++-- test/spec/config_spec.js | 26 +++++++------- 5 files changed, 48 insertions(+), 46 deletions(-) rename test/pages/{auctionTiming.html => auctionOptions.html} (77%) diff --git a/src/auction.js b/src/auction.js index 28ecf6be085..56b5172e047 100644 --- a/src/auction.js +++ b/src/auction.js @@ -402,9 +402,9 @@ export function auctionCallbacks(auctionDone, auctionInstance) { bidderRequestsDone.add(bidderRequest); allAdapterCalledDone = bidderRequests.filter(request => { - const auctionTimingConfig = config.getConfig('auctionTiming'); - if (auctionTimingConfig && auctionTimingConfig.secondaryBidders && !bidderRequests.every(bidder => includes(auctionTimingConfig.secondaryBidders, bidder.bidderCode))) { - return !includes(auctionTimingConfig.secondaryBidders, request.bidderCode) + const auctionOptionsConfig = config.getConfig('auctionOptions'); + if (auctionOptionsConfig && auctionOptionsConfig.secondaryBidders && !bidderRequests.every(bidder => includes(auctionOptionsConfig.secondaryBidders, bidder.bidderCode))) { + return !includes(auctionOptionsConfig.secondaryBidders, request.bidderCode) } return request; }).every(bidderRequest => bidderRequestsDone.has(bidderRequest)); diff --git a/src/config.js b/src/config.js index 94f6402a556..daaf739bbbd 100644 --- a/src/config.js +++ b/src/config.js @@ -200,13 +200,13 @@ export function newConfig() { this._disableAjaxTimeout = val; }, - _auctionTiming: {}, - get auctionTiming() { - return this._auctionTiming; + _auctionOptions: {}, + get auctionOptions() { + return this._auctionOptions; }, - set auctionTiming(val) { - if (validateAuctionTiming(val)) { - this._auctionTiming = val; + set auctionOptions(val) { + if (validateauctionOptions(val)) { + this._auctionOptions = val; } }, }; @@ -248,23 +248,23 @@ export function newConfig() { return true; } - function validateAuctionTiming(val) { + function validateauctionOptions(val) { if (!utils.isPlainObject(val)) { - utils.logWarn('Auction Timing must be an object') + utils.logWarn('Auction Options must be an object') return false } for (let k of Object.keys(val)) { if (k !== 'secondaryBidders') { - utils.logWarn(`Auction Timing given an incorrect param: ${k}`) + utils.logWarn(`Auction Options given an incorrect param: ${k}`) return false } if (k === 'secondaryBidders') { if (!utils.isArray(val[k])) { - utils.logWarn(`Auction Timing ${k} must be of type Array`); + utils.logWarn(`Auction Options ${k} must be of type Array`); return false } else if (!val[k].every(utils.isStr)) { - utils.logWarn(`Auction Timing ${k} must be only string`); + utils.logWarn(`Auction Options ${k} must be only string`); return false } } diff --git a/test/pages/auctionTiming.html b/test/pages/auctionOptions.html similarity index 77% rename from test/pages/auctionTiming.html rename to test/pages/auctionOptions.html index aeb97a677cd..1cb5a8d70e3 100644 --- a/test/pages/auctionTiming.html +++ b/test/pages/auctionOptions.html @@ -1,4 +1,4 @@ - + @@ -50,11 +50,11 @@ pbjs.aliasBidder('appnexus', 'waitForMe'); pbjs.aliasBidder('rubicon', 'doNotWaitForMe'); pbjs.setConfig({ - 'auctionTiming': { + 'auctionOptions': { 'secondaryBidders': ['doNotWaitForMe'] } }) - auctionTimingLogging(); + auctionOptionsLogging(); pbjs.requestBids({ bidsBackHandler: sendAdserverRequest, timeout: PREBID_TIMEOUT @@ -76,44 +76,46 @@ sendAdserverRequest(); }, FAILSAFE_TIMEOUT); - function auctionTimingLogging() { + // auction options debugging log that should be initialized prior to requestBids function + function auctionOptionsLogging() { pbjs.onEvent('auctionInit', auction => { - console.log(`Auction Timing: Auction Start at ${auction.timestamp} - ${auction.auctionId}`); + console.log(`Auction Options: Auction Start at ${auction.timestamp} - ${auction.auctionId}`); }) pbjs.onEvent('bidRequested', bidderRequest => { - console.log(`Auction Timing: Bid Requested from ${bidderRequest.bidderCode} at ${bidderRequest.start} - ${bidderRequest.auctionId}`); + console.log(`Auction Options: Bid Requested from ${bidderRequest.bidderCode} at ${bidderRequest.start} - ${bidderRequest.auctionId}`); }) pbjs.onEvent('bidResponse', bid => { - console.log(`Auction Timing: Bid Response from ${bid.bidderCode} at ${Date.now()} in ${bid.responseTimestamp - bid.requestTimestamp}ms - ${bid.auctionId}`); + console.log(`Auction Options: Bid Response from ${bid.bidderCode} at ${Date.now()} in ${bid.responseTimestamp - bid.requestTimestamp}ms - ${bid.auctionId}`); }) pbjs.onEvent('noBid', bid => { - console.log(`Auction Timing: No Bid from ${bid.bidder} - ${bid.auctionId}`); + console.log(`Auction Options: No Bid from ${bid.bidder} - ${bid.auctionId}`); }) pbjs.onEvent('bidderDone', bidderRequest => { - console.log(`Auction Timing: Bidder ${bidderRequest.bidderCode} Done in ${Date.now() - bidderRequest.start}ms - ${bidderRequest.auctionId}`); + console.log(`Auction Options: Bidder ${bidderRequest.bidderCode} Done in ${Date.now() - bidderRequest.start}ms - ${bidderRequest.auctionId}`); }) pbjs.onEvent('bidTimeout', timedOutBidders => { let auctionId = timedOutBidders.length > 0 ? timedOutBidders[0].auctionId : 0 - console.log(`Auction Timing: Auction End! Timed Out! Bidders: ${Array.from(new Set(timedOutBidders.map(each => each.bidder))).join(',')} - ${auctionId}`); + console.log(`Auction Options: Auction End! Timed Out! Bidders: ${Array.from(new Set(timedOutBidders.map(each => each.bidder))).join(',')} - ${auctionId}`); }) pbjs.onEvent('auctionEnd', auction => { let auctionId = auction.bidderRequests.length > 0 ? auction.bidderRequests[0].auctionId : 0 let auctionStart = auction.bidderRequests.length > 0 ? auction.bidderRequests[0].auctionStart : 0 - console.log(`Auction Timing: Auction End! After ${Date.now() - auctionStart}ms - ${auctionId}`); - auctionTimingLog(auctionId) + console.log(`Auction Options: Auction End! After ${Date.now() - auctionStart}ms - ${auctionId}`); + auctionOptionsLog(auctionId) }) - function auctionTimingLog(auctionId) { + // auction options debug that can be run post auction within the devtools console + function auctionOptionsLog(auctionId) { let winners = pbjs.getAllWinningBids(); let output = []; let auctionTime = pbjs.getConfig('bidderTimeout'); - const config = pbjs.getConfig('auctionTiming'); + const config = pbjs.getConfig('auctionOptions'); populateData(); displayData(); @@ -130,7 +132,7 @@ forEach(pbjs.getBidResponses(), function (code, bid) { output.push({ - "Auction Timing": auctionId, + "Auction Options": auctionId, bid: bid, adunit: code, adId: bid.adId, @@ -149,7 +151,7 @@ forEach(pbjs.getNoBids && pbjs.getNoBids() || {}, function (code, bid) { output.push({ - "Auction Timing": auctionId, + "Auction Options": auctionId, msg: "no bid", adunit: code, adId: bid.bidId, diff --git a/test/spec/auctionmanager_spec.js b/test/spec/auctionmanager_spec.js index 6d96a890df9..a848197b527 100644 --- a/test/spec/auctionmanager_spec.js +++ b/test/spec/auctionmanager_spec.js @@ -1253,7 +1253,7 @@ describe('auctionmanager.js', function () { }) }); - describe('auctionTiming', function() { + describe('auctionOptions', function() { let bidRequests; let doneSpy; let clock; @@ -1271,7 +1271,7 @@ describe('auctionmanager.js', function () { clock = sinon.useFakeTimers(); doneSpy = sinon.spy(); config.setConfig({ - 'auctionTiming': { + 'auctionOptions': { secondaryBidders: [ secondaryBidder ] } }) @@ -1312,7 +1312,7 @@ describe('auctionmanager.js', function () { it('should wait for all bidders if they are all secondary', function () { config.setConfig({ - 'auctionTiming': { + 'auctionOptions': { secondaryBidders: [requiredBidder, requiredBidder1, secondaryBidder] } }) diff --git a/test/spec/config_spec.js b/test/spec/config_spec.js index 7f6a18c0271..81ce966efb2 100644 --- a/test/spec/config_spec.js +++ b/test/spec/config_spec.js @@ -212,36 +212,36 @@ describe('config API', function () { expect(logWarnSpy.called).to.equal(false); }); - it('sets auctionTiming', function () { - const auctionTimingConfig = { + it('sets auctionOptions', function () { + const auctionOptionsConfig = { 'secondaryBidders': ['rubicon', 'appnexus'] } - setConfig({ auctionTiming: auctionTimingConfig }); - expect(getConfig('auctionTiming')).to.eql(auctionTimingConfig); + setConfig({ auctionOptions: auctionOptionsConfig }); + expect(getConfig('auctionOptions')).to.eql(auctionOptionsConfig); }); - it('should log warning for the wrong value passed to auctionTiming', function () { - setConfig({ auctionTiming: '' }); + it('should log warning for the wrong value passed to auctionOptions', function () { + setConfig({ auctionOptions: '' }); expect(logWarnSpy.calledOnce).to.equal(true); - const warning = 'Auction Timing must be an object'; + const warning = 'Auction Options must be an object'; assert.ok(logWarnSpy.calledWith(warning), 'expected warning was logged'); }); - it('should log warning for invalid auctionTiming bidder values', function () { - setConfig({ auctionTiming: { + it('should log warning for invalid auctionOptions bidder values', function () { + setConfig({ auctionOptions: { 'secondaryBidders': 'appnexus, rubicon', }}); expect(logWarnSpy.calledOnce).to.equal(true); - const warning = 'Auction Timing secondaryBidders must be of type Array'; + const warning = 'Auction Options secondaryBidders must be of type Array'; assert.ok(logWarnSpy.calledWith(warning), 'expected warning was logged'); }); - it('should log warning for invalid properties to auctionTiming', function () { - setConfig({ auctionTiming: { + it('should log warning for invalid properties to auctionOptions', function () { + setConfig({ auctionOptions: { 'testing': true }}); expect(logWarnSpy.calledOnce).to.equal(true); - const warning = 'Auction Timing given an incorrect param: testing'; + const warning = 'Auction Options given an incorrect param: testing'; assert.ok(logWarnSpy.calledWith(warning), 'expected warning was logged'); }); }); From 5deadea35cd3ed17a2e4a9c50034fd3fd1088072 Mon Sep 17 00:00:00 2001 From: Nick Colletti Date: Tue, 29 Sep 2020 15:12:37 -0400 Subject: [PATCH 3/4] move filtering outside of loop and organized logic. --- src/auction.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/auction.js b/src/auction.js index 56b5172e047..c9d66e82bac 100644 --- a/src/auction.js +++ b/src/auction.js @@ -399,15 +399,18 @@ export function auctionCallbacks(auctionDone, auctionInstance) { function adapterDone() { let bidderRequest = this; let bidderRequests = auctionInstance.getBidRequests(); + const auctionOptionsConfig = config.getConfig('auctionOptions'); bidderRequestsDone.add(bidderRequest); - allAdapterCalledDone = bidderRequests.filter(request => { - const auctionOptionsConfig = config.getConfig('auctionOptions'); - if (auctionOptionsConfig && auctionOptionsConfig.secondaryBidders && !bidderRequests.every(bidder => includes(auctionOptionsConfig.secondaryBidders, bidder.bidderCode))) { - return !includes(auctionOptionsConfig.secondaryBidders, request.bidderCode) + + if (auctionOptionsConfig && !utils.isEmpty(auctionOptionsConfig)) { + const secondaryBidders = auctionOptionsConfig.secondaryBidders; + if (secondaryBidders && !bidderRequests.every(bidder => includes(secondaryBidders, bidder.bidderCode))) { + bidderRequests = bidderRequests.filter(request => !includes(secondaryBidders, request.bidderCode)); } - return request; - }).every(bidderRequest => bidderRequestsDone.has(bidderRequest)); + } + + allAdapterCalledDone = bidderRequests.every(bidderRequest => bidderRequestsDone.has(bidderRequest)); bidderRequest.bids.forEach(bid => { if (!bidResponseMap[bid.bidId]) { From 606a7150c9250f274ba395bc617f0769a7e424d8 Mon Sep 17 00:00:00 2001 From: Nick Colletti Date: Fri, 9 Oct 2020 15:23:35 -0400 Subject: [PATCH 4/4] remove auctionOptions test page --- test/pages/auctionOptions.html | 199 --------------------------------- 1 file changed, 199 deletions(-) delete mode 100644 test/pages/auctionOptions.html diff --git a/test/pages/auctionOptions.html b/test/pages/auctionOptions.html deleted file mode 100644 index 1cb5a8d70e3..00000000000 --- a/test/pages/auctionOptions.html +++ /dev/null @@ -1,199 +0,0 @@ - - - - - - - - - - - - - - -

Prebid.js Test

-
Div-1
-
- -
- - \ No newline at end of file