Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add impression tracking to VAST (Server-Side) #437

Merged
merged 7 commits into from
Sep 5, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 48 additions & 14 deletions src/main/java/org/prebid/server/auction/ExchangeService.java
Original file line number Diff line number Diff line change
Expand Up @@ -862,25 +862,21 @@ private static MetricName bidderErrorTypeToMetric(BidderError.Type errorType) {
private Future<BidResponse> toBidResponse(List<BidderResponse> bidderResponses, BidRequest bidRequest,
ExtRequestTargeting targeting, BidRequestCacheInfo cacheInfo,
Account account, Timeout timeout, boolean debugEnabled) {
final Set<Bid> bids = bidderResponses.stream()
.map(BidderResponse::getSeatBid)
.filter(Objects::nonNull)
.map(BidderSeatBid::getBids)
.filter(Objects::nonNull)
.flatMap(Collection::stream)
.map(BidderBid::getBid)
.collect(Collectors.toSet());

return toBidsWithCacheIds(bids, bidRequest.getImp(), cacheInfo, account, timeout)
return toBidsWithCacheIds(bidderResponses, bidRequest.getImp(), cacheInfo, account, timeout)
.map(cacheResult -> bidResponseCreator.create(bidderResponses, bidRequest, targeting, cacheInfo,
cacheResult, account, debugEnabled));
}

/**
* Corresponds cacheId (or null if not present) to each {@link Bid}.
*/
private Future<CacheServiceResult> toBidsWithCacheIds(Set<Bid> bids, List<Imp> imps, BidRequestCacheInfo cacheInfo,
Account account, Timeout timeout) {
private Future<CacheServiceResult> toBidsWithCacheIds(List<BidderResponse> bidderResponses, List<Imp> imps,
BidRequestCacheInfo cacheInfo, Account account,
Timeout timeout) {
final Set<Bid> bids = bidderResponses.stream()
.flatMap(ExchangeService::getBids)
.collect(Collectors.toSet());

final Future<CacheServiceResult> result;

if (!cacheInfo.isDoCaching()) {
Expand All @@ -891,8 +887,20 @@ private Future<CacheServiceResult> toBidsWithCacheIds(Set<Bid> bids, List<Imp> i
.filter(bid -> bid.getPrice().compareTo(BigDecimal.ZERO) > 0)
.collect(Collectors.toList());

final CacheContext cacheContext = CacheContext.of(cacheInfo.isShouldCacheBids(),
cacheInfo.getCacheBidsTtl(), cacheInfo.isShouldCacheVideoBids(), cacheInfo.getCacheVideoBidsTtl());
final boolean shouldCacheVideoBids = cacheInfo.isShouldCacheVideoBids();
final boolean eventsEnabled = Objects.equals(account.getEventsEnabled(), true);

final List<String> videoBidIdsToModify = shouldCacheVideoBids && eventsEnabled
? getVideoBidIdsToModify(bidderResponses, imps)
: Collections.emptyList();

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

result = cacheService.cacheBidsOpenrtb(bidsWithNonZeroPrice, imps, cacheContext, account, timeout)
.map(cacheResult -> addNotCachedBids(cacheResult, bids));
Expand All @@ -901,6 +909,32 @@ private Future<CacheServiceResult> toBidsWithCacheIds(Set<Bid> bids, List<Imp> i
return result;
}

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);
}

private List<String> getVideoBidIdsToModify(List<BidderResponse> bidderResponses, List<Imp> imps) {
return bidderResponses.stream()
.filter(bidderResponse -> bidderCatalog.isModifyingVastXmlAllowed(bidderResponse.getBidder()))
.flatMap(ExchangeService::getBids)
.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.
*/
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/org/prebid/server/bidder/BidderCatalog.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,13 @@ public boolean isValidName(String name) {
return bidderDepsMap.containsKey(name);
}

/**
* Tells if given bidder allows to modify video's Vast XML.
*/
public boolean isModifyingVastXmlAllowed(String name) {
return bidderDepsMap.containsKey(name) && bidderDepsMap.get(name).getBidderInfo().isModifyingVastXmlAllowed();
}

/**
* Tells if given name corresponds to any of the registered deprecated bidder's name.
*/
Expand Down
62 changes: 49 additions & 13 deletions src/main/java/org/prebid/server/cache/CacheService.java
Original file line number Diff line number Diff line change
Expand Up @@ -152,20 +152,23 @@ public Future<CacheServiceResult> cacheBidsOpenrtb(List<com.iab.openrtb.response
final Map<String, Integer> impIdToTtl = new HashMap<>(imps.size());
boolean impWithNoExpExists = false; // indicates at least one impression without expire presents
final List<String> videoImpIds = new ArrayList<>();
final boolean shouldCacheVideoBids = cacheContext.isShouldCacheVideoBids();
for (Imp imp : imps) {
impIdToTtl.put(imp.getId(), imp.getExp());
final String impId = imp.getId();
impIdToTtl.put(impId, imp.getExp());
impWithNoExpExists |= imp.getExp() == null;
if (cacheContext.isShouldCacheVideoBids() && imp.getId() != null && imp.getVideo() != null) {
videoImpIds.add(imp.getId());
if (shouldCacheVideoBids && impId != null && imp.getVideo() != null) {
videoImpIds.add(impId);
}
}

final List<CacheBid> cacheBids = getCacheBids(cacheContext.isShouldCacheBids(), bids, impIdToTtl,
impWithNoExpExists, cacheContext.getCacheBidsTtl(), account);
final List<CacheBid> videoCacheBids = getVideoCacheBids(cacheContext.isShouldCacheVideoBids(), bids,
final List<CacheBid> videoCacheBids = getVideoCacheBids(shouldCacheVideoBids, bids,
impIdToTtl, videoImpIds, impWithNoExpExists, cacheContext.getCacheVideoBidsTtl(), account);

result = doCacheOpenrtb(cacheBids, videoCacheBids, timeout);
result = doCacheOpenrtb(cacheBids, videoCacheBids, cacheContext.getVideoBidIdsToModify(), account.getId(),
timeout);
}

return result;
Expand Down Expand Up @@ -238,10 +241,12 @@ private CacheBid toCacheBid(com.iab.openrtb.response.Bid bid, Map<String, Intege
* <p>
* The returned result will always have the number of elements equals to sum of sizes of bids and video bids.
*/
private Future<CacheServiceResult> doCacheOpenrtb(List<CacheBid> bids, List<CacheBid> videoBids, Timeout timeout) {
private Future<CacheServiceResult> doCacheOpenrtb(
List<CacheBid> bids, List<CacheBid> videoBids, List<String> videoBidIdsToModify, String accountId,
Timeout timeout) {
final List<PutObject> putObjects = Stream.concat(
bids.stream().map(CacheService::createJsonPutObjectOpenrtb),
videoBids.stream().map(CacheService::createXmlPutObjectOpenrtb))
videoBids.stream().map(cacheBid -> createXmlPutObjectOpenrtb(cacheBid, videoBidIdsToModify, accountId)))
.collect(Collectors.toList());

if (putObjects.isEmpty()) {
Expand Down Expand Up @@ -326,16 +331,47 @@ private static PutObject createJsonPutObjectOpenrtb(CacheBid cacheBid) {
/**
* Makes XML type {@link PutObject} from {@link com.iab.openrtb.response.Bid}. Used for OpenRTB auction request.
*/
private static PutObject createXmlPutObjectOpenrtb(CacheBid cacheBid) {
if (cacheBid.getBid().getAdm() == null) {
return PutObject.of("xml", new TextNode("<VAST version=\"3.0\"><Ad><Wrapper>"
private static PutObject createXmlPutObjectOpenrtb(CacheBid cacheBid, List<String> videoBidIdsToModify,
String accountId) {
final com.iab.openrtb.response.Bid bid = cacheBid.getBid();
String vastXml;
if (bid.getAdm() == null) {
vastXml = "<VAST version=\"3.0\"><Ad><Wrapper>"
+ "<AdSystem>prebid.org wrapper</AdSystem>"
+ "<VASTAdTagURI><![CDATA[" + cacheBid.getBid().getNurl() + "]]></VASTAdTagURI>"
+ "<VASTAdTagURI><![CDATA[" + bid.getNurl() + "]]></VASTAdTagURI>"
+ "<Impression></Impression><Creatives></Creatives>"
+ "</Wrapper></Ad></VAST>"), cacheBid.getTtl());
+ "</Wrapper></Ad></VAST>";
} else {
return PutObject.of("xml", new TextNode(cacheBid.getBid().getAdm()), cacheBid.getTtl());
vastXml = bid.getAdm();
}

final String bidId = bid.getId();
if (CollectionUtils.isNotEmpty(videoBidIdsToModify) && videoBidIdsToModify.contains(bidId)) {
vastXml = modifyVastXml(vastXml, bidId, accountId);
}

return PutObject.of("xml", new TextNode(vastXml), cacheBid.getTtl());
}

private static String modifyVastXml(String stringValue, String bidId, String accountId) {
final String closeTag = "</Impression>";
final int closeTagIndex = stringValue.indexOf(closeTag);

// no impression tag - pass it as it is
if (closeTagIndex == -1) {
return stringValue;
}

final String impressionUrl = String.format("https://prebid-server.rubiconproject.com/event?t=imp&b=%s&f=b&a=%s",
rpanchyk marked this conversation as resolved.
Show resolved Hide resolved
bidId, accountId);
final String openTag = "<Impression>";

// empty impression tag - just insert the link
if (closeTagIndex - stringValue.indexOf(openTag) == openTag.length()) {
return stringValue.replaceFirst(openTag, openTag + impressionUrl);
}

return stringValue.replaceFirst(closeTag, closeTag + "\n" + openTag + impressionUrl + closeTag);
rpanchyk marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
8 changes: 6 additions & 2 deletions src/main/java/org/prebid/server/cache/model/CacheContext.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package org.prebid.server.cache.model;

import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Value;

import java.util.List;

/**
* Holds the state needed to perform caching response bids.
*/
@AllArgsConstructor(staticName = "of")
@Builder
@Value
public class CacheContext {

Expand All @@ -17,4 +19,6 @@ public class CacheContext {
boolean shouldCacheVideoBids;

Integer cacheVideoBidsTtl;

List<String> videoBidIdsToModify;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,17 @@ public class BidderInfo {

GdprInfo gdpr;

boolean modifyingVastXmlAllowed;

public static BidderInfo create(boolean enabled, String maintainerEmail, List<String> appMediaTypes,
List<String> siteMediaTypes, List<String> supportedVendors,
int vendorId, boolean enforceGdpr) {
List<String> siteMediaTypes, List<String> supportedVendors, int vendorId,
boolean enforceGdpr, boolean modifyingVastXmlAllowed) {
final MaintainerInfo maintainer = new MaintainerInfo(maintainerEmail);
final CapabilitiesInfo capabilities = new CapabilitiesInfo(platformInfo(appMediaTypes),
platformInfo(siteMediaTypes));
final GdprInfo gdpr = new GdprInfo(vendorId, enforceGdpr);

return new BidderInfo(enabled, maintainer, capabilities, supportedVendors, gdpr);
return new BidderInfo(enabled, maintainer, capabilities, supportedVendors, gdpr, modifyingVastXmlAllowed);
}

private static PlatformInfo platformInfo(List<String> mediaTypes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ public class BidderConfigurationProperties {
@NotNull
private MetaInfo metaInfo;

private boolean modifyingVastXmlAllowed;
rpanchyk marked this conversation as resolved.
Show resolved Hide resolved

@NotNull
private UsersyncConfigurationProperties usersync;
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public static BidderInfo create(BidderConfigurationProperties configurationPrope
metaInfo.getSiteMediaTypes(),
metaInfo.getSupportedVendors(),
metaInfo.getVendorId(),
configurationProperties.getPbsEnforcesGdpr());
configurationProperties.getPbsEnforcesGdpr(),
configurationProperties.isModifyingVastXmlAllowed());
}
}
42 changes: 38 additions & 4 deletions src/test/java/org/prebid/server/auction/ExchangeServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.iab.openrtb.request.Regs;
import com.iab.openrtb.request.Site;
import com.iab.openrtb.request.User;
import com.iab.openrtb.request.Video;
import com.iab.openrtb.response.Bid;
import com.iab.openrtb.response.BidResponse;
import com.iab.openrtb.response.SeatBid;
Expand Down Expand Up @@ -62,6 +63,7 @@
import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid;
import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidCache;
import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidCacheBids;
import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidCacheVastxml;
import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebidData;
import org.prebid.server.proto.openrtb.ext.request.ExtRequestTargeting;
import org.prebid.server.proto.openrtb.ext.request.ExtSite;
Expand Down Expand Up @@ -1033,7 +1035,6 @@ public void shouldRequestCacheServiceWithExpectedArguments() {
givenBidder("bidder1", mock(Bidder.class), givenSeatBid(singletonList(givenBid(bid1))));
givenBidder("bidder2", mock(Bidder.class), givenSeatBid(singletonList(givenBid(bid2))));

// imp ids are not really used for matching, included them here for clarity
final Imp imp1 = givenImp(singletonMap("bidder1", 1), builder -> builder.id("impId1"));
final Imp imp2 = givenImp(singletonMap("bidder2", 2), builder -> builder.id("impId2"));
final BidRequest bidRequest = givenBidRequest(asList(imp1, imp2),
Expand All @@ -1048,10 +1049,43 @@ public void shouldRequestCacheServiceWithExpectedArguments() {
// then
verify(cacheService).cacheBidsOpenrtb(
argThat(t -> t.containsAll(asList(bid1, bid2))), eq(asList(imp1, imp2)),
eq(CacheContext.of(true, null, false, null)),
eq(CacheContext.builder().shouldCacheBids(true).videoBidIdsToModify(emptyList()).build()),
eq(Account.builder().id("accountId").eventsEnabled(false).build()), eq(timeout));
}

@Test
public void shouldRequestCacheServiceWithVideoBidsToModifyWhenEventsEnabledAndForBidderThatAllowsModifyVastXml() {
// given
final Bid bid1 = Bid.builder().id("bidId1").impid("impId1").price(BigDecimal.valueOf(5.67)).build();
final Bid bid2 = Bid.builder().id("bidId2").impid("impId2").price(BigDecimal.valueOf(7.19)).build();
givenBidder("bidder1", mock(Bidder.class), givenSeatBid(singletonList(givenBid(bid1))));
givenBidder("bidder2", mock(Bidder.class), givenSeatBid(singletonList(givenBid(bid2))));

given(bidderCatalog.isModifyingVastXmlAllowed(eq("bidder1"))).willReturn(true);

final Imp imp1 = givenImp(singletonMap("bidder1", 1),
builder -> builder.id("impId1").video(Video.builder().build()));
final Imp imp2 = givenImp(singletonMap("bidder2", 2),
builder -> builder.id("impId2").video(Video.builder().build()));
final BidRequest bidRequest = givenBidRequest(asList(imp1, imp2),
builder -> builder.ext(mapper.valueToTree(ExtBidRequest.of(ExtRequestPrebid.builder()
.targeting(givenTargeting())
.cache(ExtRequestPrebidCache.of(ExtRequestPrebidCacheBids.of(null, null),
ExtRequestPrebidCacheVastxml.of(null, true)))
.build()))));

// when
exchangeService.holdAuction(givenRequestContext(bidRequest,
Account.builder().id("accountId").eventsEnabled(true).build()));

// then
verify(cacheService).cacheBidsOpenrtb(
argThat(t -> t.containsAll(asList(bid1, bid2))), eq(asList(imp1, imp2)),
eq(CacheContext.builder().shouldCacheBids(true).shouldCacheVideoBids(true)
.videoBidIdsToModify(singletonList("bidId1")).build()),
eq(Account.builder().id("accountId").eventsEnabled(true).build()), eq(timeout));
}

@Test
public void shouldCallCacheServiceEvenRoundedCpmIsZero() {
// given
Expand All @@ -1071,7 +1105,7 @@ public void shouldCallCacheServiceEvenRoundedCpmIsZero() {

// then
verify(cacheService).cacheBidsOpenrtb(argThat(bids -> bids.contains(bid1)), eq(singletonList(imp1)),
eq(CacheContext.of(true, null, false, null)),
eq(CacheContext.builder().shouldCacheBids(true).videoBidIdsToModify(emptyList()).build()),
eq(Account.builder().id("accountId").eventsEnabled(false).build()), eq(timeout));
}

Expand Down Expand Up @@ -1549,7 +1583,7 @@ private static <K, V> Map<K, V> doubleMap(K key1, V value1, K key2, V value2) {
}

private static BidderInfo givenBidderInfo(int gdprVendorId, boolean enforceGdpr) {
return new BidderInfo(true, null, null, null, new BidderInfo.GdprInfo(gdprVendorId, enforceGdpr));
return new BidderInfo(true, null, null, null, new BidderInfo.GdprInfo(gdprVendorId, enforceGdpr), false);
}

private static ExtRequestTargeting givenTargeting() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,7 @@ private static <T> Imp givenImp(T ext, Function<Imp.ImpBuilder, Imp.ImpBuilder>
}

private static BidderInfo givenBidderInfo(int gdprVendorId, boolean enforceGdpr) {
return new BidderInfo(true, null, null, null, new BidderInfo.GdprInfo(gdprVendorId, enforceGdpr));
return new BidderInfo(true, null, null, null,
new BidderInfo.GdprInfo(gdprVendorId, enforceGdpr), false);
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public void nameByAliasShouldReturnBidderName() {
public void metaInfoByNameShouldReturnMetaInfoForKnownBidder() {
// given
final BidderInfo bidderInfo = BidderInfo.create(true, "test@email.com",
singletonList("banner"), singletonList("video"), null, 99, true);
singletonList("banner"), singletonList("video"), null, 99, true, false);

bidderDeps = BidderDeps.builder()
.name(BIDDER)
Expand Down
Loading