diff --git a/src/main/java/org/prebid/server/bidder/marsmedia/MarsmediaBidder.java b/src/main/java/org/prebid/server/bidder/marsmedia/MarsmediaBidder.java index c802177b32d..6bf0b5c431f 100644 --- a/src/main/java/org/prebid/server/bidder/marsmedia/MarsmediaBidder.java +++ b/src/main/java/org/prebid/server/bidder/marsmedia/MarsmediaBidder.java @@ -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; @@ -32,6 +33,9 @@ import java.util.Objects; import java.util.stream.Collectors; +/** + * Marsmedia {@link Bidder} implementation. + */ public class MarsmediaBidder implements Bidder { private static final TypeReference> MARSMEDIA_EXT_TYPE_REFERENCE = @@ -48,16 +52,16 @@ public MarsmediaBidder(String endpointUrl, JacksonMapper mapper) { @Override public Result>> 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); @@ -71,12 +75,12 @@ public Result>> 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(); @@ -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 validImps = new ArrayList<>(); - - final List 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); } @@ -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) { @@ -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; @@ -166,19 +153,18 @@ private static List extractBids(BidResponse bidResponse, BidRequest b } private static List bidsFromResponse(List seatbid, List 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 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; diff --git a/src/test/java/org/prebid/server/bidder/marsmedia/MarsmediaBidderTest.java b/src/test/java/org/prebid/server/bidder/marsmedia/MarsmediaBidderTest.java index 1ec7dcc0344..bc078b014a7 100644 --- a/src/test/java/org/prebid/server/bidder/marsmedia/MarsmediaBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/marsmedia/MarsmediaBidderTest.java @@ -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; @@ -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; @@ -63,8 +65,7 @@ public void makeHttpRequestsShouldReturnErrorIfImpExtCouldNotBeParsed() { final Result>> 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(); } @@ -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 @@ -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 @@ -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>> 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>> 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 @@ -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>> 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 @@ -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 @@ -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")); @@ -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) @@ -224,9 +267,11 @@ public void makeBidsShouldReturnErrorIfResponseBodyCouldNotBeParsed() { final Result> 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(); } @@ -266,7 +311,7 @@ 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> result = marsmediaBidder.makeBids(httpCall, null); @@ -274,7 +319,21 @@ public void makeBidsShouldReturnBannerBidByDefault() throws JsonProcessingExcept // 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 httpCall = givenHttpCall(BidRequest.builder().build(), + mapper.writeValueAsString(BidResponse.builder().seatbid(singletonList(null)).build())); + + // when + final Result> result = marsmediaBidder.makeBids(httpCall, null); + + // then + assertThat(result.getErrors()).isEmpty(); + assertThat(result.getValue()).isEmpty(); } @Test @@ -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( diff --git a/src/test/java/org/prebid/server/it/MarsmediaTest.java b/src/test/java/org/prebid/server/it/MarsmediaTest.java index 2cfaa0245ff..dfbd811f5be 100644 --- a/src/test/java/org/prebid/server/it/MarsmediaTest.java +++ b/src/test/java/org/prebid/server/it/MarsmediaTest.java @@ -25,11 +25,13 @@ public class MarsmediaTest extends IntegrationTest { public void openrtb2AuctionShouldRespondWithBidsFromMarsmedia() throws IOException, JSONException { // given // Marsmedia bid response for imp 001 - WIRE_MOCK_RULE.stubFor(post(urlPathEqualTo("/marsmedia-exchange&zone=zone_1")) - //.withQueryParam("zone", equalTo("zone_1")) + WIRE_MOCK_RULE.stubFor(post(urlPathEqualTo("/marsmedia-exchange")) + .withQueryParam("zone", equalTo("zone_1")) .withHeader("x-openrtb-version", equalTo("2.5")) .withHeader("DNT", equalTo("2")) .withHeader("Accept-Language", equalTo("en")) + .withHeader("User-Agent", equalTo("userAgent")) + .withHeader("X-Forwarded-For", equalTo("193.168.244.1")) .withRequestBody(equalToJson(jsonFrom("openrtb2/marsmedia/test-marsmedia-bid-request-1.json"))) .willReturn(aResponse().withBody(jsonFrom("openrtb2/marsmedia/test-marsmedia-bid-response-1.json")))); diff --git a/src/test/resources/org/prebid/server/it/test-application.properties b/src/test/resources/org/prebid/server/it/test-application.properties index cf2c87ee50d..4d94ef4b46f 100644 --- a/src/test/resources/org/prebid/server/it/test-application.properties +++ b/src/test/resources/org/prebid/server/it/test-application.properties @@ -200,7 +200,7 @@ adapters.lunamedia.endpoint=http://localhost:8090/lunamedia-exchange?pubid= adapters.lunamedia.pbs-enforces-gdpr=true adapters.lunamedia.usersync.url=//lunamedia-usersync adapters.marsmedia.enabled=true -adapters.marsmedia.endpoint=http://localhost:8090/marsmedia-exchange +adapters.marsmedia.endpoint=http://localhost:8090/marsmedia-exchange?param=testParam adapters.marsmedia.pbs-enforces-gdpr=true adapters.marsmedia.usersync.url=//marsmedia-usersync adapters.mgid.enabled=true