Skip to content

Commit

Permalink
Price Floors: Max Rules and Max Dimensions
Browse files Browse the repository at this point in the history
  • Loading branch information
AntoxaAntoxic committed Dec 16, 2024
1 parent 00e391c commit 587c2fb
Show file tree
Hide file tree
Showing 10 changed files with 257 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,25 @@ private PriceFloorRules resolveFloors(Account account, BidRequest bidRequest, Li

if (requestFloors != null) {
try {
PriceFloorRulesValidator.validateRules(requestFloors, Integer.MAX_VALUE);
final Optional<AccountPriceFloorsConfig> priceFloorsConfig = Optional.ofNullable(account)
.map(Account::getAuction)
.map(AccountAuctionConfig::getPriceFloors);

final Long maxRules = priceFloorsConfig.map(AccountPriceFloorsConfig::getMaxRules)
.orElse(null);
final Long maxDimensions = priceFloorsConfig.map(AccountPriceFloorsConfig::getMaxSchemaDimensions)
.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()));
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 "
"Failed to parse price floors from request with id: '%s', with a reason: %s"
.formatted(bidRequest.getId(), e.getMessage()),
0.01d);
}
Expand Down
11 changes: 4 additions & 7 deletions src/main/java/org/prebid/server/floors/PriceFloorFetcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,10 @@ private ResponseCacheInfo parseFloorResponse(HttpClientResponse httpClientRespon
}

final PriceFloorData priceFloorData = parsePriceFloorData(body, accountId);
PriceFloorRulesValidator.validateRulesData(priceFloorData, resolveMaxRules(fetchConfig.getMaxRules()));
PriceFloorRulesValidator.validateRulesData(
priceFloorData,
PriceFloorsConfigResolver.resolveMaxValue(fetchConfig.getMaxRules()),
PriceFloorsConfigResolver.resolveMaxValue(fetchConfig.getMaxSchemaDimensions()));

