Skip to content

Commit

Permalink
Make generation of Bid Adm as a first step to remove duplicating call (…
Browse files Browse the repository at this point in the history 
…#1214)
  • Loading branch information
@DGarbar
DGarbar authored Apr 9, 2021
1 parent 53221a0 commit eb3fd78
Show file tree
Hide file tree
Showing 13 changed files with 151 additions and 154 deletions.
96 changes: 66 additions & 30 deletions src/main/java/org/prebid/server/auction/BidResponseCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.prebid.server.auction.model.MultiBidConfig;
import org.prebid.server.auction.model.TargetingBidInfo;
import org.prebid.server.bidder.BidderCatalog;
import org.prebid.server.bidder.model.BidderBid;
import org.prebid.server.bidder.model.BidderError;
import org.prebid.server.bidder.model.BidderSeatBid;
import org.prebid.server.cache.CacheService;
Expand Down Expand Up @@ -154,6 +155,17 @@ Future<BidResponse> create(List<BidderResponse> bidderResponses,
boolean debugEnabled) {

final long auctionTimestamp = auctionTimestamp(auctionContext);
final Account account = auctionContext.getAccount();

final EventsContext eventsContext = EventsContext.builder()
.enabledForAccount(eventsEnabledForAccount(auctionContext))
.enabledForRequest(eventsEnabledForRequest(auctionContext))
.auctionTimestamp(auctionTimestamp)
.integration(integrationFrom(auctionContext))
.build();

final Map<String, String> bidIdToGeneratedBidId = new HashMap<>();
updateBidAdmInBidderResponses(bidderResponses, account, bidIdToGeneratedBidId, eventsContext);

if (isEmptyBidderResponses(bidderResponses)) {
final BidRequest bidRequest = auctionContext.getBidRequest();
Expand All @@ -178,10 +190,41 @@ Future<BidResponse> create(List<BidderResponse> bidderResponses,
auctionContext,
cacheInfo,
bidderToMultiBids,
auctionTimestamp,
bidIdToGeneratedBidId,
eventsContext,
debugEnabled);
}

private void updateBidAdmInBidderResponses(List<BidderResponse> bidderResponses,
Account account,
Map<String, String> bidIdToGeneratedBidId,
EventsContext eventsContext) {
for (BidderResponse bidderResponse : bidderResponses) {
final String bidder = bidderResponse.getBidder();

for (BidderBid bidderBid : bidderResponse.getSeatBid().getBids()) {
final Bid bid = bidderBid.getBid();
final String generatedBidId = bidIdGenerator.getType() != IdGeneratorType.none
? bidIdGenerator.generateId()
: null;
final String bidId = bid.getId();
bidIdToGeneratedBidId.put(bidId, generatedBidId);

if (bidderBid.getType().equals(BidType.video)) {
final String adm = vastModifier.createBidVastXml(
bidder,
bid.getAdm(),
bid.getNurl(),
generatedBidId == null ? bidId : generatedBidId,
account.getId(),
eventsContext);

bid.setAdm(adm);
}
}
}
}

private static int validateTruncateAttrChars(int truncateAttrChars) {
if (truncateAttrChars < 0 || truncateAttrChars > 255) {
throw new IllegalArgumentException("truncateAttrChars must be between 0 and 255");
Expand All @@ -202,13 +245,14 @@ private Future<BidResponse> cacheBidsAndCreateResponse(List<BidderResponse> bidd
AuctionContext auctionContext,
BidRequestCacheInfo cacheInfo,
Map<String, MultiBidConfig> bidderToMultiBids,
long auctionTimestamp,
Map<String, String> bidIdToGeneratedBidId,
EventsContext eventsContext,
boolean debugEnabled) {
final BidRequest bidRequest = auctionContext.getBidRequest();

final List<Imp> imps = bidRequest.getImp();
final Map<BidderResponse, List<TargetingBidInfo>> bidderResponseToTargetingBidInfos =
toBidderResponseWithTargetingBidInfos(bidderResponses, imps, bidderToMultiBids);
toBidderResponseWithTargetingBidInfos(bidderResponses, imps, bidderToMultiBids, bidIdToGeneratedBidId);

final Set<BidInfo> bidInfos = bidderResponseToTargetingBidInfos.values().stream()
.filter(CollectionUtils::isNotEmpty)
Expand All @@ -228,13 +272,6 @@ private Future<BidResponse> cacheBidsAndCreateResponse(List<BidderResponse> bidd

final Set<BidInfo> bidsToCache = cacheInfo.isShouldCacheWinningBidsOnly() ? winningBidInfos : bidInfos;

final EventsContext eventsContext = EventsContext.builder()
.enabledForAccount(eventsEnabledForAccount(auctionContext))
.enabledForRequest(eventsEnabledForRequest(auctionContext))
.auctionTimestamp(auctionTimestamp)
.integration(integrationFrom(auctionContext))
.build();

return cacheBids(bidsToCache, auctionContext, cacheInfo, eventsContext)
.compose(cacheResult -> videoStoredDataResult(auctionContext)
.map(videoStoredDataResult -> toBidResponse(
Expand All @@ -245,7 +282,6 @@ private Future<BidResponse> cacheBidsAndCreateResponse(List<BidderResponse> bidd
cacheResult,
videoStoredDataResult,
eventsContext,
auctionTimestamp,
debugEnabled)));
}

Expand All @@ -258,12 +294,14 @@ private static ExtRequestTargeting targeting(BidRequest bidRequest) {
private Map<BidderResponse, List<TargetingBidInfo>> toBidderResponseWithTargetingBidInfos(
List<BidderResponse> bidderResponses,
List<Imp> imps,
Map<String, MultiBidConfig> bidderToMultiBids) {
Map<String, MultiBidConfig> bidderToMultiBids,
Map<String, String> bidIdToGeneratedBidId) {

final Map<BidderResponse, List<BidInfo>> bidderResponseToReducedBidInfos = bidderResponses.stream()
.collect(Collectors.toMap(
Function.identity(),
bidderResponse -> toSortedMultiBidInfo(bidderResponse, imps, bidderToMultiBids)));
bidderResponse -> toSortedMultiBidInfo(bidderResponse, imps, bidderToMultiBids,
bidIdToGeneratedBidId)));

final Map<String, Map<String, List<BidInfo>>> impIdToBidderToBidInfos = bidderResponseToReducedBidInfos.values()
.stream()
Expand Down Expand Up @@ -300,8 +338,9 @@ private Map<BidderResponse, List<TargetingBidInfo>> toBidderResponseWithTargetin

private List<BidInfo> toSortedMultiBidInfo(BidderResponse bidderResponse,
List<Imp> imps,
Map<String, MultiBidConfig> bidderToMultiBids) {
final List<BidInfo> bidInfos = toBidInfo(bidderResponse, imps);
Map<String, MultiBidConfig> bidderToMultiBids,
Map<String, String> bidIdToGeneratedBidId) {
final List<BidInfo> bidInfos = toBidInfo(bidderResponse, imps, bidIdToGeneratedBidId);
final Map<String, List<BidInfo>> impIdToBidInfos = bidInfos.stream()
.collect(Collectors.groupingBy(bidInfo -> bidInfo.getCorrespondingImp().getId()));

Expand All @@ -314,20 +353,27 @@ private List<BidInfo> toSortedMultiBidInfo(BidderResponse bidderResponse,
.collect(Collectors.toList());
}

private List<BidInfo> toBidInfo(BidderResponse bidderResponse, List<Imp> imps) {
private List<BidInfo> toBidInfo(BidderResponse bidderResponse,
List<Imp> imps,
Map<String, String> bidIdToGeneratedBidId) {
return Stream.of(bidderResponse)
.map(BidderResponse::getSeatBid)
.filter(Objects::nonNull)
.map(BidderSeatBid::getBids)
.filter(Objects::nonNull)
.flatMap(Collection::stream)
.map(bidderBid -> toBidInfo(bidderBid.getBid(), bidderBid.getType(), imps, bidderResponse.getBidder()))
.map(bidderBid -> toBidInfo(bidderBid.getBid(), bidderBid.getType(), imps, bidderResponse.getBidder(),
bidIdToGeneratedBidId))
.collect(Collectors.toList());
}

private BidInfo toBidInfo(Bid bid, BidType type, List<Imp> imps, String bidder) {
private BidInfo toBidInfo(Bid bid,
BidType type,
List<Imp> imps,
String bidder,
Map<String, String> bidIdToGeneratedBidId) {
return BidInfo.builder()
.generatedBidId(bidIdGenerator.getType() != IdGeneratorType.none ? bidIdGenerator.generateId() : null)
.generatedBidId(bidIdToGeneratedBidId.get(bid.getId()))
.bid(bid)
.bidType(type)
.bidder(bidder)
Expand Down Expand Up @@ -693,7 +739,6 @@ private BidResponse toBidResponse(Map<BidderResponse, List<TargetingBidInfo>> bi
CacheServiceResult cacheResult,
VideoStoredDataResult videoStoredDataResult,
EventsContext eventsContext,
long auctionTimestamp,
boolean debugEnabled) {

final BidRequest bidRequest = auctionContext.getBidRequest();
Expand All @@ -714,6 +759,7 @@ private BidResponse toBidResponse(Map<BidderResponse, List<TargetingBidInfo>> bi
eventsContext))
.collect(Collectors.toList());

final Long auctionTimestamp = eventsContext.getAuctionTimestamp();
final ExtBidResponse extBidResponse = toExtBidResponse(
bidderResponseToTargetingBidInfos.keySet(),
auctionContext,
Expand Down Expand Up @@ -839,16 +885,6 @@ private Bid toBid(TargetingBidInfo targetingBidInfo,
if ((videoCacheId != null && !requestCacheInfo.isReturnCreativeVideoBids())
|| (cacheId != null && !requestCacheInfo.isReturnCreativeBids())) {
bid.setAdm(null);
} else if (bidType.equals(BidType.video)) {
final String adm = vastModifier.createBidVastXml(
bidder,
bid.getAdm(),
bid.getNurl(),
bidInfo.getBidId(),
account.getId(),
eventsContext);

bid.setAdm(adm);
}

final boolean isApp = bidRequest.getApp() != null;
Expand Down
14 changes: 3 additions & 11 deletions src/main/java/org/prebid/server/cache/CacheService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ private Future<CacheServiceResult> doCacheOpenrtb(List<CacheBid> bids,
final String accountId = account.getId();
final List<CachedCreative> cachedCreatives = Stream.concat(
bids.stream().map(cacheBid -> createJsonPutObjectOpenrtb(cacheBid, accountId, eventsContext)),
videoBids.stream().map(cacheBid -> createXmlPutObjectOpenrtb(cacheBid, accountId, eventsContext)))
videoBids.stream().map(this::createXmlPutObjectOpenrtb))
.collect(Collectors.toList());

if (cachedCreatives.isEmpty()) {
Expand Down Expand Up @@ -403,18 +403,10 @@ private CachedCreative createJsonPutObjectOpenrtb(CacheBid cacheBid,
/**
* Makes XML type {@link PutObject} from {@link com.iab.openrtb.response.Bid}. Used for OpenRTB auction request.
*/
private CachedCreative createXmlPutObjectOpenrtb(CacheBid cacheBid,
String accountId,
EventsContext eventsContext) {
private CachedCreative createXmlPutObjectOpenrtb(CacheBid cacheBid) {
final BidInfo bidInfo = cacheBid.getBidInfo();
final com.iab.openrtb.response.Bid bid = bidInfo.getBid();
final String vastXml = vastModifier.createBidVastXml(
bidInfo.getBidder(),
bid.getAdm(),
bid.getNurl(),
bidInfo.getBidId(),
accountId,
eventsContext);
final String vastXml = bid.getAdm();

final PutObject payload = PutObject.builder()
.type("xml")
Expand Down
116 changes: 66 additions & 50 deletions src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import com.iab.openrtb.response.Response;
import com.iab.openrtb.response.SeatBid;
import io.vertx.core.Future;
import org.apache.commons.collections4.CollectionUtils;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -315,10 +314,12 @@ public void shouldRequestCacheServiceWithWinningBidsOnlyWhenWinningonlyIsTrue()
eq(EventsContext.builder().auctionTimestamp(1000L).build()));
}

@SuppressWarnings("unchecked")
@Test
public void shouldRequestCacheServiceWithVideoBidsToModify() {
// given
final Account account = Account.builder().id("accountId").eventsEnabled(true).build();
final String accountId = "accountId";
final Account account = Account.builder().id(accountId).eventsEnabled(true).build();

final Imp imp1 = givenImp("impId1");
final Imp imp2 = givenImp("impId2");
Expand All @@ -329,35 +330,88 @@ public void shouldRequestCacheServiceWithVideoBidsToModify() {
imp1, imp2),
contextBuilder -> contextBuilder.account(account));

final Bid bid1 = Bid.builder().id("bidId1").impid("impId1").price(BigDecimal.valueOf(5.67)).build();
final String bidId1 = "bidId1";
final Bid bid1 = Bid.builder().id(bidId1).impid("impId1").price(BigDecimal.valueOf(5.67)).nurl(BID_NURL)
.build();
final Bid bid2 = Bid.builder().id("bidId2").impid("impId2").price(BigDecimal.valueOf(7.19)).build();

final String bidder1 = "bidder1";
final List<BidderResponse> bidderResponses = asList(
BidderResponse.of("bidder1", givenSeatBid(BidderBid.of(bid1, video, "USD")), 100),
BidderResponse.of(bidder1, givenSeatBid(BidderBid.of(bid1, video, "USD")), 100),
BidderResponse.of("bidder2", givenSeatBid(BidderBid.of(bid2, banner, "USD")), 100));

final BidRequestCacheInfo cacheInfo = BidRequestCacheInfo.builder()
.doCaching(true)
.shouldCacheVideoBids(true)
.build();

final String modifiedAdm = "modifiedAdm";
given(vastModifier.createBidVastXml(any(), any(), any(), any(), any(), any())).willReturn(modifiedAdm);

// just a stub to get through method call chain
givenCacheServiceResult(singletonMap(bid1, CacheInfo.empty()));

// when
bidResponseCreator.create(bidderResponses, auctionContext, cacheInfo, MULTI_BIDS, false);

// then
final BidInfo bidInfo1 = toBidInfo(bid1, imp1, "bidder1", video);
final BidInfo bidInfo2 = toBidInfo(bid2, imp2, "bidder2", banner);
final EventsContext expectedEventContext = EventsContext.builder()
.enabledForAccount(true)
.enabledForRequest(true)
.auctionTimestamp(1000L)
.build();

verify(vastModifier).createBidVastXml(bidder1, null, BID_NURL, bidId1, accountId, expectedEventContext);

final ArgumentCaptor<List<BidInfo>> bidInfoCaptor = ArgumentCaptor.forClass(List.class);
verify(cacheService).cacheBidsOpenrtb(
argThat(argument -> CollectionUtils.isEqualCollection(argument, asList(bidInfo1, bidInfo2))),
bidInfoCaptor.capture(),
same(auctionContext),
eq(CacheContext.builder().shouldCacheVideoBids(true).build()),
eq(EventsContext.builder()
.enabledForAccount(true)
.enabledForRequest(true)
.auctionTimestamp(1000L)
.build()));
eq(expectedEventContext));

final Bid expectedUpdatedBid1 = bid1.toBuilder().adm(modifiedAdm).build();
final BidInfo bidInfo1 = toBidInfo(expectedUpdatedBid1, imp1, bidder1, video);
final BidInfo bidInfo2 = toBidInfo(bid2, imp2, "bidder2", banner);
assertThat(bidInfoCaptor.getValue()).containsOnly(bidInfo1, bidInfo2);
}

@Test
public void shouldModifyBidAdmWhenBidVideoAndVastModifierReturnValue() {
// given
final AuctionContext auctionContext = givenAuctionContext(givenBidRequest(
identity(),
extBuilder -> extBuilder.targeting(givenTargeting()),
givenImp()));

final String bidId = "bid_id";
final Bid bid = Bid.builder()
.id(bidId)
.price(BigDecimal.ONE)
.adm(BID_ADM)
.nurl(BID_NURL)
.impid(IMP_ID)
.build();

final String bidder = "bidder1";
final List<BidderResponse> bidderResponses = singletonList(
BidderResponse.of(bidder, givenSeatBid(BidderBid.of(bid, video, "USD")), 100));

final String modifiedVast = "modifiedVast";
given(vastModifier.createBidVastXml(anyString(), anyString(), anyString(), anyString(), anyString(), any()))
.willReturn(modifiedVast);

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

// then
assertThat(bidResponse.getSeatbid())
.flatExtracting(SeatBid::getBid).hasSize(1)
.extracting(Bid::getAdm)
.containsOnly(modifiedVast);

verify(vastModifier).createBidVastXml(eq(bidder), eq(BID_ADM), eq(BID_NURL), eq(bidId), eq("accountId"), any());
}

@Test
Expand Down Expand Up @@ -865,44 +919,6 @@ public void shouldSetBidAdmToNullIfVideoCacheIdIsPresentAndReturnCreativeVideoBi
verify(cacheService).cacheBidsOpenrtb(anyList(), any(), any(), any());
}

@Test
public void shouldModifyBidAdmWhenBidVideoAndVastModifierReturnValue() {
// given
final AuctionContext auctionContext = givenAuctionContext(givenBidRequest(
identity(),
extBuilder -> extBuilder.targeting(givenTargeting()),
givenImp()));

final String bidId = "bid_id";
final Bid bid = Bid.builder()
.id(bidId)
.price(BigDecimal.ONE)
.adm(BID_ADM)
.nurl(BID_NURL)
.impid(IMP_ID)
.build();

final String bidder = "bidder1";
final List<BidderResponse> bidderResponses = singletonList(
BidderResponse.of(bidder, givenSeatBid(BidderBid.of(bid, video, "USD")), 100));

final String modifiedVast = "modifiedVast";
given(vastModifier.createBidVastXml(anyString(), anyString(), anyString(), anyString(), anyString(), any()))
.willReturn(modifiedVast);

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

// then
assertThat(bidResponse.getSeatbid())
.flatExtracting(SeatBid::getBid).hasSize(1)
.extracting(Bid::getAdm)
.containsOnly(modifiedVast);

verify(vastModifier).createBidVastXml(eq(bidder), eq(BID_ADM), eq(BID_NURL), eq(bidId), eq("accountId"), any());
}

@Test
public void shouldSetBidExpWhenCacheIdIsMatched() {
// given
Expand Down
Loading

0 comments on commit eb3fd78

Please sign in to comment.