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.

Sign up for GitHub

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

Marsmedia bidder review #1079

Merged
merged 2 commits into from
Feb 4, 2021
Merged

Marsmedia bidder review #1079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9c7ed33
Marsmedia bidder review
Dec 17, 2020
ed48e01
Merge branch 'master' into marsmedia/bid/review
Feb 4, 2021
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
76 changes: 31 additions & 45 deletions src/main/java/org/prebid/server/bidder/marsmedia/MarsmediaBidder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,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 @@ -32,6 +33,9 @@
import java.util.Objects;
import java.util.stream.Collectors;

/**
* Marsmedia {@link Bidder} implementation.
*/
public class MarsmediaBidder implements Bidder<BidRequest> {

private static final TypeReference<ExtPrebid<?, ExtImpMarsmedia>> MARSMEDIA_EXT_TYPE_REFERENCE =
Expand All @@ -48,16 +52,16 @@ public MarsmediaBidder(String endpointUrl, JacksonMapper mapper) {

@Override
public Result<List<HttpRequest<BidRequest>>> makeHttpRequests(BidRequest bidRequest) {
final String requestZone;
final String firstImpZone;
final BidRequest outgoingRequest;
try {
requestZone = resolveRequestZone(bidRequest.getImp().get(0));
outgoingRequest = createOutgoingRequest(bidRequest);
firstImpZone = resolveExtZone(bidRequest.getImp().get(0));
outgoingRequest = createRequest(bidRequest);
} catch (PreBidException e) {
return Result.withError(BidderError.badInput(e.getMessage()));
}

final String uri = endpointUrl + "&zone=" + requestZone;
final String uri = String.format("%s%s%s", endpointUrl, "&zone=", firstImpZone);
final MultiMap headers = resolveHeaders(bidRequest.getDevice());

final String body = mapper.encode(outgoingRequest);
Expand All @@ -71,12 +75,12 @@ public Result<List<HttpRequest<BidRequest>>> makeHttpRequests(BidRequest bidRequ
.build());
}

private String resolveRequestZone(Imp firstImp) {
private String resolveExtZone(Imp imp) {
final ExtImpMarsmedia extImpMarsmedia;
try {
extImpMarsmedia = mapper.mapper().convertValue(firstImp.getExt(), MARSMEDIA_EXT_TYPE_REFERENCE).getBidder();
extImpMarsmedia = mapper.mapper().convertValue(imp.getExt(), MARSMEDIA_EXT_TYPE_REFERENCE).getBidder();
} catch (IllegalArgumentException e) {
throw new PreBidException(e.getMessage());
throw new PreBidException("ext.bidder not provided");
}

final String zoneId = extImpMarsmedia.getZone();
Expand All @@ -87,22 +91,18 @@ private String resolveRequestZone(Imp firstImp) {
return zoneId;
}

private static BidRequest createOutgoingRequest(BidRequest bidRequest) {
private static BidRequest createRequest(BidRequest request) {
final List<Imp> validImps = new ArrayList<>();

final List<Imp> requestImps = bidRequest.getImp();
boolean shouldUpdateImps = false;
for (Imp imp : requestImps) {
for (Imp imp : request.getImp()) {
final Banner banner = imp.getBanner();
if (banner != null) {
final boolean hasFormats = CollectionUtils.isNotEmpty(banner.getFormat());
//a shortcut to avoid cases when the call bidRequest.toBuilder() can be redundant as there are
// no changes to be made
if (!shouldUpdateImps) {
shouldUpdateImps = hasFormats;
if (CollectionUtils.isNotEmpty(banner.getFormat())) {
validImps.add(imp.toBuilder().banner(updateBanner(banner)).build());
} else if (banner.getW() != null && banner.getH() != null) {
validImps.add(imp);
} else {
throw new PreBidException("No valid banner format in the bid request");
}

validImps.add(checkBannerImp(banner, imp, hasFormats));
} else if (imp.getVideo() != null) {
validImps.add(imp);
}
Expand All @@ -112,27 +112,15 @@ private static BidRequest createOutgoingRequest(BidRequest bidRequest) {
throw new PreBidException("No valid impression in the bid request");
}

return shouldUpdateImps || !Objects.equals(bidRequest.getAt(), 1)
? bidRequest.toBuilder().imp(validImps).at(1).build()
: bidRequest;
return request.toBuilder().at(1).imp(validImps).build();
}

private static Imp checkBannerImp(Banner banner, Imp imp, boolean hasFormats) {
if (hasFormats) {
final Format firstFormat = banner.getFormat().get(0);
final Banner modifiedBanner = banner.toBuilder()
.w(firstFormat.getW())
.h(firstFormat.getH())
.build();

return imp.toBuilder().banner(modifiedBanner).build();
}

if (banner.getW() != null && banner.getH() != null) {
return imp;
}

throw new PreBidException("No valid banner format in the bid request");
private static Banner updateBanner(Banner banner) {
final Format firstFormat = banner.getFormat().get(0);
return banner.toBuilder()
.w(ObjectUtils.defaultIfNull(firstFormat.getW(), 0))
.h(ObjectUtils.defaultIfNull(firstFormat.getH(), 0))
.build();
}

private static MultiMap resolveHeaders(Device device) {
Expand All @@ -143,7 +131,6 @@ private static MultiMap resolveHeaders(Device device) {
HttpUtil.addHeaderIfValueIsNotEmpty(headers, HttpUtil.USER_AGENT_HEADER, device.getUa());
HttpUtil.addHeaderIfValueIsNotEmpty(headers, HttpUtil.ACCEPT_LANGUAGE_HEADER, device.getLanguage());
HttpUtil.addHeaderIfValueIsNotEmpty(headers, HttpUtil.X_FORWARDED_FOR_HEADER, device.getIp());
HttpUtil.addHeaderIfValueIsNotEmpty(headers, HttpUtil.X_FORWARDED_FOR_HEADER, device.getIpv6());
HttpUtil.addHeaderIfValueIsNotEmpty(headers, HttpUtil.DNT_HEADER, Objects.toString(device.getDnt(), null));
}
return headers;
Expand All @@ -166,19 +153,18 @@ private static List<BidderBid> extractBids(BidResponse bidResponse, BidRequest b
}

private static List<BidderBid> bidsFromResponse(List<SeatBid> seatbid, List<Imp> imps, String currency) {
return seatbid.get(0).getBid().stream()
final SeatBid firstSeatBid = seatbid.get(0);
return firstSeatBid != null ? firstSeatBid.getBid().stream()
.filter(Objects::nonNull)
.map(bid -> BidderBid.of(bid, getBidType(bid.getImpid(), imps), currency))
.collect(Collectors.toList());
.collect(Collectors.toList())
: Collections.emptyList();
}

private static BidType getBidType(String impid, List<Imp> imps) {
for (Imp imp : imps) {
if (imp.getId().equals(impid)) {
if (imp.getVideo() != null) {
return BidType.video;
}
return BidType.banner;
return imp.getVideo() != null ? BidType.video : BidType.banner;
}
}
return BidType.banner;
Expand Down
115 changes: 87 additions & 28 deletions src/test/java/org/prebid/server/bidder/marsmedia/MarsmediaBidderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.iab.openrtb.request.Device;
import com.iab.openrtb.request.Format;
import com.iab.openrtb.request.Imp;
import com.iab.openrtb.request.Native;
import com.iab.openrtb.request.Video;
import com.iab.openrtb.response.Bid;
import com.iab.openrtb.response.BidResponse;
Expand All @@ -29,6 +30,7 @@
import java.util.Map;
import java.util.function.Function;

import static java.util.Arrays.asList;
import static java.util.Collections.singletonList;
import static java.util.function.Function.identity;
import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -63,8 +65,7 @@ public void makeHttpRequestsShouldReturnErrorIfImpExtCouldNotBeParsed() {
final Result<List<HttpRequest<BidRequest>>> result = marsmediaBidder.makeHttpRequests(bidRequest);

// then
assertThat(result.getErrors()).hasSize(1);
assertThat(result.getErrors().get(0).getMessage()).startsWith("Cannot deserialize instance");
assertThat(result.getErrors()).containsExactly(BidderError.badInput("ext.bidder not provided"));
assertThat(result.getValue()).isEmpty();
}

Expand All @@ -79,8 +80,7 @@ public void makeHttpRequestsShouldReturnErrorIfImpExtZoneIsBlank() {

// then
assertThat(result.getValue()).isEmpty();
assertThat(result.getErrors()).hasSize(1)
.containsOnly(BidderError.badInput("Zone is empty"));
assertThat(result.getErrors()).containsExactly(BidderError.badInput("Zone is empty"));
}

@Test
Expand All @@ -94,8 +94,8 @@ public void makeHttpRequestsShouldReturnErrorIfImpBannerHasNoSizeOrFormats() {

// then
assertThat(result.getValue()).isEmpty();
assertThat(result.getErrors()).hasSize(1)
.containsOnly(BidderError.badInput("No valid banner format in the bid request"));
assertThat(result.getErrors())
.containsExactly(BidderError.badInput("No valid banner format in the bid request"));
}

@Test
Expand All @@ -109,22 +109,44 @@ public void makeHttpRequestsShouldReturnErrorIfThereAreNoValidImps() {

// then
assertThat(result.getValue()).isEmpty();
assertThat(result.getErrors()).hasSize(1)
.containsOnly(BidderError.badInput("No valid impression in the bid request"));
assertThat(result.getErrors()).containsExactly(BidderError.badInput("No valid impression in the bid request"));
}

@Test
public void makeHttpRequestsShouldReturnUnmodifiedBidRequest() {
public void makeHttpRequestsShouldAddAtAttributeToOutgoingRequest() {
// given
final BidRequest bidRequest = givenBidRequest(identity(),
requestBuilder -> requestBuilder.at(1));
final BidRequest bidRequest = givenBidRequest(identity());

// when
final Result<List<HttpRequest<BidRequest>>> result = marsmediaBidder.makeHttpRequests(bidRequest);

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue().get(0).getPayload()).isSameAs(bidRequest);
assertThat(result.getValue())
.extracting(HttpRequest::getPayload)
.extracting(BidRequest::getAt)
.containsExactly(1);
}

@Test
public void makeHttpRequestsShouldAddOnlyBannerAndVideoImp() {
// given
final BidRequest bidRequest = BidRequest.builder()
.imp(asList(givenImp(impBuilder -> impBuilder.id("123")),
givenImp(impBuilder -> impBuilder.id("456").banner(null).video(Video.builder().build())),
givenImp(impBuilder -> impBuilder.id("789").banner(null).xNative(Native.builder().build()))))
.build();

// when
final Result<List<HttpRequest<BidRequest>>> result = marsmediaBidder.makeHttpRequests(bidRequest);

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue())
.extracting(HttpRequest::getPayload)
.flatExtracting(BidRequest::getImp)
.extracting(Imp::getId)
.containsExactly("123", "456");
}

@Test
Expand All @@ -140,12 +162,33 @@ public void makeHttpRequestsShouldReplaceBannerWidthAndHeightWithValuesFromFirst

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).hasSize(1)
.extracting(httpRequest -> mapper.readValue(httpRequest.getBody(), BidRequest.class))
assertThat(result.getValue())
.extracting(HttpRequest::getPayload)
.flatExtracting(BidRequest::getImp)
.extracting(Imp::getBanner)
.extracting(Banner::getW, Banner::getH)
.containsOnly(tuple(640, 480));
.containsExactly(tuple(640, 480));
}

@Test
public void makeHttpRequestsShouldReplaceBannerWidthAndHeightWithZeroIfFormatValuesNotPresent() {
// given
final BidRequest bidRequest = givenBidRequest(impBuilder -> impBuilder
.banner(Banner.builder()
.format(singletonList(Format.builder().w(null).h(null).build()))
.build()));

// when
final Result<List<HttpRequest<BidRequest>>> result = marsmediaBidder.makeHttpRequests(bidRequest);

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue())
.extracting(HttpRequest::getPayload)
.flatExtracting(BidRequest::getImp)
.extracting(Imp::getBanner)
.extracting(Banner::getW, Banner::getH)
.containsExactly(tuple(0, 0));
}

@Test
Expand All @@ -158,10 +201,10 @@ public void makeHttpRequestsShouldAlwaysSetRequestAtToOne() {

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).hasSize(1)
.extracting(httpRequest -> mapper.readValue(httpRequest.getBody(), BidRequest.class))
assertThat(result.getValue())
.extracting(HttpRequest::getPayload)
.flatExtracting(BidRequest::getAt)
.containsOnly(1);
.containsExactly(1);
}

@Test
Expand All @@ -174,14 +217,14 @@ public void makeHttpRequestsShouldSetExpectedRequestUriAndBasicHeaders() {

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).hasSize(1)
assertThat(result.getValue())
.extracting(HttpRequest::getUri)
.containsOnly("https://test.endpoint.com/test&zone=zoneId");
.containsExactly("https://test.endpoint.com/test&zone=zoneId");
assertThat(result.getValue())
.extracting(HttpRequest::getHeaders)
.flatExtracting(MultiMap::entries)
.extracting(Map.Entry::getKey, Map.Entry::getValue)
.containsOnly(
.containsExactlyInAnyOrder(
tuple(HttpUtil.CONTENT_TYPE_HEADER.toString(), HttpUtil.APPLICATION_JSON_CONTENT_TYPE),
tuple(HttpUtil.ACCEPT_HEADER.toString(), HttpHeaderValues.APPLICATION_JSON.toString()),
tuple(HttpUtil.X_OPENRTB_VERSION_HEADER.toString(), "2.5"));
Expand All @@ -204,7 +247,7 @@ public void makeHttpRequestsShouldSetAdditionalHeadersIfRequestDeviceIsPresent()

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).hasSize(1)
assertThat(result.getValue())
.extracting(HttpRequest::getHeaders)
.flatExtracting(MultiMap::entries)
.extracting(Map.Entry::getKey, Map.Entry::getValue)
Expand All @@ -224,9 +267,11 @@ public void makeBidsShouldReturnErrorIfResponseBodyCouldNotBeParsed() {
final Result<List<BidderBid>> result = marsmediaBidder.makeBids(httpCall, null);

// then
assertThat(result.getErrors()).hasSize(1);
assertThat(result.getErrors().get(0).getMessage()).startsWith("Failed to decode: Unrecognized token");
assertThat(result.getErrors().get(0).getType()).isEqualTo(BidderError.Type.bad_server_response);
assertThat(result.getErrors()).hasSize(1)
.allSatisfy(error -> {
assertThat(error.getMessage()).startsWith("Failed to decode: Unrecognized token");
assertThat(error.getType()).isEqualTo(BidderError.Type.bad_server_response);
});
assertThat(result.getValue()).isEmpty();
}

Expand Down Expand Up @@ -266,15 +311,29 @@ public void makeBidsShouldReturnBannerBidByDefault() throws JsonProcessingExcept
.imp(singletonList(Imp.builder().id("123").build()))
.build(),
mapper.writeValueAsString(
givenBidResponse(bidBuilder -> bidBuilder.impid("123"))));
givenBidResponse(bidBuilder -> bidBuilder.impid("125"))));

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

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue())
.containsOnly(BidderBid.of(Bid.builder().impid("123").build(), banner, "USD"));
.containsExactly(BidderBid.of(Bid.builder().impid("125").build(), banner, "USD"));
}

@Test
public void makeBidsShouldReturnEmptyListIfFirstSeatIsNull() throws JsonProcessingException {
// given
final HttpCall<BidRequest> httpCall = givenHttpCall(BidRequest.builder().build(),
mapper.writeValueAsString(BidResponse.builder().seatbid(singletonList(null)).build()));

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

// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue()).isEmpty();
}

@Test
Expand All @@ -293,7 +352,7 @@ public void makeBidsShouldReturnVideoBidIfVideoIsPresent() throws JsonProcessing
// then
assertThat(result.getErrors()).isEmpty();
assertThat(result.getValue())
.containsOnly(BidderBid.of(Bid.builder().impid("123").build(), video, "USD"));
.containsExactly(BidderBid.of(Bid.builder().impid("123").build(), video, "USD"));
}

private static BidRequest givenBidRequest(
Expand Down
Loading