Skip to content

Commit

Permalink
Add modifying of VAST for video bids and add validation
Browse files Browse the repository at this point in the history 
- Add validation for Video bids. `bid.adm` or `bid.nurl` needs to be present
  - This case now, is not possible `<VASTAdTagURI><![CDATA[null]]></VASTAdTagURI>`
- Bid adm will be updated as cache. (see prebid/prebid-server#1015)
- Bid type is defined by bidder, not our (`imp.video != null`, etc) checks. For example Appnexus use `bid.ext.appnexus.bid_ad_type` to define it. (that's why there are a lot of changed cache jsons. Also add ordering for tests)

Refactored a bit.
- Removed confusing maps
- Removed confusing checks
- Removed several imp to bid matching

Possible improvements (in another ticket bc current PR is too large)
- Extract more event URL to another class
- Use bidInfo in BidResponseCreator for BidResponse
  • Loading branch information
@DGarbar
DGarbar committed Dec 18, 2020
1 parent 5e331cb commit e1cadf5
Show file tree
Hide file tree
Showing 46 changed files with 1,032 additions and 883 deletions.
130 changes: 68 additions & 62 deletions src/main/java/org/prebid/server/auction/BidResponseCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@
import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import org.prebid.server.auction.model.AuctionContext;
import org.prebid.server.auction.model.BidInfo;
import org.prebid.server.auction.model.BidRequestCacheInfo;
import org.prebid.server.auction.model.BidderResponse;
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.vast.VastModifier;
import org.prebid.server.cache.CacheService;
import org.prebid.server.cache.model.CacheContext;
import org.prebid.server.cache.model.CacheInfo;
Expand Down Expand Up @@ -98,6 +100,7 @@ public class BidResponseCreator {

private final CacheService cacheService;
private final BidderCatalog bidderCatalog;
private final VastModifier vastModifier;
private final EventsService eventsService;
private final StoredRequestProcessor storedRequestProcessor;
private final boolean generateBidId;
Expand All @@ -111,6 +114,7 @@ public class BidResponseCreator {

public BidResponseCreator(CacheService cacheService,
BidderCatalog bidderCatalog,
VastModifier vastModifier,
EventsService eventsService,
StoredRequestProcessor storedRequestProcessor,
boolean generateBidId,
Expand All @@ -120,6 +124,7 @@ public BidResponseCreator(CacheService cacheService,

this.cacheService = Objects.requireNonNull(cacheService);
this.bidderCatalog = Objects.requireNonNull(bidderCatalog);
this.vastModifier = Objects.requireNonNull(vastModifier);
this.eventsService = Objects.requireNonNull(eventsService);
this.storedRequestProcessor = Objects.requireNonNull(storedRequestProcessor);
this.generateBidId = generateBidId;
Expand Down Expand Up @@ -205,9 +210,17 @@ private Future<BidResponse> cacheBidsAndCreateResponse(List<BidderResponse> bidd
populateWinningBids(bidderResponses, winningBids, winningBidsByBidder);
}

final Set<Bid> bidsToCache = cacheInfo.isShouldCacheWinningBidsOnly()
? winningBids
: bidderResponses.stream().flatMap(BidResponseCreator::getBids).collect(Collectors.toSet());
final List<Imp> imps = bidRequest.getImp();
final Set<BidInfo> bidInfos = bidderResponses.stream()
.map(bidderResponse -> toBidInfo(bidderResponse, imps))
.flatMap(Collection::stream)
.collect(Collectors.toSet());

final Set<BidInfo> bidsToCache = !cacheInfo.isShouldCacheWinningBidsOnly()
? bidInfos
: bidInfos.stream()
.filter(bidInfo -> winningBids.contains(bidInfo.getBid()))
.collect(Collectors.toSet());

final EventsContext eventsContext = EventsContext.builder()
.enabledForAccount(eventsEnabledForAccount(auctionContext))
Expand All @@ -216,8 +229,7 @@ private Future<BidResponse> cacheBidsAndCreateResponse(List<BidderResponse> bidd
.integration(integrationFrom(auctionContext))
.build();

return toBidsWithCacheIds(
bidderResponses,
return cacheBids(
bidsToCache,
auctionContext,
cacheInfo,
Expand All @@ -243,6 +255,35 @@ private static ExtRequestTargeting targeting(BidRequest bidRequest) {
return prebid != null ? prebid.getTargeting() : null;
}

private static List<BidInfo> toBidInfo(BidderResponse bidderResponse, List<Imp> imps) {
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()))
.collect(Collectors.toList());
}

private static BidInfo toBidInfo(Bid bid, BidType type, List<Imp> imps, String bidder) {
final Imp imp = correspondingImp(bid, imps);
return BidInfo.builder()
.bid(bid)
.bidType(type)
.bidder(bidder)
.correspondingImp(imp)
.build();
}

private static Imp correspondingImp(Bid bid, List<Imp> imps) {
return imps.stream()
.filter(imp -> bid.getImpid().equals(imp.getId()))
.findFirst()
// TODO good place to add metric on missing corresponding imp
.orElse(null);
}

/**
* Extracts auction timestamp from {@link ExtRequest} or get it from {@link Clock} if it is null.
*/
Expand Down Expand Up @@ -327,7 +368,8 @@ private static Set<Bid> newOrEmptySet(ExtRequestTargeting targeting) {
* <p>
* Winning bid is the one with the highest price.
*/
private static void populateWinningBids(List<BidderResponse> bidderResponses, Set<Bid> winningBids,
private static void populateWinningBids(List<BidderResponse> bidderResponses,
Set<Bid> winningBids,
Set<Bid> winningBidsByBidder) {
final Map<String, Bid> winningBidsMap = new HashMap<>(); // impId -> Bid
final Map<String, Map<String, Bid>> winningBidsByBidderMap = new HashMap<>(); // impId -> [bidder -> Bid]
Expand Down Expand Up @@ -384,98 +426,60 @@ private static void tryAddWinningBidByBidder(Bid bid, String bidder,
}
}

private static Stream<Bid> getBids(BidderResponse bidderResponse) {
return Stream.of(bidderResponse)
.map(BidderResponse::getSeatBid)
.filter(Objects::nonNull)
.map(BidderSeatBid::getBids)
.filter(Objects::nonNull)
.flatMap(Collection::stream)
.map(BidderBid::getBid);
}

/**
* Corresponds cacheId (or null if not present) to each {@link Bid}.
*/
private Future<CacheServiceResult> toBidsWithCacheIds(List<BidderResponse> bidderResponses,
Set<Bid> bidsToCache,
AuctionContext auctionContext,
BidRequestCacheInfo cacheInfo,
EventsContext eventsContext) {

private Future<CacheServiceResult> cacheBids(Set<BidInfo> bidsToCache,
AuctionContext auctionContext,
BidRequestCacheInfo cacheInfo,
EventsContext eventsContext) {
if (!cacheInfo.isDoCaching()) {
return Future.succeededFuture(CacheServiceResult.of(null, null, toMapBidsWithEmptyCacheIds(bidsToCache)));
}

// do not submit non deals bids with zero price to prebid cache
final List<Bid> bidsValidToBeCached = bidsToCache.stream()
final List<BidInfo> bidsValidToBeCached = bidsToCache.stream()
.filter(BidResponseCreator::isValidForCaching)
// for test consistency
.sorted(Comparator.comparing(o -> o.getBid().getId()))
.collect(Collectors.toList());

final boolean shouldCacheVideoBids = cacheInfo.isShouldCacheVideoBids();

final Map<String, List<String>> bidderToVideoBidIdsToModify =
shouldCacheVideoBids && eventsEnabledForAccount(auctionContext)
? getBidderAndVideoBidIdsToModify(bidderResponses, auctionContext.getBidRequest().getImp())
: Collections.emptyMap();
final Map<String, List<String>> bidderToBidIds = bidderResponses.stream()
.collect(Collectors.toMap(BidderResponse::getBidder, bidderResponse -> getBids(bidderResponse)
.map(Bid::getId)
.collect(Collectors.toList())));

final CacheContext cacheContext = CacheContext.builder()
.cacheBidsTtl(cacheInfo.getCacheBidsTtl())
.cacheVideoBidsTtl(cacheInfo.getCacheVideoBidsTtl())
.shouldCacheBids(cacheInfo.isShouldCacheBids())
.shouldCacheVideoBids(shouldCacheVideoBids)
.bidderToVideoBidIdsToModify(bidderToVideoBidIdsToModify)
.bidderToBidIds(bidderToBidIds)
.shouldCacheVideoBids(cacheInfo.isShouldCacheVideoBids())
.build();

return cacheService.cacheBidsOpenrtb(bidsValidToBeCached, auctionContext, cacheContext, eventsContext)
.map(cacheResult -> addNotCachedBids(cacheResult, bidsToCache));
}

private static boolean isValidForCaching(Bid bid) {
private static boolean isValidForCaching(BidInfo bidInfo) {
final Bid bid = bidInfo.getBid();
final BigDecimal price = bid.getPrice();
return bid.getDealid() != null ? price.compareTo(BigDecimal.ZERO) >= 0 : price.compareTo(BigDecimal.ZERO) > 0;
}

private Map<String, List<String>> getBidderAndVideoBidIdsToModify(List<BidderResponse> bidderResponses,
List<Imp> imps) {

return bidderResponses.stream()
.filter(bidderResponse -> bidderCatalog.isModifyingVastXmlAllowed(bidderResponse.getBidder()))
.collect(Collectors.toMap(BidderResponse::getBidder, bidderResponse -> getBids(bidderResponse)
.filter(bid -> isVideoBid(bid, imps))
.map(Bid::getId)
.collect(Collectors.toList())));
}

private static boolean isVideoBid(Bid bid, List<Imp> imps) {
return imps.stream()
.filter(imp -> imp.getVideo() != null)
.map(Imp::getId)
.anyMatch(impId -> bid.getImpid().equals(impId));
}

/**
* Creates a map with {@link Bid} as a key and null as a value.
*/
private static Map<Bid, CacheInfo> toMapBidsWithEmptyCacheIds(Set<Bid> bids) {
private static Map<Bid, CacheInfo> toMapBidsWithEmptyCacheIds(Set<BidInfo> bids) {
return bids.stream()
.map(BidInfo::getBid)
.collect(Collectors.toMap(Function.identity(), ignored -> CacheInfo.empty()));
}

/**
* Adds bids with no cache id info.
*/
private static CacheServiceResult addNotCachedBids(CacheServiceResult cacheResult, Set<Bid> bids) {
private static CacheServiceResult addNotCachedBids(CacheServiceResult cacheResult, Set<BidInfo> bidInfos) {
final Map<Bid, CacheInfo> bidToCacheId = cacheResult.getCacheBids();

if (bids.size() > bidToCacheId.size()) {
if (bidInfos.size() > bidToCacheId.size()) {
final Map<Bid, CacheInfo> updatedBidToCacheInfo = new HashMap<>(bidToCacheId);
for (Bid bid : bids) {
for (BidInfo bidInfo : bidInfos) {
final Bid bid = bidInfo.getBid();
if (!updatedBidToCacheInfo.containsKey(bid)) {
updatedBidToCacheInfo.put(bid, CacheInfo.empty());
}
Expand Down Expand Up @@ -817,15 +821,17 @@ private Bid toBid(BidderBid bidderBid,
if ((videoCacheId != null && !requestCacheInfo.isReturnCreativeVideoBids())
|| (cacheId != null && !requestCacheInfo.isReturnCreativeBids())) {
bid.setAdm(null);
} else if (bidType.equals(BidType.video)) {
final String adm = vastModifier.createBidVastXml(bid, bidder, account.getId(), eventsContext);
bid.setAdm(adm);
}

final boolean isApp = bidRequest.getApp() != null;
if (isApp && bidType.equals(BidType.xNative) && bid.getAdm() != null) {
try {
addNativeMarkup(bid, bidRequest.getImp());
} catch (PreBidException e) {
bidErrors.putIfAbsent(bidder, new ArrayList<>());
bidErrors.get(bidder)
bidErrors.computeIfAbsent(bidder, ignored -> new ArrayList<>())
.add(ExtBidderError.of(BidderError.Type.bad_server_response.getCode(), e.getMessage()));
return null;
}
Expand Down
21 changes: 21 additions & 0 deletions src/main/java/org/prebid/server/auction/model/BidInfo.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package org.prebid.server.auction.model;

import com.iab.openrtb.request.Imp;
import com.iab.openrtb.response.Bid;
import lombok.Builder;
import lombok.Value;
import org.prebid.server.proto.openrtb.ext.response.BidType;

@Builder(toBuilder = true)
@Value
public class BidInfo {

Bid bid;

// Can be null
Imp correspondingImp;

String bidder;

BidType bidType;
}
Loading

0 comments on commit e1cadf5

Please sign in to comment.