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

Multiformat support #5

Closed
wants to merge 41 commits into from
Closed

Multiformat support #5

Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
7e5986c
Merge pull request #1 from prebid/master
pm-shashank-jain Nov 27, 2018
d1146d7
Merge pull request #2 from prebid/master
pm-shashank-jain Nov 29, 2018
609ec16
changes to support native in pubmaticbid adapter
pm-shashank-jain Dec 5, 2018
792b854
Removed port from endpoint
pm-shashank-jain Dec 5, 2018
b6740d0
Removed protocol from endpoint
pm-shashank-jain Dec 5, 2018
0569f41
Formatting
pm-shashank-jain Dec 5, 2018
fd6bf03
Fix request payload
pm-shashank-jain Dec 6, 2018
bd14420
Updated test case
pm-shashank-jain Dec 6, 2018
65e2f1e
Changed request and response as per ortb spec
pm-shashank-jain Dec 7, 2018
13c87de
Change in request and response
pm-shashank-jain Dec 10, 2018
d9f3562
Removed comments and extra code
pm-shashank-jain Dec 10, 2018
adbc774
Code Review comments
pm-shashank-jain Dec 11, 2018
21fe846
Code Review Comments and Test cases for request and response
pm-shashank-jain Dec 13, 2018
9faff35
Removed data type as all data asset types are handled
pm-shashank-jain Dec 14, 2018
78cd16d
Code Review Changes
pm-shashank-jain Dec 14, 2018
4a40e1e
Code Review Comments
pm-shashank-jain Dec 17, 2018
a218d43
Supporting both banner and native and sending 0x0 in case of native
pm-shashank-jain Dec 18, 2018
f65b3fb
Bug Fixes
pm-shashank-jain Dec 21, 2018
a717fbc
Bug response not processed by prebid
pm-shashank-jain Dec 21, 2018
af2503d
Change warning message
pm-shashank-jain Dec 21, 2018
21277c1
Fixed typo
pm-shashank-jain Dec 21, 2018
53ac4fe
Merge pull request #4 from prebid/master
pm-shashank-jain Dec 21, 2018
ac1919f
Merge branch 'master' of github.com:pm-shashank-jain/Prebid.js into n…
pm-shashank-jain Dec 21, 2018
1de1623
Do not send request in case of invalid native bid
pm-shashank-jain Dec 24, 2018
6b059b7
Do not send request in case of invalid native requests
pm-shashank-jain Dec 24, 2018
9c80c46
objects converted to strings in log for debug purposes
pm-shashank-jain Dec 24, 2018
e0f8763
Fixed logic to check for required parmas
pm-shashank-jain Dec 24, 2018
c4ee495
Fixed typo for stringify
pm-shashank-jain Dec 24, 2018
d4abb65
Merge pull request #3 from pm-shashank-jain/nativesupport
pm-shashank-jain Dec 26, 2018
f143cee
documentation for native
pm-shashank-jain Dec 26, 2018
06a6e34
Merge pull request #5 from pm-shashank-jain/nativesupport
pm-shashank-jain Dec 26, 2018
84a9c02
Review comments from Prebid
pm-shashank-jain Jan 8, 2019
e3d068e
Typo
pm-shashank-jain Jan 8, 2019
468c3ab
Typo
pm-shashank-jain Jan 8, 2019
11fce22
Updated pub id for native
pm-shashank-jain Jan 10, 2019
b23e2dd
Code Review
pm-shashank-jain Jan 15, 2019
6b8c31b
Merge pull request #6 from pm-shashank-jain/nativesupport
pm-shashank-jain Jan 15, 2019
137c71c
Merge branch 'master' of https://github.com/pm-shashank-jain/Prebid.j…
pm-manasi-moghe Jan 16, 2019
9dc4880
changes to support multiformat requests in single imp
pm-manasi-moghe Jan 16, 2019
8908d88
dded new test cases and changes as per code review comments
pm-manasi-moghe Jan 18, 2019
d3ab524
Merge pull request #7 from pm-manasi-moghe/master
pm-manasi-moghe Jan 18, 2019
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
178 changes: 126 additions & 52 deletions modules/pubmaticBidAdapter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import * as utils from '../src/utils';
import { registerBidder } from '../src/adapters/bidderFactory';
import { BANNER, VIDEO, NATIVE } from '../src/mediaTypes';
import {config} from '../src/config';
import * as utils from 'src/utils';
import { registerBidder } from 'src/adapters/bidderFactory';
import { BANNER, VIDEO, NATIVE } from 'src/mediaTypes';
import {config} from 'src/config';
const constants = require('src/constants.json');
Copy link

