From a830924af379aa788830e3544734be2b53feddfe Mon Sep 17 00:00:00 2001 From: Nicolas Faure Date: Wed, 27 Jun 2018 16:23:57 +0200 Subject: [PATCH 1/9] Add onSetTargeting method to bid adapter spec --- src/adaptermanager.js | 6 ++++++ src/auction.js | 5 +++++ src/auctionManager.js | 3 +++ 3 files changed, 14 insertions(+) diff --git a/src/adaptermanager.js b/src/adaptermanager.js index 8d07e0ccacc..6ae21a8f5aa 100644 --- a/src/adaptermanager.js +++ b/src/adaptermanager.js @@ -542,3 +542,9 @@ exports.callBidWonBidder = function(bidder, bid, adUnits) { bid.params = utils.getUserConfiguredParams(adUnits, bid.adUnitCode, bid.bidder); tryCallBidderMethod(bidder, 'onBidWon', bid); }; + +exports.callSetTargetingBidder = function(bidder, bid) { + // Adding user configured params to setTargeting event data + bid.params = utils.getUserConfiguredParams(adUnits, bid.adUnitCode, bid.bidder); + tryCallBidderMethod(bidder, 'onSetTargeting', bid); +}; diff --git a/src/auction.js b/src/auction.js index b9f3574ed31..ba939f10a80 100644 --- a/src/auction.js +++ b/src/auction.js @@ -275,11 +275,16 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels}) adaptermanager.callBidWonBidder(winningBid.bidder, winningBid, adUnits); } + function setBidTargeting(bid) { + adaptermanager.callSetTargetingBidder(bid.bidder, bid); + } + return { addBidReceived, executeCallback, callBids, addWinningBid, + setBidTargeting, getWinningBids: () => _winningBids, getTimeout: () => _timeout, getAuctionId: () => _auctionId, diff --git a/src/auctionManager.js b/src/auctionManager.js index e19a80e5e02..fb971c6de17 100644 --- a/src/auctionManager.js +++ b/src/auctionManager.js @@ -91,6 +91,9 @@ export function newAuctionManager() { auctionManager.setStatusForBids = function(adId, status) { let bid = auctionManager.findBidByAdId(adId); if (bid) bid.status = status; + + const auction = find(_auctions, auction => auction.getAuctionId() === bid.auctionId); + if (auction) auction.setBidTargeting(bid); } function _addAuction(auction) { From 63ccf12b760a9baa655785ed9095c7da1d0d505d Mon Sep 17 00:00:00 2001 From: "js.faure" Date: Tue, 23 Oct 2018 16:53:49 +0200 Subject: [PATCH 2/9] Reset context before each criteo adapter test --- test/spec/modules/criteoBidAdapter_spec.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/spec/modules/criteoBidAdapter_spec.js b/test/spec/modules/criteoBidAdapter_spec.js index d124ebf3709..6dbf51932a0 100755 --- a/test/spec/modules/criteoBidAdapter_spec.js +++ b/test/spec/modules/criteoBidAdapter_spec.js @@ -3,6 +3,10 @@ import { cryptoVerify, spec, FAST_BID_PUBKEY } from 'modules/criteoBidAdapter'; import * as utils from 'src/utils'; describe('The Criteo bidding adapter', function () { + beforeEach(function () { + // Remove FastBid to avoid side effects. + localStorage.removeItem('criteo_fast_bid'); + }); describe('isBidRequestValid', function () { it('should return false when given an invalid bid', function () { const bid = { From 57d9abb4134623498b5fce853c8eed215ea3dbaa Mon Sep 17 00:00:00 2001 From: "js.faure" Date: Wed, 24 Oct 2018 16:04:34 +0200 Subject: [PATCH 3/9] Add a unit test to check spec.onTimeout() is called --- test/spec/unit/core/adapterManager_spec.js | 37 ++++++++++++++++++++++ test/spec/unit/pbjs_api_spec.js | 5 ++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/test/spec/unit/core/adapterManager_spec.js b/test/spec/unit/core/adapterManager_spec.js index bde5f234ffb..01c9f1d4763 100644 --- a/test/spec/unit/core/adapterManager_spec.js +++ b/test/spec/unit/core/adapterManager_spec.js @@ -134,6 +134,43 @@ describe('adapterManager tests', function () { }); }); + describe('callTimedOutBidders', function () { + var criteoSpec = { onTimeout: sinon.stub() } + var criteoAdapter = { + bidder: 'criteo', + getSpec: function() { return criteoSpec; } + } + before(function () { + config.setConfig({s2sConfig: { enabled: false }}); + }); + + beforeEach(function () { + AdapterManager.bidderRegistry['criteo'] = criteoAdapter; + }); + + afterEach(function () { + delete AdapterManager.bidderRegistry['criteo']; + }); + + it('should call spec\'s onTimeout callback when callTimedOutBidders is called', function () { + const adUnits = [{ + code: 'adUnit-code', + sizes: [[728, 90]], + bids: [ + {bidder: 'criteo', params: {placementId: 'id'}}, + ] + }]; + const timedOutBidders = [{ + bidId: 'bidId', + bidder: 'criteo', + adUnitCode: adUnits[0].code, + auctionId: 'auctionId', + }]; + AdapterManager.callTimedOutBidders(adUnits, timedOutBidders, CONFIG.timeout); + sinon.assert.called(criteoSpec.onTimeout); + }); + }); // end callTimedOutBidders + describe('S2S tests', function () { beforeEach(function () { config.setConfig({s2sConfig: CONFIG}); diff --git a/test/spec/unit/pbjs_api_spec.js b/test/spec/unit/pbjs_api_spec.js index 61e26891bac..0db5bca216b 100644 --- a/test/spec/unit/pbjs_api_spec.js +++ b/test/spec/unit/pbjs_api_spec.js @@ -1165,7 +1165,8 @@ describe('Unit: Prebid Module', function () { isBidRequestValid: sinon.stub(), buildRequests: sinon.stub(), interpretResponse: sinon.stub(), - getUserSyncs: sinon.stub() + getUserSyncs: sinon.stub(), + onTimeout: sinon.stub() }; registerBidder(spec); @@ -1190,6 +1191,8 @@ describe('Unit: Prebid Module', function () { expect(bidsBackHandlerStub.getCall(0).args[1]).to.equal(true, 'bidsBackHandler should be called with timedOut=true'); + + sinon.assert.called(spec.onTimeout); }); }) From 35999b7dab0c210f4d778698691a6e164e7984da Mon Sep 17 00:00:00 2001 From: "js.faure" Date: Thu, 25 Oct 2018 09:53:46 +0200 Subject: [PATCH 4/9] Add a unit test to check spec.onBidWon() is called --- test/spec/unit/core/adapterManager_spec.js | 32 ++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/spec/unit/core/adapterManager_spec.js b/test/spec/unit/core/adapterManager_spec.js index 01c9f1d4763..1ce962c44a9 100644 --- a/test/spec/unit/core/adapterManager_spec.js +++ b/test/spec/unit/core/adapterManager_spec.js @@ -171,6 +171,38 @@ describe('adapterManager tests', function () { }); }); // end callTimedOutBidders + describe('onBidWon', function () { + var criteoSpec = { onBidWon: sinon.stub() } + var criteoAdapter = { + bidder: 'criteo', + getSpec: function() { return criteoSpec; } + } + before(function () { + config.setConfig({s2sConfig: { enabled: false }}); + }); + + beforeEach(function () { + AdapterManager.bidderRegistry['criteo'] = criteoAdapter; + }); + + afterEach(function () { + delete AdapterManager.bidderRegistry['criteo']; + }); + + it('should call spec\'s onBidWon callback when a bid is won', function () { + const bids = [ + {bidder: 'criteo', params: {placementId: 'id'}}, + ]; + const adUnits = [{ + code: 'adUnit-code', + sizes: [[728, 90]], + bids + }]; + + AdapterManager.callBidWonBidder(bids[0].bidder, bids[0], adUnits); + sinon.assert.called(criteoSpec.onBidWon); + }); + }); // end onBidWon describe('S2S tests', function () { beforeEach(function () { config.setConfig({s2sConfig: CONFIG}); From 655683a7454008ad7ba05f633863eb16522476a9 Mon Sep 17 00:00:00 2001 From: "js.faure" Date: Thu, 25 Oct 2018 09:54:02 +0200 Subject: [PATCH 5/9] Add a unit test to check spec.onSetTargeting() is called --- src/adaptermanager.js | 2 +- test/spec/unit/core/adapterManager_spec.js | 33 ++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/adaptermanager.js b/src/adaptermanager.js index b3652c32674..66bd0157910 100644 --- a/src/adaptermanager.js +++ b/src/adaptermanager.js @@ -530,7 +530,7 @@ exports.callBidWonBidder = function(bidder, bid, adUnits) { tryCallBidderMethod(bidder, 'onBidWon', bid); }; -exports.callSetTargetingBidder = function(bidder, bid) { +exports.callSetTargetingBidder = function(bidder, bid, adUnits) { // Adding user configured params to setTargeting event data bid.params = utils.getUserConfiguredParams(adUnits, bid.adUnitCode, bid.bidder); tryCallBidderMethod(bidder, 'onSetTargeting', bid); diff --git a/test/spec/unit/core/adapterManager_spec.js b/test/spec/unit/core/adapterManager_spec.js index 1ce962c44a9..a7b25d2524d 100644 --- a/test/spec/unit/core/adapterManager_spec.js +++ b/test/spec/unit/core/adapterManager_spec.js @@ -203,6 +203,39 @@ describe('adapterManager tests', function () { sinon.assert.called(criteoSpec.onBidWon); }); }); // end onBidWon + + describe('onSetTargeting', function () { + var criteoSpec = { onSetTargeting: sinon.stub() } + var criteoAdapter = { + bidder: 'criteo', + getSpec: function() { return criteoSpec; } + } + before(function () { + config.setConfig({s2sConfig: { enabled: false }}); + }); + + beforeEach(function () { + AdapterManager.bidderRegistry['criteo'] = criteoAdapter; + }); + + afterEach(function () { + delete AdapterManager.bidderRegistry['criteo']; + }); + + it('should call spec\'s onSetTargeting callback when setTargeting is called', function () { + const bids = [ + {bidder: 'criteo', params: {placementId: 'id'}}, + ]; + const adUnits = [{ + code: 'adUnit-code', + sizes: [[728, 90]], + bids + }]; + AdapterManager.callSetTargetingBidder(bids[0].bidder, bids[0], adUnits); + sinon.assert.called(criteoSpec.onSetTargeting); + }); + }); // end onSetTargeting + describe('S2S tests', function () { beforeEach(function () { config.setConfig({s2sConfig: CONFIG}); From fa7daf3e9a12ffa38ffef75479a9f0bc15c6e428 Mon Sep 17 00:00:00 2001 From: "js.faure" Date: Fri, 26 Oct 2018 18:33:19 +0200 Subject: [PATCH 6/9] Remove unused adUnits argument from callSetTargetingBidder --- src/adaptermanager.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/adaptermanager.js b/src/adaptermanager.js index 66bd0157910..55aab710741 100644 --- a/src/adaptermanager.js +++ b/src/adaptermanager.js @@ -530,8 +530,6 @@ exports.callBidWonBidder = function(bidder, bid, adUnits) { tryCallBidderMethod(bidder, 'onBidWon', bid); }; -exports.callSetTargetingBidder = function(bidder, bid, adUnits) { - // Adding user configured params to setTargeting event data - bid.params = utils.getUserConfiguredParams(adUnits, bid.adUnitCode, bid.bidder); +exports.callSetTargetingBidder = function(bidder, bid) { tryCallBidderMethod(bidder, 'onSetTargeting', bid); }; From 3d34b828630462ebdbb064391b21dc6d8f16e51d Mon Sep 17 00:00:00 2001 From: "js.faure" Date: Fri, 26 Oct 2018 18:33:38 +0200 Subject: [PATCH 7/9] Add integration test on onSetTargeting --- test/spec/unit/pbjs_api_spec.js | 46 +++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/test/spec/unit/pbjs_api_spec.js b/test/spec/unit/pbjs_api_spec.js index 0db5bca216b..4616178d180 100644 --- a/test/spec/unit/pbjs_api_spec.js +++ b/test/spec/unit/pbjs_api_spec.js @@ -1194,6 +1194,52 @@ describe('Unit: Prebid Module', function () { sinon.assert.called(spec.onTimeout); }); + + it('should execute callback after setTargeting', function () { + let spec = { + code: BIDDER_CODE, + isBidRequestValid: sinon.stub(), + buildRequests: sinon.stub(), + interpretResponse: sinon.stub(), + onSetTargeting: sinon.stub() + }; + + registerBidder(spec); + spec.buildRequests.returns([{'id': 123, 'method': 'POST'}]); + spec.isBidRequestValid.returns(true); + spec.interpretResponse.returns(bids); + + const bidId = 1; + const auctionId = 1; + let adResponse = Object.assign({ + auctionId: auctionId, + adId: String(bidId), + width: 300, + height: 250, + adUnitCode: bidRequests[0].bids[0].adUnitCode, + adserverTargeting: { + 'hb_bidder': BIDDER_CODE, + 'hb_adid': bidId, + 'hb_pb': bids[0].cpm, + 'hb_size': '300x250', + }, + bidder: bids[0].bidderCode, + }, bids[0]); + auction.getBidsReceived = function() { return [adResponse]; } + auction.getAuctionId = () => auctionId; + + clock = sinon.useFakeTimers(); + let requestObj = { + bidsBackHandler: null, // does not need to be defined because of newAuction mock in beforeEach + timeout: 2000, + adUnits: adUnits + }; + + $$PREBID_GLOBAL$$.requestBids(requestObj); + $$PREBID_GLOBAL$$.setTargetingForGPTAsync(); + + sinon.assert.called(spec.onSetTargeting); + }); }) describe('requestBids', function () { From fca4a5ef4baa4b121292131bf699681bc3ba46a5 Mon Sep 17 00:00:00 2001 From: "js.faure" Date: Tue, 30 Oct 2018 17:36:38 +0100 Subject: [PATCH 8/9] Move Bid status constants from targeting.js to constants.json --- src/constants.json | 4 ++++ src/prebid.js | 10 +++++----- src/targeting.js | 6 +----- test/spec/unit/pbjs_api_spec.js | 10 +++++----- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/constants.json b/src/constants.json index 9ec51e4047b..7c06db48469 100644 --- a/src/constants.json +++ b/src/constants.json @@ -77,5 +77,9 @@ "SRC" : "s2s", "DEFAULT_ENDPOINT" : "https://prebid.adnxs.com/pbs/v1/openrtb2/auction", "SYNCED_BIDDERS_KEY": "pbjsSyncs" + }, + "BID_STATUS" : { + "BID_TARGETING_SET": "targetingSet", + "RENDERED": "rendered" } } diff --git a/src/prebid.js b/src/prebid.js index 48d08719cb1..83ece2fb9fe 100644 --- a/src/prebid.js +++ b/src/prebid.js @@ -7,7 +7,7 @@ import { userSync } from 'src/userSync.js'; import { loadScript } from './adloader'; import { config } from './config'; import { auctionManager } from './auctionManager'; -import { targeting, getHighestCpmBidsFromBidPool, RENDERED, BID_TARGETING_SET } from './targeting'; +import { targeting, getHighestCpmBidsFromBidPool } from './targeting'; import { createHook } from 'src/hook'; import { sessionLoader } from 'src/debugging'; import includes from 'core-js/library/fn/array/includes'; @@ -180,7 +180,7 @@ $$PREBID_GLOBAL$$.setTargetingForGPTAsync = function (adUnit, customSlotMatching Object.keys(targetingSet).forEach((adUnitCode) => { Object.keys(targetingSet[adUnitCode]).forEach((targetingKey) => { if (targetingKey === 'hb_adid') { - auctionManager.setStatusForBids(targetingSet[adUnitCode][targetingKey], BID_TARGETING_SET); + auctionManager.setStatusForBids(targetingSet[adUnitCode][targetingKey], CONSTANTS.BID_STATUS.BID_TARGETING_SET); } }); }); @@ -234,7 +234,7 @@ $$PREBID_GLOBAL$$.renderAd = function (doc, id) { // lookup ad by ad Id const bid = auctionManager.findBidByAdId(id); if (bid) { - bid.status = RENDERED; + bid.status = CONSTANTS.BID_STATUS.RENDERED; // replace macros according to openRTB with price paid = bid.cpm bid.ad = utils.replaceAuctionPrice(bid.ad, bid.cpm); bid.adUrl = utils.replaceAuctionPrice(bid.adUrl, bid.cpm); @@ -586,7 +586,7 @@ $$PREBID_GLOBAL$$.getAllWinningBids = function () { */ $$PREBID_GLOBAL$$.getAllPrebidWinningBids = function () { return auctionManager.getBidsReceived() - .filter(bid => bid.status === BID_TARGETING_SET) + .filter(bid => bid.status === CONSTANTS.BID_STATUS.BID_TARGETING_SET) .map(removeRequestId); }; @@ -626,7 +626,7 @@ $$PREBID_GLOBAL$$.markWinningBidAsUsed = function (markBidRequest) { } if (bids.length > 0) { - bids[0].status = RENDERED; + bids[0].status = CONSTANTS.BID_STATUS.RENDERED; } }; diff --git a/src/targeting.js b/src/targeting.js index dcd59f362c6..5992686c6e0 100644 --- a/src/targeting.js +++ b/src/targeting.js @@ -10,9 +10,6 @@ var CONSTANTS = require('./constants.json'); var pbTargetingKeys = []; -export const BID_TARGETING_SET = 'targetingSet'; -export const RENDERED = 'rendered'; - const MAX_DFP_KEYLENGTH = 20; const TTL_BUFFER = 1000; @@ -24,7 +21,7 @@ export const TARGETING_KEYS = Object.keys(CONSTANTS.TARGETING_KEYS).map( export const isBidNotExpired = (bid) => (bid.responseTimestamp + bid.ttl * 1000 + TTL_BUFFER) > timestamp(); // return bids whose status is not set. Winning bid can have status `targetingSet` or `rendered`. -const isUnusedBid = (bid) => bid && ((bid.status && !includes([BID_TARGETING_SET, RENDERED], bid.status)) || !bid.status); +const isUnusedBid = (bid) => bid && ((bid.status && !includes([CONSTANTS.BID_STATUS.BID_TARGETING_SET, CONSTANTS.BID_STATUS.RENDERED], bid.status)) || !bid.status); // If two bids are found for same adUnitCode, we will use the highest one to take part in auction // This can happen in case of concurrent auctions @@ -214,7 +211,6 @@ export function newTargeting(auctionManager) { */ targeting.getWinningBids = function(adUnitCode, bidsReceived = getBidsReceived()) { const adUnitCodes = getAdUnitCodes(adUnitCode); - return bidsReceived .filter(bid => includes(adUnitCodes, bid.adUnitCode)) .filter(bid => bid.cpm > 0) diff --git a/test/spec/unit/pbjs_api_spec.js b/test/spec/unit/pbjs_api_spec.js index 4616178d180..2b7a15ef3c6 100644 --- a/test/spec/unit/pbjs_api_spec.js +++ b/test/spec/unit/pbjs_api_spec.js @@ -9,7 +9,7 @@ import { createBidReceived } from 'test/fixtures/fixtures'; import { auctionManager, newAuctionManager } from 'src/auctionManager'; -import { targeting, newTargeting, RENDERED } from 'src/targeting'; +import { targeting, newTargeting } from 'src/targeting'; import { config as configObj } from 'src/config'; import * as ajaxLib from 'src/ajax'; import * as auctionModule from 'src/auction'; @@ -1900,7 +1900,7 @@ describe('Unit: Prebid Module', function () { const markedBid = find($$PREBID_GLOBAL$$.getBidResponsesForAdUnitCode(adUnitCode).bids, bid => bid.adId === winningBid.adId); - expect(markedBid.status).to.equal(RENDERED); + expect(markedBid.status).to.equal(CONSTANTS.BID_STATUS.RENDERED); resetAuction(); }); @@ -1914,7 +1914,7 @@ describe('Unit: Prebid Module', function () { const markedBid = find($$PREBID_GLOBAL$$.getBidResponsesForAdUnitCode(adUnitCode).bids, bid => bid.adId === winningBid.adId); - expect(markedBid.status).to.not.equal(RENDERED); + expect(markedBid.status).to.not.equal(CONSTANTS.BID_STATUS.RENDERED); resetAuction(); }); @@ -1930,7 +1930,7 @@ describe('Unit: Prebid Module', function () { const markedBid = find($$PREBID_GLOBAL$$.getBidResponsesForAdUnitCode(adUnitCode).bids, bid => bid.adId === winningBid.adId); - expect(markedBid.status).to.equal(RENDERED); + expect(markedBid.status).to.equal(CONSTANTS.BID_STATUS.RENDERED); resetAuction(); }); @@ -1946,7 +1946,7 @@ describe('Unit: Prebid Module', function () { const markedBid = find($$PREBID_GLOBAL$$.getBidResponsesForAdUnitCode(adUnitCode).bids, bid => bid.adId === winningBid.adId); - expect(markedBid.status).to.equal(RENDERED); + expect(markedBid.status).to.equal(CONSTANTS.BID_STATUS.RENDERED); resetAuction(); }); }); From da0d6bccb586fbee8090eae02402e6831de52ed0 Mon Sep 17 00:00:00 2001 From: "js.faure" Date: Wed, 31 Oct 2018 10:22:37 +0100 Subject: [PATCH 9/9] Make sure onSetTargeting won't be called when the bid is not in status BID_TARGETING_SET --- src/auctionManager.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/auctionManager.js b/src/auctionManager.js index fb971c6de17..389cac31fe3 100644 --- a/src/auctionManager.js +++ b/src/auctionManager.js @@ -92,8 +92,10 @@ export function newAuctionManager() { let bid = auctionManager.findBidByAdId(adId); if (bid) bid.status = status; - const auction = find(_auctions, auction => auction.getAuctionId() === bid.auctionId); - if (auction) auction.setBidTargeting(bid); + if (bid && status === CONSTANTS.BID_STATUS.BID_TARGETING_SET) { + const auction = find(_auctions, auction => auction.getAuctionId() === bid.auctionId); + if (auction) auction.setBidTargeting(bid); + } } function _addAuction(auction) {