Skip to content

Commit

Permalink
Fixes prebid#3091 Auction closing prematurely (prebid#3096)
Browse files Browse the repository at this point in the history 
* auction closing early fix

* minor updates after code review
  • Loading branch information
@jaiminpanchal27 @ptomasroos
jaiminpanchal27 authored and ptomasroos committed Sep 25, 2018
1 parent 8f1764b commit fbfe6e9
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 54 deletions.
36 changes: 17 additions & 19 deletions src/auction.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
* @property {function(): void} callBids - sends requests to all adapters for bids
*/

import { uniques, flatten, timestamp, adUnitsFilter, getBidderRequest, deepAccess } from './utils';
import { uniques, flatten, timestamp, adUnitsFilter, getBidderRequest, deepAccess, delayExecution, getBidRequest } from './utils';
import { getPriceBucketString } from './cpmBucketManager';
import { getNativeTargeting } from './native';
import { getCacheUrl, store } from './videoCache';
Expand Down Expand Up @@ -157,16 +157,6 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels})
}

function auctionDone(bidderCount) {
let doneCalled = 0;
return function() {
doneCalled++;
if (doneCalled === bidderCount) {
closeAuction();
}
}
}

function closeAuction() {
// when all bidders have called done callback atleast once it means auction is complete
utils.logInfo(`Bids Received for Auction with id: ${_auctionId}`, _bidsReceived);
_auctionStatus = AUCTION_COMPLETED;
Expand Down Expand Up @@ -199,7 +189,7 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels})
};
events.emit(CONSTANTS.EVENTS.AUCTION_INIT, auctionInit);

