Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add bidder error bid validation type #1107

Merged
merged 14 commits into from
Mar 22, 2022
Merged
24 changes: 18 additions & 6 deletions src/main/java/org/prebid/server/auction/ExchangeService.java
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@
import org.prebid.server.settings.model.Account;
import org.prebid.server.util.DealUtil;
import org.prebid.server.util.LineItemUtil;
import org.prebid.server.util.ObjectUtil;
import org.prebid.server.util.StreamUtil;
import org.prebid.server.validation.ResponseBidValidator;
import org.prebid.server.validation.model.ValidationResult;
Expand All @@ -127,6 +128,7 @@
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;

/**
* Executes an OpenRTB v2.5 Auction.
Expand Down Expand Up @@ -1297,17 +1299,18 @@ private AuctionParticipation validBidderResponse(AuctionParticipation auctionPar
final ValidationResult validationResult =
responseBidValidator.validate(bid, bidderResponse.getBidder(), auctionContext, aliases);

if (validationResult.hasWarnings()) {
addAsBidderErrors(validationResult.getWarnings(), errors);
if (validationResult.hasWarnings() || validationResult.hasErrors()) {
errors.add(makeValidationBidderError(bid.getBid(), validationResult));
}

if (validationResult.hasErrors()) {
addAsBidderErrors(validationResult.getErrors(), errors);
maybeRecordInTxnLog(lineItemId, txnLog::lineItemsResponseInvalidated);
continue;
}

validBids.add(bid);
if (!validationResult.hasErrors()) {
validBids.add(bid);
}
}

final BidderResponse resultBidderResponse = errors.isEmpty()
Expand All @@ -1316,8 +1319,14 @@ private AuctionParticipation validBidderResponse(AuctionParticipation auctionPar
return auctionParticipation.with(resultBidderResponse);
}

private void addAsBidderErrors(List<String> messages, List<BidderError> errors) {
messages.stream().map(BidderError::generic).forEach(errors::add);
private BidderError makeValidationBidderError(Bid bid, ValidationResult validationResult) {
final String validationErrors = Stream.concat(
validationResult.getErrors().stream().map(message -> "Error: " + message),
validationResult.getWarnings().stream().map(message -> "Warning: " + message))
.collect(Collectors.joining(". "));

final String bidId = ObjectUtil.getIfNotNullOrDefault(bid, Bid::getId, () -> "unknown");
return BidderError.invalidBid("BidId `" + bidId + "` validation messages: " + validationErrors);
}

private static void maybeRecordInTxnLog(String lineItemId, Supplier<Set<String>> metricSupplier) {
Expand Down Expand Up @@ -1587,6 +1596,9 @@ private static MetricName bidderErrorTypeToMetric(BidderError.Type errorType) {
case timeout:
errorMetric = MetricName.timeout;
break;
case invalid_bid:
errorMetric = MetricName.bid_validation;
break;
case generic:
default:
errorMetric = MetricName.unknown_error;
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/org/prebid/server/bidder/model/BidderError.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ public static BidderError generic(String message) {
return BidderError.of(message, Type.generic);
}

public static BidderError invalidBid(String message) {
return BidderError.of(message, Type.invalid_bid);
}

public static BidderError badInput(String message) {
return BidderError.of(message, Type.bad_input);
}
Expand Down Expand Up @@ -69,6 +73,12 @@ public enum Type {
*/
failed_to_request_bids(4),

/**
* Covers the case where a bid does not pass validation with error or warnings. One instance per invalid bid
* created with aggregation for all warnings and errors.
*/
invalid_bid(5),

