Skip to content

Commit

Permalink
Fix multi-bid adId in Criteo bid adapter (prebid#3340)
Browse files Browse the repository at this point in the history
* Fix multi-bid adId in Criteo bid adapter

A single bid request can generate multiple bids if it has multiple
sizes. However, by default 'adId' will be filled with the bid request's
'bidId' field, so if we create two bids from the same bid request, they
will share the same 'adId'.

Because of this, Prebid will not know which bid to use once DFP makes
either of those bids win, always taking the first one (see
'auctionManager.findBidByAdId' used in 'pbjs.renderAd').

* Increment Criteo adapter version

* Add test for Criteo multi-bid fix
  • Loading branch information
Spark-NF authored and Pedro López Jiménez committed Mar 18, 2019
1 parent 31b18a0 commit fc40e4c
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
3 changes: 2 additions & 1 deletion modules/criteoBidAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import find from 'core-js/library/fn/array/find';
import JSEncrypt from 'jsencrypt/bin/jsencrypt';
import sha256 from 'crypto-js/sha256';

const ADAPTER_VERSION = 15;
const ADAPTER_VERSION = 16;
const BIDDER_CODE = 'criteo';
const CDB_ENDPOINT = '//bidder.criteo.com/cdb';
const CRITEO_VENDOR_ID = 91;
Expand Down Expand Up @@ -98,6 +98,7 @@ export const spec = {
const bidId = bidRequest.bidId;
const bid = {
requestId: bidId,
adId: slot.bidId || utils.getUniqueIdentifierStr(),
cpm: slot.cpm,
currency: slot.currency,
netRevenue: true,
Expand Down
38 changes: 38 additions & 0 deletions test/spec/modules/criteoBidAdapter_spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { expect } from 'chai';
import { cryptoVerify, spec, FAST_BID_PUBKEY } from 'modules/criteoBidAdapter';
import * as bidfactory from 'src/bidfactory';
import CONSTANTS from 'src/constants.json';
import * as utils from 'src/utils';

describe('The Criteo bidding adapter', function () {
Expand Down Expand Up @@ -241,6 +243,7 @@ describe('The Criteo bidding adapter', function () {
body: {
slots: [{
impid: 'test-requestId',
bidId: 'abc123',
cpm: 1.23,
creative: 'test-ad',
width: 728,
Expand All @@ -261,6 +264,7 @@ describe('The Criteo bidding adapter', function () {
const bids = spec.interpretResponse(response, request);
expect(bids).to.have.lengthOf(1);
expect(bids[0].requestId).to.equal('test-bidId');
expect(bids[0].adId).to.equal('abc123');
expect(bids[0].cpm).to.equal(1.23);
expect(bids[0].ad).to.equal('test-ad');
expect(bids[0].width).to.equal(728);
Expand Down Expand Up @@ -297,6 +301,40 @@ describe('The Criteo bidding adapter', function () {
expect(bids[0].width).to.equal(728);
expect(bids[0].height).to.equal(90);
});

it('should generate unique adIds if none are returned by the endpoint', function () {
const response = {
body: {
slots: [{
impid: 'test-requestId',
cpm: 1.23,
creative: 'test-ad',
width: 300,
height: 250,
}, {
impid: 'test-requestId',
cpm: 4.56,
creative: 'test-ad',
width: 728,
height: 90,
}],
},
};
const request = {
bidRequests: [{
adUnitCode: 'test-requestId',
bidId: 'test-bidId',
sizes: [[300, 250], [728, 90]],
params: {
networkId: 456,
}
}]
};
const bids = spec.interpretResponse(response, request);
expect(bids).to.have.lengthOf(2);
const prebidBids = bids.map(bid => Object.assign(bidfactory.createBid(CONSTANTS.STATUS.GOOD, request.bidRequests[0]), bid));
expect(prebidBids[0].adId).to.not.equal(prebidBids[1].adId);
});
});

describe('cryptoVerify', function () {
Expand Down

0 comments on commit fc40e4c

Please sign in to comment.