Skip to content

Rubicon Bid Adapter FPD Update #6122

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

Merged
merged 8 commits into from
Feb 7, 2021
Merged

Rubicon Bid Adapter FPD Update #6122

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7680ffc
Update to consolidate applying FPD to both banner and video requests.…
mmoschovas Dec 11, 2020
924fc81
Refectored last push to:
mmoschovas Dec 16, 2020
5780fd4
Consolidated other FPD data logic into new function
mmoschovas Dec 16, 2020
c8bd004
Merge remote-tracking branch 'upstream/master' into rubicon-fpd-update
mmoschovas Jan 20, 2021
030da98
1. Update to move pbadslot and adserver data into imp[] as opposed to…
mmoschovas Jan 21, 2021
330dbe3
Removed unnecessary conditional
mmoschovas Jan 21, 2021
151b55b
Changed conditional to check for undefined type
mmoschovas Jan 21, 2021
21a2471
Update to consolidate several lines of duplicate code into one location
mmoschovas Feb 3, 2021
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
148 changes: 69 additions & 79 deletions modules/rubiconBidAdapter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,46 +254,14 @@ export const spec = {
utils.deepSetValue(data, 'source.ext.schain', bidRequest.schain);
}

const siteData = Object.assign({}, bidRequest.params.inventory, config.getConfig('fpd.context'));
const userData = Object.assign({}, bidRequest.params.visitor, config.getConfig('fpd.user'));
if (!utils.isEmpty(siteData) || !utils.isEmpty(userData)) {
const bidderData = {
bidders: [ bidderRequest.bidderCode ],
config: {
fpd: {}
}
};

if (!utils.isEmpty(siteData)) {
bidderData.config.fpd.site = siteData;
}

if (!utils.isEmpty(userData)) {
bidderData.config.fpd.user = userData;
}
const bidFpd = {
user: bidRequest.params.visitor || {},
context: bidRequest.params.inventory || {}
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we also place lines 257 - 262 into applyFpd function? save a couple duplicate names.


utils.deepSetValue(data, 'ext.prebid.bidderconfig.0', bidderData);
}
if (bidRequest.params.keywords) bidFpd.context.keywords = bidRequest.params.keywords;

/**
* Prebid AdSlot
* @type {(string|undefined)}
*/
const pbAdSlot = utils.deepAccess(bidRequest, 'fpd.context.pbAdSlot');
if (typeof pbAdSlot === 'string' && pbAdSlot) {
utils.deepSetValue(data.imp[0].ext, 'context.data.pbadslot', pbAdSlot);
}

/**
* Copy GAM AdUnit and Name to imp
*/
['name', 'adSlot'].forEach(name => {
/** @type {(string|undefined)} */
const value = utils.deepAccess(bidRequest, `fpd.context.adserver.${name}`);
if (typeof value === 'string' && value) {
utils.deepSetValue(data.imp[0].ext, `context.data.adserver.${name.toLowerCase()}`, value);
}
});
applyFPD(utils.mergeDeep({}, config.getConfig('fpd') || {}, bidRequest.fpd || {}, bidFpd), VIDEO, data);

// if storedAuctionResponse has been set, pass SRID
if (bidRequest.storedAuctionResponse) {
Expand Down Expand Up @@ -547,49 +515,14 @@ export const spec = {
data['us_privacy'] = encodeURIComponent(bidderRequest.uspConsent);
}

// visitor properties
const visitorData = Object.assign({}, params.visitor, config.getConfig('fpd.user'));
Object.keys(visitorData).forEach((key) => {
if (visitorData[key] != null && key !== 'keywords') {
data[`tg_v.${key}`] = typeof visitorData[key] === 'object' && !Array.isArray(visitorData[key])
? JSON.stringify(visitorData[key])
: visitorData[key].toString(); // initialize array;
}
});

// inventory properties
const inventoryData = Object.assign({}, params.inventory, config.getConfig('fpd.context'));
Object.keys(inventoryData).forEach((key) => {
if (inventoryData[key] != null && key !== 'keywords') {
data[`tg_i.${key}`] = typeof inventoryData[key] === 'object' && !Array.isArray(inventoryData[key])
? JSON.stringify(inventoryData[key])
: inventoryData[key].toString();
}
});
const bidFpd = {
user: bidRequest.params.visitor || {},
context: bidRequest.params.inventory || {}
};

// keywords
const keywords = (params.keywords || []).concat(
utils.deepAccess(config.getConfig('fpd.user'), 'keywords') || [],
utils.deepAccess(config.getConfig('fpd.context'), 'keywords') || []);
data.kw = Array.isArray(keywords) && keywords.length ? keywords.join(',') : '';

/**
* Prebid AdSlot
* @type {(string|undefined)}
*/
const pbAdSlot = utils.deepAccess(bidRequest, 'fpd.context.pbAdSlot');
if (typeof pbAdSlot === 'string' && pbAdSlot) {
data['tg_i.pbadslot'] = pbAdSlot.replace(/^\/+/, '');
}
if (bidRequest.params.keywords) bidFpd.context.keywords = bidRequest.params.keywords;

/**
* GAM Ad Unit
* @type {(string|undefined)}
*/
const gamAdUnit = utils.deepAccess(bidRequest, 'fpd.context.adServer.adSlot');
if (typeof gamAdUnit === 'string' && gamAdUnit) {
data['tg_i.dfp_ad_unit_code'] = gamAdUnit.replace(/^\/+/, '');
}
applyFPD(utils.mergeDeep({}, config.getConfig('fpd') || {}, bidRequest.fpd || {}, bidFpd), BANNER, data);

if (config.getConfig('coppa') === true) {
data['coppa'] = 1;
Expand Down Expand Up @@ -949,6 +882,63 @@ function addVideoParameters(data, bidRequest) {
data.imp[0].video.h = size[1]
}

function applyFPD(fpd, mediaType, data) {
const map = {user: {banner: 'tg_v.', code: 'user'}, context: {banner: 'tg_i.', code: 'site'}, adserver: 'dfp_ad_unit_code'};
let obj = {};
let keywords = [];
const validate = function(e, t) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we name these vars something more useful?

The build will minify them, but for editing single letter vars are not ideal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

if (typeof e === 'object' && !Array.isArray(e)) {
utils.logWarn('Rubicon: Filtered FPD key: ', t, ': Expected value to be string, integer, or an array of strings/ints');
} else if (e) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If the value passed in is falsey I think this will not do what we want.

For example if someone wanted to pass in the literal number zero for some reason 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

return (Array.isArray(e)) ? e.filter(value => {
if (typeof value !== 'object' && value) return value;

utils.logWarn('Rubicon: Filtered value: ', value, 'for key', t, ': Expected value to be string, integer, or an array of strings/ints');
}).toString() : e.toString();
}
};

Object.keys(fpd).filter(value => fpd[value] && map[value] && typeof fpd[value] === 'object').forEach((type) => {
obj[map[type].code] = Object.keys(fpd[type]).filter(value => fpd[type][value]).reduce((result, key) => {
if (key === 'keywords') {
if (!Array.isArray(fpd[type][key]) && mediaType === BANNER) fpd[type][key] = [fpd[type][key]]

result[key] = fpd[type][key];

if (mediaType === BANNER) keywords = keywords.concat(fpd[type][key]);
} else if (key === 'data') {
utils.mergeDeep(result, {ext: {data: fpd[type][key]}});
} else if (key === 'adServer' || key === 'pbAdSlot') {
(key === 'adServer') ? ['name', 'adSlot'].forEach(name => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

These go inside of the imp array. Not at top level of the 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.

Changed to imp[0]

const value = validate(fpd[type][key][name]);

if (value) utils.mergeDeep(result, {ext: {data: {adserver: {[name.toLowerCase()]: value.replace(/^\/+/, '')}}}});
}) : utils.mergeDeep(result, {ext: {data: {[key.toLowerCase()]: fpd[type][key].replace(/^\/+/, '')}}});
} else {
utils.mergeDeep(result, {ext: {data: {[key]: fpd[type][key]}}});
}

return result;
}, {});

if (mediaType === BANNER) {
let duplicate = (typeof obj[map[type].code].ext === 'object' && obj[map[type].code].ext.data) || {};
Object.keys(duplicate).forEach((key) => {
const val = (key === 'adserver') ? duplicate.adserver.adslot : validate(duplicate[key], key);

if (val) data[(map[key]) ? `${map[type][BANNER]}${map[key]}` : `${map[type][BANNER]}${key}`] = val;
});
}
});

if (mediaType === BANNER) {
let kw = validate(keywords, 'keywords');
if (kw) data.kw = kw;
} else {
utils.mergeDeep(data, obj);
}
}

