Skip to content

Commit

Permalink
Bids Rejection Refactoring (prebid#3597)
Browse files Browse the repository at this point in the history
  • Loading branch information
@AntoxaAntoxic @sergseven
AntoxaAntoxic authored and sergseven committed Dec 23, 2024
1 parent d2c9e3d commit 7411595
Show file tree
Hide file tree
Showing 23 changed files with 491 additions and 272 deletions.
20 changes: 7 additions & 13 deletions ...ocking/src/main/java/org/prebid/server/hooks/modules/ortb2/blocking/core/BidsBlocker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ public ExecutionResult<BlockedBids> block() {
final List<String> warnings = MergeUtils.mergeMessages(blockedBidResults);

if (blockedBids != null) {
rejectBlockedBids(blockedBidResults);
blockedBidIndexes.forEach(index ->
rejectBlockedBid(blockedBidResults.get(index).getValue(), bids.get(index)));
}

return ExecutionResult.<BlockedBids>builder()
Expand Down Expand Up @@ -287,26 +288,19 @@ private String debugEntryFor(int index, BlockingResult blockingResult) {
blockingResult.getFailedChecks());
}

private void rejectBlockedBids(List<Result<BlockingResult>> blockedBidResults) {
blockedBidResults.stream()
.map(Result::getValue)
.filter(BlockingResult::isBlocked)
.forEach(this::rejectBlockedBid);
}

private void rejectBlockedBid(BlockingResult blockingResult) {
private void rejectBlockedBid(BlockingResult blockingResult, BidderBid blockedBid) {
if (blockingResult.getBattrCheckResult().isFailed()
|| blockingResult.getBappCheckResult().isFailed()
|| blockingResult.getBcatCheckResult().isFailed()) {

bidRejectionTracker.reject(
blockingResult.getImpId(),
bidRejectionTracker.rejectBid(
blockedBid,
BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
}

if (blockingResult.getBadvCheckResult().isFailed()) {
bidRejectionTracker.reject(
blockingResult.getImpId(),
bidRejectionTracker.rejectBid(
blockedBid,
BidRejectionReason.RESPONSE_REJECTED_ADVERTISER_BLOCKED);
}
}
Expand Down
115 changes: 64 additions & 51 deletions ...ng/src/test/java/org/prebid/server/hooks/modules/ortb2/blocking/core/BidsBlockerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,13 +197,13 @@ public void shouldReturnResultWithBidWhenBidWithBlockedAdomainAndEnforceBlocksTr
.build()));

// when
final List<BidderBid> bids = singletonList(bid(bid -> bid.adomain(singletonList("domain1.com"))));
final BidderBid bid = bid(bidBuilder -> bidBuilder.adomain(singletonList("domain1.com")));
final BlockedAttributes blockedAttributes = attributesWithBadv(singletonList("domain1.com"));
final BidsBlocker blocker = BidsBlocker.create(bids, "bidder1", ORTB_VERSION, accountConfig, blockedAttributes, bidRejectionTracker, false);
final BidsBlocker blocker = BidsBlocker.create(singletonList(bid), "bidder1", ORTB_VERSION, accountConfig, blockedAttributes, bidRejectionTracker, false);

// when and then
assertThat(blocker.block()).satisfies(result -> hasValue(result, 0));
verify(bidRejectionTracker).reject("impId1", BidRejectionReason.RESPONSE_REJECTED_ADVERTISER_BLOCKED);
verify(bidRejectionTracker).rejectBid(bid, BidRejectionReason.RESPONSE_REJECTED_ADVERTISER_BLOCKED);
}

@Test
Expand Down Expand Up @@ -304,9 +304,9 @@ public void shouldReturnEmptyResultWhenBidWithBlockedAdomainAndInDealsExceptions
.build()));

