Skip to content

Commit

Permalink
Remove duplicate request id and fix empty response from getHighesCpmB…
Browse files Browse the repository at this point in the history
…ids, getAdserverTargeting (#1970)

* Removed requestId and added auctionId

* Updated module fixtures to use auctionId and not requestId

* remove request id from external bid object and fix bug for empty result in public api

* use auctionId instead of requestId

* fixed lint errors
  • Loading branch information
jaiminpanchal27 authored and matthewlane committed Dec 19, 2017
1 parent 437cb05 commit d4e1e98
Show file tree
Hide file tree
Showing 31 changed files with 162 additions and 161 deletions.
2 changes: 1 addition & 1 deletion modules/conversantBidAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const spec = {
const secure = isPageSecure || (utils.getBidIdParameter('secure', bid.params) ? 1 : 0);

siteId = utils.getBidIdParameter('site_id', bid.params);
requestId = bid.requestId;
requestId = bid.auctionId;

const format = convertSizes(bid.sizes);

Expand Down
2 changes: 1 addition & 1 deletion modules/mobfoxBidAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export const spec = {
code: BIDDER_CODE,
aliases: ['mf'], // short code
isBidRequestValid: function (bid) {
return bid.params.s !== null && bid.params.s !== undefined && bid.requestId !== null && bid.requestId !== undefined;
return bid.params.s !== null && bid.params.s !== undefined;
},
buildRequests: function (validBidRequests) {
if (validBidRequests.length > 1) {
Expand Down
1 change: 0 additions & 1 deletion src/auction.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,6 @@ function getPreparedBidForAuction({adUnitCode, bid, bidRequests, auctionId}) {

let bidObject = Object.assign({}, bid, {
auctionId,
requestId: bidRequest.requestId,
responseTimestamp: timestamp(),
requestTimestamp: start,
cpm: parseFloat(bid.cpm) || 0,
Expand Down
23 changes: 13 additions & 10 deletions src/prebid.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/** @module pbjs */

import { getGlobal } from './prebidGlobal';
import { flatten, uniques, isGptPubadsDefined, adUnitsFilter } from './utils';
import { flatten, uniques, isGptPubadsDefined, adUnitsFilter, removeRequestId } from './utils';
import { videoAdUnit, videoBidder, hasNonVideoBidder } from './video';
import { nativeAdUnit, nativeBidder, hasNonNativeBidder } from './native';
import { listenMessagesFromCreative } from './secureCreatives';
Expand Down Expand Up @@ -113,7 +113,7 @@ $$PREBID_GLOBAL$$.getAdserverTargetingForAdUnitCode = function(adUnitCode) {

$$PREBID_GLOBAL$$.getAdserverTargeting = function (adUnitCode) {
utils.logInfo('Invoking $$PREBID_GLOBAL$$.getAdserverTargeting', arguments);
return targeting.getAllTargeting(adUnitCode);
return targeting.getAllTargeting(adUnitCode, auctionManager.getBidsReceived());
};

/**
Expand All @@ -127,16 +127,17 @@ $$PREBID_GLOBAL$$.getBidResponses = function () {
const responses = auctionManager.getBidsReceived()
.filter(adUnitsFilter.bind(this, auctionManager.getAdUnitCodes()));

// find the last requested id to get responses for most recent auction only
const currentRequestId = responses && responses.length && responses[responses.length - 1].requestId;
// find the last auction id to get responses for most recent auction only
const currentAuctionId = responses && responses.length && responses[responses.length - 1].auctionId;

return responses.map(bid => bid.adUnitCode)
return responses
.map(bid => bid.adUnitCode)
.filter(uniques).map(adUnitCode => responses
.filter(bid => bid.requestId === currentRequestId && bid.adUnitCode === adUnitCode))
.filter(bid => bid.auctionId === currentAuctionId && bid.adUnitCode === adUnitCode))
.filter(bids => bids && bids[0] && bids[0].adUnitCode)
.map(bids => {
return {
[bids[0].adUnitCode]: { bids: bids }
[bids[0].adUnitCode]: { bids: bids.map(removeRequestId) }
};
})
.reduce((a, b) => Object.assign(a, b), {});
Expand All @@ -152,7 +153,7 @@ $$PREBID_GLOBAL$$.getBidResponses = function () {
$$PREBID_GLOBAL$$.getBidResponsesForAdUnitCode = function (adUnitCode) {
const bids = auctionManager.getBidsReceived().filter(bid => bid.adUnitCode === adUnitCode);
return {
bids: bids
bids: bids.map(removeRequestId)
};
};

Expand Down Expand Up @@ -528,7 +529,8 @@ $$PREBID_GLOBAL$$.aliasBidder = function (bidderCode, alias) {
* @return {Array<AdapterBidResponse>} A list of bids that have won their respective auctions.
*/
$$PREBID_GLOBAL$$.getAllWinningBids = function () {
return auctionManager.getAllWinningBids();
return auctionManager.getAllWinningBids()
.map(removeRequestId);
};

/**
Expand All @@ -539,7 +541,8 @@ $$PREBID_GLOBAL$$.getAllWinningBids = function () {
* @return {Array} array containing highest cpm bid object(s)
*/
$$PREBID_GLOBAL$$.getHighestCpmBids = function (adUnitCode) {
return targeting.getWinningBids(adUnitCode);
return targeting.getWinningBids(adUnitCode, auctionManager.getBidsReceived())
.map(removeRequestId);
};

/**
Expand Down
26 changes: 13 additions & 13 deletions src/targeting.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,14 @@ export function newTargeting(auctionManager) {
* @param {string=} adUnitCode
* @return {Object.<string,targeting>} targeting
*/
targeting.getAllTargeting = function(adUnitCode) {
targeting.getAllTargeting = function(adUnitCode, bidsReceived = getBidsReceived()) {
const adUnitCodes = getAdUnitCodes(adUnitCode);

// Get targeting for the winning bid. Add targeting for any bids that have
// `alwaysUseBid=true`. If sending all bids is enabled, add targeting for losing bids.
var targeting = getWinningBidTargeting(adUnitCodes)
.concat(getCustomBidTargeting(adUnitCodes))
.concat(config.getConfig('enableSendAllBids') ? getBidLandscapeTargeting(adUnitCodes) : []);
var targeting = getWinningBidTargeting(adUnitCodes, bidsReceived)
.concat(getCustomBidTargeting(adUnitCodes, bidsReceived))
.concat(config.getConfig('enableSendAllBids') ? getBidLandscapeTargeting(adUnitCodes, bidsReceived) : []);

// store a reference of the targeting keys
targeting.map(adUnitCode => {
Expand Down Expand Up @@ -169,15 +169,15 @@ export function newTargeting(auctionManager) {
* @param {(string|string[])} adUnitCode adUnitCode or array of adUnitCodes
* @return {[type]} [description]
*/
targeting.getWinningBids = function(adUnitCode) {
targeting.getWinningBids = function(adUnitCode, bidsReceived = getBidsReceived()) {
const adUnitCodes = getAdUnitCodes(adUnitCode);

return getBidsReceived()
return bidsReceived
.filter(bid => includes(adUnitCodes, bid.adUnitCode))
.filter(bid => bid.cpm > 0)
.map(bid => bid.adUnitCode)
.filter(uniques)
.map(adUnitCode => getBidsReceived()
.map(adUnitCode => bidsReceived
.filter(bid => bid.adUnitCode === adUnitCode ? bid : null)
.reduce(getHighestCpm, getEmptyBid(adUnitCode)));
};
Expand Down Expand Up @@ -207,8 +207,8 @@ export function newTargeting(auctionManager) {
* @param {string[]} AdUnit code array
* @return {targetingArray} winning bids targeting
*/
function getWinningBidTargeting(adUnitCodes) {
let winners = targeting.getWinningBids(adUnitCodes);
function getWinningBidTargeting(adUnitCodes, bidsReceived) {
let winners = targeting.getWinningBids(adUnitCodes, bidsReceived);
winners.forEach((winner) => {
winner.status = BID_TARGETING_SET;
});
Expand Down Expand Up @@ -302,8 +302,8 @@ export function newTargeting(auctionManager) {
* @param {string[]} AdUnit code array
* @return {targetingArray} bids with custom targeting defined in bidderSettings
*/
function getCustomBidTargeting(adUnitCodes) {
return getBidsReceived()
function getCustomBidTargeting(adUnitCodes, bidsReceived) {
return bidsReceived
.filter(bid => includes(adUnitCodes, bid.adUnitCode))
.map(bid => Object.assign({}, bid))
.reduce(mergeAdServerTargeting, [])
Expand All @@ -316,11 +316,11 @@ export function newTargeting(auctionManager) {
* @param {string[]} AdUnit code array
* @return {targetingArray} all non-winning bids targeting
*/
function getBidLandscapeTargeting(adUnitCodes) {
function getBidLandscapeTargeting(adUnitCodes, bidsReceived) {
const standardKeys = CONSTANTS.TARGETING_KEYS.concat(NATIVE_TARGETING_KEYS);
const bids = [];
// bucket by adUnitcode
let buckets = groupBy(getBidsReceived(), 'adUnitCode');
let buckets = groupBy(bidsReceived, 'adUnitCode');
// filter top bid for each bucket by bidder
Object.keys(buckets).forEach(bucketKey => {
let bidsByBidder = groupBy(buckets[bucketKey], 'bidderCode');
Expand Down
23 changes: 22 additions & 1 deletion src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ export function getBidderRequest(bidRequests, bidder, adUnitCode) {
return find(bidRequests, request => {
return request.bids
.filter(bid => bid.bidder === bidder && bid.adUnitCode === adUnitCode).length > 0;
}) || { start: null, requestId: null };
}) || { start: null, auctionId: null };
}

/**
Expand Down Expand Up @@ -845,3 +845,24 @@ export function unsupportedBidderMessage(adUnit, unSupportedBidders) {
${plural} won't fetch demand.
`;
}

/**
* Delete property from object
* @param {Object} object
* @param {string} prop
* @return {Object} object
*/
export function deletePropertyFromObject(object, prop) {
let result = Object.assign({}, object)
delete result[prop];
return result
}

/**
* Delete requestId from external bid object.
* @param {Object} bid
* @return {Object} bid
*/
export function removeRequestId(bid) {
return exports.deletePropertyFromObject(bid, 'requestId');
}
Loading

0 comments on commit d4e1e98

Please sign in to comment.