From 56c395d5c64f64a69ede7f9e4c985a3bd330eaac Mon Sep 17 00:00:00 2001 From: osulzhenko Date: Fri, 7 Feb 2025 21:24:09 +0200 Subject: [PATCH 1/8] Increase coverage for floors validation --- .../pricefloors/PriceFloorsBaseSpec.groovy | 3 +- .../PriceFloorsSignalingSpec.groovy | 164 ++++++++++++++++++ 2 files changed, 165 insertions(+), 2 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 8b3f5d936bd..4ca4e3dfe6d 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 @@ -34,8 +34,7 @@ abstract class PriceFloorsBaseSpec extends BaseSpec { public static final BigDecimal FLOOR_MIN = 0.5 public static final BigDecimal FLOOR_MAX = 2 - public static final Map FLOORS_CONFIG = ["price-floors.enabled" : "true", - "settings.default-account-config": encode(defaultAccountConfigSettings)] + public static final Map FLOORS_CONFIG = ["price-floors.enabled": "true"] protected static final String BASIC_FETCH_URL = networkServiceContainer.rootUri + FloorsProvider.FLOORS_ENDPOINT protected static final int MAX_MODEL_WEIGHT = 100 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 72be2f0a0f3..badd9664671 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 @@ -1,5 +1,11 @@ package org.prebid.server.functional.tests.pricefloors +import org.junit.Ignore +import org.prebid.server.floors.model.PriceFloorModelGroup +import org.prebid.server.functional.model.config.AccountAuctionConfig +import org.prebid.server.functional.model.config.AccountConfig +import org.prebid.server.functional.model.config.AccountPriceFloorsConfig +import org.prebid.server.functional.model.db.Account import org.prebid.server.functional.model.db.StoredRequest import org.prebid.server.functional.model.pricefloors.Country import org.prebid.server.functional.model.pricefloors.ModelGroup @@ -17,6 +23,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 java.time.Instant @@ -826,6 +833,163 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { null | MAX_RULES_SIZE } + def "PBS should emit error in log and response when floors are enabled but data is invalid"() { + given: "Default BidRequest with disabled floors" + def bidRequest = bidRequestWithFloors.tap { + ext.prebid.floors.data = null + } + + when: "PBS processes auction request" + def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "Response should includer error errors" + assert bidResponse.ext?.errors[PREBID]*.code == [999] + assert bidResponse.ext?.errors[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: Price floor rules data must be present"] + + and: "PBS should process bidRequest" + assert bidder.getBidderRequests(bidRequest.id).size() == 1 + } + + def "PBS should emit error in log and response when floors skipRate is out of range"() { + given: "BidRequest with invalid skipRate" + def bidRequest = bidRequestWithFloors.tap { + ext.prebid.floors.data.skipRate = requestSkipRate + } + + when: "PBS processes auction request" + def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "Response should include error" + assert bidResponse.ext?.errors[PREBID]*.code == [999] + assert bidResponse.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 $requestSkipRate"] + + and: "PBS should process bidRequest" + assert bidder.getBidderRequests(bidRequest.id).size() == 1 + + where: + requestSkipRate << [PBSUtils.randomNegativeNumber, PBSUtils.getRandomNumber(100)] + } + + def "PBS should emit error in log and response when floors modelGroups is empty"() { + given: "BidRequest with empty modelGroups" + def bidRequest = bidRequestWithFloors.tap { + ext.prebid.floors.data.modelGroups = requestModelGroups + } + + when: "PBS processes auction request" + def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "Response should include error" + assert bidResponse.ext?.errors[PREBID]*.code == [999] + assert bidResponse.ext?.errors[PREBID]*.message == [ + "Failed to parse price floors from request, with a reason: Price floor rules should contain at least one model group" + ] + + and: "PBS should process bidRequest" + assert bidder.getBidderRequests(bidRequest.id).size() == 1 + + where: + requestModelGroups << [null, []] + } + + def "PBS should emit error in log and response when modelGroup modelWeight is out of range"() { + given: "BidRequest with invalid modelWeight" + def bidRequest = bidRequestWithFloors.tap { + ext.prebid.floors.data.modelGroups = [ + new ModelGroup(modelWeight: requestModelWeight) + ] + } + + when: "PBS processes auction request" + def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "Response should include error" + assert bidResponse.ext?.errors[PREBID]*.code == [999] + assert bidResponse.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 $requestModelWeight"] + + and: "PBS should process bidRequest" + assert bidder.getBidderRequests(bidRequest.id).size() == 1 + + where: + requestModelWeight << [PBSUtils.randomNegativeNumber, PBSUtils.getRandomNumber(100)] + } + + def "PBS should emit error in log and response when modelGroup skipRate is out of range"() { + given: "BidRequest with invalid modelGroup skipRate" + def requestModelGroupsSkipRate = PBSUtils.getRandomNumber(100) + def bidRequest = bidRequestWithFloors.tap { + ext.prebid.floors.data.modelGroups = [ + new ModelGroup(skipRate: requestModelGroupsSkipRate) + ] + } + + when: "PBS processes auction request" + def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "Response should include error" + assert bidResponse.ext?.errors[PREBID]*.code == [999] + assert bidResponse.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 $requestModelGroupsSkipRate"] + + and: "PBS should process bidRequest" + assert bidder.getBidderRequests(bidRequest.id).size() == 1 + } + + def "PBS should emit error in log and response when modelGroup defaultFloor is negative"() { + given: "BidRequest with negative defaultFloor" + def requestModelGroupsSkipRate = PBSUtils.randomNegativeNumber + + def bidRequest = bidRequestWithFloors.tap { + ext.prebid.floors.data.modelGroups = [ + new ModelGroup(defaultFloor: requestModelGroupsSkipRate) + ] + } + + when: "PBS processes auction request" + def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "Response should include error" + assert bidResponse.ext?.errors[PREBID]*.code == [999] + assert bidResponse.ext?.errors[PREBID]*.message == [ + "Price floor modelGroup default must be positive float, but was $requestModelGroupsSkipRate" + ] + + and: "PBS should process bidRequest" + assert bidder.getBidderRequests(bidRequest.id).size() == 1 + } + + def "PBS should use default floors config when original account config is invalid"() { + given: "Test start time" + def startTime = Instant.now() + + and: "Bid request with floors" + def bidRequest = bidRequestWithFloors + + and: "Account with invalid floors config" + def accountConfig = new AccountConfig(auction: new AccountAuctionConfig(priceFloors: new AccountPriceFloorsConfig(enabled: false))) + def account = new Account(uuid: bidRequest.accountId, config: accountConfig) + 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) + + and: "PBS floors validation failure should not reject the entire auction" + assert !response.seatbid?.isEmpty() + + response.seatbid + then: "PBS shouldn't log a errors" + assert !response.ext?.errors + } + private static int getSchemaSize(BidRequest bidRequest) { bidRequest?.ext?.prebid?.floors?.data?.modelGroups[0].schema.fields.size() } From 25e6876b9b87050146b780130f2d2d74da5205e4 Mon Sep 17 00:00:00 2001 From: osulzhenko Date: Mon, 10 Feb 2025 10:29:31 +0200 Subject: [PATCH 2/8] Increase coverage for floors validation --- .../tests/pricefloors/PriceFloorsSignalingSpec.groovy | 6 ++---- 1 file changed, 2 insertions(+), 4 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 badd9664671..705206cfed7 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 @@ -1,7 +1,5 @@ package org.prebid.server.functional.tests.pricefloors -import org.junit.Ignore -import org.prebid.server.floors.model.PriceFloorModelGroup import org.prebid.server.functional.model.config.AccountAuctionConfig import org.prebid.server.functional.model.config.AccountConfig import org.prebid.server.functional.model.config.AccountPriceFloorsConfig @@ -23,7 +21,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 @@ -956,7 +953,8 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { then: "Response should include error" assert bidResponse.ext?.errors[PREBID]*.code == [999] assert bidResponse.ext?.errors[PREBID]*.message == [ - "Price floor modelGroup default must be positive float, but was $requestModelGroupsSkipRate" + "Failed to parse price floors from request, with a reason: Price floor modelGroup default must be " + + "positive float, but was $requestModelGroupsSkipRate" ] and: "PBS should process bidRequest" From 0dba5f9aaa70bea3ccb891c5d724d4b18c1e54e3 Mon Sep 17 00:00:00 2001 From: osulzhenko Date: Sun, 27 Apr 2025 00:20:33 +0300 Subject: [PATCH 3/8] update tests --- .../service/PrebidServerService.groovy | 2 +- .../server/functional/tests/BaseSpec.groovy | 1 + .../functional/tests/BidValidationSpec.groovy | 4 +- .../functional/tests/MetricsSpec.groovy | 4 +- .../tests/bidder/openx/OpenxSpec.groovy | 2 +- .../pricefloors/PriceFloorsBaseSpec.groovy | 6 +- .../PriceFloorsFetchingSpec.groovy | 92 ++-- .../PriceFloorsSignalingSpec.groovy | 424 ++++++++++++++---- .../tests/privacy/GdprAmpSpec.groovy | 2 +- .../tests/privacy/GdprAuctionSpec.groovy | 2 +- 10 files changed, 396 insertions(+), 143 deletions(-) diff --git a/src/test/groovy/org/prebid/server/functional/service/PrebidServerService.groovy b/src/test/groovy/org/prebid/server/functional/service/PrebidServerService.groovy index 31df1efc8d5..2dd3106d690 100644 --- a/src/test/groovy/org/prebid/server/functional/service/PrebidServerService.groovy +++ b/src/test/groovy/org/prebid/server/functional/service/PrebidServerService.groovy @@ -77,7 +77,7 @@ class PrebidServerService implements ObjectMapperWrapper { prometheusRequestSpecification = buildAndGetRequestSpecification(pbsContainer.prometheusRootUri, authenticationScheme) } - BidResponse sendAuctionRequest(BidRequest bidRequest, Map headers = [:]) { + BidResponse sendAuctionRequest(BidRequest bidRequest, Map headers = [:]) { def response = postAuction(bidRequest, headers) checkResponseStatusCode(response) diff --git a/src/test/groovy/org/prebid/server/functional/tests/BaseSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/BaseSpec.groovy index 63ba2516ff4..a4fda13f829 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/BaseSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/BaseSpec.groovy @@ -40,6 +40,7 @@ abstract class BaseSpec extends Specification implements ObjectMapperWrapper { private static final int MIN_TIMEOUT = DEFAULT_TIMEOUT private static final int DEFAULT_TARGETING_PRECISION = 1 private static final String DEFAULT_CACHE_DIRECTORY = "/app/prebid-server/data" + protected static final String ALERT_GENERAL = "alerts.general" protected static final Map GENERIC_ALIAS_CONFIG = ["adapters.generic.aliases.alias.enabled" : "true", "adapters.generic.aliases.alias.endpoint": "$networkServiceContainer.rootUri/auction".toString()] diff --git a/src/test/groovy/org/prebid/server/functional/tests/BidValidationSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/BidValidationSpec.groovy index 579a8e16597..9dd17c31e0a 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/BidValidationSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/BidValidationSpec.groovy @@ -63,7 +63,7 @@ class BidValidationSpec extends BaseSpec { and: "Bid validation metric value is incremented" def metrics = strictPrebidService.sendCollectedMetricsRequest() - assert metrics["alerts.general"] == 1 + assert metrics[ALERT_GENERAL] == 1 where: bidRequest << [BidRequest.getDefaultBidRequest(DistributionChannel.APP).tap { @@ -105,7 +105,7 @@ class BidValidationSpec extends BaseSpec { and: "Bid validation metric value is incremented" def metrics = softPrebidService.sendCollectedMetricsRequest() - assert metrics["alerts.general"] == 1 + assert metrics[ALERT_GENERAL] == 1 and: "PBS log should contain message" def logs = softPrebidService.getLogsByTime(startTime) diff --git a/src/test/groovy/org/prebid/server/functional/tests/MetricsSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/MetricsSpec.groovy index 92ef175681e..ee3e808a56e 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/MetricsSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/MetricsSpec.groovy @@ -146,7 +146,7 @@ class MetricsSpec extends BaseSpec { assert metrics["adapter.generic.requests.type.openrtb2-dooh" as String] == 1 and: "alert.general metric should be updated" - assert metrics["alerts.general" as String] == 1 + assert metrics[ALERT_GENERAL] == 1 and: "Other channel types should not be populated" assert !metrics["account.${accountId}.requests.type.openrtb2-web" as String] @@ -175,7 +175,7 @@ class MetricsSpec extends BaseSpec { assert metrics["adapter.generic.requests.type.openrtb2-app" as String] == 1 and: "alert.general metric should be updated" - assert metrics["alerts.general" as String] == 1 + assert metrics[ALERT_GENERAL] == 1 and: "Other channel types should not be populated" assert !metrics["account.${accountId}.requests.type.openrtb2-dooh" as String] diff --git a/src/test/groovy/org/prebid/server/functional/tests/bidder/openx/OpenxSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/bidder/openx/OpenxSpec.groovy index 8e6d7f1a57d..97a0015cea5 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/bidder/openx/OpenxSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/bidder/openx/OpenxSpec.groovy @@ -444,7 +444,7 @@ class OpenxSpec extends BaseSpec { and: "Alert.general metric should be updated" def metrics = pbsService.sendCollectedMetricsRequest() - assert metrics["alerts.general" as String] == 1 + assert metrics[ALERT_GENERAL] == 1 } def "PBS shouldn't populate fledge or igi config when bidder respond with igb"() { 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 4ca4e3dfe6d..4e1cbbf349b 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 @@ -15,6 +15,7 @@ import org.prebid.server.functional.model.request.auction.BidRequest import org.prebid.server.functional.model.request.auction.BidRequestExt import org.prebid.server.functional.model.request.auction.DistributionChannel import org.prebid.server.functional.model.request.auction.ExtPrebidFloors +import org.prebid.server.functional.model.request.auction.FetchStatus import org.prebid.server.functional.model.request.auction.Prebid import org.prebid.server.functional.model.request.auction.Video import org.prebid.server.functional.model.response.currencyrates.CurrencyRatesResponse @@ -120,8 +121,9 @@ abstract class PriceFloorsBaseSpec extends BaseSpec { protected void cacheFloorsProviderRules(BidRequest bidRequest, PrebidServerService pbsService = floorsPbsService, - BidderName bidderName = BidderName.GENERIC) { - PBSUtils.waitUntil({ getRequests(pbsService.sendAuctionRequest(bidRequest))[bidderName.value]?.first?.ext?.prebid?.floors?.fetchStatus != INPROGRESS }, + BidderName bidderName = BidderName.GENERIC, + FetchStatus fetchStatus = INPROGRESS) { + PBSUtils.waitUntil({ getRequests(pbsService.sendAuctionRequest(bidRequest))[bidderName.value]?.first?.ext?.prebid?.floors?.fetchStatus != fetchStatus }, 5000, 1000) } 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 a897fbeff7c..d0316172892 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 @@ -1,5 +1,6 @@ package org.prebid.server.functional.tests.pricefloors +import org.prebid.server.functional.model.bidder.BidderName import org.prebid.server.functional.model.config.PriceFloorsFetch import org.prebid.server.functional.model.db.StoredRequest import org.prebid.server.functional.model.pricefloors.ModelGroup @@ -545,7 +546,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(bidRequest.accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService) + cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -554,7 +555,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def metrics = floorsPbsService.sendCollectedMetricsRequest() assert metrics[FETCH_FAILURE_METRIC] == 1 - then: "PBS should fetch data" + and: "PBS should fetch data" assert floorsProvider.getRequestCount(bidRequest.accountId) == 1 and: "PBS log should contain error" @@ -645,7 +646,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, BAD_REQUEST_400) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService) + cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -687,6 +688,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def invalidJson = "{{}}" floorsProvider.setResponse(accountId, invalidJson) + and: "PBS fetch rules from floors provider" + cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -726,6 +730,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "Set Floors Provider response with invalid json" floorsProvider.setResponse(accountId, "") + and: "PBS fetch rules from floors provider" + cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -768,6 +775,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { } floorsProvider.setResponse(accountId, floorsResponse) + and: "PBS fetch rules from floors provider" + cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -810,6 +820,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { } floorsProvider.setResponse(accountId, floorsResponse) + and: "PBS fetch rules from floors provider" + cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -855,6 +868,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { } floorsProvider.setResponse(accountId, floorsResponse) + and: "PBS fetch rules from floors provider" + cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -901,6 +917,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "Set Floors Provider response with timeout" floorsProvider.setResponseWithTimeout(accountId) + and: "PBS fetch rules from floors provider" + cacheFloorsProviderRules(bidRequest, pbsService, BidderName.GENERIC, NONE) + when: "PBS processes auction request" def response = pbsService.sendAuctionRequest(bidRequest) @@ -944,6 +963,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def responseSize = convertKilobyteSizeToByte(maxSize) + 75 floorsProvider.setResponse(accountId, floorsResponse, ["Content-Length": responseSize as String]) + and: "PBS fetch rules from floors provider" + cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -1284,9 +1306,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == + and: "Response should contain 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 floorMin " + "must be positive float, but was $invalidFloorMin"] } @@ -1312,9 +1334,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == + and: "Response should contain 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 " + "should contain at least one model group"] } @@ -1340,9 +1362,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == + and: "Response should contain 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 values " + "can't be null or empty, but were null"] } @@ -1372,9 +1394,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequest(bidRequest.id) assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == + and: "Response should contain 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 modelGroup modelWeight " + "must be in range(1-100), but was $invalidModelWeight"] where: @@ -1411,9 +1433,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(ampStoredRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == + and: "Response should contain 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 modelGroup modelWeight " + "must be in range(1-100), but was $invalidModelWeight"] @@ -1451,9 +1473,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == + and: "Response should contain 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 root skipRate " + "must be in range(0-100), but was $invalidSkipRate"] @@ -1491,9 +1513,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == + and: "Response should contain 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 data skipRate " + "must be in range(0-100), but was $invalidSkipRate"] @@ -1531,9 +1553,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == + and: "Response should contain 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 modelGroup skipRate " + "must be in range(0-100), but was $invalidSkipRate"] @@ -1567,9 +1589,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == + and: "Response should contain 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 modelGroup default " + "must be positive float, but was $invalidDefaultFloorValue"] } @@ -1651,7 +1673,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest) + cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -1669,7 +1691,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "PBS log should contain error" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL) + 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 modelGroup modelWeight" + @@ -1710,7 +1732,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest) + cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -1769,7 +1791,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest) + cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -1828,7 +1850,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest) + cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) 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 705206cfed7..d862f53c2af 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 @@ -33,13 +33,23 @@ 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.request.auction.FetchStatus.INPROGRESS +import static org.prebid.server.functional.model.request.auction.FetchStatus.NONE import static org.prebid.server.functional.model.response.auction.ErrorType.PREBID class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { private static final Closure INVALID_CONFIG_METRIC = { account -> "alerts.account_config.${account}.price-floors" } + private static final Closure WARNING_MESSAGE = { message -> "Failed to parse price floors from request, with a reason: $message" } + private static final Closure ERROR_LOG = { bidRequest, message -> "Failed to parse price floors from request with id: " + + "'${bidRequest.id}', with a reason: $message" } private static final int MAX_SCHEMA_DIMENSIONS_SIZE = 1 private static final int MAX_RULES_SIZE = 1 + private static Instant startTime + + def setupSpec() { + startTime = Instant.now() + } def "PBS should skip signalling for request with rules when ext.prebid.floors.enabled = false in request"() { given: "Default BidRequest with disabled floors" @@ -284,7 +294,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) - then: "PBS should not log warning" + then: "PBS should not log warning or errors" assert !response.ext.warnings assert !response.ext.errors @@ -323,8 +333,9 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { when: "PBS processes amp request" def response = floorsPbsService.sendAmpRequest(ampRequest) - then: "PBS should not log warning" + then: "PBS should not log warning or errors" assert !response.ext.warnings + assert !response.ext.errors and: "Bidder request should contain bidFloor from the request" def bidderRequest = bidder.getBidderRequest(ampStoredRequest.id) @@ -524,7 +535,7 @@ 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"() { + def "PBS shouldn't emit warning when request schema.fields equal to floor-config.max-schema-dims"() { given: "Bid request with schema 2 fields" def bidRequest = bidRequestWithFloors.tap { ext.prebid.floors.maxSchemaDims = PBSUtils.getRandomNumber(2) @@ -542,11 +553,12 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) - then: "PBS shouldn't log a errors" - assert !response.ext?.errors + then: "PBS should not add warning or errors" + assert !response.ext.warnings + assert !response.ext.errors } - def "PBS should emit errors when request has more rules than price-floor.max-rules"() { + 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 { @@ -568,22 +580,29 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { bidResponse.seatbid.first().bid.first().price = requestFloorValue bidder.setResponse(bidRequest.id, bidResponse) + and: "Flush metrics" + flushMetrics(floorsPbsService) + 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 == + 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 ${getRuleSize(bidRequest)} exceeded its maximum number ${MAX_RULES_SIZE}"] + and: "Alerts.general metrics should be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert !metrics[ALERT_GENERAL] + where: 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"() { + 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 @@ -599,22 +618,29 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) bidder.setResponse(bidRequest.id, bidResponse) + and: "Flush metrics" + flushMetrics(floorsPbsService) + 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 == + 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 ${getSchemaSize(bidRequest)} exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}"] + and: "Alerts.general metrics should be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert !metrics[ALERT_GENERAL] + where: 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"() { + def "PBS should emit warning 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 = MAX_SCHEMA_DIMENSIONS_SIZE @@ -639,29 +665,34 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) bidder.setResponse(bidRequest.id, bidResponse) + and: "Flush metrics" + flushMetrics(floorsPbsService) + 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 ${getSchemaSize(bidRequest)} " + - "exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}"] + then: "PBS should log a warning" + def message = "Price floor schema dimensions ${getSchemaSize(bidRequest)} exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + + and: "PBS should log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) + assert floorsLogs.size() == 1 + assert floorsLogs[0].contains(message) 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 + assert !metrics[ALERT_GENERAL] 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: "BidRequest with schema 2 fields" + def "PBS should emit warning when request has more schema.fields than default-account.fetch.max-schema-dims"() { + given: "BidRequest with schema 2 fields" def bidRequest = bidRequestWithFloors and: "Floor config with default account" @@ -669,7 +700,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { auction.priceFloors.fetch.enabled = true auction.priceFloors.fetch.url = BASIC_FETCH_URL + bidRequest.site.publisher.id auction.priceFloors.fetch.maxSchemaDims = MAX_SCHEMA_DIMENSIONS_SIZE - auction.priceFloors.maxSchemaDims = null + auction.priceFloors.maxSchemaDims = MAX_SCHEMA_DIMENSIONS_SIZE } def pbsFloorConfig = GENERIC_ALIAS_CONFIG + ["price-floors.enabled" : "true", "settings.default-account-config": encode(accountConfig)] @@ -694,7 +725,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { when: "PBS processes auction request" floorsPbsService.sendAuctionRequest(bidRequest) - then: "PBS should log a errors" + then: "PBS shouldn't log a errors" def logs = floorsPbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) assert floorsLogs.size() == 1 @@ -705,13 +736,14 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { 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 + assert metrics[ALERT_GENERAL] == 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 "PBS should emit warning when request has more schema.fields than fetch.max-schema-dims"() { + given: "BidRequest with schema 2 fields" def bidRequest = bidRequestWithFloors and: "Account with disabled fetch in the DB" @@ -721,14 +753,26 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { } accountDao.save(account) + and: "Flush metrics" + flushMetrics(floorsPbsService) + 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 ${getSchemaSize(bidRequest)} exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}"] + then: "PBS should log a warning" + def message = "Price floor schema dimensions ${getSchemaSize(bidRequest)} exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + + and: "PBS should log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) + assert floorsLogs.size() == 1 + assert floorsLogs[0].contains(message) + + and: "Alerts.general metrics shouldn't be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert !metrics[ALERT_GENERAL] where: maxSchemaDims | maxSchemaDimsSnakeCase @@ -753,15 +797,27 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) bidder.setResponse(bidRequest.id, bidResponse) + and: "Flush metrics" + flushMetrics(floorsPbsService) + 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 ${floorSchemaFilesSize} " + - "exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}"] + then: "PBS should log a warning" + def message = "Price floor schema dimensions ${floorSchemaFilesSize} " + + "exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + + and: "PBS should log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) + assert floorsLogs.size() == 1 + assert floorsLogs[0].contains(message) + + and: "Alerts.general metrics shouldn't be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert !metrics[ALERT_GENERAL] } def "PBS shouldn't fail with error and maxSchemaDims take precede over fetch.maxSchemaDims when requested both"() { @@ -783,15 +839,15 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) - then: "PBS shouldn't log a errors" - assert !response.ext?.errors + then: "PBS shouldn't add warnings or errors" + assert !response.ext?.warnings } - def "PBS should emit errors when stored request has more rules than price-floor.max-rules for amp request"() { + 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 " + and: "Default stored request with 2 rules" def requestFloorValue = PBSUtils.randomFloorValue def ampStoredRequest = BidRequest.defaultStoredRequest.tap { ext.prebid.floors = ExtPrebidFloors.extPrebidFloors @@ -814,15 +870,27 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { bidResponse.seatbid.first().bid.first().price = requestFloorValue bidder.setResponse(ampStoredRequest.id, bidResponse) + and: "Flush metrics" + flushMetrics(floorsPbsService) + when: "PBS processes amp request" def response = floorsPbsService.sendAmpRequest(ampRequest) - 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 ${getRuleSize(ampStoredRequest)} " + - "exceeded its maximum number ${MAX_RULES_SIZE}"] + then: "PBS should log a warning" + def message = "Price floor rules number ${getRuleSize(ampStoredRequest)} " + + "exceeded its maximum number ${MAX_RULES_SIZE}" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + + and: "PBS should log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, ERROR_LOG(ampStoredRequest, message)) + assert floorsLogs.size() == 1 + assert floorsLogs[0].contains(message) + + and: "Alerts.general metrics shouldn't be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert !metrics[ALERT_GENERAL] where: maxRules | maxRulesSnakeCase @@ -830,22 +898,86 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { null | MAX_RULES_SIZE } - def "PBS should emit error in log and response when floors are enabled but data is invalid"() { - given: "Default BidRequest with disabled floors" + def "PBS should emit error in log and response when data is invalid and floors status is in progress"() { + given: "Default BidRequest with empty floors.data" + def bidRequest = bidRequestWithFloors.tap { + ext.prebid.floors.data = null + } + + and: "Account with invalid floors config" + def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { + config.auction.priceFloors.enabled = true + } + accountDao.save(account) + + and: "Flush metrics" + flushMetrics(floorsPbsService) + + when: "PBS processes auction request" + def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "Response should includer error warning" + def message = 'Price floor rules data must be present' + assert bidResponse.ext?.warnings[PREBID]*.code == [999] + assert bidResponse.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + + and: "PBS shouldn't log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidResponse, message)) + assert !floorsLogs.size() + + and: "PBS request status should be in progress" + assert getRequests(bidResponse)[GENERIC]?.first?.ext?.prebid?.floors?.fetchStatus == INPROGRESS + def bidderRequest = bidder.getBidderRequests(bidRequest.id) + assert bidderRequest.ext.prebid.floors.fetchStatus == [INPROGRESS] + + and: "Alerts.general metrics shouldn't be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert !metrics[ALERT_GENERAL] + } + + def "PBS shouldn't emit error in log and response when data is invalid and floors status is in progress"() { + given: "Default BidRequest with empty floors.data" def bidRequest = bidRequestWithFloors.tap { ext.prebid.floors.data = null } + and: "PBS fetch rules from floors provider" + cacheFloorsProviderRules(bidRequest, floorsPbsService) + + and: "Account with invalid floors config" + def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { + config.auction.priceFloors.enabled = true + } + accountDao.save(account) + + and: "Start time" + def startTime = Instant.now() + + and: "Flush metrics" + flushMetrics(floorsPbsService) + when: "PBS processes auction request" def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) - then: "Response should includer error errors" - assert bidResponse.ext?.errors[PREBID]*.code == [999] - assert bidResponse.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: Price floor rules data must be present"] + then: "PBS should log a warning" + def message = 'Price floor rules data must be present' + assert bidResponse.ext?.warnings[PREBID]*.code == [999] + assert bidResponse.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + + and: "PBS should log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) + assert floorsLogs.size() == 1 + assert floorsLogs[0].contains(message) + + and: "PBS request status shouldn't be in progress" + def bidderRequest = bidder.getBidderRequests(bidRequest.id) + assert bidderRequest.first.ext.prebid.floors.fetchStatus == NONE - and: "PBS should process bidRequest" - assert bidder.getBidderRequests(bidRequest.id).size() == 1 + and: "Alerts.general metrics shouldn't be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert !metrics[ALERT_GENERAL] } def "PBS should emit error in log and response when floors skipRate is out of range"() { @@ -854,16 +986,26 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { ext.prebid.floors.data.skipRate = requestSkipRate } + and: "Flush metrics" + flushMetrics(floorsPbsService) + when: "PBS processes auction request" def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) - then: "Response should include error" - assert bidResponse.ext?.errors[PREBID]*.code == [999] - assert bidResponse.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 $requestSkipRate"] + then: "PBS should log a warning" + def message = "Price floor data skipRate must be in range(0-100), but was $requestSkipRate" + assert bidResponse.ext?.warnings[PREBID]*.code == [999] + assert bidResponse.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] - and: "PBS should process bidRequest" - assert bidder.getBidderRequests(bidRequest.id).size() == 1 + and: "PBS should log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) + assert floorsLogs.size() == 1 + assert floorsLogs[0].contains(message) + + and: "Alerts.general metrics shouldn't be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert !metrics[ALERT_GENERAL] where: requestSkipRate << [PBSUtils.randomNegativeNumber, PBSUtils.getRandomNumber(100)] @@ -875,17 +1017,26 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { ext.prebid.floors.data.modelGroups = requestModelGroups } + and: "Flush metrics" + flushMetrics(floorsPbsService) + when: "PBS processes auction request" def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) - then: "Response should include error" - assert bidResponse.ext?.errors[PREBID]*.code == [999] - assert bidResponse.ext?.errors[PREBID]*.message == [ - "Failed to parse price floors from request, with a reason: Price floor rules should contain at least one model group" - ] + then: "PBS should log a warning" + def message = "Price floor rules should contain at least one model group" + assert bidResponse.ext?.warnings[PREBID]*.code == [999] + assert bidResponse.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] - and: "PBS should process bidRequest" - assert bidder.getBidderRequests(bidRequest.id).size() == 1 + and: "PBS should log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) + assert floorsLogs.size() == 1 + assert floorsLogs[0].contains(message) + + and: "Alerts.general metrics shouldn't be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert !metrics[ALERT_GENERAL] where: requestModelGroups << [null, []] @@ -899,17 +1050,26 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { ] } + and: "Flush metrics" + flushMetrics(floorsPbsService) + when: "PBS processes auction request" def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) - then: "Response should include error" - assert bidResponse.ext?.errors[PREBID]*.code == [999] - assert bidResponse.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 $requestModelWeight"] + then: "PBS should log a warning" + def message = "Price floor modelGroup modelWeight must be in range(1-100), but was $requestModelWeight" + assert bidResponse.ext?.warnings[PREBID]*.code == [999] + assert bidResponse.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + + and: "PBS should log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) + assert floorsLogs.size() == 1 + assert floorsLogs[0].contains(message) - and: "PBS should process bidRequest" - assert bidder.getBidderRequests(bidRequest.id).size() == 1 + and: "Alerts.general metrics shouldn't be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert !metrics[ALERT_GENERAL] where: requestModelWeight << [PBSUtils.randomNegativeNumber, PBSUtils.getRandomNumber(100)] @@ -917,29 +1077,38 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def "PBS should emit error in log and response when modelGroup skipRate is out of range"() { given: "BidRequest with invalid modelGroup skipRate" - def requestModelGroupsSkipRate = PBSUtils.getRandomNumber(100) + def requestModelGroupsSkipRate = PBSUtils.getRandomNumber(100) def bidRequest = bidRequestWithFloors.tap { ext.prebid.floors.data.modelGroups = [ new ModelGroup(skipRate: requestModelGroupsSkipRate) ] } + and: "Flush metrics" + flushMetrics(floorsPbsService) + when: "PBS processes auction request" def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) - then: "Response should include error" - assert bidResponse.ext?.errors[PREBID]*.code == [999] - assert bidResponse.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 $requestModelGroupsSkipRate"] + then: "PBS should log a warning" + def message = "Price floor modelGroup skipRate must be in range(0-100), but was $requestModelGroupsSkipRate" + assert bidResponse.ext?.warnings[PREBID]*.code == [999] + assert bidResponse.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] - and: "PBS should process bidRequest" - assert bidder.getBidderRequests(bidRequest.id).size() == 1 + and: "PBS should log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) + assert floorsLogs.size() == 1 + assert floorsLogs[0].contains(message) + + and: "Alerts.general metrics shouldn't be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert !metrics[ALERT_GENERAL] } def "PBS should emit error in log and response when modelGroup defaultFloor is negative"() { given: "BidRequest with negative defaultFloor" - def requestModelGroupsSkipRate = PBSUtils.randomNegativeNumber + def requestModelGroupsSkipRate = PBSUtils.randomNegativeNumber def bidRequest = bidRequestWithFloors.tap { ext.prebid.floors.data.modelGroups = [ @@ -947,18 +1116,26 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { ] } + and: "Flush metrics" + flushMetrics(floorsPbsService) + when: "PBS processes auction request" def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) - then: "Response should include error" - assert bidResponse.ext?.errors[PREBID]*.code == [999] - assert bidResponse.ext?.errors[PREBID]*.message == [ - "Failed to parse price floors from request, with a reason: Price floor modelGroup default must be " + - "positive float, but was $requestModelGroupsSkipRate" - ] + then: "PBS should log a warning" + def message = "Price floor modelGroup default must be positive float, but was $requestModelGroupsSkipRate" + assert bidResponse.ext?.warnings[PREBID]*.code == [999] + assert bidResponse.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + + and: "PBS should log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) + assert floorsLogs.size() == 1 + assert floorsLogs[0].contains(message) - and: "PBS should process bidRequest" - assert bidder.getBidderRequests(bidRequest.id).size() == 1 + and: "Alerts.general metrics shouldn't be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert !metrics[ALERT_GENERAL] } def "PBS should use default floors config when original account config is invalid"() { @@ -966,7 +1143,9 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def startTime = Instant.now() and: "Bid request with floors" - def bidRequest = bidRequestWithFloors + def bidRequest = bidRequestWithFloors.tap { + ext.prebid.floors = null + } and: "Account with invalid floors config" def accountConfig = new AccountConfig(auction: new AccountAuctionConfig(priceFloors: new AccountPriceFloorsConfig(enabled: false))) @@ -977,15 +1156,64 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) bidder.setResponse(bidRequest.id, bidResponse) + and: "Flush metrics" + flushMetrics(floorsPbsService) + when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) and: "PBS floors validation failure should not reject the entire auction" assert !response.seatbid?.isEmpty() - response.seatbid - then: "PBS shouldn't log a errors" + then: "PBS shouldn't log warning or errors" + assert !response.ext?.warnings assert !response.ext?.errors + + and: "Alerts.general metrics shouldn't be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert !metrics[ALERT_GENERAL] + } + + def "PBS should emit error in log and response when account have disabled dynamic data config"() { + given: "Test start time" + def startTime = Instant.now() + + and: "Bid request with floors" + def bidRequest = bidRequestWithFloors.tap { + ext.prebid.floors = null + } + + and: "Account with invalid floors config" + def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { + config.auction.priceFloors.enabled = true + config.auction.priceFloors.useDynamicData = false + } + accountDao.save(account) + + and: "Set bidder response" + def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) + bidder.setResponse(bidRequest.id, bidResponse) + + and: "Flush metrics" + flushMetrics(floorsPbsService) + + when: "PBS processes auction request" + def response = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "PBS should log a warning" + def message = "Using dynamic data is not allowed for account ${account.uuid}".toString() + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == [message] + + and: "PBS should log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, message) + assert floorsLogs.size() == 1 + assert floorsLogs[0].contains(message) + + and: "Alerts.general metrics should be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert metrics[ALERT_GENERAL] == 1 } private static int getSchemaSize(BidRequest bidRequest) { diff --git a/src/test/groovy/org/prebid/server/functional/tests/privacy/GdprAmpSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/privacy/GdprAmpSpec.groovy index 771fac9bd84..717c9f32d5b 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/privacy/GdprAmpSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/privacy/GdprAmpSpec.groovy @@ -394,7 +394,7 @@ class GdprAmpSpec extends PrivacyBaseSpec { and: "Alerts.general metrics should be populated" def metrics = privacyPbsService.sendCollectedMetricsRequest() - assert metrics["alerts.general"] == 1 + assert metrics[ALERT_GENERAL] == 1 and: "Bidder should be called" assert bidder.getBidderRequest(ampStoredRequest.id) diff --git a/src/test/groovy/org/prebid/server/functional/tests/privacy/GdprAuctionSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/privacy/GdprAuctionSpec.groovy index 7302ad79aed..5fc0f5d7bca 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/privacy/GdprAuctionSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/privacy/GdprAuctionSpec.groovy @@ -342,7 +342,7 @@ class GdprAuctionSpec extends PrivacyBaseSpec { and: "Alerts.general metrics should be populated" def metrics = privacyPbsService.sendCollectedMetricsRequest() - assert metrics["alerts.general"] == 1 + assert metrics[ALERT_GENERAL] == 1 and: "Bid response should contain seatBid" assert response.seatbid.size() == 1 From 052a442315fb6104e87298b5ffed2d222389836f Mon Sep 17 00:00:00 2001 From: osulzhenko Date: Mon, 28 Apr 2025 11:51:01 +0300 Subject: [PATCH 4/8] update tests --- .../service/PrebidServerService.groovy | 2 +- .../PriceFloorsSignalingSpec.groovy | 26 +++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/test/groovy/org/prebid/server/functional/service/PrebidServerService.groovy b/src/test/groovy/org/prebid/server/functional/service/PrebidServerService.groovy index 2dd3106d690..31df1efc8d5 100644 --- a/src/test/groovy/org/prebid/server/functional/service/PrebidServerService.groovy +++ b/src/test/groovy/org/prebid/server/functional/service/PrebidServerService.groovy @@ -77,7 +77,7 @@ class PrebidServerService implements ObjectMapperWrapper { prometheusRequestSpecification = buildAndGetRequestSpecification(pbsContainer.prometheusRootUri, authenticationScheme) } - BidResponse sendAuctionRequest(BidRequest bidRequest, Map headers = [:]) { + BidResponse sendAuctionRequest(BidRequest bidRequest, Map headers = [:]) { def response = postAuction(bidRequest, headers) checkResponseStatusCode(response) 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 d862f53c2af..054ec034dc5 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 @@ -723,20 +723,22 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { bidder.setResponse(bidRequest.id, bidResponse) when: "PBS processes auction request" - floorsPbsService.sendAuctionRequest(bidRequest) + def response = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "Response should includer error warning" + def message = "Price floor schema dimensions ${getSchemaSize(bidRequest)} exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] - then: "PBS shouldn't log a errors" + and: "PBS shouldn't log a errors" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidResponse, message)) 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 ${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() assert metrics[INVALID_CONFIG_METRIC(bidRequest.accountId) as String] == 1 - assert metrics[ALERT_GENERAL] == 1 + assert !metrics[ALERT_GENERAL] cleanup: "Stop and remove pbs container" pbsServiceFactory.removeContainer(pbsFloorConfig) @@ -936,7 +938,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert !metrics[ALERT_GENERAL] } - def "PBS shouldn't emit error in log and response when data is invalid and floors status is in progress"() { + def "PBS shouldn't emit error in log and response when data is invalid and floors status not in progress"() { given: "Default BidRequest with empty floors.data" def bidRequest = bidRequestWithFloors.tap { ext.prebid.floors.data = null @@ -973,7 +975,8 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { and: "PBS request status shouldn't be in progress" def bidderRequest = bidder.getBidderRequests(bidRequest.id) - assert bidderRequest.first.ext.prebid.floors.fetchStatus == NONE + assert getRequests(bidResponse)[GENERIC]?.first?.ext?.prebid?.floors?.fetchStatus == NONE + assert bidderRequest.ext.prebid.floors.fetchStatus.sort() == [NONE, INPROGRESS].sort() and: "Alerts.general metrics shouldn't be populated" def metrics = floorsPbsService.sendCollectedMetricsRequest() @@ -1139,10 +1142,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { } def "PBS should use default floors config when original account config is invalid"() { - given: "Test start time" - def startTime = Instant.now() - - and: "Bid request with floors" + given: "Bid request with empty floors" def bidRequest = bidRequestWithFloors.tap { ext.prebid.floors = null } From deab6b8fa46bffa5f5ff51626754b6bd2920aaee Mon Sep 17 00:00:00 2001 From: osulzhenko Date: Mon, 28 Apr 2025 11:56:13 +0300 Subject: [PATCH 5/8] update tests --- .../tests/pricefloors/PriceFloorsSignalingSpec.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 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 054ec034dc5..13dd53d6804 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 @@ -900,7 +900,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { null | MAX_RULES_SIZE } - def "PBS should emit error in log and response when data is invalid and floors status is in progress"() { + def "PBS shouldn't emit error in log and response when data is invalid and floors status is in progress"() { given: "Default BidRequest with empty floors.data" def bidRequest = bidRequestWithFloors.tap { ext.prebid.floors.data = null @@ -938,7 +938,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert !metrics[ALERT_GENERAL] } - def "PBS shouldn't emit error in log and response when data is invalid and floors status not in progress"() { + def "PBS should emit error in log and response when data is invalid and floors status not in progress"() { given: "Default BidRequest with empty floors.data" def bidRequest = bidRequestWithFloors.tap { ext.prebid.floors.data = null From 331ef997d588e71d5f74917debb7431081b3c165 Mon Sep 17 00:00:00 2001 From: osulzhenko Date: Wed, 30 Apr 2025 02:35:46 +0300 Subject: [PATCH 6/8] update tests --- .../floors/BasicPriceFloorProcessor.java | 4 +- .../PriceFloorsFetchingSpec.groovy | 173 +++++++++--------- .../pricefloors/PriceFloorsRulesSpec.groovy | 10 +- .../PriceFloorsSignalingSpec.groovy | 152 +++++++-------- 4 files changed, 172 insertions(+), 167 deletions(-) diff --git a/src/main/java/org/prebid/server/floors/BasicPriceFloorProcessor.java b/src/main/java/org/prebid/server/floors/BasicPriceFloorProcessor.java index b0b24b7e097..0b48b2b224d 100644 --- a/src/main/java/org/prebid/server/floors/BasicPriceFloorProcessor.java +++ b/src/main/java/org/prebid/server/floors/BasicPriceFloorProcessor.java @@ -154,7 +154,7 @@ private PriceFloorRules resolveFloors(Account account, BidRequest bidRequest, Li final String fetchErrorMessage = resolveFetchErrorMessage(fetchResult, isUsingDynamicDataAllowed); return requestFloors == null - ? noPriceFloorData(fetchStatus, bidRequest.getId(), account.getId(), fetchErrorMessage, warnings) + ? noPriceFloorData(fetchStatus, account.getId(), bidRequest.getId(), fetchErrorMessage, warnings) : getPriceFloorRules(bidRequest, account, requestFloors, fetchStatus, fetchErrorMessage, warnings); } @@ -210,7 +210,7 @@ private PriceFloorRules getPriceFloorRules(BidRequest bidRequest, ? null : "%s. Following parsing of request price floors is failed: %s" .formatted(fetchErrorMessage, e.getMessage()); - return noPriceFloorData(fetchStatus, bidRequest.getId(), account.getId(), errorMessage, warnings); + return noPriceFloorData(fetchStatus, account.getId(), bidRequest.getId(), errorMessage, warnings); } } 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 d0316172892..572136feeef 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 @@ -1,6 +1,5 @@ package org.prebid.server.functional.tests.pricefloors -import org.prebid.server.functional.model.bidder.BidderName import org.prebid.server.functional.model.config.PriceFloorsFetch import org.prebid.server.functional.model.db.StoredRequest import org.prebid.server.functional.model.pricefloors.ModelGroup @@ -18,6 +17,7 @@ import java.time.Instant import static org.mockserver.model.HttpStatusCode.BAD_REQUEST_400 import static org.prebid.server.functional.model.Currency.EUR import static org.prebid.server.functional.model.Currency.JPY +import static org.prebid.server.functional.model.bidder.BidderName.GENERIC import static org.prebid.server.functional.model.pricefloors.Country.MULTIPLE import static org.prebid.server.functional.model.pricefloors.MediaType.BANNER import static org.prebid.server.functional.model.request.auction.DistributionChannel.APP @@ -46,6 +46,18 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { private static final Closure INVALID_CONFIG_METRIC = { account -> "alerts.account_config.${account}.price-floors" } private static final String FETCH_FAILURE_METRIC = "price-floors.fetch.failure" + private static final Closure URL_EMPTY_ERROR = { url -> "Failed to fetch price floor from provider for fetch.url '${url}'" } + private static final String FETCHING_DISABLED_ERROR = 'Fetching is disabled' + private static final Closure URL_EMPTY_WARNING_MESSAGE = { url, message -> + "${URL_EMPTY_ERROR(url)}, with a reason: $message" + } + private static final Closure URL_EMPTY_ERROR_LOG = { bidRequest, message -> + "No price floor data for account ${bidRequest.accountId} and " + + "request ${bidRequest.id}, reason: ${URL_EMPTY_WARNING_MESSAGE("$BASIC_FETCH_URL$bidRequest.accountId", message)}" + } + private static final Closure WARNING_MESSAGE = { message -> + "$FETCHING_DISABLED_ERROR. Following parsing of request price floors is failed: $message" + } def "PBS should activate floors feature when price-floors.enabled = true in PBS config"() { given: "Pbs with PF configuration" @@ -122,7 +134,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def logs = floorsPbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, bidRequest.accountId) assert floorsLogs.size() == 1 - assert floorsLogs.first().contains("Malformed fetch.url: 'null', passed for account $bidRequest.accountId") + assert floorsLogs.first().contains("No price floor data for account $bidRequest.accountId and request $bidRequest.id, " + + "reason: Malformed fetch.url 'null' passed") and: "PBS floors validation failure should not reject the entire auction" assert !response.seatbid.isEmpty() @@ -546,7 +559,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(bidRequest.accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -560,9 +573,10 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "PBS log should contain error" def logs = floorsPbsService.getLogsByTime(startTime) + def message = "Price floor data useFetchDataRate must be in range(0-100), but was $accounntUseFetchDataRate" def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains("reason : Price floor data useFetchDataRate must be in range(0-100), but was $accounntUseFetchDataRate") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -646,7 +660,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, BAD_REQUEST_400) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -658,12 +672,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Failed to request, provider respond with status 400" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL) + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.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 : Failed to request for " + - "account $accountId, provider respond with status 400") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -689,7 +702,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, invalidJson) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -701,12 +714,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Failed to parse price floor response, cause: DecodeException: Failed to decode" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$accountId") + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.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 : Failed to parse price floor " + - "response for account $accountId, cause: DecodeException: Failed to decode") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -731,7 +743,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, "") and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -743,12 +755,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Failed to parse price floor response, response body can not be empty" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.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 : Failed to parse price floor " + - "response for account $accountId, response body can not be empty" as String) + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -776,7 +787,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -788,12 +799,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Price floor rules should contain at least one model group" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.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 rules should contain " + - "at least one model group " as String) + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -821,7 +831,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -833,12 +843,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Price floor rules values can't be null or empty, but were null" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.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 rules values can't " + - "be null or empty, but were null" as String) + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -869,7 +878,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -881,12 +890,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Price floor rules number 2 exceeded its maximum number $maxRules" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.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 rules number " + - "2 exceeded its maximum number $maxRules") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -918,7 +926,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponseWithTimeout(accountId) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, pbsService, BidderName.GENERIC, NONE) + cacheFloorsProviderRules(bidRequest, pbsService, GENERIC, NONE) when: "PBS processes auction request" def response = pbsService.sendAuctionRequest(bidRequest) @@ -933,8 +941,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def logs = pbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains("Fetch price floor request timeout for fetch.url: '$BASIC_FETCH_URL$accountId', " + - "account $accountId exceeded") + assert floorsLogs[0].contains("No price floor data for account $bidRequest.accountId and request $bidRequest.id, " + + "reason: Fetch price floor request timeout for fetch.url '$BASIC_FETCH_URL$accountId' exceeded") and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -964,7 +972,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse, ["Content-Length": responseSize as String]) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -976,12 +984,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Response size $responseSize exceeded ${convertKilobyteSizeToByte(maxSize)} bytes limit" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.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 : Response size " + - "$responseSize exceeded ${convertKilobyteSizeToByte(maxSize)} bytes limit") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -1307,10 +1314,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp[0].bidFloor == floorValue and: "Response should contain warning" + def message = "Price floor floorMin must be positive float, but was $invalidFloorMin" 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 floorMin " + - "must be positive float, but was $invalidFloorMin"] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] } def "PBS should validate rules from request when request doesn't contain modelGroups"() { @@ -1335,10 +1341,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp[0].bidFloor == floorValue and: "Response should contain warning" + def message = "Price floor rules should contain at least one model group" 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 " + - "should contain at least one model group"] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] } def "PBS should validate rules from request when request doesn't contain values"() { @@ -1363,10 +1368,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp[0].bidFloor == floorValue and: "Response should contain warning" + def message = "Price floor rules values can't be null or empty, but were null" 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 values " + - "can't be null or empty, but were null"] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] } def "PBS should validate rules from request when modelWeight from request is invalid"() { @@ -1395,10 +1399,10 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp[0].bidFloor == floorValue and: "Response should contain warning" + def message = "Price floor modelGroup modelWeight must be in range(1-100), but was $invalidModelWeight" 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 modelGroup modelWeight " + - "must be in range(1-100), but was $invalidModelWeight"] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + where: invalidModelWeight << [0, MAX_MODEL_WEIGHT + 1] } @@ -1434,10 +1438,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp[0].bidFloor == floorValue and: "Response should contain warning" + def message = "Price floor modelGroup modelWeight must be in range(1-100), but was $invalidModelWeight" 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 modelGroup modelWeight " + - "must be in range(1-100), but was $invalidModelWeight"] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] where: invalidModelWeight << [0, MAX_MODEL_WEIGHT + 1] @@ -1474,10 +1477,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp[0].bidFloor == floorValue and: "Response should contain warning" + def message = "Price floor root skipRate must be in range(0-100), but was $invalidSkipRate" 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 root skipRate " + - "must be in range(0-100), but was $invalidSkipRate"] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] where: invalidSkipRate << [SKIP_RATE_MIN - 1, SKIP_RATE_MAX + 1] @@ -1514,10 +1516,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp[0].bidFloor == floorValue and: "Response should contain warning" + def message = "Price floor data skipRate must be in range(0-100), but was $invalidSkipRate" 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 data skipRate " + - "must be in range(0-100), but was $invalidSkipRate"] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] where: invalidSkipRate << [SKIP_RATE_MIN - 1, SKIP_RATE_MAX + 1] @@ -1554,10 +1555,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp[0].bidFloor == floorValue and: "Response should contain warning" + def message = "Price floor modelGroup skipRate must be in range(0-100), but was $invalidSkipRate" 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 modelGroup skipRate " + - "must be in range(0-100), but was $invalidSkipRate"] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] where: invalidSkipRate << [SKIP_RATE_MIN - 1, SKIP_RATE_MAX + 1] @@ -1590,10 +1590,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp[0].bidFloor == floorValue and: "Response should contain warning" + def message = "Price floor modelGroup default must be positive float, but was $invalidDefaultFloorValue" 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 modelGroup default " + - "must be positive float, but was $invalidDefaultFloorValue"] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] } def "PBS should not invalidate previously good fetched data when floors provider return invalid data"() { @@ -1673,7 +1672,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -1690,12 +1689,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Price floor modelGroup modelWeight must be in range(1-100), but was $invalidModelWeight" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$accountId") + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.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 modelGroup modelWeight" + - " must be in range(1-100), but was $invalidModelWeight") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -1732,7 +1730,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -1749,12 +1747,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Price floor data skipRate must be in range(0-100), but was $invalidSkipRate" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$accountId") + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.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 data skipRate" + - " must be in range(0-100), but was $invalidSkipRate") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -1791,7 +1788,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -1808,12 +1805,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Price floor modelGroup skipRate must be in range(0-100), but was $invalidSkipRate" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$accountId") + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.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 modelGroup skipRate" + - " must be in range(0-100), but was $invalidSkipRate") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -1850,7 +1846,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -1867,12 +1863,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Price floor modelGroup default must be positive float, but was $invalidDefaultFloor" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$accountId") + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.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 modelGroup default" + - " must be positive float, but was $invalidDefaultFloor") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() 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 ee861a24587..30acd9b4bdc 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 @@ -61,6 +61,7 @@ import static org.prebid.server.functional.model.request.auction.FetchStatus.ERR import static org.prebid.server.functional.model.request.auction.Location.NO_DATA import static org.prebid.server.functional.model.request.auction.Prebid.Channel import static org.prebid.server.functional.model.response.auction.BidRejectionReason.RESPONSE_REJECTED_DUE_TO_PRICE_FLOOR +import static org.prebid.server.functional.model.response.auction.ErrorType.PREBID import static org.prebid.server.functional.testcontainers.Dependencies.getNetworkServiceContainer class PriceFloorsRulesSpec extends PriceFloorsBaseSpec { @@ -217,10 +218,15 @@ class PriceFloorsRulesSpec extends PriceFloorsBaseSpec { assert bidderRequest.ext?.prebid?.floors?.location == NO_DATA assert bidderRequest.ext?.prebid?.floors?.fetchStatus == ERROR - and: "PBS should not contain errors, warnings" - assert !response.ext?.warnings + and: "PBS should not contain errors" assert !response.ext?.errors + and: "PBS should log a warning" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message.first.contains("Cannot deserialize value of type " + + "`org.prebid.server.floors.model.PriceFloorField` " + + "from String \"bogus\": not one of the values accepted for Enum class") + and: "PBS should not reject the entire auction" assert !response.seatbid.isEmpty() } 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 13dd53d6804..957a2b0be37 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 @@ -35,14 +35,21 @@ import static org.prebid.server.functional.model.pricefloors.PriceFloorField.SIT import static org.prebid.server.functional.model.request.auction.DistributionChannel.APP import static org.prebid.server.functional.model.request.auction.FetchStatus.INPROGRESS import static org.prebid.server.functional.model.request.auction.FetchStatus.NONE +import static org.prebid.server.functional.model.request.auction.FetchStatus.SUCCESS import static org.prebid.server.functional.model.response.auction.ErrorType.PREBID class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { + // Required: Sync no longer logs "in progress"—only "none" or "error" statuses are recorded + private static final String FETCHING_DISABLED_ERROR = 'Fetching is disabled' private static final Closure INVALID_CONFIG_METRIC = { account -> "alerts.account_config.${account}.price-floors" } - private static final Closure WARNING_MESSAGE = { message -> "Failed to parse price floors from request, with a reason: $message" } - private static final Closure ERROR_LOG = { bidRequest, message -> "Failed to parse price floors from request with id: " + - "'${bidRequest.id}', with a reason: $message" } + private static final Closure WARNING_MESSAGE = { message -> + "$FETCHING_DISABLED_ERROR. Following parsing of request price floors is failed: $message" + } + private static final Closure ERROR_LOG = { bidRequest, message -> + "No price floor data for account ${bidRequest.accountId} and " + + "request ${bidRequest.id}, reason: ${WARNING_MESSAGE(message)}" + } private static final int MAX_SCHEMA_DIMENSIONS_SIZE = 1 private static final int MAX_RULES_SIZE = 1 private static Instant startTime @@ -556,6 +563,10 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { then: "PBS should not add warning or errors" assert !response.ext.warnings assert !response.ext.errors + + and: "Alerts.general metrics shouldn't be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert !metrics[ALERT_GENERAL] } def "PBS should emit warning when request has more rules than price-floor.max-rules"() { @@ -570,6 +581,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { and: "Account with maxRules in the DB" def accountId = bidRequest.site.publisher.id def account = getAccountWithEnabledFetch(accountId).tap { + config.auction.priceFloors.fetch.enabled = false config.auction.priceFloors.maxRules = maxRules config.auction.priceFloors.maxRulesSnakeCase = maxRulesSnakeCase } @@ -587,14 +599,13 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def response = floorsPbsService.sendAuctionRequest(bidRequest) then: "PBS should log a warning" + def message = "Price floor rules number ${getRuleSize(bidRequest)} exceeded its maximum number ${MAX_RULES_SIZE}" 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 ${getRuleSize(bidRequest)} exceeded its maximum number ${MAX_RULES_SIZE}"] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] and: "Alerts.general metrics should be populated" def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert !metrics[ALERT_GENERAL] + assert metrics[ALERT_GENERAL] == 1 where: maxRules | maxRulesSnakeCase @@ -609,11 +620,15 @@ 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.fetch.enabled = false config.auction.priceFloors.maxSchemaDims = maxSchemaDims config.auction.priceFloors.maxSchemaDimsSnakeCase = maxSchemaDimsSnakeCase } accountDao.save(account) + and: "PBS fetch rules from floors provider" + cacheFloorsProviderRules(bidRequest, floorsPbsService) + and: "Set bidder response" def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) bidder.setResponse(bidRequest.id, bidResponse) @@ -625,14 +640,13 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def response = floorsPbsService.sendAuctionRequest(bidRequest) then: "PBS should log a warning" + def message = "Price floor schema dimensions ${getSchemaSize(bidRequest)} exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}" 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 ${getSchemaSize(bidRequest)} exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}"] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] and: "Alerts.general metrics should be populated" def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert !metrics[ALERT_GENERAL] + assert metrics[ALERT_GENERAL] == 1 where: maxSchemaDims | maxSchemaDimsSnakeCase @@ -657,6 +671,7 @@ 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 = PBSUtils.randomNegativeNumber } accountDao.save(account) @@ -680,12 +695,11 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def logs = floorsPbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(message) - and: "Metric alerts.account_config.ACCOUNT.price-floors should be update" + and: "Metrics should be updated" def metrics = floorsPbsService.sendCollectedMetricsRequest() assert metrics[INVALID_CONFIG_METRIC(bidRequest.accountId) as String] == 1 - assert !metrics[ALERT_GENERAL] + assert metrics[ALERT_GENERAL] == 1 cleanup: "Stop and remove pbs container" pbsServiceFactory.removeContainer(pbsFloorConfig) @@ -697,7 +711,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { and: "Floor config with default account" def accountConfig = getDefaultAccountConfigSettings().tap { - auction.priceFloors.fetch.enabled = true + auction.priceFloors.fetch.enabled = false auction.priceFloors.fetch.url = BASIC_FETCH_URL + bidRequest.site.publisher.id auction.priceFloors.fetch.maxSchemaDims = MAX_SCHEMA_DIMENSIONS_SIZE auction.priceFloors.maxSchemaDims = MAX_SCHEMA_DIMENSIONS_SIZE @@ -732,13 +746,13 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { and: "PBS shouldn't log a errors" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, ERROR_LOG(bidResponse, message)) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) assert floorsLogs.size() == 1 - and: "Metric alerts.account_config.ACCOUNT.price-floors should be update" + and: "Metrics should be updated" def metrics = floorsPbsService.sendCollectedMetricsRequest() assert metrics[INVALID_CONFIG_METRIC(bidRequest.accountId) as String] == 1 - assert !metrics[ALERT_GENERAL] + assert metrics[ALERT_GENERAL] == 1 cleanup: "Stop and remove pbs container" pbsServiceFactory.removeContainer(pbsFloorConfig) @@ -750,6 +764,7 @@ 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 } @@ -770,11 +785,10 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def logs = floorsPbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(message) - and: "Alerts.general metrics shouldn't be populated" + and: "Alerts.general metrics should be populated" def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert !metrics[ALERT_GENERAL] + assert metrics[ALERT_GENERAL] == 1 where: maxSchemaDims | maxSchemaDimsSnakeCase @@ -790,6 +804,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def accountId = bidRequest.site.publisher.id def floorSchemaFilesSize = getSchemaSize(bidRequest) def account = getAccountWithEnabledFetch(accountId).tap { + config.auction.priceFloors.fetch.enabled = false config.auction.priceFloors.maxSchemaDims = MAX_SCHEMA_DIMENSIONS_SIZE config.auction.priceFloors.fetch.maxSchemaDims = floorSchemaFilesSize } @@ -815,11 +830,10 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def logs = floorsPbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(message) - and: "Alerts.general metrics shouldn't be populated" + and: "Alerts.general metrics should be populated" def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert !metrics[ALERT_GENERAL] + assert metrics[ALERT_GENERAL] == 1 } def "PBS shouldn't fail with error and maxSchemaDims take precede over fetch.maxSchemaDims when requested both"() { @@ -845,6 +859,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert !response.ext?.warnings } +// @IgnoreRest 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 @@ -862,6 +877,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { and: "Account with maxRules in the DB" def account = getAccountWithEnabledFetch(ampRequest.account as String).tap { + config.auction.priceFloors.fetch.enabled = false config.auction.priceFloors.maxRules = maxRules config.auction.priceFloors.maxRulesSnakeCase = maxRulesSnakeCase } @@ -886,13 +902,13 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { and: "PBS should log a errors" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, ERROR_LOG(ampStoredRequest, message)) + def floorsLogs = getLogsByText(logs, "No price floor data for account $ampRequest.account and " + + "request $ampStoredRequest.id, reason: ${WARNING_MESSAGE(message)}") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(message) - and: "Alerts.general metrics shouldn't be populated" + and: "Alerts.general metrics should be populated" def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert !metrics[ALERT_GENERAL] + assert metrics[ALERT_GENERAL] == 1 where: maxRules | maxRulesSnakeCase @@ -908,7 +924,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { and: "Account with invalid floors config" def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { - config.auction.priceFloors.enabled = true + config.auction.priceFloors.fetch.enabled = true } accountDao.save(account) @@ -918,18 +934,19 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { when: "PBS processes auction request" def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) - then: "Response should includer error warning" - def message = 'Price floor rules data must be present' - assert bidResponse.ext?.warnings[PREBID]*.code == [999] - assert bidResponse.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + then: "Response shouldn't includer error warning" + assert !bidResponse.ext?.warnings and: "PBS shouldn't log a errors" + def message = 'Price floor rules data must be present' def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, ERROR_LOG(bidResponse, message)) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) assert !floorsLogs.size() - and: "PBS request status should be in progress" - assert getRequests(bidResponse)[GENERIC]?.first?.ext?.prebid?.floors?.fetchStatus == INPROGRESS + and: "PBS request on response object status should be in progress" + assert getRequests(bidResponse)[GENERIC.value]?.first?.ext?.prebid?.floors?.fetchStatus == INPROGRESS + + and: "PBS bidderRequest status should be in progress" def bidderRequest = bidder.getBidderRequests(bidRequest.id) assert bidderRequest.ext.prebid.floors.fetchStatus == [INPROGRESS] @@ -944,21 +961,15 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { ext.prebid.floors.data = null } - and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService) + and: "Flush metrics" + flushMetrics(floorsPbsService) and: "Account with invalid floors config" def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { - config.auction.priceFloors.enabled = true + config.auction.priceFloors.fetch.enabled = false } accountDao.save(account) - and: "Start time" - def startTime = Instant.now() - - and: "Flush metrics" - flushMetrics(floorsPbsService) - when: "PBS processes auction request" def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) @@ -971,16 +982,15 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def logs = floorsPbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(message) and: "PBS request status shouldn't be in progress" def bidderRequest = bidder.getBidderRequests(bidRequest.id) - assert getRequests(bidResponse)[GENERIC]?.first?.ext?.prebid?.floors?.fetchStatus == NONE - assert bidderRequest.ext.prebid.floors.fetchStatus.sort() == [NONE, INPROGRESS].sort() + assert getRequests(bidResponse)[GENERIC.value]?.first?.ext?.prebid?.floors?.fetchStatus == NONE + assert bidderRequest.ext.prebid.floors.fetchStatus == [NONE] - and: "Alerts.general metrics shouldn't be populated" + and: "Alerts.general metrics should be populated" def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert !metrics[ALERT_GENERAL] + assert metrics[ALERT_GENERAL] == 1 } def "PBS should emit error in log and response when floors skipRate is out of range"() { @@ -1004,11 +1014,10 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def logs = floorsPbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(message) - and: "Alerts.general metrics shouldn't be populated" + and: "Alerts.general metrics should be populated" def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert !metrics[ALERT_GENERAL] + assert metrics[ALERT_GENERAL] == 1 where: requestSkipRate << [PBSUtils.randomNegativeNumber, PBSUtils.getRandomNumber(100)] @@ -1035,11 +1044,10 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def logs = floorsPbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(message) - and: "Alerts.general metrics shouldn't be populated" + and: "Alerts.general metrics should be populated" def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert !metrics[ALERT_GENERAL] + assert metrics[ALERT_GENERAL] == 1 where: requestModelGroups << [null, []] @@ -1068,11 +1076,10 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def logs = floorsPbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(message) - and: "Alerts.general metrics shouldn't be populated" + and: "Alerts.general metrics should be populated" def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert !metrics[ALERT_GENERAL] + assert metrics[ALERT_GENERAL] == 1 where: requestModelWeight << [PBSUtils.randomNegativeNumber, PBSUtils.getRandomNumber(100)] @@ -1098,15 +1105,14 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert bidResponse.ext?.warnings[PREBID]*.code == [999] assert bidResponse.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] - and: "PBS should log a errors" + and: "PBS should log an errors" def logs = floorsPbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(message) - and: "Alerts.general metrics shouldn't be populated" + and: "Alerts.general metrics should be populated" def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert !metrics[ALERT_GENERAL] + assert metrics[ALERT_GENERAL] == 1 } def "PBS should emit error in log and response when modelGroup defaultFloor is negative"() { @@ -1134,11 +1140,10 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def logs = floorsPbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(message) - and: "Alerts.general metrics shouldn't be populated" + and: "Alerts.general metrics should be populated" def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert !metrics[ALERT_GENERAL] + assert metrics[ALERT_GENERAL] == 1 } def "PBS should use default floors config when original account config is invalid"() { @@ -1175,17 +1180,13 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { } def "PBS should emit error in log and response when account have disabled dynamic data config"() { - given: "Test start time" - def startTime = Instant.now() - - and: "Bid request with floors" + given: "Bid request without floors" def bidRequest = bidRequestWithFloors.tap { ext.prebid.floors = null } and: "Account with invalid floors config" def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { - config.auction.priceFloors.enabled = true config.auction.priceFloors.useDynamicData = false } accountDao.save(account) @@ -1194,6 +1195,9 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) bidder.setResponse(bidRequest.id, bidResponse) + and: "PBS fetch rules from floors provider" + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, SUCCESS) + and: "Flush metrics" flushMetrics(floorsPbsService) @@ -1201,15 +1205,15 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def response = floorsPbsService.sendAuctionRequest(bidRequest) then: "PBS should log a warning" - def message = "Using dynamic data is not allowed for account ${account.uuid}".toString() + def message = "Using dynamic data is not allowed" assert response.ext?.warnings[PREBID]*.code == [999] assert response.ext?.warnings[PREBID]*.message == [message] and: "PBS should log a errors" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, message) + def floorsLogs = getLogsByText(logs, "No price floor data for account ${bidRequest.accountId} " + + "and request ${bidRequest.id}, reason: Using dynamic data is not allowed") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(message) and: "Alerts.general metrics should be populated" def metrics = floorsPbsService.sendCollectedMetricsRequest() From 192582471348cd9db727dc05aab60104cdd83cbf Mon Sep 17 00:00:00 2001 From: osulzhenko Date: Wed, 30 Apr 2025 13:35:17 +0300 Subject: [PATCH 7/8] update after review --- .../PriceFloorsFetchingSpec.groovy | 29 +++++++++---------- .../PriceFloorsSignalingSpec.groovy | 20 ++++++------- 2 files changed, 22 insertions(+), 27 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 572136feeef..699b9bdcda0 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 @@ -44,10 +44,14 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { private static final int DEFAULT_FLOOR_VALUE_MIN = 0 private static final int FLOOR_MIN = 0 - private static final Closure INVALID_CONFIG_METRIC = { account -> "alerts.account_config.${account}.price-floors" } private static final String FETCH_FAILURE_METRIC = "price-floors.fetch.failure" - private static final Closure URL_EMPTY_ERROR = { url -> "Failed to fetch price floor from provider for fetch.url '${url}'" } private static final String FETCHING_DISABLED_ERROR = 'Fetching is disabled' + private static final String PRICE_FLOOR_VALUES_MISSING = 'Price floor rules values can\'t be null or empty, but were null' + private static final String MODEL_WEIGHT_INVALID = "Price floor modelGroup modelWeight must be in range(1-100), but was %s" + private static final String SKIP_RATE_INVALID = "Price floor modelGroup skipRate must be in range(0-100), but was %s" + + private static final Closure INVALID_CONFIG_METRIC = { account -> "alerts.account_config.${account}.price-floors" } + private static final Closure URL_EMPTY_ERROR = { url -> "Failed to fetch price floor from provider for fetch.url '${url}'" } private static final Closure URL_EMPTY_WARNING_MESSAGE = { url, message -> "${URL_EMPTY_ERROR(url)}, with a reason: $message" } @@ -843,11 +847,10 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" - def message = "Price floor rules values can't be null or empty, but were null" def logs = floorsPbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, PRICE_FLOOR_VALUES_MISSING)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -1368,9 +1371,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp[0].bidFloor == floorValue and: "Response should contain warning" - def message = "Price floor rules values can't be null or empty, but were null" assert response.ext?.warnings[PREBID]*.code == [999] - assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(PRICE_FLOOR_VALUES_MISSING)] } def "PBS should validate rules from request when modelWeight from request is invalid"() { @@ -1399,9 +1401,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp[0].bidFloor == floorValue and: "Response should contain warning" - def message = "Price floor modelGroup modelWeight must be in range(1-100), but was $invalidModelWeight" assert response.ext?.warnings[PREBID]*.code == [999] - assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(MODEL_WEIGHT_INVALID.formatted(invalidModelWeight))] where: invalidModelWeight << [0, MAX_MODEL_WEIGHT + 1] @@ -1438,9 +1439,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp[0].bidFloor == floorValue and: "Response should contain warning" - def message = "Price floor modelGroup modelWeight must be in range(1-100), but was $invalidModelWeight" assert response.ext?.warnings[PREBID]*.code == [999] - assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(MODEL_WEIGHT_INVALID.formatted(invalidModelWeight))] where: invalidModelWeight << [0, MAX_MODEL_WEIGHT + 1] @@ -1555,9 +1555,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp[0].bidFloor == floorValue and: "Response should contain warning" - def message = "Price floor modelGroup skipRate must be in range(0-100), but was $invalidSkipRate" assert response.ext?.warnings[PREBID]*.code == [999] - assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(SKIP_RATE_INVALID.formatted(invalidSkipRate))] where: invalidSkipRate << [SKIP_RATE_MIN - 1, SKIP_RATE_MAX + 1] @@ -1689,11 +1688,10 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" - def message = "Price floor modelGroup modelWeight must be in range(1-100), but was $invalidModelWeight" def logs = floorsPbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, MODEL_WEIGHT_INVALID.formatted(invalidModelWeight))) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -1805,11 +1803,10 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" - def message = "Price floor modelGroup skipRate must be in range(0-100), but was $invalidSkipRate" def logs = floorsPbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, SKIP_RATE_INVALID.formatted(invalidSkipRate))) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() 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 957a2b0be37..141bdfb8a1f 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 @@ -578,7 +578,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { (new Rule(mediaType: BANNER, country: Country.MULTIPLE).rule): requestFloorValue] } - and: "Account with maxRules in the DB" + and: "Account with maxRules and disabled fetching in the DB" def accountId = bidRequest.site.publisher.id def account = getAccountWithEnabledFetch(accountId).tap { config.auction.priceFloors.fetch.enabled = false @@ -617,7 +617,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { given: "BidRequest with schema 2 fields" def bidRequest = bidRequestWithFloors - and: "Account with maxSchemaDims in the DB" + and: "Account with maxSchemaDims and disabled fetching in the DB" def accountId = bidRequest.site.publisher.id def account = getAccountWithEnabledFetch(accountId).tap { config.auction.priceFloors.fetch.enabled = false @@ -800,7 +800,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { given: "BidRequest with schema 2 fields" def bidRequest = bidRequestWithFloors - and: "Account with maxSchemaDims in the DB" + and: "Account with maxSchemaDims and disabled fetching in the DB" def accountId = bidRequest.site.publisher.id def floorSchemaFilesSize = getSchemaSize(bidRequest) def account = getAccountWithEnabledFetch(accountId).tap { @@ -859,7 +859,6 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert !response.ext?.warnings } -// @IgnoreRest 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 @@ -875,7 +874,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def storedRequest = StoredRequest.getStoredRequest(ampRequest, ampStoredRequest) storedRequestDao.save(storedRequest) - and: "Account with maxRules in the DB" + and: "Account with maxRules and disabled fetching in the DB" def account = getAccountWithEnabledFetch(ampRequest.account as String).tap { config.auction.priceFloors.fetch.enabled = false config.auction.priceFloors.maxRules = maxRules @@ -922,7 +921,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { ext.prebid.floors.data = null } - and: "Account with invalid floors config" + and: "Account with enabled fetching" def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { config.auction.priceFloors.fetch.enabled = true } @@ -964,7 +963,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { and: "Flush metrics" flushMetrics(floorsPbsService) - and: "Account with invalid floors config" + and: "Account with disabled fetching" def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { config.auction.priceFloors.fetch.enabled = false } @@ -1152,7 +1151,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { ext.prebid.floors = null } - and: "Account with invalid floors config" + and: "Account with disabled price floors" def accountConfig = new AccountConfig(auction: new AccountAuctionConfig(priceFloors: new AccountPriceFloorsConfig(enabled: false))) def account = new Account(uuid: bidRequest.accountId, config: accountConfig) accountDao.save(account) @@ -1185,7 +1184,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { ext.prebid.floors = null } - and: "Account with invalid floors config" + and: "Account with disabled dynamic data" def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { config.auction.priceFloors.useDynamicData = false } @@ -1205,9 +1204,8 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def response = floorsPbsService.sendAuctionRequest(bidRequest) then: "PBS should log a warning" - def message = "Using dynamic data is not allowed" assert response.ext?.warnings[PREBID]*.code == [999] - assert response.ext?.warnings[PREBID]*.message == [message] + assert response.ext?.warnings[PREBID]*.message == ['Using dynamic data is not allowed'] and: "PBS should log a errors" def logs = floorsPbsService.getLogsByTime(startTime) From 03ce834c52cc92535b18f6f9b0578b54fc70cafb Mon Sep 17 00:00:00 2001 From: osulzhenko Date: Wed, 30 Apr 2025 15:02:14 +0300 Subject: [PATCH 8/8] update after review --- .../PriceFloorsSignalingSpec.groovy | 49 +++---------------- 1 file changed, 7 insertions(+), 42 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 141bdfb8a1f..1de08b44cc9 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 @@ -553,10 +553,6 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { 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) @@ -629,10 +625,6 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { and: "PBS fetch rules from floors provider" cacheFloorsProviderRules(bidRequest, floorsPbsService) - and: "Set bidder response" - def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) - bidder.setResponse(bidRequest.id, bidResponse) - and: "Flush metrics" flushMetrics(floorsPbsService) @@ -676,10 +668,6 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { } accountDao.save(account) - and: "Set bidder response" - def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) - bidder.setResponse(bidRequest.id, bidResponse) - and: "Flush metrics" flushMetrics(floorsPbsService) @@ -732,10 +720,6 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { } 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) @@ -810,10 +794,6 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { } accountDao.save(account) - and: "Set bidder response" - def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) - bidder.setResponse(bidRequest.id, bidResponse) - and: "Flush metrics" flushMetrics(floorsPbsService) @@ -848,10 +828,6 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { } 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) @@ -922,9 +898,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { } and: "Account with enabled fetching" - def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { - config.auction.priceFloors.fetch.enabled = true - } + def account = getAccountWithEnabledFetch(bidRequest.accountId) accountDao.save(account) and: "Flush metrics" @@ -982,11 +956,14 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) assert floorsLogs.size() == 1 + and: "PBS request on response object status should be none" + assert getRequests(bidResponse)[GENERIC.value]?.first?.ext?.prebid?.floors?.fetchStatus == NONE + and: "PBS request status shouldn't be in progress" def bidderRequest = bidder.getBidderRequests(bidRequest.id) - assert getRequests(bidResponse)[GENERIC.value]?.first?.ext?.prebid?.floors?.fetchStatus == NONE assert bidderRequest.ext.prebid.floors.fetchStatus == [NONE] + and: "Alerts.general metrics should be populated" def metrics = floorsPbsService.sendCollectedMetricsRequest() assert metrics[ALERT_GENERAL] == 1 @@ -1147,19 +1124,13 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def "PBS should use default floors config when original account config is invalid"() { given: "Bid request with empty floors" - def bidRequest = bidRequestWithFloors.tap { - ext.prebid.floors = null - } + def bidRequest = BidRequest.defaultBidRequest and: "Account with disabled price floors" def accountConfig = new AccountConfig(auction: new AccountAuctionConfig(priceFloors: new AccountPriceFloorsConfig(enabled: false))) def account = new Account(uuid: bidRequest.accountId, config: accountConfig) accountDao.save(account) - and: "Set bidder response" - def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) - bidder.setResponse(bidRequest.id, bidResponse) - and: "Flush metrics" flushMetrics(floorsPbsService) @@ -1180,9 +1151,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def "PBS should emit error in log and response when account have disabled dynamic data config"() { given: "Bid request without floors" - def bidRequest = bidRequestWithFloors.tap { - ext.prebid.floors = null - } + def bidRequest = BidRequest.defaultBidRequest and: "Account with disabled dynamic data" def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { @@ -1190,10 +1159,6 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { } accountDao.save(account) - and: "Set bidder response" - def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) - bidder.setResponse(bidRequest.id, bidResponse) - and: "PBS fetch rules from floors provider" cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, SUCCESS)