// when
final List<BidderBid> bids = singletonList(bid(bid -> bid.adomain(singletonList("domain1.com"))));
final BidderBid bid = bid(bidBuilder -> bidBuilder.adomain(singletonList("domain1.com")));
final BlockedAttributes blockedAttributes = attributesWithBadv(singletonList("domain1.com"));
final BidsBlocker blocker = BidsBlocker.create(bids, "bidder1", ORTB_VERSION, accountConfig, blockedAttributes, bidRejectionTracker, true);
final BidsBlocker blocker = BidsBlocker.create(singletonList(bid), "bidder1", ORTB_VERSION, accountConfig, blockedAttributes, bidRejectionTracker, true);

// when and then
assertThat(blocker.block()).satisfies(BidsBlockerTest::isEmpty);
Expand All @@ -324,13 +324,13 @@ public void shouldReturnResultWithBidWhenBidWithBlockedAdomainAndNotInDealsExcep
.build()));

// when
final List<BidderBid> bids = singletonList(bid(bid -> bid.adomain(singletonList("domain1.com"))));
final BidderBid bid = bid(bidBuilder -> bidBuilder.adomain(singletonList("domain1.com")));
final BlockedAttributes blockedAttributes = attributesWithBadv(singletonList("domain1.com"));
final BidsBlocker blocker = BidsBlocker.create(bids, "bidder1", ORTB_VERSION, accountConfig, blockedAttributes, bidRejectionTracker, false);
final BidsBlocker blocker = BidsBlocker.create(singletonList(bid), "bidder1", ORTB_VERSION, accountConfig, blockedAttributes, bidRejectionTracker, false);

// when and then
assertThat(blocker.block()).satisfies(result -> hasValue(result, 0));
verify(bidRejectionTracker).reject("impId1", BidRejectionReason.RESPONSE_REJECTED_ADVERTISER_BLOCKED);
verify(bidRejectionTracker).rejectBid(bid, BidRejectionReason.RESPONSE_REJECTED_ADVERTISER_BLOCKED);
}

@Test
Expand All @@ -344,16 +344,16 @@ public void shouldReturnResultWithBidAndDebugMessageWhenBidIsBlocked() {
.build()));

// when
final List<BidderBid> bids = singletonList(bid());
final BidsBlocker blocker = BidsBlocker.create(bids, "bidder1", ORTB_VERSION, accountConfig, null, bidRejectionTracker, true);
final BidderBid bid = bid();
final BidsBlocker blocker = BidsBlocker.create(singletonList(bid), "bidder1", ORTB_VERSION, accountConfig, null, bidRejectionTracker, true);

// when and then
assertThat(blocker.block()).satisfies(result -> {
assertThat(result.getValue()).isEqualTo(BlockedBids.of(singleton(0)));
assertThat(result.getDebugMessages()).containsOnly(
"Bid 0 from bidder bidder1 has been rejected, failed checks: [bcat]");
});
verify(bidRejectionTracker).reject("impId1", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
verify(bidRejectionTracker).rejectBid(bid, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
}

@Test
Expand All @@ -367,12 +367,12 @@ public void shouldReturnResultWithBidWithoutDebugMessageWhenBidIsBlockedAndDebug
.build()));

// when
final List<BidderBid> bids = singletonList(bid());
final BidsBlocker blocker = BidsBlocker.create(bids, "bidder1", ORTB_VERSION, accountConfig, null, bidRejectionTracker, false);
final BidderBid bid = bid();
final BidsBlocker blocker = BidsBlocker.create(singletonList(bid), "bidder1", ORTB_VERSION, accountConfig, null, bidRejectionTracker, false);

// when and then
assertThat(blocker.block()).satisfies(result -> hasValue(result, 0));
verify(bidRejectionTracker).reject("impId1", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
verify(bidRejectionTracker).rejectBid(bid, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
}

@Test
Expand All @@ -393,22 +393,23 @@ public void shouldReturnResultWithAnalyticsResults() {
.build())
.build()));

final BidderBid bid1 = bid(bid -> bid
.impid("impId1")
.adomain(asList("domain2.com", "domain3.com", "domain4.com"))
.bundle("app2"));
final BidderBid bid2 = bid(bid -> bid
.impid("impId2")
.cat(asList("cat2", "cat3", "cat4"))
.attr(asList(2, 3, 4)));
final BidderBid bid3 = bid(bid -> bid
.impid("impId1")
.adomain(singletonList("domain5.com"))
.cat(singletonList("cat5"))
.bundle("app5")
.attr(singletonList(5)));

