Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 52 additions & 17 deletions src/main/java/org/prebid/server/bidder/yahooads/YahooAdsBidder.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import io.vertx.core.MultiMap;
import io.vertx.core.http.HttpMethod;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import org.prebid.server.bidder.Bidder;
import org.prebid.server.bidder.model.BidderBid;
Expand All @@ -38,6 +39,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.regex.Pattern;

public class YahooAdsBidder implements Bidder<BidRequest> {

Expand All @@ -49,6 +51,9 @@ public class YahooAdsBidder implements Bidder<BidRequest> {
private static final String GPP_PROPERTY = "gpp";
private static final String GPP_SID_PROPERTY = "gpp_sid";

// VAST_MARKUP is a heuristic to find video bids without mtype on banner+video imps.
private static final Pattern VAST_MARKUP = Pattern.compile("<VAST[ \\t\\r\\n/>]");
Comment thread
CTMBNara marked this conversation as resolved.
Resolved

private final String endpointUrl;
private final JacksonMapper mapper;

Expand Down Expand Up @@ -248,50 +253,80 @@ private static MultiMap makeHeaders(Device device) {
public Result<List<BidderBid>> makeBids(BidderCall<BidRequest> httpCall, BidRequest bidRequest) {
try {
final BidResponse bidResponse = mapper.decodeValue(httpCall.getResponse().getBody(), BidResponse.class);
return Result.of(extractBids(bidResponse, httpCall.getRequest().getPayload()), Collections.emptyList());
final List<BidderError> errors = new ArrayList<>();
final List<BidderBid> bids = extractBids(bidResponse, httpCall.getRequest().getPayload(), errors);
return Result.of(bids, errors);
} catch (DecodeException | PreBidException e) {
return Result.withError(BidderError.badServerResponse(e.getMessage()));
}
}

private static List<BidderBid> extractBids(BidResponse bidResponse, BidRequest bidRequest) {
private static List<BidderBid> extractBids(BidResponse bidResponse,
BidRequest bidRequest,
List<BidderError> errors) {
Comment thread
CTMBNara marked this conversation as resolved.
Resolved

final List<SeatBid> seatBids = bidResponse != null ? bidResponse.getSeatbid() : null;
if (seatBids == null) {
return Collections.emptyList();
}
return bidsFromResponse(bidResponse, bidRequest.getImp());
return bidsFromResponse(bidResponse, bidRequest.getImp(), errors);
}

private static List<BidderBid> bidsFromResponse(BidResponse bidResponse, List<Imp> imps) {
private static List<BidderBid> bidsFromResponse(BidResponse bidResponse,
List<Imp> imps,
List<BidderError> errors) {
Comment thread
CTMBNara marked this conversation as resolved.
Resolved

return bidResponse.getSeatbid().stream()
.filter(Objects::nonNull)
.map(SeatBid::getBid)
.filter(Objects::nonNull)
.flatMap(Collection::stream)
.filter(Objects::nonNull)
.map(bid -> makeBidderBid(bid, imps, bidResponse.getCur()))
.map(bid -> makeBidderBid(bid, imps, bidResponse.getCur(), errors))
.filter(Objects::nonNull)
.toList();
}

private static BidderBid makeBidderBid(Bid bid, List<Imp> imps, String currency) {
final BidType bidType = getBidType(bid, imps);
return bidType != null
? BidderBid.of(bid, bidType, currency)
: null;
private static BidderBid makeBidderBid(Bid bid, List<Imp> imps, String currency, List<BidderError> errors) {
final Imp imp = findImp(bid, imps);
try {
final BidType bidType = resolveBidType(bid, imp);
return bidType != null
? BidderBid.of(bid, bidType, currency)
: null;
} catch (PreBidException e) {
errors.add(BidderError.badServerResponse(e.getMessage()));
return null;
}
}

private static BidType getBidType(Bid bid, List<Imp> imps) {
private static Imp findImp(Bid bid, List<Imp> imps) {
for (Imp imp : imps) {
if (imp.getId().equals(bid.getImpid())) {
if (imp.getBanner() != null) {
return BidType.banner;
} else if (imp.getVideo() != null) {
return BidType.video;
}
return null;
return imp;
}
}
throw new PreBidException("Unknown ad unit code '%s'".formatted(bid.getImpid()));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This already happened before this PR: an unknown impid makes the exception escape the stream, and makeBids returns Result.withError(...). That drops valid bids and any errors already collected. Now that errors are collected per bid, consider adding this error and continuing too. It's a behaviour change, so it's fine as a follow-up (same as on the Go PR).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, will handle this in a separate PR.

}

private static BidType resolveBidType(Bid bid, Imp imp) {
return switch (ObjectUtils.getIfNull(bid.getMtype(), 0)) {
case 0 -> resolveBidTypeFromImp(bid, imp);
case 1 -> BidType.banner;
case 2 -> BidType.video;
default -> throw new PreBidException(
"Unsupported mtype %d for bid '%s'".formatted(bid.getMtype(), bid.getId()));
};
}

private static BidType resolveBidTypeFromImp(Bid bid, Imp imp) {
if (imp.getVideo() != null && (imp.getBanner() == null || isVast(bid.getAdm()))) {
return BidType.video;
}
return imp.getBanner() != null ? BidType.banner : null;
}

private static boolean isVast(String adm) {
return StringUtils.isNotEmpty(adm) && VAST_MARKUP.matcher(adm).find();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,167 @@ public void makeBidsShouldSkipNotSupportedImpAndReturnVideoBidWhenVideoPresent()
.containsOnly(BidderBid.of(Bid.builder().impid("321").build(), video, "USD"));
}

@Test
public void makeBidsShouldReturnVideoBidWhenMtypeIsVideo() throws JsonProcessingException {
// given
final Bid bid = Bid.builder().impid("321").mtype(2).build();
final BidderCall<BidRequest> httpCall = givenHttpCall(Imp.builder().id("321").build(), bid);

// when
final Result<List<BidderBid>> result = target.makeBids(httpCall, null);

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).containsOnly(BidderBid.of(bid, video, "USD"));
}

@Test
public void makeBidsShouldReturnBannerBidWhenMtypeIsBanner() throws JsonProcessingException {
// given
final Bid bid = Bid.builder().impid("321").mtype(1).build();
final BidderCall<BidRequest> httpCall = givenHttpCall(Imp.builder().id("321").build(), bid);

// when
final Result<List<BidderBid>> result = target.makeBids(httpCall, null);

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).containsOnly(BidderBid.of(bid, banner, "USD"));
}

@Test
public void makeBidsShouldTreatZeroMtypeAsAbsent() throws JsonProcessingException {
// given
final Bid bid = Bid.builder().impid("321").mtype(0).adm("<VAST version=\"4.0\"></VAST>").build();
final BidderCall<BidRequest> httpCall = givenHttpCall(givenMultiFormatImp(), bid);

// when
final Result<List<BidderBid>> result = target.makeBids(httpCall, null);

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).containsOnly(BidderBid.of(bid, video, "USD"));
}

@Test
public void makeBidsShouldReturnErrorForUnsupportedMtypeAndKeepOtherBids() throws JsonProcessingException {
Comment thread
CTMBNara marked this conversation as resolved.
Resolved
// given
final Bid audioBid = Bid.builder().id("audioBidId").impid("321").mtype(3).build();
final Bid nativeBid = Bid.builder().id("nativeBidId").impid("321").mtype(4).build();
final Bid videoBid = Bid.builder().id("videoBidId").impid("321").mtype(2).build();
final BidderCall<BidRequest> httpCall = givenHttpCall(givenMultiFormatImp(), audioBid, nativeBid, videoBid);

// when
final Result<List<BidderBid>> result = target.makeBids(httpCall, null);

// then
assertThat(result.getErrors()).containsExactly(
BidderError.badServerResponse("Unsupported mtype 3 for bid 'audioBidId'"),
BidderError.badServerResponse("Unsupported mtype 4 for bid 'nativeBidId'"));
assertThat(result.getValue()).containsExactly(BidderBid.of(videoBid, video, "USD"));
}

@Test
public void makeBidsShouldReturnVideoBidWhenNoMtypeAndAdmIsVastOnMultiFormatImp() throws JsonProcessingException {
// given
final Bid bid = Bid.builder().impid("321").adm("<VAST version=\"4.0\"></VAST>").build();
final BidderCall<BidRequest> httpCall = givenHttpCall(givenMultiFormatImp(), bid);

// when
final Result<List<BidderBid>> result = target.makeBids(httpCall, null);

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).containsOnly(BidderBid.of(bid, video, "USD"));
}

@Test
public void makeBidsShouldReturnVideoBidWhenVastFollowsXmlDeclaration() throws JsonProcessingException {
// given
final Bid bid = Bid.builder()
.impid("321")
.adm("<?xml version=\"1.0\" ?><VAST version=\"2.0\"><Ad></Ad></VAST>")
.build();
final BidderCall<BidRequest> httpCall = givenHttpCall(givenMultiFormatImp(), bid);

// when
final Result<List<BidderBid>> result = target.makeBids(httpCall, null);

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).containsOnly(BidderBid.of(bid, video, "USD"));
}

@Test
public void makeBidsShouldReturnVideoBidWhenVastAttributesStartOnNextLine() throws JsonProcessingException {
// given
final Bid bid = Bid.builder().impid("321").adm("<VAST\n version=\"4.0\"></VAST>").build();
final BidderCall<BidRequest> httpCall = givenHttpCall(givenMultiFormatImp(), bid);

// when
final Result<List<BidderBid>> result = target.makeBids(httpCall, null);

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).containsOnly(BidderBid.of(bid, video, "USD"));
}

@Test
public void makeBidsShouldReturnBannerBidWhenVastTagIsLowercase() throws JsonProcessingException {
// given
final Bid bid = Bid.builder().impid("321").adm("<vast version=\"4.0\"></vast>").build();
final BidderCall<BidRequest> httpCall = givenHttpCall(givenMultiFormatImp(), bid);

// when
final Result<List<BidderBid>> result = target.makeBids(httpCall, null);

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).containsOnly(BidderBid.of(bid, banner, "USD"));
}

