From d8b0509441e1988c529eb6a73d050aa7ea3145bf Mon Sep 17 00:00:00 2001 From: Demetrio Girardi Date: Fri, 22 Apr 2022 07:54:52 -0700 Subject: [PATCH] PBS adapter: fix bug with priceFloors sometimes not being set in request (#8309) * PBS adapter: fix bug with priceFloors sometimes not being set in request This addresses https://github.com/prebid/Prebid.js/issues/8307 by picking what's likely to be the minimum floor for an adUnit (instead of just looking at the floor defined by the first request it finds, which may not be there) * Do not send pricefloor if any bid cannot provide one * Test errors from currency conversion * Revert whitespace changes * fix spepelling --- modules/prebidServerBidAdapter/index.js | 72 +++++-- .../modules/prebidServerBidAdapter_spec.js | 200 ++++++++++++++++++ 2 files changed, 258 insertions(+), 14 deletions(-) diff --git a/modules/prebidServerBidAdapter/index.js b/modules/prebidServerBidAdapter/index.js index d30fd5bf810..62f550eb216 100644 --- a/modules/prebidServerBidAdapter/index.js +++ b/modules/prebidServerBidAdapter/index.js @@ -38,6 +38,7 @@ import {find, includes} from '../../src/polyfill.js'; import { S2S_VENDORS } from './config.js'; import { ajax } from '../../src/ajax.js'; import {hook} from '../../src/hook.js'; +import {getGlobal} from '../../src/prebidGlobal.js'; const getConfig = config.getConfig; @@ -755,21 +756,64 @@ Object.assign(ORTB2.prototype, { deepSetValue(imp, 'ext.prebid.storedauctionresponse.id', storedAuctionResponseBid.storedAuctionResponse.toString()); } - const getFloorBid = find(firstBidRequest.bids, bid => bid.adUnitCode === adUnit.code && typeof bid.getFloor === 'function'); + const floor = (() => { + // we have to pick a floor for the imp - here we attempt to find the minimum floor + // across all bids for this adUnit + + const convertCurrency = typeof getGlobal().convertCurrency !== 'function' + ? (amount) => amount + : (amount, from, to) => { + if (from === to) return amount; + let result = null; + try { + result = getGlobal().convertCurrency(amount, from, to); + } catch (e) { + } + return result; + } + const s2sCurrency = config.getConfig('currency.adServerCurrency') || DEFAULT_S2S_CURRENCY; + + return adUnit.bids + .map((bid) => this.getBidRequest(imp.id, bid.bidder)) + .map((bid) => { + if (!bid || typeof bid.getFloor !== 'function') return; + try { + const {currency, floor} = bid.getFloor({ + currency: s2sCurrency + }); + return { + currency, + floor: parseFloat(floor) + } + } catch (e) { + logError('PBS: getFloor threw an error: ', e); + } + }) + .reduce((min, floor) => { + // if any bid does not have a valid floor, do not attempt to send any to PBS + if (floor == null || floor.currency == null || floor.floor == null || isNaN(floor.floor)) { + min.min = null; + } + if (min.min === null) { + return min; + } + // otherwise, pick the minimum one (or, in some strange confluence of circumstances, the one in the best currency) + if (min.ref == null) { + min.ref = min.min = floor; + } else { + const value = convertCurrency(floor.floor, floor.currency, min.ref.currency); + if (value != null && value < min.ref.floor) { + min.ref.floor = value; + min.min = floor; + } + } + return min; + }, {}).min + })(); - if (getFloorBid) { - let floorInfo; - try { - floorInfo = getFloorBid.getFloor({ - currency: config.getConfig('currency.adServerCurrency') || DEFAULT_S2S_CURRENCY, - }); - } catch (e) { - logError('PBS: getFloor threw an error: ', e); - } - if (floorInfo && floorInfo.currency && !isNaN(parseFloat(floorInfo.floor))) { - imp.bidfloor = parseFloat(floorInfo.floor); - imp.bidfloorcur = floorInfo.currency - } + if (floor) { + imp.bidfloor = floor.floor; + imp.bidfloorcur = floor.currency } if (imp.banner || imp.video || imp.native) { diff --git a/test/spec/modules/prebidServerBidAdapter_spec.js b/test/spec/modules/prebidServerBidAdapter_spec.js index a0c7903091c..895099a4748 100644 --- a/test/spec/modules/prebidServerBidAdapter_spec.js +++ b/test/spec/modules/prebidServerBidAdapter_spec.js @@ -18,6 +18,7 @@ import { decorateAdUnitsWithNativeParams } from '../../../src/native.js'; import { auctionManager } from '../../../src/auctionManager.js'; import { stubAuctionIndex } from '../../helpers/indexStub.js'; import { registerBidder } from 'src/adapters/bidderFactory.js'; +import {getGlobal} from '../../../src/prebidGlobal.js'; let CONFIG = { accountId: '1', @@ -1038,6 +1039,205 @@ describe('S2S Adapter', function () { }) ).to.be.true; }); + + it('should find the floor when not all bidderRequests contain it', () => { + config.setConfig({ + s2sConfig: { + ...CONFIG, + bidders: ['b1', 'b2'] + }, + }); + const bidderRequests = [ + { + ...BID_REQUESTS[0], + bidderCode: 'b1', + bids: [{ + bidder: 'b1', + bidId: 1, + }] + }, + { + ...BID_REQUESTS[0], + bidderCode: 'b2', + bids: [{ + bidder: 'b2', + bidId: 2, + getFloor: () => ({ + currency: 'CUR', + floor: 123 + }) + }], + } + ]; + const adUnits = [ + { + code: 'au1', + transactionId: 't1', + mediaTypes: { + banner: {sizes: [1, 1]} + }, + bids: [{bidder: 'b1', bid_id: 1}] + }, + { + code: 'au2', + transactionId: 't2', + bids: [{bidder: 'b2', bid_id: 2}], + mediaTypes: { + banner: {sizes: [1, 1]} + } + } + ]; + const s2sReq = { + ...REQUEST, + ad_units: adUnits + } + + adapter.callBids(s2sReq, bidderRequests, addBidResponse, done, ajax); + + const pbsReq = JSON.parse(server.requests[server.requests.length - 1].requestBody); + const [imp1, imp2] = pbsReq.imp; + + expect(imp1.bidfloor).to.be.undefined; + expect(imp1.bidfloorcur).to.be.undefined; + + expect(imp2.bidfloor).to.eql(123); + expect(imp2.bidfloorcur).to.eql('CUR'); + }); + + describe('when different bids have different floors', () => { + let s2sReq; + beforeEach(() => { + config.setConfig({ + s2sConfig: { + ...CONFIG, + bidders: ['b1', 'b2', 'b3'] + }, + }); + BID_REQUESTS = [ + { + ...BID_REQUESTS[0], + bidderCode: 'b2', + bids: [{ + bidder: 'b2', + bidId: 2, + getFloor: () => ({ + currency: '1', + floor: 2 + }) + }], + }, + { + ...BID_REQUESTS[0], + bidderCode: 'b1', + bids: [{ + bidder: 'b1', + bidId: 1, + getFloor: () => ({ + floor: 10, + currency: '0.1' + }) + }] + }, + { + ...BID_REQUESTS[0], + bidderCode: 'b3', + bids: [{ + bidder: 'b3', + bidId: 3, + getFloor: () => ({ + currency: '10', + floor: 1 + }) + }], + } + ]; + s2sReq = { + ...REQUEST, + ad_units: [ + { + code: 'au1', + transactionId: 't1', + mediaTypes: { + banner: {sizes: [1, 1]} + }, + bids: [ + {bidder: 'b2', bid_id: 2}, + {bidder: 'b3', bid_id: 3}, + {bidder: 'b1', bid_id: 1}, + ] + } + ] + }; + }); + + Object.entries({ + 'cannot compute a floor': (bid) => { bid.getFloor = () => { throw new Error() } }, + 'does not set a floor': (bid) => { delete bid.getFloor; }, + }).forEach(([t, updateBid]) => { + it(`should not set pricefloor if any one of them ${t}`, () => { + updateBid(BID_REQUESTS[1].bids[0]); + adapter.callBids(s2sReq, BID_REQUESTS, addBidResponse, done, ajax); + const pbsReq = JSON.parse(server.requests[server.requests.length - 1].requestBody); + expect(pbsReq.imp[0].bidfloor).to.be.undefined; + expect(pbsReq.imp[0].bidfloorcur).to.be.undefined; + }); + }) + + Object.entries({ + 'is available': { + expectDesc: 'minimum after conversion', + expectedFloor: 10, + expectedCur: '0.1', + conversionFn: (amount, from, to) => { + from = parseFloat(from); + to = parseFloat(to); + return amount * from / to; + }, + }, + 'is not available': { + expectDesc: 'absolute minimum', + expectedFloor: 1, + expectedCur: '10', + conversionFn: null + }, + 'is not working': { + expectDesc: 'first', + expectedFloor: 2, + expectedCur: '1', + conversionFn: () => { + throw new Error(); + } + } + }).forEach(([t, {expectDesc, expectedFloor, expectedCur, conversionFn}]) => { + describe(`and currency conversion ${t}`, () => { + let mockConvertCurrency; + const origConvertCurrency = getGlobal().convertCurrency; + beforeEach(() => { + if (conversionFn) { + getGlobal().convertCurrency = mockConvertCurrency = sinon.stub().callsFake(conversionFn) + } else { + mockConvertCurrency = null; + delete getGlobal().convertCurrency; + } + }); + + afterEach(() => { + if (origConvertCurrency != null) { + getGlobal().convertCurrency = origConvertCurrency; + } else { + delete getGlobal().convertCurrency; + } + }) + + it(`should pick the ${expectDesc}`, () => { + adapter.callBids(s2sReq, BID_REQUESTS, addBidResponse, done, ajax); + const pbsReq = JSON.parse(server.requests[server.requests.length - 1].requestBody); + expect(pbsReq.imp[0].bidfloor).to.eql(expectedFloor); + expect(pbsReq.imp[0].bidfloorcur).to.eql(expectedCur); + }); + }); + }); + }); }); it('adds device.w and device.h even if the config lacks a device object', function () {