Skip to content

Commit

Permalink
Core: fix bidderRequestsCount (prebid#11295)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dgirardi @mefjush
dgirardi authored and mefjush committed Apr 25, 2024
1 parent ac8556e commit e27ec73
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 75 deletions.
30 changes: 9 additions & 21 deletions src/adUnits.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { deepAccess } from './utils.js';

let adUnits = {};
export function reset() {
adUnits = {}
}

function ensureAdUnit(adunit, bidderCode) {
let adUnit = adUnits[adunit] = adUnits[adunit] || { bidders: {} };
Expand All @@ -21,7 +24,7 @@ function incrementAdUnitCount(adunit, counter, bidderCode) {
* @param {string} adunit id
* @returns {number} current adunit count
*/
function incrementRequestsCounter(adunit) {
export function incrementRequestsCounter(adunit) {
return incrementAdUnitCount(adunit, 'requestsCounter');
}

Expand All @@ -31,7 +34,7 @@ function incrementRequestsCounter(adunit) {
* @param {string} bidderCode code
* @returns {number} current adunit bidder requests count
*/
function incrementBidderRequestsCounter(adunit, bidderCode) {
export function incrementBidderRequestsCounter(adunit, bidderCode) {
return incrementAdUnitCount(adunit, 'requestsCounter', bidderCode);
}

Expand All @@ -41,7 +44,7 @@ function incrementBidderRequestsCounter(adunit, bidderCode) {
* @param {string} bidderCode code
* @returns {number} current adunit bidder requests count
*/
function incrementBidderWinsCounter(adunit, bidderCode) {
export function incrementBidderWinsCounter(adunit, bidderCode) {
return incrementAdUnitCount(adunit, 'winsCounter', bidderCode);
}

Expand All @@ -50,7 +53,7 @@ function incrementBidderWinsCounter(adunit, bidderCode) {
* @param {string} adunit id
* @returns {number} current adunit count
*/
function getRequestsCounter(adunit) {
export function getRequestsCounter(adunit) {
return deepAccess(adUnits, `${adunit}.requestsCounter`) || 0;
}

Expand All @@ -60,7 +63,7 @@ function getRequestsCounter(adunit) {
* @param {string} bidder code
* @returns {number} current adunit bidder requests count
*/
function getBidderRequestsCounter(adunit, bidder) {
export function getBidderRequestsCounter(adunit, bidder) {
return deepAccess(adUnits, `${adunit}.bidders.${bidder}.requestsCounter`) || 0;
}

Expand All @@ -70,21 +73,6 @@ function getBidderRequestsCounter(adunit, bidder) {
* @param {string} bidder code
* @returns {number} current adunit bidder requests count
*/
function getBidderWinsCounter(adunit, bidder) {
export function getBidderWinsCounter(adunit, bidder) {
return deepAccess(adUnits, `${adunit}.bidders.${bidder}.winsCounter`) || 0;
}

/**
* A module which counts how many times an adunit was called
* @module adunitCounter
*/
let adunitCounter = {
incrementRequestsCounter,
incrementBidderRequestsCounter,
incrementBidderWinsCounter,
getRequestsCounter,
getBidderRequestsCounter,
getBidderWinsCounter
}

export { adunitCounter };
20 changes: 15 additions & 5 deletions src/adapterManager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@ import {ajaxBuilder} from './ajax.js';
import {config, RANDOM} from './config.js';
import {hook} from './hook.js';
import {find, includes} from './polyfill.js';
import {adunitCounter} from './adUnits.js';
import {
getBidderRequestsCounter,
getBidderWinsCounter,
getRequestsCounter, incrementBidderRequestsCounter,
incrementBidderWinsCounter, incrementRequestsCounter
} from './adUnits.js';
import {getRefererInfo} from './refererDetection.js';
import {GDPR_GVLIDS, gdprDataHandler, gppDataHandler, uspDataHandler, } from './consentHandler.js';
import * as events from './events.js';
Expand Down Expand Up @@ -112,6 +117,10 @@ function getBids({bidderCode, auctionId, bidderRequestId, adUnits, src, metrics}
);
}

if (src === 'client') {
incrementBidderRequestsCounter(adUnit.code, bidderCode);
}

bids.push(Object.assign({}, bid, {
adUnitCode: adUnit.code,
transactionId: adUnit.transactionId,
Expand All @@ -122,9 +131,9 @@ function getBids({bidderCode, auctionId, bidderRequestId, adUnits, src, metrics}
auctionId,
src,
metrics,
bidRequestsCount: adunitCounter.getRequestsCounter(adUnit.code),
bidderRequestsCount: adunitCounter.getBidderRequestsCounter(adUnit.code, bid.bidder),
bidderWinsCount: adunitCounter.getBidderWinsCounter(adUnit.code, bid.bidder),
bidRequestsCount: getRequestsCounter(adUnit.code),
bidderRequestsCount: getBidderRequestsCounter(adUnit.code, bid.bidder),
bidderWinsCount: getBidderWinsCounter(adUnit.code, bid.bidder),
}));
return bids;
}, [])
Expand Down Expand Up @@ -253,6 +262,7 @@ adapterManager.makeBidRequests = hook('sync', function (adUnits, auctionStart, a
}
// filter out bidders that cannot participate in the auction
au.bids = au.bids.filter((bid) => !bid.bidder || dep.isAllowed(ACTIVITY_FETCH_BIDS, activityParams(MODULE_TYPE_BIDDER, bid.bidder)))
incrementRequestsCounter(au.code);
});

adUnits = setupAdUnitMediaTypes(adUnits, labels);
Expand Down Expand Up @@ -655,7 +665,7 @@ adapterManager.callTimedOutBidders = function(adUnits, timedOutBidders, cbTimeou
adapterManager.callBidWonBidder = function(bidder, bid, adUnits) {
// Adding user configured params to bidWon event data
bid.params = getUserConfiguredParams(adUnits, bid.adUnitCode, bid.bidder);
adunitCounter.incrementBidderWinsCounter(bid.adUnitCode, bid.bidder);
incrementBidderWinsCounter(bid.adUnitCode, bid.bidder);
tryCallBidderMethod(bidder, 'onBidWon', bid);
};

Expand Down
4 changes: 0 additions & 4 deletions src/prebid.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import {isBidUsable, targeting} from './targeting.js';
import {hook, wrapHook} from './hook.js';
import {loadSession} from './debugging.js';
import {includes} from './polyfill.js';
import {adunitCounter} from './adUnits.js';
import {createBid} from './bidfactory.js';
import {storageCallbacks} from './storageManager.js';
import {default as adapterManager, getS2SBidderSet} from './adapterManager.js';
Expand Down Expand Up @@ -593,11 +592,8 @@ export const startAuction = hook('async', function ({ bidsBackHandler, timeout:
// drop the bidder from the ad unit if it's not compatible
logWarn(unsupportedBidderMessage(adUnit, bidder));
adUnit.bids = adUnit.bids.filter(bid => bid.bidder !== bidder);
} else {
adunitCounter.incrementBidderRequestsCounter(adUnit.code, bidder);
}
});
adunitCounter.incrementRequestsCounter(adUnit.code);
});
if (!adUnits || adUnits.length === 0) {
logMessage('No adUnits configured. No bids requested.');
Expand Down
2 changes: 1 addition & 1 deletion test/spec/unit/adUnits_spec.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect } from 'chai';
import { adunitCounter } from 'src/adUnits.js';
import * as adunitCounter from 'src/adUnits.js';

describe('Adunit Counter', function () {
const ADUNIT_ID_1 = 'test1';
Expand Down
128 changes: 84 additions & 44 deletions test/spec/unit/core/adapterManager_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import {auctionManager} from '../../../../src/auctionManager.js';
import {GDPR_GVLIDS} from '../../../../src/consentHandler.js';
import {MODULE_TYPE_ANALYTICS, MODULE_TYPE_BIDDER} from '../../../../src/activities/modules.js';
import {ACTIVITY_FETCH_BIDS, ACTIVITY_REPORT_ANALYTICS} from '../../../../src/activities/activities.js';
import {reset as resetAdUnitCounters} from '../../../../src/adUnits.js';
import {deepClone} from 'src/utils.js';
var events = require('../../../../src/events');

const CONFIG = {
Expand Down Expand Up @@ -1675,12 +1677,24 @@ describe('adapterManager tests', function () {
describe('makeBidRequests', function () {
let adUnits;
beforeEach(function () {
resetAdUnitCounters();
adUnits = utils.deepClone(getAdUnits()).map(adUnit => {
adUnit.bids = adUnit.bids.filter(bid => includes(['appnexus', 'rubicon'], bid.bidder));
return adUnit;
})
});

function makeBidRequests(au = adUnits) {
return adapterManager.makeBidRequests(
au,
Date.now(),
utils.getUniqueIdentifierStr(),
function callback() {
},
[]
);
}

if (FEATURES.NATIVE) {
it('should add nativeParams to adUnits after BEFORE_REQUEST_BIDS', () => {
function beforeReqBids(adUnits) {
Expand All @@ -1692,14 +1706,7 @@ describe('adapterManager tests', function () {
}

events.on(EVENTS.BEFORE_REQUEST_BIDS, beforeReqBids);
adapterManager.makeBidRequests(
adUnits,
Date.now(),
utils.getUniqueIdentifierStr(),
function callback() {
},
[]
);
makeBidRequests();
events.off(EVENTS.BEFORE_REQUEST_BIDS, beforeReqBids);
expect(adUnits.map((u) => u.nativeParams).some(i => i == null)).to.be.false;
});
Expand All @@ -1708,13 +1715,7 @@ describe('adapterManager tests', function () {
it('should make separate bidder request objects for each bidder', () => {
adUnits = [utils.deepClone(getAdUnits()[0])];

let bidRequests = adapterManager.makeBidRequests(
adUnits,
Date.now(),
utils.getUniqueIdentifierStr(),
function callback() {},
[]
);
let bidRequests = makeBidRequests();

let sizes1 = bidRequests[1].bids[0].sizes;
let sizes2 = bidRequests[0].bids[0].sizes;
Expand All @@ -1725,6 +1726,70 @@ describe('adapterManager tests', function () {
expect(sizes1).not.to.deep.equal(sizes2);
});

it('should set and increment bidRequestsCounter', () => {
const [au1, au2] = adUnits;
makeBidRequests([au1, au2]).flatMap(br => br.bids).forEach(bid => {
expect(bid.bidRequestsCount).to.eql(1);
})
makeBidRequests([au1]);
makeBidRequests([au1, au2]).flatMap(br => br.bids).forEach(bid => {
expect(bid.bidRequestsCount).to.eql(bid.adUnitCode === au1.code ? 3 : 2);
});
})

describe('bidderRequestsCounter', () => {
it('should be set and incremented', () => {
const [au1, au2] = adUnits;
makeBidRequests([au1, au2]).flatMap(br => br.bids).forEach(bid => {
expect(bid.bidderRequestsCount).to.eql(1);
});
const au3 = {
...au2,
bids: [
au2.bids[0]
]
}
makeBidRequests([au3]);
const counts = Object.fromEntries(
makeBidRequests([au1, au2])
.map(br => [br.bidderCode, Object.fromEntries(br.bids.map(bid => [bid.adUnitCode, bid.bidderRequestsCount]))])
);
expect(counts).to.eql({
rubicon: {
[au2.code]: 2
},
appnexus: {
[au1.code]: 2,
[au2.code]: 3
},
});
});

it('should NOT be incremented for s2s bids', () => {
config.setConfig({
s2sConfig: {
enabled: true,
adapter: 'rubicon',
bidders: ['appnexus']
}
});
function expectBidderCounts(bidders) {
makeBidRequests().forEach(br => {
br.bids.forEach(bid => expect(bid.bidderRequestsCount).to.exist.and.eql(bidders[br.bidderCode]));
})
}
expectBidderCounts({
appnexus: 0,
rubicon: 1
});
config.resetConfig();
expectBidderCounts({
appnexus: 1,
rubicon: 2
})
})
});

describe('and activity controls', () => {
let redactOrtb2;
let redactBidRequest;
Expand Down Expand Up @@ -1757,13 +1822,7 @@ describe('adapterManager tests', function () {
componentType === MODULE_TYPE_BIDDER &&
allowed.includes(componentName);
});
let reqs = adapterManager.makeBidRequests(
adUnits,
Date.now(),
utils.getUniqueIdentifierStr(),
function callback() {},
[]
);
let reqs = makeBidRequests();
const bidders = Array.from(new Set(reqs.flatMap(br => br.bids).map(bid => bid.bidder)).keys());
expect(bidders).to.have.members(allowed);
});
Expand All @@ -1773,13 +1832,7 @@ describe('adapterManager tests', function () {
adUnits = [
{code: 'one', bids: [{bidder: 'mockBidder1'}]}
];
let reqs = adapterManager.makeBidRequests(
adUnits,
Date.now(),
utils.getUniqueIdentifierStr(),
function callback() {},
[]
);
let reqs = makeBidRequests();
sinon.assert.calledWith(redactBidRequest, reqs[0].bids[0]);
sinon.assert.calledWith(redactOrtb2, reqs[0].ortb2);
})
Expand Down Expand Up @@ -1812,13 +1865,7 @@ describe('adapterManager tests', function () {
{code: 'two', bids: [{module: 'pbsBidAdapter', params: {configName: 'mock1'}}, {module: 'pbsBidAdapter', params: {configName: 'mock2'}}]}
]
dep.isAllowed.callsFake(({componentType}) => componentType !== 'bidder');
let bidRequests = adapterManager.makeBidRequests(
adUnits,
Date.now(),
utils.getUniqueIdentifierStr(),
function callback() {},
[]
);
let bidRequests = makeBidRequests();
expect(new Set(bidRequests.map(br => br.uniquePbsTid)).size).to.equal(3);
});

Expand All @@ -1835,14 +1882,7 @@ describe('adapterManager tests', function () {
}
];
dep.isAllowed.callsFake((_, {configName, componentName}) => !(componentName === 'pbsBidAdapter' && configName === 'mock1'));
let bidRequests = adapterManager.makeBidRequests(
adUnits,
Date.now(),
utils.getUniqueIdentifierStr(),
function callback() {
},
[]
);
let bidRequests = makeBidRequests();
expect(new Set(bidRequests.map(br => br.uniquePbsTid)).size).to.eql(2)
});
});
Expand Down

0 comments on commit e27ec73

Please sign in to comment.