Fix bidder json schema validation warnings collection mechanism

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

@@ -156,12 +156,13 @@ private static ExtAmpVideoResponse extResponseFrom(BidResponse bidResponse) {
         final ExtResponseDebug extDebug = ext != null ? ext.getDebug() : null;
 
         final Map<String, List<ExtBidderError>> extErrors = ext != null ? ext.getErrors() : null;
+        final Map<String, List<ExtBidderError>> extWarnings = ext != null ? ext.getWarnings() : null;
 
         final ExtModules extModules = extPrebid != null ? extPrebid.getModules() : null;
         final ExtAmpVideoPrebid extAmpVideoPrebid = extModules != null ? ExtAmpVideoPrebid.of(extModules) : null;
 
-        return ObjectUtils.anyNotNull(extDebug, extErrors, extAmpVideoPrebid)
-                ? ExtAmpVideoResponse.of(extDebug, extErrors, extAmpVideoPrebid)
+        return ObjectUtils.anyNotNull(extDebug, extErrors, extWarnings, extAmpVideoPrebid)
+                ? ExtAmpVideoResponse.of(extDebug, extErrors, extWarnings, extAmpVideoPrebid)
                 : null;
     }
 

src/main/java/org/prebid/server/auction/requestfactory/AmpRequestFactory.java

