From 3b230a02531e2439cb231d4390a342f1d53af538 Mon Sep 17 00:00:00 2001 From: markiian Date: Fri, 20 Dec 2024 14:30:28 +0200 Subject: [PATCH 01/10] Add functional tests for price floors max rules dimensions --- .../config/AccountPriceFloorsConfig.groovy | 6 + .../model/config/PriceFloorsFetch.groovy | 3 + .../pricefloors/PriceFloorsBaseSpec.groovy | 6 +- .../PriceFloorsFetchingSpec.groovy | 121 ++++++++++++--- .../pricefloors/PriceFloorsRulesSpec.groovy | 6 +- .../PriceFloorsSignalingSpec.groovy | 138 ++++++++++++++++++ 6 files changed, 256 insertions(+), 24 deletions(-) diff --git a/src/test/groovy/org/prebid/server/functional/model/config/AccountPriceFloorsConfig.groovy b/src/test/groovy/org/prebid/server/functional/model/config/AccountPriceFloorsConfig.groovy index 28f908aba44..c83c69280fa 100644 --- a/src/test/groovy/org/prebid/server/functional/model/config/AccountPriceFloorsConfig.groovy +++ b/src/test/groovy/org/prebid/server/functional/model/config/AccountPriceFloorsConfig.groovy @@ -15,6 +15,8 @@ class AccountPriceFloorsConfig { Boolean adjustForBidAdjustment Boolean enforceDealFloors Boolean useDynamicData + Long maxRules + Long maxSchemaDims @JsonProperty("enforce_floors_rate") Integer enforceFloorsRateSnakeCase @@ -24,4 +26,8 @@ class AccountPriceFloorsConfig { Boolean enforceDealFloorsSnakeCase @JsonProperty("use_dynamic_data") Boolean useDynamicDataSnakeCase + @JsonProperty("max_rules") + Long maxRulesSnakeCase + @JsonProperty("max_schema_dims") + Long maxSchemaDimsSnakeCase } diff --git a/src/test/groovy/org/prebid/server/functional/model/config/PriceFloorsFetch.groovy b/src/test/groovy/org/prebid/server/functional/model/config/PriceFloorsFetch.groovy index 1501f2e1366..89f32a951a4 100644 --- a/src/test/groovy/org/prebid/server/functional/model/config/PriceFloorsFetch.groovy +++ b/src/test/groovy/org/prebid/server/functional/model/config/PriceFloorsFetch.groovy @@ -26,4 +26,7 @@ class PriceFloorsFetch { Integer periodSec @JsonProperty("period_sec") Integer periodSecSnakeCase + Integer maxSchemaDims + @JsonProperty("max_schema_dims") + Integer maxSchemaDimsSnakeCase } diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsBaseSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsBaseSpec.groovy index fba9a5b44d5..e96eea9995a 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsBaseSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsBaseSpec.groovy @@ -57,13 +57,15 @@ abstract class PriceFloorsBaseSpec extends BaseSpec { maxRules: 0, maxFileSizeKb: 200, maxAgeSec: 86400, - periodSec: 3600) + periodSec: 3600, + maxSchemaDims: 0) def floors = new AccountPriceFloorsConfig(enabled: true, fetch: fetch, enforceFloorsRate: 100, enforceDealFloors: true, adjustForBidAdjustment: true, - useDynamicData: true) + useDynamicData: true, + maxRules: 0) new AccountConfig(auction: new AccountAuctionConfig(priceFloors: floors)) } diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy index f282d69f600..12571025c5c 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy @@ -1299,8 +1299,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "Response should contain error" assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason : Price floor floorMin " + - "must be positive float, but was $invalidFloorMin "] + ["Failed to parse price floors from request, with a reason: Price floor floorMin " + + "must be positive float, but was $invalidFloorMin"] } def "PBS should validate rules from request when request doesn't contain modelGroups"() { @@ -1327,8 +1327,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "Response should contain error" assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason : Price floor rules " + - "should contain at least one model group "] + ["Failed to parse price floors from request, with a reason: Price floor rules " + + "should contain at least one model group"] } def "PBS should validate rules from request when request doesn't contain values"() { @@ -1355,8 +1355,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "Response should contain error" assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason : Price floor rules values " + - "can't be null or empty, but were null "] + ["Failed to parse price floors from request, with a reason: Price floor rules values " + + "can't be null or empty, but were null"] } def "PBS should validate rules from request when modelWeight from request is invalid"() { @@ -1387,8 +1387,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "Response should contain error" assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason : Price floor modelGroup modelWeight " + - "must be in range(1-100), but was $invalidModelWeight "] + ["Failed to parse price floors from request, with a reason: Price floor modelGroup modelWeight " + + "must be in range(1-100), but was $invalidModelWeight"] where: invalidModelWeight << [0, MAX_MODEL_WEIGHT + 1] } @@ -1426,8 +1426,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "Response should contain error" assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason : Price floor modelGroup modelWeight " + - "must be in range(1-100), but was $invalidModelWeight "] + ["Failed to parse price floors from request, with a reason: Price floor modelGroup modelWeight " + + "must be in range(1-100), but was $invalidModelWeight"] where: invalidModelWeight << [0, MAX_MODEL_WEIGHT + 1] @@ -1466,8 +1466,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "Response should contain error" assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason : Price floor root skipRate " + - "must be in range(0-100), but was $invalidSkipRate "] + ["Failed to parse price floors from request, with a reason: Price floor root skipRate " + + "must be in range(0-100), but was $invalidSkipRate"] where: invalidSkipRate << [SKIP_RATE_MIN - 1, SKIP_RATE_MAX + 1] @@ -1506,8 +1506,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "Response should contain error" assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason : Price floor data skipRate " + - "must be in range(0-100), but was $invalidSkipRate "] + ["Failed to parse price floors from request, with a reason: Price floor data skipRate " + + "must be in range(0-100), but was $invalidSkipRate"] where: invalidSkipRate << [SKIP_RATE_MIN - 1, SKIP_RATE_MAX + 1] @@ -1546,8 +1546,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "Response should contain error" assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason : Price floor modelGroup skipRate " + - "must be in range(0-100), but was $invalidSkipRate "] + ["Failed to parse price floors from request, with a reason: Price floor modelGroup skipRate " + + "must be in range(0-100), but was $invalidSkipRate"] where: invalidSkipRate << [SKIP_RATE_MIN - 1, SKIP_RATE_MAX + 1] @@ -1582,8 +1582,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "Response should contain error" assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason : Price floor modelGroup default " + - "must be positive float, but was $invalidDefaultFloorValue "] + ["Failed to parse price floors from request, with a reason: Price floor modelGroup default " + + "must be positive float, but was $invalidDefaultFloorValue"] } def "PBS should not invalidate previously good fetched data when floors provider return invalid data"() { @@ -2046,6 +2046,91 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { } } + def "PBS should validate fetch.max-schema-dims from account config"() { + given: "Default BidRequest" + def bidRequest = BidRequest.getDefaultBidRequest(APP) + + and: "Account with enabled fetch, fetch.url, maxSchemaDims in the DB" + def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { + config.auction.priceFloors.fetch.maxSchemaDims = maxSchemaDims + config.auction.priceFloors.fetch.maxSchemaDimsSnakeCase = maxSchemaDimsSnakeCase + } + accountDao.save(account) + + when: "PBS processes auction request" + def response = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "Metric alerts.account_config.ACCOUNT.price-floors should be update" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert metrics[INVALID_CONFIG_METRIC(bidRequest.accountId) as String] == 1 + + and: "PBS floors validation failure should not reject the entire auction" + assert !response.seatbid?.isEmpty() + + where: + maxSchemaDims | maxSchemaDimsSnakeCase + null | PBSUtils.randomNegativeNumber + null | PBSUtils.getRandomNumber(20) + PBSUtils.randomNegativeNumber | null + PBSUtils.getRandomNumber(20) | null + } + + def "PBS should validate price-floor.max-rules from account config"() { + given: "Default BidRequest" + def bidRequest = BidRequest.getDefaultBidRequest(APP) + + and: "Account with enabled fetch, fetch.url, maxSchemaDims in the DB" + def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { + config.auction.priceFloors.maxRules = maxRules + config.auction.priceFloors.maxRulesSnakeCase = maxRulesSnakeCase + } + accountDao.save(account) + + when: "PBS processes auction request" + def response = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "Metric alerts.account_config.ACCOUNT.price-floors should be update" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert metrics[INVALID_CONFIG_METRIC(bidRequest.accountId) as String] == 1 + + and: "PBS floors validation failure should not reject the entire auction" + assert !response.seatbid?.isEmpty() + + where: + maxRules | maxRulesSnakeCase + null | PBSUtils.randomNegativeNumber + PBSUtils.randomNegativeNumber | null + } + + def "PBS should validate price-floor.max-schema-dims from account config"() { + given: "Default BidRequest" + def bidRequest = BidRequest.getDefaultBidRequest(APP) + + and: "Account with enabled fetch, fetch.url, maxSchemaDims in the DB" + def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { + config.auction.priceFloors.maxSchemaDims = maxSchemaDims + config.auction.priceFloors.maxSchemaDimsSnakeCase = maxSchemaDimsSnakeCase + } + accountDao.save(account) + + when: "PBS processes auction request" + def response = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "Metric alerts.account_config.ACCOUNT.price-floors should be update" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert metrics[INVALID_CONFIG_METRIC(bidRequest.accountId) as String] == 1 + + and: "PBS floors validation failure should not reject the entire auction" + assert !response.seatbid?.isEmpty() + + where: + maxSchemaDims | maxSchemaDimsSnakeCase + null | PBSUtils.randomNegativeNumber + null | PBSUtils.getRandomNumber(20) + PBSUtils.randomNegativeNumber | null + PBSUtils.getRandomNumber(20) | null + } + static int convertKilobyteSizeToByte(int kilobyteSize) { kilobyteSize * 1024 } diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsRulesSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsRulesSpec.groovy index 478248d3f46..d10f6b07b1b 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsRulesSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsRulesSpec.groovy @@ -277,10 +277,8 @@ class PriceFloorsRulesSpec extends PriceFloorsBaseSpec { where: bidRequest | bothFloorValue | bannerFloorValue | videoFloorValue - bidRequestWithMultipleMediaTypes | 0.6 | PBSUtils.randomFloorValue | - PBSUtils.randomFloorValue - BidRequest.defaultBidRequest | PBSUtils.randomFloorValue | 0.6 | - PBSUtils.randomFloorValue + bidRequestWithMultipleMediaTypes | 0.6 | PBSUtils.randomFloorValue | PBSUtils.randomFloorValue + BidRequest.defaultBidRequest | PBSUtils.randomFloorValue | 0.6 | PBSUtils.randomFloorValue BidRequest.defaultVideoRequest | PBSUtils.randomFloorValue | PBSUtils.randomFloorValue | 0.6 } diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy index 737aed289e4..5facc6e2ae2 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy @@ -27,6 +27,7 @@ import static org.prebid.server.functional.model.pricefloors.MediaType.VIDEO import static org.prebid.server.functional.model.pricefloors.PriceFloorField.MEDIA_TYPE import static org.prebid.server.functional.model.pricefloors.PriceFloorField.SITE_DOMAIN import static org.prebid.server.functional.model.request.auction.DistributionChannel.APP +import static org.prebid.server.functional.model.response.auction.ErrorType.PREBID class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { @@ -511,4 +512,141 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp.first().bidFloor == bannerFloorValue assert bidderRequest.imp.last().bidFloor == videoFloorValue } + + def "PBS should emit warning when request has more rules than price-floor.max-rules"() { + given: "BidRequest with 2 rules" + def requestFloorValue = PBSUtils.randomFloorValue + def bidRequest = bidRequestWithFloors.tap { + ext.prebid.floors.data.modelGroups[0].values = + [(rule) : requestFloorValue + 0.1, + (new Rule(mediaType: BANNER, country: Country.MULTIPLE).rule): requestFloorValue] + } + + and: "Account with maxRules in the DB" + def accountId = bidRequest.site.publisher.id + def account = getAccountWithEnabledFetch(accountId).tap { + config.auction.priceFloors.maxRules = maxRules + config.auction.priceFloors.maxRulesSnakeCase = maxRulesSnakeCase + } + accountDao.save(account) + + and: "Set bidder response" + def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) + bidResponse.seatbid.first().bid.first().price = requestFloorValue + bidder.setResponse(bidRequest.id, bidResponse) + + when: "PBS processes auction request" + def response = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "PBS should log a warning" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: " + + "Price floor rules number 2 exceeded its maximum number 1"] + + where: + maxRules | maxRulesSnakeCase + 1 | null + null | 1 + } + + def "PBS should emit warning when request has more schema.fields than floor-config.max-schema-dims"() { + given: "BidRequest with schema 2 fields" + def bidRequest = bidRequestWithFloors + + and: "Account with maxSchemaDims in the DB" + def accountId = bidRequest.site.publisher.id + def account = getAccountWithEnabledFetch(accountId).tap { + config.auction.priceFloors.maxSchemaDims = maxSchemaDims + config.auction.priceFloors.maxSchemaDimsSnakeCase = maxSchemaDimsSnakeCase + } + accountDao.save(account) + + and: "Set bidder response" + def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) + bidder.setResponse(bidRequest.id, bidResponse) + + when: "PBS processes auction request" + def response = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "PBS should log a warning" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: " + + "Price floor schema dimensions 2 exceeded its maximum number 1"] + + where: + maxSchemaDims | maxSchemaDimsSnakeCase + 1 | null + null | 1 + } + + def "PBS should emit warning when request has more schema.fields than fetch.max-schema-dims"() { + given: "Default BidRequest with floorMin" + def bidRequest = bidRequestWithFloors + + and: "Account with disabled fetch in the DB" + def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { + config.auction.priceFloors.fetch.enabled = false + config.auction.priceFloors.maxSchemaDims = maxSchemaDims + config.auction.priceFloors.maxSchemaDimsSnakeCase = maxSchemaDimsSnakeCase + } + accountDao.save(account) + + when: "PBS processes auction request" + def response = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "PBS should log a warning" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: " + + "Price floor schema dimensions 2 exceeded its maximum number 1"] + + where: + maxSchemaDims | maxSchemaDimsSnakeCase + 1 | null + null | 1 + } + + def "PBS should emit warning when stored request has more rules than price-floor.max-rules for amp request"() { + given: "Default AmpRequest" + def ampRequest = AmpRequest.defaultAmpRequest + + and: "Default stored request with 2 rules " + def requestFloorValue = PBSUtils.randomFloorValue + def ampStoredRequest = BidRequest.defaultStoredRequest.tap { + ext.prebid.floors = ExtPrebidFloors.extPrebidFloors + ext.prebid.floors.data.modelGroups[0].values = + [(rule) : requestFloorValue + 0.1, + (new Rule(mediaType: BANNER, country: Country.MULTIPLE).rule): requestFloorValue] + } + def storedRequest = StoredRequest.getStoredRequest(ampRequest, ampStoredRequest) + storedRequestDao.save(storedRequest) + + and: "Account with maxRules in the DB" + def account = getAccountWithEnabledFetch(ampRequest.account as String).tap { + config.auction.priceFloors.maxRules = maxRules + config.auction.priceFloors.maxRulesSnakeCase = maxRulesSnakeCase + } + accountDao.save(account) + + and: "Set bidder response" + def bidResponse = BidResponse.getDefaultBidResponse(ampStoredRequest) + bidResponse.seatbid.first().bid.first().price = requestFloorValue + bidder.setResponse(ampStoredRequest.id, bidResponse) + + when: "PBS processes amp request" + def response = floorsPbsService.sendAmpRequest(ampRequest) + + then: "PBS should log a warning" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: " + + "Price floor schema dimensions 2 exceeded its maximum number 1"] + + where: + maxRules | maxRulesSnakeCase + 1 | null + null | 1 + } } From 40ee5a82523019ec5d28aa02feefc23e56c898cc Mon Sep 17 00:00:00 2001 From: markiian Date: Fri, 20 Dec 2024 14:33:21 +0200 Subject: [PATCH 02/10] Update naming --- .../tests/pricefloors/PriceFloorsFetchingSpec.groovy | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy index 12571025c5c..743e53aee4d 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy @@ -2050,7 +2050,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { given: "Default BidRequest" def bidRequest = BidRequest.getDefaultBidRequest(APP) - and: "Account with enabled fetch, fetch.url, maxSchemaDims in the DB" + and: "Account with enabled fetch, maxSchemaDims in the DB" def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { config.auction.priceFloors.fetch.maxSchemaDims = maxSchemaDims config.auction.priceFloors.fetch.maxSchemaDimsSnakeCase = maxSchemaDimsSnakeCase @@ -2079,7 +2079,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { given: "Default BidRequest" def bidRequest = BidRequest.getDefaultBidRequest(APP) - and: "Account with enabled fetch, fetch.url, maxSchemaDims in the DB" + and: "Account with enabled fetch, maxRules in the DB" def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { config.auction.priceFloors.maxRules = maxRules config.auction.priceFloors.maxRulesSnakeCase = maxRulesSnakeCase @@ -2106,7 +2106,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { given: "Default BidRequest" def bidRequest = BidRequest.getDefaultBidRequest(APP) - and: "Account with enabled fetch, fetch.url, maxSchemaDims in the DB" + and: "Account with enabled fetch, maxSchemaDims in the DB" def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { config.auction.priceFloors.maxSchemaDims = maxSchemaDims config.auction.priceFloors.maxSchemaDimsSnakeCase = maxSchemaDimsSnakeCase From f72cd2524c0bdd46d2f34e8c205a6860999c228b Mon Sep 17 00:00:00 2001 From: markiian Date: Mon, 23 Dec 2024 13:51:28 +0200 Subject: [PATCH 03/10] Update tests --- .../PriceFloorsSignalingSpec.groovy | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy index 5facc6e2ae2..1d113df5e44 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy @@ -513,7 +513,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp.last().bidFloor == videoFloorValue } - def "PBS should emit warning when request has more rules than price-floor.max-rules"() { + def "PBS should emit errors when request has more rules than price-floor.max-rules"() { given: "BidRequest with 2 rules" def requestFloorValue = PBSUtils.randomFloorValue def bidRequest = bidRequestWithFloors.tap { @@ -538,9 +538,9 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) - then: "PBS should log a warning" - assert response.ext?.warnings[PREBID]*.code == [999] - assert response.ext?.warnings[PREBID]*.message == + then: "PBS should log a errors" + assert response.ext?.errors[PREBID]*.code == [999] + assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + "Price floor rules number 2 exceeded its maximum number 1"] @@ -550,7 +550,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { null | 1 } - def "PBS should emit warning when request has more schema.fields than floor-config.max-schema-dims"() { + def "PBS should emit errors when request has more schema.fields than floor-config.max-schema-dims"() { given: "BidRequest with schema 2 fields" def bidRequest = bidRequestWithFloors @@ -569,9 +569,9 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) - then: "PBS should log a warning" - assert response.ext?.warnings[PREBID]*.code == [999] - assert response.ext?.warnings[PREBID]*.message == + then: "PBS should log a errors" + assert response.ext?.errors[PREBID]*.code == [999] + assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + "Price floor schema dimensions 2 exceeded its maximum number 1"] @@ -581,7 +581,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { null | 1 } - def "PBS should emit warning when request has more schema.fields than fetch.max-schema-dims"() { + def "PBS should emit errors when request has more schema.fields than fetch.max-schema-dims"() { given: "Default BidRequest with floorMin" def bidRequest = bidRequestWithFloors @@ -596,9 +596,9 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) - then: "PBS should log a warning" - assert response.ext?.warnings[PREBID]*.code == [999] - assert response.ext?.warnings[PREBID]*.message == + then: "PBS should log a errors" + assert response.ext?.errors[PREBID]*.code == [999] + assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + "Price floor schema dimensions 2 exceeded its maximum number 1"] @@ -608,7 +608,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { null | 1 } - def "PBS should emit warning when stored request has more rules than price-floor.max-rules for amp request"() { + def "PBS should emit errors when stored request has more rules than price-floor.max-rules for amp request"() { given: "Default AmpRequest" def ampRequest = AmpRequest.defaultAmpRequest @@ -638,11 +638,11 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { when: "PBS processes amp request" def response = floorsPbsService.sendAmpRequest(ampRequest) - then: "PBS should log a warning" - assert response.ext?.warnings[PREBID]*.code == [999] - assert response.ext?.warnings[PREBID]*.message == + then: "PBS should log a errors" + assert response.ext?.errors[PREBID]*.code == [999] + assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + - "Price floor schema dimensions 2 exceeded its maximum number 1"] + "Price floor rules number 2 exceeded its maximum number 1"] where: maxRules | maxRulesSnakeCase From 79bfd0d421010a9082125ff937b8c8dac0f00409 Mon Sep 17 00:00:00 2001 From: markiian Date: Mon, 6 Jan 2025 23:59:01 +0200 Subject: [PATCH 04/10] Update after review --- .../request/auction/ExtPrebidFloors.groovy | 1 + .../PriceFloorsFetchingSpec.groovy | 6 +- .../PriceFloorsSignalingSpec.groovy | 102 +++++++++++++++++- 3 files changed, 105 insertions(+), 4 deletions(-) diff --git a/src/test/groovy/org/prebid/server/functional/model/request/auction/ExtPrebidFloors.groovy b/src/test/groovy/org/prebid/server/functional/model/request/auction/ExtPrebidFloors.groovy index cb5abf3ff87..c0c80038f5c 100644 --- a/src/test/groovy/org/prebid/server/functional/model/request/auction/ExtPrebidFloors.groovy +++ b/src/test/groovy/org/prebid/server/functional/model/request/auction/ExtPrebidFloors.groovy @@ -20,6 +20,7 @@ class ExtPrebidFloors { ExtPrebidPriceFloorEnforcement enforcement Integer skipRate PriceFloorData data + Long maxSchemaDims static ExtPrebidFloors getExtPrebidFloors() { new ExtPrebidFloors(floorMin: FLOOR_MIN, diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy index 743e53aee4d..508eb3a619b 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy @@ -2048,7 +2048,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def "PBS should validate fetch.max-schema-dims from account config"() { given: "Default BidRequest" - def bidRequest = BidRequest.getDefaultBidRequest(APP) + def bidRequest = BidRequest.defaultBidRequest and: "Account with enabled fetch, maxSchemaDims in the DB" def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { @@ -2077,7 +2077,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def "PBS should validate price-floor.max-rules from account config"() { given: "Default BidRequest" - def bidRequest = BidRequest.getDefaultBidRequest(APP) + def bidRequest = BidRequest.defaultBidRequest and: "Account with enabled fetch, maxRules in the DB" def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { @@ -2104,7 +2104,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def "PBS should validate price-floor.max-schema-dims from account config"() { given: "Default BidRequest" - def bidRequest = BidRequest.getDefaultBidRequest(APP) + def bidRequest = BidRequest.defaultBidRequest and: "Account with enabled fetch, maxSchemaDims in the DB" def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy index 1d113df5e44..eba2e65f06a 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy @@ -21,6 +21,7 @@ import org.prebid.server.functional.util.PBSUtils import static org.mockserver.model.HttpStatusCode.BAD_REQUEST_400 import static org.prebid.server.functional.model.Currency.USD import static org.prebid.server.functional.model.bidder.BidderName.GENERIC +import static org.prebid.server.functional.model.pricefloors.DeviceType.PHONE import static org.prebid.server.functional.model.pricefloors.MediaType.BANNER import static org.prebid.server.functional.model.pricefloors.MediaType.MULTIPLE import static org.prebid.server.functional.model.pricefloors.MediaType.VIDEO @@ -513,6 +514,28 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp.last().bidFloor == videoFloorValue } + def "PBS shouldn't emit errors when request schema.fields than floor-config.max-schema-dims"() { + given: "Bid request with schema 2 fields" + def bidRequest = bidRequestWithFloors.tap { + ext.prebid.floors.maxSchemaDims = PBSUtils.randomNumber + } + + and: "Account with maxSchemaDims in the DB" + def accountId = bidRequest.site.publisher.id + def account = getAccountWithEnabledFetch(accountId) + accountDao.save(account) + + and: "Set bidder response" + def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) + bidder.setResponse(bidRequest.id, bidResponse) + + when: "PBS processes auction request" + def response = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "PBS shouldn't log a errors" + assert !response.ext?.errors + } + def "PBS should emit errors when request has more rules than price-floor.max-rules"() { given: "BidRequest with 2 rules" def requestFloorValue = PBSUtils.randomFloorValue @@ -587,7 +610,6 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { and: "Account with disabled fetch in the DB" def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { - config.auction.priceFloors.fetch.enabled = false config.auction.priceFloors.maxSchemaDims = maxSchemaDims config.auction.priceFloors.maxSchemaDimsSnakeCase = maxSchemaDimsSnakeCase } @@ -608,6 +630,84 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { null | 1 } + def "PBS should fail with error and maxSchemaDims take precede over fetch.maxSchemaDims when requested both"() { + given: "BidRequest with schema 2 fields" + def bidRequest = bidRequestWithFloors + + and: "Account with maxSchemaDims in the DB" + def accountId = bidRequest.site.publisher.id + def account = getAccountWithEnabledFetch(accountId).tap { + config.auction.priceFloors.maxSchemaDims = 1 + config.auction.priceFloors.fetch.maxSchemaDims = 2 + } + accountDao.save(account) + + and: "Set bidder response" + def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) + bidder.setResponse(bidRequest.id, bidResponse) + + when: "PBS processes auction request" + def response = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "PBS should log a errors" + assert response.ext?.errors[PREBID]*.code == [999] + assert response.ext?.errors[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: " + + "Price floor schema dimensions 2 exceeded its maximum number 1"] + } + + def "PBS shouldn't fail with error and maxSchemaDims take precede over fetch.maxSchemaDims when requested both"() { + given: "BidRequest with schema 2 fields" + def bidRequest = bidRequestWithFloors + + and: "Account with maxSchemaDims in the DB" + def accountId = bidRequest.site.publisher.id + def account = getAccountWithEnabledFetch(accountId).tap { + config.auction.priceFloors.maxSchemaDims = 2 + config.auction.priceFloors.fetch.maxSchemaDims = 1 + } + accountDao.save(account) + + and: "Set bidder response" + def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) + bidder.setResponse(bidRequest.id, bidResponse) + + when: "PBS processes auction request" + def response = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "PBS shouldn't log a errors" + assert !response.ext?.errors + } + + def "PBS should fail by default with error when schema dimensions more than 3"() { + given: "BidRequest with schema 4 fields" + def bidRequest = bidRequestWithFloors.tap { + ext.prebid.floors.data.modelGroups[0].values = [(new Rule( + mediaType: MULTIPLE, + country: Country.MULTIPLE, + deviceType: PHONE, + pubDomain: PBSUtils.randomString).rule): PBSUtils.randomFloorValue] + } + + and: "Account with maxSchemaDims in the DB" + def accountId = bidRequest.site.publisher.id + def account = getAccountWithEnabledFetch(accountId) + accountDao.save(account) + + and: "Set bidder response" + def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) + bidder.setResponse(bidRequest.id, bidResponse) + + when: "PBS processes auction request" + def response = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "PBS should log a errors" + assert response.ext?.errors[PREBID]*.code == [999] + assert response.ext?.errors[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: " + + "Price floor schema dimensions 4 exceeded its maximum number 3"] + } + def "PBS should emit errors when stored request has more rules than price-floor.max-rules for amp request"() { given: "Default AmpRequest" def ampRequest = AmpRequest.defaultAmpRequest From 2889b7751941b8c8f240d9b578e7e72cc2970307 Mon Sep 17 00:00:00 2001 From: markiian Date: Tue, 7 Jan 2025 12:35:38 +0200 Subject: [PATCH 05/10] Update after review --- .../PriceFloorsSignalingSpec.groovy | 31 ++----------------- 1 file changed, 2 insertions(+), 29 deletions(-) diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy index eba2e65f06a..9cb096fc0fe 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy @@ -2,6 +2,7 @@ package org.prebid.server.functional.tests.pricefloors import org.prebid.server.functional.model.db.StoredRequest import org.prebid.server.functional.model.pricefloors.Country +import org.prebid.server.functional.model.pricefloors.DeviceType import org.prebid.server.functional.model.pricefloors.ModelGroup import org.prebid.server.functional.model.pricefloors.PriceFloorData import org.prebid.server.functional.model.pricefloors.PriceFloorSchema @@ -17,6 +18,7 @@ import org.prebid.server.functional.model.request.auction.Video import org.prebid.server.functional.model.response.auction.BidResponse import org.prebid.server.functional.model.response.auction.MediaType import org.prebid.server.functional.util.PBSUtils +import spock.lang.IgnoreRest import static org.mockserver.model.HttpStatusCode.BAD_REQUEST_400 import static org.prebid.server.functional.model.Currency.USD @@ -679,35 +681,6 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert !response.ext?.errors } - def "PBS should fail by default with error when schema dimensions more than 3"() { - given: "BidRequest with schema 4 fields" - def bidRequest = bidRequestWithFloors.tap { - ext.prebid.floors.data.modelGroups[0].values = [(new Rule( - mediaType: MULTIPLE, - country: Country.MULTIPLE, - deviceType: PHONE, - pubDomain: PBSUtils.randomString).rule): PBSUtils.randomFloorValue] - } - - and: "Account with maxSchemaDims in the DB" - def accountId = bidRequest.site.publisher.id - def account = getAccountWithEnabledFetch(accountId) - accountDao.save(account) - - and: "Set bidder response" - def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) - bidder.setResponse(bidRequest.id, bidResponse) - - when: "PBS processes auction request" - def response = floorsPbsService.sendAuctionRequest(bidRequest) - - then: "PBS should log a errors" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: " + - "Price floor schema dimensions 4 exceeded its maximum number 3"] - } - def "PBS should emit errors when stored request has more rules than price-floor.max-rules for amp request"() { given: "Default AmpRequest" def ampRequest = AmpRequest.defaultAmpRequest From e3d885ee39cc7c6f7378af82e6ab779dbc0d69b6 Mon Sep 17 00:00:00 2001 From: markiian Date: Tue, 7 Jan 2025 15:20:25 +0200 Subject: [PATCH 06/10] Update after review --- .../pricefloors/PriceFloorsBaseSpec.groovy | 5 +- .../PriceFloorsSignalingSpec.groovy | 101 +++++++++++++++++- 2 files changed, 102 insertions(+), 4 deletions(-) diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsBaseSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsBaseSpec.groovy index e96eea9995a..8b3f5d936bd 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsBaseSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsBaseSpec.groovy @@ -58,14 +58,15 @@ abstract class PriceFloorsBaseSpec extends BaseSpec { maxFileSizeKb: 200, maxAgeSec: 86400, periodSec: 3600, - maxSchemaDims: 0) + maxSchemaDims: 5) def floors = new AccountPriceFloorsConfig(enabled: true, fetch: fetch, enforceFloorsRate: 100, enforceDealFloors: true, adjustForBidAdjustment: true, useDynamicData: true, - maxRules: 0) + maxRules: 0, + maxSchemaDims: 3) new AccountConfig(auction: new AccountAuctionConfig(priceFloors: floors)) } diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy index 9cb096fc0fe..9074b7fa96b 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy @@ -2,7 +2,6 @@ package org.prebid.server.functional.tests.pricefloors import org.prebid.server.functional.model.db.StoredRequest import org.prebid.server.functional.model.pricefloors.Country -import org.prebid.server.functional.model.pricefloors.DeviceType import org.prebid.server.functional.model.pricefloors.ModelGroup import org.prebid.server.functional.model.pricefloors.PriceFloorData import org.prebid.server.functional.model.pricefloors.PriceFloorSchema @@ -20,10 +19,11 @@ import org.prebid.server.functional.model.response.auction.MediaType import org.prebid.server.functional.util.PBSUtils import spock.lang.IgnoreRest +import java.time.Instant + import static org.mockserver.model.HttpStatusCode.BAD_REQUEST_400 import static org.prebid.server.functional.model.Currency.USD import static org.prebid.server.functional.model.bidder.BidderName.GENERIC -import static org.prebid.server.functional.model.pricefloors.DeviceType.PHONE import static org.prebid.server.functional.model.pricefloors.MediaType.BANNER import static org.prebid.server.functional.model.pricefloors.MediaType.MULTIPLE import static org.prebid.server.functional.model.pricefloors.MediaType.VIDEO @@ -34,6 +34,8 @@ import static org.prebid.server.functional.model.response.auction.ErrorType.PREB class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { + private static final Closure INVALID_CONFIG_METRIC = { account -> "alerts.account_config.${account}.price-floors" } + def "PBS should skip signalling for request with rules when ext.prebid.floors.enabled = false in request"() { given: "Default BidRequest with disabled floors" def bidRequest = bidRequestWithFloors.tap { @@ -606,6 +608,101 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { null | 1 } + def "PBS should emit errors when request has more schema.fields than default-account.max-schema-dims"() { + given: "Floor config with default account" + def accountConfig = getDefaultAccountConfigSettings().tap { + auction.priceFloors.maxSchemaDims = 1 + } + def pbsFloorConfig = GENERIC_ALIAS_CONFIG + ["price-floors.enabled" : "true", + "settings.default-account-config": encode(accountConfig)] + + and: "Prebid server with floor config" + def floorsPbsService = pbsServiceFactory.getService(pbsFloorConfig) + + and: "BidRequest with schema 2 fields" + def bidRequest = bidRequestWithFloors + + and: "Account with maxSchemaDims in the DB" + def accountId = bidRequest.site.publisher.id + def account = getAccountWithEnabledFetch(accountId).tap { + config.auction.priceFloors.maxSchemaDims = PBSUtils.randomNegativeNumber + } + accountDao.save(account) + + and: "Set bidder response" + def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) + bidder.setResponse(bidRequest.id, bidResponse) + + when: "PBS processes auction request" + def response = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "PBS should log a errors" + assert response.ext?.errors[PREBID]*.code == [999] + assert response.ext?.errors[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: " + + "Price floor schema dimensions 2 exceeded its maximum number 1"] + + and: "Metric alerts.account_config.ACCOUNT.price-floors should be update" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert metrics[INVALID_CONFIG_METRIC(bidRequest.accountId) as String] == 1 + + cleanup: "Stop and remove pbs container" + pbsServiceFactory.removeContainer(pbsFloorConfig) + } + + def "PBS should emit errors when request has more schema.fields than default-account.fetch.max-schema-dims"() { + given: "Test start time" + def startTime = Instant.now() + + and: "Flush metrics" + flushMetrics(floorsPbsService) + + and: "BidRequest with schema 2 fields" + def bidRequest = bidRequestWithFloors + + and: "Floor config with default account" + def accountConfig = getDefaultAccountConfigSettings().tap { + auction.priceFloors.fetch.enabled = true + auction.priceFloors.fetch.url = BASIC_FETCH_URL + bidRequest.site.publisher.id + auction.priceFloors.fetch.maxSchemaDims = 1 + auction.priceFloors.maxSchemaDims = null + } + def pbsFloorConfig = GENERIC_ALIAS_CONFIG + ["price-floors.enabled" : "true", + "settings.default-account-config": encode(accountConfig)] + + and: "Prebid server with floor config" + def floorsPbsService = pbsServiceFactory.getService(pbsFloorConfig) + + and: "Account with maxSchemaDims in the DB" + def accountId = bidRequest.site.publisher.id + def account = getAccountWithEnabledFetch(accountId).tap { + config.auction.priceFloors.fetch.maxSchemaDims = PBSUtils.randomNegativeNumber + } + accountDao.save(account) + + and: "Set bidder response" + def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) + bidder.setResponse(bidRequest.id, bidResponse) + + when: "PBS processes auction request" + floorsPbsService.sendAuctionRequest(bidRequest) + + then: "PBS should log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) + assert floorsLogs.size() == 1 + assert floorsLogs[0].contains("Failed to fetch price floor from provider for fetch.url: " + + "'$BASIC_FETCH_URL$accountId', account = $accountId with a reason : Price floor schema dimensions 2 " + + "exceeded its maximum number 1") + + and: "Metric alerts.account_config.ACCOUNT.price-floors should be update" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert metrics[INVALID_CONFIG_METRIC(bidRequest.accountId) as String] == 1 + + cleanup: "Stop and remove pbs container" + pbsServiceFactory.removeContainer(pbsFloorConfig) + } + def "PBS should emit errors when request has more schema.fields than fetch.max-schema-dims"() { given: "Default BidRequest with floorMin" def bidRequest = bidRequestWithFloors From f5d3e6fa0d7c7ff1d58aa06f703758455dcb06a9 Mon Sep 17 00:00:00 2001 From: markiian Date: Tue, 7 Jan 2025 17:47:48 +0200 Subject: [PATCH 07/10] Update after review --- .../PriceFloorsFetchingSpec.groovy | 6 ++-- .../PriceFloorsSignalingSpec.groovy | 30 +++++++++++-------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy index 508eb3a619b..55180fe60d4 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy @@ -2046,7 +2046,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { } } - def "PBS should validate fetch.max-schema-dims from account config"() { + def "PBS should validate fetch.max-schema-dims from account config and not reject entire auction"() { given: "Default BidRequest" def bidRequest = BidRequest.defaultBidRequest @@ -2075,7 +2075,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { PBSUtils.getRandomNumber(20) | null } - def "PBS should validate price-floor.max-rules from account config"() { + def "PBS should validate price-floor.max-rules from account config and not reject entire auction"() { given: "Default BidRequest" def bidRequest = BidRequest.defaultBidRequest @@ -2102,7 +2102,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { PBSUtils.randomNegativeNumber | null } - def "PBS should validate price-floor.max-schema-dims from account config"() { + def "PBS should validate price-floor.max-schema-dims from account config and not reject entire auction"() { given: "Default BidRequest" def bidRequest = BidRequest.defaultBidRequest diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy index 9074b7fa96b..0ff5521c881 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy @@ -17,7 +17,6 @@ import org.prebid.server.functional.model.request.auction.Video import org.prebid.server.functional.model.response.auction.BidResponse import org.prebid.server.functional.model.response.auction.MediaType import org.prebid.server.functional.util.PBSUtils -import spock.lang.IgnoreRest import java.time.Instant @@ -569,7 +568,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + - "Price floor rules number 2 exceeded its maximum number 1"] + "Price floor rules number ${bidRequest.ext.prebid.floors.data.modelGroups[0].values.keySet().size()} exceeded its maximum number 1"] where: maxRules | maxRulesSnakeCase @@ -600,7 +599,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + - "Price floor schema dimensions 2 exceeded its maximum number 1"] + "Price floor schema dimensions ${bidRequest.ext.prebid.floors.data.modelGroups[0].schema.fields.size()} exceeded its maximum number 1"] where: maxSchemaDims | maxSchemaDimsSnakeCase @@ -610,8 +609,9 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def "PBS should emit errors when request has more schema.fields than default-account.max-schema-dims"() { given: "Floor config with default account" + def maxSchemaDimension = 1 def accountConfig = getDefaultAccountConfigSettings().tap { - auction.priceFloors.maxSchemaDims = 1 + auction.priceFloors.maxSchemaDims = maxSchemaDimension } def pbsFloorConfig = GENERIC_ALIAS_CONFIG + ["price-floors.enabled" : "true", "settings.default-account-config": encode(accountConfig)] @@ -640,7 +640,8 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + - "Price floor schema dimensions 2 exceeded its maximum number 1"] + "Price floor schema dimensions ${bidRequest.ext.prebid.floors.data.modelGroups[0].schema.fields.size()} " + + "exceeded its maximum number ${maxSchemaDimension}"] and: "Metric alerts.account_config.ACCOUNT.price-floors should be update" def metrics = floorsPbsService.sendCollectedMetricsRequest() @@ -654,9 +655,6 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { given: "Test start time" def startTime = Instant.now() - and: "Flush metrics" - flushMetrics(floorsPbsService) - and: "BidRequest with schema 2 fields" def bidRequest = bidRequestWithFloors @@ -673,6 +671,9 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { and: "Prebid server with floor config" def floorsPbsService = pbsServiceFactory.getService(pbsFloorConfig) + and: "Flush metrics" + flushMetrics(floorsPbsService) + and: "Account with maxSchemaDims in the DB" def accountId = bidRequest.site.publisher.id def account = getAccountWithEnabledFetch(accountId).tap { @@ -721,7 +722,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + - "Price floor schema dimensions 2 exceeded its maximum number 1"] + "Price floor schema dimensions ${bidRequest.ext.prebid.floors.data.modelGroups[0].schema.fields.size()} exceeded its maximum number 1"] where: maxSchemaDims | maxSchemaDimsSnakeCase @@ -735,9 +736,10 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { and: "Account with maxSchemaDims in the DB" def accountId = bidRequest.site.publisher.id + def floorSchemaFilesSize = bidRequest.ext.prebid.floors.data.modelGroups[0].schema.fields.size() def account = getAccountWithEnabledFetch(accountId).tap { - config.auction.priceFloors.maxSchemaDims = 1 - config.auction.priceFloors.fetch.maxSchemaDims = 2 + config.auction.priceFloors.maxSchemaDims = floorSchemaFilesSize - 1 + config.auction.priceFloors.fetch.maxSchemaDims = floorSchemaFilesSize } accountDao.save(account) @@ -752,7 +754,8 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + - "Price floor schema dimensions 2 exceeded its maximum number 1"] + "Price floor schema dimensions ${floorSchemaFilesSize} " + + "exceeded its maximum number ${floorSchemaFilesSize - 1}"] } def "PBS shouldn't fail with error and maxSchemaDims take precede over fetch.maxSchemaDims when requested both"() { @@ -812,7 +815,8 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + - "Price floor rules number 2 exceeded its maximum number 1"] + "Price floor rules number ${ampStoredRequest.ext.prebid.floors.data.modelGroups[0].values.keySet().size()} " + + "exceeded its maximum number 1"] where: maxRules | maxRulesSnakeCase From 230c4878d8bed77841101e23ff31985fc64ed2e4 Mon Sep 17 00:00:00 2001 From: markiian Date: Wed, 8 Jan 2025 09:31:01 +0200 Subject: [PATCH 08/10] Update after review --- .../PriceFloorsSignalingSpec.groovy | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy index 0ff5521c881..9af53d5076b 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy @@ -520,7 +520,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def "PBS shouldn't emit errors when request schema.fields than floor-config.max-schema-dims"() { given: "Bid request with schema 2 fields" def bidRequest = bidRequestWithFloors.tap { - ext.prebid.floors.maxSchemaDims = PBSUtils.randomNumber + ext.prebid.floors.maxSchemaDims = PBSUtils.getRandomNumber(2) } and: "Account with maxSchemaDims in the DB" @@ -568,7 +568,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + - "Price floor rules number ${bidRequest.ext.prebid.floors.data.modelGroups[0].values.keySet().size()} exceeded its maximum number 1"] + "Price floor rules number ${getRuleSize(bidRequest)} exceeded its maximum number 1"] where: maxRules | maxRulesSnakeCase @@ -599,7 +599,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + - "Price floor schema dimensions ${bidRequest.ext.prebid.floors.data.modelGroups[0].schema.fields.size()} exceeded its maximum number 1"] + "Price floor schema dimensions ${getSchemaSize(bidRequest)} exceeded its maximum number 1"] where: maxSchemaDims | maxSchemaDimsSnakeCase @@ -640,7 +640,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + - "Price floor schema dimensions ${bidRequest.ext.prebid.floors.data.modelGroups[0].schema.fields.size()} " + + "Price floor schema dimensions ${getSchemaSize(bidRequest)} " + "exceeded its maximum number ${maxSchemaDimension}"] and: "Metric alerts.account_config.ACCOUNT.price-floors should be update" @@ -722,7 +722,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + - "Price floor schema dimensions ${bidRequest.ext.prebid.floors.data.modelGroups[0].schema.fields.size()} exceeded its maximum number 1"] + "Price floor schema dimensions ${getSchemaSize(bidRequest)} exceeded its maximum number 1"] where: maxSchemaDims | maxSchemaDimsSnakeCase @@ -736,7 +736,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { and: "Account with maxSchemaDims in the DB" def accountId = bidRequest.site.publisher.id - def floorSchemaFilesSize = bidRequest.ext.prebid.floors.data.modelGroups[0].schema.fields.size() + def floorSchemaFilesSize = getSchemaSize(bidRequest) def account = getAccountWithEnabledFetch(accountId).tap { config.auction.priceFloors.maxSchemaDims = floorSchemaFilesSize - 1 config.auction.priceFloors.fetch.maxSchemaDims = floorSchemaFilesSize @@ -815,7 +815,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + - "Price floor rules number ${ampStoredRequest.ext.prebid.floors.data.modelGroups[0].values.keySet().size()} " + + "Price floor rules number ${getRuleSize(ampStoredRequest)} " + "exceeded its maximum number 1"] where: @@ -823,4 +823,12 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { 1 | null null | 1 } + + private static int getSchemaSize(BidRequest bidRequest) { + bidRequest?.ext?.prebid?.floors?.data?.modelGroups[0].schema.fields.size() + } + + private static int getRuleSize(BidRequest bidRequest) { + bidRequest?.ext?.prebid?.floors?.data?.modelGroups[0].values.size() + } } From 3965783a85ebae4e5c8697609b420ecc262a583e Mon Sep 17 00:00:00 2001 From: markiian Date: Wed, 8 Jan 2025 11:47:30 +0200 Subject: [PATCH 09/10] Update after review --- .../PriceFloorsSignalingSpec.groovy | 51 ++++++++++--------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy index 9af53d5076b..ece4faf6534 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy @@ -34,6 +34,8 @@ import static org.prebid.server.functional.model.response.auction.ErrorType.PREB class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { private static final Closure INVALID_CONFIG_METRIC = { account -> "alerts.account_config.${account}.price-floors" } + private static final int MAX_SCHEMA_DIMENSIONS_SIZE = 1 + private static final int MAX_RULES_SIZE = 1 def "PBS should skip signalling for request with rules when ext.prebid.floors.enabled = false in request"() { given: "Default BidRequest with disabled floors" @@ -568,12 +570,12 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + - "Price floor rules number ${getRuleSize(bidRequest)} exceeded its maximum number 1"] + "Price floor rules number ${getRuleSize(bidRequest)} exceeded its maximum number ${MAX_RULES_SIZE}"] where: - maxRules | maxRulesSnakeCase - 1 | null - null | 1 + maxRules | maxRulesSnakeCase + MAX_RULES_SIZE | null + null | MAX_RULES_SIZE } def "PBS should emit errors when request has more schema.fields than floor-config.max-schema-dims"() { @@ -599,19 +601,18 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + - "Price floor schema dimensions ${getSchemaSize(bidRequest)} exceeded its maximum number 1"] + "Price floor schema dimensions ${getSchemaSize(bidRequest)} exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}"] where: - maxSchemaDims | maxSchemaDimsSnakeCase - 1 | null - null | 1 + maxSchemaDims | maxSchemaDimsSnakeCase + MAX_SCHEMA_DIMENSIONS_SIZE | null + null | MAX_SCHEMA_DIMENSIONS_SIZE } def "PBS should emit errors when request has more schema.fields than default-account.max-schema-dims"() { given: "Floor config with default account" - def maxSchemaDimension = 1 def accountConfig = getDefaultAccountConfigSettings().tap { - auction.priceFloors.maxSchemaDims = maxSchemaDimension + auction.priceFloors.maxSchemaDims = MAX_SCHEMA_DIMENSIONS_SIZE } def pbsFloorConfig = GENERIC_ALIAS_CONFIG + ["price-floors.enabled" : "true", "settings.default-account-config": encode(accountConfig)] @@ -641,7 +642,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + "Price floor schema dimensions ${getSchemaSize(bidRequest)} " + - "exceeded its maximum number ${maxSchemaDimension}"] + "exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}"] and: "Metric alerts.account_config.ACCOUNT.price-floors should be update" def metrics = floorsPbsService.sendCollectedMetricsRequest() @@ -662,7 +663,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def accountConfig = getDefaultAccountConfigSettings().tap { auction.priceFloors.fetch.enabled = true auction.priceFloors.fetch.url = BASIC_FETCH_URL + bidRequest.site.publisher.id - auction.priceFloors.fetch.maxSchemaDims = 1 + auction.priceFloors.fetch.maxSchemaDims = MAX_SCHEMA_DIMENSIONS_SIZE auction.priceFloors.maxSchemaDims = null } def pbsFloorConfig = GENERIC_ALIAS_CONFIG + ["price-floors.enabled" : "true", @@ -693,8 +694,8 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) assert floorsLogs.size() == 1 assert floorsLogs[0].contains("Failed to fetch price floor from provider for fetch.url: " + - "'$BASIC_FETCH_URL$accountId', account = $accountId with a reason : Price floor schema dimensions 2 " + - "exceeded its maximum number 1") + "'$BASIC_FETCH_URL$accountId', account = $accountId with a reason : Price floor schema dimensions ${getSchemaSize(bidRequest)} " + + "exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}") and: "Metric alerts.account_config.ACCOUNT.price-floors should be update" def metrics = floorsPbsService.sendCollectedMetricsRequest() @@ -722,12 +723,12 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.code == [999] assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + - "Price floor schema dimensions ${getSchemaSize(bidRequest)} exceeded its maximum number 1"] + "Price floor schema dimensions ${getSchemaSize(bidRequest)} exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}"] where: - maxSchemaDims | maxSchemaDimsSnakeCase - 1 | null - null | 1 + maxSchemaDims | maxSchemaDimsSnakeCase + MAX_SCHEMA_DIMENSIONS_SIZE | null + null | MAX_SCHEMA_DIMENSIONS_SIZE } def "PBS should fail with error and maxSchemaDims take precede over fetch.maxSchemaDims when requested both"() { @@ -755,7 +756,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + "Price floor schema dimensions ${floorSchemaFilesSize} " + - "exceeded its maximum number ${floorSchemaFilesSize - 1}"] + "exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}"] } def "PBS shouldn't fail with error and maxSchemaDims take precede over fetch.maxSchemaDims when requested both"() { @@ -765,8 +766,8 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { and: "Account with maxSchemaDims in the DB" def accountId = bidRequest.site.publisher.id def account = getAccountWithEnabledFetch(accountId).tap { - config.auction.priceFloors.maxSchemaDims = 2 - config.auction.priceFloors.fetch.maxSchemaDims = 1 + config.auction.priceFloors.maxSchemaDims = getSchemaSize(bidRequest) + config.auction.priceFloors.fetch.maxSchemaDims = getSchemaSize(bidRequest) - 1 } accountDao.save(account) @@ -816,12 +817,12 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert response.ext?.errors[PREBID]*.message == ["Failed to parse price floors from request, with a reason: " + "Price floor rules number ${getRuleSize(ampStoredRequest)} " + - "exceeded its maximum number 1"] + "exceeded its maximum number ${MAX_RULES_SIZE}"] where: - maxRules | maxRulesSnakeCase - 1 | null - null | 1 + maxRules | maxRulesSnakeCase + MAX_RULES_SIZE | null + null | MAX_RULES_SIZE } private static int getSchemaSize(BidRequest bidRequest) { From 312263bdfbb42af06f51c1e562f8c534b28b13c5 Mon Sep 17 00:00:00 2001 From: markiian Date: Wed, 8 Jan 2025 11:49:05 +0200 Subject: [PATCH 10/10] Update after review --- .../tests/pricefloors/PriceFloorsSignalingSpec.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy index ece4faf6534..f1385b1649d 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy @@ -739,7 +739,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def accountId = bidRequest.site.publisher.id def floorSchemaFilesSize = getSchemaSize(bidRequest) def account = getAccountWithEnabledFetch(accountId).tap { - config.auction.priceFloors.maxSchemaDims = floorSchemaFilesSize - 1 + config.auction.priceFloors.maxSchemaDims = MAX_SCHEMA_DIMENSIONS_SIZE config.auction.priceFloors.fetch.maxSchemaDims = floorSchemaFilesSize } accountDao.save(account)