Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Price Floors: Max Rules and Max Dimensions #3630

Merged
merged 4 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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::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()));
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.getMaxSchemaDims()));

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.getMaxSchemaDims();
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.getMaxSchemaDims();
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 maxSchemaDims;
}
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 maxSchemaDims;

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

Expand Down
5 changes: 4 additions & 1 deletion src/main/resources/application.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,17 @@ settings:
"enabled": false,
"timeout-ms": 5000,
"max-rules": 0,
"max-schema-dims": 5,
"max-file-size-kb": 200,
"max-age-sec": 86400,
"period-sec": 3600
},
"enforce-floors-rate": 100,
"adjust-for-bid-adjustment": true,
"enforce-deal-floors": true,
"use-dynamic-data": true
"use-dynamic-data": true,
"max-rules": 100,
"max-schema-dims": 3
}
}
}
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.maxSchemaDims(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
Loading