// when
final List<BidderBid> bids = asList(
bid(bid -> bid
.impid("impId1")
.adomain(asList("domain2.com", "domain3.com", "domain4.com"))
.bundle("app2")),
bid(bid -> bid
.impid("impId2")
.cat(asList("cat2", "cat3", "cat4"))
.attr(asList(2, 3, 4))),
bid(bid -> bid
.impid("impId1")
.adomain(singletonList("domain5.com"))
.cat(singletonList("cat5"))
.bundle("app5")
.attr(singletonList(5))));
final List<BidderBid> bids = asList(bid1, bid2, bid3);
final BlockedAttributes blockedAttributes = BlockedAttributes.builder()
.badv(asList("domain1.com", "domain2.com", "domain3.com"))
.bcat(asList("cat1", "cat2", "cat3"))
Expand Down Expand Up @@ -436,9 +437,9 @@ public void shouldReturnResultWithAnalyticsResults() {
AnalyticsResult.of("success-allow", null, "bidder1", "impId1"));
});

verify(bidRejectionTracker).reject("impId1", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
verify(bidRejectionTracker).reject("impId2", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
verify(bidRejectionTracker).reject("impId1", BidRejectionReason.RESPONSE_REJECTED_ADVERTISER_BLOCKED);
verify(bidRejectionTracker).rejectBid(bid1, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
verify(bidRejectionTracker).rejectBid(bid2, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
verify(bidRejectionTracker).rejectBid(bid1, BidRejectionReason.RESPONSE_REJECTED_ADVERTISER_BLOCKED);
verifyNoMoreInteractions(bidRejectionTracker);
}

Expand Down Expand Up @@ -467,23 +468,30 @@ public void shouldReturnResultWithoutSomeBidsWhenAllAttributesInConfig() {
.build()));

// when
final List<BidderBid> bids = asList(
bid(bid -> bid.adomain(singletonList("domain1.com"))),
bid(bid -> bid.adomain(singletonList("domain2.com")).cat(singletonList("cat1"))),
bid(bid -> bid.adomain(singletonList("domain2.com")).cat(singletonList("cat2"))),
bid(bid -> bid.adomain(singletonList("domain2.com")).cat(singletonList("cat2")).bundle("app1")),
bid(bid -> bid.adomain(singletonList("domain2.com")).cat(singletonList("cat2")).bundle("app2")),
bid(bid -> bid
.adomain(singletonList("domain2.com"))
.cat(singletonList("cat2"))
.bundle("app2")
.attr(singletonList(1))),
bid(bid -> bid
.adomain(singletonList("domain2.com"))
.cat(singletonList("cat2"))
.bundle("app2")
.attr(singletonList(2))),
bid());
final BidderBid bid1 = bid(bid -> bid.adomain(singletonList("domain1.com")));
final BidderBid bid2 = bid(bid -> bid.adomain(singletonList("domain2.com")).cat(singletonList("cat1")));
final BidderBid bid3 = bid(bid -> bid.adomain(singletonList("domain2.com")).cat(singletonList("cat2")));
final BidderBid bid4 = bid(bid -> bid
.adomain(singletonList("domain2.com"))
.cat(singletonList("cat2"))
.bundle("app1"));
final BidderBid bid5 = bid(bid -> bid
.adomain(singletonList("domain2.com"))
.cat(singletonList("cat2"))
.bundle("app2"));
final BidderBid bid6 = bid(bid -> bid
.adomain(singletonList("domain2.com"))
.cat(singletonList("cat2"))
.bundle("app2")
.attr(singletonList(1)));
final BidderBid bid7 = bid(bid -> bid
.adomain(singletonList("domain2.com"))
.cat(singletonList("cat2"))
.bundle("app2")
.attr(singletonList(2)));
final BidderBid bid8 = bid();

final List<BidderBid> bids = asList(bid1, bid2, bid3, bid4, bid5, bid6, bid7, bid8);
final BlockedAttributes blockedAttributes = BlockedAttributes.builder()
.badv(asList("domain1.com", "domain2.com"))
.bcat(asList("cat1", "cat2"))
Expand All @@ -503,8 +511,13 @@ public void shouldReturnResultWithoutSomeBidsWhenAllAttributesInConfig() {
"Bid 7 from bidder bidder1 has been rejected, failed checks: [badv, bcat]");
});

