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.

Sign up for GitHub

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

Added the ability to send multiple bids in one ad request for mediaforce bid adapter #5834

Merged
merged 10 commits into from
Oct 12, 2020
Merged

Added the ability to send multiple bids in one ad request for mediaforce bid adapter #5834

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
95ad415
add mediaforce bid adapter
ksanksana Mar 5, 2020
5203f41
make use of unused variable language
ksanksana Mar 10, 2020
0fd7bcf
Merge remote-tracking branch 'upstream/master'
Niksok Jul 21, 2020
8471910
Added native support for Mediaforce Bid Adapter
Niksok Jul 21, 2020
6a3c4ca
Fix test endpoint url for Mediaforce Bid Adapter
Niksok Jul 22, 2020
833be29
cleanup
Niksok Jul 22, 2020
8cecd1a
Merge remote-tracking branch 'upstream/master'
Niksok Oct 1, 2020
21173a9
Added the ability to send multiple bids in one ad request for mediafo…
Niksok Oct 7, 2020
bd75f43
Merge remote-tracking branch 'upstream/master'
Niksok Oct 7, 2020
aa88b62
Fixes after review for mediaforce bid adapter
Niksok Oct 9, 2020
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
76 changes: 51 additions & 25 deletions modules/mediaforceBidAdapter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,28 @@ export const spec = {
}

