From 2c95a7073e96f5020a69389e5413c0935bbaaf16 Mon Sep 17 00:00:00 2001 From: Serhii Nahornyi Date: Tue, 8 Jun 2021 10:43:52 +0300 Subject: [PATCH 1/5] Implement x-prebid headers support --- .../server/bidder/HttpBidderRequester.java | 62 ++++++++++++++++--- .../spring/config/ServiceConfiguration.java | 8 ++- .../java/org/prebid/server/util/HttpUtil.java | 3 +- .../bidder/HttpBidderRequesterTest.java | 45 +++++++++++++- 4 files changed, 106 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/prebid/server/bidder/HttpBidderRequester.java b/src/main/java/org/prebid/server/bidder/HttpBidderRequester.java index 215ecfe7ac2..eafdfdce30e 100644 --- a/src/main/java/org/prebid/server/bidder/HttpBidderRequester.java +++ b/src/main/java/org/prebid/server/bidder/HttpBidderRequester.java @@ -1,5 +1,6 @@ package org.prebid.server.bidder; +import com.iab.openrtb.request.App; import com.iab.openrtb.request.BidRequest; import io.netty.channel.ConnectTimeoutException; import io.netty.handler.codec.http.HttpResponseStatus; @@ -21,6 +22,11 @@ import org.prebid.server.bidder.model.HttpResponse; import org.prebid.server.bidder.model.Result; import org.prebid.server.execution.Timeout; +import org.prebid.server.proto.openrtb.ext.request.ExtApp; +import org.prebid.server.proto.openrtb.ext.request.ExtAppPrebid; +import org.prebid.server.proto.openrtb.ext.request.ExtRequest; +import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid; +import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidChannel; import org.prebid.server.proto.openrtb.ext.response.ExtHttpCall; import org.prebid.server.util.HttpUtil; import org.prebid.server.vertx.http.HttpClient; @@ -50,19 +56,22 @@ public class HttpBidderRequester { private static final Logger logger = LoggerFactory.getLogger(HttpBidderRequester.class); - private static final Set HEADERS_TO_COPY = Collections.singleton(HttpUtil.SEC_GPC.toString()); + private static final Set HEADERS_TO_COPY = Collections.singleton(HttpUtil.SEC_GPC_HEADER.toString()); private final HttpClient httpClient; private final BidderRequestCompletionTrackerFactory completionTrackerFactory; private final BidderErrorNotifier bidderErrorNotifier; + private final String pbsVersion; public HttpBidderRequester(HttpClient httpClient, BidderRequestCompletionTrackerFactory completionTrackerFactory, - BidderErrorNotifier bidderErrorNotifier) { + BidderErrorNotifier bidderErrorNotifier, + String pbsVersion) { this.httpClient = Objects.requireNonNull(httpClient); this.completionTrackerFactory = completionTrackerFactoryOrFallback(completionTrackerFactory); this.bidderErrorNotifier = Objects.requireNonNull(bidderErrorNotifier); + this.pbsVersion = pbsVersion; } /** @@ -77,7 +86,8 @@ public Future requestBids(Bidder bidder, final Result>> httpRequestsWithErrors = bidder.makeHttpRequests(bidRequest); final List bidderErrors = httpRequestsWithErrors.getErrors(); - final List> httpRequests = enrichRequests(httpRequestsWithErrors.getValue(), routingContext); + final List> httpRequests = + enrichRequests(httpRequestsWithErrors.getValue(), routingContext, bidRequest); if (CollectionUtils.isEmpty(httpRequests)) { return emptyBidderSeatBidWithErrors(bidderErrors); @@ -108,16 +118,19 @@ public Future requestBids(Bidder bidder, .map(ignored -> resultBuilder.toBidderSeatBid(debugEnabled)); } - private static List> enrichRequests(List> httpRequests, - RoutingContext routingContext) { + private List> enrichRequests(List> httpRequests, + RoutingContext routingContext, + BidRequest bidRequest) { return httpRequests.stream().map(httpRequest -> httpRequest.toBuilder() - .headers(enrichHeaders(httpRequest.getHeaders(), routingContext.request().headers())) + .headers(enrichHeaders(httpRequest.getHeaders(), routingContext.request().headers(), bidRequest)) .build()) .collect(Collectors.toList()); } - private static MultiMap enrichHeaders(MultiMap bidderRequestHeaders, MultiMap originalRequestHeaders) { + private MultiMap enrichHeaders(MultiMap bidderRequestHeaders, + MultiMap originalRequestHeaders, + BidRequest bidRequest) { // some bidders has headers on class level, so we create copy to not affect them final MultiMap bidderRequestHeadersCopy = copyMultiMap(bidderRequestHeaders); @@ -125,6 +138,7 @@ private static MultiMap enrichHeaders(MultiMap bidderRequestHeaders, MultiMap or .filter(entry -> HEADERS_TO_COPY.contains(entry.getKey()) && !bidderRequestHeadersCopy.contains(entry.getKey())) .forEach(entry -> bidderRequestHeadersCopy.add(entry.getKey(), entry.getValue())); + addXPrebidHeader(bidderRequestHeadersCopy, bidRequest); return bidderRequestHeadersCopy; } @@ -136,6 +150,40 @@ private static MultiMap copyMultiMap(MultiMap source) { return copiedMultiMap; } + private void addXPrebidHeader(MultiMap headers, BidRequest bidRequest) { + final String channelRecord = resolveChannelVersionRecord(bidRequest.getExt()); + final String sdkRecord = resolveSdkVersionRecord(bidRequest.getApp()); + final String pbsRecord = createNameVersionRecord("pbs-java", pbsVersion); + final String value = Stream.of(channelRecord, sdkRecord, pbsRecord) + .filter(Objects::nonNull) + .collect(Collectors.joining(",")); + HttpUtil.addHeaderIfValueIsNotEmpty(headers, HttpUtil.X_PREBID_HEADER, value); + } + + private static String resolveChannelVersionRecord(ExtRequest extRequest) { + final ExtRequestPrebid extPrebid = extRequest != null ? extRequest.getPrebid() : null; + final ExtRequestPrebidChannel channel = extPrebid != null ? extPrebid.getChannel() : null; + final String channelName = channel != null ? channel.getName() : null; + final String channelVersion = channel != null ? channel.getVersion() : null; + + return createNameVersionRecord(channelName, channelVersion); + } + + private static String resolveSdkVersionRecord(App app) { + final ExtApp extApp = app != null ? app.getExt() : null; + final ExtAppPrebid extPrebid = extApp != null ? extApp.getPrebid() : null; + final String sdkSource = extPrebid != null ? extPrebid.getSource() : null; + final String sdkVersion = extPrebid != null ? extPrebid.getVersion() : null; + + return createNameVersionRecord(sdkSource, sdkVersion); + } + + private static String createNameVersionRecord(String name, String version) { + return StringUtils.isNotEmpty(name) && StringUtils.isNotEmpty(version) + ? String.format("%s/%s", name, version) + : null; + } + private boolean isStoredResponse(List> httpRequests, String storedResponse, String bidder) { if (StringUtils.isBlank(storedResponse)) { return false; 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 74444f530a4..d5dc1afa25e 100644 --- a/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java +++ b/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java @@ -462,9 +462,13 @@ BidderCatalog bidderCatalog(List bidderDeps) { HttpBidderRequester httpBidderRequester( HttpClient httpClient, @Autowired(required = false) BidderRequestCompletionTrackerFactory bidderRequestCompletionTrackerFactory, - BidderErrorNotifier bidderErrorNotifier) { + BidderErrorNotifier bidderErrorNotifier, + VersionInfo versionInfo) { - return new HttpBidderRequester(httpClient, bidderRequestCompletionTrackerFactory, bidderErrorNotifier); + return new HttpBidderRequester(httpClient, + bidderRequestCompletionTrackerFactory, + bidderErrorNotifier, + versionInfo.getVersion()); } @Bean diff --git a/src/main/java/org/prebid/server/util/HttpUtil.java b/src/main/java/org/prebid/server/util/HttpUtil.java index c3f6b3278b1..2827bb172db 100644 --- a/src/main/java/org/prebid/server/util/HttpUtil.java +++ b/src/main/java/org/prebid/server/util/HttpUtil.java @@ -32,7 +32,7 @@ public final class HttpUtil { public static final CharSequence X_REQUEST_AGENT_HEADER = HttpHeaders.createOptimized("X-Request-Agent"); public static final CharSequence ORIGIN_HEADER = HttpHeaders.createOptimized("Origin"); public static final CharSequence ACCEPT_HEADER = HttpHeaders.createOptimized("Accept"); - public static final CharSequence SEC_GPC = HttpHeaders.createOptimized("Sec-GPC"); + public static final CharSequence SEC_GPC_HEADER = HttpHeaders.createOptimized("Sec-GPC"); public static final CharSequence CONTENT_TYPE_HEADER = HttpHeaders.createOptimized("Content-Type"); public static final CharSequence X_REQUESTED_WITH_HEADER = HttpHeaders.createOptimized("X-Requested-With"); public static final CharSequence REFERER_HEADER = HttpHeaders.createOptimized("Referer"); @@ -49,6 +49,7 @@ public final class HttpUtil { public static final CharSequence CONNECTION_HEADER = HttpHeaders.createOptimized("Connection"); public static final CharSequence ACCEPT_ENCODING_HEADER = HttpHeaders.createOptimized("Accept-Encoding"); public static final CharSequence X_OPENRTB_VERSION_HEADER = HttpHeaders.createOptimized("x-openrtb-version"); + public static final CharSequence X_PREBID_HEADER = HttpHeaders.createOptimized("x-prebid"); private HttpUtil() { } diff --git a/src/test/java/org/prebid/server/bidder/HttpBidderRequesterTest.java b/src/test/java/org/prebid/server/bidder/HttpBidderRequesterTest.java index 3a8eb9daca8..34425aec66c 100644 --- a/src/test/java/org/prebid/server/bidder/HttpBidderRequesterTest.java +++ b/src/test/java/org/prebid/server/bidder/HttpBidderRequesterTest.java @@ -1,5 +1,6 @@ package org.prebid.server.bidder; +import com.iab.openrtb.request.App; import com.iab.openrtb.request.BidRequest; import io.vertx.core.Future; import io.vertx.core.MultiMap; @@ -29,6 +30,11 @@ import org.prebid.server.bidder.model.Result; import org.prebid.server.execution.Timeout; import org.prebid.server.execution.TimeoutFactory; +import org.prebid.server.proto.openrtb.ext.request.ExtApp; +import org.prebid.server.proto.openrtb.ext.request.ExtAppPrebid; +import org.prebid.server.proto.openrtb.ext.request.ExtRequest; +import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid; +import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidChannel; import org.prebid.server.proto.openrtb.ext.response.ExtHttpCall; import org.prebid.server.vertx.http.HttpClient; import org.prebid.server.vertx.http.model.HttpClientResponse; @@ -91,7 +97,7 @@ public void setUp() { timeout = timeoutFactory.create(500L); expiredTimeout = timeoutFactory.create(clock.instant().minusMillis(1500L).toEpochMilli(), 1000L); - httpBidderRequester = new HttpBidderRequester(httpClient, null, bidderErrorNotifier); + httpBidderRequester = new HttpBidderRequester(httpClient, null, bidderErrorNotifier, "1.00"); } @Test @@ -154,10 +160,45 @@ public void shouldSendPopulatedPostRequest() { httpBidderRequester.requestBids(bidder, bidderRequest, timeout, routingContext, false); // then - verify(httpClient).request(eq(HttpMethod.POST), eq("uri"), argThat(new MultiMapMatcher(headers)), + final MultiMap expectedHeaders = new CaseInsensitiveHeaders(); + expectedHeaders.addAll(headers); + expectedHeaders.add("x-prebid", "pbs-java/1.00"); + verify(httpClient).request(eq(HttpMethod.POST), eq("uri"), argThat(new MultiMapMatcher(expectedHeaders)), eq("requestBody"), eq(500L)); } + @Test + public void shouldCreateXPrebidHeaderForOutgoingRequest() { + // given + givenHttpClientReturnsResponse(200, null); + given(bidder.makeHttpRequests(any())).willReturn(Result.of(singletonList( + HttpRequest.builder() + .method(HttpMethod.POST) + .uri("uri") + .body("requestBody") + .headers(new CaseInsensitiveHeaders()) + .build()), + emptyList())); + + final BidRequest bidRequest = BidRequest.builder() + .ext(ExtRequest.of(ExtRequestPrebid.builder() + .channel(ExtRequestPrebidChannel.of("pbjs", "4.39")) + .build())) + .app(App.builder() + .ext(ExtApp.of(ExtAppPrebid.of("prebid-mobile", "1.2.3"), null)) + .build()) + .build(); + final BidderRequest bidderRequest = BidderRequest.of("bidder", null, bidRequest); + + // when + httpBidderRequester.requestBids(bidder, bidderRequest, timeout, routingContext, false); + + // then + final MultiMap expectedHeaders = new CaseInsensitiveHeaders(); + expectedHeaders.add("x-prebid", "pbjs/4.39,prebid-mobile/1.2.3,pbs-java/1.00"); + verify(httpClient).request(any(), any(), argThat(new MultiMapMatcher(expectedHeaders)), anyString(), anyLong()); + } + @Test public void shouldPassStoredResponseToBidderMakeBidsMethodAndReturnSeatBids() { // given From 6d17a7c07136d2e1c9b662756f0f411e5297806c Mon Sep 17 00:00:00 2001 From: Serhii Nahornyi Date: Tue, 8 Jun 2021 14:36:15 +0300 Subject: [PATCH 2/5] Update AnalyticsReporterDelegator to tolerate with missing BidRequest --- .../server/analytics/AnalyticsReporterDelegator.java | 10 ++++++---- .../analytics/AnalyticsReporterDelegatorTest.java | 12 ++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/prebid/server/analytics/AnalyticsReporterDelegator.java b/src/main/java/org/prebid/server/analytics/AnalyticsReporterDelegator.java index 987c945a749..0ff38edf60c 100644 --- a/src/main/java/org/prebid/server/analytics/AnalyticsReporterDelegator.java +++ b/src/main/java/org/prebid/server/analytics/AnalyticsReporterDelegator.java @@ -111,8 +111,9 @@ private void checkUnknownAdaptersForAuctionEvent(T event) { } private void logUnknownAdapters(AuctionEvent auctionEvent) { - final BidRequest bidRequest = auctionEvent.getAuctionContext().getBidRequest(); - final ExtRequest extRequest = bidRequest.getExt(); + final AuctionContext auctionContext = auctionEvent.getAuctionContext(); + final BidRequest bidRequest = auctionContext != null ? auctionContext.getBidRequest() : null; + final ExtRequest extRequest = bidRequest != null ? bidRequest.getExt() : null; final ExtRequestPrebid extPrebid = extRequest != null ? extRequest.getPrebid() : null; final JsonNode analytics = extPrebid != null ? extPrebid.getAnalytics() : null; final Iterator analyticsFieldNames = isNotEmptyObjectNode(analytics) ? analytics.fieldNames() : null; @@ -149,13 +150,14 @@ private static T updateEvent(T event, String adapter) { } private static AuctionContext updateAuctionContextAdapter(AuctionContext context, String adapter) { - final BidRequest updatedBidRequest = updateBidRequest(context.getBidRequest(), adapter); + final BidRequest bidRequest = context != null ? context.getBidRequest() : null; + final BidRequest updatedBidRequest = updateBidRequest(bidRequest, adapter); return updatedBidRequest != null ? context.toBuilder().bidRequest(updatedBidRequest).build() : null; } private static BidRequest updateBidRequest(BidRequest bidRequest, String adapterName) { - final ExtRequest requestExt = bidRequest.getExt(); + final ExtRequest requestExt = bidRequest != null ? bidRequest.getExt() : null; final ExtRequestPrebid extPrebid = requestExt != null ? requestExt.getPrebid() : null; final JsonNode analytics = extPrebid != null ? extPrebid.getAnalytics() : null; ObjectNode preparedAnalytics = null; diff --git a/src/test/java/org/prebid/server/analytics/AnalyticsReporterDelegatorTest.java b/src/test/java/org/prebid/server/analytics/AnalyticsReporterDelegatorTest.java index eaf81aca549..9870700a8a0 100644 --- a/src/test/java/org/prebid/server/analytics/AnalyticsReporterDelegatorTest.java +++ b/src/test/java/org/prebid/server/analytics/AnalyticsReporterDelegatorTest.java @@ -127,6 +127,18 @@ public void shouldTolerateInvalidExtPrebidAnalyticsNode() { .containsExactly(analyticsNode, analyticsNode); } + @Test + public void shouldTolerateWithMissingBidRequest() { + // given + final AuctionEvent givenAuctionEvent = AuctionEvent.builder().build(); + + // when + target.processEvent(givenAuctionEvent, TcfContext.empty()); + + // then + verify(vertx, times(2)).runOnContext(any()); + } + @Test public void shouldPassOnlyAdapterRelatedEntriesToAnalyticReporters() { // given From f2d6d28cd9f0a5ccb3f912b4a782413b2cd35cd8 Mon Sep 17 00:00:00 2001 From: Serhii Nahornyi Date: Wed, 9 Jun 2021 01:45:15 +0300 Subject: [PATCH 3/5] Refactor --- .../bidder/HttpBidderRequestEnricher.java | 82 +++++++++++ .../server/bidder/HttpBidderRequester.java | 77 +--------- .../spring/config/ServiceConfiguration.java | 11 +- .../bidder/HttpBidderRequestEnricherTest.java | 103 +++++++++++++ .../bidder/HttpBidderRequesterTest.java | 135 ++---------------- 5 files changed, 211 insertions(+), 197 deletions(-) create mode 100644 src/main/java/org/prebid/server/bidder/HttpBidderRequestEnricher.java create mode 100644 src/test/java/org/prebid/server/bidder/HttpBidderRequestEnricherTest.java diff --git a/src/main/java/org/prebid/server/bidder/HttpBidderRequestEnricher.java b/src/main/java/org/prebid/server/bidder/HttpBidderRequestEnricher.java new file mode 100644 index 00000000000..3eb4dddefb8 --- /dev/null +++ b/src/main/java/org/prebid/server/bidder/HttpBidderRequestEnricher.java @@ -0,0 +1,82 @@ +package org.prebid.server.bidder; + +import com.iab.openrtb.request.App; +import com.iab.openrtb.request.BidRequest; +import io.vertx.core.MultiMap; +import org.apache.commons.lang3.StringUtils; +import org.prebid.server.proto.openrtb.ext.request.ExtApp; +import org.prebid.server.proto.openrtb.ext.request.ExtAppPrebid; +import org.prebid.server.proto.openrtb.ext.request.ExtRequest; +import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid; +import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidChannel; +import org.prebid.server.util.HttpUtil; + +import java.util.Collections; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +public class HttpBidderRequestEnricher { + + private static final Set HEADERS_TO_COPY = Collections.singleton(HttpUtil.SEC_GPC_HEADER.toString()); + + private final String pbsRecord; + + public HttpBidderRequestEnricher(String pbsVersion) { + this.pbsRecord = createNameVersionRecord("pbs-java", Objects.requireNonNull(pbsVersion)); + } + + MultiMap enrichHeaders(MultiMap bidderRequestHeaders, MultiMap originalRequestHeaders, BidRequest bidRequest) { + // some bidders has headers on class level, so we create copy to not affect them + final MultiMap bidderRequestHeadersCopy = copyMultiMap(bidderRequestHeaders); + + originalRequestHeaders.entries().stream() + .filter(entry -> HEADERS_TO_COPY.contains(entry.getKey()) + && !bidderRequestHeadersCopy.contains(entry.getKey())) + .forEach(entry -> bidderRequestHeadersCopy.add(entry.getKey(), entry.getValue())); + addXPrebidHeader(bidderRequestHeadersCopy, bidRequest); + return bidderRequestHeadersCopy; + } + + private static MultiMap copyMultiMap(MultiMap source) { + final MultiMap copiedMultiMap = MultiMap.caseInsensitiveMultiMap(); + if (source != null && !source.isEmpty()) { + source.forEach(entry -> copiedMultiMap.add(entry.getKey(), entry.getValue())); + } + return copiedMultiMap; + } + + private void addXPrebidHeader(MultiMap headers, BidRequest bidRequest) { + final String channelRecord = resolveChannelVersionRecord(bidRequest.getExt()); + final String sdkRecord = resolveSdkVersionRecord(bidRequest.getApp()); + final String value = Stream.of(channelRecord, sdkRecord, pbsRecord) + .filter(Objects::nonNull) + .collect(Collectors.joining(",")); + HttpUtil.addHeaderIfValueIsNotEmpty(headers, HttpUtil.X_PREBID_HEADER, value); + } + + private static String resolveChannelVersionRecord(ExtRequest extRequest) { + final ExtRequestPrebid extPrebid = extRequest != null ? extRequest.getPrebid() : null; + final ExtRequestPrebidChannel channel = extPrebid != null ? extPrebid.getChannel() : null; + final String channelName = channel != null ? channel.getName() : null; + final String channelVersion = channel != null ? channel.getVersion() : null; + + return createNameVersionRecord(channelName, channelVersion); + } + + private static String resolveSdkVersionRecord(App app) { + final ExtApp extApp = app != null ? app.getExt() : null; + final ExtAppPrebid extPrebid = extApp != null ? extApp.getPrebid() : null; + final String sdkSource = extPrebid != null ? extPrebid.getSource() : null; + final String sdkVersion = extPrebid != null ? extPrebid.getVersion() : null; + + return createNameVersionRecord(sdkSource, sdkVersion); + } + + private static String createNameVersionRecord(String name, String version) { + return StringUtils.isNotEmpty(name) && StringUtils.isNotEmpty(version) + ? String.format("%s/%s", name, version) + : null; + } +} diff --git a/src/main/java/org/prebid/server/bidder/HttpBidderRequester.java b/src/main/java/org/prebid/server/bidder/HttpBidderRequester.java index eafdfdce30e..891e6184a44 100644 --- a/src/main/java/org/prebid/server/bidder/HttpBidderRequester.java +++ b/src/main/java/org/prebid/server/bidder/HttpBidderRequester.java @@ -1,12 +1,10 @@ package org.prebid.server.bidder; -import com.iab.openrtb.request.App; import com.iab.openrtb.request.BidRequest; import io.netty.channel.ConnectTimeoutException; import io.netty.handler.codec.http.HttpResponseStatus; import io.vertx.core.CompositeFuture; import io.vertx.core.Future; -import io.vertx.core.MultiMap; import io.vertx.core.logging.Logger; import io.vertx.core.logging.LoggerFactory; import io.vertx.ext.web.RoutingContext; @@ -22,13 +20,7 @@ import org.prebid.server.bidder.model.HttpResponse; import org.prebid.server.bidder.model.Result; import org.prebid.server.execution.Timeout; -import org.prebid.server.proto.openrtb.ext.request.ExtApp; -import org.prebid.server.proto.openrtb.ext.request.ExtAppPrebid; -import org.prebid.server.proto.openrtb.ext.request.ExtRequest; -import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid; -import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidChannel; import org.prebid.server.proto.openrtb.ext.response.ExtHttpCall; -import org.prebid.server.util.HttpUtil; import org.prebid.server.vertx.http.HttpClient; import org.prebid.server.vertx.http.model.HttpClientResponse; @@ -38,7 +30,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Set; import java.util.concurrent.TimeoutException; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -56,22 +47,20 @@ public class HttpBidderRequester { private static final Logger logger = LoggerFactory.getLogger(HttpBidderRequester.class); - private static final Set HEADERS_TO_COPY = Collections.singleton(HttpUtil.SEC_GPC_HEADER.toString()); - private final HttpClient httpClient; private final BidderRequestCompletionTrackerFactory completionTrackerFactory; private final BidderErrorNotifier bidderErrorNotifier; - private final String pbsVersion; + private final HttpBidderRequestEnricher requestEnricher; public HttpBidderRequester(HttpClient httpClient, BidderRequestCompletionTrackerFactory completionTrackerFactory, BidderErrorNotifier bidderErrorNotifier, - String pbsVersion) { + HttpBidderRequestEnricher requestEnricher) { this.httpClient = Objects.requireNonNull(httpClient); this.completionTrackerFactory = completionTrackerFactoryOrFallback(completionTrackerFactory); this.bidderErrorNotifier = Objects.requireNonNull(bidderErrorNotifier); - this.pbsVersion = pbsVersion; + this.requestEnricher = Objects.requireNonNull(requestEnricher); } /** @@ -121,69 +110,13 @@ public Future requestBids(Bidder bidder, private List> enrichRequests(List> httpRequests, RoutingContext routingContext, BidRequest bidRequest) { - return httpRequests.stream().map(httpRequest -> httpRequest.toBuilder() - .headers(enrichHeaders(httpRequest.getHeaders(), routingContext.request().headers(), bidRequest)) + .headers(requestEnricher.enrichHeaders(httpRequest.getHeaders(), + routingContext.request().headers(), bidRequest)) .build()) .collect(Collectors.toList()); } - private MultiMap enrichHeaders(MultiMap bidderRequestHeaders, - MultiMap originalRequestHeaders, - BidRequest bidRequest) { - // some bidders has headers on class level, so we create copy to not affect them - final MultiMap bidderRequestHeadersCopy = copyMultiMap(bidderRequestHeaders); - - originalRequestHeaders.entries().stream() - .filter(entry -> HEADERS_TO_COPY.contains(entry.getKey()) - && !bidderRequestHeadersCopy.contains(entry.getKey())) - .forEach(entry -> bidderRequestHeadersCopy.add(entry.getKey(), entry.getValue())); - addXPrebidHeader(bidderRequestHeadersCopy, bidRequest); - return bidderRequestHeadersCopy; - } - - private static MultiMap copyMultiMap(MultiMap source) { - final MultiMap copiedMultiMap = MultiMap.caseInsensitiveMultiMap(); - if (source != null && !source.isEmpty()) { - source.forEach(entry -> copiedMultiMap.add(entry.getKey(), entry.getValue())); - } - return copiedMultiMap; - } - - private void addXPrebidHeader(MultiMap headers, BidRequest bidRequest) { - final String channelRecord = resolveChannelVersionRecord(bidRequest.getExt()); - final String sdkRecord = resolveSdkVersionRecord(bidRequest.getApp()); - final String pbsRecord = createNameVersionRecord("pbs-java", pbsVersion); - final String value = Stream.of(channelRecord, sdkRecord, pbsRecord) - .filter(Objects::nonNull) - .collect(Collectors.joining(",")); - HttpUtil.addHeaderIfValueIsNotEmpty(headers, HttpUtil.X_PREBID_HEADER, value); - } - - private static String resolveChannelVersionRecord(ExtRequest extRequest) { - final ExtRequestPrebid extPrebid = extRequest != null ? extRequest.getPrebid() : null; - final ExtRequestPrebidChannel channel = extPrebid != null ? extPrebid.getChannel() : null; - final String channelName = channel != null ? channel.getName() : null; - final String channelVersion = channel != null ? channel.getVersion() : null; - - return createNameVersionRecord(channelName, channelVersion); - } - - private static String resolveSdkVersionRecord(App app) { - final ExtApp extApp = app != null ? app.getExt() : null; - final ExtAppPrebid extPrebid = extApp != null ? extApp.getPrebid() : null; - final String sdkSource = extPrebid != null ? extPrebid.getSource() : null; - final String sdkVersion = extPrebid != null ? extPrebid.getVersion() : null; - - return createNameVersionRecord(sdkSource, sdkVersion); - } - - private static String createNameVersionRecord(String name, String version) { - return StringUtils.isNotEmpty(name) && StringUtils.isNotEmpty(version) - ? String.format("%s/%s", name, version) - : null; - } - private boolean isStoredResponse(List> httpRequests, String storedResponse, String bidder) { if (StringUtils.isBlank(storedResponse)) { return false; 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 d5dc1afa25e..a8f95969be0 100644 --- a/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java +++ b/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java @@ -32,6 +32,7 @@ import org.prebid.server.bidder.BidderDeps; import org.prebid.server.bidder.BidderErrorNotifier; import org.prebid.server.bidder.BidderRequestCompletionTrackerFactory; +import org.prebid.server.bidder.HttpBidderRequestEnricher; import org.prebid.server.bidder.HttpBidderRequester; import org.prebid.server.cache.CacheService; import org.prebid.server.cache.model.CacheTtl; @@ -463,12 +464,18 @@ HttpBidderRequester httpBidderRequester( HttpClient httpClient, @Autowired(required = false) BidderRequestCompletionTrackerFactory bidderRequestCompletionTrackerFactory, BidderErrorNotifier bidderErrorNotifier, - VersionInfo versionInfo) { + HttpBidderRequestEnricher requestEnricher) { return new HttpBidderRequester(httpClient, bidderRequestCompletionTrackerFactory, bidderErrorNotifier, - versionInfo.getVersion()); + requestEnricher); + } + + @Bean + HttpBidderRequestEnricher httpBidderRequestEnricher(VersionInfo versionInfo) { + + return new HttpBidderRequestEnricher(versionInfo.getVersion()); } @Bean diff --git a/src/test/java/org/prebid/server/bidder/HttpBidderRequestEnricherTest.java b/src/test/java/org/prebid/server/bidder/HttpBidderRequestEnricherTest.java new file mode 100644 index 00000000000..a9cb92bbd58 --- /dev/null +++ b/src/test/java/org/prebid/server/bidder/HttpBidderRequestEnricherTest.java @@ -0,0 +1,103 @@ +package org.prebid.server.bidder; + +import com.iab.openrtb.request.App; +import com.iab.openrtb.request.BidRequest; +import io.vertx.core.MultiMap; +import io.vertx.core.http.CaseInsensitiveHeaders; +import org.junit.Before; +import org.junit.Test; +import org.prebid.server.proto.openrtb.ext.request.ExtApp; +import org.prebid.server.proto.openrtb.ext.request.ExtAppPrebid; +import org.prebid.server.proto.openrtb.ext.request.ExtRequest; +import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid; +import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidChannel; + +import static org.assertj.core.api.Assertions.assertThat; + +public class HttpBidderRequestEnricherTest { + + private HttpBidderRequestEnricher requestEnricher; + + @Before + public void setUp() { + + requestEnricher = new HttpBidderRequestEnricher("1.00"); + } + + @Test + public void shouldSendPopulatedPostRequest() { + // given + final MultiMap headers = new CaseInsensitiveHeaders(); + headers.add("header1", "value1"); + headers.add("header2", "value2"); + + // when + final MultiMap resultHeaders = + requestEnricher.enrichHeaders(headers, new CaseInsensitiveHeaders(), BidRequest.builder().build()); + + // then + final MultiMap expectedHeaders = new CaseInsensitiveHeaders(); + expectedHeaders.addAll(headers); + expectedHeaders.add("x-prebid", "pbs-java/1.00"); + assertThat(resultHeaders).hasSize(3); + assertThat(isEqualsMultiMaps(resultHeaders, expectedHeaders)).isTrue(); + } + + @Test + public void shouldAddSecGpcHeaderFromOriginalRequest() { + // given + final MultiMap originalHeaders = new CaseInsensitiveHeaders().add("Sec-GPC", "1"); + + // when + final MultiMap resultHeaders = + requestEnricher.enrichHeaders(new CaseInsensitiveHeaders(), + originalHeaders, BidRequest.builder().build()); + + // then + assertThat(resultHeaders.contains("Sec-GPC")).isTrue(); + assertThat(resultHeaders.get("Sec-GPC")).isEqualTo("1"); + } + + @Test + public void shouldNotOverrideHeadersFromBidRequest() { + // given + final MultiMap originalHeaders = new CaseInsensitiveHeaders().add("Sec-GPC", "1"); + final MultiMap bidderRequestHeaders = new CaseInsensitiveHeaders().add("Sec-GPC", "0"); + + // when + final MultiMap resultHeaders = requestEnricher.enrichHeaders(bidderRequestHeaders, + originalHeaders, BidRequest.builder().build()); + + // then + assertThat(resultHeaders.contains("Sec-GPC")).isTrue(); + assertThat(resultHeaders.getAll("Sec-GPC")).hasSize(1); + assertThat(resultHeaders.get("Sec-GPC")).isEqualTo("0"); + } + + @Test + public void shouldCreateXPrebidHeaderForOutgoingRequest() { + // given + final BidRequest bidRequest = BidRequest.builder() + .ext(ExtRequest.of(ExtRequestPrebid.builder() + .channel(ExtRequestPrebidChannel.of("pbjs", "4.39")) + .build())) + .app(App.builder() + .ext(ExtApp.of(ExtAppPrebid.of("prebid-mobile", "1.2.3"), null)) + .build()) + .build(); + + // when + final MultiMap resultHeaders = requestEnricher.enrichHeaders(new CaseInsensitiveHeaders(), + new CaseInsensitiveHeaders(), bidRequest); + + // then + final MultiMap expectedHeaders = new CaseInsensitiveHeaders(); + expectedHeaders.add("x-prebid", "pbjs/4.39,prebid-mobile/1.2.3,pbs-java/1.00"); + assertThat(isEqualsMultiMaps(resultHeaders, expectedHeaders)).isTrue(); + } + + private static boolean isEqualsMultiMaps(MultiMap left, MultiMap right) { + return left.size() == right.size() && left.entries().stream() + .allMatch(entry -> right.contains(entry.getKey(), entry.getValue(), true)); + } +} diff --git a/src/test/java/org/prebid/server/bidder/HttpBidderRequesterTest.java b/src/test/java/org/prebid/server/bidder/HttpBidderRequesterTest.java index 34425aec66c..403d9ee307b 100644 --- a/src/test/java/org/prebid/server/bidder/HttpBidderRequesterTest.java +++ b/src/test/java/org/prebid/server/bidder/HttpBidderRequesterTest.java @@ -1,6 +1,5 @@ package org.prebid.server.bidder; -import com.iab.openrtb.request.App; import com.iab.openrtb.request.BidRequest; import io.vertx.core.Future; import io.vertx.core.MultiMap; @@ -30,11 +29,6 @@ import org.prebid.server.bidder.model.Result; import org.prebid.server.execution.Timeout; import org.prebid.server.execution.TimeoutFactory; -import org.prebid.server.proto.openrtb.ext.request.ExtApp; -import org.prebid.server.proto.openrtb.ext.request.ExtAppPrebid; -import org.prebid.server.proto.openrtb.ext.request.ExtRequest; -import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid; -import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidChannel; import org.prebid.server.proto.openrtb.ext.response.ExtHttpCall; import org.prebid.server.vertx.http.HttpClient; import org.prebid.server.vertx.http.model.HttpClientResponse; @@ -51,8 +45,6 @@ import static org.apache.commons.lang3.StringUtils.EMPTY; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.same; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.anyLong; @@ -75,6 +67,8 @@ public class HttpBidderRequesterTest extends VertxTest { @Mock private BidderErrorNotifier bidderErrorNotifier; @Mock + private HttpBidderRequestEnricher requestEnricher; + @Mock private RoutingContext routingContext; @Mock private HttpServerRequest httpServerRequest; @@ -91,13 +85,14 @@ public void setUp() { given(bidderErrorNotifier.processTimeout(any(), any())).will(invocation -> invocation.getArgument(0)); given(routingContext.request()).willReturn(httpServerRequest); given(httpServerRequest.headers()).willReturn(new CaseInsensitiveHeaders()); + given(requestEnricher.enrichHeaders(any(), any(), any())).willReturn(new CaseInsensitiveHeaders()); final Clock clock = Clock.fixed(Instant.now(), ZoneId.systemDefault()); final TimeoutFactory timeoutFactory = new TimeoutFactory(clock); timeout = timeoutFactory.create(500L); expiredTimeout = timeoutFactory.create(clock.instant().minusMillis(1500L).toEpochMilli(), 1000L); - httpBidderRequester = new HttpBidderRequester(httpClient, null, bidderErrorNotifier, "1.00"); + httpBidderRequester = new HttpBidderRequester(httpClient, null, bidderErrorNotifier, requestEnricher); } @Test @@ -136,69 +131,6 @@ public void shouldTolerateBidderReturningErrorsAndNoHttpRequests() { .extracting(BidderError::getMessage).containsOnly("error1", "error2"); } - @Test - public void shouldSendPopulatedPostRequest() { - // given - givenHttpClientReturnsResponse(200, null); - - final MultiMap headers = new CaseInsensitiveHeaders(); - headers.add("header1", "value1"); - headers.add("header2", "value2"); - - given(bidder.makeHttpRequests(any())).willReturn(Result.of(singletonList( - HttpRequest.builder() - .method(HttpMethod.POST) - .uri("uri") - .body("requestBody") - .headers(headers) - .build()), - emptyList())); - - final BidderRequest bidderRequest = BidderRequest.of("bidder", null, BidRequest.builder().build()); - - // when - httpBidderRequester.requestBids(bidder, bidderRequest, timeout, routingContext, false); - - // then - final MultiMap expectedHeaders = new CaseInsensitiveHeaders(); - expectedHeaders.addAll(headers); - expectedHeaders.add("x-prebid", "pbs-java/1.00"); - verify(httpClient).request(eq(HttpMethod.POST), eq("uri"), argThat(new MultiMapMatcher(expectedHeaders)), - eq("requestBody"), eq(500L)); - } - - @Test - public void shouldCreateXPrebidHeaderForOutgoingRequest() { - // given - givenHttpClientReturnsResponse(200, null); - given(bidder.makeHttpRequests(any())).willReturn(Result.of(singletonList( - HttpRequest.builder() - .method(HttpMethod.POST) - .uri("uri") - .body("requestBody") - .headers(new CaseInsensitiveHeaders()) - .build()), - emptyList())); - - final BidRequest bidRequest = BidRequest.builder() - .ext(ExtRequest.of(ExtRequestPrebid.builder() - .channel(ExtRequestPrebidChannel.of("pbjs", "4.39")) - .build())) - .app(App.builder() - .ext(ExtApp.of(ExtAppPrebid.of("prebid-mobile", "1.2.3"), null)) - .build()) - .build(); - final BidderRequest bidderRequest = BidderRequest.of("bidder", null, bidRequest); - - // when - httpBidderRequester.requestBids(bidder, bidderRequest, timeout, routingContext, false); - - // then - final MultiMap expectedHeaders = new CaseInsensitiveHeaders(); - expectedHeaders.add("x-prebid", "pbjs/4.39,prebid-mobile/1.2.3,pbs-java/1.00"); - verify(httpClient).request(any(), any(), argThat(new MultiMapMatcher(expectedHeaders)), anyString(), anyLong()); - } - @Test public void shouldPassStoredResponseToBidderMakeBidsMethodAndReturnSeatBids() { // given @@ -427,57 +359,6 @@ public void shouldReturnPartialDebugInfoIfDebugEnabledAndHttpErrorOccurs() { ExtHttpCall.builder().uri("uri1").requestbody("requestBody1").build()); } - @Test - public void shouldAddSecGpcHeaderFromOriginalRequest() { - // given - givenHttpClientReturnsResponse(200, null); - given(httpServerRequest.headers()).willReturn(new CaseInsensitiveHeaders().add("Sec-GPC", "1")); - - given(bidder.makeHttpRequests(any())).willReturn(Result.of(singletonList( - HttpRequest.builder() - .method(HttpMethod.POST) - .uri("uri") - .body("requestBody") - .headers(new CaseInsensitiveHeaders()) - .build()), - emptyList())); - - final BidderRequest bidderRequest = BidderRequest.of("bidder", null, BidRequest.builder().build()); - // when - httpBidderRequester.requestBids(bidder, bidderRequest, timeout, routingContext, false); - - // then - verify(httpClient).request(eq(HttpMethod.POST), eq("uri"), headerCaptor.capture(), eq("requestBody"), eq(500L)); - assertThat(headerCaptor.getValue().contains("Sec-GPC")).isTrue(); - assertThat(headerCaptor.getValue().get("Sec-GPC")).isEqualTo("1"); - } - - @Test - public void shouldNotOverrideHeadersFromBidRequest() { - // given - givenHttpClientReturnsResponse(200, null); - given(httpServerRequest.headers()).willReturn(new CaseInsensitiveHeaders().add("Sec-GPC", "1")); - - given(bidder.makeHttpRequests(any())).willReturn(Result.of(singletonList( - HttpRequest.builder() - .method(HttpMethod.POST) - .uri("uri") - .body("requestBody") - .headers(new CaseInsensitiveHeaders().add("Sec-GPC", "0")) - .build()), - emptyList())); - - final BidderRequest bidderRequest = BidderRequest.of("bidder", null, BidRequest.builder().build()); - // when - httpBidderRequester.requestBids(bidder, bidderRequest, timeout, routingContext, false); - - // then - verify(httpClient).request(eq(HttpMethod.POST), eq("uri"), headerCaptor.capture(), eq("requestBody"), eq(500L)); - assertThat(headerCaptor.getValue().contains("Sec-GPC")).isTrue(); - assertThat(headerCaptor.getValue().getAll("Sec-GPC")).hasSize(1); - assertThat(headerCaptor.getValue().get("Sec-GPC")).isEqualTo("0"); - } - @Test public void shouldReturnFullDebugInfoIfDebugEnabledAndErrorStatus() { // given @@ -618,6 +499,14 @@ public void shouldTolerateMultipleErrors() { // simulate 200 status .willReturn(Future.succeededFuture(HttpClientResponse.of(200, null, EMPTY))); + given(requestEnricher.enrichHeaders(any(), any(), any())) + .willReturn(new CaseInsensitiveHeaders()) + .willReturn(new CaseInsensitiveHeaders()) + .willReturn(new CaseInsensitiveHeaders()) + .willReturn(new CaseInsensitiveHeaders()) + .willReturn(new CaseInsensitiveHeaders()) + .willReturn(new CaseInsensitiveHeaders()); + given(bidder.makeBids(any(), any())).willReturn( Result.of(singletonList(BidderBid.of(null, null, null)), singletonList(BidderError.badServerResponse("makeBidsError")))); From cbaded253686c1a1f337d125dedff133b9e7a59a Mon Sep 17 00:00:00 2001 From: Serhii Nahornyi Date: Wed, 9 Jun 2021 12:59:42 +0300 Subject: [PATCH 4/5] Test refactor --- .../prebid/server/bidder/HttpBidderRequesterTest.java | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/test/java/org/prebid/server/bidder/HttpBidderRequesterTest.java b/src/test/java/org/prebid/server/bidder/HttpBidderRequesterTest.java index 403d9ee307b..0dfd7b8edaa 100644 --- a/src/test/java/org/prebid/server/bidder/HttpBidderRequesterTest.java +++ b/src/test/java/org/prebid/server/bidder/HttpBidderRequesterTest.java @@ -54,6 +54,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; public class HttpBidderRequesterTest extends VertxTest { @@ -484,7 +485,7 @@ public void shouldTolerateMultipleErrors() { .headers(new CaseInsensitiveHeaders()) .build()), singletonList(BidderError.badInput("makeHttpRequestsError")))); - + when(requestEnricher.enrichHeaders(any(), any(), any())).thenAnswer(invocation -> new CaseInsensitiveHeaders()); given(httpClient.request(any(), anyString(), any(), anyString(), anyLong())) // simulate response error for the first request .willReturn(Future.failedFuture(new RuntimeException("Response exception"))) @@ -499,14 +500,6 @@ public void shouldTolerateMultipleErrors() { // simulate 200 status .willReturn(Future.succeededFuture(HttpClientResponse.of(200, null, EMPTY))); - given(requestEnricher.enrichHeaders(any(), any(), any())) - .willReturn(new CaseInsensitiveHeaders()) - .willReturn(new CaseInsensitiveHeaders()) - .willReturn(new CaseInsensitiveHeaders()) - .willReturn(new CaseInsensitiveHeaders()) - .willReturn(new CaseInsensitiveHeaders()) - .willReturn(new CaseInsensitiveHeaders()); - given(bidder.makeBids(any(), any())).willReturn( Result.of(singletonList(BidderBid.of(null, null, null)), singletonList(BidderError.badServerResponse("makeBidsError")))); From 313dbd6522bac74a8b27e86b3544bc057340be0d Mon Sep 17 00:00:00 2001 From: Serhii Nahornyi Date: Wed, 9 Jun 2021 18:11:23 +0300 Subject: [PATCH 5/5] Fixes after review --- .../server/bidder/HttpBidderRequestEnricher.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/prebid/server/bidder/HttpBidderRequestEnricher.java b/src/main/java/org/prebid/server/bidder/HttpBidderRequestEnricher.java index 3eb4dddefb8..1e36609e93a 100644 --- a/src/main/java/org/prebid/server/bidder/HttpBidderRequestEnricher.java +++ b/src/main/java/org/prebid/server/bidder/HttpBidderRequestEnricher.java @@ -24,18 +24,16 @@ public class HttpBidderRequestEnricher { private final String pbsRecord; public HttpBidderRequestEnricher(String pbsVersion) { - this.pbsRecord = createNameVersionRecord("pbs-java", Objects.requireNonNull(pbsVersion)); + pbsRecord = createNameVersionRecord("pbs-java", Objects.requireNonNull(pbsVersion)); } MultiMap enrichHeaders(MultiMap bidderRequestHeaders, MultiMap originalRequestHeaders, BidRequest bidRequest) { // some bidders has headers on class level, so we create copy to not affect them final MultiMap bidderRequestHeadersCopy = copyMultiMap(bidderRequestHeaders); - originalRequestHeaders.entries().stream() - .filter(entry -> HEADERS_TO_COPY.contains(entry.getKey()) - && !bidderRequestHeadersCopy.contains(entry.getKey())) - .forEach(entry -> bidderRequestHeadersCopy.add(entry.getKey(), entry.getValue())); + addOriginalRequestHeaders(originalRequestHeaders, bidderRequestHeadersCopy); addXPrebidHeader(bidderRequestHeadersCopy, bidRequest); + return bidderRequestHeadersCopy; } @@ -47,6 +45,13 @@ private static MultiMap copyMultiMap(MultiMap source) { return copiedMultiMap; } + private static void addOriginalRequestHeaders(MultiMap originalHeaders, MultiMap bidderHeaders) { + originalHeaders.entries().stream() + .filter(entry -> HEADERS_TO_COPY.contains(entry.getKey()) + && !bidderHeaders.contains(entry.getKey())) + .forEach(entry -> bidderHeaders.add(entry.getKey(), entry.getValue())); + } + private void addXPrebidHeader(MultiMap headers, BidRequest bidRequest) { final String channelRecord = resolveChannelVersionRecord(bidRequest.getExt()); final String sdkRecord = resolveSdkVersionRecord(bidRequest.getApp());