timeout(1),
generic(999);

Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/prebid/server/metric/MetricName.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ public enum MetricName {
badserverresponse,
failedtorequestbids,
timeout,
bid_validation,
unknown_error,
err,
networkerr,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,12 @@ class PgBidResponseSpec extends BasePgSpec {
def auctionResponse = pgPbsService.sendAuctionRequest(bidRequest)

then: "Bidder response is invalid"
def bidderError = auctionResponse.ext?.errors?.get(GENERIC)
assert bidderError?.size() == 1
assert bidderError[0].message.startsWith("Bid \"${bidResponse.seatbid[0].bid[0].id}\" has no corresponding imp in request")
def bidderErrors = auctionResponse.ext?.errors?.get(GENERIC)
def bidId = bidResponse.seatbid[0].bid[0].id

assert bidderErrors?.size() == 1
assert bidderErrors[0].message ==~
/BidId `$bidId` validation messages:.* Error: Bid \"$bidId\" has no corresponding imp in request.*/
}

def "PBS should invalidate bidder response when deal id doesn't match to the bid request deal id"() {
Expand All @@ -87,9 +90,12 @@ class PgBidResponseSpec extends BasePgSpec {
def auctionResponse = pgPbsService.sendAuctionRequest(bidRequest)

then: "Bidder response is invalid"
def bidderError = auctionResponse.ext?.errors?.get(GENERIC)
assert bidderError?.size() == 1
assert bidderError[0].message.startsWith("WARNING: Bid \"${bidResponse.seatbid[0].bid[0].id}\" has 'dealid' not present in corresponding imp in request.")
def bidderErrors = auctionResponse.ext?.errors?.get(GENERIC)
def bidId = bidResponse.seatbid[0].bid[0].id

assert bidderErrors?.size() == 1
assert bidderErrors[0].message ==~ /BidId `$bidId` validation messages:.* Warning: / +
/WARNING: Bid "$bidId" has 'dealid' not present in corresponding imp in request.*/
}

def "PBS should invalidate bidder response when non-matched to the bid request size is returned"() {
Expand Down Expand Up @@ -120,10 +126,12 @@ class PgBidResponseSpec extends BasePgSpec {
assert auctionResponse.ext?.debug?.pgmetrics?.responseInvalidated?.size() == plansResponse.lineItems.size()

and: "PBS invalidated response as unmatched by size"
def bidderError = auctionResponse.ext?.errors?.get(GENERIC)
assert bidderError?.size() == 1
assert bidderError[0].message == "Bid \"$bid.id\" has 'w' and 'h' not supported by " +
"corresponding imp in request. Bid dimensions: '${bid.w}x$bid.h', formats in imp: '${impFormat.w}x$impFormat.h'"
def bidderErrors = auctionResponse.ext?.errors?.get(GENERIC)

assert bidderErrors?.size() == 1
assert bidderErrors[0].message ==~ /BidId `$bid.id` validation messages:.* Error: / +
/Bid "$bid.id" has 'w' and 'h' not supported by corresponding imp in request\. / +
/Bid dimensions: '${bid.w}x$bid.h', formats in imp: '${impFormat.w}x$impFormat.h'.*/
}

def "PBS should invalidate bidder response when non-matched to the PBS line item size response is returned"() {
Expand Down Expand Up @@ -158,9 +166,11 @@ class PgBidResponseSpec extends BasePgSpec {
assert auctionResponse.ext?.debug?.pgmetrics?.responseInvalidated?.size() == plansResponse.lineItems.size()

and: "PBS invalidated response as not matched by size"
def bidderError = auctionResponse.ext?.errors?.get(GENERIC)
assert bidderError?.size() == 1
assert bidderError[0].message == "Bid \"$bid.id\" has 'w' and 'h' not matched to Line Item. " +
"Bid dimensions: '${bid.w}x$bid.h', Line Item sizes: '${lineItemSize.w}x$lineItemSize.h'"
def bidderErrors = auctionResponse.ext?.errors?.get(GENERIC)

assert bidderErrors?.size() == 1
assert bidderErrors[0].message ==~ /BidId `$bid.id` validation messages:.* Error: / +
/Bid "$bid.id" has 'w' and 'h' not matched to Line Item\. / +
/Bid dimensions: '${bid.w}x$bid.h', Line Item sizes: '${lineItemSize.w}x$lineItemSize.h'.*/
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,9 @@ class PgDealsOnlySpec extends BasePgSpec {
and: "PBS returns an error of missing 'dealid' field in bid"
def bidErrors = auctionResponse.ext?.errors?.get(GENERIC)
assert bidErrors?.size() == 1
assert bidErrors[0].code == 999
assert bidErrors[0].message == "Bid \"$bidResponseId\" missing required field 'dealid'"
assert bidErrors[0].code == 5
assert bidErrors[0].message ==~ /BidId `$bidResponseId` validation messages:.* Error: / +
/Bid "$bidResponseId" missing required field 'dealid'.*/
}

def "PBS should add dealsonly flag when it is not specified and pgdealsonly flag is set to true"() {
Expand Down Expand Up @@ -149,9 +150,11 @@ class PgDealsOnlySpec extends BasePgSpec {

and: "PBS returns an error of missing 'dealid' field in bid"
def bidErrors = auctionResponse.ext?.errors?.get(GENERIC)

assert bidErrors?.size() == 1
assert bidErrors[0].code == 999
assert bidErrors[0].message == "Bid \"$bidResponseId\" missing required field 'dealid'"
assert bidErrors[0].code == 5
assert bidErrors[0].message ==~ /BidId `$bidResponseId` validation messages:.* Error: / +
/Bid "$bidResponseId" missing required field 'dealid'.*/
}

def "PBS shouldn't return an error when bidder dealsonly flag is set to true, no available PG line items and bid response misses 'dealid' field"() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1363,8 +1363,8 @@ public void shouldTolerateResponseBidValidationErrors() {
.extracting(BidderResponse::getSeatBid)
.flatExtracting(BidderSeatBid::getErrors)
.containsOnly(
BidderError.generic("bid validation warning"),
BidderError.generic("bid validation error"));
BidderError.invalidBid("BidId `bidId1` validation messages: Error: bid validation error."
+ " Warning: bid validation warning"));
}

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

@Test
Expand Down