From aa48b672060e0eebba1f88dc613a729dae16e605 Mon Sep 17 00:00:00 2001 From: Demetrio Girardi Date: Wed, 7 Aug 2024 09:56:40 -0700 Subject: [PATCH 1/2] Core: fix bug where custom priceGranularity does not work with setBidderConfig --- src/config.js | 278 ++++++++++++++++++++------------------- test/spec/config_spec.js | 46 +++++-- 2 files changed, 180 insertions(+), 144 deletions(-) diff --git a/src/config.js b/src/config.js index e9c37a8d329..5ba75370db6 100644 --- a/src/config.js +++ b/src/config.js @@ -60,89 +60,161 @@ const GRANULARITY_OPTIONS = { const ALL_TOPICS = '*'; -export function newConfig() { - let listeners = []; - let defaults; - let config; - let bidderConfig; - let currBidder = null; - - function resetConfig() { - defaults = {}; - - function getProp(name) { - return props[name].val; - } +function attachProperties(config, useDefaultValues = true) { + const values = useDefaultValues ? { + priceGranularity: GRANULARITY_OPTIONS.MEDIUM, + customPriceBucket: {}, + mediaTypePriceGranularity: {}, + bidderSequence: DEFAULT_BIDDER_SEQUENCE, + auctionOptions: {} + } : {} + + function getProp(name) { + return values[name]; + } - function setProp(name, val) { - props[name].val = val; + function setProp(name, val) { + if (!values.hasOwnProperty(name)) { + Object.defineProperty(config, name, {enumerable: true, configurable: false}); } + values[name] = val; + } - const props = { - publisherDomain: { - set(val) { - if (val != null) { - logWarn('publisherDomain is deprecated and has no effect since v7 - use pageUrl instead') + const props = { + publisherDomain: { + set(val) { + if (val != null) { + logWarn('publisherDomain is deprecated and has no effect since v7 - use pageUrl instead') + } + setProp('publisherDomain', val); + } + }, + priceGranularity: { + set(val) { + if (validatePriceGranularity(val)) { + if (typeof val === 'string') { + setProp('priceGranularity', (hasGranularity(val)) ? val : GRANULARITY_OPTIONS.MEDIUM); + } else if (isPlainObject(val)) { + setProp('customPriceBucket', val); + setProp('priceGranularity', GRANULARITY_OPTIONS.CUSTOM) + logMessage('Using custom price granularity'); } - setProp('publisherDomain', val); } - }, - priceGranularity: { - val: GRANULARITY_OPTIONS.MEDIUM, - set(val) { - if (validatePriceGranularity(val)) { + } + }, + customPriceBucket: { + set() {} + }, + mediaTypePriceGranularity: { + set(val) { + val != null && setProp('mediaTypePriceGranularity', Object.keys(val).reduce((aggregate, item) => { + if (validatePriceGranularity(val[item])) { if (typeof val === 'string') { - setProp('priceGranularity', (hasGranularity(val)) ? val : GRANULARITY_OPTIONS.MEDIUM); + aggregate[item] = (hasGranularity(val[item])) ? val[item] : getProp('priceGranularity'); } else if (isPlainObject(val)) { - setProp('customPriceBucket', val); - setProp('priceGranularity', GRANULARITY_OPTIONS.CUSTOM) - logMessage('Using custom price granularity'); + aggregate[item] = val[item]; + logMessage(`Using custom price granularity for ${item}`); } + } else { + logWarn(`Invalid price granularity for media type: ${item}`); } + return aggregate; + }, {})); + } + }, + bidderSequence: { + set(val) { + if (VALID_ORDERS[val]) { + setProp('bidderSequence', val); + } else { + logWarn(`Invalid order: ${val}. Bidder Sequence was not set.`); } - }, - customPriceBucket: { - val: {}, - set() {} - }, - mediaTypePriceGranularity: { - val: {}, - set(val) { - val != null && setProp('mediaTypePriceGranularity', Object.keys(val).reduce((aggregate, item) => { - if (validatePriceGranularity(val[item])) { - if (typeof val === 'string') { - aggregate[item] = (hasGranularity(val[item])) ? val[item] : getProp('priceGranularity'); - } else if (isPlainObject(val)) { - aggregate[item] = val[item]; - logMessage(`Using custom price granularity for ${item}`); - } - } else { - logWarn(`Invalid price granularity for media type: ${item}`); - } - return aggregate; - }, {})); + } + }, + auctionOptions: { + set(val) { + if (validateauctionOptions(val)) { + setProp('auctionOptions', val); } - }, - bidderSequence: { - val: DEFAULT_BIDDER_SEQUENCE, - set(val) { - if (VALID_ORDERS[val]) { - setProp('bidderSequence', val); - } else { - logWarn(`Invalid order: ${val}. Bidder Sequence was not set.`); - } + } + } + } + + Object.defineProperties(config, Object.fromEntries( + Object.entries(props) + .map(([k, def]) => [k, Object.assign({ + get: getProp.bind(null, k), + set: setProp.bind(null, k), + enumerable: values.hasOwnProperty(k), + configurable: !values.hasOwnProperty(k) + }, def)]) + )); + + return config; + + function hasGranularity(val) { + return find(Object.keys(GRANULARITY_OPTIONS), option => val === GRANULARITY_OPTIONS[option]); + } + + function validatePriceGranularity(val) { + if (!val) { + logError('Prebid Error: no value passed to `setPriceGranularity()`'); + return false; + } + if (typeof val === 'string') { + if (!hasGranularity(val)) { + logWarn('Prebid Warning: setPriceGranularity was called with invalid setting, using `medium` as default.'); + } + } else if (isPlainObject(val)) { + if (!isValidPriceConfig(val)) { + logError('Invalid custom price value passed to `setPriceGranularity()`'); + return false; + } + } + return true; + } + + function validateauctionOptions(val) { + if (!isPlainObject(val)) { + logWarn('Auction Options must be an object') + return false + } + + for (let k of Object.keys(val)) { + if (k !== 'secondaryBidders' && k !== 'suppressStaleRender') { + logWarn(`Auction Options given an incorrect param: ${k}`) + return false + } + if (k === 'secondaryBidders') { + if (!isArray(val[k])) { + logWarn(`Auction Options ${k} must be of type Array`); + return false + } else if (!val[k].every(isStr)) { + logWarn(`Auction Options ${k} must be only string`); + return false } - }, - auctionOptions: { - val: {}, - set(val) { - if (validateauctionOptions(val)) { - setProp('auctionOptions', val); - } + } else if (k === 'suppressStaleRender') { + if (!isBoolean(val[k])) { + logWarn(`Auction Options ${k} must be of type boolean`); + return false; } } } - let newConfig = { + return true; + } +} + +export function newConfig() { + let listeners = []; + let defaults; + let config; + let bidderConfig; + let currBidder = null; + + function resetConfig() { + defaults = {}; + + let newConfig = attachProperties({ // `debug` is equivalent to legacy `pbjs.logging` property debug: DEFAULT_DEBUG, bidderTimeout: DEFAULT_BIDDER_TIMEOUT, @@ -165,16 +237,7 @@ export function newConfig() { userSync: { topics: DEFAULT_IFRAMES_CONFIG } - }; - - Object.defineProperties(newConfig, - Object.fromEntries(Object.entries(props) - .map(([k, def]) => [k, Object.assign({ - get: getProp.bind(null, k), - set: setProp.bind(null, k), - enumerable: true, - }, def)])) - ); + }); if (config) { callSubscribers( @@ -190,57 +253,6 @@ export function newConfig() { config = newConfig; bidderConfig = {}; - - function hasGranularity(val) { - return find(Object.keys(GRANULARITY_OPTIONS), option => val === GRANULARITY_OPTIONS[option]); - } - - function validatePriceGranularity(val) { - if (!val) { - logError('Prebid Error: no value passed to `setPriceGranularity()`'); - return false; - } - if (typeof val === 'string') { - if (!hasGranularity(val)) { - logWarn('Prebid Warning: setPriceGranularity was called with invalid setting, using `medium` as default.'); - } - } else if (isPlainObject(val)) { - if (!isValidPriceConfig(val)) { - logError('Invalid custom price value passed to `setPriceGranularity()`'); - return false; - } - } - return true; - } - - function validateauctionOptions(val) { - if (!isPlainObject(val)) { - logWarn('Auction Options must be an object') - return false - } - - for (let k of Object.keys(val)) { - if (k !== 'secondaryBidders' && k !== 'suppressStaleRender') { - logWarn(`Auction Options given an incorrect param: ${k}`) - return false - } - if (k === 'secondaryBidders') { - if (!isArray(val[k])) { - logWarn(`Auction Options ${k} must be of type Array`); - return false - } else if (!val[k].every(isStr)) { - logWarn(`Auction Options ${k} must be only string`); - return false - } - } else if (k === 'suppressStaleRender') { - if (!isBoolean(val[k])) { - logWarn(`Auction Options ${k} must be of type boolean`); - return false; - } - } - } - return true; - } } /** @@ -451,14 +463,14 @@ export function newConfig() { check(config); config.bidders.forEach(bidder => { if (!bidderConfig[bidder]) { - bidderConfig[bidder] = {}; + bidderConfig[bidder] = attachProperties({}, false); } Object.keys(config.config).forEach(topic => { let option = config.config[topic]; - - if (isPlainObject(option)) { + const currentConfig = bidderConfig[bidder][topic]; + if (isPlainObject(option) && (currentConfig == null || isPlainObject(currentConfig))) { const func = mergeFlag ? mergeDeep : Object.assign; - bidderConfig[bidder][topic] = func({}, bidderConfig[bidder][topic] || {}, option); + bidderConfig[bidder][topic] = func({}, currentConfig || {}, option); } else { bidderConfig[bidder][topic] = option; } diff --git a/test/spec/config_spec.js b/test/spec/config_spec.js index d7f6b9de6c0..b54f207e6a9 100644 --- a/test/spec/config_spec.js +++ b/test/spec/config_spec.js @@ -252,19 +252,43 @@ describe('config API', function () { expect(configResult.native).to.be.equal('high'); }); - it('sets priceGranularity and customPriceBucket', function () { - const goodConfig = { - 'buckets': [{ - 'max': 3, - 'increment': 0.01, - 'cap': true - }] - }; - setConfig({ priceGranularity: goodConfig }); - expect(getConfig('priceGranularity')).to.be.equal('custom'); - expect(getConfig('customPriceBucket')).to.equal(goodConfig); + Object.entries({ + 'using setConfig': { + setter: () => config.setConfig, + getter: () => config.getConfig + }, + 'using setBidderConfig': { + setter: () => (config) => setBidderConfig({bidders: ['mockBidder'], config}), + getter: () => (option) => config.runWithBidder('mockBidder', () => config.getConfig(option)) + } + }).forEach(([t, {getter, setter}]) => { + describe(t, () => { + let getConfig, setConfig; + beforeEach(() => { + getConfig = getter(); + setConfig = setter(); + }); + it('sets priceGranularity and customPriceBucket', function () { + const goodConfig = { + 'buckets': [{ + 'max': 3, + 'increment': 0.01, + 'cap': true + }] + }; + setConfig({ priceGranularity: goodConfig }); + expect(getConfig('priceGranularity')).to.be.equal('custom'); + expect(getConfig('customPriceBucket')).to.eql(goodConfig); + }); + }); }); + it('does not force defaults for bidder config', () => { + config.setConfig({bidderSequence: 'fixed'}); + config.setBidderConfig({bidders: ['mockBidder'], config: {other: 'config'}}) + expect(config.runWithBidder('mockBidder', () => config.getConfig('bidderSequence'))).to.eql('fixed'); + }) + it('sets deviceAccess', function () { // When the deviceAccess flag config option is not set, cookies may be read and set expect(getConfig('deviceAccess')).to.be.equal(true); From 6749bdd125608c950619ddc17a005c7f8e86d82d Mon Sep 17 00:00:00 2001 From: Demetrio Girardi Date: Thu, 8 Aug 2024 08:12:40 -0700 Subject: [PATCH 2/2] allow setting customPriceBucket directly --- src/config.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/config.js b/src/config.js index 5ba75370db6..53f53f99529 100644 --- a/src/config.js +++ b/src/config.js @@ -75,7 +75,7 @@ function attachProperties(config, useDefaultValues = true) { function setProp(name, val) { if (!values.hasOwnProperty(name)) { - Object.defineProperty(config, name, {enumerable: true, configurable: false}); + Object.defineProperty(config, name, {enumerable: true}); } values[name] = val; } @@ -102,9 +102,7 @@ function attachProperties(config, useDefaultValues = true) { } } }, - customPriceBucket: { - set() {} - }, + customPriceBucket: {}, mediaTypePriceGranularity: { set(val) { val != null && setProp('mediaTypePriceGranularity', Object.keys(val).reduce((aggregate, item) => {