From d9cce9061f097941ce44c1bdb29f842780bdf9f3 Mon Sep 17 00:00:00 2001 From: Serhii Nahornyi Date: Tue, 17 Nov 2020 15:48:33 +0200 Subject: [PATCH] Adman bid review --- .../server/bidder/adman/AdmanBidder.java | 17 +++ .../server/bidder/adman/AdmanBidderTest.java | 103 +++++++++--------- 2 files changed, 66 insertions(+), 54 deletions(-) diff --git a/src/main/java/org/prebid/server/bidder/adman/AdmanBidder.java b/src/main/java/org/prebid/server/bidder/adman/AdmanBidder.java index 45634bf5a4d..229d937d20c 100644 --- a/src/main/java/org/prebid/server/bidder/adman/AdmanBidder.java +++ b/src/main/java/org/prebid/server/bidder/adman/AdmanBidder.java @@ -3,8 +3,12 @@ import com.iab.openrtb.request.Imp; import org.prebid.server.bidder.Bidder; import org.prebid.server.bidder.OpenrtbBidder; +import org.prebid.server.exception.PreBidException; import org.prebid.server.json.JacksonMapper; import org.prebid.server.proto.openrtb.ext.request.adman.ExtImpAdman; +import org.prebid.server.proto.openrtb.ext.response.BidType; + +import java.util.List; /** * Adman {@link Bidder} implementation. @@ -21,4 +25,17 @@ protected Imp modifyImp(Imp imp, ExtImpAdman impExt) { .tagid(impExt.getTagId()) .build(); } + + @Override + protected BidType getBidType(String impId, List imps) { + for (Imp imp : imps) { + if (imp.getId().equals(impId)) { + if (imp.getBanner() == null && imp.getVideo() != null) { + return BidType.video; + } + return BidType.banner; + } + } + throw new PreBidException(String.format("Failed to find impression %s", impId)); + } } diff --git a/src/test/java/org/prebid/server/bidder/adman/AdmanBidderTest.java b/src/test/java/org/prebid/server/bidder/adman/AdmanBidderTest.java index fe78fb7ab5d..d32963a99d9 100644 --- a/src/test/java/org/prebid/server/bidder/adman/AdmanBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/adman/AdmanBidderTest.java @@ -1,9 +1,11 @@ package org.prebid.server.bidder.adman; import com.fasterxml.jackson.core.JsonProcessingException; +import com.iab.openrtb.request.Audio; import com.iab.openrtb.request.Banner; import com.iab.openrtb.request.BidRequest; 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; @@ -27,7 +29,6 @@ import static java.util.Collections.singletonList; import static java.util.function.Function.identity; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.prebid.server.proto.openrtb.ext.response.BidType.banner; import static org.prebid.server.proto.openrtb.ext.response.BidType.video; @@ -42,29 +43,6 @@ public void setUp() { admanBidder = new AdmanBidder(ENDPOINT_URL, jacksonMapper); } - @Test - public void creationShouldFailOnInvalidEndpointUrl() { - assertThatIllegalArgumentException().isThrownBy(() -> new AdmanBidder("invalid_url", jacksonMapper)); - } - - @Test - public void makeHttpRequestsShouldReturnErrorIfImpExtCouldNotBeParsed() { - // given - final BidRequest bidRequest = BidRequest.builder() - .imp(singletonList(Imp.builder() - .ext(mapper.valueToTree(ExtPrebid.of(null, mapper.createArrayNode()))) - .build())) - .build(); - - // when - final Result>> result = admanBidder.makeHttpRequests(bidRequest); - - // then - assertThat(result.getErrors()).hasSize(1); - assertThat(result.getErrors().get(0).getMessage()).startsWith("Cannot deserialize instance"); - assertThat(result.getValue()).isEmpty(); - } - @Test public void makeHttpRequestsShouldReturnExpectedBidRequest() { // given @@ -80,7 +58,7 @@ public void makeHttpRequestsShouldReturnExpectedBidRequest() { assertThat(result.getErrors()).isEmpty(); assertThat(result.getValue()).hasSize(1) .extracting(httpRequest -> mapper.readValue(httpRequest.getBody(), BidRequest.class)) - .containsOnly(expectedRequest); + .containsExactly(expectedRequest); } @Test @@ -103,98 +81,115 @@ public void makeHttpRequestsShouldMakeOneRequestPerImp() { .extracting(httpRequest -> mapper.readValue(httpRequest.getBody(), BidRequest.class)) .flatExtracting(BidRequest::getImp).hasSize(2) .extracting(Imp::getTagid) - .containsOnly("tagidString", "otherTagId"); + .containsExactly("tagidString", "otherTagId"); } @Test - public void makeBidsShouldReturnErrorIfResponseBodyCouldNotBeParsed() { + public void makeBidsShouldReturnBannerBidByDefault() throws JsonProcessingException { // given - final HttpCall httpCall = givenHttpCall(null, "invalid"); + final HttpCall httpCall = givenHttpCall( + BidRequest.builder().imp(singletonList(Imp.builder().id("123").build())).build(), + mapper.writeValueAsString(givenBidResponse(bidBuilder -> bidBuilder.impid("123")))); // when final Result> result = admanBidder.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.getValue()).isEmpty(); + assertThat(result.getErrors()).isEmpty(); + assertThat(result.getValue()) + .containsExactly(BidderBid.of(Bid.builder().impid("123").build(), banner, "USD")); } @Test - public void makeBidsShouldReturnEmptyListIfBidResponseIsNull() throws JsonProcessingException { + public void makeBidsShouldReturnVideoBidIfNoBannerAndHasVideo() throws JsonProcessingException { // given - final HttpCall httpCall = givenHttpCall(null, mapper.writeValueAsString(null)); + final HttpCall httpCall = givenHttpCall( + BidRequest.builder() + .imp(singletonList(Imp.builder().video(Video.builder().build()).id("123").build())) + .build(), + mapper.writeValueAsString(givenBidResponse(bidBuilder -> bidBuilder.impid("123")))); // when final Result> result = admanBidder.makeBids(httpCall, null); // then assertThat(result.getErrors()).isEmpty(); - assertThat(result.getValue()).isEmpty(); + assertThat(result.getValue()) + .containsExactly(BidderBid.of(Bid.builder().impid("123").build(), video, "USD")); } @Test - public void makeBidsShouldReturnEmptyListIfBidResponseSeatBidIsNull() throws JsonProcessingException { + public void makeBidsShouldReturnBannerBidIfHasBothBannerAndVideo() throws JsonProcessingException { // given - final HttpCall httpCall = givenHttpCall(null, - mapper.writeValueAsString(BidResponse.builder().build())); + final HttpCall httpCall = givenHttpCall( + BidRequest.builder() + .imp(singletonList(givenImp(identity()))) + .build(), + mapper.writeValueAsString(givenBidResponse(bidBuilder -> bidBuilder.impid("123")))); // when final Result> result = admanBidder.makeBids(httpCall, null); // then assertThat(result.getErrors()).isEmpty(); - assertThat(result.getValue()).isEmpty(); + assertThat(result.getValue()) + .containsExactly(BidderBid.of(Bid.builder().impid("123").build(), banner, "USD")); } @Test - public void makeBidsShouldReturnBannerBidByDefault() throws JsonProcessingException { + public void makeBidsShouldReturnBannerBidIfNativeIsPresentInRequestImp() throws JsonProcessingException { // given final HttpCall httpCall = givenHttpCall( - BidRequest.builder().imp(singletonList(Imp.builder().id("123").build())).build(), - mapper.writeValueAsString(givenBidResponse(bidBuilder -> bidBuilder.impid("123")))); + BidRequest.builder() + .imp(singletonList(Imp.builder().id("123").xNative(Native.builder().build()).build())) + .build(), + mapper.writeValueAsString( + givenBidResponse(bidBuilder -> bidBuilder.impid("123")))); // when final Result> result = admanBidder.makeBids(httpCall, null); // then assertThat(result.getErrors()).isEmpty(); - assertThat(result.getValue()).containsOnly(BidderBid.of(Bid.builder().impid("123").build(), banner, "USD")); + assertThat(result.getValue()) + .containsExactly(BidderBid.of(Bid.builder().impid("123").build(), banner, "USD")); } @Test - public void makeBidsShouldReturnVideoBidIfNoBannerAndHasVideo() throws JsonProcessingException { + public void makeBidsShouldReturnBannerBidIfAudioIsPresentInRequestImp() throws JsonProcessingException { // given final HttpCall httpCall = givenHttpCall( BidRequest.builder() - .imp(singletonList(Imp.builder().video(Video.builder().build()).id("123").build())) + .imp(singletonList(Imp.builder().id("123").audio(Audio.builder().build()).build())) .build(), - mapper.writeValueAsString(givenBidResponse(bidBuilder -> bidBuilder.impid("123")))); + mapper.writeValueAsString( + givenBidResponse(bidBuilder -> bidBuilder.impid("123")))); // when final Result> result = admanBidder.makeBids(httpCall, null); // then assertThat(result.getErrors()).isEmpty(); - assertThat(result.getValue()).containsOnly(BidderBid.of(Bid.builder().impid("123").build(), video, "USD")); + assertThat(result.getValue()) + .containsExactly(BidderBid.of(Bid.builder().impid("123").build(), banner, "USD")); } @Test - public void makeBidsShouldReturnBannerBidIfHasBothBannerAndVideo() throws JsonProcessingException { + public void makeBidsShouldReturnErrorWithUnknownBidTypeIfNotSupportedBidType() throws JsonProcessingException { // given - final HttpCall httpCall = givenHttpCall( - BidRequest.builder() - .imp(singletonList(givenImp(identity()))) + final HttpCall httpCall = givenHttpCall(BidRequest.builder() + .imp(singletonList(Imp.builder().id("123").build())) .build(), - mapper.writeValueAsString(givenBidResponse(bidBuilder -> bidBuilder.impid("123")))); + mapper.writeValueAsString( + givenBidResponse(bidBuilder -> bidBuilder.impid("125")))); // when final Result> result = admanBidder.makeBids(httpCall, null); // then - assertThat(result.getErrors()).isEmpty(); - assertThat(result.getValue()).containsOnly(BidderBid.of(Bid.builder().impid("123").build(), banner, "USD")); + assertThat(result.getErrors()).hasSize(1) + .containsExactly(BidderError.badServerResponse("Failed to find impression 125")); + assertThat(result.getValue()).isEmpty(); } private static BidRequest givenBidRequest(