From 465cf2ab79942011a3fcef9651ce09d99b1be892 Mon Sep 17 00:00:00 2001 From: Sergii Chernysh Date: Mon, 10 Aug 2020 15:12:04 +0300 Subject: [PATCH 1/2] Implement default bid request support --- docs/config-app.md | 3 + .../auction/StoredRequestProcessor.java | 125 ++++++++++++------ .../server/auction/VideoRequestFactory.java | 8 +- .../auction/VideoStoredRequestProcessor.java | 92 ++++++++++--- .../JsonMerger.java} | 8 +- ...figuration.java => JsonConfiguration.java} | 8 +- .../spring/config/ServiceConfiguration.java | 54 +++++--- .../auction/StoredRequestProcessorTest.java | 11 +- .../VideoStoredRequestProcessorTest.java | 20 ++- .../JsonMergerTest.java} | 8 +- 10 files changed, 245 insertions(+), 92 deletions(-) rename src/main/java/org/prebid/server/{util/JsonMergeUtil.java => json/JsonMerger.java} (93%) rename src/main/java/org/prebid/server/spring/config/{JacksonConfiguration.java => JsonConfiguration.java} (68%) rename src/test/java/org/prebid/server/{util/JsonMergeUtilTest.java => json/JsonMergerTest.java} (93%) diff --git a/docs/config-app.md b/docs/config-app.md index bbb318e3402..3a9c8d0ccc3 100644 --- a/docs/config-app.md +++ b/docs/config-app.md @@ -50,6 +50,9 @@ Removes and downloads file again if depending service cant process probably corr - `.remote-file-syncer.http-client.connect-timeout-ms` - set the connect timeout. - `.remote-file-syncer.http-client.max-redirects` - set the maximum amount of HTTP redirections to follow. A value of 0 (the default) prevents redirections from being followed. +## Default bid request +- `default-request.file.path` - path to a JSON file containing the default request + ## Auction (OpenRTB) - `auction.blacklisted-accounts` - comma separated list of blacklisted account IDs. - `auction.blacklisted-apps` - comma separated list of blacklisted applications IDs, requests from which should not be processed. diff --git a/src/main/java/org/prebid/server/auction/StoredRequestProcessor.java b/src/main/java/org/prebid/server/auction/StoredRequestProcessor.java index 86a74d7d218..cd9d98405f1 100644 --- a/src/main/java/org/prebid/server/auction/StoredRequestProcessor.java +++ b/src/main/java/org/prebid/server/auction/StoredRequestProcessor.java @@ -5,12 +5,14 @@ import com.iab.openrtb.request.Imp; import com.iab.openrtb.request.Video; import io.vertx.core.Future; +import io.vertx.core.file.FileSystem; import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; import org.prebid.server.exception.InvalidRequestException; import org.prebid.server.execution.Timeout; import org.prebid.server.execution.TimeoutFactory; import org.prebid.server.json.JacksonMapper; +import org.prebid.server.json.JsonMerger; import org.prebid.server.metric.Metrics; import org.prebid.server.proto.openrtb.ext.request.ExtImp; import org.prebid.server.proto.openrtb.ext.request.ExtImpPrebid; @@ -20,7 +22,6 @@ import org.prebid.server.settings.ApplicationSettings; import org.prebid.server.settings.model.StoredDataResult; import org.prebid.server.settings.model.VideoStoredDataResult; -import org.prebid.server.util.JsonMergeUtil; import java.util.ArrayList; import java.util.Collections; @@ -39,25 +40,48 @@ public class StoredRequestProcessor { private final long defaultTimeout; + private final BidRequest defaultBidRequest; private final ApplicationSettings applicationSettings; private final TimeoutFactory timeoutFactory; private final Metrics metrics; private final JacksonMapper mapper; - private JsonMergeUtil jsonMergeUtil; + private final JsonMerger jsonMerger; - public StoredRequestProcessor(long defaultTimeout, - ApplicationSettings applicationSettings, - Metrics metrics, - TimeoutFactory timeoutFactory, - JacksonMapper mapper) { + private StoredRequestProcessor(long defaultTimeout, + BidRequest defaultBidRequest, + ApplicationSettings applicationSettings, + Metrics metrics, + TimeoutFactory timeoutFactory, + JacksonMapper mapper, + JsonMerger jsonMerger) { this.defaultTimeout = defaultTimeout; - this.applicationSettings = Objects.requireNonNull(applicationSettings); - this.timeoutFactory = Objects.requireNonNull(timeoutFactory); - this.metrics = Objects.requireNonNull(metrics); - this.mapper = Objects.requireNonNull(mapper); + this.defaultBidRequest = defaultBidRequest; + this.applicationSettings = applicationSettings; + this.timeoutFactory = timeoutFactory; + this.metrics = metrics; + this.mapper = mapper; + this.jsonMerger = jsonMerger; + } - jsonMergeUtil = new JsonMergeUtil(mapper); + public static StoredRequestProcessor create(long defaultTimeout, + String defaultBidRequestPath, + FileSystem fileSystem, + ApplicationSettings applicationSettings, + Metrics metrics, + TimeoutFactory timeoutFactory, + JacksonMapper mapper, + JsonMerger jsonMerger) { + + return new StoredRequestProcessor( + defaultTimeout, + readBidRequest( + defaultBidRequestPath, Objects.requireNonNull(fileSystem), Objects.requireNonNull(mapper)), + Objects.requireNonNull(applicationSettings), + Objects.requireNonNull(metrics), + Objects.requireNonNull(timeoutFactory), + Objects.requireNonNull(mapper), + Objects.requireNonNull(jsonMerger)); } /** @@ -89,24 +113,16 @@ Future processStoredRequests(BidRequest bidRequest) { applicationSettings.getStoredData(requestIds, impIds, timeout(bidRequest)) .compose(storedDataResult -> updateMetrics(storedDataResult, requestIds, impIds)); - return storedRequestsToBidRequest(storedDataFuture, bidRequest, - bidRequestToStoredRequestId.get(bidRequest), impToStoredRequestId); - } - - private Future updateMetrics(StoredDataResult storedDataResult, Set requestIds, - Set impIds) { - requestIds.forEach( - id -> metrics.updateStoredRequestMetric(storedDataResult.getStoredIdToRequest().containsKey(id))); - impIds.forEach(id -> metrics.updateStoredImpsMetric(storedDataResult.getStoredIdToImp().containsKey(id))); - - return Future.succeededFuture(storedDataResult); + return storedRequestsToBidRequest( + storedDataFuture, bidRequest, bidRequestToStoredRequestId.get(bidRequest), impToStoredRequestId); } /** * Fetches AMP request from the source. */ Future processAmpRequest(String ampRequestId) { - final BidRequest bidRequest = BidRequest.builder().build(); + final BidRequest bidRequest = defaultBidRequest != null ? defaultBidRequest : BidRequest.builder().build(); + final Future ampStoredDataFuture = applicationSettings.getAmpStoredData( Collections.singleton(ampRequestId), Collections.emptySet(), timeout(bidRequest)) @@ -130,9 +146,27 @@ Future videoStoredDataResult(List imps, List .map(storedDataResult -> makeVideoStoredDataResult(storedDataResult, storedIdToImpId, errors)); } + private Future updateMetrics(StoredDataResult storedDataResult, Set requestIds, + Set impIds) { + requestIds.forEach( + id -> metrics.updateStoredRequestMetric(storedDataResult.getStoredIdToRequest().containsKey(id))); + impIds.forEach(id -> metrics.updateStoredImpsMetric(storedDataResult.getStoredIdToImp().containsKey(id))); + + return Future.succeededFuture(storedDataResult); + } + + private static BidRequest readBidRequest( + String defaultBidRequestPath, FileSystem fileSystem, JacksonMapper mapper) { + + return StringUtils.isNotBlank(defaultBidRequestPath) + ? mapper.decodeValue(fileSystem.readFileBlocking(defaultBidRequestPath), BidRequest.class) + : null; + } + private VideoStoredDataResult makeVideoStoredDataResult(StoredDataResult storedDataResult, Map storedIdToImpId, List errors) { + final Map storedIdToStoredImp = storedDataResult.getStoredIdToImp(); final Map impIdToStoredVideo = new HashMap<>(); @@ -170,8 +204,10 @@ private Video parseVideoFromImp(String storedJson) { } private Future storedRequestsToBidRequest(Future storedDataFuture, - BidRequest bidRequest, String storedBidRequestId, + BidRequest bidRequest, + String storedBidRequestId, Map impsToStoredRequestId) { + return storedDataFuture .recover(exception -> Future.failedFuture(new InvalidRequestException( String.format("Stored request fetching failed: %s", exception.getMessage())))) @@ -185,21 +221,31 @@ private Future storedRequestsToBidRequest(Future s /** * Runs {@link BidRequest} and {@link Imp}s merge processes. */ - private BidRequest mergeBidRequestAndImps(BidRequest bidRequest, String storedRequestId, - Map impToStoredId, StoredDataResult storedDataResult) { - return mergeBidRequestImps(mergeBidRequest(bidRequest, storedRequestId, storedDataResult), - impToStoredId, storedDataResult); + private BidRequest mergeBidRequestAndImps(BidRequest bidRequest, + String storedRequestId, + Map impToStoredId, + StoredDataResult storedDataResult) { + + return mergeBidRequestImps( + mergeBidRequest(mergeDefaultRequest(bidRequest), storedRequestId, storedDataResult), + impToStoredId, + storedDataResult); + } + + private BidRequest mergeDefaultRequest(BidRequest bidRequest) { + return jsonMerger.merge(bidRequest, defaultBidRequest, BidRequest.class); } /** * Merges original request with request from stored request source. Values from original request * has higher priority than stored request values. */ - private BidRequest mergeBidRequest(BidRequest originalRequest, String storedRequestId, - StoredDataResult storedDataResult) { + private BidRequest mergeBidRequest( + BidRequest originalRequest, String storedRequestId, StoredDataResult storedDataResult) { + final String storedRequest = storedDataResult.getStoredIdToRequest().get(storedRequestId); return StringUtils.isNotBlank(storedRequestId) - ? jsonMergeUtil.merge(originalRequest, storedRequest, storedRequestId, BidRequest.class) + ? jsonMerger.merge(originalRequest, storedRequest, storedRequestId, BidRequest.class) : originalRequest; } @@ -207,8 +253,9 @@ private BidRequest mergeBidRequest(BidRequest originalRequest, String storedRequ * Merges {@link Imp}s from original request with Imps from stored request source. Values from original request * has higher priority than stored request values. */ - private BidRequest mergeBidRequestImps(BidRequest bidRequest, Map impToStoredId, - StoredDataResult storedDataResult) { + private BidRequest mergeBidRequestImps( + BidRequest bidRequest, Map impToStoredId, StoredDataResult storedDataResult) { + if (impToStoredId.isEmpty()) { return bidRequest; } @@ -218,7 +265,7 @@ private BidRequest mergeBidRequestImps(BidRequest bidRequest, Map i final String storedRequestId = impToStoredId.get(imp); if (storedRequestId != null) { final String storedImp = storedDataResult.getStoredIdToImp().get(storedRequestId); - final Imp mergedImp = jsonMergeUtil.merge(imp, storedImp, storedRequestId, Imp.class); + final Imp mergedImp = jsonMerger.merge(imp, storedImp, storedRequestId, Imp.class); mergedImps.set(i, mergedImp); } } @@ -226,10 +273,10 @@ private BidRequest mergeBidRequestImps(BidRequest bidRequest, Map i } /** - * Maps object to its StoredRequestId if exists. If object's extension contains storedRequest field, expected that - * it includes id too, in another case error about missed id in stored request will be added to error list. - * Gathers all errors into list, and in case if it is not empty, throws {@link InvalidRequestException} with - * list of errors. + * Maps object to its StoredRequestId if exists. If object's extension contains storedRequest field, expected + * that it includes id too, in another case error about missed id in stored request will be added to error list. + * Gathers all errors into list, and in case if it is not empty, throws {@link InvalidRequestException} with list + * of errors. */ private Map mapStoredRequestHolderToStoredRequestId( List storedRequestHolders, Function storedRequestExtractor) { diff --git a/src/main/java/org/prebid/server/auction/VideoRequestFactory.java b/src/main/java/org/prebid/server/auction/VideoRequestFactory.java index b0d5c55439d..6ec1289d8c4 100644 --- a/src/main/java/org/prebid/server/auction/VideoRequestFactory.java +++ b/src/main/java/org/prebid/server/auction/VideoRequestFactory.java @@ -35,10 +35,13 @@ public class VideoRequestFactory { private final TimeoutResolver timeoutResolver; private final JacksonMapper mapper; - public VideoRequestFactory(int maxRequestSize, boolean enforceStoredRequest, + public VideoRequestFactory(int maxRequestSize, + boolean enforceStoredRequest, VideoStoredRequestProcessor storedRequestProcessor, - AuctionRequestFactory auctionRequestFactory, TimeoutResolver timeoutResolver, + AuctionRequestFactory auctionRequestFactory, + TimeoutResolver timeoutResolver, JacksonMapper mapper) { + this.enforceStoredRequest = enforceStoredRequest; this.maxRequestSize = maxRequestSize; this.storedRequestProcessor = Objects.requireNonNull(storedRequestProcessor); @@ -132,6 +135,7 @@ private Future> createBidRequest(RoutingContext routin BidRequestVideo bidRequestVideo, String storedVideoId, Set podConfigIds) { + return storedRequestProcessor.processVideoRequest(storedVideoId, podConfigIds, bidRequestVideo) .map(bidRequestToErrors -> fillImplicitParameters(routingContext, bidRequestToErrors)) .map(this::validateRequest); diff --git a/src/main/java/org/prebid/server/auction/VideoStoredRequestProcessor.java b/src/main/java/org/prebid/server/auction/VideoStoredRequestProcessor.java index 1a82530aa56..dd17aef3411 100644 --- a/src/main/java/org/prebid/server/auction/VideoStoredRequestProcessor.java +++ b/src/main/java/org/prebid/server/auction/VideoStoredRequestProcessor.java @@ -16,6 +16,7 @@ import com.iab.openrtb.request.video.PodError; import com.iab.openrtb.request.video.Podconfig; import io.vertx.core.Future; +import io.vertx.core.file.FileSystem; import io.vertx.core.logging.Logger; import io.vertx.core.logging.LoggerFactory; import org.apache.commons.collections4.CollectionUtils; @@ -27,6 +28,7 @@ import org.prebid.server.exception.InvalidRequestException; import org.prebid.server.execution.TimeoutFactory; import org.prebid.server.json.JacksonMapper; +import org.prebid.server.json.JsonMerger; import org.prebid.server.metric.Metrics; import org.prebid.server.proto.openrtb.ext.ExtIncludeBrandCategory; import org.prebid.server.proto.openrtb.ext.request.ExtRequest; @@ -36,7 +38,6 @@ import org.prebid.server.proto.openrtb.ext.request.ExtRequestTargeting; import org.prebid.server.settings.ApplicationSettings; import org.prebid.server.settings.model.StoredDataResult; -import org.prebid.server.util.JsonMergeUtil; import org.prebid.server.validation.VideoRequestValidator; import java.util.ArrayList; @@ -59,37 +60,74 @@ public class VideoStoredRequestProcessor { private static final String DEFAULT_CURRENCY = "USD"; private static final String DEFAULT_BUYERUID = "appnexus"; - private final ApplicationSettings applicationSettings; - private final VideoRequestValidator validator; private final boolean enforceStoredRequest; private final List blacklistedAccounts; + private final long defaultTimeout; + private final String currency; private final BidRequest defaultBidRequest; + private final ApplicationSettings applicationSettings; + private final VideoRequestValidator validator; private final Metrics metrics; private final TimeoutResolver timeoutResolver; private final TimeoutFactory timeoutFactory; - private final long defaultTimeout; - private final String currency; private final JacksonMapper mapper; - private JsonMergeUtil jsonMergeUtil; - - public VideoStoredRequestProcessor(ApplicationSettings applicationSettings, VideoRequestValidator validator, - boolean enforceStoredRequest, List blacklistedAccounts, - BidRequest defaultBidRequest, Metrics metrics, TimeoutFactory timeoutFactory, - TimeoutResolver timeoutResolver, long defaultTimeout, String adServerCurrency, - JacksonMapper mapper) { - this.applicationSettings = Objects.requireNonNull(applicationSettings); - this.validator = Objects.requireNonNull(validator); + private final JsonMerger jsonMerger; + + private VideoStoredRequestProcessor(boolean enforceStoredRequest, + List blacklistedAccounts, + long defaultTimeout, + String adServerCurrency, + BidRequest defaultBidRequest, + ApplicationSettings applicationSettings, + VideoRequestValidator validator, + Metrics metrics, + TimeoutFactory timeoutFactory, + TimeoutResolver timeoutResolver, + JacksonMapper mapper, + JsonMerger jsonMerger) { + this.enforceStoredRequest = enforceStoredRequest; this.blacklistedAccounts = blacklistedAccounts; - this.defaultBidRequest = Objects.requireNonNull(defaultBidRequest); - this.timeoutFactory = Objects.requireNonNull(timeoutFactory); - this.timeoutResolver = Objects.requireNonNull(timeoutResolver); - this.metrics = Objects.requireNonNull(metrics); this.defaultTimeout = defaultTimeout; this.currency = StringUtils.isBlank(adServerCurrency) ? DEFAULT_CURRENCY : adServerCurrency; - this.mapper = Objects.requireNonNull(mapper); + this.defaultBidRequest = defaultBidRequest; + this.applicationSettings = applicationSettings; + this.validator = validator; + this.metrics = metrics; + this.timeoutFactory = timeoutFactory; + this.timeoutResolver = timeoutResolver; + this.mapper = mapper; + this.jsonMerger = jsonMerger; + } - jsonMergeUtil = new JsonMergeUtil(mapper); + public static VideoStoredRequestProcessor create(boolean enforceStoredRequest, + List blacklistedAccounts, + long defaultTimeout, + String adServerCurrency, + String defaultBidRequestPath, + FileSystem fileSystem, + ApplicationSettings applicationSettings, + VideoRequestValidator validator, + Metrics metrics, + TimeoutFactory timeoutFactory, + TimeoutResolver timeoutResolver, + JacksonMapper mapper, + JsonMerger jsonMerger) { + + return new VideoStoredRequestProcessor( + enforceStoredRequest, + Objects.requireNonNull(blacklistedAccounts), + defaultTimeout, + adServerCurrency, + readBidRequest( + defaultBidRequestPath, Objects.requireNonNull(fileSystem), Objects.requireNonNull(mapper)), + Objects.requireNonNull(applicationSettings), + Objects.requireNonNull(validator), + Objects.requireNonNull(metrics), + Objects.requireNonNull(timeoutFactory), + Objects.requireNonNull(timeoutResolver), + Objects.requireNonNull(mapper), + Objects.requireNonNull(jsonMerger)); } /** @@ -108,6 +146,14 @@ Future> processVideoRequest(String storedBidRequestId, String.format("Stored request fetching failed: %s", exception.getMessage())))); } + private static BidRequest readBidRequest( + String defaultBidRequestPath, FileSystem fileSystem, JacksonMapper mapper) { + + return StringUtils.isNotBlank(defaultBidRequestPath) + ? mapper.decodeValue(fileSystem.readFileBlocking(defaultBidRequestPath), BidRequest.class) + : null; + } + private Future updateMetrics(StoredDataResult storedDataResult, Set requestIds, Set impIds) { requestIds.forEach( @@ -142,7 +188,7 @@ private BidRequestVideo mergeBidRequest(BidRequestVideo originalRequest, String } return StringUtils.isNotBlank(storedRequest) - ? jsonMergeUtil.merge(originalRequest, storedRequest, storedRequestId, BidRequestVideo.class) + ? jsonMerger.merge(originalRequest, storedRequest, storedRequestId, BidRequestVideo.class) : originalRequest; } @@ -248,7 +294,9 @@ private static Video updateVideo(Video video, Integer maxDuration, Integer minDu } private BidRequest mergeWithDefaultBidRequest(BidRequestVideo validatedVideoRequest, List imps) { - final BidRequest.BidRequestBuilder bidRequestBuilder = defaultBidRequest.toBuilder(); + final BidRequest.BidRequestBuilder bidRequestBuilder = + defaultBidRequest != null ? defaultBidRequest.toBuilder() : BidRequest.builder(); + final Site site = validatedVideoRequest.getSite(); if (site != null) { final Site.SiteBuilder siteBuilder = site.toBuilder(); diff --git a/src/main/java/org/prebid/server/util/JsonMergeUtil.java b/src/main/java/org/prebid/server/json/JsonMerger.java similarity index 93% rename from src/main/java/org/prebid/server/util/JsonMergeUtil.java rename to src/main/java/org/prebid/server/json/JsonMerger.java index 4c70592248c..885ca4da02a 100644 --- a/src/main/java/org/prebid/server/util/JsonMergeUtil.java +++ b/src/main/java/org/prebid/server/json/JsonMerger.java @@ -1,4 +1,4 @@ -package org.prebid.server.util; +package org.prebid.server.json; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; @@ -6,17 +6,15 @@ import com.github.fge.jsonpatch.mergepatch.JsonMergePatch; import org.apache.commons.lang3.ObjectUtils; import org.prebid.server.exception.InvalidRequestException; -import org.prebid.server.json.JacksonMapper; import java.io.IOException; import java.util.Objects; -// TODO: refactor to be instance instead of util -public class JsonMergeUtil { +public class JsonMerger { private final JacksonMapper mapper; - public JsonMergeUtil(JacksonMapper mapper) { + public JsonMerger(JacksonMapper mapper) { this.mapper = Objects.requireNonNull(mapper); } diff --git a/src/main/java/org/prebid/server/spring/config/JacksonConfiguration.java b/src/main/java/org/prebid/server/spring/config/JsonConfiguration.java similarity index 68% rename from src/main/java/org/prebid/server/spring/config/JacksonConfiguration.java rename to src/main/java/org/prebid/server/spring/config/JsonConfiguration.java index 52239591ca5..eda4e2fb32d 100644 --- a/src/main/java/org/prebid/server/spring/config/JacksonConfiguration.java +++ b/src/main/java/org/prebid/server/spring/config/JsonConfiguration.java @@ -1,15 +1,21 @@ package org.prebid.server.spring.config; import org.prebid.server.json.JacksonMapper; +import org.prebid.server.json.JsonMerger; import org.prebid.server.json.ObjectMapperProvider; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @Configuration -public class JacksonConfiguration { +public class JsonConfiguration { @Bean JacksonMapper jacksonMapper() { return new JacksonMapper(ObjectMapperProvider.mapper()); } + + @Bean + JsonMerger jsonMerger(JacksonMapper mapper) { + return new JsonMerger(mapper); + } } diff --git a/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java b/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java index 364ffa0e557..c6bd9c9db08 100644 --- a/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java +++ b/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java @@ -1,9 +1,9 @@ package org.prebid.server.spring.config; -import com.iab.openrtb.request.BidRequest; import de.malkusch.whoisServerList.publicSuffixList.PublicSuffixList; import de.malkusch.whoisServerList.publicSuffixList.PublicSuffixListFactory; import io.vertx.core.Vertx; +import io.vertx.core.file.FileSystem; import io.vertx.core.http.HttpClientOptions; import io.vertx.core.net.JksOptions; import org.prebid.server.auction.AmpRequestFactory; @@ -39,6 +39,7 @@ import org.prebid.server.identity.NoneIdGenerator; import org.prebid.server.identity.UUIDIdGenerator; import org.prebid.server.json.JacksonMapper; +import org.prebid.server.json.JsonMerger; import org.prebid.server.manager.AdminManager; import org.prebid.server.metric.Metrics; import org.prebid.server.optout.GoogleRecaptchaVerifier; @@ -245,27 +246,39 @@ VideoResponseFactory videoResponseFactory(JacksonMapper mapper) { @Bean VideoStoredRequestProcessor videoStoredRequestProcessor( - ApplicationSettings applicationSettings, @Value("${auction.video.stored-required:#{false}}") boolean enforceStoredRequest, @Value("${auction.blacklisted-accounts}") String blacklistedAccountsString, - BidRequest defaultVideoBidRequest, + @Value("${video.stored-requests-timeout-ms}") long defaultTimeoutMs, + @Value("${auction.ad-server-currency:#{null}}") String adServerCurrency, + @Value("${default-request.file.path:#{null}}") String defaultBidRequestPath, + FileSystem fileSystem, + ApplicationSettings applicationSettings, + VideoRequestValidator videoRequestValidator, Metrics metrics, TimeoutFactory timeoutFactory, TimeoutResolver timeoutResolver, - @Value("${video.stored-requests-timeout-ms}") long defaultTimeoutMs, - @Value("${auction.ad-server-currency:#{null}}") String adServerCurrency, - JacksonMapper mapper) { - - final List blacklistedAccounts = splitCommaSeparatedString(blacklistedAccountsString); + JacksonMapper mapper, + JsonMerger jsonMerger) { - return new VideoStoredRequestProcessor(applicationSettings, new VideoRequestValidator(), enforceStoredRequest, - blacklistedAccounts, defaultVideoBidRequest, metrics, timeoutFactory, timeoutResolver, defaultTimeoutMs, - adServerCurrency, mapper); + return VideoStoredRequestProcessor.create( + enforceStoredRequest, + splitCommaSeparatedString(blacklistedAccountsString), + defaultTimeoutMs, + adServerCurrency, + defaultBidRequestPath, + fileSystem, + applicationSettings, + videoRequestValidator, + metrics, + timeoutFactory, + timeoutResolver, + mapper, + jsonMerger); } @Bean - BidRequest defaultVideoBidRequest() { - return BidRequest.builder().build(); + VideoRequestValidator videoRequestValidator() { + return new VideoRequestValidator(); } @Bean @@ -437,12 +450,23 @@ ExchangeService exchangeService( @Bean StoredRequestProcessor storedRequestProcessor( @Value("${auction.stored-requests-timeout-ms}") long defaultTimeoutMs, + @Value("${default-request.file.path:#{null}}") String defaultBidRequestPath, + FileSystem fileSystem, ApplicationSettings applicationSettings, Metrics metrics, TimeoutFactory timeoutFactory, - JacksonMapper mapper) { + JacksonMapper mapper, + JsonMerger jsonMerger) { - return new StoredRequestProcessor(defaultTimeoutMs, applicationSettings, metrics, timeoutFactory, mapper); + return StoredRequestProcessor.create( + defaultTimeoutMs, + defaultBidRequestPath, + fileSystem, + applicationSettings, + metrics, + timeoutFactory, + mapper, + jsonMerger); } @Bean diff --git a/src/test/java/org/prebid/server/auction/StoredRequestProcessorTest.java b/src/test/java/org/prebid/server/auction/StoredRequestProcessorTest.java index f3b4dffcc11..262f28c7e3e 100644 --- a/src/test/java/org/prebid/server/auction/StoredRequestProcessorTest.java +++ b/src/test/java/org/prebid/server/auction/StoredRequestProcessorTest.java @@ -9,6 +9,7 @@ import com.iab.openrtb.request.Imp.ImpBuilder; import com.iab.openrtb.request.Video; import io.vertx.core.Future; +import io.vertx.core.file.FileSystem; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -20,6 +21,7 @@ import org.prebid.server.exception.InvalidRequestException; import org.prebid.server.execution.Timeout; import org.prebid.server.execution.TimeoutFactory; +import org.prebid.server.json.JsonMerger; import org.prebid.server.metric.Metrics; import org.prebid.server.proto.openrtb.ext.request.ExtImp; import org.prebid.server.proto.openrtb.ext.request.ExtImpPrebid; @@ -62,6 +64,8 @@ public class StoredRequestProcessorTest extends VertxTest { @Rule public final MockitoRule mockitoRule = MockitoJUnit.rule(); + @Mock + private FileSystem fileSystem; @Mock private ApplicationSettings applicationSettings; @Mock @@ -72,12 +76,15 @@ public class StoredRequestProcessorTest extends VertxTest { @Before public void setUp() { final TimeoutFactory timeoutFactory = new TimeoutFactory(Clock.fixed(Instant.now(), ZoneId.systemDefault())); - storedRequestProcessor = new StoredRequestProcessor( + storedRequestProcessor = StoredRequestProcessor.create( DEFAULT_TIMEOUT, + null, + fileSystem, applicationSettings, metrics, timeoutFactory, - jacksonMapper); + jacksonMapper, + new JsonMerger(jacksonMapper)); } @Test diff --git a/src/test/java/org/prebid/server/auction/VideoStoredRequestProcessorTest.java b/src/test/java/org/prebid/server/auction/VideoStoredRequestProcessorTest.java index 4c64e691d21..8eea9e29bd8 100644 --- a/src/test/java/org/prebid/server/auction/VideoStoredRequestProcessorTest.java +++ b/src/test/java/org/prebid/server/auction/VideoStoredRequestProcessorTest.java @@ -12,6 +12,7 @@ import com.iab.openrtb.request.video.PodError; import com.iab.openrtb.request.video.Podconfig; import io.vertx.core.Future; +import io.vertx.core.file.FileSystem; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -22,6 +23,7 @@ import org.prebid.server.auction.model.WithPodErrors; import org.prebid.server.exception.InvalidRequestException; import org.prebid.server.execution.TimeoutFactory; +import org.prebid.server.json.JsonMerger; import org.prebid.server.metric.Metrics; import org.prebid.server.proto.openrtb.ext.ExtIncludeBrandCategory; import org.prebid.server.proto.openrtb.ext.request.ExtRequest; @@ -57,6 +59,8 @@ public class VideoStoredRequestProcessorTest extends VertxTest { @Rule public final MockitoRule mockitoRule = MockitoJUnit.rule(); + @Mock + private FileSystem fileSystem; @Mock private ApplicationSettings applicationSettings; @Mock @@ -72,8 +76,20 @@ public class VideoStoredRequestProcessorTest extends VertxTest { @Before public void setUp() { - target = new VideoStoredRequestProcessor(applicationSettings, validator, false, emptyList(), - BidRequest.builder().build(), metrics, timeoutFactory, timeoutResolver, 2000L, "USD", jacksonMapper); + target = VideoStoredRequestProcessor.create( + false, + emptyList(), + 2000L, + "USD", + null, + fileSystem, + applicationSettings, + validator, + metrics, + timeoutFactory, + timeoutResolver, + jacksonMapper, + new JsonMerger(jacksonMapper)); } @Test diff --git a/src/test/java/org/prebid/server/util/JsonMergeUtilTest.java b/src/test/java/org/prebid/server/json/JsonMergerTest.java similarity index 93% rename from src/test/java/org/prebid/server/util/JsonMergeUtilTest.java rename to src/test/java/org/prebid/server/json/JsonMergerTest.java index 11dda2a2d0f..27837079b17 100644 --- a/src/test/java/org/prebid/server/util/JsonMergeUtilTest.java +++ b/src/test/java/org/prebid/server/json/JsonMergerTest.java @@ -1,4 +1,4 @@ -package org.prebid.server.util; +package org.prebid.server.json; import com.iab.openrtb.request.App; import com.iab.openrtb.request.Publisher; @@ -10,13 +10,13 @@ import static org.assertj.core.api.Assertions.assertThat; -public class JsonMergeUtilTest extends VertxTest { +public class JsonMergerTest extends VertxTest { - private JsonMergeUtil target; + private JsonMerger target; @Before public void setUp() { - target = new JsonMergeUtil(jacksonMapper); + target = new JsonMerger(jacksonMapper); } @Test From 4b726969ccff4bc83e277b644da541d48dce33ad Mon Sep 17 00:00:00 2001 From: Sergii Chernysh Date: Mon, 10 Aug 2020 15:39:47 +0300 Subject: [PATCH 2/2] Add tests and documentation --- docs/developers/default-request.md | 29 +++++++ .../auction/StoredRequestProcessorTest.java | 81 +++++++++++++++++++ .../VideoStoredRequestProcessorTest.java | 11 ++- 3 files changed, 119 insertions(+), 2 deletions(-) create mode 100644 docs/developers/default-request.md diff --git a/docs/developers/default-request.md b/docs/developers/default-request.md new file mode 100644 index 00000000000..08bf1041009 --- /dev/null +++ b/docs/developers/default-request.md @@ -0,0 +1,29 @@ +# Server Based Global Default Request + +This allows a default request to be defined that allows the server to set up some defaults for all incoming +requests. A stored request(s) referenced by a bid request override default request, and of course any options specified +directly in the bid request override both. The default request is only read on server startup, it is meant as an +installation static default rather than a dynamic tuning option. + +## Config Options + +Following config options are exposed to support this feature. +```yaml +default-request: + file: + path : path/to/default/request.json +``` + +The `path` option is the path to a JSON file containing the default request JSON. +```json +{ + "tmax": "2000", + "regs": { + "ext": { + "gdpr": 1 + } + } +} +``` +This will be JSON merged into the incoming requests at the top level. These will be used as fallbacks which can be +overridden by both Stored Requests _and_ the incoming HTTP request payload. diff --git a/src/test/java/org/prebid/server/auction/StoredRequestProcessorTest.java b/src/test/java/org/prebid/server/auction/StoredRequestProcessorTest.java index 262f28c7e3e..c9269d86714 100644 --- a/src/test/java/org/prebid/server/auction/StoredRequestProcessorTest.java +++ b/src/test/java/org/prebid/server/auction/StoredRequestProcessorTest.java @@ -9,6 +9,7 @@ import com.iab.openrtb.request.Imp.ImpBuilder; import com.iab.openrtb.request.Video; import io.vertx.core.Future; +import io.vertx.core.buffer.Buffer; import io.vertx.core.file.FileSystem; import org.junit.Before; import org.junit.Rule; @@ -52,6 +53,7 @@ import static org.assertj.core.api.Assertions.entry; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anySet; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.verify; @@ -165,6 +167,52 @@ public void shouldReturnMergedBidRequest() throws IOException { .build()); } + @Test + public void shouldReturnMergedDefaultAndBidRequest() throws IOException { + // given + given(fileSystem.readFileBlocking(anyString())) + .willReturn(Buffer.buffer(mapper.writeValueAsString(BidRequest.builder().at(1).build()))); + + final TimeoutFactory timeoutFactory = new TimeoutFactory(Clock.fixed(Instant.now(), ZoneId.systemDefault())); + storedRequestProcessor = StoredRequestProcessor.create( + DEFAULT_TIMEOUT, + "path/to/default/request.json", + fileSystem, + applicationSettings, + metrics, + timeoutFactory, + jacksonMapper, + new JsonMerger(jacksonMapper)); + + final BidRequest bidRequest = givenBidRequest(builder -> builder + .ext(ExtRequest.of(ExtRequestPrebid.builder() + .storedrequest(ExtStoredRequest.of("123")) + .build()))); + + final String storedRequestBidRequestJson = mapper.writeValueAsString(BidRequest.builder() + .id("test-request-id") + .tmax(1000L) + .imp(singletonList(Imp.builder().build())) + .build()); + given(applicationSettings.getStoredData(anySet(), anySet(), any())) + .willReturn(Future.succeededFuture( + StoredDataResult.of(singletonMap("123", storedRequestBidRequestJson), emptyMap(), + emptyList()))); + + // when + final Future bidRequestFuture = storedRequestProcessor.processStoredRequests(bidRequest); + + // then + assertThat(bidRequestFuture.succeeded()).isTrue(); + assertThat(bidRequestFuture.result()).isEqualTo(BidRequest.builder() + .id("test-request-id") + .at(1) + .tmax(1000L) + .imp(singletonList(Imp.builder().build())) + .ext(ExtRequest.of(ExtRequestPrebid.builder().storedrequest(ExtStoredRequest.of("123")).build())) + .build()); + } + @Test public void shouldReturnAmpRequest() throws IOException { // given @@ -183,6 +231,39 @@ public void shouldReturnAmpRequest() throws IOException { .build()); } + @Test + public void shouldReturnMergedDefaultAndAmpRequest() throws IOException { + // given + given(fileSystem.readFileBlocking(anyString())) + .willReturn(Buffer.buffer(mapper.writeValueAsString(BidRequest.builder().at(1).build()))); + + final TimeoutFactory timeoutFactory = new TimeoutFactory(Clock.fixed(Instant.now(), ZoneId.systemDefault())); + storedRequestProcessor = StoredRequestProcessor.create( + DEFAULT_TIMEOUT, + "path/to/default/request.json", + fileSystem, + applicationSettings, + metrics, + timeoutFactory, + jacksonMapper, + new JsonMerger(jacksonMapper)); + + given(applicationSettings.getAmpStoredData(anySet(), anySet(), any())) + .willReturn(Future.succeededFuture(StoredDataResult.of( + singletonMap("123", mapper.writeValueAsString( + BidRequest.builder().id("test-request-id").build())), emptyMap(), emptyList()))); + + // when + final Future bidRequestFuture = storedRequestProcessor.processAmpRequest("123"); + + // then + assertThat(bidRequestFuture.succeeded()).isTrue(); + assertThat(bidRequestFuture.result()).isEqualTo(BidRequest.builder() + .id("test-request-id") + .at(1) + .build()); + } + @Test public void shouldReturnFailedFutureWhenStoredBidRequestJsonIsNotValid() { // given diff --git a/src/test/java/org/prebid/server/auction/VideoStoredRequestProcessorTest.java b/src/test/java/org/prebid/server/auction/VideoStoredRequestProcessorTest.java index 8eea9e29bd8..99fa4081d41 100644 --- a/src/test/java/org/prebid/server/auction/VideoStoredRequestProcessorTest.java +++ b/src/test/java/org/prebid/server/auction/VideoStoredRequestProcessorTest.java @@ -1,5 +1,6 @@ package org.prebid.server.auction; +import com.fasterxml.jackson.core.JsonProcessingException; import com.iab.openrtb.request.BidRequest; import com.iab.openrtb.request.Content; import com.iab.openrtb.request.Imp; @@ -12,6 +13,7 @@ import com.iab.openrtb.request.video.PodError; import com.iab.openrtb.request.video.Podconfig; import io.vertx.core.Future; +import io.vertx.core.buffer.Buffer; import io.vertx.core.file.FileSystem; import org.junit.Before; import org.junit.Rule; @@ -47,6 +49,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.never; @@ -75,13 +78,16 @@ public class VideoStoredRequestProcessorTest extends VertxTest { private VideoStoredRequestProcessor target; @Before - public void setUp() { + public void setUp() throws JsonProcessingException { + given(fileSystem.readFileBlocking(anyString())) + .willReturn(Buffer.buffer(mapper.writeValueAsString(BidRequest.builder().at(1).build()))); + target = VideoStoredRequestProcessor.create( false, emptyList(), 2000L, "USD", - null, + "path/to/default/request.json", fileSystem, applicationSettings, validator, @@ -174,6 +180,7 @@ public void shouldReturnFutureWithMergedStoredAndDefaultRequest() { .build(); final BidRequest expectedMergedRequest = BidRequest.builder() .id("bid_id") + .at(1) .imp(Arrays.asList(expectedImp1, expectedImp2)) .user(User.builder().buyeruid("appnexus").yob(123).gender("gender").keywords("keywords").build()) .site(Site.builder().id("siteId").content(content).build())