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.

Sign up for GitHub

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

Fix event generation for vast #932

Merged
merged 2 commits into from
Sep 29, 2020
Merged

Fix event generation for vast #932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
08fc46c
Do not generate vast tracking URL if events are not enabled for account
Sep 28, 2020
9935b30
Split events configuration flag into two - per account and per request
Sep 29, 2020
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
35 changes: 18 additions & 17 deletions src/main/java/org/prebid/server/auction/BidResponseCreator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,8 @@ private Future<BidResponse> cacheBidsAndCreateResponse(List<BidderResponse> bidd
: bidderResponses.stream().flatMap(BidResponseCreator::getBids).collect(Collectors.toSet());

final EventsContext eventsContext = EventsContext.builder()
.enabledForAccountAndRequest(eventsEnabledForAccountAndRequest(auctionContext))
.enabledForAccount(eventsEnabledForAccount(auctionContext))
.enabledForRequest(eventsEnabledForRequest(auctionContext))
.auctionTimestamp(auctionTimestamp)
.integration(integrationFrom(auctionContext))
.build();
Expand Down Expand Up @@ -405,11 +406,11 @@ private Future<CacheServiceResult> toBidsWithCacheIds(List<BidderResponse> bidde
.collect(Collectors.toList());

final boolean shouldCacheVideoBids = cacheInfo.isShouldCacheVideoBids();
final boolean eventsEnabled = BooleanUtils.isTrue(auctionContext.getAccount().getEventsEnabled());

final Map<String, List<String>> bidderToVideoBidIdsToModify = shouldCacheVideoBids && eventsEnabled
? getBidderAndVideoBidIdsToModify(bidderResponses, auctionContext.getBidRequest().getImp())
: Collections.emptyMap();
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)
Expand Down Expand Up @@ -915,7 +916,7 @@ private Events createEvents(String bidder,
String eventBidId,
EventsContext eventsContext) {

return eventsContext.isEnabledForAccountAndRequest()
return eventsContext.isEnabledForAccount() && eventsContext.isEnabledForRequest()
? eventsService.createEvent(
eventBidId,
bidder,
Expand All @@ -925,20 +926,20 @@ private Events createEvents(String bidder,
: null;
}

private static boolean eventsEnabledForAccountAndRequest(AuctionContext auctionContext) {
final Account account = auctionContext.getAccount();
final BidRequest bidRequest = auctionContext.getBidRequest();
private static boolean eventsEnabledForAccount(AuctionContext auctionContext) {
return BooleanUtils.isTrue(auctionContext.getAccount().getEventsEnabled());
}

final boolean eventsEnabledForAccount = BooleanUtils.isTrue(account.getEventsEnabled());
return eventsEnabledForAccount
&& (eventsEnabledForChannel(account, bidRequest) || eventsAllowedByRequest(bidRequest));
private static boolean eventsEnabledForRequest(AuctionContext auctionContext) {
return eventsEnabledForChannel(auctionContext) || eventsAllowedByRequest(auctionContext);
}

private static boolean eventsEnabledForChannel(Account account, BidRequest bidRequest) {
final AccountAnalyticsConfig analyticsConfig = account.getAnalyticsConfig();
private static boolean eventsEnabledForChannel(AuctionContext auctionContext) {
final AccountAnalyticsConfig analyticsConfig = auctionContext.getAccount().getAnalyticsConfig();
final Map<String, Boolean> channelConfig = analyticsConfig != null ? analyticsConfig.getAuctionEvents() : null;

return channelConfig != null && BooleanUtils.toBoolean(channelConfig.get(channelFromRequest(bidRequest)));
return channelConfig != null
&& BooleanUtils.toBoolean(channelConfig.get(channelFromRequest(auctionContext.getBidRequest())));
}

private static String channelFromRequest(BidRequest bidRequest) {
Expand All @@ -949,8 +950,8 @@ private static String channelFromRequest(BidRequest bidRequest) {
return channel != null ? channel.getName() : null;
}

private static boolean eventsAllowedByRequest(BidRequest bidRequest) {
final ExtRequest requestExt = bidRequest.getExt();
private static boolean eventsAllowedByRequest(AuctionContext auctionContext) {
final ExtRequest requestExt = auctionContext.getBidRequest().getExt();
final ExtRequestPrebid prebid = requestExt != null ? requestExt.getPrebid() : null;

return prebid != null && prebid.getEvents() != null;
Expand Down
26 changes: 15 additions & 11 deletions src/main/java/org/prebid/server/cache/CacheService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@
import org.prebid.server.auction.model.AuctionContext;
import org.prebid.server.cache.model.CacheBid;
import org.prebid.server.cache.model.CacheContext;
import org.prebid.server.cache.model.DebugHttpCall;
import org.prebid.server.cache.model.CacheHttpRequest;
import org.prebid.server.cache.model.CacheHttpResponse;
import org.prebid.server.cache.model.CacheIdInfo;
import org.prebid.server.cache.model.CacheServiceResult;
import org.prebid.server.cache.model.CacheTtl;
import org.prebid.server.cache.model.DebugHttpCall;
import org.prebid.server.cache.proto.BidCacheResult;
import org.prebid.server.cache.proto.request.BannerValue;
import org.prebid.server.cache.proto.request.BidCacheRequest;
Expand Down Expand Up @@ -505,7 +505,7 @@ private String generateWinUrl(Map<String, List<String>> biddersToCacheBidIds,
Account account,
EventsContext eventsContext) {

if (eventsContext.isEnabledForAccountAndRequest()) {
if (eventsContext.isEnabledForAccount() && eventsContext.isEnabledForRequest()) {
final String bidId = bid.getId();
return findBidderForBidId(biddersToCacheBidIds, bidId)
.map(bidder -> eventsService.winUrl(
Expand All @@ -525,15 +525,19 @@ private String generateVastUrlTracking(Map<String, List<String>> bidderToVideoBi
Account account,
EventsContext eventsContext) {

final String bidId = bid.getId();
return findBidderForBidId(bidderToVideoBidIdsToModify, bidId)
.map(bidder -> eventsService.vastUrlTracking(
bidId,
bidder,
account.getId(),
eventsContext.getAuctionTimestamp(),
eventsContext.getIntegration()))
.orElse(null);
if (eventsContext.isEnabledForAccount() && eventsContext.isEnabledForRequest()) {
final String bidId = bid.getId();
return findBidderForBidId(bidderToVideoBidIdsToModify, bidId)
.map(bidder -> eventsService.vastUrlTracking(
bidId,
bidder,
account.getId(),
eventsContext.getAuctionTimestamp(),
eventsContext.getIntegration()))
.orElse(null);
}

return null;
}

private static Optional<String> findBidderForBidId(Map<String, List<String>> biddersToCacheBidIds, String bidId) {
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/org/prebid/server/events/EventsContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
@Value
public class EventsContext {

boolean enabledForAccountAndRequest;
boolean enabledForAccount;

boolean enabledForRequest;

Long auctionTimestamp;

Expand Down
12 changes: 10 additions & 2 deletions src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,11 @@ public void shouldRequestCacheServiceWithExpectedArguments() {
.bidderToVideoBidIdsToModify(emptyMap())
.bidderToBidIds(biddersToCacheBidIds)
.build()),
eq(EventsContext.builder().enabledForAccountAndRequest(true).auctionTimestamp(1000L).build()));
eq(EventsContext.builder()
.enabledForAccount(true)
.enabledForRequest(true)
.auctionTimestamp(1000L)
.build()));
}

@Test
Expand Down Expand Up @@ -314,7 +318,11 @@ public void shouldRequestCacheServiceWithVideoBidsToModifyWhenEventsEnabledAndFo
.bidderToVideoBidIdsToModify(singletonMap("bidder1", singletonList("bidId1")))
.bidderToBidIds(biddersToCacheBidIds)
.build()),
eq(EventsContext.builder().enabledForAccountAndRequest(true).auctionTimestamp(1000L).build()));
eq(EventsContext.builder()
.enabledForAccount(true)
.enabledForRequest(true)
.auctionTimestamp(1000L)
.build()));
}

@Test
Expand Down
44 changes: 41 additions & 3 deletions src/test/java/org/prebid/server/cache/CacheServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ public void cacheBidsOpenrtbShouldStoreWinUrl() {
.shouldCacheBids(true)
.bidderToBidIds(singletonMap("bidder", singletonList("bidId1")))
.build(),
EventsContext.builder().enabledForAccountAndRequest(true).build());
EventsContext.builder().enabledForAccount(true).enabledForRequest(true).build());

// then
verify(eventsService).winUrl(eq("bidId1"), eq("bidder"), eq("accountId"), isNull(), isNull());
Expand Down Expand Up @@ -979,7 +979,7 @@ public void cacheBidsOpenrtbShouldAddTrackingLinkToImpTagWhenItIsEmpty() throws
.bidderToVideoBidIdsToModify(singletonMap("bidder", singletonList("bid1")))
.bidderToBidIds(singletonMap("bidder", singletonList("bid1")))
.build(),
eventsContext);
EventsContext.builder().enabledForAccount(true).enabledForRequest(true).build());

// then
final BidCacheRequest bidCacheRequest = captureBidCacheRequest();
Expand Down Expand Up @@ -1021,7 +1021,7 @@ public void cacheBidsOpenrtbShouldAddTrackingImpToBidAdmXmlWhenThatBidShouldBeMo
.bidderToVideoBidIdsToModify(singletonMap("bidder", singletonList("bid1")))
.bidderToBidIds(singletonMap("bidder", singletonList("bid1")))
.build(),
eventsContext);
EventsContext.builder().enabledForAccount(true).enabledForRequest(true).build());

// then
final BidCacheRequest bidCacheRequest = captureBidCacheRequest();
Expand All @@ -1039,6 +1039,44 @@ public void cacheBidsOpenrtbShouldAddTrackingImpToBidAdmXmlWhenThatBidShouldBeMo
.build());
}

@Test
public void cacheBidsOpenrtbShouldNotAddTrackingImpWhenEventsNotEnabled() throws IOException {
// given
final com.iab.openrtb.response.Bid bid = givenBidOpenrtb(builder -> builder
.id("bid1")
.impid("impId1")
.adm("<Impression>http:/test.com</Impression>"));
final Imp imp1 = givenImp(builder -> builder
.id("impId1")
.video(Video.builder().build()));

// when
cacheService.cacheBidsOpenrtb(
singletonList(bid),
givenAuctionContext(bidRequestBuilder -> bidRequestBuilder.imp(singletonList(imp1))),
CacheContext.builder()
.shouldCacheBids(true)
.shouldCacheVideoBids(true)
.bidderToVideoBidIdsToModify(singletonMap("bidder", singletonList("bid1")))
.bidderToBidIds(singletonMap("bidder", singletonList("bid1")))
.build(),
EventsContext.builder().enabledForAccount(false).build());

// then
final BidCacheRequest bidCacheRequest = captureBidCacheRequest();
assertThat(bidCacheRequest.getPuts()).hasSize(2)
.containsOnly(
PutObject.builder()
.type("json")
.value(mapper.valueToTree(bid))
.build(),
PutObject.builder()
.type("xml")
.value(new TextNode("<Impression>http:/test.com</Impression>"))
.build());
verifyZeroInteractions(eventsService);
}

@Test
public void cachePutObjectsShouldTolerateGlobalTimeoutAlreadyExpired() {
// when
Expand Down