diff --git a/src/main/java/org/prebid/server/floors/BasicPriceFloorProcessor.java b/src/main/java/org/prebid/server/floors/BasicPriceFloorProcessor.java index 8619a4b12a9..d492981f568 100644 --- a/src/main/java/org/prebid/server/floors/BasicPriceFloorProcessor.java +++ b/src/main/java/org/prebid/server/floors/BasicPriceFloorProcessor.java @@ -21,8 +21,6 @@ import org.prebid.server.log.ConditionalLogger; import org.prebid.server.log.Logger; import org.prebid.server.log.LoggerFactory; -import org.prebid.server.metric.MetricName; -import org.prebid.server.metric.Metrics; import org.prebid.server.proto.openrtb.ext.request.ExtImpPrebidFloors; import org.prebid.server.proto.openrtb.ext.request.ExtRequest; import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid; @@ -54,23 +52,17 @@ public class BasicPriceFloorProcessor implements PriceFloorProcessor { private final PriceFloorFetcher floorFetcher; private final PriceFloorResolver floorResolver; - private final Metrics metrics; private final JacksonMapper mapper; - private final double logSamplingRate; private final RandomWeightedEntrySupplier modelPicker; public BasicPriceFloorProcessor(PriceFloorFetcher floorFetcher, PriceFloorResolver floorResolver, - Metrics metrics, - JacksonMapper mapper, - double logSamplingRate) { + JacksonMapper mapper) { this.floorFetcher = Objects.requireNonNull(floorFetcher); this.floorResolver = Objects.requireNonNull(floorResolver); - this.metrics = Objects.requireNonNull(metrics); this.mapper = Objects.requireNonNull(mapper); - this.logSamplingRate = logSamplingRate; modelPicker = new RandomPositiveWeightedEntrySupplier<>(BasicPriceFloorProcessor::resolveModelGroupWeight); } @@ -90,7 +82,7 @@ public BidRequest enrichWithPriceFloors(BidRequest bidRequest, return disableFloorsForRequest(bidRequest); } - final PriceFloorRules floors = resolveFloors(account, bidRequest, warnings); + final PriceFloorRules floors = resolveFloors(account, bidRequest, errors); return updateBidRequestWithFloors(bidRequest, bidder, floors, errors, warnings); } @@ -130,13 +122,49 @@ private static PriceFloorRules extractRequestFloors(BidRequest bidRequest) { return ObjectUtil.getIfNotNull(prebid, ExtRequestPrebid::getFloors); } - private PriceFloorRules resolveFloors(Account account, BidRequest bidRequest, List warnings) { + private PriceFloorRules resolveFloors(Account account, BidRequest bidRequest, List errors) { final PriceFloorRules requestFloors = extractRequestFloors(bidRequest); final FetchResult fetchResult = floorFetcher.fetch(account); - final FetchStatus fetchStatus = fetchResult.getFetchStatus(); + final FetchStatus fetchStatus = ObjectUtil.getIfNotNull(fetchResult, FetchResult::getFetchStatus); - final boolean isUsingDynamicDataAllowed = Optional.ofNullable(account.getAuction()) + if (fetchResult != null && fetchStatus == FetchStatus.success && shouldUseDynamicData(account, fetchResult)) { + final PriceFloorRules mergedFloors = mergeFloors(requestFloors, fetchResult.getRulesData()); + return createFloorsFrom(mergedFloors, fetchStatus, PriceFloorLocation.fetch); + } + + if (requestFloors != null) { + try { + final Optional priceFloorsConfig = Optional.of(account) + .map(Account::getAuction) + .map(AccountAuctionConfig::getPriceFloors); + + final Long maxRules = priceFloorsConfig.map(AccountPriceFloorsConfig::getMaxRules) + .orElse(null); + final Long maxDimensions = priceFloorsConfig.map(AccountPriceFloorsConfig::getMaxSchemaDims) + .orElse(null); + + PriceFloorRulesValidator.validateRules( + requestFloors, + PriceFloorsConfigResolver.resolveMaxValue(maxRules), + PriceFloorsConfigResolver.resolveMaxValue(maxDimensions)); + + return createFloorsFrom(requestFloors, fetchStatus, PriceFloorLocation.request); + } catch (PreBidException e) { + errors.add("Failed to parse price floors from request, with a reason: %s".formatted(e.getMessage())); + conditionalLogger.error( + "Failed to parse price floors from request with id: '%s', with a reason: %s" + .formatted(bidRequest.getId(), e.getMessage()), + 0.01d); + } + } + + return createFloorsFrom(null, fetchStatus, PriceFloorLocation.noData); + } + + private static boolean shouldUseDynamicData(Account account, FetchResult fetchResult) { + final boolean isUsingDynamicDataAllowed = Optional.of(account) + .map(Account::getAuction) .map(AccountAuctionConfig::getPriceFloors) .map(AccountPriceFloorsConfig::getUseDynamicData) .map(BooleanUtils::isNotFalse) @@ -147,73 +175,12 @@ private PriceFloorRules resolveFloors(Account account, BidRequest bidRequest, Li .map(rate -> ThreadLocalRandom.current().nextInt(USE_FETCH_DATA_RATE_MAX) < rate) .orElse(true); - if (fetchStatus == FetchStatus.success && isUsingDynamicDataAllowed && shouldUseDynamicData) { - final PriceFloorRules mergedFloors = mergeFloors(requestFloors, fetchResult.getRulesData()); - return createFloorsFrom(mergedFloors, fetchStatus, PriceFloorLocation.fetch); - } - - final String fetchErrorMessage = resolveFetchErrorMessage(fetchResult, isUsingDynamicDataAllowed); - return requestFloors == null - ? noPriceFloorData(fetchStatus, account.getId(), bidRequest.getId(), fetchErrorMessage, warnings) - : getPriceFloorRules(bidRequest, account, requestFloors, fetchStatus, fetchErrorMessage, warnings); - } - - private static String resolveFetchErrorMessage(FetchResult fetchResult, boolean isUsingDynamicDataAllowed) { - return switch (fetchResult.getFetchStatus()) { - case inprogress -> null; - case error, timeout, none -> fetchResult.getErrorMessage(); - case success -> isUsingDynamicDataAllowed ? null : "Using dynamic data is not allowed"; - }; - } - - private PriceFloorRules noPriceFloorData(FetchStatus fetchStatus, - String accountId, - String requestId, - String errorMessage, - List warnings) { - - if (errorMessage != null) { - warnings.add(errorMessage); - conditionalLogger.error("No price floor data for account %s and request %s, reason: %s" - .formatted(accountId, requestId, errorMessage), logSamplingRate); - metrics.updateAlertsMetrics(MetricName.general); - } - - return createFloorsFrom(null, fetchStatus, PriceFloorLocation.noData); + return isUsingDynamicDataAllowed && shouldUseDynamicData; } - private PriceFloorRules getPriceFloorRules(BidRequest bidRequest, - Account account, - PriceFloorRules requestFloors, - FetchStatus fetchStatus, - String fetchErrorMessage, - List warnings) { - - try { - final Optional priceFloorsConfig = Optional.of(account.getAuction()) - .map(AccountAuctionConfig::getPriceFloors); - - final Long maxRules = priceFloorsConfig.map(AccountPriceFloorsConfig::getMaxRules) - .orElse(null); - final Long maxDimensions = priceFloorsConfig.map(AccountPriceFloorsConfig::getMaxSchemaDims) - .orElse(null); - - PriceFloorRulesValidator.validateRules( - requestFloors, - PriceFloorsConfigResolver.resolveMaxValue(maxRules), - PriceFloorsConfigResolver.resolveMaxValue(maxDimensions)); - - return createFloorsFrom(requestFloors, fetchStatus, PriceFloorLocation.request); - } catch (PreBidException e) { - final String errorMessage = fetchErrorMessage == null - ? null - : "%s. Following parsing of request price floors is failed: %s" - .formatted(fetchErrorMessage, e.getMessage()); - return noPriceFloorData(fetchStatus, account.getId(), bidRequest.getId(), errorMessage, warnings); - } - } + private PriceFloorRules mergeFloors(PriceFloorRules requestFloors, + PriceFloorData providerRulesData) { - private PriceFloorRules mergeFloors(PriceFloorRules requestFloors, PriceFloorData providerRulesData) { final Price floorMinPrice = resolveFloorMinPrice(requestFloors); return (requestFloors != null ? requestFloors.toBuilder() : PriceFloorRules.builder()) diff --git a/src/main/java/org/prebid/server/floors/PriceFloorFetcher.java b/src/main/java/org/prebid/server/floors/PriceFloorFetcher.java index 7f49b1c09a7..b7d4ac4185f 100644 --- a/src/main/java/org/prebid/server/floors/PriceFloorFetcher.java +++ b/src/main/java/org/prebid/server/floors/PriceFloorFetcher.java @@ -90,10 +90,7 @@ public FetchResult fetch(Account account) { final AccountFetchContext accountFetchContext = fetchedData.get(account.getId()); return accountFetchContext != null - ? FetchResult.of( - accountFetchContext.getRulesData(), - accountFetchContext.getFetchStatus(), - accountFetchContext.getErrorMessage()) + ? FetchResult.of(accountFetchContext.getRulesData(), accountFetchContext.getFetchStatus()) : fetchPriceFloorData(account); } @@ -102,19 +99,20 @@ private FetchResult fetchPriceFloorData(Account account) { final Boolean fetchEnabled = ObjectUtil.getIfNotNull(fetchConfig, AccountPriceFloorsFetchConfig::getEnabled); if (BooleanUtils.isFalse(fetchEnabled)) { - return FetchResult.none("Fetching is disabled"); + return FetchResult.of(null, FetchStatus.none); } final String accountId = account.getId(); final String fetchUrl = ObjectUtil.getIfNotNull(fetchConfig, AccountPriceFloorsFetchConfig::getUrl); if (!isUrlValid(fetchUrl)) { - return FetchResult.error("Malformed fetch.url '%s' passed".formatted(fetchUrl)); + logger.error("Malformed fetch.url: '%s', passed for account %s".formatted(fetchUrl, accountId)); + return FetchResult.of(null, FetchStatus.error); } if (!fetchInProgress.contains(accountId)) { fetchPriceFloorDataAsynchronous(fetchConfig, accountId); } - return FetchResult.inProgress(); + return FetchResult.of(null, FetchStatus.inprogress); } private boolean isUrlValid(String url) { @@ -150,8 +148,8 @@ private void fetchPriceFloorDataAsynchronous(AccountPriceFloorsFetchConfig fetch fetchInProgress.add(accountId); httpClient.get(fetchUrl, timeout, resolveMaxFileSize(maxFetchFileSizeKb)) - .map(httpClientResponse -> parseFloorResponse(httpClientResponse, fetchConfig)) - .recover(throwable -> recoverFromFailedFetching(throwable, fetchUrl)) + .map(httpClientResponse -> parseFloorResponse(httpClientResponse, fetchConfig, accountId)) + .recover(throwable -> recoverFromFailedFetching(throwable, fetchUrl, accountId)) .map(cacheInfo -> updateCache(cacheInfo, fetchConfig, accountId)) .map(priceFloorData -> createPeriodicTimerForRulesFetch(priceFloorData, fetchConfig, accountId)); } @@ -161,20 +159,23 @@ private static long resolveMaxFileSize(Long maxSizeInKBytes) { } private ResponseCacheInfo parseFloorResponse(HttpClientResponse httpClientResponse, - AccountPriceFloorsFetchConfig fetchConfig) { + AccountPriceFloorsFetchConfig fetchConfig, + String accountId) { final int statusCode = httpClientResponse.getStatusCode(); if (statusCode != HttpStatus.SC_OK) { - throw new PreBidException("Failed to request, provider respond with status %s".formatted(statusCode)); + throw new PreBidException("Failed to request for account %s, provider respond with status %s" + .formatted(accountId, statusCode)); } final String body = httpClientResponse.getBody(); if (StringUtils.isBlank(body)) { - throw new PreBidException("Failed to parse price floor response, response body can not be empty"); + throw new PreBidException( + "Failed to parse price floor response for account %s, response body can not be empty" + .formatted(accountId)); } - final PriceFloorData priceFloorData = parsePriceFloorData(body); - + final PriceFloorData priceFloorData = parsePriceFloorData(body, accountId); PriceFloorRulesValidator.validateRulesData( priceFloorData, PriceFloorsConfigResolver.resolveMaxValue(fetchConfig.getMaxRules()), @@ -182,17 +183,16 @@ private ResponseCacheInfo parseFloorResponse(HttpClientResponse httpClientRespon return ResponseCacheInfo.of(priceFloorData, FetchStatus.success, - null, cacheTtlFromResponse(httpClientResponse, fetchConfig.getUrl())); } - private PriceFloorData parsePriceFloorData(String body) { + private PriceFloorData parsePriceFloorData(String body, String accountId) { final PriceFloorData priceFloorData; try { priceFloorData = mapper.decodeValue(body, PriceFloorData.class); } catch (DecodeException e) { - throw new PreBidException( - "Failed to parse price floor response, cause: %s".formatted(ExceptionUtils.getMessage(e))); + throw new PreBidException("Failed to parse price floor response for account %s, cause: %s" + .formatted(accountId, ExceptionUtils.getMessage(e))); } return priceFloorData; } @@ -220,11 +220,8 @@ private PriceFloorData updateCache(ResponseCacheInfo cacheInfo, String accountId) { final long maxAgeTimerId = createMaxAgeTimer(accountId, resolveCacheTtl(cacheInfo, fetchConfig)); - final AccountFetchContext fetchContext = AccountFetchContext.of( - cacheInfo.getRulesData(), - cacheInfo.getFetchStatus(), - cacheInfo.getErrorMessage(), - maxAgeTimerId); + final AccountFetchContext fetchContext = + AccountFetchContext.of(cacheInfo.getRulesData(), cacheInfo.getFetchStatus(), maxAgeTimerId); if (cacheInfo.getFetchStatus() == FetchStatus.success || !fetchedData.containsKey(accountId)) { fetchedData.put(accountId, fetchContext); @@ -270,27 +267,30 @@ private Long createMaxAgeTimer(String accountId, long cacheTtl) { return vertx.setTimer(TimeUnit.SECONDS.toMillis(effectiveCacheTtl), id -> fetchedData.remove(accountId)); } - private Future recoverFromFailedFetching(Throwable throwable, String fetchUrl) { + private Future recoverFromFailedFetching(Throwable throwable, + String fetchUrl, + String accountId) { + metrics.updatePriceFloorFetchMetric(MetricName.failure); final FetchStatus fetchStatus; - final String errorMessage; if (throwable instanceof TimeoutException || throwable instanceof ConnectTimeoutException) { fetchStatus = FetchStatus.timeout; - errorMessage = "Fetch price floor request timeout for fetch.url '%s' exceeded.".formatted(fetchUrl); + logger.error("Fetch price floor request timeout for fetch.url: '%s', account %s exceeded." + .formatted(fetchUrl, accountId)); } else { fetchStatus = FetchStatus.error; - errorMessage = "Failed to fetch price floor from provider for fetch.url '%s', with a reason: %s" - .formatted(fetchUrl, throwable.getMessage()); + logger.error( + "Failed to fetch price floor from provider for fetch.url: '%s', account = %s with a reason : %s " + .formatted(fetchUrl, accountId, throwable.getMessage())); } - return Future.succeededFuture(ResponseCacheInfo.withError(fetchStatus, errorMessage)); + return Future.succeededFuture(ResponseCacheInfo.withStatus(fetchStatus)); } private PriceFloorData createPeriodicTimerForRulesFetch(PriceFloorData priceFloorData, AccountPriceFloorsFetchConfig fetchConfig, String accountId) { - final long accountPeriodicTimeSec = ObjectUtil.getIfNotNull(fetchConfig, AccountPriceFloorsFetchConfig::getPeriodSec); final long periodicTimeSec = @@ -318,8 +318,6 @@ private static class AccountFetchContext { FetchStatus fetchStatus; - String errorMessage; - Long maxAgeTimerId; } @@ -330,12 +328,10 @@ private static class ResponseCacheInfo { FetchStatus fetchStatus; - String errorMessage; - Long cacheTtl; - public static ResponseCacheInfo withError(FetchStatus status, String errorMessage) { - return ResponseCacheInfo.of(null, status, errorMessage, null); + public static ResponseCacheInfo withStatus(FetchStatus status) { + return ResponseCacheInfo.of(null, status, null); } } } diff --git a/src/main/java/org/prebid/server/floors/proto/FetchResult.java b/src/main/java/org/prebid/server/floors/proto/FetchResult.java index 336bb26ee43..36c4fda58e0 100644 --- a/src/main/java/org/prebid/server/floors/proto/FetchResult.java +++ b/src/main/java/org/prebid/server/floors/proto/FetchResult.java @@ -9,18 +9,4 @@ public class FetchResult { PriceFloorData rulesData; FetchStatus fetchStatus; - - String errorMessage; - - public static FetchResult none(String errorMessage) { - return FetchResult.of(null, FetchStatus.none, errorMessage); - } - - public static FetchResult error(String errorMessage) { - return FetchResult.of(null, FetchStatus.error, errorMessage); - } - - public static FetchResult inProgress() { - return FetchResult.of(null, FetchStatus.inprogress, null); - } } diff --git a/src/main/java/org/prebid/server/spring/config/PriceFloorsConfiguration.java b/src/main/java/org/prebid/server/spring/config/PriceFloorsConfiguration.java index 16a79d6c0f6..8a483e92a4d 100644 --- a/src/main/java/org/prebid/server/spring/config/PriceFloorsConfiguration.java +++ b/src/main/java/org/prebid/server/spring/config/PriceFloorsConfiguration.java @@ -20,7 +20,6 @@ import org.prebid.server.metric.Metrics; import org.prebid.server.settings.ApplicationSettings; import org.prebid.server.vertx.httpclient.HttpClient; -import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.context.annotation.Bean; @@ -85,11 +84,9 @@ PriceFloorResolver noOpPriceFloorResolver() { @ConditionalOnProperty(prefix = "price-floors", name = "enabled", havingValue = "true") PriceFloorProcessor basicPriceFloorProcessor(PriceFloorFetcher floorFetcher, PriceFloorResolver floorResolver, - Metrics metrics, - JacksonMapper mapper, - @Value("${logging.sampling-rate:0.01}") double logSamplingRate) { + JacksonMapper mapper) { - return new BasicPriceFloorProcessor(floorFetcher, floorResolver, metrics, mapper, logSamplingRate); + return new BasicPriceFloorProcessor(floorFetcher, floorResolver, mapper); } @Bean 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 a4fda13f829..63ba2516ff4 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/BaseSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/BaseSpec.groovy @@ -40,7 +40,6 @@ 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 9dd17c31e0a..579a8e16597 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[ALERT_GENERAL] == 1 + assert metrics["alerts.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[ALERT_GENERAL] == 1 + assert metrics["alerts.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 ee3e808a56e..92ef175681e 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[ALERT_GENERAL] == 1 + assert metrics["alerts.general" as String] == 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[ALERT_GENERAL] == 1 + assert metrics["alerts.general" as String] == 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 97a0015cea5..8e6d7f1a57d 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[ALERT_GENERAL] == 1 + assert metrics["alerts.general" as String] == 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 4e1cbbf349b..8b3f5d936bd 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsBaseSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsBaseSpec.groovy @@ -15,7 +15,6 @@ 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 @@ -35,7 +34,8 @@ 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"] + public static final Map FLOORS_CONFIG = ["price-floors.enabled" : "true", + "settings.default-account-config": encode(defaultAccountConfigSettings)] protected static final String BASIC_FETCH_URL = networkServiceContainer.rootUri + FloorsProvider.FLOORS_ENDPOINT protected static final int MAX_MODEL_WEIGHT = 100 @@ -121,9 +121,8 @@ abstract class PriceFloorsBaseSpec extends BaseSpec { protected void cacheFloorsProviderRules(BidRequest bidRequest, PrebidServerService pbsService = floorsPbsService, - BidderName bidderName = BidderName.GENERIC, - FetchStatus fetchStatus = INPROGRESS) { - PBSUtils.waitUntil({ getRequests(pbsService.sendAuctionRequest(bidRequest))[bidderName.value]?.first?.ext?.prebid?.floors?.fetchStatus != fetchStatus }, + BidderName bidderName = BidderName.GENERIC) { + PBSUtils.waitUntil({ getRequests(pbsService.sendAuctionRequest(bidRequest))[bidderName.value]?.first?.ext?.prebid?.floors?.fetchStatus != INPROGRESS }, 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 699b9bdcda0..a897fbeff7c 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 @@ -17,7 +17,6 @@ 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 @@ -44,24 +43,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { private static final int DEFAULT_FLOOR_VALUE_MIN = 0 private static final int FLOOR_MIN = 0 - private static final String FETCH_FAILURE_METRIC = "price-floors.fetch.failure" - 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" - } - 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" - } + private static final String FETCH_FAILURE_METRIC = "price-floors.fetch.failure" def "PBS should activate floors feature when price-floors.enabled = true in PBS config"() { given: "Pbs with PF configuration" @@ -138,8 +121,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def logs = floorsPbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, bidRequest.accountId) assert floorsLogs.size() == 1 - assert floorsLogs.first().contains("No price floor data for account $bidRequest.accountId and request $bidRequest.id, " + - "reason: Malformed fetch.url 'null' passed") + assert floorsLogs.first().contains("Malformed fetch.url: 'null', passed for account $bidRequest.accountId") and: "PBS floors validation failure should not reject the entire auction" assert !response.seatbid.isEmpty() @@ -563,7 +545,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(bidRequest.accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) + cacheFloorsProviderRules(bidRequest, floorsPbsService) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -572,15 +554,14 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def metrics = floorsPbsService.sendCollectedMetricsRequest() assert metrics[FETCH_FAILURE_METRIC] == 1 - and: "PBS should fetch data" + then: "PBS should fetch data" assert floorsProvider.getRequestCount(bidRequest.accountId) == 1 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(URL_EMPTY_ERROR_LOG(bidRequest, message)) + assert floorsLogs[0].contains("reason : Price floor data useFetchDataRate must be in range(0-100), but was $accounntUseFetchDataRate") and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -664,7 +645,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, BAD_REQUEST_400) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) + cacheFloorsProviderRules(bidRequest, floorsPbsService) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -676,11 +657,12 @@ 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$bidRequest.accountId") + def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) + 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") and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -705,9 +687,6 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def invalidJson = "{{}}" floorsProvider.setResponse(accountId, invalidJson) - and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) - when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -718,11 +697,12 @@ 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$bidRequest.accountId") + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) + 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") and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -746,9 +726,6 @@ 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, GENERIC, NONE) - when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -759,11 +736,12 @@ 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$bidRequest.accountId") + def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) + 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) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -790,9 +768,6 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { } floorsProvider.setResponse(accountId, floorsResponse) - and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) - when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -803,11 +778,12 @@ 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$bidRequest.accountId") + def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) + 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) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -834,9 +810,6 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { } floorsProvider.setResponse(accountId, floorsResponse) - and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) - when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -848,9 +821,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "PBS log should contain error" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") + def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, PRICE_FLOOR_VALUES_MISSING)) + 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) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -880,9 +855,6 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { } floorsProvider.setResponse(accountId, floorsResponse) - and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) - when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -893,11 +865,12 @@ 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$bidRequest.accountId") + def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) + 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") and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -928,9 +901,6 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "Set Floors Provider response with timeout" floorsProvider.setResponseWithTimeout(accountId) - and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, pbsService, GENERIC, NONE) - when: "PBS processes auction request" def response = pbsService.sendAuctionRequest(bidRequest) @@ -944,8 +914,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("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") + assert floorsLogs[0].contains("Fetch price floor request timeout for fetch.url: '$BASIC_FETCH_URL$accountId', " + + "account $accountId exceeded") and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -974,9 +944,6 @@ 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, GENERIC, NONE) - when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -987,11 +954,12 @@ 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$bidRequest.accountId") + def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) + 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") and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -1316,10 +1284,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() 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 == [WARNING_MESSAGE(message)] + and: "Response should contain error" + assert response.ext?.errors[PREBID]*.code == [999] + assert response.ext?.errors[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: Price floor floorMin " + + "must be positive float, but was $invalidFloorMin"] } def "PBS should validate rules from request when request doesn't contain modelGroups"() { @@ -1343,10 +1312,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() 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 == [WARNING_MESSAGE(message)] + and: "Response should contain error" + assert response.ext?.errors[PREBID]*.code == [999] + assert response.ext?.errors[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: Price floor rules " + + "should contain at least one model group"] } def "PBS should validate rules from request when request doesn't contain values"() { @@ -1370,9 +1340,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain warning" - assert response.ext?.warnings[PREBID]*.code == [999] - assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(PRICE_FLOOR_VALUES_MISSING)] + and: "Response should contain error" + assert response.ext?.errors[PREBID]*.code == [999] + assert response.ext?.errors[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: Price floor rules values " + + "can't be null or empty, but were null"] } def "PBS should validate rules from request when modelWeight from request is invalid"() { @@ -1400,10 +1372,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequest(bidRequest.id) assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain warning" - assert response.ext?.warnings[PREBID]*.code == [999] - assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(MODEL_WEIGHT_INVALID.formatted(invalidModelWeight))] - + and: "Response should contain error" + assert response.ext?.errors[PREBID]*.code == [999] + assert response.ext?.errors[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: Price floor modelGroup modelWeight " + + "must be in range(1-100), but was $invalidModelWeight"] where: invalidModelWeight << [0, MAX_MODEL_WEIGHT + 1] } @@ -1438,9 +1411,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(ampStoredRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain warning" - assert response.ext?.warnings[PREBID]*.code == [999] - assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(MODEL_WEIGHT_INVALID.formatted(invalidModelWeight))] + and: "Response should contain error" + assert response.ext?.errors[PREBID]*.code == [999] + assert response.ext?.errors[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: Price floor modelGroup modelWeight " + + "must be in range(1-100), but was $invalidModelWeight"] where: invalidModelWeight << [0, MAX_MODEL_WEIGHT + 1] @@ -1476,10 +1451,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() 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 == [WARNING_MESSAGE(message)] + and: "Response should contain error" + assert response.ext?.errors[PREBID]*.code == [999] + assert response.ext?.errors[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: Price floor root skipRate " + + "must be in range(0-100), but was $invalidSkipRate"] where: invalidSkipRate << [SKIP_RATE_MIN - 1, SKIP_RATE_MAX + 1] @@ -1515,10 +1491,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() 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 == [WARNING_MESSAGE(message)] + and: "Response should contain error" + assert response.ext?.errors[PREBID]*.code == [999] + assert response.ext?.errors[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: Price floor data skipRate " + + "must be in range(0-100), but was $invalidSkipRate"] where: invalidSkipRate << [SKIP_RATE_MIN - 1, SKIP_RATE_MAX + 1] @@ -1554,9 +1531,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain warning" - assert response.ext?.warnings[PREBID]*.code == [999] - assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(SKIP_RATE_INVALID.formatted(invalidSkipRate))] + and: "Response should contain error" + assert response.ext?.errors[PREBID]*.code == [999] + assert response.ext?.errors[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: Price floor modelGroup skipRate " + + "must be in range(0-100), but was $invalidSkipRate"] where: invalidSkipRate << [SKIP_RATE_MIN - 1, SKIP_RATE_MAX + 1] @@ -1588,10 +1567,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() 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 == [WARNING_MESSAGE(message)] + and: "Response should contain error" + assert response.ext?.errors[PREBID]*.code == [999] + assert response.ext?.errors[PREBID]*.message == + ["Failed to parse price floors from request, with a reason: Price floor modelGroup default " + + "must be positive float, but was $invalidDefaultFloorValue"] } def "PBS should not invalidate previously good fetched data when floors provider return invalid data"() { @@ -1671,7 +1651,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) + cacheFloorsProviderRules(bidRequest) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -1689,9 +1669,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "PBS log should contain error" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") + def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, MODEL_WEIGHT_INVALID.formatted(invalidModelWeight))) + 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") and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -1728,7 +1710,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) + cacheFloorsProviderRules(bidRequest) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -1745,11 +1727,12 @@ 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$bidRequest.accountId") + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) + 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") and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -1786,7 +1769,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) + cacheFloorsProviderRules(bidRequest) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -1804,9 +1787,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "PBS log should contain error" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, SKIP_RATE_INVALID.formatted(invalidSkipRate))) + 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") and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -1843,7 +1828,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) + cacheFloorsProviderRules(bidRequest) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -1860,11 +1845,12 @@ 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$bidRequest.accountId") + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) + 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") 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 30acd9b4bdc..ee861a24587 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,7 +61,6 @@ 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 { @@ -218,15 +217,10 @@ 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" + and: "PBS should not contain errors, warnings" + assert !response.ext?.warnings 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 1de08b44cc9..72be2f0a0f3 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,9 +1,5 @@ package org.prebid.server.functional.tests.pricefloors -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 @@ -33,30 +29,13 @@ 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.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 -> - "$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 - - 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" @@ -301,7 +280,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) - then: "PBS should not log warning or errors" + then: "PBS should not log warning" assert !response.ext.warnings assert !response.ext.errors @@ -340,9 +319,8 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { when: "PBS processes amp request" def response = floorsPbsService.sendAmpRequest(ampRequest) - then: "PBS should not log warning or errors" + then: "PBS should not log warning" assert !response.ext.warnings - assert !response.ext.errors and: "Bidder request should contain bidFloor from the request" def bidderRequest = bidder.getBidderRequest(ampStoredRequest.id) @@ -542,7 +520,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp.last().bidFloor == videoFloorValue } - def "PBS shouldn't emit warning when request schema.fields equal to floor-config.max-schema-dims"() { + def "PBS shouldn't emit errors when request schema.fields than floor-config.max-schema-dims"() { given: "Bid request with schema 2 fields" def bidRequest = bidRequestWithFloors.tap { ext.prebid.floors.maxSchemaDims = PBSUtils.getRandomNumber(2) @@ -553,19 +531,18 @@ 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) - 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] + then: "PBS shouldn't log a errors" + assert !response.ext?.errors } - def "PBS should emit warning when request has more rules than price-floor.max-rules"() { + def "PBS should emit errors when request has more rules than price-floor.max-rules"() { given: "BidRequest with 2 rules" def requestFloorValue = PBSUtils.randomFloorValue def bidRequest = bidRequestWithFloors.tap { @@ -574,10 +551,9 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { (new Rule(mediaType: BANNER, country: Country.MULTIPLE).rule): requestFloorValue] } - and: "Account with maxRules and disabled fetching in the DB" + 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 } @@ -588,20 +564,14 @@ 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 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 == [WARNING_MESSAGE(message)] - - and: "Alerts.general metrics should be populated" - def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert metrics[ALERT_GENERAL] == 1 + 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(bidRequest)} exceeded its maximum number ${MAX_RULES_SIZE}"] where: maxRules | maxRulesSnakeCase @@ -609,36 +579,30 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { null | MAX_RULES_SIZE } - def "PBS should emit warning when request has more schema.fields than floor-config.max-schema-dims"() { + def "PBS should emit errors when request has more schema.fields than floor-config.max-schema-dims"() { given: "BidRequest with schema 2 fields" def bidRequest = bidRequestWithFloors - and: "Account with maxSchemaDims and disabled fetching in the DB" + 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: "Flush metrics" - flushMetrics(floorsPbsService) + and: "Set bidder response" + def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) + bidder.setResponse(bidRequest.id, bidResponse) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) - then: "PBS should log a warning" - 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: "Alerts.general metrics should be populated" - def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert metrics[ALERT_GENERAL] == 1 + 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}"] where: maxSchemaDims | maxSchemaDimsSnakeCase @@ -646,7 +610,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { null | MAX_SCHEMA_DIMENSIONS_SIZE } - def "PBS should emit warning when request has more schema.fields than default-account.max-schema-dims"() { + def "PBS should emit errors when request has more schema.fields than default-account.max-schema-dims"() { given: "Floor config with default account" def accountConfig = getDefaultAccountConfigSettings().tap { auction.priceFloors.maxSchemaDims = MAX_SCHEMA_DIMENSIONS_SIZE @@ -663,46 +627,45 @@ 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) - and: "Flush metrics" - flushMetrics(floorsPbsService) + and: "Set bidder response" + def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) + bidder.setResponse(bidRequest.id, bidResponse) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) - then: "PBS should log a warning" - 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 + 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}"] - and: "Metrics should be updated" + 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 warning when request has more schema.fields than default-account.fetch.max-schema-dims"() { - given: "BidRequest with schema 2 fields" + 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 bidRequest = bidRequestWithFloors and: "Floor config with default account" def accountConfig = getDefaultAccountConfigSettings().tap { - auction.priceFloors.fetch.enabled = false + 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 = MAX_SCHEMA_DIMENSIONS_SIZE + auction.priceFloors.maxSchemaDims = null } def pbsFloorConfig = GENERIC_ALIAS_CONFIG + ["price-floors.enabled" : "true", "settings.default-account-config": encode(accountConfig)] @@ -720,59 +683,48 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { } accountDao.save(account) - when: "PBS processes auction request" - def response = floorsPbsService.sendAuctionRequest(bidRequest) + and: "Set bidder response" + def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) + bidder.setResponse(bidRequest.id, bidResponse) - 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)] + when: "PBS processes auction request" + floorsPbsService.sendAuctionRequest(bidRequest) - and: "PBS shouldn't log a errors" + then: "PBS should log a errors" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) + def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) assert floorsLogs.size() == 1 + assert floorsLogs[0].contains("Failed to fetch price floor from provider for fetch.url: " + + "'$BASIC_FETCH_URL$accountId', account = $accountId with a reason : Price floor schema dimensions ${getSchemaSize(bidRequest)} " + + "exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}") - and: "Metrics should be updated" + 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 warning when request has more schema.fields than fetch.max-schema-dims"() { - given: "BidRequest with schema 2 fields" + def "PBS should emit errors when request has more schema.fields than fetch.max-schema-dims"() { + given: "Default BidRequest with floorMin" def bidRequest = bidRequestWithFloors and: "Account with disabled fetch in the DB" def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { - config.auction.priceFloors.fetch.enabled = false config.auction.priceFloors.maxSchemaDims = maxSchemaDims config.auction.priceFloors.maxSchemaDimsSnakeCase = maxSchemaDimsSnakeCase } accountDao.save(account) - and: "Flush metrics" - flushMetrics(floorsPbsService) - when: "PBS processes auction request" 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 == [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 - - and: "Alerts.general metrics should be populated" - def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert metrics[ALERT_GENERAL] == 1 + 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}"] where: maxSchemaDims | maxSchemaDimsSnakeCase @@ -784,36 +736,28 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { given: "BidRequest with schema 2 fields" def bidRequest = bidRequestWithFloors - and: "Account with maxSchemaDims and disabled fetching in the DB" + and: "Account with maxSchemaDims in the DB" 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 } accountDao.save(account) - and: "Flush metrics" - flushMetrics(floorsPbsService) + and: "Set bidder response" + def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) + bidder.setResponse(bidRequest.id, bidResponse) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) - then: "PBS should log a warning" - 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 - - and: "Alerts.general metrics should be populated" - def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert metrics[ALERT_GENERAL] == 1 + 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}"] } def "PBS shouldn't fail with error and maxSchemaDims take precede over fetch.maxSchemaDims when requested both"() { @@ -828,18 +772,22 @@ 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) - then: "PBS shouldn't add warnings or errors" - assert !response.ext?.warnings + then: "PBS shouldn't log a errors" + assert !response.ext?.errors } - def "PBS should emit warning when stored request has more rules than price-floor.max-rules for amp request"() { + def "PBS should emit errors when stored request has more rules than price-floor.max-rules for amp request"() { given: "Default AmpRequest" def ampRequest = AmpRequest.defaultAmpRequest - 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 @@ -850,9 +798,8 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def storedRequest = StoredRequest.getStoredRequest(ampRequest, ampStoredRequest) storedRequestDao.save(storedRequest) - and: "Account with maxRules and disabled fetching in the DB" + 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 } @@ -863,27 +810,15 @@ 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 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, "No price floor data for account $ampRequest.account and " + - "request $ampStoredRequest.id, reason: ${WARNING_MESSAGE(message)}") - assert floorsLogs.size() == 1 - - and: "Alerts.general metrics should be populated" - def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert metrics[ALERT_GENERAL] == 1 + 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}"] where: maxRules | maxRulesSnakeCase @@ -891,298 +826,6 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { null | MAX_RULES_SIZE } - 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: "Account with enabled fetching" - def account = getAccountWithEnabledFetch(bidRequest.accountId) - accountDao.save(account) - - and: "Flush metrics" - flushMetrics(floorsPbsService) - - when: "PBS processes auction request" - def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) - - 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(bidRequest, message)) - assert !floorsLogs.size() - - 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] - - 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 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 - } - - and: "Flush metrics" - flushMetrics(floorsPbsService) - - and: "Account with disabled fetching" - def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { - config.auction.priceFloors.fetch.enabled = false - } - accountDao.save(account) - - when: "PBS processes auction request" - def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) - - 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 - - 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 bidderRequest.ext.prebid.floors.fetchStatus == [NONE] - - - and: "Alerts.general metrics should be populated" - def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert metrics[ALERT_GENERAL] == 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 - } - - and: "Flush metrics" - flushMetrics(floorsPbsService) - - when: "PBS processes auction request" - def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) - - 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 log a errors" - def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) - assert floorsLogs.size() == 1 - - and: "Alerts.general metrics should be populated" - def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert metrics[ALERT_GENERAL] == 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 - } - - and: "Flush metrics" - flushMetrics(floorsPbsService) - - when: "PBS processes auction request" - def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) - - 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 log a errors" - def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) - assert floorsLogs.size() == 1 - - and: "Alerts.general metrics should be populated" - def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert metrics[ALERT_GENERAL] == 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) - ] - } - - and: "Flush metrics" - flushMetrics(floorsPbsService) - - when: "PBS processes auction request" - def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) - - 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 - - and: "Alerts.general metrics should be populated" - def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert metrics[ALERT_GENERAL] == 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) - ] - } - - and: "Flush metrics" - flushMetrics(floorsPbsService) - - when: "PBS processes auction request" - def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) - - 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 log an errors" - def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) - assert floorsLogs.size() == 1 - - and: "Alerts.general metrics should be populated" - def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert metrics[ALERT_GENERAL] == 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) - ] - } - - and: "Flush metrics" - flushMetrics(floorsPbsService) - - when: "PBS processes auction request" - def bidResponse = floorsPbsService.sendAuctionRequest(bidRequest) - - 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 - - and: "Alerts.general metrics should be populated" - def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert metrics[ALERT_GENERAL] == 1 - } - - def "PBS should use default floors config when original account config is invalid"() { - given: "Bid request with empty floors" - 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: "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() - - 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: "Bid request without floors" - def bidRequest = BidRequest.defaultBidRequest - - and: "Account with disabled dynamic data" - def account = getAccountWithEnabledFetch(bidRequest.accountId).tap { - config.auction.priceFloors.useDynamicData = false - } - accountDao.save(account) - - and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, SUCCESS) - - and: "Flush metrics" - flushMetrics(floorsPbsService) - - when: "PBS processes auction request" - def response = floorsPbsService.sendAuctionRequest(bidRequest) - - then: "PBS should log a warning" - assert response.ext?.warnings[PREBID]*.code == [999] - assert response.ext?.warnings[PREBID]*.message == ['Using dynamic data is not allowed'] - - and: "PBS should log a errors" - def logs = floorsPbsService.getLogsByTime(startTime) - 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 - - and: "Alerts.general metrics should be populated" - def metrics = floorsPbsService.sendCollectedMetricsRequest() - assert metrics[ALERT_GENERAL] == 1 - } - private static int getSchemaSize(BidRequest bidRequest) { bidRequest?.ext?.prebid?.floors?.data?.modelGroups[0].schema.fields.size() } 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 717c9f32d5b..771fac9bd84 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[ALERT_GENERAL] == 1 + assert metrics["alerts.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 5fc0f5d7bca..7302ad79aed 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[ALERT_GENERAL] == 1 + assert metrics["alerts.general"] == 1 and: "Bid response should contain seatBid" assert response.seatbid.size() == 1 diff --git a/src/test/java/org/prebid/server/floors/BasicPriceFloorProcessorTest.java b/src/test/java/org/prebid/server/floors/BasicPriceFloorProcessorTest.java index 432b68c0c6b..78ce82dd3fd 100644 --- a/src/test/java/org/prebid/server/floors/BasicPriceFloorProcessorTest.java +++ b/src/test/java/org/prebid/server/floors/BasicPriceFloorProcessorTest.java @@ -20,8 +20,6 @@ import org.prebid.server.floors.model.PriceFloorSchema; import org.prebid.server.floors.proto.FetchResult; import org.prebid.server.floors.proto.FetchStatus; -import org.prebid.server.metric.MetricName; -import org.prebid.server.metric.Metrics; import org.prebid.server.proto.openrtb.ext.request.ExtImpPrebidFloors; import org.prebid.server.proto.openrtb.ext.request.ExtRequest; import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid; @@ -53,14 +51,12 @@ public class BasicPriceFloorProcessorTest extends VertxTest { private PriceFloorFetcher priceFloorFetcher; @Mock private PriceFloorResolver floorResolver; - @Mock - private Metrics metrics; private BasicPriceFloorProcessor target; @BeforeEach public void setUp() { - target = new BasicPriceFloorProcessor(priceFloorFetcher, floorResolver, metrics, jacksonMapper, 0.0d); + target = new BasicPriceFloorProcessor(priceFloorFetcher, floorResolver, jacksonMapper); } @Test @@ -74,7 +70,7 @@ public void shouldSetRulesEnabledFieldToFalseIfPriceFloorsDisabledForAccount() { new ArrayList<>()); // then - verifyNoInteractions(priceFloorFetcher, metrics); + verifyNoInteractions(priceFloorFetcher); assertThat(result).isEqualTo(givenBidRequest(identity(), PriceFloorRules.builder().enabled(false).build())); } @@ -89,7 +85,7 @@ public void shouldSetRulesEnabledFieldToFalseIfPriceFloorsDisabledForRequest() { new ArrayList<>()); // then - verifyNoInteractions(priceFloorFetcher, metrics); + verifyNoInteractions(priceFloorFetcher); assertThat(result) .extracting(BidRequest::getExt) @@ -102,8 +98,8 @@ public void shouldSetRulesEnabledFieldToFalseIfPriceFloorsDisabledForRequest() { @Test public void shouldUseFloorsDataFromProviderIfPresent() { // given - final PriceFloorData providerFloorsData = givenFloorData(identity()); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); + final PriceFloorData providerFloorsData = givenFloorData(floors -> floors.floorProvider("provider.com")); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -122,16 +118,16 @@ public void shouldUseFloorsDataFromProviderIfPresent() { .data(providerFloorsData) .fetchStatus(FetchStatus.success) .location(PriceFloorLocation.fetch))); - verifyNoInteractions(metrics); - } @Test public void shouldUseFloorsFromProviderIfUseDynamicDataAndUseFetchDataRateAreAbsent() { // given - final PriceFloorData providerFloorsData = givenFloorData(floors -> floors.useFetchDataRate(null)); + final PriceFloorData providerFloorsData = givenFloorData(floors -> floors + .floorProvider("provider.com") + .useFetchDataRate(null)); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -149,15 +145,16 @@ public void shouldUseFloorsFromProviderIfUseDynamicDataAndUseFetchDataRateAreAbs .data(providerFloorsData) .fetchStatus(FetchStatus.success) .location(PriceFloorLocation.fetch))); - verifyNoInteractions(metrics); } @Test public void shouldUseFloorsFromProviderIfUseDynamicDataIsAbsentAndUseFetchDataRateIs100() { // given - final PriceFloorData providerFloorsData = givenFloorData(floors -> floors.useFetchDataRate(100)); + final PriceFloorData providerFloorsData = givenFloorData(floors -> floors + .floorProvider("provider.com") + .useFetchDataRate(100)); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -175,15 +172,16 @@ public void shouldUseFloorsFromProviderIfUseDynamicDataIsAbsentAndUseFetchDataRa .data(providerFloorsData) .fetchStatus(FetchStatus.success) .location(PriceFloorLocation.fetch))); - verifyNoInteractions(metrics); } @Test public void shouldNotUseFloorsFromProviderIfUseDynamicDataIsAbsentAndUseFetchDataRateIs0() { // given - final PriceFloorData providerFloorsData = givenFloorData(floors -> floors.useFetchDataRate(0)); + final PriceFloorData providerFloorsData = givenFloorData(floors -> floors + .floorProvider("provider.com") + .useFetchDataRate(0)); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -201,44 +199,15 @@ public void shouldNotUseFloorsFromProviderIfUseDynamicDataIsAbsentAndUseFetchDat assertThat(actualRules) .extracting(PriceFloorRules::getLocation) .isEqualTo(PriceFloorLocation.noData); - verifyNoInteractions(metrics); } @Test - public void shouldTolerateInvalidFloorsFromRequestWhenFetchIsSuccessAndUseFetchDataRateIs0() { + public void shouldUseFloorsFromProviderIfUseDynamicDataIsTrueAndUseFetchDataRateIsAbsent() { // given final PriceFloorData providerFloorsData = givenFloorData(floors -> floors .floorProvider("provider.com") - .useFetchDataRate(0)); - - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); - final ArrayList warnings = new ArrayList<>(); - - // when - final BidRequest result = target.enrichWithPriceFloors( - givenBidRequest(identity(), givenFloors(floors -> floors.data(null))), - givenAccount(identity()), - "bidder", - new ArrayList<>(), - warnings); - - // then - assertThat(extractFloors(result)).isEqualTo(PriceFloorRules.builder() - .fetchStatus(FetchStatus.success) - .enabled(true) - .skipped(false) - .location(PriceFloorLocation.noData) - .build()); - - assertThat(warnings).isEmpty(); - verifyNoInteractions(metrics); - } - - @Test - public void shouldUseFloorsFromProviderIfUseDynamicDataIsTrueAndUseFetchDataRateIsAbsent() { - // given - final PriceFloorData providerFloorsData = givenFloorData(floors -> floors.useFetchDataRate(null)); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); + .useFetchDataRate(null)); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -257,330 +226,65 @@ public void shouldUseFloorsFromProviderIfUseDynamicDataIsTrueAndUseFetchDataRate .floorMin(BigDecimal.ONE) .fetchStatus(FetchStatus.success) .location(PriceFloorLocation.fetch))); - verifyNoInteractions(metrics); } @Test - public void shouldNotUseFloorsFromProviderIfUseDynamicDataIsFalse() { + public void shouldNotUseFloorsFromProviderIfUseDynamicDataIsFalseAndUseFetchDataRateIsAbsent() { // given - final PriceFloorData providerFloorsData = givenFloorData(identity()); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); + final PriceFloorData providerFloorsData = givenFloorData(floors -> floors + .floorProvider("provider.com") + .useFetchDataRate(null)); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); // when - final ArrayList warnings = new ArrayList<>(); final BidRequest result = target.enrichWithPriceFloors( givenBidRequest(identity(), null), givenAccount(floorsConfig -> floorsConfig.useDynamicData(false)), "bidder", new ArrayList<>(), - warnings); + new ArrayList<>()); // then - assertThat(extractFloors(result)).isEqualTo(PriceFloorRules.builder() - .fetchStatus(FetchStatus.success) - .enabled(true) - .skipped(false) - .location(PriceFloorLocation.noData) - .build()); - verify(metrics).updateAlertsMetrics(MetricName.general); - assertThat(warnings).containsExactly("Using dynamic data is not allowed"); + final PriceFloorRules actualRules = extractFloors(result); + assertThat(actualRules) + .extracting(PriceFloorRules::getFetchStatus) + .isEqualTo(FetchStatus.success); + assertThat(actualRules) + .extracting(PriceFloorRules::getLocation) + .isEqualTo(PriceFloorLocation.noData); } @Test - public void shouldNotTolerateInvalidFloorsFromRequestWhenFetchIsSuccessAndUseDynamicDataIsFalse() { + public void shouldNotUseFloorsFromProviderIfUseDynamicDataIsFalseAndUseFetchDataRateIs100() { // given final PriceFloorData providerFloorsData = givenFloorData(floors -> floors .floorProvider("provider.com") - .useFetchDataRate(0)); - - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); - final ArrayList warnings = new ArrayList<>(); - - // when - final BidRequest result = target.enrichWithPriceFloors( - givenBidRequest(identity(), givenFloors(floors -> floors.data(null))), - givenAccount(floorsConfig -> floorsConfig.useDynamicData(false)), - "bidder", - new ArrayList<>(), - warnings); - - // then - assertThat(extractFloors(result)).isEqualTo(PriceFloorRules.builder() - .fetchStatus(FetchStatus.success) - .enabled(true) - .skipped(false) - .location(PriceFloorLocation.noData) - .build()); - - verify(metrics).updateAlertsMetrics(MetricName.general); - assertThat(warnings).containsExactly("Using dynamic data is not allowed. " - + "Following parsing of request price floors is failed: Price floor rules data must be present"); - } - - @Test - public void shouldUseFloorsFromRequestIfUseDynamicDataIsFalse() { - // given - final PriceFloorData providerFloorsData = givenFloorData(identity()); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); - - // when - final ArrayList warnings = new ArrayList<>(); - final BidRequest result = target.enrichWithPriceFloors( - givenBidRequest(identity(), givenFloors(floors -> floors.floorMin(BigDecimal.ONE))), - givenAccount(floorsConfig -> floorsConfig.useDynamicData(false)), - "bidder", - new ArrayList<>(), - warnings); - - // then - assertThat(extractFloors(result)).isEqualTo(givenFloors(floors -> floors - .enabled(true) - .skipped(false) - .fetchStatus(FetchStatus.success) - .floorMin(BigDecimal.ONE) - .location(PriceFloorLocation.request))); - verifyNoInteractions(metrics); - assertThat(warnings).isEmpty(); - } - - @Test - public void shouldNotUseFloorsWhenProviderFetchingIsDisabled() { - // given - final PriceFloorData providerFloorsData = givenFloorData(identity()); - given(priceFloorFetcher.fetch(any())) - .willReturn(FetchResult.of(providerFloorsData, FetchStatus.none, "errorMessage")); - - // when - final ArrayList warnings = new ArrayList<>(); - final BidRequest result = target.enrichWithPriceFloors( - givenBidRequest(identity(), null), - givenAccount(identity()), - "bidder", - new ArrayList<>(), - warnings); - - // then - assertThat(extractFloors(result)).isEqualTo(PriceFloorRules.builder() - .fetchStatus(FetchStatus.none) - .enabled(true) - .skipped(false) - .location(PriceFloorLocation.noData) - .build()); - verify(metrics).updateAlertsMetrics(MetricName.general); - assertThat(warnings).containsExactly("errorMessage"); - } - - @Test - public void shouldNotTolerateInvalidFloorsFromRequestWhenFetchIsDisabled() { - // given - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); - - // when - final ArrayList warnings = new ArrayList<>(); - final BidRequest result = target.enrichWithPriceFloors( - givenBidRequest(identity(), givenFloors(floors -> floors.data(null))), - givenAccount(identity()), - "bidder", - new ArrayList<>(), - warnings); - - // then - assertThat(extractFloors(result)).isEqualTo(PriceFloorRules.builder() - .fetchStatus(FetchStatus.none) - .enabled(true) - .skipped(false) - .location(PriceFloorLocation.noData) - .build()); - verify(metrics).updateAlertsMetrics(MetricName.general); - assertThat(warnings).containsExactly("errorMessage. " - + "Following parsing of request price floors is failed: Price floor rules data must be present"); - } - - @Test - public void shouldNotUseFloorsWhenProviderFetchingIsFailed() { - // given - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.error("errorMessage")); - - // when - final ArrayList warnings = new ArrayList<>(); - final BidRequest result = target.enrichWithPriceFloors( - givenBidRequest(identity(), null), - givenAccount(identity()), - "bidder", - new ArrayList<>(), - warnings); - - // then - assertThat(extractFloors(result)).isEqualTo(PriceFloorRules.builder() - .fetchStatus(FetchStatus.error) - .enabled(true) - .skipped(false) - .location(PriceFloorLocation.noData) - .build()); - verify(metrics).updateAlertsMetrics(MetricName.general); - assertThat(warnings).containsExactly("errorMessage"); - } - - @Test - public void shouldNotTolerateInvalidFloorsFromRequestWhenFetchIsFailed() { - // given - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.error("errorMessage")); + .useFetchDataRate(100)); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); // when - final ArrayList warnings = new ArrayList<>(); - final BidRequest result = target.enrichWithPriceFloors( - givenBidRequest(identity(), givenFloors(floors -> floors.data(null))), - givenAccount(identity()), - "bidder", - new ArrayList<>(), - warnings); - - // then - assertThat(extractFloors(result)).isEqualTo(PriceFloorRules.builder() - .fetchStatus(FetchStatus.error) - .enabled(true) - .skipped(false) - .location(PriceFloorLocation.noData) - .build()); - verify(metrics).updateAlertsMetrics(MetricName.general); - assertThat(warnings).containsExactly("errorMessage. " - + "Following parsing of request price floors is failed: Price floor rules data must be present"); - } - - @Test - public void shouldNotUseFloorsWhenProviderFetchingIsFailedWithTimeout() { - // given - given(priceFloorFetcher.fetch(any())) - .willReturn(FetchResult.of(null, FetchStatus.timeout, "errorMessage")); - - // when - final ArrayList warnings = new ArrayList<>(); final BidRequest result = target.enrichWithPriceFloors( givenBidRequest(identity(), null), - givenAccount(identity()), - "bidder", - new ArrayList<>(), - warnings); - - // then - assertThat(extractFloors(result)).isEqualTo(PriceFloorRules.builder() - .fetchStatus(FetchStatus.timeout) - .enabled(true) - .skipped(false) - .location(PriceFloorLocation.noData) - .build()); - verify(metrics).updateAlertsMetrics(MetricName.general); - assertThat(warnings).containsExactly("errorMessage"); - } - - @Test - public void shouldNotTolerateInvalidFloorsFromRequestWhenFetchIsFailedWithTimeout() { - // given - given(priceFloorFetcher.fetch(any())) - .willReturn(FetchResult.of(null, FetchStatus.timeout, "errorMessage")); - - // when - final ArrayList warnings = new ArrayList<>(); - final BidRequest result = target.enrichWithPriceFloors( - givenBidRequest(identity(), givenFloors(floors -> floors.data(null))), - givenAccount(identity()), + givenAccount(floorsConfig -> floorsConfig.useDynamicData(false)), "bidder", new ArrayList<>(), - warnings); + new ArrayList<>()); // then final PriceFloorRules actualRules = extractFloors(result); assertThat(actualRules) .extracting(PriceFloorRules::getFetchStatus) - .isEqualTo(FetchStatus.timeout); + .isEqualTo(FetchStatus.success); assertThat(actualRules) .extracting(PriceFloorRules::getLocation) .isEqualTo(PriceFloorLocation.noData); - verify(metrics).updateAlertsMetrics(MetricName.general); - assertThat(warnings).containsExactly("errorMessage. " - + "Following parsing of request price floors is failed: Price floor rules data must be present"); - } - - @Test - public void shouldUseFloorsFromRequestIfFetchingIsDisabled() { - // given - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); - - // when - final ArrayList warnings = new ArrayList<>(); - final BidRequest result = target.enrichWithPriceFloors( - givenBidRequest(identity(), givenFloors(floors -> floors.floorMin(BigDecimal.ONE))), - givenAccount(identity()), - "bidder", - new ArrayList<>(), - warnings); - - // then - assertThat(extractFloors(result)).isEqualTo(givenFloors(floors -> floors - .enabled(true) - .skipped(false) - .fetchStatus(FetchStatus.none) - .floorMin(BigDecimal.ONE) - .location(PriceFloorLocation.request))); - verifyNoInteractions(metrics); - assertThat(warnings).isEmpty(); - } - - @Test - public void shouldUseFloorsFromRequestIfFetchingIsFailed() { - // given - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.error("errorMessage")); - - // when - final ArrayList warnings = new ArrayList<>(); - final BidRequest result = target.enrichWithPriceFloors( - givenBidRequest(identity(), givenFloors(floors -> floors.floorMin(BigDecimal.ONE))), - givenAccount(identity()), - "bidder", - new ArrayList<>(), - warnings); - - // then - assertThat(extractFloors(result)).isEqualTo(givenFloors(floors -> floors - .enabled(true) - .skipped(false) - .fetchStatus(FetchStatus.error) - .floorMin(BigDecimal.ONE) - .location(PriceFloorLocation.request))); - verifyNoInteractions(metrics); - assertThat(warnings).isEmpty(); - } - - @Test - public void shouldUseFloorsFromRequestIfFetchingIsFailedWithTimeout() { - // given - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(null, FetchStatus.timeout, "errorMessage")); - - // when - final ArrayList warnings = new ArrayList<>(); - final BidRequest result = target.enrichWithPriceFloors( - givenBidRequest(identity(), givenFloors(floors -> floors.floorMin(BigDecimal.ONE))), - givenAccount(identity()), - "bidder", - new ArrayList<>(), - warnings); - - // then - assertThat(extractFloors(result)).isEqualTo(givenFloors(floors -> floors - .enabled(true) - .skipped(false) - .fetchStatus(FetchStatus.timeout) - .floorMin(BigDecimal.ONE) - .location(PriceFloorLocation.request))); - verifyNoInteractions(metrics); - assertThat(warnings).isEmpty(); } @Test public void shouldMergeProviderWithRequestFloors() { // given - final PriceFloorData providerFloorsData = givenFloorData(identity()); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); + final PriceFloorData providerFloorsData = givenFloorData(floors -> floors.floorProvider("provider.com")); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -611,8 +315,8 @@ public void shouldMergeProviderWithRequestFloors() { @Test public void shouldReturnProviderFloorsWhenNotEnabledByRequestAndEnforceRateAndFloorPriceAreAbsent() { // given - final PriceFloorData providerFloorsData = givenFloorData(identity()); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); + final PriceFloorData providerFloorsData = givenFloorData(floors -> floors.floorProvider("provider.com")); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -644,8 +348,8 @@ public void shouldReturnFloorsWithFloorMinAndCurrencyFromRequestWhenPresent() { .floorMin(BigDecimal.ONE) .data(givenFloorData(floorsDataConfig -> floorsDataConfig.currency("USD")))); - final PriceFloorData providerFloorsData = givenFloorData(identity()); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); + final PriceFloorData providerFloorsData = givenFloorData(floors -> floors.floorProvider("provider.com")); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -664,7 +368,7 @@ public void shouldReturnFloorsWithFloorMinAndCurrencyFromRequestWhenPresent() { @Test public void shouldUseFloorsFromRequestIfProviderFloorsMissing() { // given - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); + given(priceFloorFetcher.fetch(any())).willReturn(null); // when final BidRequest result = target.enrichWithPriceFloors( @@ -676,19 +380,17 @@ public void shouldUseFloorsFromRequestIfProviderFloorsMissing() { // then assertThat(extractFloors(result)).isEqualTo(givenFloors(floors -> floors - .fetchStatus(FetchStatus.none) - .enabled(true) - .skipped(false) - .floorMin(BigDecimal.ONE) - .location(PriceFloorLocation.request))); - verifyNoInteractions(metrics); + .enabled(true) + .skipped(false) + .floorMin(BigDecimal.ONE) + .location(PriceFloorLocation.request))); } @Test public void shouldTolerateUsingFloorsFromRequestWhenRulesNumberMoreThanMaxRulesNumber() { // given - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); - final ArrayList warnings = new ArrayList<>(); + given(priceFloorFetcher.fetch(any())).willReturn(null); + final ArrayList errors = new ArrayList<>(); // when final BidRequest result = target.enrichWithPriceFloors( @@ -702,27 +404,25 @@ public void shouldTolerateUsingFloorsFromRequestWhenRulesNumberMoreThanMaxRulesN )), givenAccount(floorConfigBuilder -> floorConfigBuilder.maxRules(1L)), "bidder", - new ArrayList<>(), - warnings); + errors, + new ArrayList<>()); // then assertThat(extractFloors(result)).isEqualTo(PriceFloorRules.builder() - .fetchStatus(FetchStatus.none) .enabled(true) .skipped(false) .location(PriceFloorLocation.noData) .build()); - assertThat(warnings).containsOnly("errorMessage. " - + "Following parsing of request price floors is failed: " + assertThat(errors).containsOnly("Failed to parse price floors from request, with a reason: " + "Price floor rules number 2 exceeded its maximum number 1"); } @Test public void shouldTolerateUsingFloorsFromRequestWhenDimensionsNumberMoreThanMaxDimensionsNumber() { // given - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); - final ArrayList warnings = new ArrayList<>(); + given(priceFloorFetcher.fetch(any())).willReturn(null); + final ArrayList errors = new ArrayList<>(); // when final BidRequest result = target.enrichWithPriceFloors( @@ -736,52 +436,24 @@ public void shouldTolerateUsingFloorsFromRequestWhenDimensionsNumberMoreThanMaxD )), givenAccount(floorConfigBuilder -> floorConfigBuilder.maxSchemaDims(1L)), "bidder", - new ArrayList<>(), - warnings); + errors, + new ArrayList<>()); // then assertThat(extractFloors(result)).isEqualTo(PriceFloorRules.builder() - .fetchStatus(FetchStatus.none) .enabled(true) .skipped(false) .location(PriceFloorLocation.noData) .build()); - assertThat(warnings).containsOnly("errorMessage. " - + "Following parsing of request price floors is failed: " + assertThat(errors).containsOnly("Failed to parse price floors from request, with a reason: " + "Price floor schema dimensions 2 exceeded its maximum number 1"); } - @Test - public void shouldTolerateInvalidFloorsFromRequestWhenFetchIsInProgress() { - // given - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.inProgress()); - final ArrayList warnings = new ArrayList<>(); - - // when - final BidRequest result = target.enrichWithPriceFloors( - givenBidRequest(identity(), givenFloors(floors -> floors.data(null))), - givenAccount(identity()), - "bidder", - new ArrayList<>(), - warnings); - - // then - assertThat(extractFloors(result)).isEqualTo(PriceFloorRules.builder() - .fetchStatus(FetchStatus.inprogress) - .enabled(true) - .skipped(false) - .location(PriceFloorLocation.noData) - .build()); - - assertThat(warnings).isEmpty(); - verifyNoInteractions(metrics); - } - @Test public void shouldTolerateMissingRequestAndProviderFloors() { // given - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); + given(priceFloorFetcher.fetch(any())).willReturn(null); // when final BidRequest result = target.enrichWithPriceFloors( @@ -803,9 +475,6 @@ public void shouldTolerateMissingRequestAndProviderFloors() { @Test public void shouldNotSkipFloorsIfRootSkipRateIsOff() { - // given - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); - // when final BidRequest result = target.enrichWithPriceFloors( givenBidRequest(identity(), givenFloors(floors -> floors.skipRate(0))), @@ -817,7 +486,6 @@ public void shouldNotSkipFloorsIfRootSkipRateIsOff() { // then assertThat(extractFloors(result)) .isEqualTo(givenFloors(floors -> floors - .fetchStatus(FetchStatus.none) .enabled(true) .skipped(false) .skipRate(0) @@ -826,9 +494,6 @@ public void shouldNotSkipFloorsIfRootSkipRateIsOff() { @Test public void shouldSkipFloorsIfRootSkipRateIsOn() { - // given - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); - // when final BidRequest result = target.enrichWithPriceFloors( givenBidRequest(identity(), givenFloors(floors -> floors.skipRate(100))), @@ -839,18 +504,16 @@ public void shouldSkipFloorsIfRootSkipRateIsOn() { // then assertThat(extractFloors(result)).isEqualTo(givenFloors(floors -> floors - .fetchStatus(FetchStatus.none) - .skipRate(100) - .enabled(true) - .skipped(true) - .location(PriceFloorLocation.request))); + .skipRate(100) + .enabled(true) + .skipped(true) + .location(PriceFloorLocation.request))); } @Test public void shouldSkipFloorsIfDataSkipRateIsOn() { // given final PriceFloorData priceFloorData = givenFloorData(floorData -> floorData.skipRate(100)); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); // when final BidRequest result = target.enrichWithPriceFloors( @@ -862,13 +525,11 @@ public void shouldSkipFloorsIfDataSkipRateIsOn() { // then assertThat(extractFloors(result)).isEqualTo(givenFloors(floors -> floors - .fetchStatus(FetchStatus.none) - .enabled(true) - .skipRate(100) - .floorProvider("provider.com") - .data(priceFloorData) - .skipped(true) - .location(PriceFloorLocation.request))); + .enabled(true) + .skipRate(100) + .data(priceFloorData) + .skipped(true) + .location(PriceFloorLocation.request))); } @Test @@ -877,7 +538,6 @@ public void shouldSkipFloorsIfModelGroupSkipRateIsOn() { final PriceFloorData priceFloorData = givenFloorData(floorData -> floorData .skipRate(0) .modelGroups(singletonList(givenModelGroup(group -> group.skipRate(100))))); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); // when final BidRequest result = target.enrichWithPriceFloors( @@ -889,13 +549,11 @@ public void shouldSkipFloorsIfModelGroupSkipRateIsOn() { // then assertThat(extractFloors(result)).isEqualTo(givenFloors(floors -> floors - .fetchStatus(FetchStatus.none) - .data(priceFloorData) - .skipRate(100) - .floorProvider("provider.com") - .enabled(true) - .skipped(true) - .location(PriceFloorLocation.request))); + .data(priceFloorData) + .skipRate(100) + .enabled(true) + .skipped(true) + .location(PriceFloorLocation.request))); } @Test @@ -904,7 +562,6 @@ public void shouldNotUpdateImpsIfSelectedModelGroupIsMissing() { final List imps = singletonList(givenImp(identity())); final PriceFloorRules requestFloors = givenFloors(floors -> floors .data(givenFloorData(floorData -> floorData.modelGroups(null)))); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); // when final BidRequest result = target.enrichWithPriceFloors( @@ -925,7 +582,6 @@ public void shouldUseSelectedModelGroup() { final PriceFloorModelGroup modelGroup = givenModelGroup(identity()); final PriceFloorRules requestFloors = givenFloors(floors -> floors .data(givenFloorData(floorData -> floorData.modelGroups(singletonList(modelGroup))))); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); // when target.enrichWithPriceFloors( @@ -947,8 +603,8 @@ public void shouldUseSelectedModelGroup() { @Test public void shouldCopyFloorProviderValueFromDataLevel() { // given - final PriceFloorData providerFloorsData = givenFloorData(identity()); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); + final PriceFloorData providerFloorsData = givenFloorData(floors -> floors.floorProvider("provider.com")); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -974,7 +630,6 @@ public void shouldNotUpdateImpsIfBidFloorNotResolved() { .data(givenFloorData(floorData -> floorData .modelGroups(singletonList(givenModelGroup(identity())))))); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); given(floorResolver.resolve(any(), any(), any(), eq("bidder"), any())).willReturn(null); // when @@ -1005,7 +660,6 @@ public void shouldUpdateImpsIfBidFloorResolved() { final List imps = singletonList(givenImp(impBuilder -> impBuilder.ext(givenImpExt))); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); given(floorResolver.resolve(any(), any(), any(), eq("bidder"), any())) .willReturn(PriceFloorResult.of("rule", BigDecimal.ONE, BigDecimal.TEN, "USD")); @@ -1038,7 +692,6 @@ public void shouldTolerateFloorResolvingError() { final PriceFloorRules requestFloors = givenFloors(floors -> floors .data(givenFloorData(floorData -> floorData.modelGroups(singletonList(givenModelGroup(identity())))))); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); given(floorResolver.resolve(any(), any(), any(), eq("bidder"), any())) .willThrow(new IllegalStateException("error")); @@ -1059,7 +712,6 @@ private static Account givenAccount( UnaryOperator floorsConfigCustomizer) { return Account.builder() - .id("accountId") .auction(AccountAuctionConfig.builder() .priceFloors(floorsConfigCustomizer.apply(AccountPriceFloorsConfig.builder()).build()) .build()) @@ -1070,7 +722,6 @@ private static BidRequest givenBidRequest(UnaryOperator floorDataCustomizer) { return floorDataCustomizer.apply(PriceFloorData.builder() - .floorProvider("provider.com") .modelGroups(singletonList(PriceFloorModelGroup.builder() .value("someKey", BigDecimal.ONE) .schema(PriceFloorSchema.of("|", List.of(size))) diff --git a/src/test/java/org/prebid/server/floors/PriceFloorFetcherTest.java b/src/test/java/org/prebid/server/floors/PriceFloorFetcherTest.java index 542559bd862..4fe2e31180b 100644 --- a/src/test/java/org/prebid/server/floors/PriceFloorFetcherTest.java +++ b/src/test/java/org/prebid/server/floors/PriceFloorFetcherTest.java @@ -104,8 +104,8 @@ public void fetchShouldReturnPriceFloorFetchedFromProviderAndCache() { verifyNoMoreInteractions(httpClient); final FetchResult priceFloorRulesCached = priceFloorFetcher.fetch(givenAccount); - assertThat(priceFloorRulesCached) - .isEqualTo(FetchResult.of(givenPriceFloorData(), FetchStatus.success, null)); + assertThat(priceFloorRulesCached.getFetchStatus()).isEqualTo(FetchStatus.success); + assertThat(priceFloorRulesCached.getRulesData()).isEqualTo(givenPriceFloorData()); } @@ -119,7 +119,8 @@ public void fetchShouldReturnEmptyRulesAndInProgressStatusForTheFirstInvocation( final FetchResult fetchResult = priceFloorFetcher.fetch(givenAccount(identity())); // then - assertThat(fetchResult).isEqualTo(FetchResult.inProgress()); + assertThat(fetchResult.getRulesData()).isNull(); + assertThat(fetchResult.getFetchStatus()).isEqualTo(FetchStatus.inprogress); verify(vertx).setTimer(eq(1200000L), any()); } @@ -133,13 +134,13 @@ public void fetchShouldReturnEmptyRulesAndInProgressStatusForTheFirstInvocationA final FetchResult firstInvocationResult = priceFloorFetcher.fetch(givenAccount(identity())); // then - assertThat(firstInvocationResult).isEqualTo(FetchResult.inProgress()); + assertThat(firstInvocationResult.getRulesData()).isNull(); + assertThat(firstInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.inprogress); verify(vertx).setTimer(eq(1200000L), any()); final FetchResult secondInvocationResult = priceFloorFetcher.fetch(givenAccount(identity())); - assertThat(secondInvocationResult).isEqualTo(FetchResult.error( - "Failed to fetch price floor from provider for fetch.url " - + "'http://test.host.com', with a reason: failed")); + assertThat(secondInvocationResult.getRulesData()).isNull(); + assertThat(secondInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.error); } @Test @@ -157,10 +158,8 @@ public void fetchShouldReturnEmptyRulesAndInProgressStatusForTheFirstInvocationA verify(vertx).setTimer(eq(1200000L), any()); final FetchResult secondInvocationResult = priceFloorFetcher.fetch(givenAccount(identity())); - assertThat(secondInvocationResult).isEqualTo(FetchResult.of( - null, - FetchStatus.timeout, - "Fetch price floor request timeout for fetch.url 'http://test.host.com' exceeded.")); + assertThat(secondInvocationResult.getRulesData()).isNull(); + assertThat(secondInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.timeout); } @Test @@ -318,8 +317,8 @@ public void fetchShouldNotPrepareAnyRequestsWhenFetchUrlIsMalformedAndReturnErro // then verifyNoInteractions(httpClient); - assertThat(fetchResult).isEqualTo(FetchResult.error( - "Malformed fetch.url 'MalformedURl' passed")); + assertThat(fetchResult.getRulesData()).isNull(); + assertThat(fetchResult.getFetchStatus()).isEqualTo(FetchStatus.error); verifyNoInteractions(vertx); } @@ -330,7 +329,8 @@ public void fetchShouldNotPrepareAnyRequestsWhenFetchUrlIsBlankAndReturnErrorSta // then verifyNoInteractions(httpClient); - assertThat(fetchResult).isEqualTo(FetchResult.error("Malformed fetch.url ' ' passed")); + assertThat(fetchResult.getRulesData()).isNull(); + assertThat(fetchResult.getFetchStatus()).isEqualTo(FetchStatus.error); verifyNoInteractions(vertx); } @@ -341,7 +341,8 @@ public void fetchShouldNotPrepareAnyRequestsWhenFetchUrlIsNotProvidedAndReturnEr // then verifyNoInteractions(httpClient); - assertThat(fetchResult).isEqualTo(FetchResult.error("Malformed fetch.url 'null' passed")); + assertThat(fetchResult.getRulesData()).isNull(); + assertThat(fetchResult.getFetchStatus()).isEqualTo(FetchStatus.error); verifyNoInteractions(vertx); } @@ -352,7 +353,8 @@ public void fetchShouldNotPrepareAnyRequestsWhenFetchEnabledIsFalseAndReturnNone // then verifyNoInteractions(httpClient); - assertThat(fetchResult).isEqualTo(FetchResult.none("Fetching is disabled")); + assertThat(fetchResult.getRulesData()).isNull(); + assertThat(fetchResult.getFetchStatus()).isEqualTo(FetchStatus.none); verifyNoInteractions(vertx); } @@ -368,13 +370,13 @@ public void fetchShouldReturnEmptyRulesAndErrorStatusForSecondCallAndCreatePerio // then verify(httpClient).get(anyString(), anyLong(), anyLong()); - assertThat(firstInvocationResult).isEqualTo(FetchResult.inProgress()); + assertThat(firstInvocationResult.getRulesData()).isNull(); + assertThat(firstInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.inprogress); verify(vertx).setTimer(eq(1200000L), any()); verify(vertx).setTimer(eq(1500000L), any()); final FetchResult secondInvocationResult = priceFloorFetcher.fetch(givenAccount(identity())); - assertThat(secondInvocationResult).isEqualTo(FetchResult.error( - "Failed to fetch price floor from provider for fetch.url 'http://test.host.com', " - + "with a reason: Failed to request, provider respond with status 400")); + assertThat(secondInvocationResult.getRulesData()).isNull(); + assertThat(secondInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.error); verifyNoMoreInteractions(vertx); } @@ -390,16 +392,13 @@ public void fetchShouldReturnEmptyRulesWithErrorStatusAndCreatePeriodicTimerWhen // then verify(httpClient).get(anyString(), anyLong(), anyLong()); - assertThat(firstInvocationResult).isEqualTo(FetchResult.inProgress()); + assertThat(firstInvocationResult.getRulesData()).isNull(); + assertThat(firstInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.inprogress); verify(vertx).setTimer(eq(1200000L), any()); verify(vertx).setTimer(eq(1500000L), any()); final FetchResult secondInvocationResult = priceFloorFetcher.fetch(givenAccount(identity())); assertThat(secondInvocationResult.getRulesData()).isNull(); assertThat(secondInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.error); - assertThat(secondInvocationResult.getErrorMessage()).startsWith( - "Failed to fetch price floor from provider for fetch.url 'http://test.host.com', " - + "with a reason: Failed to parse price floor response, " - + "cause: DecodeException: Failed to decode:"); verifyNoMoreInteractions(vertx); } @@ -415,14 +414,13 @@ public void fetchShouldReturnEmptyRulesWithErrorStatusForSecondCallAndCreatePeri // then verify(httpClient).get(anyString(), anyLong(), anyLong()); - assertThat(firstInvocationResult).isEqualTo(FetchResult.inProgress()); + assertThat(firstInvocationResult.getRulesData()).isNull(); + assertThat(firstInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.inprogress); verify(vertx).setTimer(eq(1200000L), any()); verify(vertx).setTimer(eq(1500000L), any()); final FetchResult secondInvocationResult = priceFloorFetcher.fetch(givenAccount(identity())); - assertThat(secondInvocationResult).isEqualTo(FetchResult.error( - "Failed to fetch price floor from provider for fetch.url 'http://test.host.com', " - + "with a reason: Failed to parse price floor response, " - + "response body can not be empty")); + assertThat(secondInvocationResult.getRulesData()).isNull(); + assertThat(secondInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.error); verifyNoMoreInteractions(vertx); } @@ -438,14 +436,13 @@ public void fetchShouldReturnEmptyRulesWithErrorStatusForSecondCallAndCreatePeri // then verify(httpClient).get(anyString(), anyLong(), anyLong()); - assertThat(firstInvocationResult).isEqualTo(FetchResult.inProgress()); + assertThat(firstInvocationResult.getRulesData()).isNull(); + assertThat(firstInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.inprogress); verify(vertx).setTimer(eq(1200000L), any()); verify(vertx).setTimer(eq(1500000L), any()); final FetchResult secondInvocationResult = priceFloorFetcher.fetch(givenAccount(identity())); - assertThat(secondInvocationResult).isEqualTo(FetchResult.error( - "Failed to fetch price floor from provider for fetch.url 'http://test.host.com', " - + "with a reason: Failed to parse price floor response, " - + "response body can not be empty")); + assertThat(secondInvocationResult.getRulesData()).isNull(); + assertThat(secondInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.error); verifyNoMoreInteractions(vertx); } @@ -463,7 +460,8 @@ public void fetchShouldNotCallPriceFloorProviderWhileFetchIsAlreadyInProgress() verify(httpClient).get(anyString(), anyLong(), anyLong()); verifyNoMoreInteractions(httpClient); - assertThat(secondFetch).isEqualTo(FetchResult.inProgress()); + assertThat(secondFetch.getRulesData()).isNull(); + assertThat(secondFetch.getFetchStatus()).isEqualTo(FetchStatus.inprogress); fetchPromise.tryComplete( HttpClientResponse.of(200, MultiMap.caseInsensitiveMultiMap() @@ -492,13 +490,13 @@ public void fetchShouldReturnNullAndCreatePeriodicTimerWhenResponseExceededRules // then verify(httpClient).get(anyString(), anyLong(), anyLong()); - assertThat(firstInvocationResult).isEqualTo(FetchResult.inProgress()); + assertThat(firstInvocationResult.getRulesData()).isNull(); + assertThat(firstInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.inprogress); verify(vertx).setTimer(eq(1200000L), any()); verify(vertx).setTimer(eq(1500000L), any()); final FetchResult secondInvocationResult = priceFloorFetcher.fetch(givenAccount(identity())); - assertThat(secondInvocationResult).isEqualTo(FetchResult.error( - "Failed to fetch price floor from provider for fetch.url 'http://test.host.com', " - + "with a reason: Price floor rules number 2 exceeded its maximum number 1")); + assertThat(secondInvocationResult.getRulesData()).isNull(); + assertThat(secondInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.error); verifyNoMoreInteractions(vertx); } @@ -520,13 +518,13 @@ public void fetchShouldReturnNullAndCreatePeriodicTimerWhenResponseExceededDimen // then verify(httpClient).get(anyString(), anyLong(), anyLong()); - assertThat(firstInvocationResult).isEqualTo(FetchResult.inProgress()); + assertThat(firstInvocationResult.getRulesData()).isNull(); + assertThat(firstInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.inprogress); verify(vertx).setTimer(eq(1200000L), any()); verify(vertx).setTimer(eq(1500000L), any()); final FetchResult secondInvocationResult = priceFloorFetcher.fetch(givenAccount(identity())); - assertThat(secondInvocationResult).isEqualTo(FetchResult.error( - "Failed to fetch price floor from provider for fetch.url 'http://test.host.com', " - + "with a reason: Price floor rules values can't be null or empty, but were {}")); + assertThat(secondInvocationResult.getRulesData()).isNull(); + assertThat(secondInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.error); verifyNoMoreInteractions(vertx); }