From 5fc475ed7423dcc295dafc6a552097b191f049d5 Mon Sep 17 00:00:00 2001 From: Demetrio Girardi Date: Fri, 15 Sep 2023 06:43:23 -0700 Subject: [PATCH] priceFloors: fix bug where `default` does not work on adUnit-level floors (#10475) * priceFloors: fix bug where `default` does not work on adUnit-level floors * do not require adUnits to repeat the same schema * Improve schema selection and validation for adUnit-level floors --- modules/priceFloors.js | 49 +++++-- test/spec/modules/priceFloors_spec.js | 183 ++++++++++++++++++++++++-- 2 files changed, 211 insertions(+), 21 deletions(-) diff --git a/modules/priceFloors.js b/modules/priceFloors.js index a72a79c2b40..e62e615ea86 100644 --- a/modules/priceFloors.js +++ b/modules/priceFloors.js @@ -13,7 +13,8 @@ import { mergeDeep, parseGPTSingleSizeArray, parseUrl, - pick + pick, + deepEqual } from '../src/utils.js'; import {getGlobal} from '../src/prebidGlobal.js'; import {config} from '../src/config.js'; @@ -40,10 +41,13 @@ const MODULE_NAME = 'Price Floors'; */ const ajax = ajaxBuilder(10000); +// eslint-disable-next-line symbol-description +const SYN_FIELD = Symbol(); + /** * @summary Allowed fields for rules to have */ -export let allowedFields = ['gptSlot', 'adUnitCode', 'size', 'domain', 'mediaType']; +export let allowedFields = [SYN_FIELD, 'gptSlot', 'adUnitCode', 'size', 'domain', 'mediaType']; /** * @summary This is a flag to indicate if a AJAX call is processing for a floors request @@ -104,6 +108,7 @@ function getAdUnitCode(request, response, {index = auctionManager.index} = {}) { * @summary floor field types with their matching functions to resolve the actual matched value */ export let fieldMatchingFunctions = { + [SYN_FIELD]: () => '*', 'size': (bidRequest, bidResponse) => parseGPTSingleSizeArray(bidResponse.size) || '*', 'mediaType': (bidRequest, bidResponse) => bidResponse.mediaType || 'banner', 'gptSlot': (bidRequest, bidResponse) => getGptSlotFromAdUnit((bidRequest || bidResponse).transactionId) || getGptSlotInfoForAdUnitCode(getAdUnitCode(bidRequest, bidResponse)).gptSlot, @@ -117,6 +122,7 @@ export let fieldMatchingFunctions = { * Returns array of Tuple [exact match, catch all] for each field in rules file */ function enumeratePossibleFieldValues(floorFields, bidObject, responseObject) { + if (!floorFields.length) return []; // generate combination of all exact matches and catch all for each field type return floorFields.reduce((accum, field) => { let exactMatch = fieldMatchingFunctions[field](bidObject, responseObject) || '*'; @@ -132,7 +138,9 @@ function enumeratePossibleFieldValues(floorFields, bidObject, responseObject) { */ export function getFirstMatchingFloor(floorData, bidObject, responseObject = {}) { let fieldValues = enumeratePossibleFieldValues(deepAccess(floorData, 'schema.fields') || [], bidObject, responseObject); - if (!fieldValues.length) return { matchingFloor: floorData.default }; + if (!fieldValues.length) { + return {matchingFloor: undefined} + } // look to see if a request for this context was made already let matchingInput = fieldValues.map(field => field[0]).join('-'); @@ -146,9 +154,9 @@ export function getFirstMatchingFloor(floorData, bidObject, responseObject = {}) let matchingData = { floorMin: floorData.floorMin || 0, - floorRuleValue: isNaN(floorData.values[matchingRule]) ? floorData.default : floorData.values[matchingRule], + floorRuleValue: floorData.values[matchingRule], matchingData: allPossibleMatches[0], // the first possible match is an "exact" so contains all data relevant for anlaytics adapters - matchingRule + matchingRule: matchingRule === floorData.meta?.defaultRule ? undefined : matchingRule }; // use adUnit floorMin as priority! const floorMin = deepAccess(bidObject, 'ortb2Imp.ext.prebid.floors.floorMin'); @@ -300,14 +308,20 @@ function normalizeRulesForAuction(floorData, adUnitCode) { * Only called if no set config or fetch level data has returned */ export function getFloorDataFromAdUnits(adUnits) { + const schemaAu = adUnits.find(au => au.floors?.schema != null); return adUnits.reduce((accum, adUnit) => { - if (isFloorsDataValid(adUnit.floors)) { + if (adUnit.floors?.schema != null && !deepEqual(adUnit.floors.schema, schemaAu?.floors?.schema)) { + logError(`${MODULE_NAME}: adUnit '${adUnit.code}' declares a different schema from one previously declared by adUnit '${schemaAu.code}'. Floor config for '${adUnit.code}' will be ignored.`) + return accum; + } + const floors = Object.assign({}, schemaAu?.floors, {values: undefined}, adUnit.floors) + if (isFloorsDataValid(floors)) { // if values already exist we want to not overwrite them if (!accum.values) { - accum = getFloorsDataForAuction(adUnit.floors, adUnit.code); + accum = getFloorsDataForAuction(floors, adUnit.code); accum.location = 'adUnit'; } else { - let newRules = getFloorsDataForAuction(adUnit.floors, adUnit.code).values; + let newRules = getFloorsDataForAuction(floors, adUnit.code).values; // copy over the new rules into our values object Object.assign(accum.values, newRules); } @@ -443,7 +457,26 @@ function validateRules(floorsData, numFields, delimiter) { return Object.keys(floorsData.values).length > 0; } +export function normalizeDefault(model) { + if (isNumber(model.default)) { + let defaultRule = '*'; + const numFields = (model.schema?.fields || []).length; + if (!numFields) { + deepSetValue(model, 'schema.fields', [SYN_FIELD]); + } else { + defaultRule = Array(numFields).fill('*').join(model.schema?.delimiter || '|'); + } + model.values = model.values || {}; + if (model.values[defaultRule] == null) { + model.values[defaultRule] = model.default; + model.meta = {defaultRule}; + } + } + return model; +} + function modelIsValid(model) { + model = normalizeDefault(model); // schema.fields has only allowed attributes if (!validateSchemaFields(deepAccess(model, 'schema.fields'))) { return false; diff --git a/test/spec/modules/priceFloors_spec.js b/test/spec/modules/priceFloors_spec.js index e2b8ca38792..950e039491d 100644 --- a/test/spec/modules/priceFloors_spec.js +++ b/test/spec/modules/priceFloors_spec.js @@ -12,7 +12,7 @@ import { isFloorsDataValid, addBidResponseHook, fieldMatchingFunctions, - allowedFields + allowedFields, parseFloorData, normalizeDefault, getFloorDataFromAdUnits } from 'modules/priceFloors.js'; import * as events from 'src/events.js'; import * as mockGpt from '../integration/faker/googletag.js'; @@ -123,7 +123,7 @@ describe('the price floors module', function () { return { code, mediaTypes: {banner: { sizes: [[300, 200], [300, 600]] }, native: {}}, - bids: [{bidder: 'someBidder'}, {bidder: 'someOtherBidder'}] + bids: [{bidder: 'someBidder', adUnitCode: code}, {bidder: 'someOtherBidder', adUnitCode: code}] }; } beforeEach(function() { @@ -143,6 +143,76 @@ describe('the price floors module', function () { getGlobal().bidderSettings = {}; }); + describe('parseFloorData', () => { + it('should accept just a default floor', () => { + const fd = parseFloorData({ + default: 1.23 + }); + expect(getFirstMatchingFloor(fd, {}, {}).matchingFloor).to.eql(1.23); + }); + }); + + describe('getFloorDataFromAdUnits', () => { + let adUnits; + + function setFloorValues(rule) { + adUnits.forEach((au, i) => { + au.floors = { + values: { + [rule]: i + 1 + } + } + }) + } + + beforeEach(() => { + adUnits = ['au1', 'au2', 'au3'].map(getAdUnitMock); + }) + + it('should use one schema for all adUnits', () => { + setFloorValues('*;*') + adUnits[1].floors.schema = { + fields: ['mediaType', 'gptSlot'], + delimiter: ';' + } + sinon.assert.match(getFloorDataFromAdUnits(adUnits), { + schema: { + fields: ['adUnitCode', 'mediaType', 'gptSlot'], + delimiter: ';' + }, + values: { + 'au1;*;*': 1, + 'au2;*;*': 2, + 'au3;*;*': 3 + } + }) + }); + it('should ignore adUnits that declare different schema', () => { + setFloorValues('*|*'); + adUnits[0].floors.schema = { + fields: ['mediaType', 'gptSlot'] + }; + adUnits[2].floors.schema = { + fields: ['gptSlot', 'mediaType'] + }; + expect(getFloorDataFromAdUnits(adUnits).values).to.eql({ + 'au1|*|*': 1, + 'au2|*|*': 2 + }) + }); + it('should ignore adUnits that declare no values', () => { + setFloorValues('*'); + adUnits[0].floors.schema = { + fields: ['mediaType'] + }; + delete adUnits[2].floors.values; + expect(getFloorDataFromAdUnits(adUnits).values).to.eql({ + 'au1|*': 1, + 'au2|*': 2, + }) + }) + }) + describe('getFloorsDataForAuction', function () { it('converts basic input floor data into a floorData map for the auction correctly', function () { // basic input where nothing needs to be updated @@ -233,8 +303,8 @@ describe('the price floors module', function () { }); describe('getFirstMatchingFloor', function () { - it('uses a 0 floor as overrite', function () { - let inputFloorData = { + it('uses a 0 floor as override', function () { + let inputFloorData = normalizeDefault({ currency: 'USD', schema: { delimiter: '|', @@ -245,7 +315,7 @@ describe('the price floors module', function () { 'test_div_2': 2 }, default: 0.5 - }; + }); expect(getFirstMatchingFloor(inputFloorData, basicBidRequest, {mediaType: 'banner', size: '*'})).to.deep.equal({ floorMin: 0, @@ -434,7 +504,7 @@ describe('the price floors module', function () { }); }); it('selects the right floor for more complex rules', function () { - let inputFloorData = { + let inputFloorData = normalizeDefault({ currency: 'USD', schema: { delimiter: '^', @@ -448,7 +518,7 @@ describe('the price floors module', function () { 'weird_div^*^300x250': 5.5 }, default: 0.5 - }; + }); // banner with 300x250 size expect(getFirstMatchingFloor(inputFloorData, basicBidRequest, {mediaType: 'banner', size: [300, 250]})).to.deep.equal({ floorMin: 0, @@ -490,10 +560,8 @@ describe('the price floors module', function () { matchingFloor: undefined }); // if default is there use it - inputFloorData = { default: 5.0 }; - expect(getFirstMatchingFloor(inputFloorData, basicBidRequest, {mediaType: 'banner', size: '*'})).to.deep.equal({ - matchingFloor: 5.0 - }); + inputFloorData = normalizeDefault({ default: 5.0 }); + expect(getFirstMatchingFloor(inputFloorData, basicBidRequest, {mediaType: 'banner', size: '*'}).matchingFloor).to.equal(5.0); }); describe('with gpt enabled', function () { let gptFloorData; @@ -693,6 +761,95 @@ describe('the price floors module', function () { floorProvider: undefined }); }); + describe('default floor', () => { + let adUnits; + beforeEach(() => { + adUnits = ['au1', 'au2'].map(getAdUnitMock); + }) + function expectFloors(floors) { + runStandardAuction(adUnits); + adUnits.forEach((au, i) => { + au.bids.forEach(bid => { + expect(bid.getFloor().floor).to.eql(floors[i]); + }) + }) + } + describe('should be sufficient by itself', () => { + it('globally', () => { + handleSetFloorsConfig({ + ...basicFloorConfig, + data: { + default: 1.23 + } + }); + expectFloors([1.23, 1.23]) + }); + it('on adUnits', () => { + handleSetFloorsConfig({ + ...basicFloorConfig, + data: undefined + }); + adUnits[0].floors = {default: 1}; + adUnits[1].floors = {default: 2}; + expectFloors([1, 2]) + }); + it('on an adUnit with hidden schema', () => { + handleSetFloorsConfig({ + ...basicFloorConfig, + data: undefined + }); + adUnits[0].floors = { + schema: { + fields: ['mediaType', 'gptSlot'], + }, + default: 1 + } + adUnits[1].floors = { + default: 2 + } + expectFloors([1, 2]); + }) + }); + describe('should NOT be used when a star rule exists', () => { + it('globally', () => { + handleSetFloorsConfig({ + ...basicFloorConfig, + data: { + schema: { + fields: ['mediaType', 'gptSlot'], + }, + values: { + '*|*': 2 + }, + default: 3, + } + }); + expectFloors([2, 2]); + }); + it('on adUnits', () => { + handleSetFloorsConfig({ + ...basicFloorConfig, + data: undefined + }); + adUnits[0].floors = { + schema: { + fields: ['mediaType', 'gptSlot'], + }, + values: { + '*|*': 1 + }, + default: 3 + }; + adUnits[1].floors = { + values: { + '*|*': 2 + }, + default: 4 + } + expectFloors([1, 2]); + }) + }); + }) it('bidRequests should have getFloor function and flooring meta data when setConfig occurs', function () { handleSetFloorsConfig({...basicFloorConfig, floorProvider: 'floorprovider'}); runStandardAuction(); @@ -1382,7 +1539,7 @@ describe('the price floors module', function () { it('picks the right rule with more complex rules', function () { _floorDataForAuction[bidRequest.auctionId] = { ...basicFloorConfig, - data: { + data: normalizeDefault({ currency: 'USD', schema: { fields: ['mediaType', 'size'], delimiter: '|' }, values: { @@ -1394,7 +1551,7 @@ describe('the price floors module', function () { 'video|*': 5.5 }, default: 10.0 - } + }) }; // assumes banner *