From 643ca3d9a24d7aaebc664da9d99c5ab3fa1ad34f Mon Sep 17 00:00:00 2001 From: Andrew Paliy <59088410+apaliy@users.noreply.github.com> Date: Wed, 20 May 2020 13:13:22 +0300 Subject: [PATCH] Overwrite Bid Id (#616) --- .../server/auction/BidResponseCreator.java | 15 +++++-- .../openrtb/ext/response/ExtBidPrebid.java | 2 + .../spring/config/ServiceConfiguration.java | 4 +- src/main/resources/application.yaml | 1 + .../auction/BidResponseCreatorTest.java | 45 +++++++++++++++++-- .../server/auction/ExchangeServiceTest.java | 2 +- .../auction/StoredResponseProcessorTest.java | 2 +- .../auction/VideoResponseFactoryTest.java | 7 +-- .../handler/openrtb2/AmpHandlerTest.java | 29 ++++++------ 9 files changed, 80 insertions(+), 27 deletions(-) diff --git a/src/main/java/org/prebid/server/auction/BidResponseCreator.java b/src/main/java/org/prebid/server/auction/BidResponseCreator.java index 62da284d2c4..54578cc22b8 100644 --- a/src/main/java/org/prebid/server/auction/BidResponseCreator.java +++ b/src/main/java/org/prebid/server/auction/BidResponseCreator.java @@ -20,6 +20,7 @@ import org.apache.commons.collections4.ListUtils; import org.apache.commons.collections4.MapUtils; import org.apache.commons.lang3.BooleanUtils; +import org.apache.commons.lang3.ObjectUtils; import org.prebid.server.auction.model.BidRequestCacheInfo; import org.prebid.server.auction.model.BidderResponse; import org.prebid.server.bidder.BidderCatalog; @@ -72,6 +73,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.UUID; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -86,6 +88,7 @@ public class BidResponseCreator { private final BidderCatalog bidderCatalog; private final EventsService eventsService; private final StoredRequestProcessor storedRequestProcessor; + private final boolean generateBidId; private final JacksonMapper mapper; private final String cacheHost; @@ -93,11 +96,13 @@ public class BidResponseCreator { private final String cacheAssetUrlTemplate; public BidResponseCreator(CacheService cacheService, BidderCatalog bidderCatalog, EventsService eventsService, - StoredRequestProcessor storedRequestProcessor, JacksonMapper mapper) { + StoredRequestProcessor storedRequestProcessor, + boolean generateBidId, JacksonMapper mapper) { this.cacheService = Objects.requireNonNull(cacheService); this.bidderCatalog = Objects.requireNonNull(bidderCatalog); this.eventsService = Objects.requireNonNull(eventsService); this.storedRequestProcessor = Objects.requireNonNull(storedRequestProcessor); + this.generateBidId = generateBidId; this.mapper = Objects.requireNonNull(mapper); cacheHost = Objects.requireNonNull(cacheService.getEndpointHost()); @@ -658,12 +663,16 @@ private Bid toBid(BidderBid bidderBid, String bidder, ExtRequestTargeting target cache = null; } + final String generatedBidId = generateBidId ? UUID.randomUUID().toString() : null; + final String eventBidId = ObjectUtils.defaultIfNull(generatedBidId, bid.getId()); final Video storedVideo = impIdToStoredVideo.get(bid.getImpid()); final Events events = eventsEnabled && eventsAllowedByRequest - ? eventsService.createEvent(bid.getId(), bidder, account.getId(), auctionTimestamp) + ? eventsService.createEvent(eventBidId, bidder, account.getId(), auctionTimestamp) : null; - final ExtBidPrebid prebidExt = ExtBidPrebid.of(bidType, targetingKeywords, cache, storedVideo, events, null); + final ExtBidPrebid prebidExt = ExtBidPrebid.of( + generatedBidId, bidType, targetingKeywords, cache, storedVideo, events, null); + final ExtPrebid bidExt = ExtPrebid.of(prebidExt, bid.getExt()); bid.setExt(mapper.mapper().valueToTree(bidExt)); diff --git a/src/main/java/org/prebid/server/proto/openrtb/ext/response/ExtBidPrebid.java b/src/main/java/org/prebid/server/proto/openrtb/ext/response/ExtBidPrebid.java index b9976575a06..d4fb6fd0b00 100644 --- a/src/main/java/org/prebid/server/proto/openrtb/ext/response/ExtBidPrebid.java +++ b/src/main/java/org/prebid/server/proto/openrtb/ext/response/ExtBidPrebid.java @@ -14,6 +14,8 @@ @Value public class ExtBidPrebid { + String bidid; + BidType type; Map targeting; 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 943ae205b91..a11542d5ea0 100644 --- a/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java +++ b/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java @@ -469,9 +469,11 @@ BidResponseCreator bidResponseCreator( BidderCatalog bidderCatalog, EventsService eventsService, StoredRequestProcessor storedRequestProcessor, + @Value("${auction.generate-bid-id}") boolean generateBidId, JacksonMapper mapper) { - return new BidResponseCreator(cacheService, bidderCatalog, eventsService, storedRequestProcessor, mapper); + return new BidResponseCreator(cacheService, bidderCatalog, eventsService, storedRequestProcessor, + generateBidId, mapper); } @Bean diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml index 05008eb16e5..46d42714fac 100644 --- a/src/main/resources/application.yaml +++ b/src/main/resources/application.yaml @@ -37,6 +37,7 @@ auction: timeout-adjustment-ms: 30 stored-requests-timeout-ms: 50 max-request-size: 262144 + generate-bid-id: false cache: expected-request-time-ms: 10 only-winning-bids: false diff --git a/src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java b/src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java index e1fffecd09d..3aa95219beb 100644 --- a/src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java +++ b/src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java @@ -71,6 +71,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.UUID; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; @@ -125,7 +126,7 @@ public void setUp() { .willReturn(Future.succeededFuture(VideoStoredDataResult.empty())); bidResponseCreator = new BidResponseCreator(cacheService, bidderCatalog, eventsService, storedRequestProcessor, - jacksonMapper); + false, jacksonMapper); timeout = new TimeoutFactory(Clock.fixed(Instant.now(), ZoneId.systemDefault())).create(500); } @@ -221,7 +222,7 @@ public void shouldRequestCacheServiceWithWinningBidsOnlyWhenWinningonlyIsTrue() // just a stub to get through method call chain givenCacheServiceResult(singletonMap(bid1, CacheIdInfo.of(null, null))); - // when\ + // when bidResponseCreator.create(bidderResponses, bidRequest, targeting, cacheInfo, ACCOUNT, false, 1000L, false, timeout); @@ -414,6 +415,41 @@ public void shouldSkipBidderResponsesWhereSeatBidContainEmptyBids() { verify(cacheService, never()).cacheBidsOpenrtb(anyList(), anyList(), any(), any(), any(), any()); } + @Test + public void shouldOverrideBidIdWhenGenerateBidIdIsTurnedOn() { + // given + final BidRequest bidRequest = givenBidRequest(); + + final ExtPrebid prebid = ExtPrebid.of(ExtBidPrebid.of(null, banner, null, + null, null, null, null), null); + final Bid bid = Bid.builder() + .id("123") + .impid("imp123") + .ext(mapper.valueToTree(prebid)) + .build(); + final List bidderResponses = singletonList( + BidderResponse.of("bidder2", givenSeatBid(BidderBid.of(bid, banner, "USD")), 0)); + + final BidResponseCreator bidResponseCreator = new BidResponseCreator(cacheService, bidderCatalog, eventsService, + storedRequestProcessor, true, jacksonMapper); + // when + final BidResponse bidResponse = bidResponseCreator.create(bidderResponses, bidRequest, + null, CACHE_INFO, ACCOUNT, false, 1000L, false, timeout).result(); + + // then + assertThat(bidResponse.getSeatbid()) + .flatExtracting(SeatBid::getBid) + .extracting(Bid::getExt) + .extracting(ext -> ext.get("prebid")) + .extracting(extPrebid -> mapper.treeToValue(extPrebid, ExtBidPrebid.class)) + .extracting(ExtBidPrebid::getBidid) + .hasSize(1) + .first() + .satisfies(bidId -> assertThat(UUID.fromString(bidId)).isInstanceOf(UUID.class)); + + verify(cacheService, never()).cacheBidsOpenrtb(anyList(), anyList(), any(), any(), any(), any()); + } + @Test public void shouldSetExpectedResponseSeatBidAndBidFields() { // given @@ -437,7 +473,8 @@ public void shouldSetExpectedResponseSeatBidAndBidFields() { .price(BigDecimal.ONE) .adm("adm") .ext(mapper.valueToTree(ExtPrebid.of( - ExtBidPrebid.of(banner, null, null, null, null, null), singletonMap("bidExt", 1)))) + ExtBidPrebid.of(null, banner, null, null, null, null, null), + singletonMap("bidExt", 1)))) .build())) .build()); @@ -572,7 +609,7 @@ public void shouldTolerateMissingExtInSeatBidAndBid() { .id("bidId") .price(BigDecimal.ONE) .ext(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(banner, null, null, null, null, null), null))) + ExtPrebid.of(ExtBidPrebid.of(null, banner, null, null, null, null, null), null))) .build()); verify(cacheService, never()).cacheBidsOpenrtb(anyList(), anyList(), any(), any(), any(), any()); diff --git a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java index b6c9b33abce..86a9375421d 100644 --- a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java +++ b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java @@ -1883,7 +1883,7 @@ private static Bid givenBid(Function bidBuilder) return bidBuilder.apply(Bid.builder() .id("bidId") .price(BigDecimal.ONE) - .ext(mapper.valueToTree(ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null)))) + .ext(mapper.valueToTree(ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))) .build(); } diff --git a/src/test/java/org/prebid/server/auction/StoredResponseProcessorTest.java b/src/test/java/org/prebid/server/auction/StoredResponseProcessorTest.java index 0d4918aee38..305e803726c 100644 --- a/src/test/java/org/prebid/server/auction/StoredResponseProcessorTest.java +++ b/src/test/java/org/prebid/server/auction/StoredResponseProcessorTest.java @@ -551,7 +551,7 @@ public void mergeWithBidderResponsesShouldResolveBidTypeFromStoredBidExt() { singletonList(BidderBid.of(Bid.builder().id("bid1").build(), BidType.banner, "USD")), emptyList(), emptyList()), 100)); - final ExtBidPrebid extBidPrebid = ExtBidPrebid.of(BidType.video, null, null, null, null, null); + final ExtBidPrebid extBidPrebid = ExtBidPrebid.of(null, BidType.video, null, null, null, null, null); final List seatBid = singletonList(SeatBid.builder() .seat("rubicon").bid(singletonList(Bid.builder().ext(mapper.createObjectNode() diff --git a/src/test/java/org/prebid/server/auction/VideoResponseFactoryTest.java b/src/test/java/org/prebid/server/auction/VideoResponseFactoryTest.java index d4949155a4f..1e2a6b945e0 100644 --- a/src/test/java/org/prebid/server/auction/VideoResponseFactoryTest.java +++ b/src/test/java/org/prebid/server/auction/VideoResponseFactoryTest.java @@ -42,19 +42,20 @@ public void shouldReturnExpectedVideoResponse() { final Bid bid0 = Bid.builder() .impid("0_0") .ext(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, targeting, null, null, null, null), + ExtPrebid.of(ExtBidPrebid.of(null, null, targeting, null, null, null, null), mapper.createObjectNode()))) .build(); final Bid bid1 = Bid.builder() .impid("1_1") .ext(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, targeting, null, null, null, null), + ExtPrebid.of(ExtBidPrebid.of(null, null, targeting, null, null, null, null), mapper.createObjectNode()))) .build(); final Bid bid2 = Bid.builder() .impid("2_1") .ext(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), mapper.createObjectNode()))) + ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), + mapper.createObjectNode()))) .build(); final BidResponse bidResponse = BidResponse.builder() diff --git a/src/test/java/org/prebid/server/handler/openrtb2/AmpHandlerTest.java b/src/test/java/org/prebid/server/handler/openrtb2/AmpHandlerTest.java index e624c4b014b..54de27ddbbe 100644 --- a/src/test/java/org/prebid/server/handler/openrtb2/AmpHandlerTest.java +++ b/src/test/java/org/prebid/server/handler/openrtb2/AmpHandlerTest.java @@ -347,7 +347,7 @@ public void shouldRespondWithExpectedResponse() { targeting.put("hb_cache_id_bidder1", "value2"); given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree(ExtPrebid.of(ExtBidPrebid.of( - null, targeting, null, null, null, null), null)))); + null, null, targeting, null, null, null, null), null)))); // when ampHandler.handle(routingContext); @@ -377,7 +377,8 @@ public void shouldRespondWithCustomTargetingIncluded() { .seat("bidder1") .bid(singletonList(Bid.builder() .ext(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, targeting, null, null, null, null), + ExtPrebid.of( + ExtBidPrebid.of(null, null, targeting, null, null, null, null), mapper.createObjectNode()))) .build())) .build())) @@ -457,7 +458,7 @@ public void shouldIncrementOkAmpRequestMetrics() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null)))); + ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))); // when ampHandler.handle(routingContext); @@ -474,7 +475,7 @@ public void shouldIncrementAppRequestMetrics() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null)))); + ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))); // when ampHandler.handle(routingContext); @@ -491,7 +492,7 @@ public void shouldIncrementNoCookieMetrics() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null)))); + ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))); given(uidsCookie.hasLiveUids()).willReturn(false); @@ -514,7 +515,7 @@ public void shouldIncrementImpsRequestedMetrics() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null)))); + ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))); // when ampHandler.handle(routingContext); @@ -533,7 +534,7 @@ public void shouldIncrementImpsTypesMetrics() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null)))); + ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))); // when ampHandler.handle(routingContext); @@ -617,7 +618,7 @@ public void shouldUpdateNetworkErrorMetric() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null)))); + ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))); // simulate calling exception handler that is supposed to update networkerr timer value given(httpResponse.exceptionHandler(any())).willAnswer(inv -> { @@ -640,7 +641,7 @@ public void shouldNotUpdateNetworkErrorMetricIfResponseSucceeded() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null)))); + ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))); // when ampHandler.handle(routingContext); @@ -657,7 +658,7 @@ public void shouldUpdateNetworkErrorMetricIfClientClosedConnection() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null), null)))); + ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))); given(routingContext.response().closed()).willReturn(true); @@ -724,8 +725,8 @@ public void shouldPassSuccessfulEventToAnalyticsReporter() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, singletonMap("hb_cache_id_bidder1", "value1"), null, null, - null, null), + ExtPrebid.of(ExtBidPrebid.of(null, null, singletonMap("hb_cache_id_bidder1", "value1"), + null, null, null, null), null)))); // when @@ -743,8 +744,8 @@ public void shouldPassSuccessfulEventToAnalyticsReporter() { .bidResponse(BidResponse.builder().seatbid(singletonList(SeatBid.builder() .bid(singletonList(Bid.builder() .ext(mapper.valueToTree(ExtPrebid.of( - ExtBidPrebid.of(null, singletonMap("hb_cache_id_bidder1", "value1"), null, - null, null, null), + ExtBidPrebid.of(null, null, singletonMap("hb_cache_id_bidder1", "value1"), + null, null, null, null), null))) .build())) .build()))