Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core: fix bidderRequestsCount not incrementing when used with s2sTesting #11295

Merged
merged 2 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 9 additions & 21 deletions src/adUnits.js
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
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
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
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
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