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

Remove AdUnitConfig support as it is not used anymore #1157

Merged
merged 1 commit into from
Feb 18, 2021
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -25,11 +25,6 @@ public interface ApplicationSettings {
*/
Future<Account> getAccountById(String accountId, Timeout timeout);

/**
* Returns configuration for the given adUnit config ID.
*/
Future<String> getAdUnitConfigById(String adUnitConfigId, Timeout timeout);

/**
* Fetches stored requests and imps by IDs.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ public class CachingApplicationSettings implements ApplicationSettings {

private final Map<String, Account> accountCache;
private final Map<String, String> accountToErrorCache;
private final Map<String, String> adUnitConfigCache;
private final SettingsCache cache;
private final SettingsCache ampCache;
private final SettingsCache videoCache;
Expand All @@ -55,7 +54,6 @@ public CachingApplicationSettings(ApplicationSettings delegate,
this.delegate = Objects.requireNonNull(delegate);
this.accountCache = SettingsCache.createCache(ttl, size);
this.accountToErrorCache = SettingsCache.createCache(ttl, size);
this.adUnitConfigCache = SettingsCache.createCache(ttl, size);
this.cache = Objects.requireNonNull(cache);
this.ampCache = Objects.requireNonNull(ampCache);
this.videoCache = Objects.requireNonNull(videoCache);
Expand All @@ -76,20 +74,6 @@ public Future<Account> getAccountById(String accountId, Timeout timeout) {
event -> metrics.updateSettingsCacheEventMetric(MetricName.account, event));
}

/**
* Retrieves adUnit config from cache or delegates it to original fetcher.
*/
@Override
public Future<String> getAdUnitConfigById(String adUnitConfigId, Timeout timeout) {
return getFromCacheOrDelegate(
adUnitConfigCache,
accountToErrorCache,
adUnitConfigId,
timeout,
delegate::getAdUnitConfigById,
CachingApplicationSettings::noOp);
}

/**
* Retrieves stored data from cache or delegates it to original fetcher.
*/
Expand Down Expand Up @@ -247,6 +231,4 @@ public void invalidateAccountCache(String accountId) {
logger.debug("Account with id {0} was invalidated", accountId);
}

private static <ANY> void noOp(ANY any) {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

import io.vertx.core.Future;
import org.prebid.server.execution.Timeout;
import org.prebid.server.settings.model.Account;
import org.prebid.server.settings.helper.StoredDataFetcher;
import org.prebid.server.settings.model.Account;
import org.prebid.server.settings.model.StoredDataResult;
import org.prebid.server.settings.model.StoredResponseDataResult;

Expand Down Expand Up @@ -51,15 +51,6 @@ public Future<Account> getAccountById(String accountId, Timeout timeout) {
return proxy.getAccountById(accountId, timeout);
}

/**
* Runs a process to get AdUnit config by id from a chain of retrievers
* and returns {@link Future&lt;{@link String}&gt;}.
*/
@Override
public Future<String> getAdUnitConfigById(String adUnitConfigId, Timeout timeout) {
return proxy.getAdUnitConfigById(adUnitConfigId, timeout);
}

/**
* Runs a process to get stored requests by a collection of ids from a chain of retrievers
* and returns {@link Future&lt;{@link StoredDataResult }&gt;}.
Expand Down Expand Up @@ -114,12 +105,6 @@ public Future<Account> getAccountById(String accountId, Timeout timeout) {
next != null ? next::getAccountById : null);
}

@Override
public Future<String> getAdUnitConfigById(String adUnitConfigId, Timeout timeout) {
return getConfig(adUnitConfigId, timeout, applicationSettings::getAdUnitConfigById,
next != null ? next::getAdUnitConfigById : null);
}

private static <T> Future<T> getConfig(String key, Timeout timeout,
BiFunction<String, Timeout, Future<T>> retriever,
BiFunction<String, Timeout, Future<T>> nextRetriever) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@ public Future<Account> getAccountById(String accountId, Timeout timeout) {
.otherwise(Account.empty(accountId).merge(defaultAccount));
}

@Override
public Future<String> getAdUnitConfigById(String adUnitConfigId, Timeout timeout) {
return delegate.getAdUnitConfigById(adUnitConfigId, timeout);
}

@Override
public Future<StoredDataResult> getStoredData(String accountId,
Set<String> requestIds,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,10 @@
import io.vertx.core.buffer.Buffer;
import io.vertx.core.file.FileSystem;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import org.prebid.server.exception.PreBidException;
import org.prebid.server.execution.Timeout;
import org.prebid.server.settings.model.Account;
import org.prebid.server.settings.model.AdUnitConfig;
import org.prebid.server.settings.model.SettingsFile;
import org.prebid.server.settings.model.StoredDataResult;
import org.prebid.server.settings.model.StoredDataType;
Expand Down Expand Up @@ -41,7 +39,6 @@ public class FileApplicationSettings implements ApplicationSettings {
private static final String JSON_SUFFIX = ".json";

private final Map<String, Account> accounts;
private final Map<String, String> configs;
private final Map<String, String> storedIdToRequest;
private final Map<String, String> storedIdToImp;
private final Map<String, String> storedIdToSeatBid;
Expand All @@ -56,10 +53,6 @@ public FileApplicationSettings(FileSystem fileSystem, String settingsFileName, S
Account::getId,
Function.identity());

configs = toMap(settingsFile.getConfigs(),
AdUnitConfig::getId,
config -> ObjectUtils.defaultIfNull(config.getConfig(), StringUtils.EMPTY));

this.storedIdToRequest = readStoredData(fileSystem, Objects.requireNonNull(storedRequestsDir));
this.storedIdToImp = readStoredData(fileSystem, Objects.requireNonNull(storedImpsDir));
this.storedIdToSeatBid = readStoredData(fileSystem, Objects.requireNonNull(storedResponsesDir));
Expand All @@ -70,11 +63,6 @@ public Future<Account> getAccountById(String accountId, Timeout timeout) {
return mapValueToFuture(accounts, accountId, "Account");
}

@Override
public Future<String> getAdUnitConfigById(String adUnitConfigId, Timeout timeout) {
return mapValueToFuture(configs, adUnitConfigId, "AdUnitConfig");
}

/**
* Creates {@link StoredDataResult} by checking if any ids are missed in storedRequest map
* and adding an error to list for each missed Id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,6 @@ private Set<Account> toAccountsResult(int statusCode, String body, Set<String> a
return MapUtils.isNotEmpty(accounts) ? new HashSet<>(accounts.values()) : Collections.emptySet();
}

/**
* Not supported and returns failed result.
*/
@Override
public Future<String> getAdUnitConfigById(String adUnitConfigId, Timeout timeout) {
return Future.failedFuture(new PreBidException("Not supported"));
}

/**
* Runs a process to get stored requests by a collection of ids from http service
* and returns {@link Future&lt;{@link StoredDataResult }&gt;}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,19 +131,6 @@ public Future<Account> getAccountById(String accountId, Timeout timeout) {
.compose(result -> failedIfNull(result, accountId, "Account"));
}

/**
* Runs a process to get AdUnit config by id from database
* and returns {@link Future&lt;{@link String}&gt;}.
*/
@Override
public Future<String> getAdUnitConfigById(String adUnitConfigId, Timeout timeout) {
return jdbcClient.executeQuery("SELECT config FROM s2sconfig_config where uuid = ? LIMIT 1",
Collections.singletonList(adUnitConfigId),
result -> mapToModelOrError(result, row -> row.getString(0)),
timeout)
.compose(result -> failedIfNull(result, adUnitConfigId, "AdUnitConfig"));
}

/**
* Transforms the first row of {@link ResultSet} to required object or returns null.
* <p>
Expand Down
13 changes: 0 additions & 13 deletions src/main/java/org/prebid/server/settings/model/AdUnitConfig.java

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,5 @@ public class SettingsFile {

List<Account> accounts;

List<AdUnitConfig> configs;

List<String> domains;
}
Original file line number Diff line number Diff line change
Expand Up @@ -158,82 +158,6 @@ public void getAccountByIdShouldUpdateMetrics() {
verify(metrics).updateSettingsCacheEventMetric(eq(MetricName.account), eq(MetricName.hit));
}

@Test
public void getAdUnitConfigByIdShouldReturnResultFromCacheOnSuccessiveCalls() {
// given
given(applicationSettings.getAdUnitConfigById(eq("adUnitConfigId"), same(timeout)))
.willReturn(Future.succeededFuture("config"));

// when
final Future<String> future = cachingApplicationSettings.getAdUnitConfigById("adUnitConfigId", timeout);
cachingApplicationSettings.getAdUnitConfigById("adUnitConfigId", timeout);

// then
assertThat(future.succeeded()).isTrue();
assertThat(future.result()).isEqualTo("config");
verify(applicationSettings).getAdUnitConfigById(eq("adUnitConfigId"), same(timeout));
verifyNoMoreInteractions(applicationSettings);
}

@Test
public void getAdUnitConfigByIdShouldPropagateFailure() {
// given
given(applicationSettings.getAdUnitConfigById(anyString(), any()))
.willReturn(Future.failedFuture(new PreBidException("error")));

// when
final Future<String> future =
cachingApplicationSettings.getAdUnitConfigById("adUnitConfigId", timeout);

// then
assertThat(future.failed()).isTrue();
assertThat(future.cause())
.isInstanceOf(PreBidException.class)
.hasMessage("error");
}

@Test
public void getAdUnitConfigByIdShouldCachePreBidException() {
// given
given(applicationSettings.getAdUnitConfigById(anyString(), any()))
.willReturn(Future.failedFuture(new PreBidException("error")));

// when
cachingApplicationSettings.getAdUnitConfigById("adUnitConfigId", timeout);
cachingApplicationSettings.getAdUnitConfigById("adUnitConfigId", timeout);
cachingApplicationSettings.getAdUnitConfigById("adUnitConfigId", timeout);
final Future<String> lastFuture =
cachingApplicationSettings.getAdUnitConfigById("adUnitConfigId", timeout);

// then
verify(applicationSettings).getAdUnitConfigById(anyString(), any());
assertThat(lastFuture.failed()).isTrue();
assertThat(lastFuture.cause())
.isInstanceOf(PreBidException.class)
.hasMessage("error");
}

@Test
public void getAdUnitConfigByIdShouldNotCacheNotPreBidException() {
// given
given(applicationSettings.getAdUnitConfigById(anyString(), any()))
.willReturn(Future.failedFuture(new InvalidRequestException("error")));

// when

cachingApplicationSettings.getAdUnitConfigById("adUnitConfigId", timeout);
cachingApplicationSettings.getAdUnitConfigById("adUnitConfigId", timeout);
final Future<String> lastFuture =
cachingApplicationSettings.getAdUnitConfigById("adUnitConfigId", timeout);

// then
verify(applicationSettings, times(3)).getAdUnitConfigById(anyString(), any());
assertThat(lastFuture.failed()).isTrue();
assertThat(lastFuture.cause())
.isInstanceOf(InvalidRequestException.class)
.hasMessage("error");
}

@Test
public void getStoredDataShouldReturnResultOnSuccessiveCalls() {
// given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,55 +107,6 @@ public void getAccountByIdShouldReturnEmptyResultIfAllDelegatesFail() {
assertThat(future.cause().getMessage()).isEqualTo("error2");
}

@Test
public void getAdUnitConfigByIdShouldReturnConfigFromFirstDelegateIfPresent() {
// given
given(delegate1.getAdUnitConfigById(anyString(), any()))
.willReturn(Future.succeededFuture("adUnitConfig1"));

// when
final Future<String> future = compositeApplicationSettings.getAdUnitConfigById("ignore", null);

// then
assertThat(future.succeeded()).isTrue();
assertThat(future.result()).isEqualTo("adUnitConfig1");
verifyZeroInteractions(delegate2);
}

@Test
public void getAdUnitConfigByIdShouldReturnConfigFromSecondDelegateIfFirstDelegateFails() {
// given
given(delegate1.getAdUnitConfigById(anyString(), any()))
.willReturn(Future.failedFuture(new PreBidException("error1")));

given(delegate2.getAdUnitConfigById(anyString(), any()))
.willReturn(Future.succeededFuture("adUnitConfig1"));

// when
final Future<String> future = compositeApplicationSettings.getAdUnitConfigById("ignore", null);

// then
assertThat(future.succeeded()).isTrue();
assertThat(future.result()).isEqualTo("adUnitConfig1");
}

@Test
public void getAdUnitConfigByIdShouldReturnEmptyResultIfAllDelegatesFail() {
// given
given(delegate1.getAdUnitConfigById(anyString(), any()))
.willReturn(Future.failedFuture(new PreBidException("error1")));

given(delegate2.getAdUnitConfigById(anyString(), any()))
.willReturn(Future.failedFuture(new PreBidException("error2")));

// when
final Future<String> future = compositeApplicationSettings.getAdUnitConfigById("ignore", null);

// then
assertThat(future.failed()).isTrue();
assertThat(future.cause().getMessage()).isEqualTo("error2");
}

@Test
public void getStoredDataShouldReturnResultFromFirstDelegateIfPresent() {
// given
Expand Down
Loading