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
Original file line number Diff line number Diff line change
Expand Up @@ -1335,8 +1335,9 @@ private static void setAssetTypes(Asset responseAsset, List<com.iab.openrtb.requ
if (type != null) {
responseAsset.getImg().setType(type);
} else {
throw new PreBidException(String.format("Response has an Image asset with ID:%s present that doesn't "
+ "exist in the request", responseAsset.getId()));
final Integer assetId = responseAsset.getId();
throw new PreBidException(String.format("Response has an Image asset with ID:'%s' present that doesn't "
+ "exist in the request", assetId != null ? assetId : StringUtils.EMPTY));
}
}
if (responseAsset.getData() != null) {
Expand All @@ -1351,11 +1352,11 @@ private static void setAssetTypes(Asset responseAsset, List<com.iab.openrtb.requ
}
}

private static com.iab.openrtb.request.Asset getAssetById(int assetId,
private static com.iab.openrtb.request.Asset getAssetById(Integer assetId,
List<com.iab.openrtb.request.Asset> requestAssets) {

return requestAssets.stream()
.filter(asset -> asset.getId() == assetId)
.filter(asset -> Objects.equals(assetId, asset.getId()))
.findFirst()
.orElse(com.iab.openrtb.request.Asset.EMPTY);
}
Expand Down
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 @@ -190,4 +190,25 @@ class BidValidationSpec extends BaseSpec {
null | PBSUtils.randomNumber
null | null
}

def "PBS should update 'adapter.generic.requests.bid_validation' metric when bid validation error appears"() {
given: "Initial 'adapter.generic.requests.bid_validation' metric value"
def initialMetricValue = getCurrentMetricValue("adapter.generic.requests.bid_validation")

and: "Bid request"
def bidRequest = BidRequest.defaultBidRequest

and: "Set invalid bid response"
def bidResponse = BidResponse.getDefaultBidResponse(bidRequest).tap {
seatbid[0].bid[0].impid = PBSUtils.randomNumber as String
}
bidder.setResponse(bidRequest.id, bidResponse)

when: "Sending auction request to PBS"
defaultPbsService.sendAuctionRequest(bidRequest)

then: "Bid validation metric value is incremented"
def metrics = defaultPbsService.sendCollectedMetricsRequest()
assert metrics["adapter.generic.requests.bid_validation"] == initialMetricValue + 1
}
}
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 @@ -102,7 +102,6 @@ class PgDealsOnlySpec extends BasePgSpec {
def bidResponse = BidResponse.getDefaultBidResponse(bidRequest)
bidResponse.seatbid[0].bid[0].dealid = null
bidder.setResponse(bidRequest.id, bidResponse)
def bidResponseId = bidResponse.seatbid[0].bid[0].id

and: "Line items are fetched by PBS"
updateLineItemsAndWait()
Expand All @@ -115,9 +114,12 @@ class PgDealsOnlySpec extends BasePgSpec {

and: "PBS returns an error of missing 'dealid' field in bid"
def bidErrors = auctionResponse.ext?.errors?.get(GENERIC)
def bidId = bidResponse.seatbid[0].bid[0].id

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

def "PBS should add dealsonly flag when it is not specified and pgdealsonly flag is set to true"() {
Expand All @@ -133,7 +135,6 @@ class PgDealsOnlySpec extends BasePgSpec {
def bidResponse = BidResponse.getDefaultBidResponse(bidRequest)
bidResponse.seatbid[0].bid[0].dealid = null
bidder.setResponse(bidRequest.id, bidResponse)
def bidResponseId = bidResponse.seatbid[0].bid[0].id

and: "Line items are fetched by PBS"
updateLineItemsAndWait()
Expand All @@ -149,9 +150,12 @@ class PgDealsOnlySpec extends BasePgSpec {

and: "PBS returns an error of missing 'dealid' field in bid"
def bidErrors = auctionResponse.ext?.errors?.get(GENERIC)
def bidId = bidResponse.seatbid[0].bid[0].id

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

def "PBS shouldn't return an error when bidder dealsonly flag is set to true, no available PG line items and bid response misses 'dealid' field"() {
Expand Down
120 changes: 120 additions & 0 deletions src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1139,6 +1139,126 @@ public void shouldReturnEmptyAssetIfImageTypeIsEmpty() throws JsonProcessingExce
.isEmpty();
}

@Test
public void shouldReturnEmptyAssetIfNoRelatedNativeAssetFound() throws JsonProcessingException {
// given
final Request nativeRequest = Request.builder()
.assets(singletonList(Asset.builder()
.id(null)
.img(ImageObject.builder().type(null).build())
.data(DataObject.builder().type(2).build())
.build()))
.build();

final BidRequest bidRequest = BidRequest.builder()
.cur(singletonList("USD"))
.tmax(1000L)
.app(App.builder().build())
.imp(singletonList(Imp.builder()
.id(IMP_ID)
.xNative(Native.builder().request(mapper.writeValueAsString(nativeRequest)).build())
.build()))
.build();

final AuctionContext auctionContext = givenAuctionContext(bidRequest);

final Response responseAdm = Response.builder()
.assets(singletonList(com.iab.openrtb.response.Asset.builder()
.id(123)
.img(com.iab.openrtb.response.ImageObject.builder().type(null).build())
.data(com.iab.openrtb.response.DataObject.builder().build())
.build()))
.build();

final Bid bid = Bid.builder()
.id("bidId")
.price(BigDecimal.ONE)
.impid(IMP_ID)
.adm(mapper.writeValueAsString(responseAdm))
.ext(mapper.valueToTree(singletonMap("bidExt", 1)))
.build();
final List<BidderResponse> bidderResponses = singletonList(BidderResponse.of("bidder1",
givenSeatBid(BidderBid.of(bid, xNative, "USD")), 100));

// when
final BidResponse bidResponse =
bidResponseCreator.create(toAuctionParticipant(bidderResponses), auctionContext, CACHE_INFO, MULTI_BIDS)
.result();

// then

assertThat(bidResponse.getSeatbid()).hasSize(1)
.flatExtracting(SeatBid::getBid)
.extracting(Bid::getAdm)
.extracting(adm -> mapper.readValue(adm, Response.class))
.flatExtracting(Response::getAssets)
.isEmpty();
assertThat(bidResponse.getExt())
.extracting(ExtBidResponse::getErrors)
.isEqualTo(Map.of("bidder1", singletonList(ExtBidderError.of(3,
"Response has an Image asset with ID:'123' present that doesn't exist in the request"))));
}

@Test
public void shouldReturnEmptyAssetIfIdIsNotPresentRelatedNativeAssetFound() throws JsonProcessingException {
// given
final Request nativeRequest = Request.builder()
.assets(singletonList(Asset.builder()
.id(123)
.img(ImageObject.builder().type(null).build())
.data(DataObject.builder().type(2).build())
.build()))
.build();

final BidRequest bidRequest = BidRequest.builder()
.cur(singletonList("USD"))
.tmax(1000L)
.app(App.builder().build())
.imp(singletonList(Imp.builder()
.id(IMP_ID)
.xNative(Native.builder().request(mapper.writeValueAsString(nativeRequest)).build())
.build()))
.build();

final AuctionContext auctionContext = givenAuctionContext(bidRequest);

final Response responseAdm = Response.builder()
.assets(singletonList(com.iab.openrtb.response.Asset.builder()
.id(null)
.img(com.iab.openrtb.response.ImageObject.builder().type(null).build())
.data(com.iab.openrtb.response.DataObject.builder().build())
.build()))
.build();

final Bid bid = Bid.builder()
.id("bidId")
.price(BigDecimal.ONE)
.impid(IMP_ID)
.adm(mapper.writeValueAsString(responseAdm))
.ext(mapper.valueToTree(singletonMap("bidExt", 1)))
.build();
final List<BidderResponse> bidderResponses = singletonList(BidderResponse.of("bidder1",
givenSeatBid(BidderBid.of(bid, xNative, "USD")), 100));

// when
final BidResponse bidResponse =
bidResponseCreator.create(toAuctionParticipant(bidderResponses), auctionContext, CACHE_INFO, MULTI_BIDS)
.result();

// then

assertThat(bidResponse.getSeatbid()).hasSize(1)
.flatExtracting(SeatBid::getBid)
.extracting(Bid::getAdm)
.extracting(adm -> mapper.readValue(adm, Response.class))
.flatExtracting(Response::getAssets)
.isEmpty();
assertThat(bidResponse.getExt())
.extracting(ExtBidResponse::getErrors)
.isEqualTo(Map.of("bidder1", singletonList(ExtBidderError.of(3,
"Response has an Image asset with ID:'' present that doesn't exist in the request"))));
}

@Test
public void shouldReturnEmptyAssetIfDataTypeIsEmpty() throws JsonProcessingException {
// given
Expand Down
Loading