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.

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

Prebid Core: refactor bidderSettings to have only one entry point #7712

Merged
merged 13 commits into from
Jan 25, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
3 changes: 2 additions & 1 deletion modules/priceFloors.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { getHook } from '../src/hook.js';
import { createBid } from '../src/bidfactory.js';
import find from 'core-js-pure/features/array/find.js';
import { getRefererInfo } from '../src/refererDetection.js';
import {bidderSettings} from '../src/bidderSettings.js';

/**
* @summary This Module is intended to provide users with the ability to dynamically set and enforce price floors on a per auction basis.
Expand Down Expand Up @@ -147,7 +148,7 @@ function generatePossibleEnumerations(arrayOfFields, delimiter) {
* @summary If a the input bidder has a registered cpmadjustment it returns the input CPM after being adjusted
*/
export function getBiddersCpmAdjustment(bidderName, inputCpm, bid = {}) {
const adjustmentFunction = deepAccess(getGlobal(), `bidderSettings.${bidderName}.bidCpmAdjustment`) || deepAccess(getGlobal(), 'bidderSettings.standard.bidCpmAdjustment');
const adjustmentFunction = bidderSettings.get(bidderName, 'bidCpmAdjustment');
if (adjustmentFunction) {
return parseFloat(adjustmentFunction(inputCpm, {...bid, cpm: inputCpm}));
}
Expand Down
9 changes: 5 additions & 4 deletions modules/sortableAnalyticsAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import adapterManager from '../src/adapterManager.js';
import {ajax} from '../src/ajax.js';
import {getGlobal} from '../src/prebidGlobal.js';
import { config } from '../src/config.js';
import {bidderSettings} from '../src/bidderSettings.js';

const DEFAULT_PROTOCOL = 'https';
const DEFAULT_HOST = 'pa.deployads.com';
Expand Down Expand Up @@ -182,11 +183,11 @@ function getFactor(bidder) {
}

function getBiddersFactors() {
const pb = getGlobal();
const result = {};
if (pb && pb.bidderSettings) {
Object.keys(pb.bidderSettings).forEach(bidderKey => {
const bidder = pb.bidderSettings[bidderKey];
const settings = bidderSettings.getSettings();
if (settings) {
Object.keys(settings).forEach(bidderKey => {
const bidder = settings[bidderKey];
const factor = getFactor(bidder);
if (factor !== null) {
result[bidderKey] = factor;
Expand Down
121 changes: 58 additions & 63 deletions src/auction.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@

import {
flatten, timestamp, adUnitsFilter, deepAccess, getBidRequest, getValue, parseUrl, generateUUID,
logMessage, bind, logError, logInfo, logWarn, isEmpty, _each, isFn, isEmptyStr, isAllowZeroCpmBidsEnabled
logMessage, bind, logError, logInfo, logWarn, isEmpty, _each, isFn, isEmptyStr, cache, isAllowZeroCpmBidsEnabled
} from './utils.js';
import { getPriceBucketString } from './cpmBucketManager.js';
import { getNativeTargeting } from './native.js';
Expand All @@ -72,6 +72,7 @@ import find from 'core-js-pure/features/array/find.js';
import includes from 'core-js-pure/features/array/includes.js';
import { OUTSTREAM } from './video.js';
import { VIDEO } from './mediaTypes.js';
import {bidderSettings} from './bidderSettings.js';

const { syncUsers } = userSync;

Expand Down Expand Up @@ -615,18 +616,18 @@ export const getPriceGranularity = (mediaType, bidReq) => {
*/
export const getPriceByGranularity = (granularity) => {
return (bid, bidReq) => {
granularity = granularity || getPriceGranularity(bid.mediaType, bidReq);
if (granularity === CONSTANTS.GRANULARITY_OPTIONS.AUTO) {
const bidGranularity = granularity || getPriceGranularity(bid.mediaType, bidReq);
if (bidGranularity === CONSTANTS.GRANULARITY_OPTIONS.AUTO) {
return bid.pbAg;
} else if (granularity === CONSTANTS.GRANULARITY_OPTIONS.DENSE) {
} else if (bidGranularity === CONSTANTS.GRANULARITY_OPTIONS.DENSE) {
return bid.pbDg;
} else if (granularity === CONSTANTS.GRANULARITY_OPTIONS.LOW) {
} else if (bidGranularity === CONSTANTS.GRANULARITY_OPTIONS.LOW) {
return bid.pbLg;
} else if (granularity === CONSTANTS.GRANULARITY_OPTIONS.MEDIUM) {
} else if (bidGranularity === CONSTANTS.GRANULARITY_OPTIONS.MEDIUM) {
return bid.pbMg;
} else if (granularity === CONSTANTS.GRANULARITY_OPTIONS.HIGH) {
} else if (bidGranularity === CONSTANTS.GRANULARITY_OPTIONS.HIGH) {
return bid.pbHg;
} else if (granularity === CONSTANTS.GRANULARITY_OPTIONS.CUSTOM) {
} else if (bidGranularity === CONSTANTS.GRANULARITY_OPTIONS.CUSTOM) {
return bid.pbCg;
}
}
Expand All @@ -642,47 +643,51 @@ export const getAdvertiserDomain = () => {
}
}

// factory for key value objs
function createKeyVal(key, value) {
return {
key,
val: (typeof value === 'function')
? function (bidResponse, bidReq) {
return value(bidResponse, bidReq);
}
: function (bidResponse) {
return getValue(bidResponse, value);
}
};
}

const defaultAdserverTargeting = cache(() => {
FilipStamenkovic marked this conversation as resolved.
Show resolved Hide resolved
const TARGETING_KEYS = CONSTANTS.TARGETING_KEYS;
return [
createKeyVal(TARGETING_KEYS.BIDDER, 'bidderCode'),
createKeyVal(TARGETING_KEYS.AD_ID, 'adId'),
createKeyVal(TARGETING_KEYS.PRICE_BUCKET, getPriceByGranularity()),
createKeyVal(TARGETING_KEYS.SIZE, 'size'),
createKeyVal(TARGETING_KEYS.DEAL, 'dealId'),
createKeyVal(TARGETING_KEYS.SOURCE, 'source'),
createKeyVal(TARGETING_KEYS.FORMAT, 'mediaType'),
createKeyVal(TARGETING_KEYS.ADOMAIN, getAdvertiserDomain()),
]
})

/**
* @param {string} mediaType
* @param {string} bidderCode
* @param {BidRequest} bidReq
* @returns {*}
*/
export function getStandardBidderSettings(mediaType, bidderCode) {
// factory for key value objs
function createKeyVal(key, value) {
return {
key,
val: (typeof value === 'function')
? function (bidResponse, bidReq) {
return value(bidResponse, bidReq);
}
: function (bidResponse) {
return getValue(bidResponse, value);
}
};
}
const TARGETING_KEYS = CONSTANTS.TARGETING_KEYS;
const standardSettings = Object.assign({}, bidderSettings.settingsFor(null));

let bidderSettings = $$PREBID_GLOBAL$$.bidderSettings;
if (!bidderSettings[CONSTANTS.JSON_MAPPING.BD_SETTING_STANDARD]) {
bidderSettings[CONSTANTS.JSON_MAPPING.BD_SETTING_STANDARD] = {};
}
if (!bidderSettings[CONSTANTS.JSON_MAPPING.BD_SETTING_STANDARD][CONSTANTS.JSON_MAPPING.ADSERVER_TARGETING]) {
bidderSettings[CONSTANTS.JSON_MAPPING.BD_SETTING_STANDARD][CONSTANTS.JSON_MAPPING.ADSERVER_TARGETING] = [
createKeyVal(TARGETING_KEYS.BIDDER, 'bidderCode'),
createKeyVal(TARGETING_KEYS.AD_ID, 'adId'),
createKeyVal(TARGETING_KEYS.PRICE_BUCKET, getPriceByGranularity()),
createKeyVal(TARGETING_KEYS.SIZE, 'size'),
createKeyVal(TARGETING_KEYS.DEAL, 'dealId'),
createKeyVal(TARGETING_KEYS.SOURCE, 'source'),
createKeyVal(TARGETING_KEYS.FORMAT, 'mediaType'),
createKeyVal(TARGETING_KEYS.ADOMAIN, getAdvertiserDomain()),
]
if (!standardSettings[CONSTANTS.JSON_MAPPING.ADSERVER_TARGETING]) {
standardSettings[CONSTANTS.JSON_MAPPING.ADSERVER_TARGETING] = defaultAdserverTargeting();
}

if (mediaType === 'video') {
const adserverTargeting = bidderSettings[CONSTANTS.JSON_MAPPING.BD_SETTING_STANDARD][CONSTANTS.JSON_MAPPING.ADSERVER_TARGETING];
const adserverTargeting = standardSettings[CONSTANTS.JSON_MAPPING.ADSERVER_TARGETING].slice();
standardSettings[CONSTANTS.JSON_MAPPING.ADSERVER_TARGETING] = adserverTargeting;

// Adding hb_uuid + hb_cache_id
[TARGETING_KEYS.UUID, TARGETING_KEYS.CACHE_ID].forEach(targetingKeyVal => {
Expand All @@ -692,7 +697,7 @@ export function getStandardBidderSettings(mediaType, bidderCode) {
});

// Adding hb_cache_host
if (config.getConfig('cache.url') && (!bidderCode || deepAccess(bidderSettings, `${bidderCode}.sendStandardTargeting`) !== false)) {
if (config.getConfig('cache.url') && (!bidderCode || bidderSettings.get(bidderCode, 'sendStandardTargeting') !== false)) {
Comment on lines -730 to +735
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is potentially a breaking change: previously, this would be evaluated the first time this function was called with mediaType === 'video' and would set hb_cache_host globally after that. There was also no fallback to bidderSettings.standard.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Breaking change... more like a bug fix!

const urlInfo = parseUrl(config.getConfig('cache.url'));

if (typeof find(adserverTargeting, targetingKeyVal => targetingKeyVal.key === TARGETING_KEYS.CACHE_HOST) === 'undefined') {
Expand All @@ -703,7 +708,7 @@ export function getStandardBidderSettings(mediaType, bidderCode) {
}
}
}
return bidderSettings[CONSTANTS.JSON_MAPPING.BD_SETTING_STANDARD];
return standardSettings;
}

export function getKeyValueTargetingPairs(bidderCode, custBidObj, bidReq) {
Expand All @@ -712,19 +717,15 @@ export function getKeyValueTargetingPairs(bidderCode, custBidObj, bidReq) {
}

var keyValues = {};
var bidderSettings = $$PREBID_GLOBAL$$.bidderSettings;

// 1) set the keys from "standard" setting or from prebid defaults
if (bidderSettings) {
// initialize default if not set
const standardSettings = getStandardBidderSettings(custBidObj.mediaType, bidderCode);
setKeys(keyValues, standardSettings, custBidObj, bidReq);

// 2) set keys from specific bidder setting override if they exist
if (bidderCode && bidderSettings[bidderCode] && bidderSettings[bidderCode][CONSTANTS.JSON_MAPPING.ADSERVER_TARGETING]) {
setKeys(keyValues, bidderSettings[bidderCode], custBidObj, bidReq);
custBidObj.sendStandardTargeting = bidderSettings[bidderCode].sendStandardTargeting;
}
const standardSettings = getStandardBidderSettings(custBidObj.mediaType, bidderCode);
setKeys(keyValues, standardSettings, custBidObj, bidReq);

// 2) set keys from specific bidder setting override if they exist
if (bidderCode && bidderSettings.getOwn(bidderCode, CONSTANTS.JSON_MAPPING.ADSERVER_TARGETING)) {
setKeys(keyValues, bidderSettings.ownSettingsFor(bidderCode), custBidObj, bidReq);
custBidObj.sendStandardTargeting = bidderSettings.get(bidderCode, 'sendStandardTargeting');
Copy link
Collaborator Author

@dgirardi dgirardi Nov 16, 2021

Choose a reason for hiding this comment

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

Another potentially breaking change: the logic here was a bit confusing, copying sendStandardTargeting only if adserverTargeting is defined, which was also overriding standard.sendStandardTargeting. From the public documentation it seems like we should be using the standard setting if there is no bidder-specific one.

}

// set native key value targeting
Expand Down Expand Up @@ -776,19 +777,13 @@ function setKeys(keyValues, bidderSettings, custBidObj, bidReq) {
export function adjustBids(bid) {
let code = bid.bidderCode;
let bidPriceAdjusted = bid.cpm;
let bidCpmAdjustment;
if ($$PREBID_GLOBAL$$.bidderSettings) {
if (code && $$PREBID_GLOBAL$$.bidderSettings[code] && typeof $$PREBID_GLOBAL$$.bidderSettings[code].bidCpmAdjustment === 'function') {
bidCpmAdjustment = $$PREBID_GLOBAL$$.bidderSettings[code].bidCpmAdjustment;
} else if ($$PREBID_GLOBAL$$.bidderSettings[CONSTANTS.JSON_MAPPING.BD_SETTING_STANDARD] && typeof $$PREBID_GLOBAL$$.bidderSettings[CONSTANTS.JSON_MAPPING.BD_SETTING_STANDARD].bidCpmAdjustment === 'function') {
bidCpmAdjustment = $$PREBID_GLOBAL$$.bidderSettings[CONSTANTS.JSON_MAPPING.BD_SETTING_STANDARD].bidCpmAdjustment;
}
if (bidCpmAdjustment) {
try {
bidPriceAdjusted = bidCpmAdjustment(bid.cpm, Object.assign({}, bid));
} catch (e) {
logError('Error during bid adjustment', 'bidmanager.js', e);
}
const bidCpmAdjustment = bidderSettings.get(code || null, 'bidCpmAdjustment');

if (bidCpmAdjustment && typeof bidCpmAdjustment === 'function') {
try {
bidPriceAdjusted = bidCpmAdjustment(bid.cpm, Object.assign({}, bid));
} catch (e) {
logError('Error during bid adjustment', 'bidmanager.js', e);
}
}

Expand Down
67 changes: 67 additions & 0 deletions src/bidderSettings.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import {deepAccess, deepMerge} from './utils.js';
import {getGlobal} from './prebidGlobal.js';

const CONSTANTS = require('./constants.json');

export function ScopedSettings(getSettings, defaultScope) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not big fan of OOP concept.
Looking at various modules in pbjs we barely use OOP concept.

In modules we have exported functions (that can be used in other modules/bidders) and private functions that are used only inside that module.
To me it looks much cleaner this way. But, this isn't hard requirement from my side, if you feel OOP is a way to go, I won't object.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason to not using Class if you choose OOP?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is there a reason to not using Class if you choose OOP?

Because when I started working on this IE11 was still a target :) I don't have a strong preference on the syntax, I can rewrite using class - should I take your comment as an encouragement?

I'm not big fan of OOP concept.

Then some day I'll have to explain to you why you're wrong.

But more seriously, I don't understand how you can say we barely use OOP - config, auctionManager, auction, adapterManager, adapters, etc are all objects with capabilities. Is the problem that I use prototype instead of hidden closures as "private" methods? or the fact that you have to use new for this function but not for the others?

Copy link
Collaborator

Choose a reason for hiding this comment

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

should I take your comment as an encouragement?

Yes, totally :)

this.getSettings = getSettings;
this.defaultScope = defaultScope;
}

/**
* Get setting value at `path` under the given scope, falling back to the default scope if needed.
* If `scope` is `null`, get the setting's default value.
* @param scope {String|null}
* @param path {String}
* @returns {*}
*/
ScopedSettings.prototype.get = function (scope, path) {
let value = this.getOwn(scope, path);
if (typeof value === 'undefined') {
value = this.getOwn(null, path);
}
return value;
}

/**
* Get the setting value at `path` *without* falling back to the default value.
* @param scope {String}
* @param path {String}
* @returns {*}
*/
ScopedSettings.prototype.getOwn = function (scope, path) {
scope = this.resolveScope(scope);
return deepAccess(this.getSettings(), `${scope}.${path}`)
}

/**
* @returns {string[]} all existing scopes except the default one.
*/
ScopedSettings.prototype.getScopes = function () {
return Object.keys(this.getSettings()).filter((scope) => scope !== this.defaultScope);
}

/**
* @returns all settings in the given scope, merged with the settings for the default scope.
*/
ScopedSettings.prototype.settingsFor = function (scope) {
return deepMerge(this.ownSettingsFor(null), this.ownSettingsFor(scope));
}

/**
* @returns all settings in the given scope, *without* any of the default settings.
*/
ScopedSettings.prototype.ownSettingsFor = function (scope) {
scope = this.resolveScope(scope);
return this.getSettings()[scope] || {};
}

ScopedSettings.prototype.resolveScope = function (scope) {
if (scope == null) {
return this.defaultScope;
} else {
return scope;
}
}

export const bidderSettings = new ScopedSettings(() => getGlobal().bidderSettings || {}, CONSTANTS.JSON_MAPPING.BD_SETTING_STANDARD);
46 changes: 45 additions & 1 deletion src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1299,5 +1299,49 @@ export function cyrb53Hash(str, seed = 0) {
export function isAllowZeroCpmBidsEnabled(bidderCode) {
const bidderSettings = getGlobal().bidderSettings;
return ((bidderSettings[bidderCode] && bidderSettings[bidderCode].allowZeroCpmBids === true) ||
(bidderSettings.standard && bidderSettings.standard.allowZeroCpmBids === true));
(bidderSettings.standard && bidderSettings.standard.allowZeroCpmBids === true));
}

/**
* Return a memoized version of the given function that uses the first argument as cache key.
*/
export function memoize1(fun) {
FilipStamenkovic marked this conversation as resolved.
Show resolved Hide resolved
const cache = {};
return function (a0, ...rest) {
if (!cache.hasOwnProperty(a0)) {
cache[a0] = fun(a0, ...rest);
}
return cache[a0];
}
}

/**
* Return a memoized version of the given zero-arg function.
*/
export function cache(fun) {
FilipStamenkovic marked this conversation as resolved.
Show resolved Hide resolved
const f = memoize1((arg) => fun());
return function () {
return f(null);
}
}

/**
* Return an object that is the result of recursively merging properties from the given objects. If multiple
* objects define the same "leaf" properties, the right-most's value is picked.
*/
export function deepMerge(left, right) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just learned that there is already a mergeDeep in here. I'll post a revision early next week to slim this down some.

let result;
if (typeof left === 'object' && typeof right === 'object' && left.constructor === Object && right.constructor === Object) {
result = Object.assign({}, left);
Object.keys(right).forEach((key) => {
if (!left.hasOwnProperty(key)) {
result[key] = right[key];
} else {
result[key] = deepMerge(left[key], right[key]);
}
});
} else {
result = right;
}
return result;
}
Loading