Skip to content

Commit

Permalink
PBS adapter: fix bug with priceFloors sometimes not being set in requ…
Browse files Browse the repository at this point in the history 
…est (prebid#8309)

* PBS adapter: fix bug with priceFloors sometimes not being set in request

This addresses prebid#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
  • Loading branch information
@dgirardi
dgirardi authored Apr 22, 2022
1 parent 18612ab commit d8b0509
Show file tree
Hide file tree
Showing 2 changed files with 258 additions and 14 deletions.
72 changes: 58 additions & 14 deletions modules/prebidServerBidAdapter/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down
200 changes: 200 additions & 0 deletions test/spec/modules/prebidServerBidAdapter_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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 () {
Expand Down

0 comments on commit d8b0509

Please sign in to comment.