From ea6a2e8db6ac4bd62362350833cf1c89e63c80be Mon Sep 17 00:00:00 2001 From: Demetrio Girardi Date: Tue, 3 Dec 2024 09:17:14 -0800 Subject: [PATCH] Core: remove individual bids from cache when minBidCacheTTL is set --- src/auction.js | 19 ++- src/auctionManager.js | 24 +--- src/bidTTL.js | 24 ++-- src/targeting.js | 4 +- test/spec/auctionmanager_spec.js | 195 ++++++++++++++++++++----------- 5 files changed, 163 insertions(+), 103 deletions(-) diff --git a/src/auction.js b/src/auction.js index 759397275d5..9cc6dd48401 100644 --- a/src/auction.js +++ b/src/auction.js @@ -98,6 +98,8 @@ import {defer, GreedyPromise} from './utils/promise.js'; import {useMetrics} from './utils/perfMetrics.js'; import {adjustCpm} from './utils/cpm.js'; import {getGlobal} from './prebidGlobal.js'; +import {ttlCollection} from './utils/ttlCollection.js'; +import {getMinBidCacheTTL, onMinBidCacheTTLChange} from './bidTTL.js'; const { syncUsers } = userSync; @@ -153,7 +155,10 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a let _bidsRejected = []; let _callback = callback; let _bidderRequests = []; - let _bidsReceived = []; + let _bidsReceived = ttlCollection({ + startTime: (bid) => bid.responseTimestamp, + ttl: (bid) => getMinBidCacheTTL() == null ? null : Math.max(getMinBidCacheTTL(), bid.ttl) * 1000 + }); let _noBids = []; let _winningBids = []; let _auctionStart; @@ -162,8 +167,10 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a let _auctionStatus; let _nonBids = []; + onMinBidCacheTTLChange(() => _bidsReceived.refresh()); + function addBidRequests(bidderRequests) { _bidderRequests = _bidderRequests.concat(bidderRequests); } - function addBidReceived(bidsReceived) { _bidsReceived = _bidsReceived.concat(bidsReceived); } + function addBidReceived(bid) { _bidsReceived.add(bid); } function addBidRejected(bidsRejected) { _bidsRejected = _bidsRejected.concat(bidsRejected); } function addNoBid(noBid) { _noBids = _noBids.concat(noBid); } function addNonBids(seatnonbids) { _nonBids = _nonBids.concat(seatnonbids); } @@ -179,7 +186,7 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a labels: _labels, bidderRequests: _bidderRequests, noBids: _noBids, - bidsReceived: _bidsReceived, + bidsReceived: _bidsReceived.toArray(), bidsRejected: _bidsRejected, winningBids: _winningBids, timeout: _timeout, @@ -219,7 +226,7 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a bidsBackCallback(_adUnits, function () { try { if (_callback != null) { - const bids = _bidsReceived + const bids = _bidsReceived.toArray() .filter(bid => _adUnitCodes.includes(bid.adUnitCode)) .reduce(groupByPlacement, {}); _callback.apply(pbjsInstance, [bids, timedOut, _auctionId]); @@ -246,7 +253,7 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a function auctionDone() { config.resetBidder(); // when all bidders have called done callback atleast once it means auction is complete - logInfo(`Bids Received for Auction with id: ${_auctionId}`, _bidsReceived); + logInfo(`Bids Received for Auction with id: ${_auctionId}`, _bidsReceived.toArray()); _auctionStatus = AUCTION_COMPLETED; executeCallback(false); } @@ -404,7 +411,7 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels, a getAdUnits: () => _adUnits, getAdUnitCodes: () => _adUnitCodes, getBidRequests: () => _bidderRequests, - getBidsReceived: () => _bidsReceived, + getBidsReceived: () => _bidsReceived.toArray(), getNoBids: () => _noBids, getNonBids: () => _nonBids, getFPD: () => ortb2Fragments, diff --git a/src/auctionManager.js b/src/auctionManager.js index 27a2c2beaf2..ab2947b96b4 100644 --- a/src/auctionManager.js +++ b/src/auctionManager.js @@ -26,10 +26,7 @@ import {AuctionIndex} from './auctionIndex.js'; import { BID_STATUS, JSON_MAPPING } from './constants.js'; import {useMetrics} from './utils/perfMetrics.js'; import {ttlCollection} from './utils/ttlCollection.js'; -import {getTTL, onTTLBufferChange} from './bidTTL.js'; -import {config} from './config.js'; - -const CACHE_TTL_SETTING = 'minBidCacheTTL'; +import {getMinBidCacheTTL, onMinBidCacheTTLChange} from './bidTTL.js'; /** * Creates new instance of auctionManager. There will only be one instance of auctionManager but @@ -38,27 +35,14 @@ const CACHE_TTL_SETTING = 'minBidCacheTTL'; * @returns {AuctionManager} auctionManagerInstance */ export function newAuctionManager() { - let minCacheTTL = null; - const _auctions = ttlCollection({ startTime: (au) => au.end.then(() => au.getAuctionEnd()), - ttl: (au) => minCacheTTL == null ? null : au.end.then(() => { - return Math.max(minCacheTTL, ...au.getBidsReceived().map(getTTL)) * 1000 + ttl: (au) => getMinBidCacheTTL() == null ? null : au.end.then(() => { + return Math.max(getMinBidCacheTTL(), ...au.getBidsReceived().map(bid => bid.ttl)) * 1000 }), }); - onTTLBufferChange(() => { - if (minCacheTTL != null) _auctions.refresh(); - }) - - config.getConfig(CACHE_TTL_SETTING, (cfg) => { - const prev = minCacheTTL; - minCacheTTL = cfg?.[CACHE_TTL_SETTING]; - minCacheTTL = typeof minCacheTTL === 'number' ? minCacheTTL : null; - if (prev !== minCacheTTL) { - _auctions.refresh(); - } - }) + onMinBidCacheTTLChange(() => _auctions.refresh()); const auctionManager = { onExpiry: _auctions.onExpiry diff --git a/src/bidTTL.js b/src/bidTTL.js index 55ba0c026b0..d685c4aa4f0 100644 --- a/src/bidTTL.js +++ b/src/bidTTL.js @@ -1,25 +1,35 @@ import {config} from './config.js'; import {logError} from './utils.js'; +const CACHE_TTL_SETTING = 'minBidCacheTTL'; let TTL_BUFFER = 1; - +let minCacheTTL = null; const listeners = []; config.getConfig('ttlBuffer', (cfg) => { if (typeof cfg.ttlBuffer === 'number') { - const prev = TTL_BUFFER; TTL_BUFFER = cfg.ttlBuffer; - if (prev !== TTL_BUFFER) { - listeners.forEach(l => l(TTL_BUFFER)) - } } else { logError('Invalid value for ttlBuffer', cfg.ttlBuffer); } }) -export function getTTL(bid) { +export function getBufferedTTL(bid) { return bid.ttl - (bid.hasOwnProperty('ttlBuffer') ? bid.ttlBuffer : TTL_BUFFER); } -export function onTTLBufferChange(listener) { +export function getMinBidCacheTTL() { + return minCacheTTL; +} + +config.getConfig(CACHE_TTL_SETTING, (cfg) => { + const prev = minCacheTTL; + minCacheTTL = cfg?.[CACHE_TTL_SETTING]; + minCacheTTL = typeof minCacheTTL === 'number' ? minCacheTTL : null; + if (prev !== minCacheTTL) { + listeners.forEach(l => l(minCacheTTL)) + } +}) + +export function onMinBidCacheTTLChange(listener) { listeners.push(listener); } diff --git a/src/targeting.js b/src/targeting.js index aecc29787c7..55783c9632c 100644 --- a/src/targeting.js +++ b/src/targeting.js @@ -1,5 +1,5 @@ import { auctionManager } from './auctionManager.js'; -import { getTTL } from './bidTTL.js'; +import { getBufferedTTL } from './bidTTL.js'; import { bidderSettings } from './bidderSettings.js'; import { config } from './config.js'; import { @@ -48,7 +48,7 @@ export const TARGETING_KEYS_ARR = Object.keys(TARGETING_KEYS).map( ); // return unexpired bids -const isBidNotExpired = (bid) => (bid.responseTimestamp + getTTL(bid) * 1000) > timestamp(); +const isBidNotExpired = (bid) => (bid.responseTimestamp + getBufferedTTL(bid) * 1000) > timestamp(); // return bids whose status is not set. Winning bids can only have a status of `rendered`. const isUnusedBid = (bid) => bid && ((bid.status && !includes([BID_STATUS.RENDERED], bid.status)) || !bid.status); diff --git a/test/spec/auctionmanager_spec.js b/test/spec/auctionmanager_spec.js index afc90fae342..72a8940e4d8 100644 --- a/test/spec/auctionmanager_spec.js +++ b/test/spec/auctionmanager_spec.js @@ -29,6 +29,7 @@ import { setConfig as setCurrencyConfig } from '../../modules/currency.js'; import { REJECTION_REASON } from '../../src/constants.js'; import { setDocumentHidden } from './unit/utils/focusTimeout_spec.js'; import {sandbox} from 'sinon'; +import {getMinBidCacheTTL, onMinBidCacheTTLChange} from '../../src/bidTTL.js'; /** * @typedef {import('../src/adapters/bidderFactory.js').BidRequest} BidRequest @@ -849,7 +850,23 @@ describe('auctionmanager.js', function () { await auction.requestsDone; }) - describe('stale auctions', () => { + describe('setConfig(minBidCacheTTL)', () => { + it('should update getMinBidCacheTTL', () => { + expect(getMinBidCacheTTL()).to.eql(null); + config.setConfig({minBidCacheTTL: 123}); + expect(getMinBidCacheTTL()).to.eql(123); + }); + + it('should run listeners registered with onMinBidCacheTTLChange', () => { + config.setConfig({minBidCacheTTL: 1}); + let newTTL = null; + onMinBidCacheTTLChange((ttl) => { newTTL = ttl; }); + config.setConfig({minBidCacheTTL: 2}); + expect(newTTL).to.eql(2); + }) + }) + + describe('minBidCacheTTL', () => { let clock, auction; beforeEach(() => { clock = sinon.useFakeTimers(); @@ -861,79 +878,121 @@ describe('auctionmanager.js', function () { config.resetConfig(); }); - it('are dropped after their last bid becomes stale (if minBidCacheTTL is set)', () => { - config.setConfig({ - minBidCacheTTL: 0 - }); - bids = [ - { - adUnitCode: ADUNIT_CODE, - adUnitId: ADUNIT_CODE, - ttl: 10 - }, { - adUnitCode: ADUNIT_CODE, - adUnitId: ADUNIT_CODE, - ttl: 100 - } - ]; - auction.callBids(); - return auction.end.then(() => { - clock.tick(50 * 1000); - expect(auctionManager.getBidsReceived().length).to.equal(2); - clock.tick(56 * 1000); - expect(auctionManager.getBidsReceived()).to.eql([]); + describe('individual bids', () => { + beforeEach(() => { + bids = [ + { + adUnitCode: ADUNIT_CODE, + adUnitId: ADUNIT_CODE, + ttl: 10 + }, { + adUnitCode: ADUNIT_CODE, + adUnitId: ADUNIT_CODE, + ttl: 100 + } + ]; + }) + it('are dropped when stale (if minBidCacheTTL is set)', () => { + config.setConfig({ + minBidCacheTTL: 30 + }); + auction.callBids(); + return auction.end.then(() => { + clock.tick(20 * 1000); + expect(auctionManager.getBidsReceived().length).to.equal(2); + clock.tick(50 * 1000); + expect(auctionManager.getBidsReceived().length).to.equal(1); + }); }); - }); - it('are dropped after `minBidCacheTTL` seconds if they had no bid', () => { - auction.callBids(); - config.setConfig({ - minBidCacheTTL: 2 - }); - return auction.end.then(() => { - expect(auctionManager.getNoBids().length).to.eql(1); - clock.tick(10 * 10000); - expect(auctionManager.getNoBids().length).to.eql(0); + it('pick up updates to minBidCacheTTL that happen during bid lifetime', () => { + auction.callBids(); + return auction.end.then(() => { + clock.tick(10 * 1000); + config.setConfig({ + minBidCacheTTL: 20 + }) + clock.tick(20 * 1000); + expect(auctionManager.getBidsReceived().length).to.equal(1); + }); }) - }); + }) - it('are not dropped after `minBidCacheTTL` seconds if the page was hidden', () => { - auction.callBids(); - config.setConfig({ - minBidCacheTTL: 10 + describe('stale auctions', () => { + it('are dropped after their last bid becomes stale (if minBidCacheTTL is set)', () => { + config.setConfig({ + minBidCacheTTL: 90 + }); + bids = [ + { + adUnitCode: ADUNIT_CODE, + adUnitId: ADUNIT_CODE, + ttl: 10 + }, { + adUnitCode: ADUNIT_CODE, + adUnitId: ADUNIT_CODE, + ttl: 100 + } + ]; + auction.callBids(); + return auction.end.then(() => { + clock.tick(50 * 1000); + expect(auctionManager.getBidsReceived().length).to.equal(2); + clock.tick(56 * 1000); + expect(auctionManager.getBidsReceived()).to.eql([]); + }); }); - return auction.end.then(() => { - expect(auctionManager.getNoBids().length).to.eql(1); - setDocumentHidden(true); - clock.tick(10 * 10000); - setDocumentHidden(false); - expect(auctionManager.getNoBids().length).to.eql(1); - }) - }); - Object.entries({ - 'bids': { - bd: [{ - adUnitCode: ADUNIT_CODE, - adUnitId: ADUNIT_CODE, - ttl: 10 - }], - entries: () => auctionManager.getBidsReceived() - }, - 'no bids': { - bd: [], - entries: () => auctionManager.getNoBids() - } - }).forEach(([t, {bd, entries}]) => { - it(`with ${t} are never dropped if minBidCacheTTL is not set`, () => { - bids = bd; + it('are dropped after `minBidCacheTTL` seconds if they had no bid', () => { auction.callBids(); + config.setConfig({ + minBidCacheTTL: 2 + }); return auction.end.then(() => { - clock.tick(100 * 1000); - expect(entries().length > 0).to.be.true; + expect(auctionManager.getNoBids().length).to.eql(1); + clock.tick(10 * 10000); + expect(auctionManager.getNoBids().length).to.eql(0); }) - }) - }); + }); + + it('are not dropped after `minBidCacheTTL` seconds if the page was hidden', () => { + auction.callBids(); + config.setConfig({ + minBidCacheTTL: 10 + }); + return auction.end.then(() => { + expect(auctionManager.getNoBids().length).to.eql(1); + setDocumentHidden(true); + clock.tick(10 * 10000); + setDocumentHidden(false); + expect(auctionManager.getNoBids().length).to.eql(1); + }) + }); + + Object.entries({ + 'bids': { + bd: [{ + adUnitCode: ADUNIT_CODE, + adUnitId: ADUNIT_CODE, + ttl: 10 + }], + entries: () => auctionManager.getBidsReceived() + }, + 'no bids': { + bd: [], + entries: () => auctionManager.getNoBids() + } + }).forEach(([t, {bd, entries}]) => { + it(`with ${t} are never dropped if minBidCacheTTL is not set`, () => { + bids = bd; + auction.callBids(); + return auction.end.then(() => { + clock.tick(100 * 1000); + expect(entries().length > 0).to.be.true; + }) + }) + }); + }) }) }); @@ -1443,10 +1502,10 @@ describe('auctionmanager.js', function () { it('should not alter bid requestID', function () { auction.callBids(); - - const addedBid2 = auction.getBidsReceived().pop(); + const bidsReceived = auction.getBidsReceived(); + const addedBid2 = bidsReceived.pop(); assert.equal(addedBid2.requestId, bids1[0].requestId); - const addedBid1 = auction.getBidsReceived().pop(); + const addedBid1 = bidsReceived.pop(); assert.equal(addedBid1.requestId, bids[0].requestId); });