const referer = bidderRequest && bidderRequest.refererInfo ? encodeURIComponent(bidderRequest.refererInfo.referer) : '';
const auctionId = bidderRequest && bidderRequest.auctionId;
const timeout = bidderRequest && bidderRequest.timeout;
const dnt = utils.getDNT() ? 1 : 0;
let requests = [];
const requestsMap = {};
const requests = [];
let isTest = false;
validBidRequests.forEach(bid => {
isTest = isTest || bid.params.is_test;
let tagid = bid.params.placement_id;
let bidfloor = bid.params.bidfloor ? parseFloat(bid.params.bidfloor) : 0;
let imp = [];
let validImp = false;
let impObj = {
id: bid.bidId,
tagid: tagid,
secure: 1,
ncolletti marked this conversation as resolved.
Show resolved Hide resolved
bidfloor: bidfloor,
ext: {
mediaforce: {
transactionId: bid.transactionId
}
}

};
for (let mediaTypes in bid.mediaTypes) {
switch (mediaTypes) {
Expand All @@ -140,31 +150,47 @@ export const spec = {
default: return;
}
}
validImp && imp.push(impObj);

let request = {
id: bid.transactionId,
site: {
page: referer,
ref: referer,
id: bid.params.publisher_id,
publisher: {
id: bid.params.publisher_id
let request = requestsMap[bid.params.publisher_id];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why you are keying against the publisher_id and sending multiple requests if the goal of the ticket is to allow for multiple bids in one request? It does not seem to change the endpoint url or any other attributes of the request.

Couldn't you organize your imp object by params.publisher_id and send that in one request?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As i wrote upper, some bids in theory can have different publisher_id in params, and because the publisher_id defines site field, which is common to all imps, we must prepare separate request for each publisher_id.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain a situation where one Prebid auction can have multiple sites or publishers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think not, but just in case, I would like to leave this opportunity

if (!request) {
request = {
id: Math.round(Math.random() * 1e16).toString(16),
site: {
page: referer,
ncolletti marked this conversation as resolved.
Show resolved Hide resolved
ref: referer,
id: bid.params.publisher_id,
publisher: {
id: bid.params.publisher_id
},
},
},
device: {
ua: navigator.userAgent,
js: 1,
dnt: dnt,
language: getLanguage()
},
imp
};
requests.push({
method: 'POST',
url: bid.params.is_test ? TEST_ENDPOINT_URL : ENDPOINT_URL,
data: JSON.stringify(request)
});
device: {
ua: navigator.userAgent,
js: 1,
dnt: dnt,
language: getLanguage()
},
ext: {
mediaforce: {
hb_key: auctionId
}
},
tmax: timeout,
imp: []
};
requestsMap[bid.params.publisher_id] = request;
requests.push({
method: 'POST',
url: ENDPOINT_URL,
data: request
});
}
validImp && request.imp.push(impObj);
});
requests.forEach((req) => {
if (isTest) {
req.url = TEST_ENDPOINT_URL;
}
req.data = JSON.stringify(req.data);
Comment on lines +189 to +193
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for the separate loop, couldn't this be applied when building the request above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameter "isTest" can be configured in any bid in the middle of list, and therefore we need make second loop after all bid was processed. And some bids in theory can have different publisher_id in params, and because the publisher_id defines site field, which is common to all imps, we must prepare separate request for each publisher_id.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I suppose you expect to have different publisher_id values with different site values from the same publisher/site. I don't see why that would occur but I'm not against your adapter supporting it.

For isTest, wouldn't you want this to be configured on a per request basis? For example, publisher_id: 203 want's test debug request url, but publisher_id: 204 in a different request does not. In this configuration, publisher_id: 204 would use the test endpoint. Again, fine with this but would like to confirm.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isTest parameter used in the main in case to test new changes before commit it. And therefore i would to leave it as is.

});
return requests;
},
Expand Down
164 changes: 161 additions & 3 deletions test/spec/modules/mediaforceBidAdapter_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,37 @@ describe('mediaforce bid adapter', function () {
transactionId: 'd45dd707-a418-42ec-b8a7-b70a6c6fab0b',
};

const multiBid = [
{
publisher_id: 'pub123',
placement_id: '202',
},
{
publisher_id: 'pub123',
placement_id: '203',
},
{
publisher_id: 'pub124',
placement_id: '202',
},
{
publisher_id: 'pub123',
placement_id: '203',
transactionId: '8df76688-1618-417a-87b1-60ad046841c9'
}
].map(({publisher_id, placement_id, transactionId}) => {
return {
bidder: 'mediaforce',
params: {publisher_id, placement_id},
mediaTypes: {
banner: {
sizes: [[300, 250], [600, 400]]
}
},
transactionId: transactionId || 'd45dd707-a418-42ec-b8a7-b70a6c6fab0b'
}
});

const refererInfo = {
referer: 'https://www.prebid.org',
reachedTop: true,
Expand All @@ -112,6 +143,7 @@ describe('mediaforce bid adapter', function () {
const requestUrl = `${baseUrl}/header_bid`;
const dnt = utils.getDNT() ? 1 : 0;
const secure = 1;
const timeout = 1500;

it('should return undefined if no validBidRequests passed', function () {
assert.equal(spec.buildRequests([]), undefined);
Expand All @@ -127,13 +159,24 @@ describe('mediaforce bid adapter', function () {
bid.params.bidfloor = 0.5;

let bidRequests = [bid];
let bidderRequest = {bids: bidRequests, refererInfo: refererInfo};
let bidderRequest = {
bids: bidRequests,
refererInfo: refererInfo,
timeout: timeout,
auctionId: '210a474e-88f0-4646-837f-4253b7cf14fb'
};

let [request] = spec.buildRequests(bidRequests, bidderRequest);

let data = JSON.parse(request.data);
assert.deepEqual(data, {
id: bid.transactionId,
id: data.id,
tmax: timeout,
ext: {
mediaforce: {
hb_key: bidderRequest.auctionId
}
},
site: {
id: bid.params.publisher_id,
publisher: {id: bid.params.publisher_id},
Expand All @@ -150,6 +193,11 @@ describe('mediaforce bid adapter', function () {
tagid: bid.params.placement_id,
secure: secure,
bidfloor: bid.params.bidfloor,
ext: {
mediaforce: {
transactionId: bid.transactionId
}
},
banner: {w: 300, h: 250},
native: {
ver: '1.2',
Expand All @@ -170,7 +218,7 @@ describe('mediaforce bid adapter', function () {
assert.deepEqual(request, {
method: 'POST',
url: requestUrl,
data: '{"id":"d45dd707-a418-42ec-b8a7-b70a6c6fab0b","site":{"page":"https%3A%2F%2Fwww.prebid.org","ref":"https%3A%2F%2Fwww.prebid.org","id":"pub123","publisher":{"id":"pub123"}},"device":{"ua":"' + navigator.userAgent + '","js":1,"dnt":' + dnt + ',"language":"' + language + '"},"imp":[{"tagid":"202","secure":1,"bidfloor":0.5,"banner":{"w":300,"h":250},"native":{"ver":"1.2","request":{"assets":[{"required":1,"id":1,"title":{"len":800}},{"required":1,"id":3,"img":{"type":3,"w":300,"h":250}},{"required":1,"id":5,"data":{"type":1}}],"context":1,"plcmttype":1,"ver":"1.2"}}}]}',
data: '{"id":"' + data.id + '","site":{"page":"https%3A%2F%2Fwww.prebid.org","ref":"https%3A%2F%2Fwww.prebid.org","id":"pub123","publisher":{"id":"pub123"}},"device":{"ua":"' + navigator.userAgent + '","js":1,"dnt":' + dnt + ',"language":"' + language + '"},"ext":{"mediaforce":{"hb_key":"210a474e-88f0-4646-837f-4253b7cf14fb"}},"tmax":1500,"imp":[{"tagid":"202","secure":1,"bidfloor":0.5,"ext":{"mediaforce":{"transactionId":"d45dd707-a418-42ec-b8a7-b70a6c6fab0b"}},"banner":{"w":300,"h":250},"native":{"ver":"1.2","request":{"assets":[{"required":1,"id":1,"title":{"len":800}},{"required":1,"id":3,"img":{"type":3,"w":300,"h":250}},{"required":1,"id":5,"data":{"type":1}}],"context":1,"plcmttype":1,"ver":"1.2"}}}]}',
});
});

Expand All @@ -186,6 +234,116 @@ describe('mediaforce bid adapter', function () {
let data = JSON.parse(request.data);
assert.deepEqual(data.imp[0].banner, {w: 300, h: 600, format: [{w: 300, h: 250}]});
});

it('should return proper requests for multiple imps', function () {
let bidderRequest = {
bids: multiBid,
refererInfo: refererInfo,
timeout: timeout,
auctionId: '210a474e-88f0-4646-837f-4253b7cf14fb'
};

let requests = spec.buildRequests(multiBid, bidderRequest);
assert.equal(requests.length, 2);
requests.forEach((req) => {
req.data = JSON.parse(req.data);
});

assert.deepEqual(requests, [
{
method: 'POST',
url: requestUrl,
data: {
id: requests[0].data.id,
tmax: timeout,
ext: {
mediaforce: {
hb_key: bidderRequest.auctionId
}
},
site: {
id: 'pub123',
publisher: {id: 'pub123'},
ref: encodeURIComponent(refererInfo.referer),
page: encodeURIComponent(refererInfo.referer),
},
device: {
ua: navigator.userAgent,
dnt: dnt,
js: 1,
language: language,
},
imp: [{
tagid: '202',
secure: secure,
bidfloor: 0,
ext: {
mediaforce: {
transactionId: 'd45dd707-a418-42ec-b8a7-b70a6c6fab0b'
}
},
banner: {w: 300, h: 250, format: [{w: 600, h: 400}]},
}, {
tagid: '203',
secure: secure,
bidfloor: 0,
ext: {
mediaforce: {
transactionId: 'd45dd707-a418-42ec-b8a7-b70a6c6fab0b'
}
},
banner: {w: 300, h: 250, format: [{w: 600, h: 400}]},
}, {
tagid: '203',
secure: secure,
bidfloor: 0,
ext: {
mediaforce: {
transactionId: '8df76688-1618-417a-87b1-60ad046841c9'
}
},
banner: {w: 300, h: 250, format: [{w: 600, h: 400}]},
}]
}
},
{
method: 'POST',
url: requestUrl,
data: {
id: requests[1].data.id,
tmax: timeout,
ext: {
mediaforce: {
hb_key: bidderRequest.auctionId
}
},
site: {
id: 'pub124',
publisher: {id: 'pub124'},
ref: encodeURIComponent(refererInfo.referer),
page: encodeURIComponent(refererInfo.referer),
},
device: {
ua: navigator.userAgent,
dnt: dnt,
js: 1,
language: language,
},
imp: [{
tagid: '202',
secure: secure,
bidfloor: 0,
ext: {
mediaforce: {
transactionId: 'd45dd707-a418-42ec-b8a7-b70a6c6fab0b'
}
},
banner: {w: 300, h: 250, format: [{w: 600, h: 400}]},
}]
}
}
]);
});
});

describe('interpretResponse() banner', function () {
Expand Down