From 7761ad1db10faa6ea52aa314984bc997b7f8ce2f Mon Sep 17 00:00:00 2001 From: Js Date: Tue, 6 Nov 2018 15:38:16 +0100 Subject: [PATCH] On set targeting (#3203) * Add onSetTargeting method to bid adapter spec * Reset context before each criteo adapter test * Add a unit test to check spec.onTimeout() is called * Add a unit test to check spec.onBidWon() is called * Add a unit test to check spec.onSetTargeting() is called * Remove unused adUnits argument from callSetTargetingBidder * Add integration test on onSetTargeting * Move Bid status constants from targeting.js to constants.json * Make sure onSetTargeting won't be called when the bid is not in status BID_TARGETING_SET --- src/adaptermanager.js | 4 + src/auction.js | 5 + src/auctionManager.js | 5 + src/constants.json | 4 + src/prebid.js | 10 +- src/targeting.js | 6 +- test/spec/modules/criteoBidAdapter_spec.js | 4 + test/spec/unit/core/adapterManager_spec.js | 102 +++++++++++++++++++++ test/spec/unit/pbjs_api_spec.js | 61 ++++++++++-- 9 files changed, 185 insertions(+), 16 deletions(-) diff --git a/src/adaptermanager.js b/src/adaptermanager.js index c324bb72e6c6..55aab7107411 100644 --- a/src/adaptermanager.js +++ b/src/adaptermanager.js @@ -529,3 +529,7 @@ exports.callBidWonBidder = function(bidder, bid, adUnits) { bid.params = utils.getUserConfiguredParams(adUnits, bid.adUnitCode, bid.bidder); tryCallBidderMethod(bidder, 'onBidWon', bid); }; + +exports.callSetTargetingBidder = function(bidder, bid) { + tryCallBidderMethod(bidder, 'onSetTargeting', bid); +}; diff --git a/src/auction.js b/src/auction.js index 221cd5198155..37b2c6896f22 100644 --- a/src/auction.js +++ b/src/auction.js @@ -290,11 +290,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 e19a80e5e02f..389cac31fe34 100644 --- a/src/auctionManager.js +++ b/src/auctionManager.js @@ -91,6 +91,11 @@ export function newAuctionManager() { auctionManager.setStatusForBids = function(adId, status) { let bid = auctionManager.findBidByAdId(adId); if (bid) bid.status = status; + + 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) { diff --git a/src/constants.json b/src/constants.json index 9ec51e4047bc..7c06db484693 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 61c5421c0d34..0460b99bbb0a 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); @@ -584,7 +584,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); }; @@ -624,7 +624,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 30407994b8ea..0fb482c3377a 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 @@ -205,7 +202,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/modules/criteoBidAdapter_spec.js b/test/spec/modules/criteoBidAdapter_spec.js index d124ebf37090..6dbf51932a0c 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 = { diff --git a/test/spec/unit/core/adapterManager_spec.js b/test/spec/unit/core/adapterManager_spec.js index bde5f234ffb1..a7b25d2524dc 100644 --- a/test/spec/unit/core/adapterManager_spec.js +++ b/test/spec/unit/core/adapterManager_spec.js @@ -134,6 +134,108 @@ 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('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('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}); diff --git a/test/spec/unit/pbjs_api_spec.js b/test/spec/unit/pbjs_api_spec.js index 215bf2cd7579..e19f40c4644b 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'; @@ -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,54 @@ 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); + }); + + 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); }); }) @@ -1885,7 +1934,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(); }); @@ -1899,7 +1948,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(); }); @@ -1915,7 +1964,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(); }); @@ -1931,7 +1980,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(); }); });