diff --git a/src/main/java/org/prebid/server/floors/BasicPriceFloorProcessor.java b/src/main/java/org/prebid/server/floors/BasicPriceFloorProcessor.java index d492981f568..8619a4b12a9 100644 --- a/src/main/java/org/prebid/server/floors/BasicPriceFloorProcessor.java +++ b/src/main/java/org/prebid/server/floors/BasicPriceFloorProcessor.java @@ -21,6 +21,8 @@ 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; @@ -52,17 +54,23 @@ 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, - JacksonMapper mapper) { + Metrics metrics, + JacksonMapper mapper, + double logSamplingRate) { 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); } @@ -82,7 +90,7 @@ public BidRequest enrichWithPriceFloors(BidRequest bidRequest, return disableFloorsForRequest(bidRequest); } - final PriceFloorRules floors = resolveFloors(account, bidRequest, errors); + final PriceFloorRules floors = resolveFloors(account, bidRequest, warnings); return updateBidRequestWithFloors(bidRequest, bidder, floors, errors, warnings); } @@ -122,49 +130,13 @@ private static PriceFloorRules extractRequestFloors(BidRequest bidRequest) { return ObjectUtil.getIfNotNull(prebid, ExtRequestPrebid::getFloors); } - private PriceFloorRules resolveFloors(Account account, BidRequest bidRequest, List errors) { + private PriceFloorRules resolveFloors(Account account, BidRequest bidRequest, List warnings) { final PriceFloorRules requestFloors = extractRequestFloors(bidRequest); final FetchResult fetchResult = floorFetcher.fetch(account); - final FetchStatus fetchStatus = ObjectUtil.getIfNotNull(fetchResult, FetchResult::getFetchStatus); + final FetchStatus fetchStatus = fetchResult.getFetchStatus(); - 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) + final boolean isUsingDynamicDataAllowed = Optional.ofNullable(account.getAuction()) .map(AccountAuctionConfig::getPriceFloors) .map(AccountPriceFloorsConfig::getUseDynamicData) .map(BooleanUtils::isNotFalse) @@ -175,12 +147,73 @@ private static boolean shouldUseDynamicData(Account account, FetchResult fetchRe .map(rate -> ThreadLocalRandom.current().nextInt(USE_FETCH_DATA_RATE_MAX) < rate) .orElse(true); - return isUsingDynamicDataAllowed && shouldUseDynamicData; + 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); } - private PriceFloorRules mergeFloors(PriceFloorRules requestFloors, - PriceFloorData providerRulesData) { + 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) { 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 b7d4ac4185f..7f49b1c09a7 100644 --- a/src/main/java/org/prebid/server/floors/PriceFloorFetcher.java +++ b/src/main/java/org/prebid/server/floors/PriceFloorFetcher.java @@ -90,7 +90,10 @@ public FetchResult fetch(Account account) { final AccountFetchContext accountFetchContext = fetchedData.get(account.getId()); return accountFetchContext != null - ? FetchResult.of(accountFetchContext.getRulesData(), accountFetchContext.getFetchStatus()) + ? FetchResult.of( + accountFetchContext.getRulesData(), + accountFetchContext.getFetchStatus(), + accountFetchContext.getErrorMessage()) : fetchPriceFloorData(account); } @@ -99,20 +102,19 @@ private FetchResult fetchPriceFloorData(Account account) { final Boolean fetchEnabled = ObjectUtil.getIfNotNull(fetchConfig, AccountPriceFloorsFetchConfig::getEnabled); if (BooleanUtils.isFalse(fetchEnabled)) { - return FetchResult.of(null, FetchStatus.none); + return FetchResult.none("Fetching is disabled"); } final String accountId = account.getId(); final String fetchUrl = ObjectUtil.getIfNotNull(fetchConfig, AccountPriceFloorsFetchConfig::getUrl); if (!isUrlValid(fetchUrl)) { - logger.error("Malformed fetch.url: '%s', passed for account %s".formatted(fetchUrl, accountId)); - return FetchResult.of(null, FetchStatus.error); + return FetchResult.error("Malformed fetch.url '%s' passed".formatted(fetchUrl)); } if (!fetchInProgress.contains(accountId)) { fetchPriceFloorDataAsynchronous(fetchConfig, accountId); } - return FetchResult.of(null, FetchStatus.inprogress); + return FetchResult.inProgress(); } private boolean isUrlValid(String url) { @@ -148,8 +150,8 @@ private void fetchPriceFloorDataAsynchronous(AccountPriceFloorsFetchConfig fetch fetchInProgress.add(accountId); httpClient.get(fetchUrl, timeout, resolveMaxFileSize(maxFetchFileSizeKb)) - .map(httpClientResponse -> parseFloorResponse(httpClientResponse, fetchConfig, accountId)) - .recover(throwable -> recoverFromFailedFetching(throwable, fetchUrl, accountId)) + .map(httpClientResponse -> parseFloorResponse(httpClientResponse, fetchConfig)) + .recover(throwable -> recoverFromFailedFetching(throwable, fetchUrl)) .map(cacheInfo -> updateCache(cacheInfo, fetchConfig, accountId)) .map(priceFloorData -> createPeriodicTimerForRulesFetch(priceFloorData, fetchConfig, accountId)); } @@ -159,23 +161,20 @@ private static long resolveMaxFileSize(Long maxSizeInKBytes) { } private ResponseCacheInfo parseFloorResponse(HttpClientResponse httpClientResponse, - AccountPriceFloorsFetchConfig fetchConfig, - String accountId) { + AccountPriceFloorsFetchConfig fetchConfig) { final int statusCode = httpClientResponse.getStatusCode(); if (statusCode != HttpStatus.SC_OK) { - throw new PreBidException("Failed to request for account %s, provider respond with status %s" - .formatted(accountId, statusCode)); + throw new PreBidException("Failed to request, provider respond with status %s".formatted(statusCode)); } final String body = httpClientResponse.getBody(); if (StringUtils.isBlank(body)) { - throw new PreBidException( - "Failed to parse price floor response for account %s, response body can not be empty" - .formatted(accountId)); + throw new PreBidException("Failed to parse price floor response, response body can not be empty"); } - final PriceFloorData priceFloorData = parsePriceFloorData(body, accountId); + final PriceFloorData priceFloorData = parsePriceFloorData(body); + PriceFloorRulesValidator.validateRulesData( priceFloorData, PriceFloorsConfigResolver.resolveMaxValue(fetchConfig.getMaxRules()), @@ -183,16 +182,17 @@ private ResponseCacheInfo parseFloorResponse(HttpClientResponse httpClientRespon return ResponseCacheInfo.of(priceFloorData, FetchStatus.success, + null, cacheTtlFromResponse(httpClientResponse, fetchConfig.getUrl())); } - private PriceFloorData parsePriceFloorData(String body, String accountId) { + private PriceFloorData parsePriceFloorData(String body) { final PriceFloorData priceFloorData; try { priceFloorData = mapper.decodeValue(body, PriceFloorData.class); } catch (DecodeException e) { - throw new PreBidException("Failed to parse price floor response for account %s, cause: %s" - .formatted(accountId, ExceptionUtils.getMessage(e))); + throw new PreBidException( + "Failed to parse price floor response, cause: %s".formatted(ExceptionUtils.getMessage(e))); } return priceFloorData; } @@ -220,8 +220,11 @@ private PriceFloorData updateCache(ResponseCacheInfo cacheInfo, String accountId) { final long maxAgeTimerId = createMaxAgeTimer(accountId, resolveCacheTtl(cacheInfo, fetchConfig)); - final AccountFetchContext fetchContext = - AccountFetchContext.of(cacheInfo.getRulesData(), cacheInfo.getFetchStatus(), maxAgeTimerId); + final AccountFetchContext fetchContext = AccountFetchContext.of( + cacheInfo.getRulesData(), + cacheInfo.getFetchStatus(), + cacheInfo.getErrorMessage(), + maxAgeTimerId); if (cacheInfo.getFetchStatus() == FetchStatus.success || !fetchedData.containsKey(accountId)) { fetchedData.put(accountId, fetchContext); @@ -267,30 +270,27 @@ 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, - String accountId) { - + private Future recoverFromFailedFetching(Throwable throwable, String fetchUrl) { metrics.updatePriceFloorFetchMetric(MetricName.failure); final FetchStatus fetchStatus; + final String errorMessage; if (throwable instanceof TimeoutException || throwable instanceof ConnectTimeoutException) { fetchStatus = FetchStatus.timeout; - logger.error("Fetch price floor request timeout for fetch.url: '%s', account %s exceeded." - .formatted(fetchUrl, accountId)); + errorMessage = "Fetch price floor request timeout for fetch.url '%s' exceeded.".formatted(fetchUrl); } else { fetchStatus = FetchStatus.error; - logger.error( - "Failed to fetch price floor from provider for fetch.url: '%s', account = %s with a reason : %s " - .formatted(fetchUrl, accountId, throwable.getMessage())); + errorMessage = "Failed to fetch price floor from provider for fetch.url '%s', with a reason: %s" + .formatted(fetchUrl, throwable.getMessage()); } - return Future.succeededFuture(ResponseCacheInfo.withStatus(fetchStatus)); + return Future.succeededFuture(ResponseCacheInfo.withError(fetchStatus, errorMessage)); } private PriceFloorData createPeriodicTimerForRulesFetch(PriceFloorData priceFloorData, AccountPriceFloorsFetchConfig fetchConfig, String accountId) { + final long accountPeriodicTimeSec = ObjectUtil.getIfNotNull(fetchConfig, AccountPriceFloorsFetchConfig::getPeriodSec); final long periodicTimeSec = @@ -318,6 +318,8 @@ private static class AccountFetchContext { FetchStatus fetchStatus; + String errorMessage; + Long maxAgeTimerId; } @@ -328,10 +330,12 @@ private static class ResponseCacheInfo { FetchStatus fetchStatus; + String errorMessage; + Long cacheTtl; - public static ResponseCacheInfo withStatus(FetchStatus status) { - return ResponseCacheInfo.of(null, status, null); + public static ResponseCacheInfo withError(FetchStatus status, String errorMessage) { + return ResponseCacheInfo.of(null, status, errorMessage, 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 36c4fda58e0..336bb26ee43 100644 --- a/src/main/java/org/prebid/server/floors/proto/FetchResult.java +++ b/src/main/java/org/prebid/server/floors/proto/FetchResult.java @@ -9,4 +9,18 @@ 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 8a483e92a4d..16a79d6c0f6 100644 --- a/src/main/java/org/prebid/server/spring/config/PriceFloorsConfiguration.java +++ b/src/main/java/org/prebid/server/spring/config/PriceFloorsConfiguration.java @@ -20,6 +20,7 @@ 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; @@ -84,9 +85,11 @@ PriceFloorResolver noOpPriceFloorResolver() { @ConditionalOnProperty(prefix = "price-floors", name = "enabled", havingValue = "true") PriceFloorProcessor basicPriceFloorProcessor(PriceFloorFetcher floorFetcher, PriceFloorResolver floorResolver, - JacksonMapper mapper) { + Metrics metrics, + JacksonMapper mapper, + @Value("${logging.sampling-rate:0.01}") double logSamplingRate) { - return new BasicPriceFloorProcessor(floorFetcher, floorResolver, mapper); + return new BasicPriceFloorProcessor(floorFetcher, floorResolver, metrics, mapper, logSamplingRate); } @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 63ba2516ff4..a4fda13f829 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/BaseSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/BaseSpec.groovy @@ -40,6 +40,7 @@ abstract class BaseSpec extends Specification implements ObjectMapperWrapper { private static final int MIN_TIMEOUT = DEFAULT_TIMEOUT private static final int DEFAULT_TARGETING_PRECISION = 1 private static final String DEFAULT_CACHE_DIRECTORY = "/app/prebid-server/data" + protected static final String ALERT_GENERAL = "alerts.general" protected static final Map GENERIC_ALIAS_CONFIG = ["adapters.generic.aliases.alias.enabled" : "true", "adapters.generic.aliases.alias.endpoint": "$networkServiceContainer.rootUri/auction".toString()] diff --git a/src/test/groovy/org/prebid/server/functional/tests/BidValidationSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/BidValidationSpec.groovy index 579a8e16597..9dd17c31e0a 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/BidValidationSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/BidValidationSpec.groovy @@ -63,7 +63,7 @@ class BidValidationSpec extends BaseSpec { and: "Bid validation metric value is incremented" def metrics = strictPrebidService.sendCollectedMetricsRequest() - assert metrics["alerts.general"] == 1 + assert metrics[ALERT_GENERAL] == 1 where: bidRequest << [BidRequest.getDefaultBidRequest(DistributionChannel.APP).tap { @@ -105,7 +105,7 @@ class BidValidationSpec extends BaseSpec { and: "Bid validation metric value is incremented" def metrics = softPrebidService.sendCollectedMetricsRequest() - assert metrics["alerts.general"] == 1 + assert metrics[ALERT_GENERAL] == 1 and: "PBS log should contain message" def logs = softPrebidService.getLogsByTime(startTime) diff --git a/src/test/groovy/org/prebid/server/functional/tests/MetricsSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/MetricsSpec.groovy index 92ef175681e..ee3e808a56e 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/MetricsSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/MetricsSpec.groovy @@ -146,7 +146,7 @@ class MetricsSpec extends BaseSpec { assert metrics["adapter.generic.requests.type.openrtb2-dooh" as String] == 1 and: "alert.general metric should be updated" - assert metrics["alerts.general" as String] == 1 + assert metrics[ALERT_GENERAL] == 1 and: "Other channel types should not be populated" assert !metrics["account.${accountId}.requests.type.openrtb2-web" as String] @@ -175,7 +175,7 @@ class MetricsSpec extends BaseSpec { assert metrics["adapter.generic.requests.type.openrtb2-app" as String] == 1 and: "alert.general metric should be updated" - assert metrics["alerts.general" as String] == 1 + assert metrics[ALERT_GENERAL] == 1 and: "Other channel types should not be populated" assert !metrics["account.${accountId}.requests.type.openrtb2-dooh" as String] diff --git a/src/test/groovy/org/prebid/server/functional/tests/bidder/openx/OpenxSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/bidder/openx/OpenxSpec.groovy index 8e6d7f1a57d..97a0015cea5 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/bidder/openx/OpenxSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/bidder/openx/OpenxSpec.groovy @@ -444,7 +444,7 @@ class OpenxSpec extends BaseSpec { and: "Alert.general metric should be updated" def metrics = pbsService.sendCollectedMetricsRequest() - assert metrics["alerts.general" as String] == 1 + assert metrics[ALERT_GENERAL] == 1 } def "PBS shouldn't populate fledge or igi config when bidder respond with igb"() { diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsBaseSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsBaseSpec.groovy index 8b3f5d936bd..4e1cbbf349b 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsBaseSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsBaseSpec.groovy @@ -15,6 +15,7 @@ import org.prebid.server.functional.model.request.auction.BidRequest import org.prebid.server.functional.model.request.auction.BidRequestExt import org.prebid.server.functional.model.request.auction.DistributionChannel import org.prebid.server.functional.model.request.auction.ExtPrebidFloors +import org.prebid.server.functional.model.request.auction.FetchStatus import org.prebid.server.functional.model.request.auction.Prebid import org.prebid.server.functional.model.request.auction.Video import org.prebid.server.functional.model.response.currencyrates.CurrencyRatesResponse @@ -34,8 +35,7 @@ abstract class PriceFloorsBaseSpec extends BaseSpec { public static final BigDecimal FLOOR_MIN = 0.5 public static final BigDecimal FLOOR_MAX = 2 - public static final Map FLOORS_CONFIG = ["price-floors.enabled" : "true", - "settings.default-account-config": encode(defaultAccountConfigSettings)] + public static final Map FLOORS_CONFIG = ["price-floors.enabled": "true"] protected static final String BASIC_FETCH_URL = networkServiceContainer.rootUri + FloorsProvider.FLOORS_ENDPOINT protected static final int MAX_MODEL_WEIGHT = 100 @@ -121,8 +121,9 @@ abstract class PriceFloorsBaseSpec extends BaseSpec { protected void cacheFloorsProviderRules(BidRequest bidRequest, PrebidServerService pbsService = floorsPbsService, - BidderName bidderName = BidderName.GENERIC) { - PBSUtils.waitUntil({ getRequests(pbsService.sendAuctionRequest(bidRequest))[bidderName.value]?.first?.ext?.prebid?.floors?.fetchStatus != INPROGRESS }, + BidderName bidderName = BidderName.GENERIC, + FetchStatus fetchStatus = INPROGRESS) { + PBSUtils.waitUntil({ getRequests(pbsService.sendAuctionRequest(bidRequest))[bidderName.value]?.first?.ext?.prebid?.floors?.fetchStatus != fetchStatus }, 5000, 1000) } diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy index a897fbeff7c..699b9bdcda0 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy @@ -17,6 +17,7 @@ import java.time.Instant import static org.mockserver.model.HttpStatusCode.BAD_REQUEST_400 import static org.prebid.server.functional.model.Currency.EUR import static org.prebid.server.functional.model.Currency.JPY +import static org.prebid.server.functional.model.bidder.BidderName.GENERIC import static org.prebid.server.functional.model.pricefloors.Country.MULTIPLE import static org.prebid.server.functional.model.pricefloors.MediaType.BANNER import static org.prebid.server.functional.model.request.auction.DistributionChannel.APP @@ -43,8 +44,24 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { private static final int DEFAULT_FLOOR_VALUE_MIN = 0 private static final int FLOOR_MIN = 0 - private static final Closure INVALID_CONFIG_METRIC = { account -> "alerts.account_config.${account}.price-floors" } private static final String FETCH_FAILURE_METRIC = "price-floors.fetch.failure" + private static final 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" + } def "PBS should activate floors feature when price-floors.enabled = true in PBS config"() { given: "Pbs with PF configuration" @@ -121,7 +138,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def logs = floorsPbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, bidRequest.accountId) assert floorsLogs.size() == 1 - assert floorsLogs.first().contains("Malformed fetch.url: 'null', passed for account $bidRequest.accountId") + assert floorsLogs.first().contains("No price floor data for account $bidRequest.accountId and request $bidRequest.id, " + + "reason: Malformed fetch.url 'null' passed") and: "PBS floors validation failure should not reject the entire auction" assert !response.seatbid.isEmpty() @@ -545,7 +563,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(bidRequest.accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -554,14 +572,15 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def metrics = floorsPbsService.sendCollectedMetricsRequest() assert metrics[FETCH_FAILURE_METRIC] == 1 - then: "PBS should fetch data" + and: "PBS should fetch data" assert floorsProvider.getRequestCount(bidRequest.accountId) == 1 and: "PBS log should contain error" def logs = floorsPbsService.getLogsByTime(startTime) + def message = "Price floor data useFetchDataRate must be in range(0-100), but was $accounntUseFetchDataRate" def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains("reason : Price floor data useFetchDataRate must be in range(0-100), but was $accounntUseFetchDataRate") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -645,7 +664,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, BAD_REQUEST_400) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest, floorsPbsService) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -657,12 +676,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Failed to request, provider respond with status 400" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL) + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains("Failed to fetch price floor from provider for fetch.url: " + - "'$BASIC_FETCH_URL$accountId', account = $accountId with a reason : Failed to request for " + - "account $accountId, provider respond with status 400") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -687,6 +705,9 @@ 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) @@ -697,12 +718,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Failed to parse price floor response, cause: DecodeException: Failed to decode" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$accountId") + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains("Failed to fetch price floor from provider for fetch.url: " + - "'$BASIC_FETCH_URL$accountId', account = $accountId with a reason : Failed to parse price floor " + - "response for account $accountId, cause: DecodeException: Failed to decode") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -726,6 +746,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "Set Floors Provider response with invalid json" floorsProvider.setResponse(accountId, "") + and: "PBS fetch rules from floors provider" + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) + when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -736,12 +759,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Failed to parse price floor response, response body can not be empty" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains("Failed to fetch price floor from provider for fetch.url: " + - "'$BASIC_FETCH_URL$accountId', account = $accountId with a reason : Failed to parse price floor " + - "response for account $accountId, response body can not be empty" as String) + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -768,6 +790,9 @@ 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) @@ -778,12 +803,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Price floor rules should contain at least one model group" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains("Failed to fetch price floor from provider for fetch.url: " + - "'$BASIC_FETCH_URL$accountId', account = $accountId with a reason : Price floor rules should contain " + - "at least one model group " as String) + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -810,6 +834,9 @@ 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) @@ -821,11 +848,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "PBS log should contain error" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains("Failed to fetch price floor from provider for fetch.url: " + - "'$BASIC_FETCH_URL$accountId', account = $accountId with a reason : Price floor rules values can't " + - "be null or empty, but were null" as String) + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, PRICE_FLOOR_VALUES_MISSING)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -855,6 +880,9 @@ 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) @@ -865,12 +893,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Price floor rules number 2 exceeded its maximum number $maxRules" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains("Failed to fetch price floor from provider for fetch.url: " + - "'$BASIC_FETCH_URL$accountId', account = $accountId with a reason : Price floor rules number " + - "2 exceeded its maximum number $maxRules") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -901,6 +928,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "Set Floors Provider response with timeout" floorsProvider.setResponseWithTimeout(accountId) + and: "PBS fetch rules from floors provider" + cacheFloorsProviderRules(bidRequest, pbsService, GENERIC, NONE) + when: "PBS processes auction request" def response = pbsService.sendAuctionRequest(bidRequest) @@ -914,8 +944,8 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def logs = pbsService.getLogsByTime(startTime) def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains("Fetch price floor request timeout for fetch.url: '$BASIC_FETCH_URL$accountId', " + - "account $accountId exceeded") + assert floorsLogs[0].contains("No price floor data for account $bidRequest.accountId and request $bidRequest.id, " + + "reason: Fetch price floor request timeout for fetch.url '$BASIC_FETCH_URL$accountId' exceeded") and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -944,6 +974,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def responseSize = convertKilobyteSizeToByte(maxSize) + 75 floorsProvider.setResponse(accountId, floorsResponse, ["Content-Length": responseSize as String]) + and: "PBS fetch rules from floors provider" + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) + when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -954,12 +987,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Response size $responseSize exceeded ${convertKilobyteSizeToByte(maxSize)} bytes limit" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains("Failed to fetch price floor from provider for fetch.url: " + - "'$BASIC_FETCH_URL$accountId', account = $accountId with a reason : Response size " + - "$responseSize exceeded ${convertKilobyteSizeToByte(maxSize)} bytes limit") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -1284,11 +1316,10 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: Price floor floorMin " + - "must be positive float, but was $invalidFloorMin"] + 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)] } def "PBS should validate rules from request when request doesn't contain modelGroups"() { @@ -1312,11 +1343,10 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: Price floor rules " + - "should contain at least one model group"] + 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)] } def "PBS should validate rules from request when request doesn't contain values"() { @@ -1340,11 +1370,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: Price floor rules values " + - "can't be null or empty, but were null"] + and: "Response should contain warning" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(PRICE_FLOOR_VALUES_MISSING)] } def "PBS should validate rules from request when modelWeight from request is invalid"() { @@ -1372,11 +1400,10 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequest(bidRequest.id) assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: Price floor modelGroup modelWeight " + - "must be in range(1-100), but was $invalidModelWeight"] + 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))] + where: invalidModelWeight << [0, MAX_MODEL_WEIGHT + 1] } @@ -1411,11 +1438,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(ampStoredRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: Price floor modelGroup modelWeight " + - "must be in range(1-100), but was $invalidModelWeight"] + 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))] where: invalidModelWeight << [0, MAX_MODEL_WEIGHT + 1] @@ -1451,11 +1476,10 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: Price floor root skipRate " + - "must be in range(0-100), but was $invalidSkipRate"] + 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)] where: invalidSkipRate << [SKIP_RATE_MIN - 1, SKIP_RATE_MAX + 1] @@ -1491,11 +1515,10 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: Price floor data skipRate " + - "must be in range(0-100), but was $invalidSkipRate"] + 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)] where: invalidSkipRate << [SKIP_RATE_MIN - 1, SKIP_RATE_MAX + 1] @@ -1531,11 +1554,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: Price floor modelGroup skipRate " + - "must be in range(0-100), but was $invalidSkipRate"] + 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))] where: invalidSkipRate << [SKIP_RATE_MIN - 1, SKIP_RATE_MAX + 1] @@ -1567,11 +1588,10 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { def bidderRequest = bidder.getBidderRequests(bidRequest.id).last() assert bidderRequest.imp[0].bidFloor == floorValue - and: "Response should contain error" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: Price floor modelGroup default " + - "must be positive float, but was $invalidDefaultFloorValue"] + 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)] } def "PBS should not invalidate previously good fetched data when floors provider return invalid data"() { @@ -1651,7 +1671,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -1669,11 +1689,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "PBS log should contain error" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL) + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains("Failed to fetch price floor from provider for fetch.url: " + - "'$BASIC_FETCH_URL$accountId', account = $accountId with a reason : Price floor modelGroup modelWeight" + - " must be in range(1-100), but was $invalidModelWeight") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, MODEL_WEIGHT_INVALID.formatted(invalidModelWeight))) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -1710,7 +1728,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -1727,12 +1745,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Price floor data skipRate must be in range(0-100), but was $invalidSkipRate" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$accountId") + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains("Failed to fetch price floor from provider for fetch.url: " + - "'$BASIC_FETCH_URL$accountId', account = $accountId with a reason : Price floor data skipRate" + - " must be in range(0-100), but was $invalidSkipRate") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -1769,7 +1786,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -1787,11 +1804,9 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { and: "PBS log should contain error" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$accountId") + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains("Failed to fetch price floor from provider for fetch.url: " + - "'$BASIC_FETCH_URL$accountId', account = $accountId with a reason : Price floor modelGroup skipRate" + - " must be in range(0-100), but was $invalidSkipRate") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, SKIP_RATE_INVALID.formatted(invalidSkipRate))) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() @@ -1828,7 +1843,7 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { floorsProvider.setResponse(accountId, floorsResponse) and: "PBS fetch rules from floors provider" - cacheFloorsProviderRules(bidRequest) + cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, NONE) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) @@ -1845,12 +1860,11 @@ class PriceFloorsFetchingSpec extends PriceFloorsBaseSpec { assert metrics[FETCH_FAILURE_METRIC] == 1 and: "PBS log should contain error" + def message = "Price floor modelGroup default must be positive float, but was $invalidDefaultFloor" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$accountId") + def floorsLogs = getLogsByText(logs, "$BASIC_FETCH_URL$bidRequest.accountId") assert floorsLogs.size() == 1 - assert floorsLogs[0].contains("Failed to fetch price floor from provider for fetch.url: " + - "'$BASIC_FETCH_URL$accountId', account = $accountId with a reason : Price floor modelGroup default" + - " must be positive float, but was $invalidDefaultFloor") + assert floorsLogs[0].contains(URL_EMPTY_ERROR_LOG(bidRequest, message)) and: "Floors validation failure cannot reject the entire auction" assert !response.seatbid?.isEmpty() diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsRulesSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsRulesSpec.groovy index ee861a24587..30acd9b4bdc 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsRulesSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsRulesSpec.groovy @@ -61,6 +61,7 @@ import static org.prebid.server.functional.model.request.auction.FetchStatus.ERR import static org.prebid.server.functional.model.request.auction.Location.NO_DATA import static org.prebid.server.functional.model.request.auction.Prebid.Channel import static org.prebid.server.functional.model.response.auction.BidRejectionReason.RESPONSE_REJECTED_DUE_TO_PRICE_FLOOR +import static org.prebid.server.functional.model.response.auction.ErrorType.PREBID import static org.prebid.server.functional.testcontainers.Dependencies.getNetworkServiceContainer class PriceFloorsRulesSpec extends PriceFloorsBaseSpec { @@ -217,10 +218,15 @@ class PriceFloorsRulesSpec extends PriceFloorsBaseSpec { assert bidderRequest.ext?.prebid?.floors?.location == NO_DATA assert bidderRequest.ext?.prebid?.floors?.fetchStatus == ERROR - and: "PBS should not contain errors, warnings" - assert !response.ext?.warnings + and: "PBS should not contain errors" assert !response.ext?.errors + and: "PBS should log a warning" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message.first.contains("Cannot deserialize value of type " + + "`org.prebid.server.floors.model.PriceFloorField` " + + "from String \"bogus\": not one of the values accepted for Enum class") + and: "PBS should not reject the entire auction" assert !response.seatbid.isEmpty() } diff --git a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy index 72be2f0a0f3..1de08b44cc9 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy @@ -1,5 +1,9 @@ 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 @@ -29,13 +33,30 @@ 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" @@ -280,7 +301,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) - then: "PBS should not log warning" + then: "PBS should not log warning or errors" assert !response.ext.warnings assert !response.ext.errors @@ -319,8 +340,9 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { when: "PBS processes amp request" def response = floorsPbsService.sendAmpRequest(ampRequest) - then: "PBS should not log warning" + then: "PBS should not log warning or errors" assert !response.ext.warnings + assert !response.ext.errors and: "Bidder request should contain bidFloor from the request" def bidderRequest = bidder.getBidderRequest(ampStoredRequest.id) @@ -520,7 +542,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { assert bidderRequest.imp.last().bidFloor == videoFloorValue } - def "PBS shouldn't emit errors when request schema.fields than floor-config.max-schema-dims"() { + def "PBS shouldn't emit warning when request schema.fields equal to floor-config.max-schema-dims"() { given: "Bid request with schema 2 fields" def bidRequest = bidRequestWithFloors.tap { ext.prebid.floors.maxSchemaDims = PBSUtils.getRandomNumber(2) @@ -531,18 +553,19 @@ 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 shouldn't log a errors" - assert !response.ext?.errors + then: "PBS should not add warning or errors" + assert !response.ext.warnings + assert !response.ext.errors + + and: "Alerts.general metrics shouldn't be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert !metrics[ALERT_GENERAL] } - def "PBS should emit errors when request has more rules than price-floor.max-rules"() { + def "PBS should emit warning when request has more rules than price-floor.max-rules"() { given: "BidRequest with 2 rules" def requestFloorValue = PBSUtils.randomFloorValue def bidRequest = bidRequestWithFloors.tap { @@ -551,9 +574,10 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { (new Rule(mediaType: BANNER, country: Country.MULTIPLE).rule): requestFloorValue] } - and: "Account with maxRules in the DB" + and: "Account with maxRules and disabled fetching in the DB" def accountId = bidRequest.site.publisher.id def account = getAccountWithEnabledFetch(accountId).tap { + config.auction.priceFloors.fetch.enabled = false config.auction.priceFloors.maxRules = maxRules config.auction.priceFloors.maxRulesSnakeCase = maxRulesSnakeCase } @@ -564,14 +588,20 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { bidResponse.seatbid.first().bid.first().price = requestFloorValue bidder.setResponse(bidRequest.id, bidResponse) + and: "Flush metrics" + flushMetrics(floorsPbsService) + when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) - then: "PBS should log a errors" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: " + - "Price floor rules number ${getRuleSize(bidRequest)} exceeded its maximum number ${MAX_RULES_SIZE}"] + 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 where: maxRules | maxRulesSnakeCase @@ -579,30 +609,36 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { null | MAX_RULES_SIZE } - def "PBS should emit errors when request has more schema.fields than floor-config.max-schema-dims"() { + def "PBS should emit warning when request has more schema.fields than floor-config.max-schema-dims"() { given: "BidRequest with schema 2 fields" def bidRequest = bidRequestWithFloors - and: "Account with maxSchemaDims in the DB" + and: "Account with maxSchemaDims and disabled fetching in the DB" def accountId = bidRequest.site.publisher.id def account = getAccountWithEnabledFetch(accountId).tap { + config.auction.priceFloors.fetch.enabled = false config.auction.priceFloors.maxSchemaDims = maxSchemaDims config.auction.priceFloors.maxSchemaDimsSnakeCase = maxSchemaDimsSnakeCase } accountDao.save(account) - and: "Set bidder response" - def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) - bidder.setResponse(bidRequest.id, bidResponse) + and: "PBS fetch rules from floors provider" + cacheFloorsProviderRules(bidRequest, floorsPbsService) + + and: "Flush metrics" + flushMetrics(floorsPbsService) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) - then: "PBS should log a errors" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: " + - "Price floor schema dimensions ${getSchemaSize(bidRequest)} exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}"] + then: "PBS should log a warning" + def message = "Price floor schema dimensions ${getSchemaSize(bidRequest)} exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + + and: "Alerts.general metrics should be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert metrics[ALERT_GENERAL] == 1 where: maxSchemaDims | maxSchemaDimsSnakeCase @@ -610,7 +646,7 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { null | MAX_SCHEMA_DIMENSIONS_SIZE } - def "PBS should emit errors when request has more schema.fields than default-account.max-schema-dims"() { + def "PBS should emit warning when request has more schema.fields than default-account.max-schema-dims"() { given: "Floor config with default account" def accountConfig = getDefaultAccountConfigSettings().tap { auction.priceFloors.maxSchemaDims = MAX_SCHEMA_DIMENSIONS_SIZE @@ -627,45 +663,46 @@ 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: "Set bidder response" - def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) - bidder.setResponse(bidRequest.id, bidResponse) + and: "Flush metrics" + flushMetrics(floorsPbsService) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) - then: "PBS should log a errors" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: " + - "Price floor schema dimensions ${getSchemaSize(bidRequest)} " + - "exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}"] + then: "PBS should log a warning" + def message = "Price floor schema dimensions ${getSchemaSize(bidRequest)} exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + + and: "PBS should log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) + assert floorsLogs.size() == 1 - and: "Metric alerts.account_config.ACCOUNT.price-floors should be update" + and: "Metrics should be updated" def metrics = floorsPbsService.sendCollectedMetricsRequest() assert metrics[INVALID_CONFIG_METRIC(bidRequest.accountId) as String] == 1 + assert metrics[ALERT_GENERAL] == 1 cleanup: "Stop and remove pbs container" pbsServiceFactory.removeContainer(pbsFloorConfig) } - def "PBS should emit errors when request has more schema.fields than default-account.fetch.max-schema-dims"() { - given: "Test start time" - def startTime = Instant.now() - - and: "BidRequest with schema 2 fields" + def "PBS should emit warning when request has more schema.fields than default-account.fetch.max-schema-dims"() { + given: "BidRequest with schema 2 fields" def bidRequest = bidRequestWithFloors and: "Floor config with default account" def accountConfig = getDefaultAccountConfigSettings().tap { - auction.priceFloors.fetch.enabled = true + auction.priceFloors.fetch.enabled = false auction.priceFloors.fetch.url = BASIC_FETCH_URL + bidRequest.site.publisher.id auction.priceFloors.fetch.maxSchemaDims = MAX_SCHEMA_DIMENSIONS_SIZE - auction.priceFloors.maxSchemaDims = null + auction.priceFloors.maxSchemaDims = MAX_SCHEMA_DIMENSIONS_SIZE } def pbsFloorConfig = GENERIC_ALIAS_CONFIG + ["price-floors.enabled" : "true", "settings.default-account-config": encode(accountConfig)] @@ -683,48 +720,59 @@ 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" - floorsPbsService.sendAuctionRequest(bidRequest) + def response = floorsPbsService.sendAuctionRequest(bidRequest) + + then: "Response should includer error warning" + def message = "Price floor schema dimensions ${getSchemaSize(bidRequest)} exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] - then: "PBS should log a errors" + and: "PBS shouldn't log a errors" def logs = floorsPbsService.getLogsByTime(startTime) - def floorsLogs = getLogsByText(logs, BASIC_FETCH_URL + accountId) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) assert floorsLogs.size() == 1 - assert floorsLogs[0].contains("Failed to fetch price floor from provider for fetch.url: " + - "'$BASIC_FETCH_URL$accountId', account = $accountId with a reason : Price floor schema dimensions ${getSchemaSize(bidRequest)} " + - "exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}") - and: "Metric alerts.account_config.ACCOUNT.price-floors should be update" + and: "Metrics should be updated" def metrics = floorsPbsService.sendCollectedMetricsRequest() assert metrics[INVALID_CONFIG_METRIC(bidRequest.accountId) as String] == 1 + assert metrics[ALERT_GENERAL] == 1 cleanup: "Stop and remove pbs container" pbsServiceFactory.removeContainer(pbsFloorConfig) } - def "PBS should emit errors when request has more schema.fields than fetch.max-schema-dims"() { - given: "Default BidRequest with floorMin" + def "PBS should emit warning when request has more schema.fields than fetch.max-schema-dims"() { + given: "BidRequest with schema 2 fields" def bidRequest = bidRequestWithFloors and: "Account with disabled fetch in the DB" 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 errors" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: " + - "Price floor schema dimensions ${getSchemaSize(bidRequest)} exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}"] + then: "PBS should log a warning" + def message = "Price floor schema dimensions ${getSchemaSize(bidRequest)} exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + + and: "PBS should log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) + assert floorsLogs.size() == 1 + + and: "Alerts.general metrics should be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert metrics[ALERT_GENERAL] == 1 where: maxSchemaDims | maxSchemaDimsSnakeCase @@ -736,28 +784,36 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { given: "BidRequest with schema 2 fields" def bidRequest = bidRequestWithFloors - and: "Account with maxSchemaDims in the DB" + and: "Account with maxSchemaDims and disabled fetching in the DB" def accountId = bidRequest.site.publisher.id def floorSchemaFilesSize = getSchemaSize(bidRequest) def account = getAccountWithEnabledFetch(accountId).tap { + config.auction.priceFloors.fetch.enabled = false config.auction.priceFloors.maxSchemaDims = MAX_SCHEMA_DIMENSIONS_SIZE config.auction.priceFloors.fetch.maxSchemaDims = floorSchemaFilesSize } accountDao.save(account) - and: "Set bidder response" - def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) - bidder.setResponse(bidRequest.id, bidResponse) + and: "Flush metrics" + flushMetrics(floorsPbsService) when: "PBS processes auction request" def response = floorsPbsService.sendAuctionRequest(bidRequest) - then: "PBS should log a errors" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: " + - "Price floor schema dimensions ${floorSchemaFilesSize} " + - "exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}"] + then: "PBS should log a warning" + def message = "Price floor schema dimensions ${floorSchemaFilesSize} " + + "exceeded its maximum number ${MAX_SCHEMA_DIMENSIONS_SIZE}" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + + and: "PBS should log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, ERROR_LOG(bidRequest, message)) + assert floorsLogs.size() == 1 + + and: "Alerts.general metrics should be populated" + def metrics = floorsPbsService.sendCollectedMetricsRequest() + assert metrics[ALERT_GENERAL] == 1 } def "PBS shouldn't fail with error and maxSchemaDims take precede over fetch.maxSchemaDims when requested both"() { @@ -772,22 +828,18 @@ 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 log a errors" - assert !response.ext?.errors + then: "PBS shouldn't add warnings or errors" + assert !response.ext?.warnings } - def "PBS should emit errors when stored request has more rules than price-floor.max-rules for amp request"() { + def "PBS should emit warning when stored request has more rules than price-floor.max-rules for amp request"() { given: "Default AmpRequest" def ampRequest = AmpRequest.defaultAmpRequest - and: "Default stored request with 2 rules " + and: "Default stored request with 2 rules" def requestFloorValue = PBSUtils.randomFloorValue def ampStoredRequest = BidRequest.defaultStoredRequest.tap { ext.prebid.floors = ExtPrebidFloors.extPrebidFloors @@ -798,8 +850,9 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { def storedRequest = StoredRequest.getStoredRequest(ampRequest, ampStoredRequest) storedRequestDao.save(storedRequest) - and: "Account with maxRules in the DB" + and: "Account with maxRules and disabled fetching in the DB" def account = getAccountWithEnabledFetch(ampRequest.account as String).tap { + config.auction.priceFloors.fetch.enabled = false config.auction.priceFloors.maxRules = maxRules config.auction.priceFloors.maxRulesSnakeCase = maxRulesSnakeCase } @@ -810,15 +863,27 @@ class PriceFloorsSignalingSpec extends PriceFloorsBaseSpec { bidResponse.seatbid.first().bid.first().price = requestFloorValue bidder.setResponse(ampStoredRequest.id, bidResponse) + and: "Flush metrics" + flushMetrics(floorsPbsService) + when: "PBS processes amp request" def response = floorsPbsService.sendAmpRequest(ampRequest) - then: "PBS should log a errors" - assert response.ext?.errors[PREBID]*.code == [999] - assert response.ext?.errors[PREBID]*.message == - ["Failed to parse price floors from request, with a reason: " + - "Price floor rules number ${getRuleSize(ampStoredRequest)} " + - "exceeded its maximum number ${MAX_RULES_SIZE}"] + then: "PBS should log a warning" + def message = "Price floor rules number ${getRuleSize(ampStoredRequest)} " + + "exceeded its maximum number ${MAX_RULES_SIZE}" + assert response.ext?.warnings[PREBID]*.code == [999] + assert response.ext?.warnings[PREBID]*.message == [WARNING_MESSAGE(message)] + + and: "PBS should log a errors" + def logs = floorsPbsService.getLogsByTime(startTime) + def floorsLogs = getLogsByText(logs, "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 where: maxRules | maxRulesSnakeCase @@ -826,6 +891,298 @@ 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 771fac9bd84..717c9f32d5b 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/privacy/GdprAmpSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/privacy/GdprAmpSpec.groovy @@ -394,7 +394,7 @@ class GdprAmpSpec extends PrivacyBaseSpec { and: "Alerts.general metrics should be populated" def metrics = privacyPbsService.sendCollectedMetricsRequest() - assert metrics["alerts.general"] == 1 + assert metrics[ALERT_GENERAL] == 1 and: "Bidder should be called" assert bidder.getBidderRequest(ampStoredRequest.id) diff --git a/src/test/groovy/org/prebid/server/functional/tests/privacy/GdprAuctionSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/privacy/GdprAuctionSpec.groovy index 7302ad79aed..5fc0f5d7bca 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/privacy/GdprAuctionSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/privacy/GdprAuctionSpec.groovy @@ -342,7 +342,7 @@ class GdprAuctionSpec extends PrivacyBaseSpec { and: "Alerts.general metrics should be populated" def metrics = privacyPbsService.sendCollectedMetricsRequest() - assert metrics["alerts.general"] == 1 + assert metrics[ALERT_GENERAL] == 1 and: "Bid response should contain seatBid" assert response.seatbid.size() == 1 diff --git a/src/test/java/org/prebid/server/floors/BasicPriceFloorProcessorTest.java b/src/test/java/org/prebid/server/floors/BasicPriceFloorProcessorTest.java index 78ce82dd3fd..432b68c0c6b 100644 --- a/src/test/java/org/prebid/server/floors/BasicPriceFloorProcessorTest.java +++ b/src/test/java/org/prebid/server/floors/BasicPriceFloorProcessorTest.java @@ -20,6 +20,8 @@ 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; @@ -51,12 +53,14 @@ 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, jacksonMapper); + target = new BasicPriceFloorProcessor(priceFloorFetcher, floorResolver, metrics, jacksonMapper, 0.0d); } @Test @@ -70,7 +74,7 @@ public void shouldSetRulesEnabledFieldToFalseIfPriceFloorsDisabledForAccount() { new ArrayList<>()); // then - verifyNoInteractions(priceFloorFetcher); + verifyNoInteractions(priceFloorFetcher, metrics); assertThat(result).isEqualTo(givenBidRequest(identity(), PriceFloorRules.builder().enabled(false).build())); } @@ -85,7 +89,7 @@ public void shouldSetRulesEnabledFieldToFalseIfPriceFloorsDisabledForRequest() { new ArrayList<>()); // then - verifyNoInteractions(priceFloorFetcher); + verifyNoInteractions(priceFloorFetcher, metrics); assertThat(result) .extracting(BidRequest::getExt) @@ -98,8 +102,8 @@ public void shouldSetRulesEnabledFieldToFalseIfPriceFloorsDisabledForRequest() { @Test public void shouldUseFloorsDataFromProviderIfPresent() { // given - final PriceFloorData providerFloorsData = givenFloorData(floors -> floors.floorProvider("provider.com")); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); + final PriceFloorData providerFloorsData = givenFloorData(identity()); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -118,16 +122,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 - .floorProvider("provider.com") - .useFetchDataRate(null)); + final PriceFloorData providerFloorsData = givenFloorData(floors -> floors.useFetchDataRate(null)); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -145,16 +149,15 @@ public void shouldUseFloorsFromProviderIfUseDynamicDataAndUseFetchDataRateAreAbs .data(providerFloorsData) .fetchStatus(FetchStatus.success) .location(PriceFloorLocation.fetch))); + verifyNoInteractions(metrics); } @Test public void shouldUseFloorsFromProviderIfUseDynamicDataIsAbsentAndUseFetchDataRateIs100() { // given - final PriceFloorData providerFloorsData = givenFloorData(floors -> floors - .floorProvider("provider.com") - .useFetchDataRate(100)); + final PriceFloorData providerFloorsData = givenFloorData(floors -> floors.useFetchDataRate(100)); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -172,16 +175,15 @@ public void shouldUseFloorsFromProviderIfUseDynamicDataIsAbsentAndUseFetchDataRa .data(providerFloorsData) .fetchStatus(FetchStatus.success) .location(PriceFloorLocation.fetch))); + verifyNoInteractions(metrics); } @Test public void shouldNotUseFloorsFromProviderIfUseDynamicDataIsAbsentAndUseFetchDataRateIs0() { // given - final PriceFloorData providerFloorsData = givenFloorData(floors -> floors - .floorProvider("provider.com") - .useFetchDataRate(0)); + final PriceFloorData providerFloorsData = givenFloorData(floors -> floors.useFetchDataRate(0)); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -199,15 +201,44 @@ public void shouldNotUseFloorsFromProviderIfUseDynamicDataIsAbsentAndUseFetchDat assertThat(actualRules) .extracting(PriceFloorRules::getLocation) .isEqualTo(PriceFloorLocation.noData); + verifyNoInteractions(metrics); } @Test - public void shouldUseFloorsFromProviderIfUseDynamicDataIsTrueAndUseFetchDataRateIsAbsent() { + public void shouldTolerateInvalidFloorsFromRequestWhenFetchIsSuccessAndUseFetchDataRateIs0() { // given final PriceFloorData providerFloorsData = givenFloorData(floors -> floors .floorProvider("provider.com") - .useFetchDataRate(null)); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); + .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)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -226,65 +257,330 @@ public void shouldUseFloorsFromProviderIfUseDynamicDataIsTrueAndUseFetchDataRate .floorMin(BigDecimal.ONE) .fetchStatus(FetchStatus.success) .location(PriceFloorLocation.fetch))); + verifyNoInteractions(metrics); } @Test - public void shouldNotUseFloorsFromProviderIfUseDynamicDataIsFalseAndUseFetchDataRateIsAbsent() { + public void shouldNotUseFloorsFromProviderIfUseDynamicDataIsFalse() { // given - final PriceFloorData providerFloorsData = givenFloorData(floors -> floors - .floorProvider("provider.com") - .useFetchDataRate(null)); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); + 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(), null), givenAccount(floorsConfig -> floorsConfig.useDynamicData(false)), "bidder", new ArrayList<>(), - new ArrayList<>()); + warnings); // then - final PriceFloorRules actualRules = extractFloors(result); - assertThat(actualRules) - .extracting(PriceFloorRules::getFetchStatus) - .isEqualTo(FetchStatus.success); - assertThat(actualRules) - .extracting(PriceFloorRules::getLocation) - .isEqualTo(PriceFloorLocation.noData); + 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"); } @Test - public void shouldNotUseFloorsFromProviderIfUseDynamicDataIsFalseAndUseFetchDataRateIs100() { + public void shouldNotTolerateInvalidFloorsFromRequestWhenFetchIsSuccessAndUseDynamicDataIsFalse() { // given final PriceFloorData providerFloorsData = givenFloorData(floors -> floors .floorProvider("provider.com") - .useFetchDataRate(100)); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); + .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(), null), + givenBidRequest(identity(), givenFloors(floors -> floors.data(null))), givenAccount(floorsConfig -> floorsConfig.useDynamicData(false)), "bidder", new ArrayList<>(), - 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")); + + // 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()), + "bidder", + new ArrayList<>(), + warnings); // then final PriceFloorRules actualRules = extractFloors(result); assertThat(actualRules) .extracting(PriceFloorRules::getFetchStatus) - .isEqualTo(FetchStatus.success); + .isEqualTo(FetchStatus.timeout); 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(floors -> floors.floorProvider("provider.com")); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); + final PriceFloorData providerFloorsData = givenFloorData(identity()); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -315,8 +611,8 @@ public void shouldMergeProviderWithRequestFloors() { @Test public void shouldReturnProviderFloorsWhenNotEnabledByRequestAndEnforceRateAndFloorPriceAreAbsent() { // given - final PriceFloorData providerFloorsData = givenFloorData(floors -> floors.floorProvider("provider.com")); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); + final PriceFloorData providerFloorsData = givenFloorData(identity()); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -348,8 +644,8 @@ public void shouldReturnFloorsWithFloorMinAndCurrencyFromRequestWhenPresent() { .floorMin(BigDecimal.ONE) .data(givenFloorData(floorsDataConfig -> floorsDataConfig.currency("USD")))); - final PriceFloorData providerFloorsData = givenFloorData(floors -> floors.floorProvider("provider.com")); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); + final PriceFloorData providerFloorsData = givenFloorData(identity()); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -368,7 +664,7 @@ public void shouldReturnFloorsWithFloorMinAndCurrencyFromRequestWhenPresent() { @Test public void shouldUseFloorsFromRequestIfProviderFloorsMissing() { // given - given(priceFloorFetcher.fetch(any())).willReturn(null); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); // when final BidRequest result = target.enrichWithPriceFloors( @@ -380,17 +676,19 @@ public void shouldUseFloorsFromRequestIfProviderFloorsMissing() { // then assertThat(extractFloors(result)).isEqualTo(givenFloors(floors -> floors - .enabled(true) - .skipped(false) - .floorMin(BigDecimal.ONE) - .location(PriceFloorLocation.request))); + .fetchStatus(FetchStatus.none) + .enabled(true) + .skipped(false) + .floorMin(BigDecimal.ONE) + .location(PriceFloorLocation.request))); + verifyNoInteractions(metrics); } @Test public void shouldTolerateUsingFloorsFromRequestWhenRulesNumberMoreThanMaxRulesNumber() { // given - given(priceFloorFetcher.fetch(any())).willReturn(null); - final ArrayList errors = new ArrayList<>(); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); + final ArrayList warnings = new ArrayList<>(); // when final BidRequest result = target.enrichWithPriceFloors( @@ -404,25 +702,27 @@ public void shouldTolerateUsingFloorsFromRequestWhenRulesNumberMoreThanMaxRulesN )), givenAccount(floorConfigBuilder -> floorConfigBuilder.maxRules(1L)), "bidder", - errors, - new ArrayList<>()); + new ArrayList<>(), + warnings); // then assertThat(extractFloors(result)).isEqualTo(PriceFloorRules.builder() + .fetchStatus(FetchStatus.none) .enabled(true) .skipped(false) .location(PriceFloorLocation.noData) .build()); - assertThat(errors).containsOnly("Failed to parse price floors from request, with a reason: " + assertThat(warnings).containsOnly("errorMessage. " + + "Following parsing of request price floors is failed: " + "Price floor rules number 2 exceeded its maximum number 1"); } @Test public void shouldTolerateUsingFloorsFromRequestWhenDimensionsNumberMoreThanMaxDimensionsNumber() { // given - given(priceFloorFetcher.fetch(any())).willReturn(null); - final ArrayList errors = new ArrayList<>(); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); + final ArrayList warnings = new ArrayList<>(); // when final BidRequest result = target.enrichWithPriceFloors( @@ -436,24 +736,52 @@ public void shouldTolerateUsingFloorsFromRequestWhenDimensionsNumberMoreThanMaxD )), givenAccount(floorConfigBuilder -> floorConfigBuilder.maxSchemaDims(1L)), "bidder", - errors, - new ArrayList<>()); + new ArrayList<>(), + warnings); // then assertThat(extractFloors(result)).isEqualTo(PriceFloorRules.builder() + .fetchStatus(FetchStatus.none) .enabled(true) .skipped(false) .location(PriceFloorLocation.noData) .build()); - assertThat(errors).containsOnly("Failed to parse price floors from request, with a reason: " + assertThat(warnings).containsOnly("errorMessage. " + + "Following parsing of request price floors is failed: " + "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(null); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.none("errorMessage")); // when final BidRequest result = target.enrichWithPriceFloors( @@ -475,6 +803,9 @@ 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))), @@ -486,6 +817,7 @@ public void shouldNotSkipFloorsIfRootSkipRateIsOff() { // then assertThat(extractFloors(result)) .isEqualTo(givenFloors(floors -> floors + .fetchStatus(FetchStatus.none) .enabled(true) .skipped(false) .skipRate(0) @@ -494,6 +826,9 @@ 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))), @@ -504,16 +839,18 @@ public void shouldSkipFloorsIfRootSkipRateIsOn() { // then assertThat(extractFloors(result)).isEqualTo(givenFloors(floors -> floors - .skipRate(100) - .enabled(true) - .skipped(true) - .location(PriceFloorLocation.request))); + .fetchStatus(FetchStatus.none) + .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( @@ -525,11 +862,13 @@ public void shouldSkipFloorsIfDataSkipRateIsOn() { // then assertThat(extractFloors(result)).isEqualTo(givenFloors(floors -> floors - .enabled(true) - .skipRate(100) - .data(priceFloorData) - .skipped(true) - .location(PriceFloorLocation.request))); + .fetchStatus(FetchStatus.none) + .enabled(true) + .skipRate(100) + .floorProvider("provider.com") + .data(priceFloorData) + .skipped(true) + .location(PriceFloorLocation.request))); } @Test @@ -538,6 +877,7 @@ 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( @@ -549,11 +889,13 @@ public void shouldSkipFloorsIfModelGroupSkipRateIsOn() { // then assertThat(extractFloors(result)).isEqualTo(givenFloors(floors -> floors - .data(priceFloorData) - .skipRate(100) - .enabled(true) - .skipped(true) - .location(PriceFloorLocation.request))); + .fetchStatus(FetchStatus.none) + .data(priceFloorData) + .skipRate(100) + .floorProvider("provider.com") + .enabled(true) + .skipped(true) + .location(PriceFloorLocation.request))); } @Test @@ -562,6 +904,7 @@ 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( @@ -582,6 +925,7 @@ 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( @@ -603,8 +947,8 @@ public void shouldUseSelectedModelGroup() { @Test public void shouldCopyFloorProviderValueFromDataLevel() { // given - final PriceFloorData providerFloorsData = givenFloorData(floors -> floors.floorProvider("provider.com")); - given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success)); + final PriceFloorData providerFloorsData = givenFloorData(identity()); + given(priceFloorFetcher.fetch(any())).willReturn(FetchResult.of(providerFloorsData, FetchStatus.success, null)); // when final BidRequest result = target.enrichWithPriceFloors( @@ -630,6 +974,7 @@ 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 @@ -660,6 +1005,7 @@ 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")); @@ -692,6 +1038,7 @@ 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")); @@ -712,6 +1059,7 @@ private static Account givenAccount( UnaryOperator floorsConfigCustomizer) { return Account.builder() + .id("accountId") .auction(AccountAuctionConfig.builder() .priceFloors(floorsConfigCustomizer.apply(AccountPriceFloorsConfig.builder()).build()) .build()) @@ -722,6 +1070,7 @@ 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 4fe2e31180b..542559bd862 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.getFetchStatus()).isEqualTo(FetchStatus.success); - assertThat(priceFloorRulesCached.getRulesData()).isEqualTo(givenPriceFloorData()); + assertThat(priceFloorRulesCached) + .isEqualTo(FetchResult.of(givenPriceFloorData(), FetchStatus.success, null)); } @@ -119,8 +119,7 @@ public void fetchShouldReturnEmptyRulesAndInProgressStatusForTheFirstInvocation( final FetchResult fetchResult = priceFloorFetcher.fetch(givenAccount(identity())); // then - assertThat(fetchResult.getRulesData()).isNull(); - assertThat(fetchResult.getFetchStatus()).isEqualTo(FetchStatus.inprogress); + assertThat(fetchResult).isEqualTo(FetchResult.inProgress()); verify(vertx).setTimer(eq(1200000L), any()); } @@ -134,13 +133,13 @@ public void fetchShouldReturnEmptyRulesAndInProgressStatusForTheFirstInvocationA final FetchResult firstInvocationResult = priceFloorFetcher.fetch(givenAccount(identity())); // then - assertThat(firstInvocationResult.getRulesData()).isNull(); - assertThat(firstInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.inprogress); + assertThat(firstInvocationResult).isEqualTo(FetchResult.inProgress()); verify(vertx).setTimer(eq(1200000L), any()); final FetchResult secondInvocationResult = priceFloorFetcher.fetch(givenAccount(identity())); - assertThat(secondInvocationResult.getRulesData()).isNull(); - assertThat(secondInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.error); + assertThat(secondInvocationResult).isEqualTo(FetchResult.error( + "Failed to fetch price floor from provider for fetch.url " + + "'http://test.host.com', with a reason: failed")); } @Test @@ -158,8 +157,10 @@ public void fetchShouldReturnEmptyRulesAndInProgressStatusForTheFirstInvocationA verify(vertx).setTimer(eq(1200000L), any()); final FetchResult secondInvocationResult = priceFloorFetcher.fetch(givenAccount(identity())); - assertThat(secondInvocationResult.getRulesData()).isNull(); - assertThat(secondInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.timeout); + assertThat(secondInvocationResult).isEqualTo(FetchResult.of( + null, + FetchStatus.timeout, + "Fetch price floor request timeout for fetch.url 'http://test.host.com' exceeded.")); } @Test @@ -317,8 +318,8 @@ public void fetchShouldNotPrepareAnyRequestsWhenFetchUrlIsMalformedAndReturnErro // then verifyNoInteractions(httpClient); - assertThat(fetchResult.getRulesData()).isNull(); - assertThat(fetchResult.getFetchStatus()).isEqualTo(FetchStatus.error); + assertThat(fetchResult).isEqualTo(FetchResult.error( + "Malformed fetch.url 'MalformedURl' passed")); verifyNoInteractions(vertx); } @@ -329,8 +330,7 @@ public void fetchShouldNotPrepareAnyRequestsWhenFetchUrlIsBlankAndReturnErrorSta // then verifyNoInteractions(httpClient); - assertThat(fetchResult.getRulesData()).isNull(); - assertThat(fetchResult.getFetchStatus()).isEqualTo(FetchStatus.error); + assertThat(fetchResult).isEqualTo(FetchResult.error("Malformed fetch.url ' ' passed")); verifyNoInteractions(vertx); } @@ -341,8 +341,7 @@ public void fetchShouldNotPrepareAnyRequestsWhenFetchUrlIsNotProvidedAndReturnEr // then verifyNoInteractions(httpClient); - assertThat(fetchResult.getRulesData()).isNull(); - assertThat(fetchResult.getFetchStatus()).isEqualTo(FetchStatus.error); + assertThat(fetchResult).isEqualTo(FetchResult.error("Malformed fetch.url 'null' passed")); verifyNoInteractions(vertx); } @@ -353,8 +352,7 @@ public void fetchShouldNotPrepareAnyRequestsWhenFetchEnabledIsFalseAndReturnNone // then verifyNoInteractions(httpClient); - assertThat(fetchResult.getRulesData()).isNull(); - assertThat(fetchResult.getFetchStatus()).isEqualTo(FetchStatus.none); + assertThat(fetchResult).isEqualTo(FetchResult.none("Fetching is disabled")); verifyNoInteractions(vertx); } @@ -370,13 +368,13 @@ public void fetchShouldReturnEmptyRulesAndErrorStatusForSecondCallAndCreatePerio // then verify(httpClient).get(anyString(), anyLong(), anyLong()); - assertThat(firstInvocationResult.getRulesData()).isNull(); - assertThat(firstInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.inprogress); + assertThat(firstInvocationResult).isEqualTo(FetchResult.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).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")); verifyNoMoreInteractions(vertx); } @@ -392,13 +390,16 @@ public void fetchShouldReturnEmptyRulesWithErrorStatusAndCreatePeriodicTimerWhen // then verify(httpClient).get(anyString(), anyLong(), anyLong()); - assertThat(firstInvocationResult.getRulesData()).isNull(); - assertThat(firstInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.inprogress); + assertThat(firstInvocationResult).isEqualTo(FetchResult.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); } @@ -414,13 +415,14 @@ public void fetchShouldReturnEmptyRulesWithErrorStatusForSecondCallAndCreatePeri // then verify(httpClient).get(anyString(), anyLong(), anyLong()); - assertThat(firstInvocationResult.getRulesData()).isNull(); - assertThat(firstInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.inprogress); + assertThat(firstInvocationResult).isEqualTo(FetchResult.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).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")); verifyNoMoreInteractions(vertx); } @@ -436,13 +438,14 @@ public void fetchShouldReturnEmptyRulesWithErrorStatusForSecondCallAndCreatePeri // then verify(httpClient).get(anyString(), anyLong(), anyLong()); - assertThat(firstInvocationResult.getRulesData()).isNull(); - assertThat(firstInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.inprogress); + assertThat(firstInvocationResult).isEqualTo(FetchResult.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).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")); verifyNoMoreInteractions(vertx); } @@ -460,8 +463,7 @@ public void fetchShouldNotCallPriceFloorProviderWhileFetchIsAlreadyInProgress() verify(httpClient).get(anyString(), anyLong(), anyLong()); verifyNoMoreInteractions(httpClient); - assertThat(secondFetch.getRulesData()).isNull(); - assertThat(secondFetch.getFetchStatus()).isEqualTo(FetchStatus.inprogress); + assertThat(secondFetch).isEqualTo(FetchResult.inProgress()); fetchPromise.tryComplete( HttpClientResponse.of(200, MultiMap.caseInsensitiveMultiMap() @@ -490,13 +492,13 @@ public void fetchShouldReturnNullAndCreatePeriodicTimerWhenResponseExceededRules // then verify(httpClient).get(anyString(), anyLong(), anyLong()); - assertThat(firstInvocationResult.getRulesData()).isNull(); - assertThat(firstInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.inprogress); + assertThat(firstInvocationResult).isEqualTo(FetchResult.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).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")); verifyNoMoreInteractions(vertx); } @@ -518,13 +520,13 @@ public void fetchShouldReturnNullAndCreatePeriodicTimerWhenResponseExceededDimen // then verify(httpClient).get(anyString(), anyLong(), anyLong()); - assertThat(firstInvocationResult.getRulesData()).isNull(); - assertThat(firstInvocationResult.getFetchStatus()).isEqualTo(FetchStatus.inprogress); + assertThat(firstInvocationResult).isEqualTo(FetchResult.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).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 {}")); verifyNoMoreInteractions(vertx); }