@Test
public void makeBidsShouldReturnBannerBidWhenAdmHasNoVastElement() throws JsonProcessingException {
// given
final Bid bid = Bid.builder().impid("321").adm("<VASTAdTagURI>u</VASTAdTagURI>").build();
final BidderCall<BidRequest> httpCall = givenHttpCall(givenMultiFormatImp(), bid);

// when
final Result<List<BidderBid>> result = target.makeBids(httpCall, null);

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).containsOnly(BidderBid.of(bid, banner, "USD"));
}

@Test
public void makeBidsShouldReturnBannerBidWhenAdmMentionsVastOnlyAsText() throws JsonProcessingException {
// given
final Bid bid = Bid.builder().impid("321").adm("<div>VAST</div>").build();
final BidderCall<BidRequest> httpCall = givenHttpCall(givenMultiFormatImp(), bid);

// when
final Result<List<BidderBid>> result = target.makeBids(httpCall, null);

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).containsOnly(BidderBid.of(bid, banner, "USD"));
}

@Test
public void makeBidsShouldReturnBannerBidWhenNoMtypeAndNoAdmOnMultiFormatImp() throws JsonProcessingException {
// given
final Bid bid = Bid.builder().impid("321").build();
final BidderCall<BidRequest> httpCall = givenHttpCall(givenMultiFormatImp(), bid);

// when
final Result<List<BidderBid>> result = target.makeBids(httpCall, null);

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).containsOnly(BidderBid.of(bid, banner, "USD"));
}

@Test
public void makeHttpRequestsShouldPreserveTopLevel26RegsAndExtTypedFields() {
// given
Expand Down Expand Up @@ -955,10 +1116,27 @@ private static BidResponse givenBidResponse(Function<Bid.BidBuilder, Bid.BidBuil
.build();
}

private static Imp givenMultiFormatImp() {
return Imp.builder()
.id("321")
.banner(Banner.builder().build())
.video(Video.builder().build())
.build();
}

private static BidderCall<BidRequest> givenHttpCall(BidRequest bidRequest, String body) {
return BidderCall.succeededHttp(
HttpRequest.<BidRequest>builder().payload(bidRequest).build(),
HttpResponse.of(200, null, body),
null);
}

private static BidderCall<BidRequest> givenHttpCall(Imp imp, Bid... bids) throws JsonProcessingException {
return givenHttpCall(
BidRequest.builder().imp(singletonList(imp)).build(),
mapper.writeValueAsString(BidResponse.builder()
.cur("USD")
.seatbid(singletonList(SeatBid.builder().bid(asList(bids)).build()))
.build()));
}
}