From d8691a9fc4b68fa39c28f417ddb98850fe193a82 Mon Sep 17 00:00:00 2001 From: Anton Babak <76536883+AntoxaAntoxic@users.noreply.github.com> Date: Fri, 24 May 2024 11:56:47 +0200 Subject: [PATCH] AdView: Support Multi Imp Request (#3101) --- .../server/bidder/adview/AdviewBidder.java | 86 +++++----- .../bidder/adview/AdviewBidderTest.java | 157 +++++++++--------- .../adview/test-adview-bid-response.json | 3 +- .../adview/test-auction-adview-response.json | 1 + 4 files changed, 129 insertions(+), 118 deletions(-) diff --git a/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java b/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java index cc62739b8f0..89973ed8437 100644 --- a/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java +++ b/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java @@ -5,6 +5,7 @@ import com.iab.openrtb.request.BidRequest; import com.iab.openrtb.request.Format; import com.iab.openrtb.request.Imp; +import com.iab.openrtb.response.Bid; import com.iab.openrtb.response.BidResponse; import com.iab.openrtb.response.SeatBid; import org.apache.commons.collections4.CollectionUtils; @@ -55,20 +56,23 @@ public AdviewBidder(String endpointUrl, @Override public Result>> makeHttpRequests(BidRequest request) { - final Imp firstImp = request.getImp().get(0); - final ExtImpAdview extImpAdview; - final BidRequest modifiedBidRequest; - - try { - extImpAdview = parseExtImp(firstImp); - final Price bidFloorPrice = resolveBidFloor(firstImp, request); - modifiedBidRequest = modifyRequest(request, extImpAdview.getMasterTagId(), bidFloorPrice); - } catch (PreBidException e) { - return Result.withError(BidderError.badInput(e.getMessage())); + final List errors = new ArrayList<>(); + final List> httpRequests = new ArrayList<>(); + + for (Imp imp: request.getImp()) { + try { + final ExtImpAdview extImp = parseExtImp(imp); + final Price bidFloorPrice = resolveBidFloor(imp, request); + final Imp modifiedImp = modifyImp(imp, extImp.getMasterTagId(), bidFloorPrice); + final BidRequest modifiedRequest = modifyRequest(request, modifiedImp); + final String resolvedUrl = resolveEndpoint(extImp.getAccountId()); + httpRequests.add(BidderUtil.defaultRequest(modifiedRequest, resolvedUrl, mapper)); + } catch (PreBidException e) { + errors.add(BidderError.badInput(e.getMessage())); + } } - return Result.withValue( - BidderUtil.defaultRequest(modifiedBidRequest, resolveEndpoint(extImpAdview.getAccountId()), mapper)); + return Result.of(httpRequests, errors); } private ExtImpAdview parseExtImp(Imp imp) { @@ -99,19 +103,13 @@ private Price convertBidFloor(Price bidFloorPrice, String impId, BidRequest bidR } } - private static BidRequest modifyRequest(BidRequest bidRequest, String masterTagId, Price bidFloorPrice) { + private static BidRequest modifyRequest(BidRequest bidRequest, Imp modifiedImp) { return bidRequest.toBuilder() - .imp(modifyImps(bidRequest.getImp(), masterTagId, bidFloorPrice)) + .imp(Collections.singletonList(modifiedImp)) .cur(Collections.singletonList(BIDDER_CURRENCY)) .build(); } - private static List modifyImps(List imps, String masterTagId, Price bidFloorPrice) { - final List modifiedImps = new ArrayList<>(imps); - modifiedImps.set(0, modifyImp(imps.get(0), masterTagId, bidFloorPrice)); - return modifiedImps; - } - private static Imp modifyImp(Imp imp, String masterTagId, Price bidFloorPrice) { return imp.toBuilder() .tagid(masterTagId) @@ -140,40 +138,54 @@ private String resolveEndpoint(String accountId) { public final Result> makeBids(BidderCall httpCall, BidRequest bidRequest) { try { final BidResponse bidResponse = mapper.decodeValue(httpCall.getResponse().getBody(), BidResponse.class); - return Result.withValues(extractBids(httpCall.getRequest().getPayload(), bidResponse)); - } catch (DecodeException e) { + final List errors = new ArrayList<>(); + return Result.of(extractBids(bidResponse, errors), errors); + } catch (DecodeException | PreBidException e) { return Result.withError(BidderError.badServerResponse(e.getMessage())); } } - private static List extractBids(BidRequest bidRequest, BidResponse bidResponse) { + private static List extractBids(BidResponse bidResponse, List errors) { if (bidResponse == null || CollectionUtils.isEmpty(bidResponse.getSeatbid())) { return Collections.emptyList(); } - return bidsFromResponse(bidRequest, bidResponse); + return bidsFromResponse(bidResponse, errors); } - private static List bidsFromResponse(BidRequest bidRequest, BidResponse bidResponse) { + private static List bidsFromResponse(BidResponse bidResponse, List errors) { return bidResponse.getSeatbid().stream() .filter(Objects::nonNull) .map(SeatBid::getBid) .filter(Objects::nonNull) .flatMap(Collection::stream) - .map(bid -> BidderBid.of(bid, getBidMediaType(bid.getImpid(), bidRequest.getImp()), - bidResponse.getCur())) + .map(bid -> makeBid(bid, bidResponse.getCur(), errors)) + .filter(Objects::nonNull) .toList(); } - private static BidType getBidMediaType(String impId, List imps) { - for (Imp imp : imps) { - if (imp.getId().equals(impId)) { - if (imp.getVideo() != null) { - return BidType.video; - } else if (imp.getXNative() != null) { - return BidType.xNative; - } - } + private static BidderBid makeBid(Bid bid, String currency, List errors) { + try { + final BidType mediaType = getBidMediaType(bid); + return BidderBid.of(bid, mediaType, currency); + } catch (PreBidException e) { + errors.add(BidderError.badServerResponse(e.getMessage())); + return null; } - return BidType.banner; + + } + + private static BidType getBidMediaType(Bid bid) { + final Integer markupType = bid.getMtype(); + if (markupType == null) { + throw new PreBidException("Missing MType for bid: " + bid.getId()); + } + + return switch (markupType) { + case 1 -> BidType.banner; + case 2 -> BidType.video; + case 4 -> BidType.xNative; + default -> throw new PreBidException( + "Unable to fetch mediaType " + bid.getMtype() + " in multi-format: " + bid.getImpid()); + }; } } diff --git a/src/test/java/org/prebid/server/bidder/adview/AdviewBidderTest.java b/src/test/java/org/prebid/server/bidder/adview/AdviewBidderTest.java index 592b172df28..0ce0870ad0b 100644 --- a/src/test/java/org/prebid/server/bidder/adview/AdviewBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/adview/AdviewBidderTest.java @@ -1,12 +1,11 @@ package org.prebid.server.bidder.adview; import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.node.ObjectNode; import com.iab.openrtb.request.Banner; import com.iab.openrtb.request.BidRequest; 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; import com.iab.openrtb.response.SeatBid; @@ -30,7 +29,9 @@ import java.math.BigDecimal; import java.util.List; +import java.util.Set; import java.util.function.UnaryOperator; +import java.util.stream.Stream; import static java.util.Collections.singletonList; import static java.util.function.UnaryOperator.identity; @@ -68,7 +69,7 @@ public void creationShouldFailOnInvalidEndpointUrl() { } @Test - public void makeHttpRequestsShouldReturnErrorsOfNotValidImps() { + public void makeHttpRequestsShouldReturnErrorsWhenImpExtIsInvalid() { // given final BidRequest bidRequest = givenBidRequest( impBuilder -> impBuilder @@ -82,24 +83,40 @@ public void makeHttpRequestsShouldReturnErrorsOfNotValidImps() { } @Test - public void makeHttpRequestsShouldSetTagIdOfFirstImp() { + public void makeHttpRequestsShouldSetTagIdForImp() { // given - final BidRequest bidRequest = givenBidRequest(identity()); + final BidRequest bidRequest = givenBidRequest(impBuilder -> impBuilder.ext(givenImpExt("tagId"))); // when final Result>> result = target.makeHttpRequests(bidRequest); // then + final BidRequest expectedRequest = givenBidRequest( + impBuilder -> impBuilder.tagid("tagId").ext(givenImpExt("tagId"))); assertThat(result.getErrors()).isEmpty(); - assertThat(result.getValue()) + assertThat(result.getValue()).hasSize(1) .extracting(HttpRequest::getPayload) - .flatExtracting(BidRequest::getImp) - .extracting(Imp::getTagid) - .containsExactly("publisherId"); + .containsExactly(expectedRequest); + } + + @Test + public void makeHttpRequestsShouldCreateHttpRequestPerImp() { + // given + final BidRequest bidRequest = givenBidRequest( + impBuilder -> impBuilder.id("impId1"), + impBuilder -> impBuilder.id("impId2")); + // when + final Result>> result = target.makeHttpRequests(bidRequest); + + // then + assertThat(result.getErrors()).isEmpty(); + assertThat(result.getValue()).hasSize(2) + .extracting(HttpRequest::getImpIds) + .containsExactly(Set.of("impId1"), Set.of("impId2")); } @Test - public void makeHttpRequestsShouldModifyFirstImpBanner() { + public void makeHttpRequestsShouldModifyImpBanner() { // given final Banner banner = Banner.builder() .w(2) @@ -128,7 +145,10 @@ public void makeHttpRequestsShouldConvertCurrencyIfRequestCurrencyDoesNotMatchBi .willReturn(BigDecimal.TEN); final BidRequest bidRequest = givenBidRequest( - impBuilder -> impBuilder.bidfloor(BigDecimal.ONE).bidfloorcur("EUR")); + impBuilder -> impBuilder.bidfloor(BigDecimal.ONE).bidfloorcur("EUR")) + .toBuilder() + .cur(List.of("EUR", "UAH")) + .build(); // when final Result>> result = target.makeHttpRequests(bidRequest); @@ -140,6 +160,10 @@ public void makeHttpRequestsShouldConvertCurrencyIfRequestCurrencyDoesNotMatchBi .flatExtracting(BidRequest::getImp) .extracting(Imp::getBidfloor, Imp::getBidfloorcur) .containsOnly(tuple(BigDecimal.TEN, "USD")); + assertThat(result.getValue()) + .extracting(HttpRequest::getPayload) + .flatExtracting(BidRequest::getCur) + .containsExactly("USD"); } @Test @@ -199,25 +223,6 @@ public void makeHttpRequestsShouldNotModifyFirstImpBannerIfFirstFormatIsAbsent() .containsExactly(Banner.builder().format(singletonList(null)).w(2).h(2).build()); } - @Test - public void makeHttpRequestsShouldModifyOnlyFirstImp() { - // given - final BidRequest bidRequest = givenBidRequest(identity(), List.of( - identity(), - impBuilder -> impBuilder.id("456").ext(null))); - - // when - final Result>> result = target.makeHttpRequests(bidRequest); - - // then - assertThat(result.getErrors()).isEmpty(); - assertThat(result.getValue()) - .extracting(HttpRequest::getPayload) - .flatExtracting(BidRequest::getImp) - .extracting(Imp::getTagid) - .containsExactlyInAnyOrder(null, "publisherId"); - } - @Test public void makeHttpRequestsShouldCreateCorrectURL() { // given @@ -236,7 +241,7 @@ public void makeHttpRequestsShouldCreateCorrectURL() { @Test public void makeBidsShouldReturnErrorIfResponseBodyCouldNotBeParsed() { // given - final BidderCall httpCall = givenHttpCall(null, "invalid"); + final BidderCall httpCall = givenHttpCall("invalid"); // when final Result> result = target.makeBids(httpCall, null); @@ -253,8 +258,7 @@ public void makeBidsShouldReturnErrorIfResponseBodyCouldNotBeParsed() { @Test public void makeBidsShouldReturnEmptyListIfBidResponseSeatBidIsNull() throws JsonProcessingException { // given - final BidderCall httpCall = givenHttpCall(null, - mapper.writeValueAsString(BidResponse.builder().build())); + final BidderCall httpCall = givenHttpCall(mapper.writeValueAsString(BidResponse.builder().build())); // when final Result> result = target.makeBids(httpCall, null); @@ -265,105 +269,98 @@ public void makeBidsShouldReturnEmptyListIfBidResponseSeatBidIsNull() throws Jso } @Test - public void makeBidsShouldReturnVideoBidIfVideoIsPresentInRequestImp() throws JsonProcessingException { + public void makeBidsShouldReturnBannerBidSuccessfully() throws JsonProcessingException { // given - final BidderCall httpCall = givenHttpCall( - givenBidRequest(impBuilder -> impBuilder.video(Video.builder().build())), - mapper.writeValueAsString(givenBidResponse(bidBuilder -> bidBuilder.impid("123")))); + final Bid bannerBid = Bid.builder().impid("1").mtype(1).build(); + + final BidderCall httpCall = givenHttpCall(givenBidResponse(bannerBid)); // when final Result> result = target.makeBids(httpCall, null); // then assertThat(result.getErrors()).isEmpty(); - assertThat(result.getValue()) - .containsExactly(BidderBid.of(Bid.builder().impid("123").build(), video, null)); + assertThat(result.getValue()).containsOnly(BidderBid.of(bannerBid, banner, "USD")); + } @Test - public void makeBidsShouldReturnNativeBidIfVideoIsAbsentAndNativeIsPresentInRequestImp() - throws JsonProcessingException { + public void makeBidsShouldReturnVideoBidSuccessfully() throws JsonProcessingException { // given - final BidderCall httpCall = givenHttpCall( - givenBidRequest(impBuilder -> impBuilder.banner(null).xNative(Native.builder().build())), - mapper.writeValueAsString(givenBidResponse(bidBuilder -> bidBuilder.impid("123")))); + final Bid videoBid = Bid.builder().impid("2").mtype(2).build(); + final BidderCall httpCall = givenHttpCall(givenBidResponse(videoBid)); // when final Result> result = target.makeBids(httpCall, null); // then assertThat(result.getErrors()).isEmpty(); - assertThat(result.getValue()) - .containsExactly(BidderBid.of(Bid.builder().impid("123").build(), xNative, null)); + assertThat(result.getValue()).containsOnly(BidderBid.of(videoBid, video, "USD")); } @Test - public void makeBidsShouldReturnBannerBidIfVideoAndNativeAreAbsentInRequestImp() throws JsonProcessingException { + public void makeBidsShouldReturnNativeBidSuccessfully() throws JsonProcessingException { // given - final BidderCall httpCall = givenHttpCall( - givenBidRequest(impBuilder -> impBuilder.xNative(null).video(null)), - mapper.writeValueAsString(givenBidResponse(bidBuilder -> bidBuilder.impid("123")))); + final Bid nativeBid = Bid.builder().impid("4").mtype(4).build(); + + final BidderCall httpCall = givenHttpCall(givenBidResponse(nativeBid)); // when final Result> result = target.makeBids(httpCall, null); // then assertThat(result.getErrors()).isEmpty(); - assertThat(result.getValue()) - .containsExactly(BidderBid.of(Bid.builder().impid("123").build(), banner, null)); + assertThat(result.getValue()).containsOnly(BidderBid.of(nativeBid, xNative, "USD")); } @Test - public void makeBidsShouldReturnBannerBidIfBannerAndVideoAreAbsentInRequestImp() throws JsonProcessingException { + public void makeBidsShouldReturnErrorWhenImpTypeIsNotSupported() throws JsonProcessingException { // given - final BidderCall httpCall = givenHttpCall( - givenBidRequest(impBuilder -> impBuilder.banner(null).video(null)), - mapper.writeValueAsString(givenBidResponse(bidBuilder -> bidBuilder.impid("123")))); + final Bid bannerBid = Bid.builder().impid("id").mtype(1).build(); + final Bid audioBid = Bid.builder().impid("id").mtype(3).build(); + final BidderCall httpCall = givenHttpCall(givenBidResponse(bannerBid, audioBid)); // when final Result> result = target.makeBids(httpCall, null); - // then - assertThat(result.getErrors()).isEmpty(); - assertThat(result.getValue()) - .containsExactly(BidderBid.of(Bid.builder().impid("123").build(), banner, null)); + assertThat(result.getErrors()) + .containsExactly(BidderError.badServerResponse("Unable to fetch mediaType 3 in multi-format: id")); + assertThat(result.getValue()).containsOnly(BidderBid.of(bannerBid, banner, "USD")); } - private static BidRequest givenBidRequest( - UnaryOperator bidRequestCustomizer, - List> impCustomizers) { - - return bidRequestCustomizer.apply(BidRequest.builder() - .imp(impCustomizers.stream() - .map(AdviewBidderTest::givenImp) - .toList())) + private static BidRequest givenBidRequest(UnaryOperator... impCustomizers) { + return BidRequest.builder() + .cur(List.of("USD")) + .imp(Stream.of(impCustomizers) + .map(AdviewBidderTest::givenImp) + .toList()) .build(); } - private static BidRequest givenBidRequest(UnaryOperator impCustomizer) { - return givenBidRequest(identity(), singletonList(impCustomizer)); - } - private static Imp givenImp(UnaryOperator impCustomizer) { return impCustomizer.apply(Imp.builder() .id("123") .banner(Banner.builder().w(23).h(25).build()) - .ext(mapper.valueToTree(ExtPrebid.of(null, - ExtImpAdview.of("publisherId", "accountId"))))) + .ext(givenImpExt("publisherId"))) .build(); } - private static BidResponse givenBidResponse(UnaryOperator bidCustomizer) { - return BidResponse.builder() + private static ObjectNode givenImpExt(String masterTagId) { + return mapper.valueToTree(ExtPrebid.of(null, ExtImpAdview.of(masterTagId, "accountId"))); + } + + private static String givenBidResponse(Bid... bid) throws JsonProcessingException { + return mapper.writeValueAsString(BidResponse.builder() + .cur("USD") .seatbid(singletonList(SeatBid.builder() - .bid(singletonList(bidCustomizer.apply(Bid.builder()).build())) + .bid(List.of(bid)) .build())) - .build(); + .build()); } - private static BidderCall givenHttpCall(BidRequest bidRequest, String body) { + private static BidderCall givenHttpCall(String body) { return BidderCall.succeededHttp( - HttpRequest.builder().payload(bidRequest).build(), + HttpRequest.builder().payload(null).build(), HttpResponse.of(200, null, body), null); } diff --git a/src/test/resources/org/prebid/server/it/openrtb2/adview/test-adview-bid-response.json b/src/test/resources/org/prebid/server/it/openrtb2/adview/test-adview-bid-response.json index 8ad1ca21f66..e6fa819b842 100644 --- a/src/test/resources/org/prebid/server/it/openrtb2/adview/test-adview-bid-response.json +++ b/src/test/resources/org/prebid/server/it/openrtb2/adview/test-adview-bid-response.json @@ -9,7 +9,8 @@ "price": 0.01, "id": "bid_id", "impid": "imp_id", - "cid": "8048" + "cid": "8048", + "mtype": 1 } ], "type": "banner" diff --git a/src/test/resources/org/prebid/server/it/openrtb2/adview/test-auction-adview-response.json b/src/test/resources/org/prebid/server/it/openrtb2/adview/test-auction-adview-response.json index 689d795c1ba..eccc7f38dec 100644 --- a/src/test/resources/org/prebid/server/it/openrtb2/adview/test-auction-adview-response.json +++ b/src/test/resources/org/prebid/server/it/openrtb2/adview/test-auction-adview-response.json @@ -10,6 +10,7 @@ "adid": "2068416", "cid": "8048", "crid": "24080", + "mtype": 1, "ext": { "origbidcpm": 0.01, "prebid": {