Turn off debug by Publisher, by bidder - proposal

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

@@ -1,7 +1,7 @@
 package org.prebid.server.auction;
 
 import com.iab.openrtb.request.BidRequest;
-import org.apache.commons.lang3.BooleanUtils;
+import org.apache.commons.lang3.ObjectUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.prebid.server.auction.model.AuctionContext;
 import org.prebid.server.auction.model.DebugContext;
@@ -12,13 +12,14 @@
 import org.prebid.server.proto.openrtb.ext.request.TraceLevel;
 import org.prebid.server.settings.model.Account;
 import org.prebid.server.settings.model.AccountAuctionConfig;
+import org.prebid.server.util.ObjectUtil;
 
 import java.util.Objects;
-import java.util.function.Function;
 
 public class DebugResolver {
 
     private static final String DEBUG_OVERRIDE_HEADER = "x-pbs-debug-override";
+    private static final Boolean DEFAULT_DEBUG_ALLOWED_BY_ACCOUNT = true;
 
     private final BidderCatalog bidderCatalog;
     private final String debugOverrideToken;
@@ -29,38 +30,62 @@ public DebugResolver(BidderCatalog bidderCatalog, String debugOverrideToken) {
     }
 
     public DebugContext debugContextFrom(AuctionContext auctionContext) {
-        final BidRequest bidRequest = auctionContext.getBidRequest();
-        final ExtRequestPrebid extRequestPrebid = getIfNotNull(bidRequest.getExt(), ExtRequest::getPrebid);
+        final boolean debugEnabled = isDebugEnabled(auctionContext);
+        final TraceLevel traceLevel = getTraceLevel(auctionContext.getBidRequest());
+        return DebugContext.of(debugEnabled, traceLevel);
+    }
 
-        final boolean debugOverridden = isDebugOverridden(auctionContext.getHttpRequest());
-        final boolean debugEnabled = debugOverridden
-                || (isAccountAllowed(auctionContext) && isDebugEnabled(bidRequest, extRequestPrebid));
+    private boolean isDebugEnabled(AuctionContext auctionContext) {
+        final BidRequest bidRequest = auctionContext.getBidRequest();
+        final boolean debugOverride = isDebugOverridden(auctionContext.getHttpRequest());
+        final boolean debugEnabledForRequest = isDebugEnabledForRequest(bidRequest);
+        final boolean debugAllowedByAccount = isDebugAllowedByAccount(auctionContext.getAccount());
 
-        final TraceLevel traceLevel = getIfNotNull(extRequestPrebid, ExtRequestPrebid::getTrace);
+        if (debugEnabledForRequest && !debugOverride && !debugAllowedByAccount) {
+            auctionContext.getDebugWarnings()
+                    .add("Debug turned off for account");
+        }
 
-        return DebugContext.of(debugEnabled, debugOverridden, traceLevel);
+        return debugOverride || (debugEnabledForRequest && debugAllowedByAccount);
     }
 
-    private boolean isAccountAllowed(AuctionContext auctionContext) {
-        final AccountAuctionConfig auctionConfig = getIfNotNull(auctionContext.getAccount(), Account::getAuction);
-        return BooleanUtils.toBoolean(getIfNotNull(auctionConfig, AccountAuctionConfig::getDebugAllow));
+    private boolean isDebugOverridden(HttpRequestContext httpRequest) {
+        return StringUtils.isNotEmpty(debugOverrideToken)
+                && StringUtils.equals(httpRequest.getHeaders().get(DEBUG_OVERRIDE_HEADER), debugOverrideToken);
     }
 
-    private boolean isDebugEnabled(BidRequest bidRequest, ExtRequestPrebid extRequestPrebid) {
+    private boolean isDebugEnabledForRequest(BidRequest bidRequest) {
         return Objects.equals(bidRequest.getTest(), 1)
-                || Objects.equals(getIfNotNull(extRequestPrebid, ExtRequestPrebid::getDebug), 1);
+                || Objects.equals(ObjectUtil.getIfNotNull(getExtRequestPrebid(bidRequest),
+                ExtRequestPrebid::getDebug), 1);
     }
 
-    private boolean isDebugOverridden(HttpRequestContext httpRequestContext) {
-        return StringUtils.isNotEmpty(debugOverrideToken)
-                && StringUtils.equals(httpRequestContext.getHeaders().get(DEBUG_OVERRIDE_HEADER), debugOverrideToken);
+    private boolean isDebugAllowedByAccount(Account account) {
+        final AccountAuctionConfig auctionConfig = ObjectUtil.getIfNotNull(account, Account::getAuction);
+        final Boolean debugAllowed = ObjectUtil.getIfNotNull(auctionConfig, AccountAuctionConfig::getDebugAllow);
+        return ObjectUtils.defaultIfNull(debugAllowed, DEFAULT_DEBUG_ALLOWED_BY_ACCOUNT);
     }
 
-    public boolean resolveDebugForBidder(String bidderName, boolean debugEnabled, boolean debugOverride) {
-        return (bidderCatalog.isDebugAllowed(bidderName) && debugEnabled) || debugOverride;
+    private static TraceLevel getTraceLevel(BidRequest bidRequest) {
+        return ObjectUtil.getIfNotNull(getExtRequestPrebid(bidRequest), ExtRequestPrebid::getTrace);
     }
 
-    private static <T, R> R getIfNotNull(T target, Function<T, R> getter) {
-        return target != null ? getter.apply(target) : null;
+    private static ExtRequestPrebid getExtRequestPrebid(BidRequest bidRequest) {
+        return ObjectUtil.getIfNotNull(
+                ObjectUtil.getIfNotNull(bidRequest, BidRequest::getExt), ExtRequest::getPrebid);
+    }
+
+    public boolean resolveDebugForBidder(AuctionContext auctionContext, String bidder) {
+        final DebugContext debugContext = auctionContext.getDebugContext();
+        final boolean debugEnabled = debugContext.isDebugEnabled();
+        final boolean debugOverride = isDebugOverridden(auctionContext.getHttpRequest());
+        final boolean debugAllowedByBidder = bidderCatalog.isDebugAllowed(bidder);
+
+        if (debugEnabled && !debugOverride && !debugAllowedByBidder) {
+            auctionContext.getDebugWarnings()
+                    .add(String.format("Debug turned off for bidder: %s", bidder));
+        }
+
+        return debugOverride || (debugEnabled && debugAllowedByBidder);
     }
 }

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

@@ -30,7 +30,6 @@
 import org.prebid.server.auction.model.BidderPrivacyResult;
 import org.prebid.server.auction.model.BidderRequest;
 import org.prebid.server.auction.model.BidderResponse;
-import org.prebid.server.auction.model.DebugContext;
 import org.prebid.server.auction.model.MultiBidConfig;
 import org.prebid.server.auction.model.StoredResponseResult;
 import org.prebid.server.auction.model.Tuple2;
@@ -1099,14 +1098,11 @@ private Future<BidderResponse> invokeHooksAndRequestBids(AuctionContext auctionC
                                                              Timeout timeout,
                                                              BidderAliases aliases) {
 
-        final CaseInsensitiveMultiMap headers = auctionContext.getHttpRequest().getHeaders();
-        final DebugContext debugContext = auctionContext.getDebugContext();
-        final boolean debugEnabled = debugContext.isDebugEnabled();
-        final boolean debugOverride = debugContext.isDebugOverride();
-
         return hookStageExecutor.executeBidderRequestStage(bidderRequest, auctionContext)
+
                 .compose(stageResult -> requestBidsOrRejectBidder(
-                        stageResult, bidderRequest, timeout, headers, debugEnabled, debugOverride, aliases))
+                        stageResult, bidderRequest, auctionContext, timeout, aliases))
+
                 .compose(bidderResponse -> hookStageExecutor.executeRawBidderResponseStage(
                                 bidderResponse, auctionContext)
                         .map(stageResult -> rejectBidderResponseOrProceed(stageResult, bidderResponse)));
@@ -1115,56 +1111,52 @@ private Future<BidderResponse> invokeHooksAndRequestBids(AuctionContext auctionC
     private Future<BidderResponse> requestBidsOrRejectBidder(
             HookStageExecutionResult<BidderRequestPayload> hookStageResult,
             BidderRequest bidderRequest,
+            AuctionContext auctionContext,
             Timeout timeout,
-            CaseInsensitiveMultiMap requestHeaders,
-            boolean debugEnabled,
-            boolean debugOverride,
             BidderAliases aliases) {
 
-        return hookStageResult.isShouldReject()
-                ? Future.succeededFuture(BidderResponse.of(bidderRequest.getBidder(), BidderSeatBid.empty(), 0))
-                : requestBids(
-                bidderRequest.with(hookStageResult.getPayload().bidRequest()),
-                timeout,
-                requestHeaders,
-                debugEnabled,
-                debugOverride,
-                aliases);
-    }
-
-    private BidderResponse rejectBidderResponseOrProceed(HookStageExecutionResult<BidderResponsePayload> stageResult,
-                                                         BidderResponse bidderResponse) {
-
-        final List<BidderBid> bids = stageResult.isShouldReject()
-                ? Collections.emptyList()
-                : stageResult.getPayload().bids();
+        if (hookStageResult.isShouldReject()) {
+            return Future.succeededFuture(BidderResponse.of(bidderRequest.getBidder(), BidderSeatBid.empty(), 0));
+        }
 
-        return bidderResponse
-                .with(bidderResponse.getSeatBid().with(bids));
+        final BidderRequest enrichedBidderRequest = bidderRequest.with(hookStageResult.getPayload().bidRequest());
+        return requestBids(enrichedBidderRequest, auctionContext, timeout, aliases);
     }
 
     /**
      * Passes the request to a corresponding bidder and wraps response in {@link BidderResponse} which also holds
      * recorded response time.
      */
     private Future<BidderResponse> requestBids(BidderRequest bidderRequest,
+                                               AuctionContext auctionContext,
                                                Timeout timeout,
-                                               CaseInsensitiveMultiMap requestHeaders,
-                                               boolean debugEnabled,
-                                               boolean debugOverride,
                                                BidderAliases aliases) {
+
+        final CaseInsensitiveMultiMap requestHeaders = auctionContext.getHttpRequest().getHeaders();
+
         final String bidderName = bidderRequest.getBidder();
         final String resolvedBidderName = aliases.resolveBidder(bidderName);
         final Bidder<?> bidder = bidderCatalog.bidderByName(resolvedBidderName);
-        final boolean debugEnabledForBidder = debugResolver.resolveDebugForBidder(
-                resolvedBidderName, debugEnabled, debugOverride);
+
+        final boolean debugEnabledForBidder = debugResolver.resolveDebugForBidder(auctionContext, resolvedBidderName);
 
         final long startTime = clock.millis();
 
         return httpBidderRequester.requestBids(bidder, bidderRequest, timeout, requestHeaders, debugEnabledForBidder)
                 .map(seatBid -> BidderResponse.of(bidderName, seatBid, responseTime(startTime)));
     }
 
+    private BidderResponse rejectBidderResponseOrProceed(HookStageExecutionResult<BidderResponsePayload> stageResult,
+                                                         BidderResponse bidderResponse) {
+
+        final List<BidderBid> bids = stageResult.isShouldReject()
+                ? Collections.emptyList()
+                : stageResult.getPayload().bids();
+
+        return bidderResponse
+                .with(bidderResponse.getSeatBid().with(bids));
+    }
+
     private List<BidderResponse> validateAndAdjustBids(
             List<BidderResponse> bidderResponses, AuctionContext auctionContext, BidderAliases aliases) {
 

src/main/java/org/prebid/server/auction/model/DebugContext.java

@@ -6,12 +6,10 @@
 @Value(staticConstructor = "of")
 public class DebugContext {
 
-    private static final DebugContext EMPTY = DebugContext.of(false, false, null);
+    private static final DebugContext EMPTY = DebugContext.of(false, null);
 
     boolean debugEnabled;
 
-    boolean debugOverride;
-
     TraceLevel traceLevel;
 
     public static DebugContext empty() {

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

@@ -148,7 +148,7 @@ private Future<WithPodErrors<BidRequest>> createBidRequest(HttpRequestContext ht
         final Set<String> podConfigIds = podConfigIds(bidRequestVideo);
 
         return storedRequestProcessor.processVideoRequest(
-                accountIdFrom(bidRequestVideo), storedRequestId, podConfigIds, bidRequestVideo)
+                        accountIdFrom(bidRequestVideo), storedRequestId, podConfigIds, bidRequestVideo)
                 .map(bidRequestToErrors -> fillImplicitParametersAndValidate(httpRequest, bidRequestToErrors));
     }
 

src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java

@@ -526,7 +526,6 @@ BidderErrorNotifier bidderErrorNotifier(
     BidResponseCreator bidResponseCreator(
             CacheService cacheService,
             BidderCatalog bidderCatalog,
-            DebugResolver debugResolver,
             VastModifier vastModifier,
             EventsService eventsService,
             StoredRequestProcessor storedRequestProcessor,
@@ -540,7 +539,6 @@ BidResponseCreator bidResponseCreator(
         return new BidResponseCreator(
                 cacheService,
                 bidderCatalog,
-                debugResolver,
                 vastModifier,
                 eventsService,
                 storedRequestProcessor,

src/main/resources/bidder-config/appnexus.yaml

@@ -4,8 +4,6 @@ adapters:
     aliases:
       districtm:
         enabled: false
-    debug:
-      allowed: true
     meta-info:
       maintainer-email: prebid-server@xandr.com
       app-media-types:

src/main/resources/bidder-config/ttx.yaml

@@ -4,8 +4,6 @@ adapters:
     aliases:
       '33across':
         enabled: false
-    debug:
-      allowed: true
     meta-info:
       maintainer-email: headerbidding@33across.com
       app-media-types:

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

@@ -149,8 +149,6 @@ public class BidResponseCreatorTest extends VertxTest {
     @Mock
     private BidderCatalog bidderCatalog;
     @Mock
-    private DebugResolver debugResolver;
-    @Mock
     private VastModifier vastModifier;
     @Mock
     private EventsService eventsService;
@@ -192,7 +190,6 @@ public void setUp() {
         bidResponseCreator = new BidResponseCreator(
                 cacheService,
                 bidderCatalog,
-                debugResolver,
                 vastModifier,
                 eventsService,
                 storedRequestProcessor,
@@ -1289,7 +1286,6 @@ public void shouldTruncateTargetingKeywordsByGlobalConfig() {
         final BidResponseCreator bidResponseCreator = new BidResponseCreator(
                 cacheService,
                 bidderCatalog,
-                debugResolver,
                 vastModifier,
                 eventsService,
                 storedRequestProcessor,
@@ -1662,7 +1658,7 @@ public void shouldIncreaseLineItemSentToClientAsTopMatchMetricInTransactionLog()
 
         final AuctionContext auctionContext = givenAuctionContext(
                 bidRequest,
-                context -> context.debugContext(DebugContext.of(true, false, null)));
+                context -> context.debugContext(DebugContext.of(true, null)));
 
         final Bid bid = Bid.builder()
                 .id("bidId1")
@@ -2580,17 +2576,15 @@ public void shouldProcessRequestAndAddErrorFromAuctionContext() {
     }
 
     @Test
-    public void shouldPopulateResponseDebugExtensionAndWarningsIfDebugIsEnabledAndBidderAllowedDebug() {
+    public void shouldPopulateResponseDebugExtensionAndWarningsIfDebugIsEnabled() {
         // given
-        given(debugResolver.resolveDebugForBidder("bidder1", true, false)).willReturn(true);
-
         final BidRequest bidRequest = givenBidRequest(givenImp());
         final List<String> warnings = asList("warning1", "warning2");
         final AuctionContext auctionContext = givenAuctionContext(
                 bidRequest,
                 builder -> builder
                         .debugWarnings(warnings)
-                        .debugContext(DebugContext.of(true, false, null)));
+                        .debugContext(DebugContext.of(true, null)));
         givenCacheServiceResult(CacheServiceResult.of(
                 DebugHttpCall.builder()
                         .endpoint("http://cache-service/cache")
@@ -2639,36 +2633,6 @@ public void shouldPopulateResponseDebugExtensionAndWarningsIfDebugIsEnabledAndBi
         verify(cacheService).cacheBidsOpenrtb(anyList(), any(), any(), any());
     }
 
-    @Test
-    public void shouldNotPopulateResponseDebugExtensionWithHttpCallsIfDebugIsEnabledAndBidderDisallowedDebug() {
-        // given
-        given(bidderCatalog.isDebugAllowed("bidder1")).willReturn(false);
-        givenCacheServiceResult(CacheServiceResult.of(null, null, emptyMap()));
-
-        final BidRequest bidRequest = givenBidRequest(givenImp());
-        final AuctionContext auctionContext = givenAuctionContext(
-                bidRequest,
-                builder -> builder
-                        .debugWarnings(emptyList())
-                        .debugContext(DebugContext.of(true, false, null)));
-
-        final BidRequestCacheInfo cacheInfo = BidRequestCacheInfo.builder().doCaching(true).build();
-
-        final Bid bid = Bid.builder().id("bidId1").impid(IMP_ID).price(BigDecimal.valueOf(5.67)).build();
-        final List<BidderResponse> bidderResponses = singletonList(BidderResponse.of("bidder1",
-                BidderSeatBid.of(singletonList(BidderBid.of(bid, banner, null)),
-                        singletonList(ExtHttpCall.builder().status(200).build()), null), 100));
-
-        // when
-        final BidResponse bidResponse =
-                bidResponseCreator.create(bidderResponses, auctionContext, cacheInfo, MULTI_BIDS).result();
-
-        // then
-        final ExtBidResponse responseExt = bidResponse.getExt();
-
-        assertThat(responseExt.getDebug().getHttpcalls()).isNull();
-    }
-
     @Test
     public void shouldPassIntegrationToCacheServiceAndBidEvents() {
         // given