@@ -335,7 +335,8 @@ private Future<BidRequest> updateBidRequest(AuctionContext auctionContext) {
                 .map(this::fillExplicitParameters)
                 .map(bidRequest -> overrideParameters(bidRequest, httpRequest, auctionContext.getPrebidErrors()))
                 .map(bidRequest -> paramsResolver.resolve(bidRequest, httpRequest, timeoutResolver, ENDPOINT))
-                .map(ortb2RequestFactory::validateRequest);
+                .compose(resolvedBidRequest ->
+                        ortb2RequestFactory.validateRequest(resolvedBidRequest, auctionContext.getDebugWarnings()));
     }
 
     private static String storedRequestId(BidRequest receivedBidRequest) {

src/main/java/org/prebid/server/auction/requestfactory/AuctionRequestFactory.java

@@ -96,7 +96,7 @@ public Future<AuctionContext> fromRequest(RoutingContext routingContext, long st
                 .compose(auctionContext -> ortb2RequestFactory.executeRawAuctionRequestHooks(auctionContext)
                         .map(auctionContext::with))
 
-                .compose(auctionContext -> updateBidRequest(auctionContext)
+                .compose(auctionContext -> updateAndValidateBidRequest(auctionContext)
                         .map(auctionContext::with))
 
                 .compose(auctionContext -> privacyEnforcementService.contextFromBidRequest(auctionContext)
@@ -160,15 +160,18 @@ private BidRequest jsonNodeAsBidRequest(JsonNode bidRequestNode) {
      * Sets {@link BidRequest} properties which were not set explicitly by the client, but can be
      * updated by values derived from headers and other request attributes.
      */
-    private Future<BidRequest> updateBidRequest(AuctionContext auctionContext) {
+    private Future<BidRequest> updateAndValidateBidRequest(AuctionContext auctionContext) {
         final Account account = auctionContext.getAccount();
         final BidRequest bidRequest = auctionContext.getBidRequest();
         final HttpRequestContext httpRequest = auctionContext.getHttpRequest();
 
         return storedRequestProcessor.processStoredRequests(account.getId(), bidRequest)
                 .map(resolvedBidRequest ->
                         paramsResolver.resolve(resolvedBidRequest, httpRequest, timeoutResolver, ENDPOINT))
-                .map(ortb2RequestFactory::validateRequest)
+
+                .compose(resolvedBidRequest ->
+                        ortb2RequestFactory.validateRequest(resolvedBidRequest, auctionContext.getDebugWarnings()))
+
                 .map(interstitialProcessor::process);
     }
 

src/main/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactory.java

@@ -158,15 +158,16 @@ private Future<Account> fetchAccount(AuctionContext auctionContext, boolean isLo
                 .compose(accountId -> loadAccount(timeout, httpRequest, accountId));
     }
 
-    /**
-     * Performs thorough validation of fully constructed {@link BidRequest} that is going to be used to hold an auction.
-     */
-    public BidRequest validateRequest(BidRequest bidRequest) {
+    public Future<BidRequest> validateRequest(BidRequest bidRequest, List<String> warnings) {
         final ValidationResult validationResult = requestValidator.validate(bidRequest);
-        if (validationResult.hasErrors()) {
-            throw new InvalidRequestException(validationResult.getErrors());
+
+        if (validationResult.hasWarnings()) {
+            warnings.addAll(validationResult.getWarnings());
         }
-        return bidRequest;
+
+        return validationResult.hasErrors()
+                ? Future.failedFuture(new InvalidRequestException(validationResult.getErrors()))
+                : Future.succeededFuture(bidRequest);
     }
 
     public BidRequest enrichBidRequestWithAccountAndPrivacyData(AuctionContext auctionContext) {
@@ -195,10 +196,10 @@ public Future<HttpRequestContext> executeEntrypointHooks(RoutingContext routingC
                                                              AuctionContext auctionContext) {
 
         return hookStageExecutor.executeEntrypointStage(
-                        toCaseInsensitiveMultiMap(routingContext.queryParams()),
-                        toCaseInsensitiveMultiMap(routingContext.request().headers()),
-                        body,
-                        auctionContext.getHookExecutionContext())
+                toCaseInsensitiveMultiMap(routingContext.queryParams()),
+                toCaseInsensitiveMultiMap(routingContext.request().headers()),
+                body,
+                auctionContext.getHookExecutionContext())
                 .map(stageResult -> toHttpRequest(stageResult, routingContext, auctionContext));
     }
 

src/main/java/org/prebid/server/auction/requestfactory/VideoRequestFactory.java

@@ -103,13 +103,17 @@ public Future<WithPodErrors<AuctionContext>> fromRequest(RoutingContext routingC
                 Endpoint.openrtb2_video, MetricName.video);
 
         return ortb2RequestFactory.executeEntrypointHooks(routingContext, body, initialAuctionContext)
-                .compose(httpRequest -> createBidRequest(httpRequest)
-                        .map(bidRequestWithErrors -> populatePodErrors(
-                                bidRequestWithErrors.getPodErrors(), podErrors, bidRequestWithErrors))
-                        .map(bidRequestWithErrors -> {
-                            return ortb2RequestFactory.enrichAuctionContext(
-                                    initialAuctionContext, httpRequest, bidRequestWithErrors.getData(), startTime);
-                        }))
+                .compose(httpRequest ->
+                        createBidRequest(httpRequest)
+
+                                .compose(bidRequest ->
+                                        validateRequest(bidRequest, initialAuctionContext.getDebugWarnings()))
+
+                                .map(bidRequestWithErrors -> populatePodErrors(
+                                        bidRequestWithErrors.getPodErrors(), podErrors, bidRequestWithErrors))
+
+                                .map(bidRequestWithErrors -> ortb2RequestFactory.enrichAuctionContext(
+                                        initialAuctionContext, httpRequest, bidRequestWithErrors.getData(), startTime)))
 
                 .compose(auctionContext -> ortb2RequestFactory.fetchAccountWithoutStoredRequestLookup(auctionContext)
                         .map(auctionContext::with))
@@ -164,11 +168,10 @@ private Future<WithPodErrors<BidRequest>> createBidRequest(HttpRequestContext ht
         }
 
         final Set<String> podConfigIds = podConfigIds(bidRequestVideo);
+        final String accountId = accountIdFrom(bidRequestVideo);
 
-        return storedRequestProcessor.processVideoRequest(
-                        accountIdFrom(bidRequestVideo), storedRequestId, podConfigIds, bidRequestVideo)
-                .map(bidRequestToErrors
-                        -> fillImplicitParametersAndValidate(httpRequest, bidRequestToErrors, debugEnabled));
+        return storedRequestProcessor.processVideoRequest(accountId, storedRequestId, podConfigIds, bidRequestVideo)
+                .map(bidRequestToErrors -> fillImplicitParameters(httpRequest, bidRequestToErrors, debugEnabled));
     }
 
     private AuctionContext updateContextWithDebugLog(AuctionContext auctionContext) {
@@ -274,16 +277,23 @@ private static <T> T populatePodErrors(List<PodError> from, List<PodError> to, T
         return returnObject;
     }
 
-    private WithPodErrors<BidRequest> fillImplicitParametersAndValidate(HttpRequestContext httpRequest,
-                                                                        WithPodErrors<BidRequest> bidRequestToErrors,
-                                                                        boolean debugEnabled) {
+    private WithPodErrors<BidRequest> fillImplicitParameters(HttpRequestContext httpRequest,
+                                                             WithPodErrors<BidRequest> bidRequestToErrors,
+                                                             boolean debugEnabled) {
+
         final BidRequest bidRequest = bidRequestToErrors.getData();
-        BidRequest updatedBidRequest = paramsResolver.resolve(bidRequest, httpRequest, timeoutResolver, ENDPOINT);
-        if (debugEnabled) {
-            updatedBidRequest = updatedBidRequest.toBuilder().test(1).build();
-        }
-        final BidRequest validBidRequest = ortb2RequestFactory.validateRequest(updatedBidRequest);
+        final BidRequest updatedBidRequest = paramsResolver.resolve(bidRequest, httpRequest, timeoutResolver, ENDPOINT);
+        final BidRequest updatedWithDebugBidRequest = debugEnabled
+                ? updatedBidRequest.toBuilder().test(1).build()
+                : updatedBidRequest;
+
+        return WithPodErrors.of(updatedWithDebugBidRequest, bidRequestToErrors.getPodErrors());
+    }
+
+    private Future<WithPodErrors<BidRequest>> validateRequest(WithPodErrors<BidRequest> requestWithPodErrors,
+                                                              List<String> warnings) {
 
-        return WithPodErrors.of(validBidRequest, bidRequestToErrors.getPodErrors());
+        return ortb2RequestFactory.validateRequest(requestWithPodErrors.getData(), warnings)
+                .map(bidRequest -> requestWithPodErrors);
     }
 }

src/main/java/org/prebid/server/exception/InvalidRequestException.java

@@ -1,9 +1,11 @@
 package org.prebid.server.exception;
 
 import lombok.Getter;
+import org.apache.commons.collections4.CollectionUtils;
 
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 
 public class InvalidRequestException extends RuntimeException {
 

src/main/java/org/prebid/server/handler/openrtb2/AmpHandler.java

@@ -235,12 +235,13 @@ private static ExtAmpVideoResponse extResponseFrom(BidResponse bidResponse) {
         final ExtResponseDebug extDebug = ext != null ? ext.getDebug() : null;
 
         final Map<String, List<ExtBidderError>> extErrors = ext != null ? ext.getErrors() : null;
+        final Map<String, List<ExtBidderError>> extWarnings = ext != null ? ext.getWarnings() : null;
 
         final ExtModules extModules = extPrebid != null ? extPrebid.getModules() : null;
         final ExtAmpVideoPrebid extAmpVideoPrebid = extModules != null ? ExtAmpVideoPrebid.of(extModules) : null;
 
-        return ObjectUtils.anyNotNull(extDebug, extErrors, extAmpVideoPrebid)
-                ? ExtAmpVideoResponse.of(extDebug, extErrors, extAmpVideoPrebid)
+        return ObjectUtils.anyNotNull(extDebug, extErrors, extWarnings, extAmpVideoPrebid)
+                ? ExtAmpVideoResponse.of(extDebug, extErrors, extWarnings, extAmpVideoPrebid)
                 : null;
     }
 

src/main/java/org/prebid/server/proto/response/ExtAmpVideoResponse.java

@@ -14,5 +14,7 @@ public class ExtAmpVideoResponse {
 
     Map<String, List<ExtBidderError>> errors;
 
+    Map<String, List<ExtBidderError>> warnings;
+
     ExtAmpVideoPrebid prebid;
 }

src/main/java/org/prebid/server/validation/RequestValidator.java

@@ -954,14 +954,16 @@ private void validateImpExtPrebidBidder(JsonNode extPrebidBidder,
                 validateImpBidderExtName(impIndex, bidderExtension, aliases.getOrDefault(bidder, bidder));
             } catch (ValidationException ex) {
                 bidderExtensions.remove();
-                warnings.add(String.format("WARNING: request.imp[%d].ext.prebid.%s was dropped with a reason: %s",
-                        impIndex, bidder, ex.getMessage()));
+                warnings.add(
+                        String.format(
+                                "WARNING: request.imp[%d].ext.prebid.bidder.%s was dropped with a reason: %s",
+                                impIndex, bidder, ex.getMessage()));
             }
         }
 
         if (extPrebidBidder.size() == 0) {
-            warnings.add(String.format("WARNING: request.imp[%d].ext must contain at least one valid bidder",
-                    impIndex));
+            warnings.add(
+                    String.format("WARNING: request.imp[%d].ext must contain at least one valid bidder", impIndex));
         }
     }
 

src/test/groovy/org/prebid/server/functional/model/response/amp/AmpResponseExt.groovy

@@ -8,4 +8,5 @@ class AmpResponseExt {
 
     Debug debug
     Map<ErrorType, List<BidderError>> errors
+    Map<ErrorType, List<BidderError>> warnings
 }

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

@@ -17,12 +17,9 @@ import org.prebid.server.functional.model.response.auction.ErrorType
 import org.prebid.server.functional.testcontainers.PBSTest
 import org.prebid.server.functional.util.PBSUtils
 import org.prebid.server.functional.util.privacy.CcpaConsent
-import spock.lang.PendingFeature
-import spock.lang.Unroll
 
 import static org.prebid.server.functional.model.bidder.BidderName.APPNEXUS
 import static org.prebid.server.functional.util.privacy.CcpaConsent.Signal.ENFORCED
-import static org.prebid.server.functional.model.response.auction.ErrorType.PREBID
 
 @PBSTest
 class BidderParamsSpec extends BaseSpec {
@@ -275,7 +272,6 @@ class BidderParamsSpec extends BaseSpec {
         assert response.ext?.errors[ErrorType.GENERIC]*.message == ["no empty host accepted"]
     }
 
-    @PendingFeature
     def "PBS should reject bidder when bidder params from request doesn't satisfy json-schema for auction request"() {
         given: "BidRequest with bad bidder datatype"
         def bidRequest = BidRequest.defaultBidRequest.tap {
@@ -286,10 +282,10 @@ class BidderParamsSpec extends BaseSpec {
         def response = defaultPbsService.sendAuctionRequest(bidRequest)
 
         then: "Bidder should be dropped"
-        assert response.ext?.errors[ErrorType.GENERIC]*.code == [999]
-        assert response.ext?.errors[ErrorType.GENERIC]*.message ==
+        assert response.ext?.warnings[ErrorType.PREBID]*.code == [999, 999]
+        assert response.ext?.warnings[ErrorType.PREBID]*.message ==
                 ["WARNING: request.imp[0].ext.prebid.bidder.generic was dropped with a reason: " +
-                         "request.imp[0].ext.prebid.bidder.generic failed validation" +
+                         "request.imp[0].ext.prebid.bidder.generic failed validation.\n" +
                          "\$.exampleProperty: integer found, string expected",
                  "WARNING: request.imp[0].ext must contain at least one valid bidder"]
 
@@ -300,7 +296,6 @@ class BidderParamsSpec extends BaseSpec {
         assert response.seatbid.isEmpty()
     }
 
-    @PendingFeature
     def "PBS should reject bidder when bidder params from stored request doesn't satisfy json-schema for auction request"() {
         given: "BidRequest with stored request, without imp"
         def bidRequest = BidRequest.defaultBidRequest.tap {
@@ -321,10 +316,10 @@ class BidderParamsSpec extends BaseSpec {
         def response = defaultPbsService.sendAuctionRequest(bidRequest)
 
         then: "Bidder should be dropped"
-        assert response.ext?.errors[ErrorType.GENERIC]*.code == [999]
-        assert response.ext?.errors[ErrorType.GENERIC]*.message ==
+        assert response.ext?.warnings[ErrorType.PREBID]*.code == [999, 999]
+        assert response.ext?.warnings[ErrorType.PREBID]*.message ==
                 ["WARNING: request.imp[0].ext.prebid.bidder.generic was dropped with a reason: " +
-                         "request.imp[0].ext.prebid.bidder.generic failed validation" +
+                         "request.imp[0].ext.prebid.bidder.generic failed validation.\n" +
                          "\$.exampleProperty: integer found, string expected",
                  "WARNING: request.imp[0].ext must contain at least one valid bidder"]
 
@@ -335,7 +330,6 @@ class BidderParamsSpec extends BaseSpec {
         assert response.seatbid.isEmpty()
     }
 
-    @PendingFeature
     def "PBS should reject bidder when bidder params from stored request doesn't satisfy json-schema for amp request"() {
         given: "AmpRequest with bad bidder datatype"
         def ampRequest = AmpRequest.defaultAmpRequest
@@ -352,10 +346,10 @@ class BidderParamsSpec extends BaseSpec {
         def response = defaultPbsService.sendAmpRequest(ampRequest)
 
         then: "Bidder should be dropped"
-        assert response.ext?.errors[ErrorType.GENERIC]*.code == [999]
-        assert response.ext?.errors[PREBID]*.message ==
+        assert response.ext?.warnings[ErrorType.PREBID]*.code == [999, 999]
+        assert response.ext?.warnings[ErrorType.PREBID]*.message ==
                 ["WARNING: request.imp[0].ext.prebid.bidder.generic was dropped with a reason: " +
-                         "request.imp[0].ext.prebid.bidder.generic failed validation" +
+                         "request.imp[0].ext.prebid.bidder.generic failed validation.\n" +
                          "\$.exampleProperty: integer found, string expected",
                  "WARNING: request.imp[0].ext must contain at least one valid bidder"]
 

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

@@ -151,6 +151,7 @@ public void shouldReturnExpectedVideoResponse() {
                 ExtAmpVideoResponse.of(
                         bidResponse.getExt().getDebug(),
                         bidResponse.getExt().getErrors(),
+                        bidResponse.getExt().getWarnings(),
                         ExtAmpVideoPrebid.of(extResponse.getPrebid().getModules()))));
     }
 }

src/test/java/org/prebid/server/auction/requestfactory/AmpRequestFactoryTest.java

@@ -288,7 +288,7 @@ public void shouldReturnFailedFutureIfEntrypointHookRejectedRequest() {
     }
 
     @Test
-    public void shouldEnrichAuctionContextWithDebugContext() throws JsonProcessingException {
+    public void shouldEnrichAuctionContextWithDebugContext() {
         // given
         givenBidRequest();
 
@@ -1524,7 +1524,6 @@ public void shouldReturnBidRequestWithoutRegsExtUsPrivacyWhenConsentStringIsVali
         assertThat(result.getRegs()).isNull();
     }
 
-    @SuppressWarnings("unchecked")
     @Test
     public void shouldReturnBidRequestWithCreatedExtPrebidAmpData() {
         // given
@@ -1666,7 +1665,8 @@ private void givenBidRequest(
 
         given(ortb2ImplicitParametersResolver.resolve(any(), any(), any(), any())).willAnswer(
                 answerWithFirstArgument());
-        given(ortb2RequestFactory.validateRequest(any())).willAnswer(answerWithFirstArgument());
+        given(ortb2RequestFactory.validateRequest(any(), any()))
+                .willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(0)));
 
         given(ortb2RequestFactory.enrichBidRequestWithAccountAndPrivacyData(any()))
                 .willAnswer(invocation -> ((AuctionContext) invocation.getArgument(0)).getBidRequest());

src/test/java/org/prebid/server/auction/requestfactory/AuctionRequestFactoryTest.java

@@ -133,11 +133,12 @@ public void setUp() {
         given(ortb2RequestFactory.executeRawAuctionRequestHooks(any()))
                 .willAnswer(invocation -> Future.succeededFuture(
                         ((AuctionContext) invocation.getArgument(0)).getBidRequest()));
+        given(ortb2RequestFactory.validateRequest(any(), any()))
+                .willAnswer(invocationOnMock -> Future.succeededFuture((BidRequest) invocationOnMock.getArgument(0)));
 
         given(paramsResolver.resolve(any(), any(), any(), any()))
                 .will(invocationOnMock -> invocationOnMock.getArgument(0));
-        given(ortb2RequestFactory.validateRequest(any()))
-                .will(invocationOnMock -> invocationOnMock.getArgument(0));
+
         given(interstitialProcessor.process(any()))
                 .will(invocationOnMock -> invocationOnMock.getArgument(0));
 
@@ -554,8 +555,8 @@ public void shouldReturnFailedFutureIfRequestValidationFailed() {
         // given
         givenValidBidRequest();
 
-        given(ortb2RequestFactory.validateRequest(any()))
-                .willThrow(new InvalidRequestException("errors"));
+        given(ortb2RequestFactory.validateRequest(any(), any()))
+                .willReturn(Future.failedFuture(new InvalidRequestException("errors")));
 
         // when
         final Future<?> future = target.fromRequest(routingContext, 0L);