From e2efb5add30a5f0540b30d1ab590c5c6fb32052d Mon Sep 17 00:00:00 2001 From: Andrii Braslavskyi Date: Fri, 7 Aug 2020 17:24:17 +0300 Subject: [PATCH 1/2] Support pubmatic video duration and primary category --- .../server/auction/BidResponseCreator.java | 26 +++++- .../bidder/adoppler/AdopplerBidder.java | 4 - .../bidder/pubmatic/PubmaticBidder.java | 59 +++++++++++-- .../bidder/pubmatic/proto/PubmaticBidExt.java | 14 ++++ .../pubmatic/proto/VideoCreativeInfo.java | 11 +++ .../openrtb/ext/response/ExtBidPrebid.java | 4 +- .../auction/BidResponseCreatorTest.java | 39 +++++++-- .../server/auction/ExchangeServiceTest.java | 2 +- .../auction/StoredResponseProcessorTest.java | 2 +- .../auction/VideoResponseFactoryTest.java | 6 +- .../bidder/pubmatic/PubmaticBidderTest.java | 83 +++++++++++++++++++ .../handler/openrtb2/AmpHandlerTest.java | 30 +++---- 12 files changed, 236 insertions(+), 44 deletions(-) create mode 100644 src/main/java/org/prebid/server/bidder/pubmatic/proto/PubmaticBidExt.java create mode 100644 src/main/java/org/prebid/server/bidder/pubmatic/proto/VideoCreativeInfo.java diff --git a/src/main/java/org/prebid/server/auction/BidResponseCreator.java b/src/main/java/org/prebid/server/auction/BidResponseCreator.java index a7913e2f8a4..97407961be1 100644 --- a/src/main/java/org/prebid/server/auction/BidResponseCreator.java +++ b/src/main/java/org/prebid/server/auction/BidResponseCreator.java @@ -1,6 +1,7 @@ package org.prebid.server.auction; import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ObjectNode; import com.iab.openrtb.request.BidRequest; @@ -54,6 +55,7 @@ import org.prebid.server.proto.openrtb.ext.response.CacheAsset; import org.prebid.server.proto.openrtb.ext.response.Events; import org.prebid.server.proto.openrtb.ext.response.ExtBidPrebid; +import org.prebid.server.proto.openrtb.ext.response.ExtBidPrebidVideo; import org.prebid.server.proto.openrtb.ext.response.ExtBidResponse; import org.prebid.server.proto.openrtb.ext.response.ExtBidResponsePrebid; import org.prebid.server.proto.openrtb.ext.response.ExtBidderError; @@ -84,6 +86,10 @@ public class BidResponseCreator { + private static final TypeReference> EXT_PREBID_TYPE_REFERENCE = + new TypeReference>() { + }; + private static final String CACHE = "cache"; private static final String PREBID_EXT = "prebid"; @@ -719,9 +725,16 @@ private Bid toBid(BidderBid bidderBid, String bidder, ExtRequestTargeting target final Events events = eventsEnabled && eventsAllowedByRequest ? eventsService.createEvent(eventBidId, bidder, account.getId(), auctionTimestamp) : null; - - final ExtBidPrebid prebidExt = ExtBidPrebid.of( - generatedBidId, bidType, targetingKeywords, cache, storedVideo, events, null); + final ExtBidPrebid prebidExt = ExtBidPrebid + .builder() + .bidid(generatedBidId) + .type(bidType) + .targeting(targetingKeywords) + .cache(cache) + .storedRequestAttributes(storedVideo) + .events(events) + .video(getExtBidPrebidVideo(bid.getExt())) + .build(); final ExtPrebid bidExt = ExtPrebid.of(prebidExt, bid.getExt()); bid.setExt(mapper.mapper().valueToTree(bidExt)); @@ -729,6 +742,13 @@ private Bid toBid(BidderBid bidderBid, String bidder, ExtRequestTargeting target return bid; } + private ExtBidPrebidVideo getExtBidPrebidVideo(ObjectNode bidExt) { + final ExtPrebid extPrebid = mapper.mapper() + .convertValue(bidExt, EXT_PREBID_TYPE_REFERENCE); + final ExtBidPrebid extBidPrebid = extPrebid != null ? extPrebid.getPrebid() : null; + return extBidPrebid != null ? extBidPrebid.getVideo() : null; + } + private void addNativeMarkup(Bid bid, List imps) { final Response nativeMarkup; try { diff --git a/src/main/java/org/prebid/server/bidder/adoppler/AdopplerBidder.java b/src/main/java/org/prebid/server/bidder/adoppler/AdopplerBidder.java index 54dc9e0ce61..d750e0db631 100644 --- a/src/main/java/org/prebid/server/bidder/adoppler/AdopplerBidder.java +++ b/src/main/java/org/prebid/server/bidder/adoppler/AdopplerBidder.java @@ -187,10 +187,6 @@ private AdopplerResponseExt parseResponseExt(ObjectNode ext) { } } - private String head(List cat) { - return cat.size() == 0 ? "" : cat.get(0); - } - @Override public Map extractTargeting(ObjectNode ext) { return Collections.emptyMap(); diff --git a/src/main/java/org/prebid/server/bidder/pubmatic/PubmaticBidder.java b/src/main/java/org/prebid/server/bidder/pubmatic/PubmaticBidder.java index 12c6121327d..3f062228695 100644 --- a/src/main/java/org/prebid/server/bidder/pubmatic/PubmaticBidder.java +++ b/src/main/java/org/prebid/server/bidder/pubmatic/PubmaticBidder.java @@ -1,7 +1,7 @@ package org.prebid.server.bidder.pubmatic; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.type.TypeReference; -import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ObjectNode; import com.iab.openrtb.request.App; import com.iab.openrtb.request.Banner; @@ -9,6 +9,7 @@ import com.iab.openrtb.request.Imp; import com.iab.openrtb.request.Publisher; import com.iab.openrtb.request.Site; +import com.iab.openrtb.response.Bid; import com.iab.openrtb.response.BidResponse; import com.iab.openrtb.response.SeatBid; import io.vertx.core.http.HttpMethod; @@ -22,7 +23,9 @@ import org.prebid.server.bidder.model.HttpCall; import org.prebid.server.bidder.model.HttpRequest; import org.prebid.server.bidder.model.Result; +import org.prebid.server.bidder.pubmatic.proto.PubmaticBidExt; import org.prebid.server.bidder.pubmatic.proto.PubmaticRequestExt; +import org.prebid.server.bidder.pubmatic.proto.VideoCreativeInfo; import org.prebid.server.exception.PreBidException; import org.prebid.server.json.DecodeException; import org.prebid.server.json.JacksonMapper; @@ -31,6 +34,8 @@ import org.prebid.server.proto.openrtb.ext.request.pubmatic.ExtImpPubmatic; import org.prebid.server.proto.openrtb.ext.request.pubmatic.ExtImpPubmaticKeyVal; import org.prebid.server.proto.openrtb.ext.response.BidType; +import org.prebid.server.proto.openrtb.ext.response.ExtBidPrebid; +import org.prebid.server.proto.openrtb.ext.response.ExtBidPrebidVideo; import org.prebid.server.util.HttpUtil; import java.io.IOException; @@ -47,7 +52,7 @@ public class PubmaticBidder implements Bidder { private static final Logger logger = LoggerFactory.getLogger(PubmaticBidder.class); private static final String DEFAULT_BID_CURRENCY = "USD"; - private static final String BID_TYPE_EXT_KEY = "BidType"; + private static final String PREBID = "prebid"; private static final TypeReference> PUBMATIC_EXT_TYPE_REFERENCE = new TypeReference>() { }; @@ -255,27 +260,53 @@ public final Result> makeBids(HttpCall httpCall, Bid } } - private static List extractBids(BidResponse bidResponse) { + private List extractBids(BidResponse bidResponse) { return bidResponse == null || bidResponse.getSeatbid() == null ? Collections.emptyList() : bidsFromResponse(bidResponse); } - private static List bidsFromResponse(BidResponse bidResponse) { + private List bidsFromResponse(BidResponse bidResponse) { return bidResponse.getSeatbid().stream() .filter(Objects::nonNull) .map(SeatBid::getBid) .filter(Objects::nonNull) .flatMap(Collection::stream) - .map(bid -> BidderBid.of(bid, getBidType(bid.getExt()), DEFAULT_BID_CURRENCY)) + .map(this::bidderBid) .collect(Collectors.toList()); } - protected static BidType getBidType(ObjectNode bidExt) { - if (bidExt != null) { - final JsonNode bidTypeVal = bidExt.get(BID_TYPE_EXT_KEY); + private BidderBid bidderBid(Bid bid) { + final List bidCat = bid.getCat(); + final boolean updateBidCat = bidCat != null && bidCat.size() > 1; + final List singleElementCat = updateBidCat + ? Collections.singletonList(bidCat.get(0)) + : bidCat; + final PubmaticBidExt pubmaticBidExt = extractBidExt(bid.getExt()); + final Integer duration = getDuration(pubmaticBidExt); + final Bid updatedBid = updateBidCat || duration != null + ? bid.toBuilder() + .cat(singleElementCat) + .ext(duration != null ? updateBidExtWithExtPrebid(duration, bid.getExt()) : bid.getExt()) + .build() + : bid; + + return BidderBid.of(updatedBid, getBidType(pubmaticBidExt), DEFAULT_BID_CURRENCY); + } + + private PubmaticBidExt extractBidExt(ObjectNode bidExt) { + try { + return bidExt != null ? mapper.mapper().treeToValue(bidExt, PubmaticBidExt.class) : null; + } catch (JsonProcessingException e) { + throw new PreBidException(String.format("Error parsing pubmatic bid.ext %s", e.getMessage())); + } + } + + private static BidType getBidType(PubmaticBidExt pubmaticBidExt) { + if (pubmaticBidExt != null) { + final Integer bidTypeVal = pubmaticBidExt.getBidType(); if (bidTypeVal != null) { - switch (bidTypeVal.asInt()) { + switch (bidTypeVal) { case 1: return BidType.video; case 2: @@ -288,6 +319,16 @@ protected static BidType getBidType(ObjectNode bidExt) { return BidType.banner; } + private static Integer getDuration(PubmaticBidExt pubmaticBidExt) { + final VideoCreativeInfo video = pubmaticBidExt != null ? pubmaticBidExt.getVideo() : null; + return video != null ? video.getDuration() : null; + } + + private ObjectNode updateBidExtWithExtPrebid(Integer duration, ObjectNode extBid) { + final ExtBidPrebid extBidPrebid = ExtBidPrebid.builder().video(ExtBidPrebidVideo.of(duration, null)).build(); + return extBid.set(PREBID, mapper.mapper().valueToTree(extBidPrebid)); + } + @Override public final Map extractTargeting(ObjectNode ext) { return Collections.emptyMap(); diff --git a/src/main/java/org/prebid/server/bidder/pubmatic/proto/PubmaticBidExt.java b/src/main/java/org/prebid/server/bidder/pubmatic/proto/PubmaticBidExt.java new file mode 100644 index 00000000000..9d786e7728b --- /dev/null +++ b/src/main/java/org/prebid/server/bidder/pubmatic/proto/PubmaticBidExt.java @@ -0,0 +1,14 @@ +package org.prebid.server.bidder.pubmatic.proto; + +import com.fasterxml.jackson.annotation.JsonProperty; +import lombok.AllArgsConstructor; +import lombok.Value; + +@Value +@AllArgsConstructor(staticName = "of") +public class PubmaticBidExt { + @JsonProperty("BidType") + Integer bidType; + + VideoCreativeInfo video; +} diff --git a/src/main/java/org/prebid/server/bidder/pubmatic/proto/VideoCreativeInfo.java b/src/main/java/org/prebid/server/bidder/pubmatic/proto/VideoCreativeInfo.java new file mode 100644 index 00000000000..e7c8ef93b43 --- /dev/null +++ b/src/main/java/org/prebid/server/bidder/pubmatic/proto/VideoCreativeInfo.java @@ -0,0 +1,11 @@ +package org.prebid.server.bidder.pubmatic.proto; + +import lombok.AllArgsConstructor; +import lombok.Value; + +@Value +@AllArgsConstructor(staticName = "of") +public class VideoCreativeInfo { + + Integer duration; +} diff --git a/src/main/java/org/prebid/server/proto/openrtb/ext/response/ExtBidPrebid.java b/src/main/java/org/prebid/server/proto/openrtb/ext/response/ExtBidPrebid.java index d4fb6fd0b00..6099f23fc52 100644 --- a/src/main/java/org/prebid/server/proto/openrtb/ext/response/ExtBidPrebid.java +++ b/src/main/java/org/prebid/server/proto/openrtb/ext/response/ExtBidPrebid.java @@ -2,7 +2,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.iab.openrtb.request.Video; -import lombok.AllArgsConstructor; +import lombok.Builder; import lombok.Value; import java.util.Map; @@ -10,7 +10,7 @@ /** * Defines the contract for bidresponse.seatbid.bid[i].ext.prebid */ -@AllArgsConstructor(staticName = "of") +@Builder @Value public class ExtBidPrebid { diff --git a/src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java b/src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java index 2681219fbfe..2d9b06ec827 100644 --- a/src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java +++ b/src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java @@ -57,6 +57,7 @@ import org.prebid.server.proto.openrtb.ext.response.CacheAsset; import org.prebid.server.proto.openrtb.ext.response.Events; import org.prebid.server.proto.openrtb.ext.response.ExtBidPrebid; +import org.prebid.server.proto.openrtb.ext.response.ExtBidPrebidVideo; import org.prebid.server.proto.openrtb.ext.response.ExtBidResponse; import org.prebid.server.proto.openrtb.ext.response.ExtBidResponsePrebid; import org.prebid.server.proto.openrtb.ext.response.ExtBidderError; @@ -429,8 +430,7 @@ public void shouldSkipBidderResponsesWhereSeatBidContainEmptyBids() { public void shouldOverrideBidIdWhenGenerateBidIdIsTurnedOn() { // given final AuctionContext auctionContext = givenAuctionContext(givenBidRequest()); - final ExtPrebid prebid = ExtPrebid.of(ExtBidPrebid.of(null, banner, null, - null, null, null, null), null); + final ExtPrebid prebid = ExtPrebid.of(ExtBidPrebid.builder().type(banner).build(), null); final Bid bid = Bid.builder() .id("123") .impid("imp123") @@ -483,7 +483,7 @@ public void shouldSetExpectedResponseSeatBidAndBidFields() { .price(BigDecimal.ONE) .adm("adm") .ext(mapper.valueToTree(ExtPrebid.of( - ExtBidPrebid.of(null, banner, null, null, null, null, null), + ExtBidPrebid.builder().type(banner).build(), singletonMap("bidExt", 1)))) .build())) .build()); @@ -761,7 +761,7 @@ public void shouldTolerateMissingExtInSeatBidAndBid() { .impid("i1") .price(BigDecimal.ONE) .ext(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, banner, null, null, null, null, null), null))) + ExtPrebid.of(ExtBidPrebid.builder().type(banner).build(), null))) .build()); verify(cacheService, never()).cacheBidsOpenrtb(anyList(), anyList(), any(), any(), any(), any()); @@ -932,13 +932,13 @@ public void shouldPopulateTargetingKeywordsFromMediaTypePriceGranularities() { final ExtPriceGranularity priceGranularity = ExtPriceGranularity.of(2, singletonList(ExtGranularityRange.of(BigDecimal.valueOf(5), BigDecimal.valueOf(0.5)))); - final ExtMediaTypePriceGranularity mediaTypePriceGranuality = ExtMediaTypePriceGranularity.of( + final ExtMediaTypePriceGranularity mediaTypePriceGranularity = ExtMediaTypePriceGranularity.of( mapper.valueToTree( ExtPriceGranularity.of(3, singletonList( ExtGranularityRange.of(BigDecimal.valueOf(10), BigDecimal.valueOf(1))))), null, null); final ExtRequestTargeting targeting = ExtRequestTargeting.builder() .pricegranularity(mapper.valueToTree(priceGranularity)) - .mediatypepricegranularity(mediaTypePriceGranuality) + .mediatypepricegranularity(mediaTypePriceGranularity) .includewinners(true) .includebidderkeys(true) .build(); @@ -1102,6 +1102,33 @@ public void shouldAddExtPrebidEvents() { verify(cacheService, never()).cacheBidsOpenrtb(anyList(), anyList(), any(), any(), any(), any()); } + @Test + public void shouldAddExtPrebidVideo() { + // given + final AuctionContext auctionContext = givenAuctionContext(givenBidRequest()); + final ExtRequestTargeting targeting = givenTargeting(); + + final Bid bid = Bid.builder().id("bidId1").price(BigDecimal.valueOf(5.67)).impid("i1").impid("i1") + .ext(mapper.createObjectNode().set("prebid", mapper.valueToTree( + ExtBidPrebid.builder().video(ExtBidPrebidVideo.of(1, "category")).build()))).build(); + final List bidderResponses = singletonList(BidderResponse.of("bidder1", + givenSeatBid(BidderBid.of(bid, banner, "USD")), 100)); + + given(eventsService.winUrlTargeting(anyString(), anyString(), anyLong())).willReturn("http://win-url"); + + final Account account = Account.builder().id("accountId").eventsEnabled(true).build(); + + // when + final BidResponse bidResponse = bidResponseCreator.create(bidderResponses, auctionContext, targeting, + CACHE_INFO, account, true, 0L, false, timeout).result(); + + // then + assertThat(bidResponse.getSeatbid()).hasSize(1) + .flatExtracting(SeatBid::getBid) + .extracting(responseBid -> toExtPrebid(responseBid.getExt()).getPrebid().getVideo()) + .containsOnly(ExtBidPrebidVideo.of(1, "category")); + } + @Test public void shouldNotAddExtPrebidEventsIfEventsAreNotEnabled() { // given diff --git a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java index d44d3de9e09..ff6f94ce53d 100644 --- a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java +++ b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java @@ -2087,7 +2087,7 @@ private static Bid givenBid(Function bidBuilder) return bidBuilder.apply(Bid.builder() .id("bidId") .price(BigDecimal.ONE) - .ext(mapper.valueToTree(ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))) + .ext(mapper.valueToTree(ExtPrebid.of(ExtBidPrebid.builder().build(), null)))) .build(); } diff --git a/src/test/java/org/prebid/server/auction/StoredResponseProcessorTest.java b/src/test/java/org/prebid/server/auction/StoredResponseProcessorTest.java index 37e1aa31d31..5f6dde9645b 100644 --- a/src/test/java/org/prebid/server/auction/StoredResponseProcessorTest.java +++ b/src/test/java/org/prebid/server/auction/StoredResponseProcessorTest.java @@ -558,7 +558,7 @@ public void mergeWithBidderResponsesShouldResolveBidTypeFromStoredBidExt() { singletonList(BidderBid.of(Bid.builder().id("bid1").build(), BidType.banner, "USD")), emptyList(), emptyList()), 100)); - final ExtBidPrebid extBidPrebid = ExtBidPrebid.of(null, BidType.video, null, null, null, null, null); + final ExtBidPrebid extBidPrebid = ExtBidPrebid.builder().type(BidType.video).build(); final List seatBid = singletonList(SeatBid.builder() .seat("rubicon").bid(singletonList(Bid.builder().ext(mapper.createObjectNode() diff --git a/src/test/java/org/prebid/server/auction/VideoResponseFactoryTest.java b/src/test/java/org/prebid/server/auction/VideoResponseFactoryTest.java index 1e2a6b945e0..6916a5b9e10 100644 --- a/src/test/java/org/prebid/server/auction/VideoResponseFactoryTest.java +++ b/src/test/java/org/prebid/server/auction/VideoResponseFactoryTest.java @@ -42,19 +42,19 @@ public void shouldReturnExpectedVideoResponse() { final Bid bid0 = Bid.builder() .impid("0_0") .ext(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, targeting, null, null, null, null), + ExtPrebid.of(ExtBidPrebid.builder().targeting(targeting).build(), mapper.createObjectNode()))) .build(); final Bid bid1 = Bid.builder() .impid("1_1") .ext(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, targeting, null, null, null, null), + ExtPrebid.of(ExtBidPrebid.builder().targeting(targeting).build(), mapper.createObjectNode()))) .build(); final Bid bid2 = Bid.builder() .impid("2_1") .ext(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), + ExtPrebid.of(ExtBidPrebid.builder().build(), mapper.createObjectNode()))) .build(); diff --git a/src/test/java/org/prebid/server/bidder/pubmatic/PubmaticBidderTest.java b/src/test/java/org/prebid/server/bidder/pubmatic/PubmaticBidderTest.java index 8c9f5d2dd14..ba07245942a 100644 --- a/src/test/java/org/prebid/server/bidder/pubmatic/PubmaticBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/pubmatic/PubmaticBidderTest.java @@ -22,11 +22,15 @@ import org.prebid.server.bidder.model.HttpRequest; import org.prebid.server.bidder.model.HttpResponse; import org.prebid.server.bidder.model.Result; +import org.prebid.server.bidder.pubmatic.proto.PubmaticBidExt; import org.prebid.server.bidder.pubmatic.proto.PubmaticRequestExt; +import org.prebid.server.bidder.pubmatic.proto.VideoCreativeInfo; import org.prebid.server.proto.openrtb.ext.ExtPrebid; import org.prebid.server.proto.openrtb.ext.request.ExtRequest; import org.prebid.server.proto.openrtb.ext.request.pubmatic.ExtImpPubmatic; import org.prebid.server.proto.openrtb.ext.request.pubmatic.ExtImpPubmaticKeyVal; +import org.prebid.server.proto.openrtb.ext.response.ExtBidPrebid; +import org.prebid.server.proto.openrtb.ext.response.ExtBidPrebidVideo; import org.prebid.server.util.HttpUtil; import java.io.IOException; @@ -524,6 +528,85 @@ public void makeBidsShouldReturnXNativeBidIfExtBidContainsBidTypeTwo() throws Js .containsOnly(BidderBid.of(Bid.builder().impid("123").ext(bidType).build(), xNative, "USD")); } + @Test + public void makeBidsShouldFillExtBidPrebidVideoDurationIfDurationIsNotNull() throws JsonProcessingException { + // given + final HttpCall httpCall = givenHttpCall(null, + mapper.writeValueAsString( + givenBidResponse(bidBuilder -> bidBuilder.impid("123") + .ext(mapper.valueToTree(PubmaticBidExt.of(null, VideoCreativeInfo.of(1))))))); + + // when + final Result> result = pubmaticBidder.makeBids(httpCall, null); + + // then + assertThat(result.getErrors()).isEmpty(); + final ObjectNode bidExt = mapper.createObjectNode(); + bidExt.set("video", mapper.valueToTree(VideoCreativeInfo.of(1))); + bidExt.set("prebid", mapper.valueToTree(ExtBidPrebid.builder().video(ExtBidPrebidVideo.of(1, null)).build())); + + assertThat(result.getValue()) + .containsOnly(BidderBid.of(Bid.builder().impid("123").ext(bidExt).build(), banner, "USD")); + } + + @Test + public void makeBidsShouldNotFillExtBidPrebidVideoDurationIfDurationIsNull() throws JsonProcessingException { + // given + final HttpCall httpCall = givenHttpCall(null, + mapper.writeValueAsString( + givenBidResponse(bidBuilder -> bidBuilder.impid("123") + .ext(mapper.valueToTree(PubmaticBidExt.of(null, VideoCreativeInfo.of(null))))))); + + // when + final Result> result = pubmaticBidder.makeBids(httpCall, null); + + // then + assertThat(result.getErrors()).isEmpty(); + final ObjectNode bidExt = mapper.createObjectNode(); + bidExt.set("video", mapper.valueToTree(VideoCreativeInfo.of(null))); + + assertThat(result.getValue()) + .containsOnly(BidderBid.of(Bid.builder().impid("123").ext(bidExt).build(), banner, "USD")); + } + + @Test + public void makeBidsShouldNotFillExtBidPrebidVideoDurationIfVideoIsNull() throws JsonProcessingException { + // given + final HttpCall httpCall = givenHttpCall(null, + mapper.writeValueAsString( + givenBidResponse(bidBuilder -> bidBuilder.impid("123") + .ext(mapper.valueToTree(PubmaticBidExt.of(null, null)))))); + + // when + final Result> result = pubmaticBidder.makeBids(httpCall, null); + + // then + assertThat(result.getErrors()).isEmpty(); + + assertThat(result.getValue()) + .containsOnly(BidderBid.of(Bid.builder().impid("123").ext(mapper.createObjectNode()).build(), + banner, "USD")); + } + + @Test + public void makeBidsShouldTakeOnlyFirstCatElement() throws JsonProcessingException { + // given + final HttpCall httpCall = givenHttpCall(null, + mapper.writeValueAsString( + givenBidResponse(bidBuilder -> bidBuilder.impid("123") + .cat(asList("cat1", "cat2"))))); + + // when + final Result> result = pubmaticBidder.makeBids(httpCall, null); + + // then + assertThat(result.getErrors()).isEmpty(); + + assertThat(result.getValue()) + .containsOnly(BidderBid.of(Bid.builder().impid("123").cat(singletonList("cat1")).build(), + banner, "USD")); + } + @Test public void makeBidsShouldReturnBannerBidIfExtBidContainsIllegalBidType() throws JsonProcessingException { // given diff --git a/src/test/java/org/prebid/server/handler/openrtb2/AmpHandlerTest.java b/src/test/java/org/prebid/server/handler/openrtb2/AmpHandlerTest.java index 0dfcca73104..c57fd702009 100644 --- a/src/test/java/org/prebid/server/handler/openrtb2/AmpHandlerTest.java +++ b/src/test/java/org/prebid/server/handler/openrtb2/AmpHandlerTest.java @@ -349,8 +349,8 @@ public void shouldRespondWithExpectedResponse() { targeting.put("key1", "value1"); targeting.put("hb_cache_id_bidder1", "value2"); given(exchangeService.holdAuction(any())) - .willReturn(givenBidResponse(mapper.valueToTree(ExtPrebid.of(ExtBidPrebid.of( - null, null, targeting, null, null, null, null), null)))); + .willReturn(givenBidResponse(mapper.valueToTree(ExtPrebid.of( + ExtBidPrebid.builder().targeting(targeting).build(), null)))); // when ampHandler.handle(routingContext); @@ -381,7 +381,7 @@ public void shouldRespondWithCustomTargetingIncluded() { .bid(singletonList(Bid.builder() .ext(mapper.valueToTree( ExtPrebid.of( - ExtBidPrebid.of(null, null, targeting, null, null, null, null), + ExtBidPrebid.builder().targeting(targeting).build(), mapper.createObjectNode()))) .build())) .build())) @@ -461,7 +461,7 @@ public void shouldIncrementOkAmpRequestMetrics() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))); + ExtPrebid.of(ExtBidPrebid.builder().build(), null)))); // when ampHandler.handle(routingContext); @@ -478,7 +478,7 @@ public void shouldIncrementAppRequestMetrics() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))); + ExtPrebid.of(ExtBidPrebid.builder().build(), null)))); // when ampHandler.handle(routingContext); @@ -495,7 +495,7 @@ public void shouldIncrementNoCookieMetrics() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))); + ExtPrebid.of(ExtBidPrebid.builder().build(), null)))); given(uidsCookie.hasLiveUids()).willReturn(false); @@ -518,7 +518,7 @@ public void shouldIncrementImpsRequestedMetrics() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))); + ExtPrebid.of(ExtBidPrebid.builder().build(), null)))); // when ampHandler.handle(routingContext); @@ -537,7 +537,7 @@ public void shouldIncrementImpsTypesMetrics() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))); + ExtPrebid.of(ExtBidPrebid.builder().build(), null)))); // when ampHandler.handle(routingContext); @@ -621,7 +621,7 @@ public void shouldUpdateNetworkErrorMetric() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))); + ExtPrebid.of(ExtBidPrebid.builder().build(), null)))); // simulate calling exception handler that is supposed to update networkerr timer value given(httpResponse.exceptionHandler(any())).willAnswer(inv -> { @@ -644,7 +644,7 @@ public void shouldNotUpdateNetworkErrorMetricIfResponseSucceeded() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))); + ExtPrebid.of(ExtBidPrebid.builder().build(), null)))); // when ampHandler.handle(routingContext); @@ -661,7 +661,7 @@ public void shouldUpdateNetworkErrorMetricIfClientClosedConnection() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, null, null, null, null, null), null)))); + ExtPrebid.of(ExtBidPrebid.builder().build(), null)))); given(routingContext.response().closed()).willReturn(true); @@ -728,8 +728,8 @@ public void shouldPassSuccessfulEventToAnalyticsReporter() { given(exchangeService.holdAuction(any())) .willReturn(givenBidResponse(mapper.valueToTree( - ExtPrebid.of(ExtBidPrebid.of(null, null, singletonMap("hb_cache_id_bidder1", "value1"), - null, null, null, null), + ExtPrebid.of( + ExtBidPrebid.builder().targeting(singletonMap("hb_cache_id_bidder1", "value1")).build(), null)))); // when @@ -747,8 +747,8 @@ public void shouldPassSuccessfulEventToAnalyticsReporter() { .bidResponse(BidResponse.builder().seatbid(singletonList(SeatBid.builder() .bid(singletonList(Bid.builder() .ext(mapper.valueToTree(ExtPrebid.of( - ExtBidPrebid.of(null, null, singletonMap("hb_cache_id_bidder1", "value1"), - null, null, null, null), + ExtBidPrebid.builder().targeting(singletonMap("hb_cache_id_bidder1", "value1")) + .build(), null))) .build())) .build())) From c8ab3cd8fabcca2631ad3cff6b062a5a538101d9 Mon Sep 17 00:00:00 2001 From: Andrii Braslavskyi Date: Mon, 7 Sep 2020 15:28:55 +0300 Subject: [PATCH 2/2] Minor refactoring according to review comments --- .../server/auction/BidResponseCreator.java | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/prebid/server/auction/BidResponseCreator.java b/src/main/java/org/prebid/server/auction/BidResponseCreator.java index 97407961be1..f0317bd980f 100644 --- a/src/main/java/org/prebid/server/auction/BidResponseCreator.java +++ b/src/main/java/org/prebid/server/auction/BidResponseCreator.java @@ -725,15 +725,15 @@ private Bid toBid(BidderBid bidderBid, String bidder, ExtRequestTargeting target final Events events = eventsEnabled && eventsAllowedByRequest ? eventsService.createEvent(eventBidId, bidder, account.getId(), auctionTimestamp) : null; - final ExtBidPrebid prebidExt = ExtBidPrebid - .builder() + final ExtBidPrebidVideo extBidPrebidVideo = getExtBidPrebidVideo(bid.getExt()); + final ExtBidPrebid prebidExt = ExtBidPrebid.builder() .bidid(generatedBidId) .type(bidType) .targeting(targetingKeywords) .cache(cache) .storedRequestAttributes(storedVideo) .events(events) - .video(getExtBidPrebidVideo(bid.getExt())) + .video(extBidPrebidVideo) .build(); final ExtPrebid bidExt = ExtPrebid.of(prebidExt, bid.getExt()); @@ -742,13 +742,6 @@ private Bid toBid(BidderBid bidderBid, String bidder, ExtRequestTargeting target return bid; } - private ExtBidPrebidVideo getExtBidPrebidVideo(ObjectNode bidExt) { - final ExtPrebid extPrebid = mapper.mapper() - .convertValue(bidExt, EXT_PREBID_TYPE_REFERENCE); - final ExtBidPrebid extBidPrebid = extPrebid != null ? extPrebid.getPrebid() : null; - return extBidPrebid != null ? extBidPrebid.getVideo() : null; - } - private void addNativeMarkup(Bid bid, List imps) { final Response nativeMarkup; try { @@ -911,4 +904,14 @@ private ExtPriceGranularity parsePriceGranularity(JsonNode priceGranularity) { private CacheAsset toCacheAsset(String cacheId) { return CacheAsset.of(cacheAssetUrlTemplate.concat(cacheId), cacheId); } + + /** + * Creates {@link ExtBidPrebidVideo} from bid extension. + */ + private ExtBidPrebidVideo getExtBidPrebidVideo(ObjectNode bidExt) { + final ExtPrebid extPrebid = mapper.mapper() + .convertValue(bidExt, EXT_PREBID_TYPE_REFERENCE); + final ExtBidPrebid extBidPrebid = extPrebid != null ? extPrebid.getPrebid() : null; + return extBidPrebid != null ? extBidPrebid.getVideo() : null; + } }