verify(bidRejectionTracker, times(5)).reject("impId1", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
verify(bidRejectionTracker, times(2)).reject("impId1", BidRejectionReason.RESPONSE_REJECTED_ADVERTISER_BLOCKED);
verify(bidRejectionTracker).rejectBid(bid1, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
verify(bidRejectionTracker).rejectBid(bid1, BidRejectionReason.RESPONSE_REJECTED_ADVERTISER_BLOCKED);
verify(bidRejectionTracker).rejectBid(bid2, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
verify(bidRejectionTracker).rejectBid(bid4, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
verify(bidRejectionTracker).rejectBid(bid6, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
verify(bidRejectionTracker).rejectBid(bid8, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
verify(bidRejectionTracker).rejectBid(bid8, BidRejectionReason.RESPONSE_REJECTED_ADVERTISER_BLOCKED);
verifyNoMoreInteractions(bidRejectionTracker);
}

Expand Down
2 changes: 1 addition & 1 deletion ...ava/org/prebid/server/hooks/modules/pb/richmedia/filter/core/BidResponsesMraidFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public MraidFilterResult filterByPattern(String mraidScriptPattern,
analyticsResults.add(analyticsResult);

bidRejectionTrackers.get(bidder)
.reject(rejectedImps, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
.rejectBids(invalidBids, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);

final List<BidderError> errors = new ArrayList<>(seatBid.getErrors());
errors.add(BidderError.of("Invalid bid", BidderError.Type.invalid_bid, new HashSet<>(rejectedImps)));
Expand Down
25 changes: 12 additions & 13 deletions ...org/prebid/server/hooks/modules/pb/richmedia/filter/core/BidResponsesMraidFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,14 @@ public void filterShouldReturnOriginalBidsWhenNoBidsHaveMraidScriptInAdm() {
@Test
public void filterShouldReturnFilteredBidsWhenBidsWithMraidScriptIsFilteredOut() {
// given
final BidderResponse responseA = givenBidderResponse("bidderA", List.of(
givenBid("imp_id1", "adm1"),
givenBid("imp_id2", "adm2")));
final BidderResponse responseB = givenBidderResponse("bidderB", List.of(
givenBid("imp_id1", "adm1"),
givenBid("imp_id2", "adm2_mraid.js")));
final BidderResponse responseC = givenBidderResponse("bidderC", List.of(
givenBid("imp_id1", "adm1_mraid.js"),
givenBid("imp_id2", "adm2_mraid.js")));
final BidderBid givenBid1 = givenBid("imp_id1", "adm1");
final BidderBid givenBid2 = givenBid("imp_id2", "adm2");
final BidderBid givenInvalidBid1 = givenBid("imp_id1", "adm1_mraid.js");
final BidderBid givenInvalidBid2 = givenBid("imp_id2", "adm2_mraid.js");

final BidderResponse responseA = givenBidderResponse("bidderA", List.of(givenBid1, givenBid2));
final BidderResponse responseB = givenBidderResponse("bidderB", List.of(givenBid1, givenInvalidBid2));
final BidderResponse responseC = givenBidderResponse("bidderC", List.of(givenInvalidBid1, givenInvalidBid2));

final BidRejectionTracker bidRejectionTrackerA = mock(BidRejectionTracker.class);
final BidRejectionTracker bidRejectionTrackerB = mock(BidRejectionTracker.class);
Expand All @@ -77,10 +76,10 @@ public void filterShouldReturnFilteredBidsWhenBidsWithMraidScriptIsFilteredOut()
// then
final BidderResponse expectedResponseA = givenBidderResponse(
"bidderA",
List.of(givenBid("imp_id1", "adm1"), givenBid("imp_id2", "adm2")));
List.of(givenBid1, givenBid2));
final BidderResponse expectedResponseB = givenBidderResponse(
"bidderB",
List.of(givenBid("imp_id1", "adm1")),
List.of(givenBid1),
List.of(givenError("imp_id2")));
final BidderResponse expectedResponseC = givenBidderResponse(
"bidderC",
Expand All @@ -106,9 +105,9 @@ public void filterShouldReturnFilteredBidsWhenBidsWithMraidScriptIsFilteredOut()

verifyNoInteractions(bidRejectionTrackerA);
verify(bidRejectionTrackerB)
.reject(List.of("imp_id2"), BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
.rejectBids(List.of(givenInvalidBid2), BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
verify(bidRejectionTrackerC)
.reject(List.of("imp_id1", "imp_id2"), BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
.rejectBids(List.of(givenInvalidBid1, givenInvalidBid2), BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE);
verifyNoMoreInteractions(bidRejectionTrackerB, bidRejectionTrackerC);
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/prebid/server/analytics/reporter/greenbids/GreenbidsAnalyticsReporter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ private static Map<String, NonBid> getSeatsWithNonBids(AuctionContext auctionCon
}

private static SeatNonBid toSeatNonBid(String bidder, BidRejectionTracker bidRejectionTracker) {
final List<NonBid> nonBids = bidRejectionTracker.getRejectionReasons().entrySet().stream()
final List<NonBid> nonBids = bidRejectionTracker.getRejectedImps().entrySet().stream()
.map(entry -> NonBid.of(entry.getKey(), entry.getValue()))
.toList();

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/prebid/server/auction/BidResponseCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1758,7 +1758,7 @@ private static BidResponse populateSeatNonBid(AuctionContext auctionContext, Bid
}

private static SeatNonBid toSeatNonBid(String bidder, BidRejectionTracker bidRejectionTracker) {
final List<NonBid> nonBid = bidRejectionTracker.getRejectionReasons().entrySet().stream()
final List<NonBid> nonBid = bidRejectionTracker.getRejectedImps().entrySet().stream()
.map(entry -> NonBid.of(entry.getKey(), entry.getValue()))
.toList();

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/prebid/server/auction/DsaEnforcer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public AuctionParticipation enforce(BidRequest bidRequest,
}
} catch (PreBidException e) {
warnings.add(BidderError.invalidBid("Bid \"%s\": %s".formatted(bid.getId(), e.getMessage())));
rejectionTracker.reject(bid.getImpid(), BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY);
rejectionTracker.rejectBid(bidderBid, BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY);
updatedBidderBids.remove(bidderBid);
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/prebid/server/auction/ExchangeService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ private AuctionParticipation createAuctionParticipation(
if (blockedRequestByTcf) {
context.getBidRejectionTrackers()
.get(bidder)
.rejectAll(BidRejectionReason.REQUEST_BLOCKED_PRIVACY);
.rejectAllImps(BidRejectionReason.REQUEST_BLOCKED_PRIVACY);

return AuctionParticipation.builder()
.bidder(bidder)
Expand Down Expand Up @@ -1154,7 +1154,7 @@ private static Future<BidderResponse> processReject(AuctionContext auctionContex

auctionContext.getBidRejectionTrackers()
.get(bidderName)
.rejectAll(bidRejectionReason);
.rejectAllImps(bidRejectionReason);
final BidderSeatBid bidderSeatBid = BidderSeatBid.builder()
.warnings(warnings)
.build();
Expand Down Expand Up @@ -1193,7 +1193,7 @@ private Future<BidderResponse> requestBidsOrRejectBidder(
if (hookStageResult.isShouldReject()) {
auctionContext.getBidRejectionTrackers()
.get(bidderRequest.getBidder())
.rejectAll(BidRejectionReason.REQUEST_BLOCKED_GENERAL);
.rejectAllImps(BidRejectionReason.REQUEST_BLOCKED_GENERAL);

return Future.succeededFuture(BidderResponse.of(bidderRequest.getBidder(), BidderSeatBid.empty(), 0));
}
Expand Down
Loading

0 comments on commit 7411595

Please sign in to comment.