let callbacks = auctionCallbacks(auctionDone(bidRequests.length), this);
let callbacks = auctionCallbacks(auctionDone, this);
let boundObj = {
auctionAddBidResponse: callbacks.addBidResponse
}
Expand Down Expand Up @@ -303,11 +293,15 @@ export const addBidResponse = createHook('asyncSeries', function(adUnitCode, bid

export function auctionCallbacks(auctionDone, auctionInstance) {
let outstandingBidsAdded = 0;
let doneCalled = false;
let allAdapterCalledDone = false;

let onAllAdapterDone = delayExecution(() => {
allAdapterCalledDone = true;
}, auctionInstance.getBidRequests().length);

function afterBidAdded() {
outstandingBidsAdded--;
if (doneCalled && outstandingBidsAdded === 0) {
if (allAdapterCalledDone && outstandingBidsAdded === 0) {
auctionDone()
}
}
Expand All @@ -321,16 +315,16 @@ export function auctionCallbacks(auctionDone, auctionInstance) {
let bidResponse = getPreparedBidForAuction({adUnitCode, bid, bidRequest, auctionId});

if (bidResponse.mediaType === 'video') {
tryAddVideoBid(auctionInstance, bidResponse, afterBidAdded);
tryAddVideoBid(auctionInstance, bidResponse, bidRequest, afterBidAdded);
} else {
addBidToAuction(auctionInstance, bidResponse);
afterBidAdded();
}
}

function adapterDone() {
doneCalled = true;
if ((outstandingBidsAdded === 0)) {
onAllAdapterDone();
if (allAdapterCalledDone && outstandingBidsAdded === 0) {
auctionDone();
}
}
Expand All @@ -356,9 +350,13 @@ function addBidToAuction(auctionInstance, bidResponse) {
}

// Video bids may fail if the cache is down, or there's trouble on the network.
function tryAddVideoBid(auctionInstance, bidResponse, afterBidAdded) {
function tryAddVideoBid(auctionInstance, bidResponse, bidRequests, afterBidAdded) {
let addBid = true;
const context = deepAccess(bidResponse, 'context');

const bidRequest = getBidRequest(bidResponse.adId, [bidRequests]);
const videoMediaType =
bidRequest && deepAccess(bidRequest, 'mediaTypes.video');
const context = videoMediaType && deepAccess(videoMediaType, 'context');

if (config.getConfig('cache.url') && context !== OUTSTREAM) {
if (!bidResponse.videoCacheKey) {
Expand Down
56 changes: 28 additions & 28 deletions test/fixtures/fixtures.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export function getBidRequests() {
'placementId': '4799418',
'test': 'me'
},
'placementCode': '/19968336/header-bid-tag1',
'adUnitCode': '/19968336/header-bid-tag1',
'sizes': [
[
728,
Expand All @@ -36,7 +36,7 @@ export function getBidRequests() {
'params': {
'placementId': '4799418'
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand Down Expand Up @@ -68,7 +68,7 @@ export function getBidRequests() {
'publisherId': 39741,
'adSlot': '39620189@300x250'
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand Down Expand Up @@ -117,7 +117,7 @@ export function getBidRequests() {
10
],
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand Down Expand Up @@ -146,7 +146,7 @@ export function getBidRequests() {
'params': {
'inventoryCode': 'sortable_all_right_sports'
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand Down Expand Up @@ -176,7 +176,7 @@ export function getBidRequests() {
'params': {
'tagId': 16577
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand Down Expand Up @@ -206,7 +206,7 @@ export function getBidRequests() {
'params': {
'placementId': '4799418'
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand Down Expand Up @@ -237,7 +237,7 @@ export function getBidRequests() {
'params': {
'placementId': '4799418'
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand Down Expand Up @@ -268,7 +268,7 @@ export function getBidRequests() {
'params': {
'aId': 3080
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand Down Expand Up @@ -596,7 +596,7 @@ export function getAdUnits() {
'publisher_id': '1234567',
'bidfloor': 0.01
},
'placementCode': '/19968336/header-bid-tag1',
'adUnitCode': '/19968336/header-bid-tag1',
'sizes': [
[
728,
Expand All @@ -617,7 +617,7 @@ export function getAdUnits() {
'placementId': '543221',
'test': 'me'
},
'placementCode': '/19968336/header-bid-tag1',
'adUnitCode': '/19968336/header-bid-tag1',
'sizes': [
[
728,
Expand Down Expand Up @@ -654,7 +654,7 @@ export function getAdUnits() {
'params': {
'placementId': '5324321'
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand All @@ -676,7 +676,7 @@ export function getAdUnits() {
'publisher_id': '12353433',
'bidfloor': 0.01
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand All @@ -696,7 +696,7 @@ export function getAdUnits() {
'params': {
'inventoryCode': 'inv_code_here'
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand All @@ -719,7 +719,7 @@ export function getAdUnits() {
'supplyPartnerId': 1,
'test': true
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand Down Expand Up @@ -759,7 +759,7 @@ export function getAdUnits() {
10
]
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand All @@ -780,7 +780,7 @@ export function getAdUnits() {
'jstag_url': 'http://servedbyopenx.com/w/1.0/jstag?nc=account_key',
'unit': 2345677
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand All @@ -801,7 +801,7 @@ export function getAdUnits() {
'publisherId': 1234567,
'adSlot': '1234567@300x250'
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand All @@ -821,7 +821,7 @@ export function getAdUnits() {
'params': {
'placementId': '1234567'
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand All @@ -842,7 +842,7 @@ export function getAdUnits() {
'params': {
'placementId': '1234567'
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand All @@ -865,7 +865,7 @@ export function getAdUnits() {
'siteID': 123456,
'timeout': 10000
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand All @@ -887,7 +887,7 @@ export function getAdUnits() {
'mid': 123456,
'test': 1
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand All @@ -907,7 +907,7 @@ export function getAdUnits() {
'params': {
'aId': 3080
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand All @@ -928,7 +928,7 @@ export function getAdUnits() {
'network': '112345.45',
'placement': 12345
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand All @@ -948,7 +948,7 @@ export function getAdUnits() {
'params': {
'tagid': '123556'
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand All @@ -970,7 +970,7 @@ export function getAdUnits() {
'cp': 1233456,
'ct': 12357
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand All @@ -990,7 +990,7 @@ export function getAdUnits() {
'params': {
'tagId': 75423
},
'placementCode': '/19968336/header-bid-tag-0',
'adUnitCode': '/19968336/header-bid-tag-0',
'sizes': [
[
300,
Expand Down Expand Up @@ -1367,7 +1367,7 @@ export function getBidRequestedPayload() {
'publisher_id': '5000563',
'bidfloor': 0.01
},
'placementCode': '/19968336/header-bid-tag-1',
'adUnitCode': '/19968336/header-bid-tag-1',
'sizes': [
[
300,
Expand Down
20 changes: 18 additions & 2 deletions test/spec/auctionmanager_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -888,17 +888,30 @@ describe('auctionmanager.js', function () {
});

it('should call auction done after bid is added to auction for mediaType banner', function () {
let ADUNIT_CODE2 = 'adUnitCode2';
let BIDDER_CODE2 = 'sampleBidder2';

let bids1 = [mockBid({ bidderCode: BIDDER_CODE1 })];
let bids2 = [mockBid({ bidderCode: BIDDER_CODE2 })];
bidRequests = [
mockBidRequest(bids[0]),
mockBidRequest(bids1[0], { adUnitCode: ADUNIT_CODE1 }),
mockBidRequest(bids2[0], { adUnitCode: ADUNIT_CODE2 })
];
let cbs = auctionCallbacks(doneSpy, auction);
cbs.addBidResponse(ADUNIT_CODE, bids[0]);
cbs.adapterDone();
cbs.addBidResponse(ADUNIT_CODE1, bids1[0]);
cbs.adapterDone();
cbs.addBidResponse(ADUNIT_CODE2, bids2[0]);
cbs.adapterDone();
assert.equal(doneSpy.callCount, 1);
});

it('should call auction done after prebid cache is complete for mediaType video', function() {
bids[0].mediaType = 'video';
let bids1 = [mockBid({ bidderCode: BIDDER_CODE1 })];

let opts = {
mediaType: {
video: {
Expand All @@ -908,16 +921,19 @@ describe('auctionmanager.js', function () {
}
}
bidRequests = [
mockBidRequest(bids[0], opts)
mockBidRequest(bids[0], opts),
mockBidRequest(bids1[0], { adUnitCode: ADUNIT_CODE1 }),
]

let cbs = auctionCallbacks(doneSpy, auction);
cbs.addBidResponse(ADUNIT_CODE, bids[0]);
cbs.adapterDone();
cbs.addBidResponse(ADUNIT_CODE1, bids1[0]);
cbs.adapterDone();
assert.equal(doneSpy.callCount, 0);
const uuid = 'c488b101-af3e-4a99-b538-00423e5a3371';
const responseBody = `{"responses":[{"uuid":"${uuid}"}]}`;
requests[0].respond(200, { 'Content-Type': 'application/json' }, responseBody);
cbs.adapterDone();
assert.equal(doneSpy.callCount, 1);
})
});
Expand Down
Loading

0 comments on commit fbfe6e9

Please sign in to comment.