From f71b6c45d34fd840d96914588c0232dc4d89da1c Mon Sep 17 00:00:00 2001 From: antonbabak Date: Fri, 6 Dec 2024 13:39:04 +0100 Subject: [PATCH] Bids Rejection Refactoring --- .../ortb2/blocking/core/BidsBlocker.java | 20 +- .../ortb2/blocking/core/BidsBlockerTest.java | 115 +++++----- .../filter/core/BidResponsesMraidFilter.java | 2 +- .../core/BidResponsesMraidFilterTest.java | 25 ++- .../greenbids/GreenbidsAnalyticsReporter.java | 2 +- .../server/auction/BidResponseCreator.java | 2 +- .../prebid/server/auction/DsaEnforcer.java | 2 +- .../server/auction/ExchangeService.java | 6 +- .../auction/model/BidRejectionReason.java | 9 +- .../auction/model/BidRejectionTracker.java | 136 +++++++++--- .../server/bidder/HttpBidderRequester.java | 10 +- .../floors/BasicPriceFloorEnforcer.java | 2 +- .../validation/ResponseBidValidator.java | 45 ++-- .../auction/BidRejectionReason.groovy | 1 + .../functional/tests/SeatNonBidSpec.groovy | 3 + .../GreenbidsAnalyticsReporterTest.java | 2 +- .../auction/BidResponseCreatorTest.java | 2 +- .../server/auction/DsaEnforcerTest.java | 16 +- .../server/auction/ExchangeServiceTest.java | 2 +- .../model/BidRejectionTrackerTest.java | 199 +++++++++++++++--- .../bidder/HttpBidderRequesterTest.java | 80 +++---- .../floors/BasicPriceFloorEnforcerTest.java | 4 +- .../validation/ResponseBidValidatorTest.java | 78 ++++--- 23 files changed, 491 insertions(+), 272 deletions(-) diff --git a/extra/modules/ortb2-blocking/src/main/java/org/prebid/server/hooks/modules/ortb2/blocking/core/BidsBlocker.java b/extra/modules/ortb2-blocking/src/main/java/org/prebid/server/hooks/modules/ortb2/blocking/core/BidsBlocker.java index 2c9b40b6079..5435544e2cb 100644 --- a/extra/modules/ortb2-blocking/src/main/java/org/prebid/server/hooks/modules/ortb2/blocking/core/BidsBlocker.java +++ b/extra/modules/ortb2-blocking/src/main/java/org/prebid/server/hooks/modules/ortb2/blocking/core/BidsBlocker.java @@ -105,7 +105,8 @@ public ExecutionResult block() { final List warnings = MergeUtils.mergeMessages(blockedBidResults); if (blockedBids != null) { - rejectBlockedBids(blockedBidResults); + blockedBidIndexes.forEach(index -> + rejectBlockedBid(blockedBidResults.get(index).getValue(), bids.get(index))); } return ExecutionResult.builder() @@ -287,26 +288,19 @@ private String debugEntryFor(int index, BlockingResult blockingResult) { blockingResult.getFailedChecks()); } - private void rejectBlockedBids(List> 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); } } diff --git a/extra/modules/ortb2-blocking/src/test/java/org/prebid/server/hooks/modules/ortb2/blocking/core/BidsBlockerTest.java b/extra/modules/ortb2-blocking/src/test/java/org/prebid/server/hooks/modules/ortb2/blocking/core/BidsBlockerTest.java index 79b1309a2ba..b595b9c5fdb 100644 --- a/extra/modules/ortb2-blocking/src/test/java/org/prebid/server/hooks/modules/ortb2/blocking/core/BidsBlockerTest.java +++ b/extra/modules/ortb2-blocking/src/test/java/org/prebid/server/hooks/modules/ortb2/blocking/core/BidsBlockerTest.java @@ -197,13 +197,13 @@ public void shouldReturnResultWithBidWhenBidWithBlockedAdomainAndEnforceBlocksTr .build())); // when - final List 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 @@ -304,9 +304,9 @@ public void shouldReturnEmptyResultWhenBidWithBlockedAdomainAndInDealsExceptions .build())); // when - final List 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); @@ -324,13 +324,13 @@ public void shouldReturnResultWithBidWhenBidWithBlockedAdomainAndNotInDealsExcep .build())); // when - final List 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 @@ -344,8 +344,8 @@ public void shouldReturnResultWithBidAndDebugMessageWhenBidIsBlocked() { .build())); // when - final List 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 -> { @@ -353,7 +353,7 @@ public void shouldReturnResultWithBidAndDebugMessageWhenBidIsBlocked() { 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 @@ -367,12 +367,12 @@ public void shouldReturnResultWithBidWithoutDebugMessageWhenBidIsBlockedAndDebug .build())); // when - final List 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 @@ -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 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 bids = asList(bid1, bid2, bid3); final BlockedAttributes blockedAttributes = BlockedAttributes.builder() .badv(asList("domain1.com", "domain2.com", "domain3.com")) .bcat(asList("cat1", "cat2", "cat3")) @@ -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); } @@ -467,23 +468,30 @@ public void shouldReturnResultWithoutSomeBidsWhenAllAttributesInConfig() { .build())); // when - final List 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 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")) @@ -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); } diff --git a/extra/modules/pb-richmedia-filter/src/main/java/org/prebid/server/hooks/modules/pb/richmedia/filter/core/BidResponsesMraidFilter.java b/extra/modules/pb-richmedia-filter/src/main/java/org/prebid/server/hooks/modules/pb/richmedia/filter/core/BidResponsesMraidFilter.java index e528ce69c4e..499de57b1d8 100644 --- a/extra/modules/pb-richmedia-filter/src/main/java/org/prebid/server/hooks/modules/pb/richmedia/filter/core/BidResponsesMraidFilter.java +++ b/extra/modules/pb-richmedia-filter/src/main/java/org/prebid/server/hooks/modules/pb/richmedia/filter/core/BidResponsesMraidFilter.java @@ -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 errors = new ArrayList<>(seatBid.getErrors()); errors.add(BidderError.of("Invalid bid", BidderError.Type.invalid_bid, new HashSet<>(rejectedImps))); diff --git a/extra/modules/pb-richmedia-filter/src/test/java/org/prebid/server/hooks/modules/pb/richmedia/filter/core/BidResponsesMraidFilterTest.java b/extra/modules/pb-richmedia-filter/src/test/java/org/prebid/server/hooks/modules/pb/richmedia/filter/core/BidResponsesMraidFilterTest.java index f2066474df9..0198210d1ca 100644 --- a/extra/modules/pb-richmedia-filter/src/test/java/org/prebid/server/hooks/modules/pb/richmedia/filter/core/BidResponsesMraidFilterTest.java +++ b/extra/modules/pb-richmedia-filter/src/test/java/org/prebid/server/hooks/modules/pb/richmedia/filter/core/BidResponsesMraidFilterTest.java @@ -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); @@ -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", @@ -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); } diff --git a/src/main/java/org/prebid/server/analytics/reporter/greenbids/GreenbidsAnalyticsReporter.java b/src/main/java/org/prebid/server/analytics/reporter/greenbids/GreenbidsAnalyticsReporter.java index 51196d0c2e9..65de3f02ebc 100644 --- a/src/main/java/org/prebid/server/analytics/reporter/greenbids/GreenbidsAnalyticsReporter.java +++ b/src/main/java/org/prebid/server/analytics/reporter/greenbids/GreenbidsAnalyticsReporter.java @@ -362,7 +362,7 @@ private static Map getSeatsWithNonBids(AuctionContext auctionCon } private static SeatNonBid toSeatNonBid(String bidder, BidRejectionTracker bidRejectionTracker) { - final List nonBids = bidRejectionTracker.getRejectionReasons().entrySet().stream() + final List nonBids = bidRejectionTracker.getRejectedImps().entrySet().stream() .map(entry -> NonBid.of(entry.getKey(), entry.getValue())) .toList(); diff --git a/src/main/java/org/prebid/server/auction/BidResponseCreator.java b/src/main/java/org/prebid/server/auction/BidResponseCreator.java index b54f8d69a73..86fe28fdcbe 100644 --- a/src/main/java/org/prebid/server/auction/BidResponseCreator.java +++ b/src/main/java/org/prebid/server/auction/BidResponseCreator.java @@ -1758,7 +1758,7 @@ private static BidResponse populateSeatNonBid(AuctionContext auctionContext, Bid } private static SeatNonBid toSeatNonBid(String bidder, BidRejectionTracker bidRejectionTracker) { - final List nonBid = bidRejectionTracker.getRejectionReasons().entrySet().stream() + final List nonBid = bidRejectionTracker.getRejectedImps().entrySet().stream() .map(entry -> NonBid.of(entry.getKey(), entry.getValue())) .toList(); diff --git a/src/main/java/org/prebid/server/auction/DsaEnforcer.java b/src/main/java/org/prebid/server/auction/DsaEnforcer.java index 6719829cc56..369842b36c6 100644 --- a/src/main/java/org/prebid/server/auction/DsaEnforcer.java +++ b/src/main/java/org/prebid/server/auction/DsaEnforcer.java @@ -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); } } diff --git a/src/main/java/org/prebid/server/auction/ExchangeService.java b/src/main/java/org/prebid/server/auction/ExchangeService.java index e837b90d767..c4b6dfd7a55 100644 --- a/src/main/java/org/prebid/server/auction/ExchangeService.java +++ b/src/main/java/org/prebid/server/auction/ExchangeService.java @@ -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) @@ -1154,7 +1154,7 @@ private static Future processReject(AuctionContext auctionContex auctionContext.getBidRejectionTrackers() .get(bidderName) - .rejectAll(bidRejectionReason); + .rejectAllImps(bidRejectionReason); final BidderSeatBid bidderSeatBid = BidderSeatBid.builder() .warnings(warnings) .build(); @@ -1193,7 +1193,7 @@ private Future 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)); } diff --git a/src/main/java/org/prebid/server/auction/model/BidRejectionReason.java b/src/main/java/org/prebid/server/auction/model/BidRejectionReason.java index e916ee0b3a5..fc3ee36bd2a 100644 --- a/src/main/java/org/prebid/server/auction/model/BidRejectionReason.java +++ b/src/main/java/org/prebid/server/auction/model/BidRejectionReason.java @@ -74,6 +74,11 @@ public enum BidRejectionReason { */ RESPONSE_REJECTED_BELOW_FLOOR(301), + /** + * The bidder returns a bid that doesn't meet the price deal floor. + */ + RESPONSE_REJECTED_BELOW_DEAL_FLOOR(304), + /** * Rejected by the DSA validations */ @@ -101,14 +106,14 @@ public enum BidRejectionReason { */ RESPONSE_REJECTED_ADVERTISER_BLOCKED(356); - public final int code; + private final int code; BidRejectionReason(int code) { this.code = code; } @JsonValue - private int getValue() { + public int getValue() { return code; } diff --git a/src/main/java/org/prebid/server/auction/model/BidRejectionTracker.java b/src/main/java/org/prebid/server/auction/model/BidRejectionTracker.java index b0504ec13a3..9810606ada3 100644 --- a/src/main/java/org/prebid/server/auction/model/BidRejectionTracker.java +++ b/src/main/java/org/prebid/server/auction/model/BidRejectionTracker.java @@ -1,90 +1,162 @@ package org.prebid.server.auction.model; import com.iab.openrtb.response.Bid; +import org.apache.commons.lang3.tuple.Pair; import org.prebid.server.bidder.model.BidderBid; import org.prebid.server.log.ConditionalLogger; import org.prebid.server.log.Logger; import org.prebid.server.log.LoggerFactory; import org.prebid.server.util.MapUtil; +import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; public class BidRejectionTracker { private static final Logger logger = LoggerFactory.getLogger(BidRejectionTracker.class); - private static final ConditionalLogger MULTIPLE_BID_REJECTIONS_LOGGER = + private static final ConditionalLogger BID_REJECTIONS_LOGGER = new ConditionalLogger("multiple-bid-rejections", logger); - private static final String WARNING_TEMPLATE = - "Bid with imp id: %s for bidder: %s rejected due to: %s, but has been already rejected"; + private static final String MULTIPLE_REJECTIONS_WARNING_TEMPLATE = + "Warning: bidder %s on imp %s responded with multiple nonbid reasons."; + + private static final String INCONSISTENT_RESPONSES_WARNING_TEMPLATE = + "Warning: Inconsistent responses from bidder %s on imp %s: both bids and nonbids."; private final double logSamplingRate; private final String bidder; private final Set involvedImpIds; - private final Set succeededImpIds; - private final Map rejectedImpIds; + private final Map> succeededBidsIds; + private final Map>> rejectedBids; public BidRejectionTracker(String bidder, Set involvedImpIds, double logSamplingRate) { this.bidder = bidder; this.involvedImpIds = new HashSet<>(involvedImpIds); this.logSamplingRate = logSamplingRate; - succeededImpIds = new HashSet<>(); - rejectedImpIds = new HashMap<>(); - } - - public void succeed(String impId) { - if (involvedImpIds.contains(impId)) { - succeededImpIds.add(impId); - rejectedImpIds.remove(impId); - } + succeededBidsIds = new HashMap<>(); + rejectedBids = new HashMap<>(); } + /** + * Restores ONLY imps from rejection, rejected bids are preserved for analytics. + * A bid can be rejected only once. + */ public void succeed(Collection bids) { bids.stream() .map(BidderBid::getBid) .filter(Objects::nonNull) - .map(Bid::getImpid) - .filter(Objects::nonNull) .forEach(this::succeed); } + private void succeed(Bid bid) { + final String bidId = bid.getId(); + final String impId = bid.getImpid(); + if (involvedImpIds.contains(impId)) { + succeededBidsIds.computeIfAbsent(impId, key -> new HashSet<>()).add(bidId); + if (rejectedBids.containsKey(impId)) { + BID_REJECTIONS_LOGGER.warn( + INCONSISTENT_RESPONSES_WARNING_TEMPLATE.formatted(bidder, impId), + logSamplingRate); + } + } + } + public void restoreFromRejection(Collection bids) { succeed(bids); } - public void reject(String impId, BidRejectionReason reason) { - if (involvedImpIds.contains(impId) && !rejectedImpIds.containsKey(impId)) { - rejectedImpIds.put(impId, reason); - succeededImpIds.remove(impId); - } else if (rejectedImpIds.containsKey(impId)) { - MULTIPLE_BID_REJECTIONS_LOGGER.warn( - WARNING_TEMPLATE.formatted(impId, bidder, reason), logSamplingRate); + public void rejectBids(Collection bidderBids, BidRejectionReason reason) { + bidderBids.forEach(bidderBid -> rejectBid(bidderBid, reason)); + } + + public void rejectBid(BidderBid bidderBid, BidRejectionReason reason) { + final Bid bid = bidderBid.getBid(); + final String impId = bid.getImpid(); + + reject(impId, bidderBid, reason); + } + + private void reject(String impId, BidderBid bid, BidRejectionReason reason) { + if (involvedImpIds.contains(impId)) { + if (rejectedBids.containsKey(impId)) { + BID_REJECTIONS_LOGGER.warn( + MULTIPLE_REJECTIONS_WARNING_TEMPLATE.formatted(bidder, impId), logSamplingRate); + } + + rejectedBids.computeIfAbsent(impId, key -> new ArrayList<>()).add(Pair.of(bid, reason)); + + if (succeededBidsIds.containsKey(impId)) { + final String bidId = Optional.ofNullable(bid).map(BidderBid::getBid).map(Bid::getId).orElse(null); + final Set succeededBids = succeededBidsIds.get(impId); + final boolean removed = bidId == null || succeededBids.remove(bidId); + if (removed && !succeededBids.isEmpty()) { + BID_REJECTIONS_LOGGER.warn( + INCONSISTENT_RESPONSES_WARNING_TEMPLATE.formatted(bidder, impId), + logSamplingRate); + } + } + } + } + + public void rejectImps(Collection impIds, BidRejectionReason reason) { + impIds.forEach(impId -> rejectImp(impId, reason)); + } + + public void rejectImp(String impId, BidRejectionReason reason) { + if (reason.getValue() >= 300) { + throw new IllegalArgumentException("The non-bid code 300 and higher assumes " + + "that there is a rejected bid that shouldn't be lost"); } + reject(impId, null, reason); } - public void reject(Collection impIds, BidRejectionReason reason) { - impIds.forEach(impId -> reject(impId, reason)); + public void rejectAllImps(BidRejectionReason reason) { + involvedImpIds.forEach(impId -> rejectImp(impId, reason)); } - public void rejectAll(BidRejectionReason reason) { - involvedImpIds.forEach(impId -> reject(impId, reason)); + /** + * If an impression has at least one valid bid, it's not considered rejected. + * If no valid bids are returned for the impression, only the first one rejected reason will be returned + */ + public Map getRejectedImps() { + final Map rejectedImpIds = new HashMap<>(); + for (String impId : involvedImpIds) { + final Set succeededBids = succeededBidsIds.getOrDefault(impId, Collections.emptySet()); + if (succeededBids.isEmpty()) { + if (rejectedBids.containsKey(impId)) { + rejectedImpIds.put(impId, rejectedBids.get(impId).getFirst().getRight()); + } else { + rejectedImpIds.put(impId, BidRejectionReason.NO_BID); + } + } + } + + return rejectedImpIds; } - public Map getRejectionReasons() { - final Map missingImpIds = new HashMap<>(); + /** + * Bid is absent for the non-bid code from 0 to 299 + */ + public Map>> getRejectedBids() { + final Map>> missingImpIds = new HashMap<>(); for (String impId : involvedImpIds) { - if (!succeededImpIds.contains(impId) && !rejectedImpIds.containsKey(impId)) { - missingImpIds.put(impId, BidRejectionReason.NO_BID); + final Set succeededBids = succeededBidsIds.getOrDefault(impId, Collections.emptySet()); + if (succeededBids.isEmpty() && !rejectedBids.containsKey(impId)) { + missingImpIds.computeIfAbsent(impId, key -> new ArrayList<>()) + .add(Pair.of(null, BidRejectionReason.NO_BID)); } } - return MapUtil.merge(missingImpIds, rejectedImpIds); + return MapUtil.merge(missingImpIds, rejectedBids); } } diff --git a/src/main/java/org/prebid/server/bidder/HttpBidderRequester.java b/src/main/java/org/prebid/server/bidder/HttpBidderRequester.java index 787ee1f96d2..0345aa4c29e 100644 --- a/src/main/java/org/prebid/server/bidder/HttpBidderRequester.java +++ b/src/main/java/org/prebid/server/bidder/HttpBidderRequester.java @@ -156,7 +156,7 @@ private static void rejectErrors(BidRejectionTracker bidRejectionTracker, bidderErrors.stream() .filter(error -> CollectionUtils.isNotEmpty(error.getImpIds())) - .forEach(error -> bidRejectionTracker.reject(error.getImpIds(), reason)); + .forEach(error -> bidRejectionTracker.rejectImps(error.getImpIds(), reason)); } private boolean isStoredResponse(List> httpRequests, String storedResponse, String bidder) { @@ -397,7 +397,7 @@ private void handleBidderCallError(BidderCall bidderCall) { .orElse(null); if (statusCode != null && statusCode == HttpResponseStatus.SERVICE_UNAVAILABLE.code()) { - bidRejectionTracker.reject(requestedImpIds, BidRejectionReason.ERROR_BIDDER_UNREACHABLE); + bidRejectionTracker.rejectImps(requestedImpIds, BidRejectionReason.ERROR_BIDDER_UNREACHABLE); return; } @@ -405,7 +405,7 @@ private void handleBidderCallError(BidderCall bidderCall) { && (statusCode < HttpResponseStatus.OK.code() || statusCode >= HttpResponseStatus.BAD_REQUEST.code())) { - bidRejectionTracker.reject(requestedImpIds, BidRejectionReason.ERROR_INVALID_BID_RESPONSE); + bidRejectionTracker.rejectImps(requestedImpIds, BidRejectionReason.ERROR_INVALID_BID_RESPONSE); return; } @@ -417,9 +417,9 @@ private void handleBidderCallError(BidderCall bidderCall) { } if (callErrorType == BidderError.Type.timeout) { - bidRejectionTracker.reject(requestedImpIds, BidRejectionReason.ERROR_TIMED_OUT); + bidRejectionTracker.rejectImps(requestedImpIds, BidRejectionReason.ERROR_TIMED_OUT); } else { - bidRejectionTracker.reject(requestedImpIds, BidRejectionReason.ERROR_GENERAL); + bidRejectionTracker.rejectImps(requestedImpIds, BidRejectionReason.ERROR_GENERAL); } } diff --git a/src/main/java/org/prebid/server/floors/BasicPriceFloorEnforcer.java b/src/main/java/org/prebid/server/floors/BasicPriceFloorEnforcer.java index ee77a6bbcb5..132bf86e782 100644 --- a/src/main/java/org/prebid/server/floors/BasicPriceFloorEnforcer.java +++ b/src/main/java/org/prebid/server/floors/BasicPriceFloorEnforcer.java @@ -179,7 +179,7 @@ private AuctionParticipation applyEnforcement(BidRequest bidRequest, "Bid with id '%s' was rejected by floor enforcement: price %s is below the floor %s" .formatted(bid.getId(), price, floor), impId)); - rejectionTracker.reject(impId, BidRejectionReason.RESPONSE_REJECTED_BELOW_FLOOR); + rejectionTracker.rejectBid(bidderBid, BidRejectionReason.RESPONSE_REJECTED_BELOW_FLOOR); updatedBidderBids.remove(bidderBid); } } diff --git a/src/main/java/org/prebid/server/validation/ResponseBidValidator.java b/src/main/java/org/prebid/server/validation/ResponseBidValidator.java index 10b939ae5dd..4e2f062218b 100644 --- a/src/main/java/org/prebid/server/validation/ResponseBidValidator.java +++ b/src/main/java/org/prebid/server/validation/ResponseBidValidator.java @@ -85,7 +85,7 @@ public ValidationResult validate(BidderBid bidderBid, final Imp correspondingImp = findCorrespondingImp(bid, bidRequest); if (bidderBid.getType() == BidType.banner) { warnings.addAll(validateBannerFields( - bid, + bidderBid, bidder, bidRequest, account, @@ -95,7 +95,7 @@ public ValidationResult validate(BidderBid bidderBid, } warnings.addAll(validateSecureMarkup( - bid, + bidderBid, bidder, bidRequest, account, @@ -161,7 +161,7 @@ private ValidationException exceptionAndLogOnePercent(String message) { return new ValidationException(message); } - private List validateBannerFields(Bid bid, + private List validateBannerFields(BidderBid bidderBid, String bidder, BidRequest bidRequest, Account account, @@ -172,7 +172,7 @@ private List validateBannerFields(Bid bid, final BidValidationEnforcement bannerMaxSizeEnforcement = effectiveBannerMaxSizeEnforcement(account); if (bannerMaxSizeEnforcement != BidValidationEnforcement.skip) { final Format maxSize = maxSizeForBanner(correspondingImp); - + final Bid bid = bidderBid.getBid(); if (bannerSizeIsNotValid(bid, maxSize)) { final String accountId = account.getId(); final String message = """ @@ -189,15 +189,15 @@ private List validateBannerFields(Bid bid, bid.getW(), bid.getH()); - bidRejectionTracker.reject( - correspondingImp.getId(), - BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_SIZE_NOT_ALLOWED); - return singleWarningOrValidationException( bannerMaxSizeEnforcement, metricName -> metrics.updateSizeValidationMetrics( aliases.resolveBidder(bidder), accountId, metricName), - CREATIVE_SIZE_LOGGER, message); + CREATIVE_SIZE_LOGGER, + message, + bidRejectionTracker, + bidderBid, + BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_SIZE_NOT_ALLOWED); } } return Collections.emptyList(); @@ -236,7 +236,7 @@ private static boolean bannerSizeIsNotValid(Bid bid, Format maxSize) { || bidH == null || bidH > maxSize.getH(); } - private List validateSecureMarkup(Bid bid, + private List validateSecureMarkup(BidderBid bidderBid, String bidder, BidRequest bidRequest, Account account, @@ -250,6 +250,7 @@ private List validateSecureMarkup(Bid bid, final String accountId = account.getId(); final String referer = getReferer(bidRequest); + final Bid bid = bidderBid.getBid(); final String adm = bid.getAdm(); if (isImpSecure(correspondingImp) && markupIsNotSecure(adm)) { @@ -258,15 +259,15 @@ private List validateSecureMarkup(Bid bid, creative validation for bid %s, account=%s, referrer=%s, adm=%s""" .formatted(secureMarkupEnforcement, bidder, bid.getId(), accountId, referer, adm); - bidRejectionTracker.reject( - correspondingImp.getId(), - BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_NOT_SECURE); - return singleWarningOrValidationException( secureMarkupEnforcement, metricName -> metrics.updateSecureValidationMetrics( aliases.resolveBidder(bidder), accountId, metricName), - SECURE_CREATIVE_LOGGER, message); + SECURE_CREATIVE_LOGGER, + message, + bidRejectionTracker, + bidderBid, + BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_NOT_SECURE); } return Collections.emptyList(); @@ -281,12 +282,18 @@ private static boolean markupIsNotSecure(String adm) { || !StringUtils.containsAny(adm, SECURE_MARKUP_MARKERS); } - private List singleWarningOrValidationException(BidValidationEnforcement enforcement, - Consumer metricsRecorder, - ConditionalLogger conditionalLogger, - String message) throws ValidationException { + private List singleWarningOrValidationException( + BidValidationEnforcement enforcement, + Consumer metricsRecorder, + ConditionalLogger conditionalLogger, + String message, + BidRejectionTracker bidRejectionTracker, + BidderBid bidderBid, + BidRejectionReason bidRejectionReason) throws ValidationException { + return switch (enforcement) { case enforce -> { + bidRejectionTracker.rejectBid(bidderBid, bidRejectionReason); metricsRecorder.accept(MetricName.err); conditionalLogger.warn(message, logSamplingRate); throw new ValidationException(message); diff --git a/src/test/groovy/org/prebid/server/functional/model/response/auction/BidRejectionReason.groovy b/src/test/groovy/org/prebid/server/functional/model/response/auction/BidRejectionReason.groovy index 79cf8ad9317..2fb7d75bbf7 100644 --- a/src/test/groovy/org/prebid/server/functional/model/response/auction/BidRejectionReason.groovy +++ b/src/test/groovy/org/prebid/server/functional/model/response/auction/BidRejectionReason.groovy @@ -9,6 +9,7 @@ enum BidRejectionReason { ERROR_TIMED_OUT(101), ERROR_INVALID_BID_RESPONSE(102), ERROR_BIDDER_UNREACHABLE(103), + ERROR_REQUEST(104), REQUEST_BLOCKED_GENERAL(200), REQUEST_BLOCKED_UNSUPPORTED_CHANNEL(201), diff --git a/src/test/groovy/org/prebid/server/functional/tests/SeatNonBidSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/SeatNonBidSpec.groovy index eeff3783811..f6b02f22e76 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/SeatNonBidSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/SeatNonBidSpec.groovy @@ -172,6 +172,9 @@ class SeatNonBidSpec extends BaseSpec { assert seatNonBid.seat == GENERIC.value assert seatNonBid.nonBid[0].impId == bidRequest.imp[0].id assert seatNonBid.nonBid[0].statusCode == RESPONSE_REJECTED_INVALID_CREATIVE_NOT_SECURE + + and: "PBS response shouldn't contain seatBid" + assert !response.seatbid } def "PBS shouldn't populate seatNonBid when returnAllBidStatus=true and bidder successfully bids"() { diff --git a/src/test/java/org/prebid/server/analytics/reporter/greenbids/GreenbidsAnalyticsReporterTest.java b/src/test/java/org/prebid/server/analytics/reporter/greenbids/GreenbidsAnalyticsReporterTest.java index ab16f5a1b40..cccdbb4e149 100644 --- a/src/test/java/org/prebid/server/analytics/reporter/greenbids/GreenbidsAnalyticsReporterTest.java +++ b/src/test/java/org/prebid/server/analytics/reporter/greenbids/GreenbidsAnalyticsReporterTest.java @@ -773,7 +773,7 @@ private static BidRejectionTracker givenBidRejectionTracker() { "seat3", Set.of("adunitcodevalue"), 1.0); - bidRejectionTracker.reject("imp1", BidRejectionReason.NO_BID); + bidRejectionTracker.rejectImp("imp1", BidRejectionReason.NO_BID); return bidRejectionTracker; } diff --git a/src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java b/src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java index daf9e2d4ff6..19181bf3503 100644 --- a/src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java +++ b/src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java @@ -3575,7 +3575,7 @@ public void shouldDropFledgeResponsesReferencingUnknownImps() { public void shouldPopulateExtPrebidSeatNonBidWhenReturnAllBidStatusFlagIsTrue() { // given final BidRejectionTracker bidRejectionTracker = mock(BidRejectionTracker.class); - given(bidRejectionTracker.getRejectionReasons()).willReturn(singletonMap("impId2", BidRejectionReason.NO_BID)); + given(bidRejectionTracker.getRejectedImps()).willReturn(singletonMap("impId2", BidRejectionReason.NO_BID)); final Bid bid = Bid.builder().id("bidId").price(BigDecimal.valueOf(3.67)).impid("impId").build(); final List bidderResponses = singletonList( diff --git a/src/test/java/org/prebid/server/auction/DsaEnforcerTest.java b/src/test/java/org/prebid/server/auction/DsaEnforcerTest.java index 3c42e27b3ed..5254ea4edda 100644 --- a/src/test/java/org/prebid/server/auction/DsaEnforcerTest.java +++ b/src/test/java/org/prebid/server/auction/DsaEnforcerTest.java @@ -104,7 +104,7 @@ public void enforceShouldRejectBidAndAddWarningWhenDsaIsNotRequiredAndDsaRespons .bidderResponse(BidderResponse.of("bidder", expectedSeatBid, 100)) .build(); assertThat(actual).isEqualTo(expectedParticipation); - verify(bidRejectionTracker).reject("imp_id", BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); + verify(bidRejectionTracker).rejectBid(bid, BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); } @Test @@ -137,7 +137,7 @@ public void enforceShouldRejectBidAndAddWarningWhenDsaIsNotRequiredAndDsaRespons .bidderResponse(BidderResponse.of("bidder", expectedSeatBid, 100)) .build(); assertThat(actual).isEqualTo(expectedParticipation); - verify(bidRejectionTracker).reject("imp_id", BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); + verify(bidRejectionTracker).rejectBid(bid, BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); } @Test @@ -333,7 +333,7 @@ public void enforceShouldRejectBidAndAddWarningWhenBidExtHasEmptyDsaAndDsaIsRequ .bidderResponse(BidderResponse.of("bidder", expectedSeatBid, 100)) .build(); assertThat(actual).isEqualTo(expectedParticipation); - verify(bidRejectionTracker).reject("imp_id", BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); + verify(bidRejectionTracker).rejectBid(bid, BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); } @Test @@ -367,7 +367,7 @@ public void enforceShouldRejectBidAndAddWarningWhenDsaIsRequiredAndDsaResponseHa .bidderResponse(BidderResponse.of("bidder", expectedSeatBid, 100)) .build(); assertThat(actual).isEqualTo(expectedParticipation); - verify(bidRejectionTracker).reject("imp_id", BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); + verify(bidRejectionTracker).rejectBid(bid, BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); } @Test @@ -401,7 +401,7 @@ public void enforceShouldRejectBidAndAddWarningWhenDsaIsRequiredAndDsaResponseHa .bidderResponse(BidderResponse.of("bidder", expectedSeatBid, 100)) .build(); assertThat(actual).isEqualTo(expectedParticipation); - verify(bidRejectionTracker).reject("imp_id", BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); + verify(bidRejectionTracker).rejectBid(bid, BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); } @Test @@ -436,7 +436,7 @@ public void enforceShouldRejectBidAndAddWarningWhenDsaIsRequiredAndPublisherAndA .bidderResponse(BidderResponse.of("bidder", expectedSeatBid, 100)) .build(); assertThat(actual).isEqualTo(expectedParticipation); - verify(bidRejectionTracker).reject("imp_id", BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); + verify(bidRejectionTracker).rejectBid(bid, BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); } @Test @@ -471,7 +471,7 @@ public void enforceShouldRejectBidAndAddWarningWhenDsaIsRequiredAndPublisherAndA .bidderResponse(BidderResponse.of("bidder", expectedSeatBid, 100)) .build(); assertThat(actual).isEqualTo(expectedParticipation); - verify(bidRejectionTracker).reject("imp_id", BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); + verify(bidRejectionTracker).rejectBid(bid, BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); } @Test @@ -505,7 +505,7 @@ public void enforceShouldRejectBidAndAddWarningWhenDsaIsRequiredAndPublisherNotR .bidderResponse(BidderResponse.of("bidder", expectedSeatBid, 100)) .build(); assertThat(actual).isEqualTo(expectedParticipation); - verify(bidRejectionTracker).reject("imp_id", BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); + verify(bidRejectionTracker).rejectBid(bid, BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); } private static ExtRegs givenExtRegs(DsaRequired dsaRequired, DsaPublisherRender pubRender) { diff --git a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java index 8c1c20ac35e..bfecf6632f9 100644 --- a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java +++ b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java @@ -3801,7 +3801,7 @@ public void shouldResponseWithEmptySeatBidIfBidderNotSupportRequestCurrency() { assertThat(result.result()) .extracting(AuctionContext::getBidRejectionTrackers) .extracting(rejectionTrackers -> rejectionTrackers.get("bidder1")) - .extracting(BidRejectionTracker::getRejectionReasons) + .extracting(BidRejectionTracker::getRejectedImps) .isEqualTo(Map.of("impId1", BidRejectionReason.REQUEST_BLOCKED_UNACCEPTABLE_CURRENCY)); } diff --git a/src/test/java/org/prebid/server/auction/model/BidRejectionTrackerTest.java b/src/test/java/org/prebid/server/auction/model/BidRejectionTrackerTest.java index 5b9cc66779e..502b09108b5 100644 --- a/src/test/java/org/prebid/server/auction/model/BidRejectionTrackerTest.java +++ b/src/test/java/org/prebid/server/auction/model/BidRejectionTrackerTest.java @@ -1,14 +1,19 @@ package org.prebid.server.auction.model; +import com.iab.openrtb.response.Bid; +import org.apache.commons.lang3.tuple.Pair; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.prebid.server.bidder.model.BidderBid; +import java.util.List; import java.util.Map; import java.util.Set; import static java.util.Collections.singleton; -import static java.util.Collections.singletonMap; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.entry; public class BidRejectionTrackerTest { @@ -16,90 +21,214 @@ public class BidRejectionTrackerTest { @BeforeEach public void setUp() { - target = new BidRejectionTracker("bidder", singleton("1"), 0); + target = new BidRejectionTracker("bidder", singleton("impId1"), 0); } @Test - public void succeedShouldRestoreBidderFromRejection() { + public void succeedShouldRestoreImpFromImpRejection() { // given - target.reject("1", BidRejectionReason.ERROR_GENERAL); + target.rejectImp("impId1", BidRejectionReason.ERROR_GENERAL); // when - target.succeed("1"); + final BidderBid bid = BidderBid.builder().bid(Bid.builder().id("bidId1").impid("impId1").build()).build(); + target.succeed(singleton(bid)); // then - assertThat(target.getRejectionReasons()).isEmpty(); + assertThat(target.getRejectedImps()).isEmpty(); + assertThat(target.getRejectedBids()) + .containsOnly(entry("impId1", List.of(Pair.of(null, BidRejectionReason.ERROR_GENERAL)))); } @Test - public void succeedShouldIgnoreUninvolvedImpIds() { + public void succeedShouldRestoreImpFromBidRejection() { // given - target.reject("1", BidRejectionReason.ERROR_GENERAL); + final BidderBid bid = BidderBid.builder().bid(Bid.builder().id("bidId1").impid("impId1").build()).build(); + target.rejectBid(bid, BidRejectionReason.ERROR_GENERAL); // when - target.succeed("2"); + target.succeed(singleton(bid)); // then - assertThat(target.getRejectionReasons()) - .isEqualTo(singletonMap("1", BidRejectionReason.ERROR_GENERAL)); + assertThat(target.getRejectedImps()).isEmpty(); + assertThat(target.getRejectedBids()) + .containsOnly(entry("impId1", List.of(Pair.of(bid, BidRejectionReason.ERROR_GENERAL)))); } @Test - public void rejectShouldRecordRejectionFirstTimeIfImpIdIsInvolved() { + public void succeedShouldIgnoreUninvolvedImpIdsOnImpRejection() { + // given + target.rejectImp("impId1", BidRejectionReason.ERROR_GENERAL); + + // when + final BidderBid bid = BidderBid.builder().bid(Bid.builder().id("bidId2").impid("impId2").build()).build(); + target.succeed(singleton(bid)); + + // then + assertThat(target.getRejectedImps()).containsOnly(entry("impId1", BidRejectionReason.ERROR_GENERAL)); + assertThat(target.getRejectedBids()) + .containsOnly(entry("impId1", List.of(Pair.of(null, BidRejectionReason.ERROR_GENERAL)))); + } + + @Test + public void succeedShouldIgnoreUninvolvedImpIdsOnBidRejection() { + // given + final BidderBid bid1 = BidderBid.builder().bid(Bid.builder().id("bidId1").impid("impId1").build()).build(); + target.rejectBid(bid1, BidRejectionReason.ERROR_GENERAL); + + // when + final BidderBid bid2 = BidderBid.builder().bid(Bid.builder().id("bidId2").impid("impId2").build()).build(); + target.succeed(singleton(bid2)); + + // then + assertThat(target.getRejectedImps()).containsOnly(entry("impId1", BidRejectionReason.ERROR_GENERAL)); + assertThat(target.getRejectedBids()) + .containsOnly(entry("impId1", List.of(Pair.of(bid1, BidRejectionReason.ERROR_GENERAL)))); + } + + @Test + public void rejectImpShouldRecordImpRejectionFirstTimeIfImpIdIsInvolved() { + // when + target.rejectImp("impId1", BidRejectionReason.ERROR_GENERAL); + + // then + assertThat(target.getRejectedImps()).containsOnly(entry("impId1", BidRejectionReason.ERROR_GENERAL)); + assertThat(target.getRejectedBids()) + .containsOnly(entry("impId1", List.of(Pair.of(null, BidRejectionReason.ERROR_GENERAL)))); + } + + @Test + public void rejectBidShouldRecordBidRejectionFirstTimeIfImpIdIsInvolved() { // when - target.reject("1", BidRejectionReason.ERROR_GENERAL); + final BidderBid bid = BidderBid.builder().bid(Bid.builder().id("bidId1").impid("impId1").build()).build(); + target.rejectBid(bid, BidRejectionReason.ERROR_GENERAL); // then - assertThat(target.getRejectionReasons()) - .isEqualTo(singletonMap("1", BidRejectionReason.ERROR_GENERAL)); + assertThat(target.getRejectedImps()).containsOnly(entry("impId1", BidRejectionReason.ERROR_GENERAL)); + assertThat(target.getRejectedBids()) + .containsOnly(entry("impId1", List.of(Pair.of(bid, BidRejectionReason.ERROR_GENERAL)))); } @Test - public void rejectShouldNotRecordRejectionIfImpIdIsNotInvolved() { + public void rejectBidShouldRecordBidRejectionAfterPreviouslySucceededBid() { + // given + final BidderBid bid1 = BidderBid.builder().bid(Bid.builder().id("bidId1").impid("impId1").build()).build(); + final BidderBid bid2 = BidderBid.builder().bid(Bid.builder().id("bidId2").impid("impId1").build()).build(); + target.succeed(Set.of(bid1, bid2)); + + // when + target.rejectBid(bid1, BidRejectionReason.ERROR_GENERAL); + + // then + assertThat(target.getRejectedImps()).isEmpty(); + assertThat(target.getRejectedBids()) + .containsOnly(entry("impId1", List.of(Pair.of(bid1, BidRejectionReason.ERROR_GENERAL)))); + } + + @Test + public void rejectImpShouldNotRecordImpRejectionIfImpIdIsAlreadyRejected() { + // given + target.rejectImp("impId1", BidRejectionReason.ERROR_GENERAL); + + // when + target.rejectImp("impId1", BidRejectionReason.ERROR_INVALID_BID_RESPONSE); + + // then + assertThat(target.getRejectedImps()).containsOnly(entry("impId1", BidRejectionReason.ERROR_GENERAL)); + assertThat(target.getRejectedBids()) + .containsOnly(entry("impId1", List.of( + Pair.of(null, BidRejectionReason.ERROR_GENERAL), + Pair.of(null, BidRejectionReason.ERROR_INVALID_BID_RESPONSE)))); + } + + @Test + public void rejectBidShouldNotRecordImpRejectionButRecordBidRejectionEvenIfImpIsAlreadyRejected() { + // given + final BidderBid bid1 = BidderBid.builder().bid(Bid.builder().id("bidId1").impid("impId1").build()).build(); + target.rejectBid(bid1, BidRejectionReason.RESPONSE_REJECTED_GENERAL); + // when - target.reject("2", BidRejectionReason.ERROR_GENERAL); + final BidderBid bid2 = BidderBid.builder().bid(Bid.builder().id("bidId2").impid("impId1").build()).build(); + target.rejectBid(bid2, BidRejectionReason.RESPONSE_REJECTED_BELOW_FLOOR); // then - assertThat(target.getRejectionReasons()).doesNotContainKey("2"); + assertThat(target.getRejectedImps()) + .containsOnly(entry("impId1", BidRejectionReason.RESPONSE_REJECTED_GENERAL)); + assertThat(target.getRejectedBids()) + .containsOnly(entry("impId1", List.of( + Pair.of(bid1, BidRejectionReason.RESPONSE_REJECTED_GENERAL), + Pair.of(bid2, BidRejectionReason.RESPONSE_REJECTED_BELOW_FLOOR)))); } @Test - public void rejectShouldNotRecordRejectionIfImpIdIsAlreadyRejected() { + public void rejectAllImpsShouldTryRejectingEachImpId() { // given - target.reject("1", BidRejectionReason.ERROR_GENERAL); + target = new BidRejectionTracker("bidder", Set.of("impId1", "impId2", "impId3"), 0); + target.rejectImp("impId1", BidRejectionReason.NO_BID); // when - target.reject("1", BidRejectionReason.ERROR_INVALID_BID_RESPONSE); + target.rejectAllImps(BidRejectionReason.ERROR_TIMED_OUT); // then - assertThat(target.getRejectionReasons()) - .isEqualTo(singletonMap("1", BidRejectionReason.ERROR_GENERAL)); + assertThat(target.getRejectedImps()) + .isEqualTo(Map.of( + "impId1", BidRejectionReason.NO_BID, + "impId2", BidRejectionReason.ERROR_TIMED_OUT, + "impId3", BidRejectionReason.ERROR_TIMED_OUT)); + + assertThat(target.getRejectedBids()) + .containsOnly( + entry("impId1", List.of( + Pair.of(null, BidRejectionReason.NO_BID), + Pair.of(null, BidRejectionReason.ERROR_TIMED_OUT))), + entry("impId2", List.of(Pair.of(null, BidRejectionReason.ERROR_TIMED_OUT))), + entry("impId3", List.of(Pair.of(null, BidRejectionReason.ERROR_TIMED_OUT)))); } @Test - public void rejectAllShouldTryRejectingEachImpId() { + public void rejectBidsShouldTryRejectingEachBid() { // given - target = new BidRejectionTracker("bidder", Set.of("1", "2", "3"), 0); - target.reject("1", BidRejectionReason.NO_BID); + target = new BidRejectionTracker("bidder", Set.of("impId1", "impId2", "impId3"), 0); + final BidderBid bid0 = BidderBid.builder().bid(Bid.builder().id("bidId0").impid("impId1").build()).build(); + target.rejectBid(bid0, BidRejectionReason.RESPONSE_REJECTED_GENERAL); // when - target.rejectAll(BidRejectionReason.ERROR_TIMED_OUT); + final BidderBid bid1 = BidderBid.builder().bid(Bid.builder().id("bidId1").impid("impId1").build()).build(); + final BidderBid bid2 = BidderBid.builder().bid(Bid.builder().id("bidId2").impid("impId2").build()).build(); + final BidderBid bid3 = BidderBid.builder().bid(Bid.builder().id("bidId3").impid("impId3").build()).build(); + target.rejectBids(Set.of(bid1, bid2, bid3), BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY); // then - assertThat(target.getRejectionReasons()) + assertThat(target.getRejectedImps()) .isEqualTo(Map.of( - "1", BidRejectionReason.NO_BID, - "2", BidRejectionReason.ERROR_TIMED_OUT, - "3", BidRejectionReason.ERROR_TIMED_OUT)); + "impId1", BidRejectionReason.RESPONSE_REJECTED_GENERAL, + "impId2", BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY, + "impId3", BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY)); + + assertThat(target.getRejectedBids()) + .containsOnly( + entry("impId1", List.of( + Pair.of(bid0, BidRejectionReason.RESPONSE_REJECTED_GENERAL), + Pair.of(bid1, BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY))), + entry("impId2", List.of(Pair.of(bid2, BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY))), + entry("impId3", List.of(Pair.of(bid3, BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY)))); } @Test - public void getRejectionReasonsShouldTreatUnsuccessfulBidsAsNoBidRejection() { + public void getRejectedImpsShouldTreatUnsuccessfulImpsAsNoBidRejection() { // given - target = new BidRejectionTracker("bidder", Set.of("1", "2"), 0); - target.succeed("2"); + target = new BidRejectionTracker("bidder", Set.of("impId1", "impId2"), 0); + final BidderBid bid = BidderBid.builder().bid(Bid.builder().id("bidId1").impid("impId2").build()).build(); + target.succeed(singleton(bid)); // then - assertThat(target.getRejectionReasons()).isEqualTo(singletonMap("1", BidRejectionReason.NO_BID)); + assertThat(target.getRejectedImps()).containsOnly(entry("impId1", BidRejectionReason.NO_BID)); + } + + @Test + public void rejectImpShouldFailRejectingWithReasonThatImpliesExistingBidToReject() { + assertThatThrownBy(() -> target.rejectImp("impId1", BidRejectionReason.RESPONSE_REJECTED_DSA_PRIVACY)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("The non-bid code 300 and higher assumes " + + "that there is a rejected bid that shouldn't be lost"); } } diff --git a/src/test/java/org/prebid/server/bidder/HttpBidderRequesterTest.java b/src/test/java/org/prebid/server/bidder/HttpBidderRequesterTest.java index 81180793e22..d2ce145e7ef 100644 --- a/src/test/java/org/prebid/server/bidder/HttpBidderRequesterTest.java +++ b/src/test/java/org/prebid/server/bidder/HttpBidderRequesterTest.java @@ -226,8 +226,8 @@ public void shouldPassStoredResponseToBidderMakeBidsMethodAndReturnSeatBids() { .isEqualTo("storedResponse"); assertThat(bidderSeatBid.getBids()).hasSameElementsAs(bids); - verify(bidRejectionTracker, never()).reject(anyString(), any()); - verify(bidRejectionTracker, never()).reject(anyList(), any()); + verify(bidRejectionTracker, never()).rejectImp(anyString(), any()); + verify(bidRejectionTracker, never()).rejectImps(anyList(), any()); } @Test @@ -265,8 +265,8 @@ public void shouldMakeRequestToBidderWhenStoredResponseDefinedButBidderCreatesMo // then verify(httpClient, times(2)).request(any(), anyString(), any(), any(byte[].class), anyLong()); - verify(bidRejectionTracker, never()).reject(anyString(), any()); - verify(bidRejectionTracker, never()).reject(anyList(), any()); + verify(bidRejectionTracker, never()).rejectImp(anyString(), any()); + verify(bidRejectionTracker, never()).rejectImps(anyList(), any()); } @Test @@ -300,8 +300,8 @@ public void shouldSendPopulatedGetRequestWithoutBody() { // then verify(httpClient).request(any(), anyString(), any(), (byte[]) isNull(), anyLong()); - verify(bidRejectionTracker, never()).reject(anyString(), any()); - verify(bidRejectionTracker, never()).reject(anyList(), any()); + verify(bidRejectionTracker, never()).rejectImp(anyString(), any()); + verify(bidRejectionTracker, never()).rejectImps(anyList(), any()); } @Test @@ -336,8 +336,8 @@ public void shouldSendMultipleRequests() throws JsonProcessingException { // then verify(httpClient, times(2)).request(any(), anyString(), any(), any(byte[].class), anyLong()); - verify(bidRejectionTracker, never()).reject(anyString(), any()); - verify(bidRejectionTracker, never()).reject(anyList(), any()); + verify(bidRejectionTracker, never()).rejectImp(anyString(), any()); + verify(bidRejectionTracker, never()).rejectImps(anyList(), any()); } @Test @@ -369,8 +369,8 @@ public void shouldReturnBidsCreatedByBidder() { // then assertThat(bidderSeatBid.getBids()).hasSameElementsAs(bids); - verify(bidRejectionTracker, never()).reject(anyString(), any()); - verify(bidRejectionTracker, never()).reject(anyList(), any()); + verify(bidRejectionTracker, never()).rejectImp(anyString(), any()); + verify(bidRejectionTracker, never()).rejectImps(anyList(), any()); } @Test @@ -403,8 +403,8 @@ public void shouldReturnBidsCreatedByMakeBids() { // then assertThat(bidderSeatBid.getBids()).hasSameElementsAs(bids); - verify(bidRejectionTracker, never()).reject(anyString(), any()); - verify(bidRejectionTracker, never()).reject(anyList(), any()); + verify(bidRejectionTracker, never()).rejectImp(anyString(), any()); + verify(bidRejectionTracker, never()).rejectImps(anyList(), any()); } @Test @@ -442,8 +442,8 @@ public void shouldReturnFledgeCreatedByBidder() { assertThat(bidderSeatBid.getBids()).hasSameElementsAs(bids); assertThat(bidderSeatBid.getFledgeAuctionConfigs()).hasSameElementsAs(fledgeAuctionConfigs); - verify(bidRejectionTracker, never()).reject(anyString(), any()); - verify(bidRejectionTracker, never()).reject(anyList(), any()); + verify(bidRejectionTracker, never()).rejectImp(anyString(), any()); + verify(bidRejectionTracker, never()).rejectImps(anyList(), any()); } @Test @@ -478,8 +478,8 @@ public void shouldCompressRequestBodyIfContentEncodingHeaderIsGzip() { verify(httpClient).request(any(), anyString(), any(), actualRequestBody.capture(), anyLong()); assertThat(actualRequestBody.getValue()).isNotSameAs(EMPTY_BYTE_BODY); - verify(bidRejectionTracker, never()).reject(anyString(), any()); - verify(bidRejectionTracker, never()).reject(anyList(), any()); + verify(bidRejectionTracker, never()).rejectImp(anyString(), any()); + verify(bidRejectionTracker, never()).rejectImps(anyList(), any()); } @Test @@ -576,8 +576,8 @@ public void processBids(List bids) { assertThat(bidderSeatBid.getBids()).containsOnly(bidderBidDeal1, bidderBidDeal2); - verify(bidRejectionTracker, never()).reject(anyString(), any()); - verify(bidRejectionTracker, never()).reject(anyList(), any()); + verify(bidRejectionTracker, never()).rejectImp(anyString(), any()); + verify(bidRejectionTracker, never()).rejectImps(anyList(), any()); } @Test @@ -625,8 +625,8 @@ public void shouldFinishWhenAllDealRequestsAreFinishedAndNoDealsProvided() { assertThat(bidderSeatBid.getBids()).contains(bidderBid, bidderBid, bidderBid, bidderBid); - verify(bidRejectionTracker, never()).reject(anyString(), any()); - verify(bidRejectionTracker, never()).reject(anyList(), any()); + verify(bidRejectionTracker, never()).rejectImp(anyString(), any()); + verify(bidRejectionTracker, never()).rejectImps(anyList(), any()); } @Test @@ -692,8 +692,8 @@ public void shouldReturnFullDebugInfoIfDebugEnabled() throws JsonProcessingExcep .status(200) .build()); - verify(bidRejectionTracker, never()).reject(anyString(), any()); - verify(bidRejectionTracker, never()).reject(anyList(), any()); + verify(bidRejectionTracker, never()).rejectImp(anyString(), any()); + verify(bidRejectionTracker, never()).rejectImps(anyList(), any()); } @Test @@ -757,8 +757,8 @@ public void shouldReturnRecordBidRejections() throws JsonProcessingException { // then verify(bidRejectionTracker, atLeast(1)).succeed(secondRequestBids); - verify(bidRejectionTracker).reject(singleton("1"), BidRejectionReason.REQUEST_BLOCKED_GENERAL); - verify(bidRejectionTracker).reject(singleton("3"), BidRejectionReason.ERROR_INVALID_BID_RESPONSE); + verify(bidRejectionTracker).rejectImps(singleton("1"), BidRejectionReason.REQUEST_BLOCKED_GENERAL); + verify(bidRejectionTracker).rejectImps(singleton("3"), BidRejectionReason.ERROR_INVALID_BID_RESPONSE); } @Test @@ -802,8 +802,8 @@ public void shouldNotReturnSensitiveHeadersInFullDebugInfo() .extracting(ExtHttpCall::getRequestheaders) .containsExactly(singletonMap("headerKey", singletonList("headerValue"))); - verify(bidRejectionTracker, never()).reject(anyString(), any()); - verify(bidRejectionTracker, never()).reject(anyList(), any()); + verify(bidRejectionTracker, never()).rejectImp(anyString(), any()); + verify(bidRejectionTracker, never()).rejectImps(anyList(), any()); } @Test @@ -850,7 +850,7 @@ public void shouldReturnPartialDebugInfoIfDebugEnabledAndGlobalTimeoutAlreadyExp .requestheaders(singletonMap("headerKey", singletonList("headerValue"))) .build()); - verify(bidRejectionTracker).reject(singleton("impId"), BidRejectionReason.ERROR_TIMED_OUT); + verify(bidRejectionTracker).rejectImps(singleton("impId"), BidRejectionReason.ERROR_TIMED_OUT); } @Test @@ -898,7 +898,7 @@ public void shouldReturnPartialDebugInfoIfDebugEnabledAndHttpErrorOccurs() throw .requestheaders(singletonMap("headerKey", singletonList("headerValue"))) .build()); - verify(bidRejectionTracker).reject(singleton("impId"), BidRejectionReason.ERROR_GENERAL); + verify(bidRejectionTracker).rejectImps(singleton("impId"), BidRejectionReason.ERROR_GENERAL); } @Test @@ -951,7 +951,7 @@ public void shouldReturnFullDebugInfoIfDebugEnabledAndErrorStatus() throws JsonP .extracting(BidderError::getMessage) .containsExactly("Unexpected status code: 500. Run with request.test = 1 for more info"); - verify(bidRejectionTracker).reject(singleton("impId"), BidRejectionReason.ERROR_INVALID_BID_RESPONSE); + verify(bidRejectionTracker).rejectImps(singleton("impId"), BidRejectionReason.ERROR_INVALID_BID_RESPONSE); } @Test @@ -1004,7 +1004,7 @@ public void shouldReturnFullDebugInfoIfDebugEnabledAndBidderIsUnreachable() thro .extracting(BidderError::getMessage) .containsExactly("Unexpected status code: 503. Run with request.test = 1 for more info"); - verify(bidRejectionTracker).reject(singleton("impId"), BidRejectionReason.ERROR_BIDDER_UNREACHABLE); + verify(bidRejectionTracker).rejectImps(singleton("impId"), BidRejectionReason.ERROR_BIDDER_UNREACHABLE); } @Test @@ -1041,7 +1041,7 @@ public void shouldTolerateAlreadyExpiredGlobalTimeout() throws JsonProcessingExc .containsOnly("Timeout has been exceeded"); verifyNoInteractions(httpClient); - verify(bidRejectionTracker).reject(singleton("impId"), BidRejectionReason.ERROR_TIMED_OUT); + verify(bidRejectionTracker).rejectImps(singleton("impId"), BidRejectionReason.ERROR_TIMED_OUT); } @Test @@ -1148,13 +1148,13 @@ public void shouldTolerateMultipleErrors() { BidderError.badServerResponse("Unexpected status code: 404. Run with request.test = 1 for more info"), BidderError.badServerResponse("makeBidsError")); - verify(bidRejectionTracker).reject(singleton("1"), BidRejectionReason.ERROR_GENERAL); - verify(bidRejectionTracker).reject(singleton("2"), BidRejectionReason.ERROR_TIMED_OUT); - verify(bidRejectionTracker).reject(singleton("3"), BidRejectionReason.ERROR_BIDDER_UNREACHABLE); - verify(bidRejectionTracker).reject(singleton("4"), BidRejectionReason.ERROR_INVALID_BID_RESPONSE); - verify(bidRejectionTracker).reject(singleton("5"), BidRejectionReason.ERROR_INVALID_BID_RESPONSE); - verify(bidRejectionTracker, never()).reject(eq(singleton("6")), any()); - verify(bidRejectionTracker, never()).reject(eq(singleton("7")), any()); + verify(bidRejectionTracker).rejectImps(singleton("1"), BidRejectionReason.ERROR_GENERAL); + verify(bidRejectionTracker).rejectImps(singleton("2"), BidRejectionReason.ERROR_TIMED_OUT); + verify(bidRejectionTracker).rejectImps(singleton("3"), BidRejectionReason.ERROR_BIDDER_UNREACHABLE); + verify(bidRejectionTracker).rejectImps(singleton("4"), BidRejectionReason.ERROR_INVALID_BID_RESPONSE); + verify(bidRejectionTracker).rejectImps(singleton("5"), BidRejectionReason.ERROR_INVALID_BID_RESPONSE); + verify(bidRejectionTracker, never()).rejectImps(eq(singleton("6")), any()); + verify(bidRejectionTracker, never()).rejectImps(eq(singleton("7")), any()); } @@ -1187,8 +1187,8 @@ public void shouldNotMakeBidsIfResponseStatusIs204() { verify(bidder, never()).makeBidderResponse(any(), any()); verify(bidder, never()).makeBids(any(), any()); - verify(bidRejectionTracker, never()).reject(anyString(), any()); - verify(bidRejectionTracker, never()).reject(anyList(), any()); + verify(bidRejectionTracker, never()).rejectImp(anyString(), any()); + verify(bidRejectionTracker, never()).rejectImps(anyList(), any()); } private static BidRequest givenBidRequest(UnaryOperator bidRequestCustomizer) { diff --git a/src/test/java/org/prebid/server/floors/BasicPriceFloorEnforcerTest.java b/src/test/java/org/prebid/server/floors/BasicPriceFloorEnforcerTest.java index 61ecb9825ac..724aef1f391 100644 --- a/src/test/java/org/prebid/server/floors/BasicPriceFloorEnforcerTest.java +++ b/src/test/java/org/prebid/server/floors/BasicPriceFloorEnforcerTest.java @@ -347,7 +347,9 @@ public void shouldRejectBidsHavingPriceBelowFloor() { // then verify(priceFloorAdjuster, times(2)).revertAdjustmentForImp(any(), any(), any(), any()); - verify(rejectionTracker).reject("impId1", BidRejectionReason.RESPONSE_REJECTED_BELOW_FLOOR); + final BidderBid rejectedBid = BidderBid.of( + Bid.builder().id("bidId1").impid("impId1").price(BigDecimal.ONE).build(), null, null); + verify(rejectionTracker).rejectBid(rejectedBid, BidRejectionReason.RESPONSE_REJECTED_BELOW_FLOOR); assertThat(singleton(result)) .extracting(AuctionParticipation::getBidderResponse) .extracting(BidderResponse::getSeatBid) diff --git a/src/test/java/org/prebid/server/validation/ResponseBidValidatorTest.java b/src/test/java/org/prebid/server/validation/ResponseBidValidatorTest.java index 2d6d1db8377..ffb6c9e6804 100644 --- a/src/test/java/org/prebid/server/validation/ResponseBidValidatorTest.java +++ b/src/test/java/org/prebid/server/validation/ResponseBidValidatorTest.java @@ -141,8 +141,8 @@ public void validateShouldFailIfBidHasNoCrid() { @Test public void validateShouldFailIfBannerBidHasNoWidthAndHeight() { // when - final ValidationResult result = target.validate( - givenBid(builder -> builder.w(null).h(null)), BIDDER_NAME, givenAuctionContext(), bidderAliases); + final BidderBid givenBid = givenBid(builder -> builder.w(null).h(null)); + final ValidationResult result = target.validate(givenBid, BIDDER_NAME, givenAuctionContext(), bidderAliases); // then assertThat(result.getErrors()) @@ -151,14 +151,14 @@ public void validateShouldFailIfBannerBidHasNoWidthAndHeight() { creative size validation for bid bidId1, account=account, referrer=unknown, \ max imp size='100x200', bid response size='nullxnull'"""); verify(bidRejectionTracker) - .reject("impId1", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_SIZE_NOT_ALLOWED); + .rejectBid(givenBid, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_SIZE_NOT_ALLOWED); } @Test public void validateShouldFailIfBannerBidWidthIsGreaterThanImposedByImp() { // when - final ValidationResult result = target.validate( - givenBid(builder -> builder.w(150).h(150)), BIDDER_NAME, givenAuctionContext(), bidderAliases); + final BidderBid givenBid = givenBid(builder -> builder.w(150).h(150)); + final ValidationResult result = target.validate(givenBid, BIDDER_NAME, givenAuctionContext(), bidderAliases); // then assertThat(result.getErrors()) @@ -167,17 +167,14 @@ public void validateShouldFailIfBannerBidWidthIsGreaterThanImposedByImp() { creative size validation for bid bidId1, account=account, referrer=unknown, \ max imp size='100x200', bid response size='150x150'"""); verify(bidRejectionTracker) - .reject("impId1", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_SIZE_NOT_ALLOWED); + .rejectBid(givenBid, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_SIZE_NOT_ALLOWED); } @Test public void validateShouldFailIfBannerBidHeightIsGreaterThanImposedByImp() { // when - final ValidationResult result = target.validate( - givenBid(builder -> builder.w(50).h(250)), - BIDDER_NAME, - givenAuctionContext(), - bidderAliases); + final BidderBid givenBid = givenBid(builder -> builder.w(50).h(250)); + final ValidationResult result = target.validate(givenBid, BIDDER_NAME, givenAuctionContext(), bidderAliases); // then assertThat(result.getErrors()) @@ -186,7 +183,7 @@ public void validateShouldFailIfBannerBidHeightIsGreaterThanImposedByImp() { creative size validation for bid bidId1, account=account, referrer=unknown, \ max imp size='100x200', bid response size='50x250'"""); verify(bidRejectionTracker) - .reject("impId1", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_SIZE_NOT_ALLOWED); + .rejectBid(givenBid, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_SIZE_NOT_ALLOWED); } @Test @@ -256,8 +253,9 @@ public void validateShouldFailIfBidHasNoCorrespondingImp() { @Test public void validateShouldFailIfBidHasInsecureMarkerInCreativeInSecureContext() { // when + final BidderBid givenBid = givenBid(builder -> builder.adm("http://site.com/creative.jpg")); final ValidationResult result = target.validate( - givenBid(builder -> builder.adm("http://site.com/creative.jpg")), + givenBid, BIDDER_NAME, givenAuctionContext(givenBidRequest(builder -> builder.secure(1))), bidderAliases); @@ -269,14 +267,15 @@ public void validateShouldFailIfBidHasInsecureMarkerInCreativeInSecureContext() secure creative validation for bid bidId1, account=account, referrer=unknown, \ adm=http://site.com/creative.jpg"""); verify(bidRejectionTracker) - .reject("impId1", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_NOT_SECURE); + .rejectBid(givenBid, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_NOT_SECURE); } @Test public void validateShouldFailIfBidHasInsecureEncodedMarkerInCreativeInSecureContext() { // when + final BidderBid givenBid = givenBid(builder -> builder.adm("http%3A//site.com/creative.jpg")); final ValidationResult result = target.validate( - givenBid(builder -> builder.adm("http%3A//site.com/creative.jpg")), + givenBid, BIDDER_NAME, givenAuctionContext(givenBidRequest(builder -> builder.secure(1))), bidderAliases); @@ -288,14 +287,15 @@ public void validateShouldFailIfBidHasInsecureEncodedMarkerInCreativeInSecureCon secure creative validation for bid bidId1, account=account, referrer=unknown, \ adm=http%3A//site.com/creative.jpg"""); verify(bidRejectionTracker) - .reject("impId1", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_NOT_SECURE); + .rejectBid(givenBid, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_NOT_SECURE); } @Test public void validateShouldFailIfBidHasNoSecureMarkersInCreativeInSecureContext() { // when + final BidderBid givenBid = givenBid(builder -> builder.adm("//site.com/creative.jpg")); final ValidationResult result = target.validate( - givenBid(builder -> builder.adm("//site.com/creative.jpg")), + givenBid, BIDDER_NAME, givenAuctionContext(givenBidRequest(builder -> builder.secure(1))), bidderAliases); @@ -307,7 +307,7 @@ public void validateShouldFailIfBidHasNoSecureMarkersInCreativeInSecureContext() secure creative validation for bid bidId1, account=account, referrer=unknown, \ adm=//site.com/creative.jpg"""); verify(bidRejectionTracker) - .reject("impId1", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_NOT_SECURE); + .rejectBid(givenBid, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_NOT_SECURE); } @Test @@ -405,8 +405,9 @@ public void validateShouldReturnSuccessWithWarningIfBannerSizeEnforcementIsWarn( target = new ResponseBidValidator(warn, enforce, metrics, 0.01); // when + final BidderBid givenBid = givenBid(builder -> builder.w(null).h(null)); final ValidationResult result = target.validate( - givenBid(builder -> builder.w(null).h(null)), + givenBid, BIDDER_NAME, givenAuctionContext(), bidderAliases); @@ -418,8 +419,7 @@ public void validateShouldReturnSuccessWithWarningIfBannerSizeEnforcementIsWarn( BidResponse validation `warn`: bidder `bidder` response triggers \ creative size validation for bid bidId1, account=account, referrer=unknown, \ max imp size='100x200', bid response size='nullxnull'"""); - verify(bidRejectionTracker) - .reject("impId1", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_SIZE_NOT_ALLOWED); + verifyNoInteractions(bidRejectionTracker); } @Test @@ -445,8 +445,9 @@ public void validateShouldReturnSuccessWithWarningIfSecureMarkupEnforcementIsWar target = new ResponseBidValidator(enforce, warn, metrics, 0.01); // when + final BidderBid givenBid = givenBid(builder -> builder.adm("http://site.com/creative.jpg")); final ValidationResult result = target.validate( - givenBid(builder -> builder.adm("http://site.com/creative.jpg")), + givenBid, BIDDER_NAME, givenAuctionContext(givenBidRequest(builder -> builder.secure(1))), bidderAliases); @@ -458,23 +459,19 @@ public void validateShouldReturnSuccessWithWarningIfSecureMarkupEnforcementIsWar BidResponse validation `warn`: bidder `bidder` response triggers \ secure creative validation for bid bidId1, account=account, referrer=unknown, \ adm=http://site.com/creative.jpg"""); - verify(bidRejectionTracker) - .reject("impId1", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_NOT_SECURE); + verifyNoInteractions(bidRejectionTracker); } @Test public void validateShouldIncrementSizeValidationErrMetrics() { // when - target.validate( - givenBid(builder -> builder.w(150).h(200)), - BIDDER_NAME, - givenAuctionContext(), - bidderAliases); + final BidderBid givenBid = givenBid(builder -> builder.w(150).h(200)); + target.validate(givenBid, BIDDER_NAME, givenAuctionContext(), bidderAliases); // then verify(metrics).updateSizeValidationMetrics(BIDDER_NAME, ACCOUNT_ID, MetricName.err); verify(bidRejectionTracker) - .reject("impId1", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_SIZE_NOT_ALLOWED); + .rejectBid(givenBid, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_SIZE_NOT_ALLOWED); } @Test @@ -483,23 +480,20 @@ public void validateShouldIncrementSizeValidationWarnMetrics() { target = new ResponseBidValidator(warn, warn, metrics, 0.01); // when - target.validate( - givenBid(builder -> builder.w(150).h(200)), - BIDDER_NAME, - givenAuctionContext(), - bidderAliases); + final BidderBid givenBid = givenBid(builder -> builder.w(150).h(200)); + target.validate(givenBid, BIDDER_NAME, givenAuctionContext(), bidderAliases); // then verify(metrics).updateSizeValidationMetrics(BIDDER_NAME, ACCOUNT_ID, MetricName.warn); - verify(bidRejectionTracker) - .reject("impId1", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_SIZE_NOT_ALLOWED); + verifyNoInteractions(bidRejectionTracker); } @Test public void validateShouldIncrementSecureValidationErrMetrics() { // when + final BidderBid givenBid = givenBid(builder -> builder.adm("http://site.com/creative.jpg")); target.validate( - givenBid(builder -> builder.adm("http://site.com/creative.jpg")), + givenBid, BIDDER_NAME, givenAuctionContext(givenBidRequest(builder -> builder.secure(1))), bidderAliases); @@ -507,7 +501,7 @@ public void validateShouldIncrementSecureValidationErrMetrics() { // then verify(metrics).updateSecureValidationMetrics(BIDDER_NAME, ACCOUNT_ID, MetricName.err); verify(bidRejectionTracker) - .reject("impId1", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_NOT_SECURE); + .rejectBid(givenBid, BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_NOT_SECURE); } @Test @@ -516,16 +510,16 @@ public void validateShouldIncrementSecureValidationWarnMetrics() { target = new ResponseBidValidator(warn, warn, metrics, 0.01); // when + final BidderBid givenBid = givenBid(builder -> builder.adm("http://site.com/creative.jpg")); target.validate( - givenBid(builder -> builder.adm("http://site.com/creative.jpg")), + givenBid, BIDDER_NAME, givenAuctionContext(givenBidRequest(builder -> builder.secure(1))), bidderAliases); // then verify(metrics).updateSecureValidationMetrics(BIDDER_NAME, ACCOUNT_ID, MetricName.warn); - verify(bidRejectionTracker) - .reject("impId1", BidRejectionReason.RESPONSE_REJECTED_INVALID_CREATIVE_NOT_SECURE); + verifyNoInteractions(bidRejectionTracker); } private BidRequest givenRequest(UnaryOperator impCustomizer) {