Add bidder error bid validation type (#1107)

src/main/java/org/prebid/server/auction/ExchangeService.java

@@ -108,6 +108,7 @@
 import org.prebid.server.settings.model.Account;
 import org.prebid.server.util.DealUtil;
 import org.prebid.server.util.LineItemUtil;
+import org.prebid.server.util.ObjectUtil;
 import org.prebid.server.util.StreamUtil;
 import org.prebid.server.validation.ResponseBidValidator;
 import org.prebid.server.validation.model.ValidationResult;
@@ -127,6 +128,7 @@
 import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
  * Executes an OpenRTB v2.5 Auction.
@@ -1297,17 +1299,18 @@ private AuctionParticipation validBidderResponse(AuctionParticipation auctionPar
             final ValidationResult validationResult =
                     responseBidValidator.validate(bid, bidderResponse.getBidder(), auctionContext, aliases);
 
-            if (validationResult.hasWarnings()) {
-                addAsBidderErrors(validationResult.getWarnings(), errors);
+            if (validationResult.hasWarnings() || validationResult.hasErrors()) {
+                errors.add(makeValidationBidderError(bid.getBid(), validationResult));
             }
 
             if (validationResult.hasErrors()) {
-                addAsBidderErrors(validationResult.getErrors(), errors);
                 maybeRecordInTxnLog(lineItemId, txnLog::lineItemsResponseInvalidated);
                 continue;
             }
 
-            validBids.add(bid);
+            if (!validationResult.hasErrors()) {
+                validBids.add(bid);
+            }
         }
 
         final BidderResponse resultBidderResponse = errors.isEmpty()
@@ -1316,8 +1319,14 @@ private AuctionParticipation validBidderResponse(AuctionParticipation auctionPar
         return auctionParticipation.with(resultBidderResponse);
     }
 
-    private void addAsBidderErrors(List<String> messages, List<BidderError> errors) {
-        messages.stream().map(BidderError::generic).forEach(errors::add);
+    private BidderError makeValidationBidderError(Bid bid, ValidationResult validationResult) {
+        final String validationErrors = Stream.concat(
+                validationResult.getErrors().stream().map(message -> "Error: " + message),
+                validationResult.getWarnings().stream().map(message -> "Warning: " + message))
+                .collect(Collectors.joining(". "));
+
+        final String bidId = ObjectUtil.getIfNotNullOrDefault(bid, Bid::getId, () -> "unknown");
+        return BidderError.invalidBid("BidId `" + bidId + "` validation messages: " + validationErrors);
     }
 
     private static void maybeRecordInTxnLog(String lineItemId, Supplier<Set<String>> metricSupplier) {
@@ -1587,6 +1596,9 @@ private static MetricName bidderErrorTypeToMetric(BidderError.Type errorType) {
             case timeout:
                 errorMetric = MetricName.timeout;
                 break;
+            case invalid_bid:
+                errorMetric = MetricName.bid_validation;
+                break;
             case generic:
             default:
                 errorMetric = MetricName.unknown_error;

src/main/java/org/prebid/server/bidder/model/BidderError.java

@@ -22,6 +22,10 @@ public static BidderError generic(String message) {
         return BidderError.of(message, Type.generic);
     }
 
+    public static BidderError invalidBid(String message) {
+        return BidderError.of(message, Type.invalid_bid);
+    }
+
     public static BidderError badInput(String message) {
         return BidderError.of(message, Type.bad_input);
     }
@@ -69,6 +73,12 @@ public enum Type {
          */
         failed_to_request_bids(4),
 
+        /**
+         * Covers the case where a bid does not pass validation with error or warnings. One instance per invalid bid
+         * created with aggregation for all warnings and errors.
+         */
+        invalid_bid(5),
+
         timeout(1),
         generic(999);
 

src/main/java/org/prebid/server/metric/MetricName.java

@@ -64,6 +64,7 @@ public enum MetricName {
     badserverresponse,
     failedtorequestbids,
     timeout,
+    bid_validation,
     unknown_error,
     err,
     networkerr,

src/test/groovy/org/prebid/server/functional/tests/BidValidationSpec.groovy

@@ -190,4 +190,25 @@ class BidValidationSpec extends BaseSpec {
         null                          | PBSUtils.randomNumber
         null                          | null
     }
+
+    def "PBS should update 'adapter.generic.requests.bid_validation' metric when bid validation error appears"() {
+        given: "Initial 'adapter.generic.requests.bid_validation' metric value"
+        def initialMetricValue = getCurrentMetricValue("adapter.generic.requests.bid_validation")
+
+        and: "Bid request"
+        def bidRequest = BidRequest.defaultBidRequest
+
+        and: "Set invalid bid response"
+        def bidResponse = BidResponse.getDefaultBidResponse(bidRequest).tap {
+            seatbid[0].bid[0].impid = PBSUtils.randomNumber as String
+        }
+        bidder.setResponse(bidRequest.id, bidResponse)
+
+        when: "Sending auction request to PBS"
+        defaultPbsService.sendAuctionRequest(bidRequest)
+
+        then: "Bid validation metric value is incremented"
+        def metrics = defaultPbsService.sendCollectedMetricsRequest()
+        assert metrics["adapter.generic.requests.bid_validation"] == initialMetricValue + 1
+    }
 }

src/test/groovy/org/prebid/server/functional/tests/pg/PgBidResponseSpec.groovy

@@ -61,9 +61,12 @@ class PgBidResponseSpec extends BasePgSpec {
         def auctionResponse = pgPbsService.sendAuctionRequest(bidRequest)
 
         then: "Bidder response is invalid"
-        def bidderError = auctionResponse.ext?.errors?.get(GENERIC)
-        assert bidderError?.size() == 1
-        assert bidderError[0].message.startsWith("Bid \"${bidResponse.seatbid[0].bid[0].id}\" has no corresponding imp in request")
+        def bidderErrors = auctionResponse.ext?.errors?.get(GENERIC)
+        def bidId = bidResponse.seatbid[0].bid[0].id
+
+        assert bidderErrors?.size() == 1
+        assert bidderErrors[0].message ==~
+                /BidId `$bidId` validation messages:.* Error: Bid \"$bidId\" has no corresponding imp in request.*/
     }
 
     def "PBS should invalidate bidder response when deal id doesn't match to the bid request deal id"() {
@@ -87,9 +90,12 @@ class PgBidResponseSpec extends BasePgSpec {
         def auctionResponse = pgPbsService.sendAuctionRequest(bidRequest)
 
         then: "Bidder response is invalid"
-        def bidderError = auctionResponse.ext?.errors?.get(GENERIC)
-        assert bidderError?.size() == 1
-        assert bidderError[0].message.startsWith("WARNING: Bid \"${bidResponse.seatbid[0].bid[0].id}\" has 'dealid' not present in corresponding imp in request.")
+        def bidderErrors = auctionResponse.ext?.errors?.get(GENERIC)
+        def bidId = bidResponse.seatbid[0].bid[0].id
+
+        assert bidderErrors?.size() == 1
+        assert bidderErrors[0].message ==~ /BidId `$bidId` validation messages:.* Warning: / +
+                /WARNING: Bid "$bidId" has 'dealid' not present in corresponding imp in request.*/
     }
 
     def "PBS should invalidate bidder response when non-matched to the bid request size is returned"() {
@@ -120,10 +126,12 @@ class PgBidResponseSpec extends BasePgSpec {
         assert auctionResponse.ext?.debug?.pgmetrics?.responseInvalidated?.size() == plansResponse.lineItems.size()
 
         and: "PBS invalidated response as unmatched by size"
-        def bidderError = auctionResponse.ext?.errors?.get(GENERIC)
-        assert bidderError?.size() == 1
-        assert bidderError[0].message == "Bid \"$bid.id\" has 'w' and 'h' not supported by " +
-                "corresponding imp in request. Bid dimensions: '${bid.w}x$bid.h', formats in imp: '${impFormat.w}x$impFormat.h'"
+        def bidderErrors = auctionResponse.ext?.errors?.get(GENERIC)
+
+        assert bidderErrors?.size() == 1
+        assert bidderErrors[0].message ==~ /BidId `$bid.id` validation messages:.* Error: / +
+                /Bid "$bid.id" has 'w' and 'h' not supported by corresponding imp in request\. / +
+                /Bid dimensions: '${bid.w}x$bid.h', formats in imp: '${impFormat.w}x$impFormat.h'.*/
     }
 
     def "PBS should invalidate bidder response when non-matched to the PBS line item size response is returned"() {
@@ -158,9 +166,11 @@ class PgBidResponseSpec extends BasePgSpec {
         assert auctionResponse.ext?.debug?.pgmetrics?.responseInvalidated?.size() == plansResponse.lineItems.size()
 
         and: "PBS invalidated response as not matched by size"
-        def bidderError = auctionResponse.ext?.errors?.get(GENERIC)
-        assert bidderError?.size() == 1
-        assert bidderError[0].message == "Bid \"$bid.id\" has 'w' and 'h' not matched to Line Item. " +
-                "Bid dimensions: '${bid.w}x$bid.h', Line Item sizes: '${lineItemSize.w}x$lineItemSize.h'"
+        def bidderErrors = auctionResponse.ext?.errors?.get(GENERIC)
+
+        assert bidderErrors?.size() == 1
+        assert bidderErrors[0].message ==~ /BidId `$bid.id` validation messages:.* Error: / +
+                /Bid "$bid.id" has 'w' and 'h' not matched to Line Item\. / +
+                /Bid dimensions: '${bid.w}x$bid.h', Line Item sizes: '${lineItemSize.w}x$lineItemSize.h'.*/
     }
 }

src/test/groovy/org/prebid/server/functional/tests/pg/PgDealsOnlySpec.groovy

@@ -102,7 +102,6 @@ class PgDealsOnlySpec extends BasePgSpec {
         def bidResponse = BidResponse.getDefaultBidResponse(bidRequest)
         bidResponse.seatbid[0].bid[0].dealid = null
         bidder.setResponse(bidRequest.id, bidResponse)
-        def bidResponseId = bidResponse.seatbid[0].bid[0].id
 
         and: "Line items are fetched by PBS"
         updateLineItemsAndWait()
@@ -115,9 +114,12 @@ class PgDealsOnlySpec extends BasePgSpec {
 
         and: "PBS returns an error of missing 'dealid' field in bid"
         def bidErrors = auctionResponse.ext?.errors?.get(GENERIC)
+        def bidId = bidResponse.seatbid[0].bid[0].id
+
         assert bidErrors?.size() == 1
-        assert bidErrors[0].code == 999
-        assert bidErrors[0].message == "Bid \"$bidResponseId\" missing required field 'dealid'"
+        assert bidErrors[0].code == 5
+        assert bidErrors[0].message ==~ /BidId `$bidId` validation messages:.* Error: / +
+                /Bid "$bidId" missing required field 'dealid'.*/
     }
 
     def "PBS should add dealsonly flag when it is not specified and pgdealsonly flag is set to true"() {
@@ -133,7 +135,6 @@ class PgDealsOnlySpec extends BasePgSpec {
         def bidResponse = BidResponse.getDefaultBidResponse(bidRequest)
         bidResponse.seatbid[0].bid[0].dealid = null
         bidder.setResponse(bidRequest.id, bidResponse)
-        def bidResponseId = bidResponse.seatbid[0].bid[0].id
 
         and: "Line items are fetched by PBS"
         updateLineItemsAndWait()
@@ -149,9 +150,12 @@ class PgDealsOnlySpec extends BasePgSpec {
 
         and: "PBS returns an error of missing 'dealid' field in bid"
         def bidErrors = auctionResponse.ext?.errors?.get(GENERIC)
+        def bidId = bidResponse.seatbid[0].bid[0].id
+
         assert bidErrors?.size() == 1
-        assert bidErrors[0].code == 999
-        assert bidErrors[0].message == "Bid \"$bidResponseId\" missing required field 'dealid'"
+        assert bidErrors[0].code == 5
+        assert bidErrors[0].message ==~ /BidId `$bidId` validation messages:.* Error: / +
+                /Bid "$bidId" missing required field 'dealid'.*/
     }
 
     def "PBS shouldn't return an error when bidder dealsonly flag is set to true, no available PG line items and bid response misses 'dealid' field"() {

src/test/java/org/prebid/server/auction/ExchangeServiceTest.java

@@ -1363,8 +1363,8 @@ public void shouldTolerateResponseBidValidationErrors() {
                 .extracting(BidderResponse::getSeatBid)
                 .flatExtracting(BidderSeatBid::getErrors)
                 .containsOnly(
-                        BidderError.generic("bid validation warning"),
-                        BidderError.generic("bid validation error"));
+                        BidderError.invalidBid("BidId `bidId1` validation messages: Error: bid validation error."
+                                + " Warning: bid validation warning"));
     }
 
     @Test
@@ -1400,7 +1400,8 @@ public void shouldTolerateResponseBidValidationWarnings() {
                 .extracting(AuctionParticipation::getBidderResponse)
                 .extracting(BidderResponse::getSeatBid)
                 .flatExtracting(BidderSeatBid::getErrors)
-                .containsOnly(BidderError.generic("bid validation warning"));
+                .containsOnly(BidderError.invalidBid(
+                        "BidId `bidId1` validation messages: Warning: bid validation warning"));
     }
 
     @Test