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

Refactor Default Account Resolving #3012

Merged
merged 10 commits into from
Mar 26, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ public class Ortb2RequestFactory {
private static final ConditionalLogger EMPTY_ACCOUNT_LOGGER = new ConditionalLogger("empty_account", logger);
private static final ConditionalLogger UNKNOWN_ACCOUNT_LOGGER = new ConditionalLogger("unknown_account", logger);

private final boolean enforceValidAccount;
private final double logSamplingRate;
private final List<String> blacklistedAccounts;
private final UidsCookieService uidsCookieService;
Expand All @@ -108,8 +107,7 @@ public class Ortb2RequestFactory {
private final Metrics metrics;
private final Clock clock;

public Ortb2RequestFactory(boolean enforceValidAccount,
double logSamplingRate,
public Ortb2RequestFactory(double logSamplingRate,
List<String> blacklistedAccounts,
UidsCookieService uidsCookieService,
ActivityInfrastructureCreator activityInfrastructureCreator,
Expand All @@ -126,7 +124,6 @@ public Ortb2RequestFactory(boolean enforceValidAccount,
Metrics metrics,
Clock clock) {

this.enforceValidAccount = enforceValidAccount;
this.logSamplingRate = logSamplingRate;
this.blacklistedAccounts = Objects.requireNonNull(blacklistedAccounts);
this.uidsCookieService = Objects.requireNonNull(uidsCookieService);
Expand Down Expand Up @@ -418,20 +415,26 @@ private String validateIfAccountBlacklisted(String accountId) {
return accountId;
}

private Future<Account> loadAccount(Timeout timeout,
HttpRequestContext httpRequest,
String accountId) {

final Future<Account> accountFuture = StringUtils.isBlank(accountId)
? responseForEmptyAccount(httpRequest)
: applicationSettings.getAccountById(accountId, timeout)
.compose(this::ensureAccountActive,
exception -> accountFallback(exception, accountId, httpRequest));
private Future<Account> loadAccount(Timeout timeout, HttpRequestContext httpRequest, String accountId) {
if (StringUtils.isBlank(accountId)) {
EMPTY_ACCOUNT_LOGGER.warn(accountErrorMessage("Account not specified", httpRequest), logSamplingRate);
}

return accountFuture
return applicationSettings.getAccountById(accountId, timeout)
.compose(this::ensureAccountActive)
.recover(exception -> wrapFailure(exception, accountId, httpRequest))
.onFailure(ignored -> metrics.updateAccountRequestRejectedByInvalidAccountMetrics(accountId));
}

private Future<Account> ensureAccountActive(Account account) {
final String accountId = account.getId();

return account.getStatus() == AccountStatus.inactive
? Future.failedFuture(
new UnauthorizedAccountException("Account %s is inactive".formatted(accountId), accountId))
: Future.succeededFuture(account);
}

/**
* Extracts publisher id either from {@link BidRequest}.app.publisher or {@link BidRequest}.site.publisher.
* If neither is present returns empty string.
Expand Down Expand Up @@ -466,48 +469,26 @@ private String parentAccountIdFromExtPublisher(ExtPublisher extPublisher) {
return extPublisherPrebid != null ? StringUtils.stripToNull(extPublisherPrebid.getParentAccount()) : null;
}

private Future<Account> responseForEmptyAccount(HttpRequestContext httpRequest) {
EMPTY_ACCOUNT_LOGGER.warn(accountErrorMessage("Account not specified", httpRequest), logSamplingRate);
return responseForUnknownAccount(StringUtils.EMPTY);
}

private static String accountErrorMessage(String message, HttpRequestContext httpRequest) {
return "%s, Url: %s and Referer: %s".formatted(
message,
httpRequest.getAbsoluteUri(),
httpRequest.getHeaders().get(HttpUtil.REFERER_HEADER));
}

private Future<Account> accountFallback(Throwable exception,
String accountId,
HttpRequestContext httpRequest) {

if (exception instanceof PreBidException) {
private Future<Account> wrapFailure(Throwable exception, String accountId, HttpRequestContext httpRequest) {
if (exception instanceof UnauthorizedAccountException) {
return Future.failedFuture(exception);
} else if (exception instanceof PreBidException) {
UNKNOWN_ACCOUNT_LOGGER.warn(accountErrorMessage(exception.getMessage(), httpRequest), 100);
} else {
metrics.updateAccountRequestRejectedByFailedFetch(accountId);
logger.warn("Error occurred while fetching account: {0}", exception.getMessage());
logger.debug("Error occurred while fetching account", exception);
}

// hide all errors occurred while fetching account
return responseForUnknownAccount(accountId);
}

private Future<Account> responseForUnknownAccount(String accountId) {
return enforceValidAccount
? Future.failedFuture(new UnauthorizedAccountException(
"Unauthorized account id: " + accountId, accountId))
: Future.succeededFuture(Account.empty(accountId));
return Future.failedFuture(
new UnauthorizedAccountException("Unauthorized account id: " + accountId, accountId));
}

private Future<Account> ensureAccountActive(Account account) {
final String accountId = account.getId();

return account.getStatus() == AccountStatus.inactive
? Future.failedFuture(new UnauthorizedAccountException(
"Account %s is inactive".formatted(accountId), accountId))
: Future.succeededFuture(account);
private static String accountErrorMessage(String message, HttpRequestContext httpRequest) {
return "%s, Url: %s and Referer: %s".formatted(
message,
httpRequest.getAbsoluteUri(),
httpRequest.getHeaders().get(HttpUtil.REFERER_HEADER));
}

private ExtRequest enrichExtRequest(ExtRequest ext, Account account) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.prebid.server.settings.model.AccountPrivacySandboxCookieDeprecationConfig;
import org.prebid.server.util.HttpUtil;

import java.util.Objects;
import java.util.Optional;

public class CookieDeprecationService {
Expand All @@ -26,20 +25,12 @@ public class CookieDeprecationService {
private static final String DEVICE_EXT_COOKIE_DEPRECATION_FIELD_NAME = "cdep";
private static final long DEFAULT_MAX_AGE = 604800L;

private final Account defaultAccount;

public CookieDeprecationService(Account defaultAccount) {
this.defaultAccount = Objects.requireNonNull(defaultAccount);
}

public PartitionedCookie makeCookie(Account account, RoutingContext routingContext) {
final Account resolvedAccount = account.isEmpty() ? defaultAccount : account;

if (hasDeprecationCookieInRequest(routingContext) || isCookieDeprecationDisabled(resolvedAccount)) {
if (hasDeprecationCookieInRequest(routingContext) || isCookieDeprecationDisabled(account)) {
return null;
}

final Long maxAge = getCookieDeprecationConfig(resolvedAccount)
final Long maxAge = getCookieDeprecationConfig(account)
.map(AccountPrivacySandboxCookieDeprecationConfig::getTtlSec)
.orElse(DEFAULT_MAX_AGE);

Expand All @@ -61,13 +52,9 @@ public BidRequest updateBidRequestDevice(BidRequest bidRequest, AuctionContext a
.get(HttpUtil.SEC_COOKIE_DEPRECATION);

final Account account = auctionContext.getAccount();
final Account resolvedAccount = account.isEmpty() ? defaultAccount : account;
final Device device = bidRequest.getDevice();

if (secCookieDeprecation == null
|| containsCookieDeprecation(device)
|| isCookieDeprecationDisabled(resolvedAccount)) {

if (secCookieDeprecation == null || containsCookieDeprecation(device) || isCookieDeprecationDisabled(account)) {
return bidRequest;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,10 +310,7 @@ private void periodicFetch(String accountId) {
}

private Future<Account> accountById(String accountId) {
return StringUtils.isBlank(accountId)
? Future.succeededFuture()
: applicationSettings
.getAccountById(accountId, timeoutFactory.create(ACCOUNT_FETCH_TIMEOUT_MS))
return applicationSettings.getAccountById(accountId, timeoutFactory.create(ACCOUNT_FETCH_TIMEOUT_MS))
.recover(ignored -> Future.succeededFuture());
AntoxaAntoxic marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package org.prebid.server.floors;

import io.vertx.core.Future;
import io.vertx.core.logging.Logger;
import io.vertx.core.logging.LoggerFactory;
import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import org.prebid.server.exception.PreBidException;
import org.prebid.server.log.ConditionalLogger;
Expand All @@ -17,7 +17,7 @@

import java.util.Objects;

public class PriceFloorsConfigResolver {
public class PriceFloorsConfigValidator {

private static final Logger logger = LoggerFactory.getLogger(EnrichingApplicationSettings.class);
private static final ConditionalLogger conditionalLogger = new ConditionalLogger(logger);
Expand All @@ -35,13 +35,9 @@ public class PriceFloorsConfigResolver {
private static final int MAX_ENFORCE_RATE_VALUE = 100;
private static final long DEFAULT_MAX_AGE_SEC_VALUE = 86400L;

private final Account defaultAccount;
private final Metrics metrics;
private final AccountPriceFloorsConfig defaultFloorsConfig;

public PriceFloorsConfigResolver(Account defaultAccount, Metrics metrics) {
this.defaultAccount = Objects.requireNonNull(defaultAccount);
this.defaultFloorsConfig = getFloorsConfig(defaultAccount);
public PriceFloorsConfigValidator(Metrics metrics) {
this.metrics = Objects.requireNonNull(metrics);
}

Expand All @@ -51,10 +47,10 @@ private static AccountPriceFloorsConfig getFloorsConfig(Account account) {
return ObjectUtil.getIfNotNull(auctionConfig, AccountAuctionConfig::getPriceFloors);
}

public Future<Account> updateFloorsConfig(Account account) {
public Account validate(Account account) {
try {
validatePriceFloorConfig(account, defaultFloorsConfig);
return Future.succeededFuture(account);
validatePriceFloorConfig(account);
return account;
} catch (PreBidException e) {
final String message = "Account with id '%s' has invalid config: %s"
.formatted(account.getId(), e.getMessage());
Expand All @@ -65,75 +61,52 @@ public Future<Account> updateFloorsConfig(Account account) {
conditionalLogger.error(message, 0.01d);
}

return Future.succeededFuture(fallbackToDefaultConfig(account));
return fallbackToDefaultConfig(account);
}

private static void validatePriceFloorConfig(Account account, AccountPriceFloorsConfig defaultFloorsConfig) {
private static void validatePriceFloorConfig(Account account) {
final AccountPriceFloorsConfig floorsConfig = getFloorsConfig(account);
if (floorsConfig == null) {
return;
}
final Integer accountEnforceRate = floorsConfig.getEnforceFloorsRate();
final Integer enforceFloorsRate = accountEnforceRate != null
? accountEnforceRate
: ObjectUtil.getIfNotNull(defaultFloorsConfig, AccountPriceFloorsConfig::getEnforceFloorsRate);
if (enforceFloorsRate != null
&& isNotInRange(enforceFloorsRate, MIN_ENFORCE_RATE_VALUE, MAX_ENFORCE_RATE_VALUE)) {
throw new PreBidException(
invalidPriceFloorsPropertyMessage("enforce-floors-rate", enforceFloorsRate));

final Integer enforceRate = floorsConfig.getEnforceFloorsRate();
if (enforceRate != null && isNotInRange(enforceRate, MIN_ENFORCE_RATE_VALUE, MAX_ENFORCE_RATE_VALUE)) {
throw new PreBidException(invalidPriceFloorsPropertyMessage("enforce-floors-rate", enforceRate));
}

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

validatePriceFloorsFetchConfig(fetchConfig, defaultFetchConfig);
validatePriceFloorsFetchConfig(fetchConfig);
}

private static void validatePriceFloorsFetchConfig(AccountPriceFloorsFetchConfig fetchConfig,
AccountPriceFloorsFetchConfig defaultFetchConfig) {
private static void validatePriceFloorsFetchConfig(AccountPriceFloorsFetchConfig fetchConfig) {
if (fetchConfig == null) {
return;
}

final Long accountMaxAgeSec = fetchConfig.getMaxAgeSec();
final Long defaultMaxAgeSec =
ObjectUtil.getIfNotNull(defaultFetchConfig, AccountPriceFloorsFetchConfig::getMaxAgeSec);
final long maxAgeSec = accountMaxAgeSec != null
? accountMaxAgeSec
: defaultMaxAgeSec != null ? defaultMaxAgeSec : DEFAULT_MAX_AGE_SEC_VALUE;
final long maxAgeSec = ObjectUtils.defaultIfNull(fetchConfig.getMaxAgeSec(), DEFAULT_MAX_AGE_SEC_VALUE);
if (isNotInRange(maxAgeSec, MIN_MAX_AGE_SEC_VALUE, MAX_AGE_SEC_VALUE)) {
throw new PreBidException(invalidPriceFloorsPropertyMessage("max-age-sec", maxAgeSec));
}

final Long accountPeriodicSec = fetchConfig.getPeriodSec();
final Long periodicSec = accountPeriodicSec != null
? accountPeriodicSec
: ObjectUtil.getIfNotNull(defaultFetchConfig, AccountPriceFloorsFetchConfig::getPeriodSec);
final Long periodicSec = fetchConfig.getPeriodSec();
if (periodicSec != null && isNotInRange(periodicSec, MIN_PERIODIC_SEC_VALUE, maxAgeSec)) {
throw new PreBidException(invalidPriceFloorsPropertyMessage("period-sec", periodicSec));
}

final Long accountTimeout = fetchConfig.getTimeout();
final Long timeout = accountTimeout != null
? accountTimeout
: ObjectUtil.getIfNotNull(defaultFetchConfig, AccountPriceFloorsFetchConfig::getTimeout);
final Long timeout = fetchConfig.getTimeout();
if (timeout != null && isNotInRange(timeout, MIN_TIMEOUT_MS_VALUE, MAX_TIMEOUT_MS_VALUE)) {
throw new PreBidException(invalidPriceFloorsPropertyMessage("timeout-ms", timeout));
}

final Long accountMaxRules = fetchConfig.getMaxRules();
final Long maxRules = accountMaxRules != null
? accountMaxRules
: ObjectUtil.getIfNotNull(defaultFetchConfig, AccountPriceFloorsFetchConfig::getMaxRules);
final Long maxRules = fetchConfig.getMaxRules();
if (maxRules != null && isNotInRange(maxRules, MIN_RULES_VALUE, MAX_RULES_VALUE)) {
throw new PreBidException(invalidPriceFloorsPropertyMessage("max-rules", maxRules));
}

final Long accountMaxFileSize = fetchConfig.getMaxFileSize();
final Long maxFileSize = accountMaxFileSize != null
? accountMaxFileSize
: ObjectUtil.getIfNotNull(defaultFetchConfig, AccountPriceFloorsFetchConfig::getMaxFileSize);
final Long maxFileSize = fetchConfig.getMaxFileSize();
if (maxFileSize != null && isNotInRange(maxFileSize, MIN_FILE_SIZE_VALUE, MAX_FILE_SIZE_VALUE)) {
throw new PreBidException(invalidPriceFloorsPropertyMessage("max-file-size-kb", maxFileSize));
}
Expand All @@ -147,13 +120,9 @@ private static String invalidPriceFloorsPropertyMessage(String property, Object
return "Invalid price-floors property '%s', value passed: %s".formatted(property, value);
}

private Account fallbackToDefaultConfig(Account account) {
final AccountAuctionConfig auctionConfig = account.getAuction();
final AccountPriceFloorsConfig defaultPriceFloorsConfig =
ObjectUtil.getIfNotNull(defaultAccount.getAuction(), AccountAuctionConfig::getPriceFloors);

private static Account fallbackToDefaultConfig(Account account) {
return account.toBuilder()
.auction(auctionConfig.toBuilder().priceFloors(defaultPriceFloorsConfig).build())
.auction(account.getAuction().toBuilder().priceFloors(null).build())
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import io.vertx.core.logging.LoggerFactory;
import io.vertx.ext.web.RoutingContext;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.StringUtils;
import org.prebid.server.activity.infrastructure.creator.ActivityInfrastructureCreator;
import org.prebid.server.analytics.model.CookieSyncEvent;
import org.prebid.server.analytics.reporter.AnalyticsReporterDelegator;
Expand Down Expand Up @@ -158,10 +157,7 @@ private Future<CookieSyncContext> fillWithAccount(CookieSyncContext cookieSyncCo
}

private Future<Account> accountById(String accountId, Timeout timeout) {
return StringUtils.isBlank(accountId)
? Future.succeededFuture(Account.empty(accountId))
: applicationSettings.getAccountById(accountId, timeout)
.otherwise(Account.empty(accountId));
return applicationSettings.getAccountById(accountId, timeout).otherwise(Account.empty(accountId));
}

private CookieSyncContext fillWithGppContext(CookieSyncContext cookieSyncContext) {
Expand Down
5 changes: 1 addition & 4 deletions src/main/java/org/prebid/server/handler/SetuidHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -184,10 +184,7 @@ private Future<SetuidContext> toSetuidContext(RoutingContext routingContext) {
}

private Future<Account> accountById(String accountId, Timeout timeout) {
return StringUtils.isBlank(accountId)
? Future.succeededFuture(Account.empty(accountId))
: applicationSettings.getAccountById(accountId, timeout)
.otherwise(Account.empty(accountId));
return applicationSettings.getAccountById(accountId, timeout).otherwise(Account.empty(accountId));
}

private SetuidContext fillWithActivityInfrastructure(SetuidContext setuidContext) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,15 @@ public CachingApplicationSettings(ApplicationSettings delegate,
*/
@Override
public Future<Account> getAccountById(String accountId, Timeout timeout) {
return getFromCacheOrDelegate(
accountCache,
accountToErrorCache,
accountId,
timeout,
delegate::getAccountById,
event -> metrics.updateSettingsCacheEventMetric(MetricName.account, event));
return StringUtils.isBlank(accountId)
? delegate.getAccountById(accountId, timeout)
: getFromCacheOrDelegate(
CTMBNara marked this conversation as resolved.
Show resolved Hide resolved
accountCache,
accountToErrorCache,
accountId,
timeout,
delegate::getAccountById,
event -> metrics.updateSettingsCacheEventMetric(MetricName.account, event));
}

/**
Expand Down
Loading
Loading