/**
* @param sizes
* @returns {*}
Expand Down
63 changes: 40 additions & 23 deletions test/spec/modules/rubiconBidAdapter_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -824,16 +824,22 @@ describe('the rubicon adapter', function () {
});
});

it('should use first party data from getConfig over the bid params, if present', () => {
it('should merge first party data from getConfig with the bid params, if present', () => {
const context = {
keywords: ['e', 'f'],
rating: '4-star'
rating: '4-star',
data: {
page: 'home'
}
};
const user = {
keywords: ['d'],
gender: 'M',
yob: '1984',
geo: {country: 'ca'}
geo: {country: 'ca'},
keywords: ['d'],
data: {
age: 40
}
};

sandbox.stub(config, 'getConfig').callsFake(key => {
Expand All @@ -847,14 +853,15 @@ describe('the rubicon adapter', function () {
});

const expectedQuery = {
'kw': 'a,b,c,d,e,f',
'kw': 'e,f,a,b,c,d',
'tg_v.ucat': 'new',
'tg_v.lastsearch': 'iphone',
'tg_v.likes': 'sports,video games',
'tg_v.gender': 'M',
'tg_v.age': '40',
'tg_v.yob': '1984',
'tg_v.geo': '{"country":"ca"}',
'tg_i.rating': '4-star',
'tg_i.rating': '5-star',
'tg_i.page': 'home',
'tg_i.prodtype': 'tech,mobile',
};

Expand Down Expand Up @@ -1827,10 +1834,16 @@ describe('the rubicon adapter', function () {
createVideoBidderRequest();

const context = {
data: {
page: 'home'
},
keywords: ['e', 'f'],
rating: '4-star'
};
const user = {
data: {
age: 31
},
keywords: ['d'],
gender: 'M',
yob: '1984',
Expand All @@ -1847,18 +1860,22 @@ describe('the rubicon adapter', function () {
return utils.deepAccess(config, key);
});

const expected = [{
bidders: ['rubicon'],
config: {
fpd: {
site: Object.assign({}, bidderRequest.bids[0].params.inventory, context),
user: Object.assign({}, bidderRequest.bids[0].params.visitor, user)
}
}
}];

const [request] = spec.buildRequests(bidderRequest.bids, bidderRequest);
expect(request.data.ext.prebid.bidderconfig).to.deep.equal(expected);

const expected = {
site: Object.assign({}, context, context.data, bidderRequest.bids[0].params.inventory),
user: Object.assign({}, user, user.data, bidderRequest.bids[0].params.visitor)
};

delete expected.site.data;
delete expected.user.data;
delete expected.site.keywords;
delete expected.user.keywords;

expect(request.data.site.keywords).to.deep.equal(['e', 'f', 'a', 'b', 'c']);
expect(request.data.user.keywords).to.deep.equal(['d']);
expect(request.data.site.ext.data).to.deep.equal(expected.site);
expect(request.data.user.ext.data).to.deep.equal(expected.user);
});

it('should include storedAuctionResponse in video bid request', function () {
Expand Down Expand Up @@ -1890,14 +1907,14 @@ describe('the rubicon adapter', function () {
);

const [request] = spec.buildRequests(bidderRequest.bids, bidderRequest);
expect(request.data.imp[0].ext.context.data.pbadslot).to.equal('1234567890');
expect(request.data.site.ext.data.pbadslot).to.equal('1234567890');
});

it('should include GAM ad unit in bid request', function () {
createVideoBidderRequest();
bidderRequest.bids[0].fpd = {
context: {
adserver: {
adServer: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was this a previous bug being fixed?

Were we looking at all lowercase when it should have been camelCase the whole time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this should have been camelCase

adSlot: '1234567890',
name: 'adServerName1'
}
Expand All @@ -1909,8 +1926,8 @@ describe('the rubicon adapter', function () {
);

const [request] = spec.buildRequests(bidderRequest.bids, bidderRequest);
expect(request.data.imp[0].ext.context.data.adserver.adslot).to.equal('1234567890');
expect(request.data.imp[0].ext.context.data.adserver.name).to.equal('adServerName1');
expect(request.data.site.ext.data.adserver.adslot).to.equal('1234567890');
expect(request.data.site.ext.data.adserver.name).to.equal('adServerName1');
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do not think this is right.

PBS expects it in the imp[].ext.context.data.adserver.adslot

Because if you have multiple imps in a PBS request, you cannot just have a single adSlot in the top level site object.

I think the logic for adServer / slotName / pbAdSlot stuff needs to be outside of the applyFPD so it can be placed in the right spots. (Or you need to refference the imp[0].ext.context.data.adserver.adslot inside the function most likely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved back to imp[0] as opposed to top level

});

it('should use the integration type provided in the config instead of the default', () => {
Expand Down Expand Up @@ -2042,7 +2059,7 @@ describe('the rubicon adapter', function () {
it('should not fail if keywords param is not an array', function () {
bidderRequest.bids[0].params.keywords = 'a,b,c';
const slotParams = spec.createSlotParams(bidderRequest.bids[0], bidderRequest);
expect(slotParams.kw).to.equal('');
expect(slotParams.kw).to.equal('a,b,c');
});
});

Expand Down