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

Jump to bottom

Fix response bid targeting keywords populating #946

Merged
merged 1 commit into from
Oct 7, 2020
Merged

Fix response bid targeting keywords populating #946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
63 changes: 37 additions & 26 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 @@ -793,8 +793,21 @@ private Bid toBid(BidderBid bidderBid,
final Bid bid = bidderBid.getBid();
final BidType bidType = bidderBid.getType();

// preliminary variables are needed because bid is changing below, so we can lost it in winning bids sets
final boolean isWinningBid = winningBids.contains(bid);
final boolean isWinningBidByBidder = winningBidsByBidder.contains(bid);

final CacheIdInfo cacheIdInfo = bidsWithCacheIds.get(bid);
final String cacheId = cacheIdInfo != null ? cacheIdInfo.getCacheId() : null;
final String videoCacheId = cacheIdInfo != null ? cacheIdInfo.getVideoCacheId() : null;

if ((videoCacheId != null && !cacheInfo.isReturnCreativeVideoBids())
|| (cacheId != null && !cacheInfo.isReturnCreativeBids())) {
bid.setAdm(null);
}

final boolean isApp = bidRequest.getApp() != null;
if (isApp && bidType.equals(BidType.xNative)) {
if (isApp && bidType.equals(BidType.xNative) && bid.getAdm() != null) {
try {
addNativeMarkup(bid, bidRequest.getImp());
} catch (PreBidException e) {
Expand All @@ -805,40 +818,27 @@ private Bid toBid(BidderBid bidderBid,
}
}

final CacheIdInfo cacheIdInfo = bidsWithCacheIds.get(bid);
final String cacheId = cacheIdInfo != null ? cacheIdInfo.getCacheId() : null;
final String videoCacheId = cacheIdInfo != null ? cacheIdInfo.getVideoCacheId() : null;

if ((videoCacheId != null && !cacheInfo.isReturnCreativeVideoBids())
|| (cacheId != null && !cacheInfo.isReturnCreativeBids())) {
bid.setAdm(null);
}

final Map<String, String> targetingKeywords;
final ExtResponseCache cache;

if (targeting != null && winningBidsByBidder.contains(bid)) {
final TargetingKeywordsCreator keywordsCreator = keywordsCreator(targeting, isApp, bidRequest, account);
final Map<BidType, TargetingKeywordsCreator> keywordsCreatorByBidType =
keywordsCreatorByBidType(targeting, isApp, bidRequest, account);
final boolean isWinningBid = winningBids.contains(bid);
targetingKeywords = keywordsCreatorByBidType.getOrDefault(bidType, keywordsCreator)
.makeFor(bid, bidder, isWinningBid, cacheId, videoCacheId);

final CacheAsset bids = cacheId != null ? toCacheAsset(cacheId) : null;
final CacheAsset vastXml = videoCacheId != null ? toCacheAsset(videoCacheId) : null;
cache = bids != null || vastXml != null ? ExtResponseCache.of(bids, vastXml) : null;
if (targeting != null && isWinningBidByBidder) {
final TargetingKeywordsCreator keywordsCreator = resolveKeywordsCreator(bidType, targeting, isApp,
bidRequest, account);

targetingKeywords = keywordsCreator.makeFor(bid, bidder, isWinningBid, cacheId, videoCacheId);
} else {
targetingKeywords = null;
cache = null;
}

final CacheAsset bids = cacheId != null ? toCacheAsset(cacheId) : null;
final CacheAsset vastXml = videoCacheId != null ? toCacheAsset(videoCacheId) : null;
final ExtResponseCache cache = bids != null || vastXml != null ? ExtResponseCache.of(bids, vastXml) : null;

final String generatedBidId = generateBidId ? UUID.randomUUID().toString() : null;
final String eventBidId = ObjectUtils.defaultIfNull(generatedBidId, bid.getId());
final Video storedVideo = impIdToStoredVideo.get(bid.getImpid());
final Events events = createEvents(bidder, account, eventBidId, eventsContext);
final ExtBidPrebidVideo extBidPrebidVideo = getExtBidPrebidVideo(bid.getExt());
final ExtBidPrebid prebidExt = ExtBidPrebid.builder()

final ExtBidPrebid extBidPrebid = ExtBidPrebid.builder()
.bidid(generatedBidId)
.type(bidType)
.targeting(targetingKeywords)
Expand All @@ -848,7 +848,7 @@ private Bid toBid(BidderBid bidderBid,
.video(extBidPrebidVideo)
.build();

final ExtPrebid<ExtBidPrebid, ObjectNode> bidExt = ExtPrebid.of(prebidExt, bid.getExt());
final ExtPrebid<ExtBidPrebid, ObjectNode> bidExt = ExtPrebid.of(extBidPrebid, bid.getExt());
bid.setExt(mapper.mapper().valueToTree(bidExt));

return bid;
Expand Down Expand Up @@ -965,6 +965,17 @@ private static boolean eventsAllowedByRequest(AuctionContext auctionContext) {
return prebid != null && prebid.getEvents() != null;
}

private TargetingKeywordsCreator resolveKeywordsCreator(BidType bidType,
ExtRequestTargeting targeting,
boolean isApp,
BidRequest bidRequest,
Account account) {
final Map<BidType, TargetingKeywordsCreator> keywordsCreatorByBidType =
keywordsCreatorByBidType(targeting, isApp, bidRequest, account);

return keywordsCreatorByBidType.getOrDefault(bidType, keywordsCreator(targeting, isApp, bidRequest, account));
}

/**
* Extracts targeting keywords settings from the bid request and creates {@link TargetingKeywordsCreator}
* instance if it is present.
Expand Down
29 changes: 29 additions & 0 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 @@ -1088,6 +1088,35 @@ public void shouldPopulateTargetingKeywordsWithAdditionalValuesFromRequest() {
.contains(tuple("static_keyword1", "static_keyword1"));
}

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

final Bid bid = Bid.builder().id("bidId").price(BigDecimal.valueOf(5.67)).impid("impId").adm("adm").build();
final List<BidderResponse> bidderResponses = singletonList(BidderResponse.of("bidder1",
givenSeatBid(BidderBid.of(bid, banner, "USD")), 100));

final BidRequestCacheInfo cacheInfo = BidRequestCacheInfo.builder()
.doCaching(true)
.returnCreativeBids(false) // this will cause erasing of bid.adm
.build();

givenCacheServiceResult(singletonMap(bid, CacheIdInfo.of("cacheId", null)));

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

// then
assertThat(bidResponse.getSeatbid())
.flatExtracting(SeatBid::getBid).hasSize(1)
.extracting(extractedBid -> toExtPrebid(extractedBid.getExt()).getPrebid().getTargeting())
.doesNotContainNull();
}

@Test
public void shouldAddExtPrebidEventsIfEventsAreEnabledAndExtRequestPrebidEventPresent() {
// given
Expand Down