Skip to content

Commit

Permalink
Extract bids removal into separate component (#1080)
Browse files Browse the repository at this point in the history
  • Loading branch information
@schernysh
schernysh authored Jan 22, 2021
1 parent 9b20db8 commit 756c859
Show file tree
Hide file tree
Showing 5 changed files with 234 additions and 58 deletions.
49 changes: 10 additions & 39 deletions src/main/java/org/prebid/server/auction/BidResponseCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.EnumMap;
import java.util.HashMap;
import java.util.HashSet;
Expand All @@ -102,6 +101,7 @@ public class BidResponseCreator {
private final BidderCatalog bidderCatalog;
private final EventsService eventsService;
private final StoredRequestProcessor storedRequestProcessor;
private final BidResponseReducer bidResponseReducer;
private final IdGenerator bidIdGenerator;
private final int truncateAttrChars;
private final Clock clock;
Expand All @@ -115,6 +115,7 @@ public BidResponseCreator(CacheService cacheService,
BidderCatalog bidderCatalog,
EventsService eventsService,
StoredRequestProcessor storedRequestProcessor,
BidResponseReducer bidResponseReducer,
IdGenerator bidIdGenerator,
int truncateAttrChars,
Clock clock,
Expand All @@ -124,6 +125,7 @@ public BidResponseCreator(CacheService cacheService,
this.bidderCatalog = Objects.requireNonNull(bidderCatalog);
this.eventsService = Objects.requireNonNull(eventsService);
this.storedRequestProcessor = Objects.requireNonNull(storedRequestProcessor);
this.bidResponseReducer = Objects.requireNonNull(bidResponseReducer);
this.bidIdGenerator = Objects.requireNonNull(bidIdGenerator);
this.truncateAttrChars = validateTruncateAttrChars(truncateAttrChars);
this.clock = Objects.requireNonNull(clock);
Expand Down Expand Up @@ -195,7 +197,9 @@ private Future<BidResponse> cacheBidsAndCreateResponse(List<BidderResponse> bidd

final BidRequest bidRequest = auctionContext.getBidRequest();

bidderResponses.forEach(BidResponseCreator::removeRedundantBids);
final List<BidderResponse> updatedBidderResponses = bidderResponses.stream()
.map(bidResponseReducer::removeRedundantBids)
.collect(Collectors.toList());

ExtRequestTargeting targeting = targeting(bidRequest);
final Set<Bid> winningBids = newOrEmptySet(targeting);
Expand All @@ -208,12 +212,12 @@ private Future<BidResponse> cacheBidsAndCreateResponse(List<BidderResponse> bidd

// determine winning bids only if targeting is present
if (targeting != null) {
populateWinningBids(bidderResponses, winningBids, winningBidsByBidder);
populateWinningBids(updatedBidderResponses, winningBids, winningBidsByBidder);
}

final Set<Bid> bidsToCache = cacheInfo.isShouldCacheWinningBidsOnly()
? winningBids
: bidderResponses.stream().flatMap(BidResponseCreator::getBids).collect(Collectors.toSet());
: updatedBidderResponses.stream().flatMap(BidResponseCreator::getBids).collect(Collectors.toSet());

final EventsContext eventsContext = EventsContext.builder()
.enabledForAccount(eventsEnabledForAccount(auctionContext))
Expand All @@ -223,15 +227,15 @@ private Future<BidResponse> cacheBidsAndCreateResponse(List<BidderResponse> bidd
.build();

return toBidsWithCacheIds(
bidderResponses,
updatedBidderResponses,
bidsToCache,
generatedBidIds,
auctionContext,
cacheInfo,
eventsContext)
.compose(cacheResult -> videoStoredDataResult(auctionContext)
.map(videoStoredDataResult -> toBidResponse(
bidderResponses,
updatedBidderResponses,
auctionContext,
targeting,
winningBids,
Expand Down Expand Up @@ -286,39 +290,6 @@ private ExtBidResponse toExtBidResponse(List<BidderResponse> bidderResponses,
ExtBidResponsePrebid.of(auctionTimestamp));
}

private static void removeRedundantBids(BidderResponse bidderResponse) {
final List<BidderBid> responseBidderBids = bidderResponse.getSeatBid().getBids();
if (responseBidderBids.size() < 2) { // no reason for removing if only one bid was responded
return;
}

final Map<String, List<BidderBid>> impIdToBidderBid = responseBidderBids.stream()
.collect(Collectors.groupingBy(bidderBid -> bidderBid.getBid().getImpid()));

final Set<BidderBid> mostValuableBids = impIdToBidderBid.values().stream()
.map(BidResponseCreator::mostValuableBid)
.collect(Collectors.toSet());

responseBidderBids.clear();
responseBidderBids.addAll(mostValuableBids);
}

private static BidderBid mostValuableBid(List<BidderBid> bidderBids) {
if (bidderBids.size() == 1) {
return bidderBids.get(0);
}

final List<BidderBid> dealBidderBids = bidderBids.stream()
.filter(bidderBid -> StringUtils.isNotBlank(bidderBid.getBid().getDealid()))
.collect(Collectors.toList());

final List<BidderBid> processedBidderBids = dealBidderBids.isEmpty() ? bidderBids : dealBidderBids;

return processedBidderBids.stream()
.max(Comparator.comparing(bidderBid -> bidderBid.getBid().getPrice(), Comparator.naturalOrder()))
.orElse(bidderBids.get(0));
}

/**
* Returns new {@link HashSet} in case of existing keywordsCreator or empty collection if null.
*/
Expand Down
85 changes: 85 additions & 0 deletions src/main/java/org/prebid/server/auction/BidResponseReducer.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package org.prebid.server.auction;

import com.iab.openrtb.response.Bid;
import org.apache.commons.collections4.ListUtils;
import org.apache.commons.lang3.StringUtils;
import org.prebid.server.auction.model.BidderResponse;
import org.prebid.server.bidder.model.BidderBid;
import org.prebid.server.bidder.model.BidderSeatBid;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

/**
* Class for removing bids from response for the same bidder-imp pair.
*/
public class BidResponseReducer {

/**
* Removes {@link Bid}s with the same impId taking into account if {@link Bid} has deal.
* <p>
* Returns given list of {@link BidderResponse}s if {@link Bid}s have different impIds.
*/
public BidderResponse removeRedundantBids(BidderResponse bidderResponse) {
final List<BidderBid> bidderBids = ListUtils.emptyIfNull(bidderResponse.getSeatBid().getBids());
final Map<String, List<BidderBid>> impIdToBidderBids = bidderBids.stream()
.collect(Collectors.groupingBy(bidderBid -> bidderBid.getBid().getImpid()));

final Set<BidderBid> updatedBidderBids = impIdToBidderBids.values().stream()
.map(BidResponseReducer::removeRedundantBidsForImp)
.flatMap(Collection::stream)
.collect(Collectors.toSet());

if (bidderBids.size() == updatedBidderBids.size()) {
return bidderResponse;
}

return updateBidderResponse(bidderResponse, updatedBidderBids);
}

private static List<BidderBid> removeRedundantBidsForImp(List<BidderBid> bidderBids) {
return bidderBids.size() > 1 ? reduceBidsByImpId(bidderBids) : bidderBids;
}

private static List<BidderBid> reduceBidsByImpId(List<BidderBid> bidderBids) {
return bidderBids.stream().anyMatch(bidderBid -> bidderBid.getBid().getDealid() != null)
? removeRedundantDealsBids(bidderBids)
: removeRedundantForNonDealBids(bidderBids);
}

private static List<BidderBid> removeRedundantDealsBids(List<BidderBid> bidderBids) {
final List<BidderBid> dealBidderBids = bidderBids.stream()
.filter(bidderBid -> StringUtils.isNotBlank(bidderBid.getBid().getDealid()))
.collect(Collectors.toList());

return Collections.singletonList(getHighestPriceBid(bidderBids, dealBidderBids));
}

private static List<BidderBid> removeRedundantForNonDealBids(List<BidderBid> bidderBids) {
return Collections.singletonList(getHighestPriceBid(bidderBids, bidderBids));
}

private static BidderBid getHighestPriceBid(List<BidderBid> bidderBids, List<BidderBid> dealBidderBids) {
return dealBidderBids.stream()
.max(Comparator.comparing(bidderBid -> bidderBid.getBid().getPrice(), Comparator.naturalOrder()))
.orElse(bidderBids.get(0));
}

private static BidderResponse updateBidderResponse(BidderResponse bidderResponse,
Set<BidderBid> updatedBidderBids) {

final BidderSeatBid seatBid = bidderResponse.getSeatBid();
final BidderSeatBid updatedSeatBid = BidderSeatBid.of(
new ArrayList<>(updatedBidderBids),
seatBid.getHttpCalls(),
seatBid.getErrors());

return BidderResponse.of(bidderResponse.getBidder(), updatedSeatBid, bidderResponse.getResponseTime());
}
}
8 changes: 8 additions & 0 deletions src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.prebid.server.auction.AuctionRequestFactory;
import org.prebid.server.auction.BidResponseCreator;
import org.prebid.server.auction.BidResponsePostProcessor;
import org.prebid.server.auction.BidResponseReducer;
import org.prebid.server.auction.ExchangeService;
import org.prebid.server.auction.FpdResolver;
import org.prebid.server.auction.ImplicitParametersExtractor;
Expand Down Expand Up @@ -469,6 +470,7 @@ BidResponseCreator bidResponseCreator(
BidderCatalog bidderCatalog,
EventsService eventsService,
StoredRequestProcessor storedRequestProcessor,
BidResponseReducer bidResponseReducer,
IdGenerator bidIdGenerator,
@Value("${settings.targeting.truncate-attr-chars}") int truncateAttrChars,
Clock clock,
Expand All @@ -479,6 +481,7 @@ BidResponseCreator bidResponseCreator(
bidderCatalog,
eventsService,
storedRequestProcessor,
bidResponseReducer,
bidIdGenerator,
truncateAttrChars,
clock,
Expand Down Expand Up @@ -541,6 +544,11 @@ StoredRequestProcessor storedRequestProcessor(
jsonMerger);
}

@Bean
BidResponseReducer bidResponseReducer() {
return new BidResponseReducer();
}

@Bean
StoredResponseProcessor storedResponseProcessor(ApplicationSettings applicationSettings,
BidderCatalog bidderCatalog,
Expand Down
34 changes: 15 additions & 19 deletions src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ public class BidResponseCreatorTest extends VertxTest {
@Mock
private StoredRequestProcessor storedRequestProcessor;
@Mock
private BidResponseReducer bidResponseReducer;
@Mock
private IdGenerator idGenerator;

private Clock clock;
Expand All @@ -135,6 +137,8 @@ public void setUp() {
given(cacheService.getEndpointHost()).willReturn("testHost");
given(cacheService.getEndpointPath()).willReturn("testPath");
given(cacheService.getCachedAssetURLTemplate()).willReturn("uuid=");
given(bidResponseReducer.removeRedundantBids(any()))
.willAnswer(invocationOnMock -> invocationOnMock.getArgument(0));

given(storedRequestProcessor.videoStoredDataResult(any(), anyList(), anyList(), any()))
.willReturn(Future.succeededFuture(VideoStoredDataResult.empty()));
Expand All @@ -147,6 +151,7 @@ public void setUp() {
bidderCatalog,
eventsService,
storedRequestProcessor,
bidResponseReducer,
idGenerator,
0,
clock,
Expand Down Expand Up @@ -502,6 +507,7 @@ public void shouldOverrideBidIdWhenGenerateBidIdIsTurnedOn() {
bidderCatalog,
eventsService,
storedRequestProcessor,
bidResponseReducer,
idGenerator,
0,
clock,
Expand Down Expand Up @@ -566,6 +572,7 @@ public void shouldUseGeneratedBidIdForEventAndCacheWhenGeneratedIdIsTurnedOn() {
bidderCatalog,
eventsService,
storedRequestProcessor,
bidResponseReducer,
idGenerator,
0,
clock,
Expand Down Expand Up @@ -621,44 +628,32 @@ public void shouldSetExpectedResponseSeatBidAndBidFields() {
}

@Test
public void shouldFilterByDealsAndPriceBidsWhenImpIdsAreEqual() {
public void shouldFilterBidByBidReducerResponse() {
// given
final AuctionContext auctionContext = givenAuctionContext(givenBidRequest());

final Bid simpleBidImp1 = Bid.builder().id("bidId1i1").price(BigDecimal.valueOf(5.67)).impid("i1").build();
final Bid simpleBid1Imp2 = Bid.builder().id("bidId1i2").price(BigDecimal.valueOf(15.67)).impid("i2").build();
final Bid simpleBid2Imp2 = Bid.builder().id("bidId2i2").price(BigDecimal.valueOf(17.67)).impid("i2").build();
final Bid dealBid1Imp1 = Bid.builder().id("bidId1i1d").dealid("d1").price(BigDecimal.valueOf(4.98)).impid("i1")
.build();
final Bid dealBid2Imp1 = Bid.builder().id("bidId2i1d").dealid("d2").price(BigDecimal.valueOf(5.00)).impid("i1")
.build();
final BidderSeatBid seatBidWithDeals = givenSeatBid(
BidderBid.of(simpleBidImp1, banner, null),
BidderBid.of(simpleBid1Imp2, banner, null),
BidderBid.of(simpleBid2Imp2, banner, null), // will stay (top price)
BidderBid.of(simpleBid2Imp2, banner, null), // duplicate should be removed
BidderBid.of(dealBid2Imp1, banner, null), // will stay (deal + topPrice)
BidderBid.of(dealBid2Imp1, banner, null),
BidderBid.of(dealBid1Imp1, banner, null));

final Bid simpleBid3Imp2 = Bid.builder().id("bidId3i2").price(BigDecimal.valueOf(7.25)).impid("i2").build();
final Bid simpleBid4Imp2 = Bid.builder().id("bidId4i2").price(BigDecimal.valueOf(7.26)).impid("i2").build();
final BidderSeatBid seatBidWithSimpleBids = givenSeatBid(
BidderBid.of(simpleBid3Imp2, banner, null),
BidderBid.of(simpleBid4Imp2, banner, null)); // will stay (top price)
final List<BidderResponse> bidderResponses = singletonList(BidderResponse.of("bidder1", seatBidWithDeals, 100));

final List<BidderResponse> bidderResponses = asList(
BidderResponse.of("bidder1", seatBidWithDeals, 100),
BidderResponse.of("bidder2", seatBidWithSimpleBids, 111));
given(bidResponseReducer.removeRedundantBids(any()))
.willReturn(BidderResponse.of("bidder1", givenSeatBid(BidderBid.of(dealBid2Imp1, banner, null)), 100));

// when
final BidResponse bidResponse =
bidResponseCreator.create(bidderResponses, auctionContext, CACHE_INFO, false).result();

// then
assertThat(bidResponse.getSeatbid())
.flatExtracting(SeatBid::getBid).hasSize(3)
.flatExtracting(SeatBid::getBid).hasSize(1)
.extracting(Bid::getId)
.containsOnly("bidId2i2", "bidId2i1d", "bidId4i2");
.containsOnly("bidId2i1d");
}

@Test
Expand Down Expand Up @@ -1000,6 +995,7 @@ public void shouldTruncateTargetingKeywordsByGlobalConfig() {
bidderCatalog,
eventsService,
storedRequestProcessor,
bidResponseReducer,
idGenerator,
20,
clock,
Expand Down
Loading

0 comments on commit 756c859

Please sign in to comment.