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

Bids Rejection Refactoring #3597

Merged
merged 1 commit into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
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
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
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
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
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
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
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
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
Loading