From d9db6d03b11dabb714af7aed2b1b6f6cad7ff747 Mon Sep 17 00:00:00 2001 From: vmalitskyi Date: Mon, 23 Dec 2024 13:54:19 +0100 Subject: [PATCH 1/4] Fix #3592. Bad Input Error if pbjs s2s config contains alias configuration for a disabled adapter --- .../requestfactory/AmpRequestFactory.java | 1 + .../requestfactory/AuctionRequestFactory.java | 2 +- .../requestfactory/Ortb2RequestFactory.java | 4 +- .../requestfactory/VideoRequestFactory.java | 2 + .../org/prebid/server/metric/MetricName.java | 2 + .../org/prebid/server/metric/Metrics.java | 16 ++ .../server/validation/RequestValidator.java | 20 ++- .../requestfactory/AmpRequestFactoryTest.java | 2 +- .../AuctionRequestFactoryTest.java | 4 +- .../Ortb2RequestFactoryTest.java | 11 +- .../VideoRequestFactoryTest.java | 4 +- .../org/prebid/server/metric/MetricsTest.java | 100 +++++++++++- .../validation/RequestValidatorTest.java | 154 +++++++++--------- 13 files changed, 218 insertions(+), 104 deletions(-) diff --git a/src/main/java/org/prebid/server/auction/requestfactory/AmpRequestFactory.java b/src/main/java/org/prebid/server/auction/requestfactory/AmpRequestFactory.java index a7b85bea5f6..fe366d54cbc 100644 --- a/src/main/java/org/prebid/server/auction/requestfactory/AmpRequestFactory.java +++ b/src/main/java/org/prebid/server/auction/requestfactory/AmpRequestFactory.java @@ -415,6 +415,7 @@ private Future updateBidRequest(AuctionContext auctionContext) { .map(bidRequest -> paramsResolver.resolve(bidRequest, auctionContext, ENDPOINT, true)) .map(bidRequest -> ortb2RequestFactory.removeEmptyEids(bidRequest, auctionContext.getDebugWarnings())) .compose(resolvedBidRequest -> ortb2RequestFactory.validateRequest( + account, resolvedBidRequest, auctionContext.getHttpRequest(), auctionContext.getDebugContext(), diff --git a/src/main/java/org/prebid/server/auction/requestfactory/AuctionRequestFactory.java b/src/main/java/org/prebid/server/auction/requestfactory/AuctionRequestFactory.java index 18394806ef4..0eb24dface1 100644 --- a/src/main/java/org/prebid/server/auction/requestfactory/AuctionRequestFactory.java +++ b/src/main/java/org/prebid/server/auction/requestfactory/AuctionRequestFactory.java @@ -242,7 +242,7 @@ private Future updateAndValidateBidRequest(AuctionContext auctionCon return storedRequestProcessor.processAuctionRequest(account.getId(), auctionContext.getBidRequest()) .compose(auctionStoredResult -> updateBidRequest(auctionStoredResult, auctionContext)) .compose(bidRequest -> ortb2RequestFactory.validateRequest( - bidRequest, httpRequest, auctionContext.getDebugContext(), debugWarnings)) + account, bidRequest, httpRequest, auctionContext.getDebugContext(), debugWarnings)) .map(interstitialProcessor::process); } diff --git a/src/main/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactory.java b/src/main/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactory.java index f76d61316f9..eab0ac25213 100644 --- a/src/main/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactory.java +++ b/src/main/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactory.java @@ -192,13 +192,13 @@ public Future activityInfrastructureFrom(AuctionContext auctionContext.getDebugContext().getTraceLevel())); } - public Future validateRequest(BidRequest bidRequest, + public Future validateRequest(Account account, BidRequest bidRequest, HttpRequestContext httpRequestContext, DebugContext debugContext, List warnings) { final ValidationResult validationResult = requestValidator.validate( - bidRequest, httpRequestContext, debugContext); + account, bidRequest, httpRequestContext, debugContext); if (validationResult.hasWarnings()) { warnings.addAll(validationResult.getWarnings()); diff --git a/src/main/java/org/prebid/server/auction/requestfactory/VideoRequestFactory.java b/src/main/java/org/prebid/server/auction/requestfactory/VideoRequestFactory.java index 623e20f5ffb..38e60ba40e3 100644 --- a/src/main/java/org/prebid/server/auction/requestfactory/VideoRequestFactory.java +++ b/src/main/java/org/prebid/server/auction/requestfactory/VideoRequestFactory.java @@ -33,6 +33,7 @@ import org.prebid.server.model.HttpRequestContext; import org.prebid.server.proto.openrtb.ext.request.ExtPublisher; import org.prebid.server.proto.openrtb.ext.request.ExtPublisherPrebid; +import org.prebid.server.settings.model.Account; import org.prebid.server.util.HttpUtil; import org.prebid.server.util.ObjectUtil; @@ -120,6 +121,7 @@ public Future> fromRequest(RoutingContext routingC .map(auctionContext -> auctionContext.with(debugResolver.debugContextFrom(auctionContext))) .compose(auctionContext -> ortb2RequestFactory.validateRequest( + auctionContext.getAccount(), auctionContext.getBidRequest(), auctionContext.getHttpRequest(), auctionContext.getDebugContext(), diff --git a/src/main/java/org/prebid/server/metric/MetricName.java b/src/main/java/org/prebid/server/metric/MetricName.java index 02df8cafa81..7c41dd65e72 100644 --- a/src/main/java/org/prebid/server/metric/MetricName.java +++ b/src/main/java/org/prebid/server/metric/MetricName.java @@ -63,6 +63,8 @@ public enum MetricName { nobid, gotbids, badinput, + disabled_bidder, + unknown_bidder, blocklisted_account, blocklisted_app, badserverresponse, diff --git a/src/main/java/org/prebid/server/metric/Metrics.java b/src/main/java/org/prebid/server/metric/Metrics.java index 68085cb5556..4c001af4a3a 100644 --- a/src/main/java/org/prebid/server/metric/Metrics.java +++ b/src/main/java/org/prebid/server/metric/Metrics.java @@ -336,6 +336,22 @@ public void updateAdapterRequestErrorMetric(String bidder, MetricName errorMetri forAdapter(bidder).request().incCounter(errorMetric); } + public void updateAdapterRequestDisabledBidderMetric(String bidder, Account account) { + forAdapter(bidder).request().incCounter(MetricName.disabled_bidder); + if (accountMetricsVerbosityResolver.forAccount(account) + .isAtLeast(AccountMetricsVerbosityLevel.detailed)) { + forAccount(account.getId()).adapter().forAdapter(bidder).request().incCounter(MetricName.disabled_bidder); + } + } + + public void updateAdapterRequestUnknownBidderMetric(String bidder, Account account) { + forAdapter(bidder).request().incCounter(MetricName.unknown_bidder); + if (accountMetricsVerbosityResolver.forAccount(account) + .isAtLeast(AccountMetricsVerbosityLevel.detailed)) { + forAccount(account.getId()).adapter().forAdapter(bidder).request().incCounter(MetricName.unknown_bidder); + } + } + public void updateAnalyticEventMetric(String analyticCode, MetricName eventType, MetricName result) { forAnalyticReporter(analyticCode).forEventType(eventType).incCounter(result); } diff --git a/src/main/java/org/prebid/server/validation/RequestValidator.java b/src/main/java/org/prebid/server/validation/RequestValidator.java index cccac3edc75..66c19983197 100644 --- a/src/main/java/org/prebid/server/validation/RequestValidator.java +++ b/src/main/java/org/prebid/server/validation/RequestValidator.java @@ -41,6 +41,7 @@ import org.prebid.server.proto.openrtb.ext.request.ExtUserPrebid; import org.prebid.server.proto.openrtb.ext.request.ImpMediaType; import org.prebid.server.proto.openrtb.ext.response.BidType; +import org.prebid.server.settings.model.Account; import org.prebid.server.util.HttpUtil; import org.prebid.server.validation.model.ValidationResult; @@ -97,7 +98,8 @@ public RequestValidator(BidderCatalog bidderCatalog, * Validates the {@link BidRequest} against a list of validation checks, however, reports only one problem * at a time. */ - public ValidationResult validate(BidRequest bidRequest, + public ValidationResult validate(Account account, + BidRequest bidRequest, HttpRequestContext httpRequestContext, DebugContext debugContext) { @@ -126,7 +128,7 @@ public ValidationResult validate(BidRequest bidRequest, validateTargeting(targeting); } aliases = ObjectUtils.defaultIfNull(extRequestPrebid.getAliases(), Collections.emptyMap()); - validateAliases(aliases); + validateAliases(aliases, warnings, metrics, account); validateAliasesGvlIds(extRequestPrebid, aliases); validateBidAdjustmentFactors(extRequestPrebid.getBidadjustmentfactors(), aliases); validateExtBidPrebidData(extRequestPrebid.getData(), aliases, isDebugEnabled, warnings); @@ -505,18 +507,22 @@ private static void validateGranularityRangeIncrement(ExtGranularityRange range) * Validates aliases. Throws {@link ValidationException} in cases when alias points to invalid bidder or when alias * is equals to itself. */ - private void validateAliases(Map aliases) throws ValidationException { + private void validateAliases(Map aliases, List warnings, + Metrics metrics, Account account) throws ValidationException { + for (final Map.Entry aliasToBidder : aliases.entrySet()) { final String alias = aliasToBidder.getKey(); final String coreBidder = aliasToBidder.getValue(); if (!bidderCatalog.isValidName(coreBidder)) { - throw new ValidationException( + warnings.add( "request.ext.prebid.aliases.%s refers to unknown bidder: %s".formatted(alias, coreBidder)); - } - if (!bidderCatalog.isActive(coreBidder)) { - throw new ValidationException( + metrics.updateAdapterRequestUnknownBidderMetric(coreBidder, account); + } else if (!bidderCatalog.isActive(coreBidder)) { + warnings.add( "request.ext.prebid.aliases.%s refers to disabled bidder: %s".formatted(alias, coreBidder)); + metrics.updateAdapterRequestDisabledBidderMetric(coreBidder, account); } + if (alias.equals(coreBidder)) { throw new ValidationException(""" request.ext.prebid.aliases.%s defines a no-op alias. \ diff --git a/src/test/java/org/prebid/server/auction/requestfactory/AmpRequestFactoryTest.java b/src/test/java/org/prebid/server/auction/requestfactory/AmpRequestFactoryTest.java index 8090ebf74ef..bcd26c7c162 100644 --- a/src/test/java/org/prebid/server/auction/requestfactory/AmpRequestFactoryTest.java +++ b/src/test/java/org/prebid/server/auction/requestfactory/AmpRequestFactoryTest.java @@ -1753,7 +1753,7 @@ private void givenBidRequest( given(ortb2ImplicitParametersResolver.resolve(any(), any(), any(), anyBoolean())).willAnswer( answerWithFirstArgument()); - given(ortb2RequestFactory.validateRequest(any(), any(), any(), any())) + given(ortb2RequestFactory.validateRequest(any(), any(), any(), any(), any())) .willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(0))); given(ortb2RequestFactory.enrichBidRequestWithAccountAndPrivacyData(any())) diff --git a/src/test/java/org/prebid/server/auction/requestfactory/AuctionRequestFactoryTest.java b/src/test/java/org/prebid/server/auction/requestfactory/AuctionRequestFactoryTest.java index b9d49dfbde7..fa9ad3be13c 100644 --- a/src/test/java/org/prebid/server/auction/requestfactory/AuctionRequestFactoryTest.java +++ b/src/test/java/org/prebid/server/auction/requestfactory/AuctionRequestFactoryTest.java @@ -165,7 +165,7 @@ public void setUp() { given(ortb2RequestFactory.executeRawAuctionRequestHooks(any())) .willAnswer(invocation -> Future.succeededFuture( ((AuctionContext) invocation.getArgument(0)).getBidRequest())); - given(ortb2RequestFactory.validateRequest(any(), any(), any(), any())) + given(ortb2RequestFactory.validateRequest(any(), any(), any(), any(), any())) .willAnswer(invocationOnMock -> Future.succeededFuture((BidRequest) invocationOnMock.getArgument(0))); given(ortb2RequestFactory.removeEmptyEids(any(), any())) .willAnswer(invocationOnMock -> invocationOnMock.getArgument(0)); @@ -662,7 +662,7 @@ public void shouldReturnFailedFutureIfRequestValidationFailed() { // given givenValidBidRequest(); - given(ortb2RequestFactory.validateRequest(any(), any(), any(), any())) + given(ortb2RequestFactory.validateRequest(any(), any(), any())) .willReturn(Future.failedFuture(new InvalidRequestException("errors"))); // when diff --git a/src/test/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactoryTest.java b/src/test/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactoryTest.java index 6aa83fecb1f..c6bad5c5e94 100644 --- a/src/test/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactoryTest.java +++ b/src/test/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactoryTest.java @@ -104,6 +104,7 @@ public class Ortb2RequestFactoryTest extends VertxTest { private static final List BLOCKLISTED_ACCOUNTS = singletonList("bad_acc"); + private static final String ACCOUNT_ID = "accountId"; @Mock private UidsCookieService uidsCookieService; @@ -658,12 +659,13 @@ public void enrichAuctionContextShouldSetDebugOff() { @Test public void validateRequestShouldThrowInvalidRequestExceptionIfRequestIsInvalid() { // given - given(requestValidator.validate(any(), any(), any())).willReturn(ValidationResult.error("error")); + given(requestValidator.validate(any(), any(), any(), any())).willReturn(ValidationResult.error("error")); final BidRequest bidRequest = givenBidRequest(identity()); // when final Future result = target.validateRequest( + Account.empty(ACCOUNT_ID), bidRequest, HttpRequestContext.builder().build(), DebugContext.empty(), @@ -675,25 +677,26 @@ public void validateRequestShouldThrowInvalidRequestExceptionIfRequestIsInvalid( .isInstanceOf(InvalidRequestException.class) .hasMessage("error"); - verify(requestValidator).validate(eq(bidRequest), any(), any()); + verify(requestValidator).validate(any(), eq(bidRequest), any(), any()); } @Test public void validateRequestShouldReturnSameBidRequest() { // given - given(requestValidator.validate(any(), any(), any())).willReturn(ValidationResult.success()); + given(requestValidator.validate(any(), any(), any(), any())).willReturn(ValidationResult.success()); final BidRequest bidRequest = givenBidRequest(identity()); // when final BidRequest result = target.validateRequest( + Account.empty(ACCOUNT_ID), bidRequest, HttpRequestContext.builder().build(), DebugContext.empty(), new ArrayList<>()).result(); // then - verify(requestValidator).validate(eq(bidRequest), any(), any()); + verify(requestValidator).validate(any(), eq(bidRequest), any(), any()); assertThat(result).isSameAs(bidRequest); } diff --git a/src/test/java/org/prebid/server/auction/requestfactory/VideoRequestFactoryTest.java b/src/test/java/org/prebid/server/auction/requestfactory/VideoRequestFactoryTest.java index f44b581e071..74521b05c07 100644 --- a/src/test/java/org/prebid/server/auction/requestfactory/VideoRequestFactoryTest.java +++ b/src/test/java/org/prebid/server/auction/requestfactory/VideoRequestFactoryTest.java @@ -345,7 +345,7 @@ public void shouldReturnExpectedResultAndReturnErrors() throws JsonProcessingExc verify(ortb2RequestFactory).createAuctionContext(any(), eq(MetricName.video)); verify(ortb2RequestFactory).enrichAuctionContext(any(), any(), eq(bidRequest), eq(0L)); verify(ortb2RequestFactory).fetchAccountWithoutStoredRequestLookup(any()); - verify(ortb2RequestFactory).validateRequest(eq(bidRequest), any(), any(), any()); + verify(ortb2RequestFactory).validateRequest(any(), eq(bidRequest), any(), any(), any()); verify(paramsResolver) .resolve(eq(bidRequest), any(), eq(Endpoint.openrtb2_video.value()), eq(false)); verify(ortb2RequestFactory).enrichBidRequestWithAccountAndPrivacyData( @@ -404,7 +404,7 @@ private void givenBidRequest(BidRequest bidRequest, List podErrors) { .build()); given(ortb2RequestFactory.fetchAccountWithoutStoredRequestLookup(any())).willReturn(Future.succeededFuture()); - given(ortb2RequestFactory.validateRequest(any(), any(), any(), any())) + given(ortb2RequestFactory.validateRequest(any(), any(), any(), any(), any())) .willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(0))); given(paramsResolver.resolve(any(), any(), any(), anyBoolean())) diff --git a/src/test/java/org/prebid/server/metric/MetricsTest.java b/src/test/java/org/prebid/server/metric/MetricsTest.java index 6658d8dd721..77e9fe0956d 100644 --- a/src/test/java/org/prebid/server/metric/MetricsTest.java +++ b/src/test/java/org/prebid/server/metric/MetricsTest.java @@ -614,6 +614,40 @@ public void updateAdapterBidMetricsShouldUpdateMetrics() { assertThat(metricRegistry.counter("adapter.conversant.banner.nurl_bids_received").getCount()).isEqualTo(2); } + @Test + public void updateAdapterRequestUnknownBidderMetricsShouldIncrementMetrics() { + // when + metrics.updateAdapterRequestUnknownBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); + metrics.updateAdapterRequestUnknownBidderMetric(CONVERSANT, Account.empty(ACCOUNT_ID)); + metrics.updateAdapterRequestUnknownBidderMetric(CONVERSANT, Account.empty(ACCOUNT_ID)); + + // then + assertThat(metricRegistry.counter("adapter.rubicon.requests.unknown_bidder").getCount()).isOne(); + assertThat(metricRegistry.counter( + "account.accountId.adapter.rubicon.requests.unknown_bidder").getCount()).isOne(); + assertThat(metricRegistry.counter( + "adapter.conversant.requests.unknown_bidder").getCount()).isEqualTo(2); + assertThat(metricRegistry.counter( + "account.accountId.adapter.conversant.requests.unknown_bidder").getCount()).isEqualTo(2); + } + + @Test + public void updateAdapterRequestDisabledBidderMetricsShouldIncrementMetrics() { + // when + metrics.updateAdapterRequestDisabledBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); + metrics.updateAdapterRequestDisabledBidderMetric(CONVERSANT, Account.empty(ACCOUNT_ID)); + metrics.updateAdapterRequestDisabledBidderMetric(CONVERSANT, Account.empty(ACCOUNT_ID)); + + // then + assertThat(metricRegistry.counter("adapter.rubicon.requests.disabled_bidder").getCount()).isOne(); + assertThat(metricRegistry.counter( + "account.accountId.adapter.rubicon.requests.disabled_bidder").getCount()).isOne(); + assertThat(metricRegistry.counter( + "adapter.conversant.requests.disabled_bidder").getCount()).isEqualTo(2); + assertThat(metricRegistry.counter( + "account.accountId.adapter.conversant.requests.disabled_bidder").getCount()).isEqualTo(2); + } + @Test public void updateAdapterRequestErrorMetricShouldIncrementMetrics() { // when @@ -965,16 +999,22 @@ public void shouldNotUpdateAccountMetricsIfVerbosityIsNone() { metrics.updateAdapterRequestNobidMetrics(RUBICON, Account.empty(ACCOUNT_ID)); metrics.updateAdapterRequestGotbidsMetrics(RUBICON, Account.empty(ACCOUNT_ID)); metrics.updateAdapterBidMetrics(RUBICON, Account.empty(ACCOUNT_ID), 1234L, true, "banner"); + metrics.updateAdapterRequestDisabledBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); + metrics.updateAdapterRequestUnknownBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); // then assertThat(metricRegistry.counter("account.accountId.requests").getCount()).isZero(); assertThat(metricRegistry.counter("account.accountId.debug_requests").getCount()).isZero(); assertThat(metricRegistry.counter("account.accountId.requests.type.openrtb2-web").getCount()).isZero(); - assertThat(metricRegistry.timer("account.accountId.rubicon.request_time").getCount()).isZero(); - assertThat(metricRegistry.counter("account.accountId.rubicon.requests.nobid").getCount()).isZero(); - assertThat(metricRegistry.counter("account.accountId.rubicon.requests.gotbids").getCount()).isZero(); - assertThat(metricRegistry.histogram("account.accountId.rubicon.prices").getCount()).isZero(); - assertThat(metricRegistry.counter("account.accountId.rubicon.bids_received").getCount()).isZero(); + assertThat(metricRegistry.timer("account.accountId.adapter.rubicon.request_time").getCount()).isZero(); + assertThat(metricRegistry.counter("account.accountId.adapter.rubicon.requests.nobid").getCount()).isZero(); + assertThat(metricRegistry.counter("account.accountId.adapter.rubicon.requests.gotbids").getCount()).isZero(); + assertThat(metricRegistry.histogram("account.accountId.adapter.rubicon.prices").getCount()).isZero(); + assertThat(metricRegistry.counter("account.accountId.adapter.rubicon.bids_received").getCount()).isZero(); + assertThat(metricRegistry.counter( + "account.accountId.adapter.rubicon.requests.unknown_bidder").getCount()).isZero(); + assertThat(metricRegistry.counter( + "account.accountId.adapter.rubicon.requests.disabled_bidder").getCount()).isZero(); } @Test @@ -990,16 +1030,58 @@ public void shouldUpdateAccountRequestsMetricOnlyIfVerbosityIsBasic() { metrics.updateAdapterRequestNobidMetrics(RUBICON, Account.empty(ACCOUNT_ID)); metrics.updateAdapterRequestGotbidsMetrics(RUBICON, Account.empty(ACCOUNT_ID)); metrics.updateAdapterBidMetrics(RUBICON, Account.empty(ACCOUNT_ID), 1234L, true, "banner"); + metrics.updateAdapterRequestDisabledBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); + metrics.updateAdapterRequestUnknownBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); // then assertThat(metricRegistry.counter("account.accountId.requests").getCount()).isOne(); assertThat(metricRegistry.counter("account.accountId.debug_requests").getCount()).isZero(); assertThat(metricRegistry.counter("account.accountId.requests.type.openrtb2-web").getCount()).isZero(); assertThat(metricRegistry.timer("account.accountId.rubicon.request_time").getCount()).isZero(); - assertThat(metricRegistry.counter("account.accountId.rubicon.requests.nobid").getCount()).isZero(); - assertThat(metricRegistry.counter("account.accountId.rubicon.requests.gotbids").getCount()).isZero(); - assertThat(metricRegistry.histogram("account.accountId.rubicon.prices").getCount()).isZero(); - assertThat(metricRegistry.counter("account.accountId.rubicon.bids_received").getCount()).isZero(); + assertThat(metricRegistry.counter("account.accountId.adapter.rubicon.requests.nobid").getCount()).isZero(); + assertThat(metricRegistry.counter("account.accountId.adapter.rubicon.requests.gotbids").getCount()).isZero(); + assertThat(metricRegistry.histogram("account.accountId.adapter.rubicon.prices").getCount()).isZero(); + assertThat(metricRegistry.counter("account.accountId.adapter.rubicon.bids_received").getCount()).isZero(); + assertThat(metricRegistry.counter( + "account.accountId.adapter.rubicon.requests.unknown_bidder").getCount()).isZero(); + assertThat(metricRegistry.counter( + "account.accountId.adapter.rubicon.requests.disabled_bidder").getCount()).isZero(); + } + + @Test + public void shouldUpdateAccountRequestsMetricOnlyIfVerbosityIsDetailed() { + // given + given(accountMetricsVerbosityResolver.forAccount(any())).willReturn(AccountMetricsVerbosityLevel.detailed); + + // when + metrics.updateAccountRequestMetrics(Account.empty(ACCOUNT_ID), MetricName.openrtb2web); + metrics.updateAccountDebugRequestMetrics(Account.empty(ACCOUNT_ID), false); + metrics.updateAccountDebugRequestMetrics(Account.empty(ACCOUNT_ID), true); + metrics.updateAdapterResponseTime(RUBICON, Account.empty(ACCOUNT_ID), 500); + metrics.updateAdapterRequestNobidMetrics(RUBICON, Account.empty(ACCOUNT_ID)); + metrics.updateAdapterRequestGotbidsMetrics(RUBICON, Account.empty(ACCOUNT_ID)); + metrics.updateAdapterBidMetrics(RUBICON, Account.empty(ACCOUNT_ID), 1234L, true, "banner"); + metrics.updateAdapterRequestDisabledBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); + metrics.updateAdapterRequestUnknownBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); + + // then + assertThat(metricRegistry.counter("account.accountId.requests").getCount()).isOne(); + assertThat(metricRegistry.counter("account.accountId.debug_requests").getCount()) + .isEqualTo(1); + assertThat(metricRegistry.counter("account.accountId.requests.type.openrtb2-web").getCount()) + .isEqualTo(1); + assertThat(metricRegistry.counter("account.accountId.adapter.rubicon.requests.nobid").getCount()) + .isEqualTo(1); + assertThat(metricRegistry.counter("account.accountId.adapter.rubicon.requests.gotbids").getCount()) + .isEqualTo(1); + assertThat(metricRegistry.histogram("account.accountId.adapter.rubicon.prices").getCount()) + .isEqualTo(1); + assertThat(metricRegistry.counter("account.accountId.adapter.rubicon.bids_received").getCount()) + .isEqualTo(1); + assertThat(metricRegistry.counter( + "account.accountId.adapter.rubicon.requests.unknown_bidder").getCount()).isEqualTo(1); + assertThat(metricRegistry.counter( + "account.accountId.adapter.rubicon.requests.disabled_bidder").getCount()).isEqualTo(1); } @Test diff --git a/src/test/java/org/prebid/server/validation/RequestValidatorTest.java b/src/test/java/org/prebid/server/validation/RequestValidatorTest.java index e492ae8baeb..9e6306b144f 100644 --- a/src/test/java/org/prebid/server/validation/RequestValidatorTest.java +++ b/src/test/java/org/prebid/server/validation/RequestValidatorTest.java @@ -42,6 +42,7 @@ import org.prebid.server.proto.openrtb.ext.request.ExtUser; import org.prebid.server.proto.openrtb.ext.request.ExtUserPrebid; import org.prebid.server.proto.openrtb.ext.request.ImpMediaType; +import org.prebid.server.settings.model.Account; import org.prebid.server.validation.model.ValidationResult; import java.math.BigDecimal; @@ -67,6 +68,7 @@ public class RequestValidatorTest extends VertxTest { private static final String RUBICON = "rubicon"; + private static final String ACCOUNT_ID = "accountId"; @Mock(strictness = LENIENT) private BidderCatalog bidderCatalog; @@ -91,7 +93,7 @@ public void validateShouldReturnOnlyOneErrorAtATime() { final BidRequest bidRequest = BidRequest.builder().build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result).isNotNull(); @@ -104,7 +106,7 @@ public void validateShouldReturnValidationMessageWhenRequestIdIsEmpty() { final BidRequest bidRequest = validBidRequestBuilder().id("").build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -117,7 +119,7 @@ public void validateShouldReturnValidationMessageWhenRequestIdIsNull() { final BidRequest bidRequest = validBidRequestBuilder().id(null).build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -130,7 +132,7 @@ public void validateShouldReturnValidationMessageWhenTmaxIsNegative() { final BidRequest bidRequest = validBidRequestBuilder().id("1").tmax(-100L).build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -143,7 +145,7 @@ public void validateShouldNotReturnValidationMessageWhenTmaxIsNull() { final BidRequest bidRequest = validBidRequestBuilder().tmax(null).build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).isEmpty(); @@ -155,7 +157,7 @@ public void validateShouldReturnValidationMessageWhenCurIsNull() { final BidRequest bidRequest = validBidRequestBuilder().cur(null).build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -169,7 +171,7 @@ public void validateShouldReturnValidationMessageWhenNumberOfImpsIsZero() { final BidRequest bidRequest = validBidRequestBuilder().imp(null).build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -187,7 +189,7 @@ public void validateShouldReturnValidationMessageWhenAliasesKeyDoesntContainAlia .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()) @@ -205,7 +207,7 @@ public void validateShouldReturnValidationMessageWhenAliasgvlidsValueLowerThatOn .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()) @@ -219,7 +221,7 @@ public void validateShouldReturnValidationMessageWhenSiteIdAndPageIsNull() { final BidRequest bidRequest = validBidRequestBuilder().site(Site.builder().id(null).build()).build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -232,7 +234,7 @@ public void validateShouldReturnValidationMessageWhenSiteIdIsEmptyStringAndPageI final BidRequest bidRequest = validBidRequestBuilder().site(Site.builder().id("").build()).build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -245,7 +247,7 @@ public void validateShouldReturnEmptyValidationMessagesWhenPageIdIsNullAndSiteId final BidRequest bidRequest = validBidRequestBuilder().site(Site.builder().id("1").page(null).build()).build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.hasErrors()).isFalse(); @@ -257,7 +259,7 @@ public void validateShouldEmptyValidationMessagesWhenSitePageIsEmptyString() { final BidRequest bidRequest = validBidRequestBuilder().site(Site.builder().id("1").page("").build()).build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.hasErrors()).isFalse(); @@ -269,7 +271,7 @@ public void validateShouldReturnValidationMessageWhenSiteIdAndPageBothEmpty() { final BidRequest bidRequest = validBidRequestBuilder().site(Site.builder().id("").page("").build()).build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -284,7 +286,7 @@ public void validateShouldReturnValidationMessageWhenSiteExtAmpIsNegative() { .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -299,7 +301,7 @@ public void validateShouldReturnValidationMessageWhenSiteExtAmpIsGreaterThanOne( .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -313,7 +315,7 @@ public void validateShouldFailWhenDoohIdAndVenuetypeAreNulls() { final BidRequest bidRequest = validBidRequestBuilder().site(null).dooh(invalidDooh).build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -327,7 +329,7 @@ public void validateShouldFailWhenDoohIdIsNullAndVenuetypeIsEmpty() { final BidRequest bidRequest = validBidRequestBuilder().site(null).dooh(invalidDooh).build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -344,7 +346,7 @@ public void validateShouldReturnValidationMessageWhenRequestAppAndRequestSiteBot .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -360,7 +362,7 @@ public void validateShouldFailWhenDoohSiteAndAppArePresentInRequestAndStrictVali .app(App.builder().build()) .site(Site.builder().build()) .build(); - final ValidationResult result = target.validate(invalidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), invalidRequest, null, null); // then verify(metrics).updateAlertsMetrics(MetricName.general); @@ -377,7 +379,7 @@ public void validateShouldFailWhenSiteAndAppArePresentInRequestAndStrictValidati .app(App.builder().build()) .site(Site.builder().build()) .build(); - final ValidationResult result = target.validate(invalidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), invalidRequest, null, null); // then verify(metrics).updateAlertsMetrics(MetricName.general); @@ -394,7 +396,7 @@ public void validateShouldFailWhenDoohAndSiteArePresentInRequestAndStrictValidat .dooh(Dooh.builder().build()) .site(Site.builder().build()) .build(); - final ValidationResult result = target.validate(invalidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), invalidRequest, null, null); // then verify(metrics).updateAlertsMetrics(MetricName.general); @@ -411,7 +413,7 @@ public void validateShouldFailWhenDoohAndAppArePresentInRequestAndStrictValidati .dooh(Dooh.builder().build()) .app(App.builder().build()) .build(); - final ValidationResult result = target.validate(invalidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), invalidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -429,7 +431,7 @@ public void validateShouldReturnValidationMessageWhenMinWidthPercIsNull() { .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -446,7 +448,7 @@ public void validateShouldReturnValidationMessageWhenMinWidthPercIsLessThanZero( .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -463,7 +465,7 @@ public void validateShouldReturnValidationMessageWhenMinWidthPercGreaterThanHund .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -480,7 +482,7 @@ public void validateShouldReturnValidationMessageWhenMinHeightPercIsNull() { .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -498,7 +500,7 @@ public void validateShouldReturnValidationMessageWhenMinHeightPercIsLessThanZero .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -516,7 +518,7 @@ public void validateShouldReturnValidationMessageWhenMinHeightPercGreaterThanHun .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -530,7 +532,7 @@ public void validateShouldReturnEmptyValidationMessagesWhenBidRequestIsOk() { final BidRequest bidRequest = validBidRequestBuilder().build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).isEmpty(); @@ -549,7 +551,7 @@ public void validateShouldReturnValidationMessageWhenRequestHaveDuplicatedImpIds .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -566,7 +568,7 @@ public void validateShouldNotReturnValidationMessageIfUserExtIsEmptyJsonObject() .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).isEmpty(); @@ -580,7 +582,7 @@ public void validateShouldNotReturnErrorMessageWhenRegsIsEmptyObject() { .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).isEmpty(); @@ -598,7 +600,7 @@ public void validateShouldReturnValidationMessageWhenPrebidBuyerIdsContainsNoVal .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -616,7 +618,7 @@ public void validateShouldReturnValidationMessageWhenEidsPermissionsHasNullEleme .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -634,7 +636,7 @@ public void validateShouldReturnValidationMessageWhenEidsPermissionsBiddersIsNul .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -653,7 +655,7 @@ public void validateShouldReturnValidationMessageWhenEidsPermissionsBiddersIsEmp .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -675,7 +677,7 @@ public void validateShouldReturnWarningsMessageWhenEidsPermissionsBidderIsNotRec // when final ValidationResult result = target.validate( - bidRequest, null, DebugContext.of(true, false, null)); + Account.empty(ACCOUNT_ID), bidRequest, null, DebugContext.of(true, false, null)); // then assertThat(result.getErrors()).isEmpty(); @@ -698,7 +700,7 @@ public void validateShouldNotReturnWarningsMessageWhenEidsPermissionsBidderIsNot // when final ValidationResult result = target.validate( - bidRequest, null, DebugContext.of(false, false, null)); + Account.empty(ACCOUNT_ID), bidRequest, null, DebugContext.of(false, false, null)); // then assertThat(result.getErrors()).isEmpty(); @@ -718,7 +720,7 @@ public void validateShouldReturnWarningMessageWhenEidsPermissionsBidderHasBlankV // when final ValidationResult result = target.validate( - bidRequest, null, DebugContext.of(true, false, null)); + Account.empty(ACCOUNT_ID), bidRequest, null, DebugContext.of(true, false, null)); // then assertThat(result.getErrors()).isEmpty(); @@ -743,7 +745,7 @@ public void validateShouldNotReturnValidationErrorWhenBidderIsAlias() { .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).isEmpty(); @@ -761,7 +763,7 @@ public void validateShouldNotReturnValidationErrorWhenBidderIsAsterisk() { .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).isEmpty(); @@ -779,7 +781,7 @@ public void validateShouldReturnValidationMessageWhenEidsPermissionsHasMissingSo .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -798,7 +800,7 @@ public void validateShouldReturnValidationMessageWhenCantParseTargetingPriceGran .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -817,7 +819,7 @@ public void validateShouldReturnValidationMessageWhenRangesAreEmptyList() { .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -838,7 +840,7 @@ public void validateShouldReturnValidationMessageWhenIncrementIsZero() { .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -858,7 +860,7 @@ public void validateShouldReturnValidationMessageWhenIncrementIsMissed() { .build())) .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -879,7 +881,7 @@ public void validateShouldReturnValidationMessageWhenIncrementIsNegative() { .build())) .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -898,7 +900,7 @@ public void validateShouldReturnValidationMessageWhenPrecisionIsNegative() { .build())) .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -921,7 +923,7 @@ public void validateShouldReturnValidationMessageWhenMediaTypePriceGranularityTy .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -949,7 +951,7 @@ public void validateShouldReturnValidationMessageWithCorrectMediaType() { .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -976,7 +978,7 @@ public void validateShouldReturnValidationMessageForInvalidTargetingPrefix() { .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -998,7 +1000,7 @@ public void validateShouldReturnValidationMessageWhenRangesContainsMissedMaxValu .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -1019,7 +1021,7 @@ public void validateShouldReturnValidationMessageWhenRangesAreNotOrderedByMaxVal .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -1042,7 +1044,7 @@ public void validateShouldReturnValidationMessageWhenRangesAreNotOrderedByMaxVal .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -1064,7 +1066,7 @@ public void validateShouldReturnValidationMessageWhenIncrementIsNegativeInNotLea .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -1083,7 +1085,7 @@ public void validateShouldReturnValidationMessageWhenPrebidBuyerIdsContainsUnkno .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -1106,7 +1108,7 @@ public void validateShouldNotReturnAnyErrorInValidationResultWhenPrebidBuyerIdIs .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).isEmpty(); @@ -1124,7 +1126,7 @@ public void validateShouldNotReturnAnyErrorInValidationResultWhenPrebidBuyerIdIs .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).isEmpty(); @@ -1140,7 +1142,7 @@ public void validateShouldNotReturnValidationMessageWhenEidsIsEmpty() { .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).isEmpty(); @@ -1156,7 +1158,7 @@ public void validateShouldReturnValidationMessageWhenEidHasEmptySource() { .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -1172,7 +1174,7 @@ public void validateShouldReturnValidationMessageWhenAliasNameEqualsToBidderItPo final BidRequest bidRequest = validBidRequestBuilder().ext(ext).build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -1190,7 +1192,7 @@ public void validateShouldReturnValidationMessageWhenAliasPointOnNotValidBidderN final BidRequest bidRequest = validBidRequestBuilder().ext(ext).build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -1208,7 +1210,7 @@ public void validateShouldReturnValidationMessageWhenAliasPointOnDisabledBidder( given(bidderCatalog.isActive("appnexus")).willReturn(false); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -1224,7 +1226,7 @@ public void validateShouldReturnEmptyValidationMessagesWhenAliasesWasUsed() { final BidRequest bidRequest = validBidRequestBuilder().ext(ext).build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).isEmpty(); @@ -1238,7 +1240,7 @@ public void validateShouldReturnValidationResultWithErrorsWhenGdprIsNotOneOrZero .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -1257,7 +1259,7 @@ public void validateShouldReturnValidationMessageWhenAdjustmentFactorNegative() .build())) .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -1279,7 +1281,7 @@ public void validateShouldReturnValidationMessageWhenAdjustmentMediaFactorNegati .build())) .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -1301,7 +1303,7 @@ public void validateShouldReturnValidationMessageWhenBidderUnknown() { .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -1322,7 +1324,7 @@ public void validateShouldReturnValidationMessageWhenMediaBidderUnknown() { .build())) .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).hasSize(1) @@ -1346,7 +1348,7 @@ public void validateShouldReturnEmptyValidationMessagesWhenBidderIsKnownAndAdjus .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).isEmpty(); @@ -1370,7 +1372,7 @@ public void validateShouldReturnEmptyValidationMessagesWhenBidderIsKnownAliasFor .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).isEmpty(); @@ -1391,7 +1393,7 @@ public void validateShouldReturnEmptyValidationMessagesWhenBidderIsKnownBidderCo .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then verify(bidderCatalog).isValidName(rubiconAlias); @@ -1412,7 +1414,7 @@ public void validateShouldReturnValidationMessageWhenMultipleSchainsForSameBidde .build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()) @@ -1429,7 +1431,7 @@ public void validateShouldReturnValidationMessageWhenImpValidationFailed() throw final BidRequest bidRequest = validBidRequestBuilder().build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getErrors()).containsOnly("imp[0] validation failed"); @@ -1444,7 +1446,7 @@ public void validateShouldReturnWarningMessageWhenImpValidationWarns() throws Va final BidRequest bidRequest = validBidRequestBuilder().build(); // when - final ValidationResult result = target.validate(bidRequest, null, null); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then assertThat(result.getWarnings()).containsOnly("imp[0] validation warning"); From 9ac31e31e0648cc8258317d67847e04ea1fb86ba Mon Sep 17 00:00:00 2001 From: vmalitskyi Date: Tue, 4 Feb 2025 10:10:24 +0100 Subject: [PATCH 2/4] Fix fallen tests after rebase --- .../server/auction/requestfactory/Ortb2RequestFactory.java | 3 ++- .../server/auction/requestfactory/VideoRequestFactory.java | 1 - .../server/auction/requestfactory/AmpRequestFactoryTest.java | 2 +- .../auction/requestfactory/AuctionRequestFactoryTest.java | 4 ++-- .../auction/requestfactory/VideoRequestFactoryTest.java | 2 +- .../org/prebid/server/validation/RequestValidatorTest.java | 4 ++-- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactory.java b/src/main/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactory.java index eab0ac25213..c9ec70e8116 100644 --- a/src/main/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactory.java +++ b/src/main/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactory.java @@ -192,7 +192,8 @@ public Future activityInfrastructureFrom(AuctionContext auctionContext.getDebugContext().getTraceLevel())); } - public Future validateRequest(Account account, BidRequest bidRequest, + public Future validateRequest(Account account, + BidRequest bidRequest, HttpRequestContext httpRequestContext, DebugContext debugContext, List warnings) { diff --git a/src/main/java/org/prebid/server/auction/requestfactory/VideoRequestFactory.java b/src/main/java/org/prebid/server/auction/requestfactory/VideoRequestFactory.java index 38e60ba40e3..4493fb9601e 100644 --- a/src/main/java/org/prebid/server/auction/requestfactory/VideoRequestFactory.java +++ b/src/main/java/org/prebid/server/auction/requestfactory/VideoRequestFactory.java @@ -33,7 +33,6 @@ import org.prebid.server.model.HttpRequestContext; import org.prebid.server.proto.openrtb.ext.request.ExtPublisher; import org.prebid.server.proto.openrtb.ext.request.ExtPublisherPrebid; -import org.prebid.server.settings.model.Account; import org.prebid.server.util.HttpUtil; import org.prebid.server.util.ObjectUtil; diff --git a/src/test/java/org/prebid/server/auction/requestfactory/AmpRequestFactoryTest.java b/src/test/java/org/prebid/server/auction/requestfactory/AmpRequestFactoryTest.java index bcd26c7c162..0a59735ed07 100644 --- a/src/test/java/org/prebid/server/auction/requestfactory/AmpRequestFactoryTest.java +++ b/src/test/java/org/prebid/server/auction/requestfactory/AmpRequestFactoryTest.java @@ -1754,7 +1754,7 @@ private void givenBidRequest( given(ortb2ImplicitParametersResolver.resolve(any(), any(), any(), anyBoolean())).willAnswer( answerWithFirstArgument()); given(ortb2RequestFactory.validateRequest(any(), any(), any(), any(), any())) - .willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(0))); + .willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(1))); given(ortb2RequestFactory.enrichBidRequestWithAccountAndPrivacyData(any())) .willAnswer(invocation -> Future.succeededFuture(((AuctionContext) invocation.getArgument(0)) diff --git a/src/test/java/org/prebid/server/auction/requestfactory/AuctionRequestFactoryTest.java b/src/test/java/org/prebid/server/auction/requestfactory/AuctionRequestFactoryTest.java index fa9ad3be13c..60a9f35c603 100644 --- a/src/test/java/org/prebid/server/auction/requestfactory/AuctionRequestFactoryTest.java +++ b/src/test/java/org/prebid/server/auction/requestfactory/AuctionRequestFactoryTest.java @@ -166,7 +166,7 @@ public void setUp() { .willAnswer(invocation -> Future.succeededFuture( ((AuctionContext) invocation.getArgument(0)).getBidRequest())); given(ortb2RequestFactory.validateRequest(any(), any(), any(), any(), any())) - .willAnswer(invocationOnMock -> Future.succeededFuture((BidRequest) invocationOnMock.getArgument(0))); + .willAnswer(invocationOnMock -> Future.succeededFuture((BidRequest) invocationOnMock.getArgument(1))); given(ortb2RequestFactory.removeEmptyEids(any(), any())) .willAnswer(invocationOnMock -> invocationOnMock.getArgument(0)); given(ortb2RequestFactory.updateTimeout(any())).willAnswer(invocation -> invocation.getArgument(0)); @@ -662,7 +662,7 @@ public void shouldReturnFailedFutureIfRequestValidationFailed() { // given givenValidBidRequest(); - given(ortb2RequestFactory.validateRequest(any(), any(), any())) + given(ortb2RequestFactory.validateRequest(any(), any(), any(), any(), any())) .willReturn(Future.failedFuture(new InvalidRequestException("errors"))); // when diff --git a/src/test/java/org/prebid/server/auction/requestfactory/VideoRequestFactoryTest.java b/src/test/java/org/prebid/server/auction/requestfactory/VideoRequestFactoryTest.java index 74521b05c07..d5ebd38843e 100644 --- a/src/test/java/org/prebid/server/auction/requestfactory/VideoRequestFactoryTest.java +++ b/src/test/java/org/prebid/server/auction/requestfactory/VideoRequestFactoryTest.java @@ -405,7 +405,7 @@ private void givenBidRequest(BidRequest bidRequest, List podErrors) { given(ortb2RequestFactory.fetchAccountWithoutStoredRequestLookup(any())).willReturn(Future.succeededFuture()); given(ortb2RequestFactory.validateRequest(any(), any(), any(), any(), any())) - .willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(0))); + .willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(1))); given(paramsResolver.resolve(any(), any(), any(), anyBoolean())) .willAnswer(answerWithFirstArgument()); diff --git a/src/test/java/org/prebid/server/validation/RequestValidatorTest.java b/src/test/java/org/prebid/server/validation/RequestValidatorTest.java index 9e6306b144f..b45b074717f 100644 --- a/src/test/java/org/prebid/server/validation/RequestValidatorTest.java +++ b/src/test/java/org/prebid/server/validation/RequestValidatorTest.java @@ -1195,7 +1195,7 @@ public void validateShouldReturnValidationMessageWhenAliasPointOnNotValidBidderN final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then - assertThat(result.getErrors()).hasSize(1) + assertThat(result.getWarnings()).hasSize(1) .containsOnly("request.ext.prebid.aliases.alias refers to unknown bidder: fake"); } @@ -1213,7 +1213,7 @@ public void validateShouldReturnValidationMessageWhenAliasPointOnDisabledBidder( final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then - assertThat(result.getErrors()).hasSize(1) + assertThat(result.getWarnings()).hasSize(1) .containsOnly("request.ext.prebid.aliases.alias refers to disabled bidder: appnexus"); } From 3119f39950f6e3192ca5830d140c920300331940 Mon Sep 17 00:00:00 2001 From: vmalitskyi Date: Tue, 4 Feb 2025 10:11:14 +0100 Subject: [PATCH 3/4] Introduce hard-fail mode switchers and update metrics gathering --- docs/config-app.md | 4 ++ docs/metrics.md | 4 ++ .../org/prebid/server/metric/Metrics.java | 12 ++--- .../spring/config/ServiceConfiguration.java | 8 ++- .../server/validation/RequestValidator.java | 32 +++++++++--- src/main/resources/application.yaml | 2 + .../org/prebid/server/metric/MetricsTest.java | 48 +++++++++--------- .../validation/RequestValidatorTest.java | 50 ++++++++++++++++--- 8 files changed, 113 insertions(+), 47 deletions(-) diff --git a/docs/config-app.md b/docs/config-app.md index c037a74c482..51dd71cec1a 100644 --- a/docs/config-app.md +++ b/docs/config-app.md @@ -302,6 +302,10 @@ Preconfigured application settings can be obtained from multiple data sources co Warning! Application will not start in case of no one data source is defined and you'll get an exception in logs. +For requests validation mode available next options: +- `settings.fail-on-unknown-bidders` - fail with validation error or just make warning for unknown bidders. +- `settings.fail-on-disabled-bidders` - fail with validation error or just make warning for disabled bidders. + For filesystem data source available next options: - `settings.filesystem.settings-filename` - location of file settings. - `settings.filesystem.stored-requests-dir` - directory with stored requests. diff --git a/docs/metrics.md b/docs/metrics.md index 85df92bc269..fa540bc36e1 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -45,6 +45,8 @@ where `[DATASOURCE]` is a data source name, `DEFAULT_DS` by defaul. - `imps_video` - number of video impressions - `imps_native` - number of native impressions - `imps_audio` - number of audio impressions +- `disabled_bidder` - number of disabled bidders received within requests +- `unknown_bidder` - number of unknown bidders received within requests - `requests.(ok|badinput|err|networkerr|blocklisted_account|blocklisted_app).(openrtb2-web|openrtb-app|amp|legacy)` - number of requests broken down by status and type - `bidder-cardinality..requests` - number of requests targeting `` of bidders - `connection_accept_errors` - number of errors occurred while establishing HTTP connection @@ -92,6 +94,8 @@ Following metrics are collected and submitted if account is configured with `det - `account..requests.type.(openrtb2-web,openrtb-app,amp,legacy)` - number of requests received from account with `` broken down by type of incoming request - `account..debug_requests` - number of requests received from account with `` broken down by type of incoming request (when debug mode is enabled) - `account..requests.rejected` - number of rejected requests caused by incorrect `accountId` +- `account..requests.disabled_bidder` - number of disabled bidders received within requests from account with `` +- `account..requests.unknown_bidder` - number of unknown bidder names received within requests from account with `` - `account..adapter..request_time` - timer tracking how long did it take to make a request to `` when incoming request was from `` - `account..adapter..bids_received` - number of bids received from `` when incoming request was from `` - `account..adapter..requests.(gotbids|nobid)` - number of requests made to `` broken down by result status when incoming request was from `` diff --git a/src/main/java/org/prebid/server/metric/Metrics.java b/src/main/java/org/prebid/server/metric/Metrics.java index 4c001af4a3a..b4f3a5af9c3 100644 --- a/src/main/java/org/prebid/server/metric/Metrics.java +++ b/src/main/java/org/prebid/server/metric/Metrics.java @@ -336,19 +336,19 @@ public void updateAdapterRequestErrorMetric(String bidder, MetricName errorMetri forAdapter(bidder).request().incCounter(errorMetric); } - public void updateAdapterRequestDisabledBidderMetric(String bidder, Account account) { - forAdapter(bidder).request().incCounter(MetricName.disabled_bidder); + public void updateDisabledBidderMetric(Account account) { + incCounter(MetricName.disabled_bidder); if (accountMetricsVerbosityResolver.forAccount(account) .isAtLeast(AccountMetricsVerbosityLevel.detailed)) { - forAccount(account.getId()).adapter().forAdapter(bidder).request().incCounter(MetricName.disabled_bidder); + forAccount(account.getId()).requests().incCounter(MetricName.disabled_bidder); } } - public void updateAdapterRequestUnknownBidderMetric(String bidder, Account account) { - forAdapter(bidder).request().incCounter(MetricName.unknown_bidder); + public void updateUnknownBidderMetric(Account account) { + incCounter(MetricName.unknown_bidder); if (accountMetricsVerbosityResolver.forAccount(account) .isAtLeast(AccountMetricsVerbosityLevel.detailed)) { - forAccount(account.getId()).adapter().forAdapter(bidder).request().incCounter(MetricName.unknown_bidder); + forAccount(account.getId()).requests().incCounter(MetricName.unknown_bidder); } } diff --git a/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java b/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java index 24522e4cdd6..82a2d7dcee0 100644 --- a/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java +++ b/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java @@ -1033,7 +1033,9 @@ RequestValidator requestValidator( Metrics metrics, JacksonMapper mapper, @Value("${logging.sampling-rate:0.01}") double logSamplingRate, - @Value("${auction.strict-app-site-dooh:false}") boolean enabledStrictAppSiteDoohValidation) { + @Value("${auction.strict-app-site-dooh:false}") boolean enabledStrictAppSiteDoohValidation, + @Value("${settings.fail-on-disabled-bidders}") boolean failOnDisabledBidders, + @Value("${settings.fail-on-unknown-bidders}") boolean failOnUnknownBidders) { return new RequestValidator( bidderCatalog, @@ -1041,7 +1043,9 @@ RequestValidator requestValidator( metrics, mapper, logSamplingRate, - enabledStrictAppSiteDoohValidation); + enabledStrictAppSiteDoohValidation, + failOnDisabledBidders, + failOnUnknownBidders); } @Bean diff --git a/src/main/java/org/prebid/server/validation/RequestValidator.java b/src/main/java/org/prebid/server/validation/RequestValidator.java index 66c19983197..c810227889e 100644 --- a/src/main/java/org/prebid/server/validation/RequestValidator.java +++ b/src/main/java/org/prebid/server/validation/RequestValidator.java @@ -75,6 +75,8 @@ public class RequestValidator { private final JacksonMapper mapper; private final double logSamplingRate; private final boolean enabledStrictAppSiteDoohValidation; + private final boolean failOnDisabledBidders; + private final boolean failOnUnknownBidders; /** * Constructs a RequestValidator that will use the BidderParamValidator passed in order to validate all critical @@ -84,7 +86,9 @@ public RequestValidator(BidderCatalog bidderCatalog, ImpValidator impValidator, Metrics metrics, JacksonMapper mapper, double logSamplingRate, - boolean enabledStrictAppSiteDoohValidation) { + boolean enabledStrictAppSiteDoohValidation, + boolean failOnDisabledBidders, + boolean failOnUnknownBidders) { this.bidderCatalog = Objects.requireNonNull(bidderCatalog); this.impValidator = Objects.requireNonNull(impValidator); @@ -92,6 +96,8 @@ public RequestValidator(BidderCatalog bidderCatalog, this.mapper = Objects.requireNonNull(mapper); this.logSamplingRate = logSamplingRate; this.enabledStrictAppSiteDoohValidation = enabledStrictAppSiteDoohValidation; + this.failOnDisabledBidders = failOnDisabledBidders; + this.failOnUnknownBidders = failOnUnknownBidders; } /** @@ -514,13 +520,25 @@ private void validateAliases(Map aliases, List warnings, final String alias = aliasToBidder.getKey(); final String coreBidder = aliasToBidder.getValue(); if (!bidderCatalog.isValidName(coreBidder)) { - warnings.add( - "request.ext.prebid.aliases.%s refers to unknown bidder: %s".formatted(alias, coreBidder)); - metrics.updateAdapterRequestUnknownBidderMetric(coreBidder, account); + metrics.updateUnknownBidderMetric(account); + + final String message = String.format("request.ext.prebid.aliases.%s refers to unknown bidder: %s", + alias, coreBidder); + if (failOnUnknownBidders) { + throw new ValidationException(message); + } else { + warnings.add(message); + } } else if (!bidderCatalog.isActive(coreBidder)) { - warnings.add( - "request.ext.prebid.aliases.%s refers to disabled bidder: %s".formatted(alias, coreBidder)); - metrics.updateAdapterRequestDisabledBidderMetric(coreBidder, account); + metrics.updateDisabledBidderMetric(account); + + final String message = String.format("request.ext.prebid.aliases.%s refers to disabled bidder: %s", + alias, coreBidder); + if (failOnDisabledBidders) { + throw new ValidationException(message); + } else { + warnings.add(message); + } } if (alias.equals(coreBidder)) { diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml index 31efd30851b..4bd8ab75332 100644 --- a/src/main/resources/application.yaml +++ b/src/main/resources/application.yaml @@ -158,6 +158,8 @@ currency-converter: settings: generate-storedrequest-bidrequest-id: false enforce-valid-account: false + fail-on-unknown-bidders: true + fail-on-disabled-bidders: true database: pool-size: 20 idle-connection-timeout: 300 diff --git a/src/test/java/org/prebid/server/metric/MetricsTest.java b/src/test/java/org/prebid/server/metric/MetricsTest.java index 77e9fe0956d..f40731dcb3c 100644 --- a/src/test/java/org/prebid/server/metric/MetricsTest.java +++ b/src/test/java/org/prebid/server/metric/MetricsTest.java @@ -48,6 +48,7 @@ public class MetricsTest { private static final String RUBICON = "rubicon"; private static final String CONVERSANT = "conversant"; private static final String ACCOUNT_ID = "accountId"; + private static final String ACCOUNT_ID_1 = "accountId1"; private static final String ANALYTIC_CODE = "analyticCode"; private MetricRegistry metricRegistry; @@ -615,37 +616,34 @@ public void updateAdapterBidMetricsShouldUpdateMetrics() { } @Test - public void updateAdapterRequestUnknownBidderMetricsShouldIncrementMetrics() { + public void updateUnknownBidderMetricsShouldIncrementMetrics() { // when - metrics.updateAdapterRequestUnknownBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); - metrics.updateAdapterRequestUnknownBidderMetric(CONVERSANT, Account.empty(ACCOUNT_ID)); - metrics.updateAdapterRequestUnknownBidderMetric(CONVERSANT, Account.empty(ACCOUNT_ID)); + metrics.updateUnknownBidderMetric(Account.empty(ACCOUNT_ID)); + metrics.updateUnknownBidderMetric(Account.empty(ACCOUNT_ID)); + metrics.updateUnknownBidderMetric(Account.empty(ACCOUNT_ID_1)); // then - assertThat(metricRegistry.counter("adapter.rubicon.requests.unknown_bidder").getCount()).isOne(); + assertThat(metricRegistry.counter("unknown_bidder").getCount()).isEqualTo(3); assertThat(metricRegistry.counter( - "account.accountId.adapter.rubicon.requests.unknown_bidder").getCount()).isOne(); + "account.accountId.requests.unknown_bidder").getCount()).isEqualTo(2); assertThat(metricRegistry.counter( - "adapter.conversant.requests.unknown_bidder").getCount()).isEqualTo(2); - assertThat(metricRegistry.counter( - "account.accountId.adapter.conversant.requests.unknown_bidder").getCount()).isEqualTo(2); + "account.accountId1.requests.unknown_bidder").getCount()).isEqualTo(1); } @Test - public void updateAdapterRequestDisabledBidderMetricsShouldIncrementMetrics() { + public void updateDisabledBidderMetricsShouldIncrementMetrics() { // when - metrics.updateAdapterRequestDisabledBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); - metrics.updateAdapterRequestDisabledBidderMetric(CONVERSANT, Account.empty(ACCOUNT_ID)); - metrics.updateAdapterRequestDisabledBidderMetric(CONVERSANT, Account.empty(ACCOUNT_ID)); + metrics.updateDisabledBidderMetric(Account.empty(ACCOUNT_ID)); + metrics.updateDisabledBidderMetric(Account.empty(ACCOUNT_ID)); + metrics.updateDisabledBidderMetric(Account.empty(ACCOUNT_ID_1)); // then - assertThat(metricRegistry.counter("adapter.rubicon.requests.disabled_bidder").getCount()).isOne(); assertThat(metricRegistry.counter( - "account.accountId.adapter.rubicon.requests.disabled_bidder").getCount()).isOne(); + "disabled_bidder").getCount()).isEqualTo(3); assertThat(metricRegistry.counter( - "adapter.conversant.requests.disabled_bidder").getCount()).isEqualTo(2); + "account.accountId.requests.disabled_bidder").getCount()).isEqualTo(2); assertThat(metricRegistry.counter( - "account.accountId.adapter.conversant.requests.disabled_bidder").getCount()).isEqualTo(2); + "account.accountId1.requests.disabled_bidder").getCount()).isEqualTo(1); } @Test @@ -999,8 +997,8 @@ public void shouldNotUpdateAccountMetricsIfVerbosityIsNone() { metrics.updateAdapterRequestNobidMetrics(RUBICON, Account.empty(ACCOUNT_ID)); metrics.updateAdapterRequestGotbidsMetrics(RUBICON, Account.empty(ACCOUNT_ID)); metrics.updateAdapterBidMetrics(RUBICON, Account.empty(ACCOUNT_ID), 1234L, true, "banner"); - metrics.updateAdapterRequestDisabledBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); - metrics.updateAdapterRequestUnknownBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); + metrics.updateDisabledBidderMetric(Account.empty(ACCOUNT_ID)); + metrics.updateUnknownBidderMetric(Account.empty(ACCOUNT_ID)); // then assertThat(metricRegistry.counter("account.accountId.requests").getCount()).isZero(); @@ -1030,8 +1028,8 @@ public void shouldUpdateAccountRequestsMetricOnlyIfVerbosityIsBasic() { metrics.updateAdapterRequestNobidMetrics(RUBICON, Account.empty(ACCOUNT_ID)); metrics.updateAdapterRequestGotbidsMetrics(RUBICON, Account.empty(ACCOUNT_ID)); metrics.updateAdapterBidMetrics(RUBICON, Account.empty(ACCOUNT_ID), 1234L, true, "banner"); - metrics.updateAdapterRequestDisabledBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); - metrics.updateAdapterRequestUnknownBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); + metrics.updateDisabledBidderMetric(Account.empty(ACCOUNT_ID)); + metrics.updateUnknownBidderMetric(Account.empty(ACCOUNT_ID)); // then assertThat(metricRegistry.counter("account.accountId.requests").getCount()).isOne(); @@ -1061,8 +1059,8 @@ public void shouldUpdateAccountRequestsMetricOnlyIfVerbosityIsDetailed() { metrics.updateAdapterRequestNobidMetrics(RUBICON, Account.empty(ACCOUNT_ID)); metrics.updateAdapterRequestGotbidsMetrics(RUBICON, Account.empty(ACCOUNT_ID)); metrics.updateAdapterBidMetrics(RUBICON, Account.empty(ACCOUNT_ID), 1234L, true, "banner"); - metrics.updateAdapterRequestDisabledBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); - metrics.updateAdapterRequestUnknownBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); + metrics.updateDisabledBidderMetric(Account.empty(ACCOUNT_ID)); + metrics.updateUnknownBidderMetric(Account.empty(ACCOUNT_ID)); // then assertThat(metricRegistry.counter("account.accountId.requests").getCount()).isOne(); @@ -1079,9 +1077,9 @@ public void shouldUpdateAccountRequestsMetricOnlyIfVerbosityIsDetailed() { assertThat(metricRegistry.counter("account.accountId.adapter.rubicon.bids_received").getCount()) .isEqualTo(1); assertThat(metricRegistry.counter( - "account.accountId.adapter.rubicon.requests.unknown_bidder").getCount()).isEqualTo(1); + "unknown_bidder").getCount()).isEqualTo(1); assertThat(metricRegistry.counter( - "account.accountId.adapter.rubicon.requests.disabled_bidder").getCount()).isEqualTo(1); + "account.accountId.requests.disabled_bidder").getCount()).isEqualTo(1); } @Test diff --git a/src/test/java/org/prebid/server/validation/RequestValidatorTest.java b/src/test/java/org/prebid/server/validation/RequestValidatorTest.java index b45b074717f..e2eb6a0d205 100644 --- a/src/test/java/org/prebid/server/validation/RequestValidatorTest.java +++ b/src/test/java/org/prebid/server/validation/RequestValidatorTest.java @@ -84,7 +84,7 @@ public void setUp() { given(bidderCatalog.isValidName(eq(RUBICON))).willReturn(true); given(bidderCatalog.isActive(eq(RUBICON))).willReturn(true); - target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, false); + target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, false, true, true); } @Test @@ -356,7 +356,7 @@ public void validateShouldReturnValidationMessageWhenRequestAppAndRequestSiteBot @Test public void validateShouldFailWhenDoohSiteAndAppArePresentInRequestAndStrictValidationIsEnabled() { // when - target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, true); + target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, true, true, true); final BidRequest invalidRequest = validBidRequestBuilder() .dooh(Dooh.builder().build()) .app(App.builder().build()) @@ -374,7 +374,7 @@ public void validateShouldFailWhenDoohSiteAndAppArePresentInRequestAndStrictVali @Test public void validateShouldFailWhenSiteAndAppArePresentInRequestAndStrictValidationIsEnabled() { // when - target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, true); + target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, true, true, true); final BidRequest invalidRequest = validBidRequestBuilder() .app(App.builder().build()) .site(Site.builder().build()) @@ -391,7 +391,7 @@ public void validateShouldFailWhenSiteAndAppArePresentInRequestAndStrictValidati @Test public void validateShouldFailWhenDoohAndSiteArePresentInRequestAndStrictValidationIsEnabled() { // when - target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, true); + target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, true, true, true); final BidRequest invalidRequest = validBidRequestBuilder() .dooh(Dooh.builder().build()) .site(Site.builder().build()) @@ -408,7 +408,7 @@ public void validateShouldFailWhenDoohAndSiteArePresentInRequestAndStrictValidat @Test public void validateShouldFailWhenDoohAndAppArePresentInRequestAndStrictValidationIsEnabled() { // when - target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, true); + target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, true, true, true); final BidRequest invalidRequest = validBidRequestBuilder() .dooh(Dooh.builder().build()) .app(App.builder().build()) @@ -1184,7 +1184,7 @@ public void validateShouldReturnValidationMessageWhenAliasNameEqualsToBidderItPo } @Test - public void validateShouldReturnValidationMessageWhenAliasPointOnNotValidBidderName() { + public void validateShouldReturnValidationErrorMessageWhenAliasPointOnNotValidBidderName() { // given final ExtRequest ext = ExtRequest.of(ExtRequestPrebid.builder() .aliases(singletonMap("alias", "fake")) @@ -1194,13 +1194,48 @@ public void validateShouldReturnValidationMessageWhenAliasPointOnNotValidBidderN // when final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); + // then + assertThat(result.getErrors()).hasSize(1) + .containsOnly("request.ext.prebid.aliases.alias refers to unknown bidder: fake"); + } + + @Test + public void validateShouldReturnValidationWarningMessageWhenAliasPointOnNotValidBidderName() { + // given + final ExtRequest ext = ExtRequest.of(ExtRequestPrebid.builder() + .aliases(singletonMap("alias", "fake")) + .build()); + final BidRequest bidRequest = validBidRequestBuilder().ext(ext).build(); + + // when + target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, false, true, false); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); + // then assertThat(result.getWarnings()).hasSize(1) .containsOnly("request.ext.prebid.aliases.alias refers to unknown bidder: fake"); } @Test - public void validateShouldReturnValidationMessageWhenAliasPointOnDisabledBidder() { + public void validateShouldReturnValidationErrorMessageWhenAliasPointOnDisabledBidder() { + // given + final ExtRequest ext = ExtRequest.of(ExtRequestPrebid.builder() + .aliases(singletonMap("alias", "appnexus")) + .build()); + final BidRequest bidRequest = validBidRequestBuilder().ext(ext).build(); + given(bidderCatalog.isValidName("appnexus")).willReturn(true); + given(bidderCatalog.isActive("appnexus")).willReturn(false); + + // when + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); + + // then + assertThat(result.getErrors()).hasSize(1) + .containsOnly("request.ext.prebid.aliases.alias refers to disabled bidder: appnexus"); + } + + @Test + public void validateShouldReturnValidationWarningMessageWhenAliasPointOnDisabledBidder() { // given final ExtRequest ext = ExtRequest.of(ExtRequestPrebid.builder() .aliases(singletonMap("alias", "appnexus")) @@ -1210,6 +1245,7 @@ public void validateShouldReturnValidationMessageWhenAliasPointOnDisabledBidder( given(bidderCatalog.isActive("appnexus")).willReturn(false); // when + target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, false, false, true); final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then From 73230993850be337f65c5fd590da493aa809fef7 Mon Sep 17 00:00:00 2001 From: vmalitskyi Date: Mon, 10 Feb 2025 09:58:37 +0100 Subject: [PATCH 4/4] Code cleanup --- .../server/spring/config/ServiceConfiguration.java | 4 ++-- .../prebid/server/validation/RequestValidator.java | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java b/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java index 82a2d7dcee0..e0b00fb9623 100644 --- a/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java +++ b/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java @@ -1034,8 +1034,8 @@ RequestValidator requestValidator( JacksonMapper mapper, @Value("${logging.sampling-rate:0.01}") double logSamplingRate, @Value("${auction.strict-app-site-dooh:false}") boolean enabledStrictAppSiteDoohValidation, - @Value("${settings.fail-on-disabled-bidders}") boolean failOnDisabledBidders, - @Value("${settings.fail-on-unknown-bidders}") boolean failOnUnknownBidders) { + @Value("${settings.fail-on-disabled-bidders:true}") boolean failOnDisabledBidders, + @Value("${settings.fail-on-unknown-bidders:true}") boolean failOnUnknownBidders) { return new RequestValidator( bidderCatalog, diff --git a/src/main/java/org/prebid/server/validation/RequestValidator.java b/src/main/java/org/prebid/server/validation/RequestValidator.java index c810227889e..d4838b1a5e3 100644 --- a/src/main/java/org/prebid/server/validation/RequestValidator.java +++ b/src/main/java/org/prebid/server/validation/RequestValidator.java @@ -134,7 +134,7 @@ public ValidationResult validate(Account account, validateTargeting(targeting); } aliases = ObjectUtils.defaultIfNull(extRequestPrebid.getAliases(), Collections.emptyMap()); - validateAliases(aliases, warnings, metrics, account); + validateAliases(aliases, warnings, account); validateAliasesGvlIds(extRequestPrebid, aliases); validateBidAdjustmentFactors(extRequestPrebid.getBidadjustmentfactors(), aliases); validateExtBidPrebidData(extRequestPrebid.getData(), aliases, isDebugEnabled, warnings); @@ -514,7 +514,7 @@ private static void validateGranularityRangeIncrement(ExtGranularityRange range) * is equals to itself. */ private void validateAliases(Map aliases, List warnings, - Metrics metrics, Account account) throws ValidationException { + Account account) throws ValidationException { for (final Map.Entry aliasToBidder : aliases.entrySet()) { final String alias = aliasToBidder.getKey(); @@ -522,8 +522,8 @@ private void validateAliases(Map aliases, List warnings, if (!bidderCatalog.isValidName(coreBidder)) { metrics.updateUnknownBidderMetric(account); - final String message = String.format("request.ext.prebid.aliases.%s refers to unknown bidder: %s", - alias, coreBidder); + final String message = "request.ext.prebid.aliases.%s refers to unknown bidder: %s".formatted(alias, + coreBidder); if (failOnUnknownBidders) { throw new ValidationException(message); } else { @@ -532,8 +532,8 @@ private void validateAliases(Map aliases, List warnings, } else if (!bidderCatalog.isActive(coreBidder)) { metrics.updateDisabledBidderMetric(account); - final String message = String.format("request.ext.prebid.aliases.%s refers to disabled bidder: %s", - alias, coreBidder); + final String message = "request.ext.prebid.aliases.%s refers to disabled bidder: %s".formatted(alias, + coreBidder); if (failOnDisabledBidders) { throw new ValidationException(message); } else {