Skip to content

Commit

Permalink
Overwrite Bid Id (#616)
Browse files Browse the repository at this point in the history
  • Loading branch information
@apaliy
apaliy authored May 20, 2020
1 parent 76c53ab commit 643ca3d
Show file tree
Hide file tree
Showing 9 changed files with 80 additions and 27 deletions.
15 changes: 12 additions & 3 deletions src/main/java/org/prebid/server/auction/BidResponseCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -86,18 +88,21 @@ 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;
private final String cachePath;
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());
Expand Down Expand Up @@ -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<ExtBidPrebid, ObjectNode> bidExt = ExtPrebid.of(prebidExt, bid.getExt());
bid.setExt(mapper.mapper().valueToTree(bidExt));

Expand Down
2 changes: 2 additions & 0 deletions src/main/java/org/prebid/server/proto/openrtb/ext/response/ExtBidPrebid.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
@Value
public class ExtBidPrebid {

String bidid;

BidType type;

Map<String, String> targeting;
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/application.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 41 additions & 4 deletions src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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<ExtBidPrebid, ?> 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<BidderResponse> 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
Expand All @@ -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());

Expand Down Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/prebid/server/auction/ExchangeServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1883,7 +1883,7 @@ private static Bid givenBid(Function<Bid.BidBuilder, Bid.BidBuilder> 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();
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/prebid/server/auction/StoredResponseProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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> seatBid = singletonList(SeatBid.builder()
.seat("rubicon").bid(singletonList(Bid.builder().ext(mapper.createObjectNode()
Expand Down
7 changes: 4 additions & 3 deletions src/test/java/org/prebid/server/auction/VideoResponseFactoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
29 changes: 15 additions & 14 deletions src/test/java/org/prebid/server/handler/openrtb2/AmpHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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()))
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);

Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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 -> {
Expand All @@ -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);
Expand All @@ -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);

Expand Down Expand Up @@ -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
Expand All @@ -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()))
Expand Down

0 comments on commit 643ca3d

Please sign in to comment.