Choose a reason for hiding this comment

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

are we not using constants?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Its not getting used anywhere in the file. hence gulp lint was throwing an error for unused variable.


const BIDDER_CODE = 'pubmatic';
const ENDPOINT = '//hbopenbid.pubmatic.com/translator?source=prebid-client';
Expand Down Expand Up @@ -207,7 +208,7 @@ function _parseAdSlot(bid) {
bid.params.adUnitIndex = '0';
bid.params.width = 0;
bid.params.height = 0;
var sizesArrayExists = (bid.hasOwnProperty('sizes') && utils.isArray(bid.sizes) && bid.sizes.length >= 1);
// var sizesArrayExists = (bid.hasOwnProperty('sizes') && utils.isArray(bid.sizes) && bid.sizes.length >= 1);
Copy link

Choose a reason for hiding this comment

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

can we delete the statement?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I have just commented out old code for now, till i am testing it. Will remove all commented code before QA drop.

bid.params.adSlot = _cleanSlot(bid.params.adSlot);

var slot = bid.params.adSlot;
Expand All @@ -219,26 +220,31 @@ function _parseAdSlot(bid) {
}
// check if size is mentioned in sizes array. in that case do not check for @ in adslot
splits = slot.split('@');
if (splits.length != 2) {
if (!(sizesArrayExists)) {
/* if (splits.length != 2) {
if (!sizesArrayExists) {
utils.logWarn('AdSlot Error: adSlot not in required format');
return;
}
}
} */
bid.params.adUnit = splits[0];
if (splits.length > 1) { // i.e size is specified in adslot, so consider that and ignore sizes array
if (splits.length > 1) {
// i.e size is specified in adslot, so consider that and ignore sizes array
splits = splits[1].split('x');
if (splits.length != 2) {
utils.logWarn('AdSlot Error: adSlot not in required format');
return;
}
bid.params.width = parseInt(splits[0]);
bid.params.height = parseInt(splits[1]);
delete bid.sizes;
} else if (sizesArrayExists) {
// delete bid.sizes;
} else {
bid.params.width = parseInt(bid.mediaTypes.banner.sizes[0][0]);
Copy link

Choose a reason for hiding this comment

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

what if mediaTypes.banner is not present? this line will throw an error.

Copy link
Owner Author

Choose a reason for hiding this comment

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

In case of prebid, for a single banner format ad, if mediaTypes is not set by publisher, prebid adds that itself. So it should not happen that mediaTypes is not present. I can add a check, just in case, but what should be the behaviour in that scenario?

bid.params.height = parseInt(bid.mediaTypes.banner.sizes[0][1]);
bid.mediaTypes.banner.sizes = bid.mediaTypes.banner.sizes.splice(1, bid.mediaTypes.banner.sizes.length - 1);
} /* else if (sizesArrayExists) {
bid.params.width = parseInt(bid.sizes[0][0]);
bid.params.height = parseInt(bid.sizes[0][1]);
}
} */
Copy link

Choose a reason for hiding this comment

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

better if we delete the commented code, we can find it in older versions of code if required.

Copy link
Owner Author

Choose a reason for hiding this comment

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

yes, will delete once i am done with development.

}

function _initConf(refererInfo) {
Expand Down Expand Up @@ -590,7 +596,10 @@ function _createImpressionObject(bid, conf) {
var impObj = {};
var bannerObj = {};
var videoObj = {};
var nativeObj = {};
var sizes = bid.hasOwnProperty('sizes') ? bid.sizes : [];
var mediaTypes = '';
var format = [];

impObj = {
id: bid.bidId,
Expand All @@ -603,42 +612,85 @@ function _createImpressionObject(bid, conf) {
bidfloorcur: bid.params.currency ? _parseSlotParam('currency', bid.params.currency) : DEFAULT_CURRENCY
};

if (bid.params.hasOwnProperty('video')) {
var videoData = bid.params.video;
if (bid.hasOwnProperty('mediaTypes')) {
for (mediaTypes in bid.mediaTypes) {
switch (mediaTypes) {
case BANNER:
bannerObj = {};
Copy link

Choose a reason for hiding this comment

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

can we move following banner specific code in a separate function like _createNativeRequest?
smaller functions make code easy to understand and maintain.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ok.

sizes = bid.mediaTypes.banner.sizes;
if (sizes !== undefined && utils.isArray(sizes)) {
if (!bid.params.width && !bid.params.height) {
bannerObj.w = sizes[0][0];
Copy link

Choose a reason for hiding this comment

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

let's add parseInt() on every size: width and height.
I think the current code accepts an array of strings as well in the same format

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok.

bannerObj.h = sizes[0][1];
sizes = sizes.splice(1, sizes.length - 1);
} else {
bannerObj.w = bid.params.width;
bannerObj.h = bid.params.height;
}
if (sizes.length > 0) {
format = [];
sizes.forEach(function (size) {
format.push({ w: size[0], h: size[1] });
});
bannerObj.format = format;
}
} else {
utils.logWarn(BIDDER_CODE + 'Error: mediaTypes.banner.size missing for adunit: ' + bid.params.adUnit + '. Ignoring the adunit.');
}
bannerObj.pos = 0;
bannerObj.topframe = utils.inIframe() ? 0 : 1;
impObj.banner = bannerObj;

for (var key in VIDEO_CUSTOM_PARAMS) {
if (videoData.hasOwnProperty(key)) {
videoObj[key] = _checkParamDataType(key, videoData[key], VIDEO_CUSTOM_PARAMS[key])
}
}
// read playersize and assign to h and w.
if (utils.isArray(bid.mediaTypes.video.playerSize[0])) {
videoObj.w = bid.mediaTypes.video.playerSize[0][0];
videoObj.h = bid.mediaTypes.video.playerSize[0][1];
} else if (utils.isNumber(bid.mediaTypes.video.playerSize[0])) {
videoObj.w = bid.mediaTypes.video.playerSize[0];
videoObj.h = bid.mediaTypes.video.playerSize[1];
}
if (bid.params.video.hasOwnProperty('skippable')) {
videoObj.ext = {
'video_skippable': bid.params.video.skippable ? 1 : 0
break;
case NATIVE:
nativeObj['request'] = JSON.stringify(_createNativeRequest(bid.nativeParams));
if (!isInvalidNativeRequest) {
impObj.native = nativeObj;
} else {
utils.logWarn(BIDDER_CODE + 'Error: Error in Native adunit ' + bid.params.adUnit + '. Ignoring the adunit. Refer to ' + PREBID_NATIVE_HELP_LINK + ' for more details.');
}

break;
case VIDEO:
var videoData = bid.params.video;
Copy link

Choose a reason for hiding this comment

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

can we move following video specific code in a separate function like _createNativeRequest?
smaller functions make code easy to understand and maintain.

if (videoData !== undefined) {
for (var key in VIDEO_CUSTOM_PARAMS) {
if (videoData.hasOwnProperty(key)) {
videoObj[key] = _checkParamDataType(key, videoData[key], VIDEO_CUSTOM_PARAMS[key]);
}
}
// read playersize and assign to h and w.
if (utils.isArray(bid.mediaTypes.video.playerSize[0])) {
videoObj.w = bid.mediaTypes.video.playerSize[0][0];
Copy link

Choose a reason for hiding this comment

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

parseInt

videoObj.h = bid.mediaTypes.video.playerSize[0][1];
} else if (utils.isNumber(bid.mediaTypes.video.playerSize[0])) {
videoObj.w = bid.mediaTypes.video.playerSize[0];
videoObj.h = bid.mediaTypes.video.playerSize[1];
}
if (bid.params.video.hasOwnProperty('skippable')) {
videoObj.ext = {
'video_skippable': bid.params.video.skippable ? 1 : 0
};
}

impObj.video = videoObj;
} else {
utils.logWarn(BIDDER_CODE + 'Error: Video config params missing for adunit: ' + bid.params.adUnit + ' with mediaType set as video. Ignoring the adunit.');
Copy link

Choose a reason for hiding this comment

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

here we are ignoring video part of the adunit, not the whole adUnit, we need to correct the message here, also for other mediaTypes.

}
break;
}
}

impObj.video = videoObj;
} else if (bid.nativeParams) {
impObj.native = {};
impObj.native['request'] = JSON.stringify(_createNativeRequest(bid.nativeParams));
} else {
// mediaTypes is not present, so this is a banner only impression
// this part of code is required for older testcases with no 'mediaTypes' to run succesfully.
bannerObj = {
pos: 0,
w: bid.params.width,
h: bid.params.height,
topframe: utils.inIframe() ? 0 : 1,
}
topframe: utils.inIframe() ? 0 : 1
};
if (utils.isArray(sizes) && sizes.length > 1) {
sizes = sizes.splice(1, sizes.length - 1);
var format = [];
sizes.forEach(size => {
format.push({
w: size[0],
Expand All @@ -649,11 +701,10 @@ function _createImpressionObject(bid, conf) {
}
impObj.banner = bannerObj;
}
if (isInvalidNativeRequest && impObj.hasOwnProperty('native')) {
utils.logWarn(BIDDER_CODE + ': Call to OpenBid will not be sent for native ad unit as it does not contain required valid native params.' + JSON.stringify(bid) + ' Refer:' + PREBID_NATIVE_HELP_LINK);
return;
}
return impObj;

return impObj.hasOwnProperty(BANNER) ||
impObj.hasOwnProperty(NATIVE) ||
impObj.hasOwnProperty(VIDEO) ? impObj : undefined;
}

function _getDigiTrustObject(key) {
Expand Down Expand Up @@ -709,6 +760,23 @@ function _handleEids(payload) {
payload.user.eids = eids;
}
}
// TODO: need a better way to identify mediaType from response.
function _checkMediaType(adm, newBid) {
var admStr = '';
if (adm.indexOf('<VAST version=>') >= 0) {
Copy link

Choose a reason for hiding this comment

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

need to consider case-insensitive regex, as there will be whitespaces
version=> will not satisfy for version=3.0>

newBid.mediaType = VIDEO;
} else if (adm.indexOf('span class="PubAPIAd"') >= 0) {
Copy link

Choose a reason for hiding this comment

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

need to consider case-insensitive regex, as there will be whitespaces

newBid.mediaType = BANNER;
}
try {
admStr = JSON.parse(adm.replace(/\\/g, ''));
if (admStr && admStr.native) {
newBid.mediaType = NATIVE;
}
} catch (e) {

}
}

function _parseNativeResponse(bid, newBid) {
newBid.native = {};
Expand All @@ -721,7 +789,7 @@ function _parseNativeResponse(bid, newBid) {
return;
}
if (adm && adm.native && adm.native.assets && adm.native.assets.length > 0) {
newBid.mediaType = 'native';
newBid.mediaType = NATIVE;
for (let i = 0, len = adm.native.assets.length; i < len; i++) {
switch (adm.native.assets[i].id) {
case NATIVE_ASSET_ID.TITLE:
Expand Down Expand Up @@ -961,14 +1029,20 @@ export const spec = {
};
if (parsedRequest.imp && parsedRequest.imp.length > 0) {
parsedRequest.imp.forEach(req => {
if (bid.impid === req.id && req.hasOwnProperty('video')) {
newBid.mediaType = 'video';
newBid.width = bid.hasOwnProperty('w') ? bid.w : req.video.w;
newBid.height = bid.hasOwnProperty('h') ? bid.h : req.video.h;
newBid.vastXml = bid.adm;
}
if (bid.impid === req.id && req.hasOwnProperty('native')) {
_parseNativeResponse(bid, newBid);
if (bid.impid === req.id) {
_checkMediaType(bid.adm, newBid);
switch (newBid.mediaType) {
case BANNER:
break;
case VIDEO:
newBid.width = bid.hasOwnProperty('w') ? bid.w : req.video.w;
newBid.height = bid.hasOwnProperty('h') ? bid.h : req.video.h;
newBid.vastXml = bid.adm;
break;
case NATIVE:
_parseNativeResponse(bid, newBid);
break;
}
}
});
}
Expand Down
Loading