return ResponseCacheInfo.of(priceFloorData,
FetchStatus.success,
Expand All @@ -194,12 +197,6 @@ private PriceFloorData parsePriceFloorData(String body, String accountId) {
return priceFloorData;
}

private static int resolveMaxRules(Long accountMaxRules) {
return accountMaxRules != null && !accountMaxRules.equals(0L)
? Math.toIntExact(accountMaxRules)
: Integer.MAX_VALUE;
}

private Long cacheTtlFromResponse(HttpClientResponse httpClientResponse, String fetchUrl) {
final String cacheControlValue = httpClientResponse.getHeaders().get(HttpHeaders.CACHE_CONTROL);
final Matcher cacheHeaderMatcher = StringUtils.isNotBlank(cacheControlValue)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@
import org.apache.commons.collections4.MapUtils;
import org.prebid.server.exception.PreBidException;
import org.prebid.server.floors.model.PriceFloorData;
import org.prebid.server.floors.model.PriceFloorField;
import org.prebid.server.floors.model.PriceFloorModelGroup;
import org.prebid.server.floors.model.PriceFloorRules;
import org.prebid.server.floors.model.PriceFloorSchema;

import java.math.BigDecimal;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;

public class PriceFloorRulesValidator {

Expand All @@ -23,7 +27,7 @@ public class PriceFloorRulesValidator {
private PriceFloorRulesValidator() {
}

public static void validateRules(PriceFloorRules priceFloorRules, Integer maxRules) {
public static void validateRules(PriceFloorRules priceFloorRules, Integer maxRules, Integer maxDimensions) {

final Integer rootSkipRate = priceFloorRules.getSkipRate();
if (rootSkipRate != null && (rootSkipRate < SKIP_RATE_MIN || rootSkipRate > SKIP_RATE_MAX)) {
Expand All @@ -36,10 +40,10 @@ public static void validateRules(PriceFloorRules priceFloorRules, Integer maxRul
throw new PreBidException("Price floor floorMin must be positive float, but was " + floorMin);
}

validateRulesData(priceFloorRules.getData(), maxRules);
validateRulesData(priceFloorRules.getData(), maxRules, maxDimensions);
}

public static void validateRulesData(PriceFloorData priceFloorData, Integer maxRules) {
public static void validateRulesData(PriceFloorData priceFloorData, Integer maxRules, Integer maxDimensions) {
if (priceFloorData == null) {
throw new PreBidException("Price floor rules data must be present");
}
Expand All @@ -64,10 +68,10 @@ public static void validateRulesData(PriceFloorData priceFloorData, Integer maxR

priceFloorData.getModelGroups().stream()
.filter(Objects::nonNull)
.forEach(modelGroup -> validateModelGroup(modelGroup, maxRules));
.forEach(modelGroup -> validateModelGroup(modelGroup, maxRules, maxDimensions));
}

private static void validateModelGroup(PriceFloorModelGroup modelGroup, Integer maxRules) {
private static void validateModelGroup(PriceFloorModelGroup modelGroup, Integer maxRules, Integer maxDimensions) {
final Integer modelWeight = modelGroup.getModelWeight();
if (modelWeight != null
&& (modelWeight < MODEL_WEIGHT_MIN_VALUE || modelWeight > MODEL_WEIGHT_MAX_VALUE)) {
Expand Down Expand Up @@ -95,8 +99,21 @@ private static void validateModelGroup(PriceFloorModelGroup modelGroup, Integer
}

if (maxRules != null && values.size() > maxRules) {
throw new PreBidException(
"Price floor rules number %s exceeded its maximum number %s".formatted(values.size(), maxRules));
throw new PreBidException("Price floor rules number %s exceeded its maximum number %s"
.formatted(values.size(), maxRules));
}

final List<PriceFloorField> fields = Optional.ofNullable(modelGroup.getSchema())
.map(PriceFloorSchema::getFields)
.orElse(null);

if (CollectionUtils.isEmpty(fields)) {
throw new PreBidException("Price floor dimensions can't be null or empty, but were " + fields);
}

if (maxDimensions != null && fields.size() > maxDimensions) {
throw new PreBidException("Price floor schema dimensions %s exceeded its maximum number %s"
.formatted(fields.size(), maxDimensions));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ public class PriceFloorsConfigResolver {
private static final ConditionalLogger conditionalLogger = new ConditionalLogger(logger);

private static final int MIN_MAX_AGE_SEC_VALUE = 600;
private static final int MAX_AGE_SEC_VALUE = Integer.MAX_VALUE;
private static final int MIN_PERIODIC_SEC_VALUE = 300;
private static final int MIN_TIMEOUT_MS_VALUE = 10;
private static final int MAX_TIMEOUT_MS_VALUE = 10_000;
private static final int MIN_RULES_VALUE = 0;
private static final int MIN_FILE_SIZE_VALUE = 0;
private static final int MAX_AGE_SEC_VALUE = Integer.MAX_VALUE;
private static final int MAX_RULES_VALUE = Integer.MAX_VALUE;
private static final int MIN_DIMENSIONS_VALUE = 0;
private static final int MAX_DIMENSIONS_VALUE = 19;
private static final int MIN_FILE_SIZE_VALUE = 0;
private static final int MAX_FILE_SIZE_VALUE = Integer.MAX_VALUE;
private static final int MIN_ENFORCE_RATE_VALUE = 0;
private static final int MAX_ENFORCE_RATE_VALUE = 100;
Expand Down Expand Up @@ -71,6 +73,16 @@ private static void validatePriceFloorConfig(Account account) {
throw new PreBidException(invalidPriceFloorsPropertyMessage("enforce-floors-rate", enforceRate));
}

final Long maxRules = floorsConfig.getMaxRules();
if (maxRules != null && isNotInRange(maxRules, MIN_RULES_VALUE, MAX_RULES_VALUE)) {
throw new PreBidException(invalidPriceFloorsPropertyMessage("max-rules", maxRules));
}

final Long maxDimensions = floorsConfig.getMaxSchemaDimensions();
if (maxDimensions != null && isNotInRange(maxDimensions, MIN_DIMENSIONS_VALUE, MAX_DIMENSIONS_VALUE)) {
throw new PreBidException(invalidPriceFloorsPropertyMessage("max-schema-dimensions", maxDimensions));
}

final AccountPriceFloorsFetchConfig fetchConfig =
ObjectUtil.getIfNotNull(floorsConfig, AccountPriceFloorsConfig::getFetch);

Expand Down Expand Up @@ -108,6 +120,11 @@ private static void validatePriceFloorsFetchConfig(AccountPriceFloorsFetchConfig
throw new PreBidException(invalidPriceFloorsPropertyMessage("max-rules", maxRules));
}

final Long maxDimensions = fetchConfig.getMaxSchemaDimensions();
if (maxDimensions != null && isNotInRange(maxDimensions, MIN_DIMENSIONS_VALUE, MAX_DIMENSIONS_VALUE)) {
throw new PreBidException(invalidPriceFloorsPropertyMessage("max-schema-dimensions", maxDimensions));
}

final Long maxFileSize = fetchConfig.getMaxFileSizeKb();
if (maxFileSize != null && isNotInRange(maxFileSize, MIN_FILE_SIZE_VALUE, MAX_FILE_SIZE_VALUE)) {
throw new PreBidException(invalidPriceFloorsPropertyMessage("max-file-size-kb", maxFileSize));
Expand All @@ -121,4 +138,8 @@ private static boolean isNotInRange(long number, long min, long max) {
private static String invalidPriceFloorsPropertyMessage(String property, Object value) {
return "Invalid price-floors property '%s', value passed: %s".formatted(property, value);
}

public static int resolveMaxValue(Long value) {
return value != null && !value.equals(0L) ? Math.toIntExact(value) : Integer.MAX_VALUE;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,10 @@ public class AccountPriceFloorsConfig {

@JsonAlias("use-dynamic-data")
Boolean useDynamicData;

@JsonAlias("max-rules")
Long maxRules;

@JsonAlias("max-schema-dims")
Long maxSchemaDimensions;
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ public class AccountPriceFloorsFetchConfig {
@JsonAlias("max-rules")
Long maxRules;

@JsonAlias("max-schema-dims")
Long maxSchemaDimensions;

@JsonAlias("max-age-sec")
Long maxAgeSec;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.prebid.server.VertxTest;
import org.prebid.server.auction.model.AuctionContext;
import org.prebid.server.floors.model.PriceFloorData;
import org.prebid.server.floors.model.PriceFloorEnforcement;
import org.prebid.server.floors.model.PriceFloorLocation;
import org.prebid.server.floors.model.PriceFloorModelGroup;
import org.prebid.server.floors.model.PriceFloorResult;
import org.prebid.server.floors.model.PriceFloorRules;
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.proto.openrtb.ext.request.ExtImpPrebidFloors;
Expand All @@ -30,6 +30,7 @@
import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.function.UnaryOperator;

import static java.util.Collections.singletonList;
Expand All @@ -40,6 +41,8 @@
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.prebid.server.floors.model.PriceFloorField.siteDomain;
import static org.prebid.server.floors.model.PriceFloorField.size;

@ExtendWith(MockitoExtension.class)
public class BasicPriceFloorProcessorTest extends VertxTest {
Expand Down Expand Up @@ -383,6 +386,70 @@ public void shouldUseFloorsFromRequestIfProviderFloorsMissing() {
.location(PriceFloorLocation.request)));
}

@Test
public void shouldTolerateUsingFloorsFromRequestWhenRulesNumberMoreThanMaxRulesNumber() {
// given
given(priceFloorFetcher.fetch(any())).willReturn(null);
final ArrayList<String> errors = new ArrayList<>();

// when
final BidRequest result = target.enrichWithPriceFloors(
givenBidRequest(identity(), givenFloors(floors -> floors.data(
PriceFloorData.builder()
.modelGroups(singletonList(PriceFloorModelGroup.builder()
.values(Map.of("someKey", BigDecimal.ONE, "someKey2", BigDecimal.ONE))
.schema(PriceFloorSchema.of("|", List.of(size, siteDomain)))
.build()))
.build())
)),
givenAccount(floorConfigBuilder -> floorConfigBuilder.maxRules(1L)),
"bidder",
errors,
new ArrayList<>());

// then
assertThat(extractFloors(result)).isEqualTo(PriceFloorRules.builder()
.enabled(true)
.skipped(false)
.location(PriceFloorLocation.noData)
.build());

assertThat(errors).containsOnly("Failed to parse price floors from request, with a reason: "
+ "Price floor rules number 2 exceeded its maximum number 1");
}

@Test
public void shouldTolerateUsingFloorsFromRequestWhenDimensionsNumberMoreThanMaxDimensionsNumber() {
// given
given(priceFloorFetcher.fetch(any())).willReturn(null);
final ArrayList<String> errors = new ArrayList<>();

// when
final BidRequest result = target.enrichWithPriceFloors(
givenBidRequest(identity(), givenFloors(floors -> floors.data(
PriceFloorData.builder()
.modelGroups(singletonList(PriceFloorModelGroup.builder()
.value("someKey", BigDecimal.ONE)
.schema(PriceFloorSchema.of("|", List.of(size, siteDomain)))
.build()))
.build())
)),
givenAccount(floorConfigBuilder -> floorConfigBuilder.maxSchemaDimensions(1L)),
"bidder",
errors,
new ArrayList<>());

// then
assertThat(extractFloors(result)).isEqualTo(PriceFloorRules.builder()
.enabled(true)
.skipped(false)
.location(PriceFloorLocation.noData)
.build());

assertThat(errors).containsOnly("Failed to parse price floors from request, with a reason: "
+ "Price floor schema dimensions 2 exceeded its maximum number 1");
}

@Test
public void shouldTolerateMissingRequestAndProviderFloors() {
// given
Expand Down Expand Up @@ -641,14 +708,6 @@ public void shouldTolerateFloorResolvingError() {
assertThat(errors).containsOnly("Cannot resolve bid floor, error: error");
}

private static AuctionContext givenAuctionContext(Account account, BidRequest bidRequest) {
return AuctionContext.builder()
.prebidErrors(new ArrayList<>())
.account(account)
.bidRequest(bidRequest)
.build();
}

private static Account givenAccount(
UnaryOperator<AccountPriceFloorsConfig.AccountPriceFloorsConfigBuilder> floorsConfigCustomizer) {

Expand Down Expand Up @@ -681,6 +740,7 @@ private static PriceFloorRules givenFloors(
.data(PriceFloorData.builder()
.modelGroups(singletonList(PriceFloorModelGroup.builder()
.value("someKey", BigDecimal.ONE)
.schema(PriceFloorSchema.of("|", List.of(size)))
.build()))
.build())
).build();
Expand All @@ -692,14 +752,16 @@ private static PriceFloorData givenFloorData(
return floorDataCustomizer.apply(PriceFloorData.builder()
.modelGroups(singletonList(PriceFloorModelGroup.builder()
.value("someKey", BigDecimal.ONE)
.schema(PriceFloorSchema.of("|", List.of(size)))
.build()))).build();
}

private static PriceFloorModelGroup givenModelGroup(
UnaryOperator<PriceFloorModelGroup.PriceFloorModelGroupBuilder> modelGroupCustomizer) {

return modelGroupCustomizer.apply(PriceFloorModelGroup.builder()
.value("someKey", BigDecimal.ONE))
.value("someKey", BigDecimal.ONE)
.schema(PriceFloorSchema.of("|", List.of(size))))
.build();
}

Expand Down
Loading

0 comments on commit 587c2fb

Please sign in to comment.