From 1cd9082fd315c6e5c15049616c7a9797d50d0fca Mon Sep 17 00:00:00 2001 From: Alex Maltsev Date: Wed, 4 May 2022 17:02:37 +0300 Subject: [PATCH 1/2] Fixed invalid json property naming --- .../adnuntius/model/response/AdnuntiusAd.java | 7 +++++++ .../model/response/AdnuntiusAdsUnit.java | 3 +++ .../model/response/AdnuntiusResponse.java | 2 +- .../adnuntius/test-adnuntius-bid-response.json | 18 +++++++++--------- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/prebid/server/bidder/adnuntius/model/response/AdnuntiusAd.java b/src/main/java/org/prebid/server/bidder/adnuntius/model/response/AdnuntiusAd.java index 411493c7e26..82ea1f53045 100644 --- a/src/main/java/org/prebid/server/bidder/adnuntius/model/response/AdnuntiusAd.java +++ b/src/main/java/org/prebid/server/bidder/adnuntius/model/response/AdnuntiusAd.java @@ -1,5 +1,6 @@ package org.prebid.server.bidder.adnuntius.model.response; +import com.fasterxml.jackson.annotation.JsonProperty; import lombok.Builder; import lombok.Value; @@ -11,15 +12,21 @@ public class AdnuntiusAd { AdnuntiusBid bid; + @JsonProperty("adId") String adId; + @JsonProperty("creativeWidth") String creativeWidth; + @JsonProperty("creativeHeight") String creativeHeight; + @JsonProperty("creativeId") String creativeId; + @JsonProperty("lineItemId") String lineItemId; + @JsonProperty("destinationUrls") Map destinationUrls; } diff --git a/src/main/java/org/prebid/server/bidder/adnuntius/model/response/AdnuntiusAdsUnit.java b/src/main/java/org/prebid/server/bidder/adnuntius/model/response/AdnuntiusAdsUnit.java index bd9badb8b97..b4270a1584d 100644 --- a/src/main/java/org/prebid/server/bidder/adnuntius/model/response/AdnuntiusAdsUnit.java +++ b/src/main/java/org/prebid/server/bidder/adnuntius/model/response/AdnuntiusAdsUnit.java @@ -1,5 +1,6 @@ package org.prebid.server.bidder.adnuntius.model.response; +import com.fasterxml.jackson.annotation.JsonProperty; import lombok.Builder; import lombok.Value; @@ -9,8 +10,10 @@ @Value public class AdnuntiusAdsUnit { + @JsonProperty("auId") String auId; + @JsonProperty("targetId") String targetId; String html; diff --git a/src/main/java/org/prebid/server/bidder/adnuntius/model/response/AdnuntiusResponse.java b/src/main/java/org/prebid/server/bidder/adnuntius/model/response/AdnuntiusResponse.java index 77e471b8d1f..6ce62d9afdb 100644 --- a/src/main/java/org/prebid/server/bidder/adnuntius/model/response/AdnuntiusResponse.java +++ b/src/main/java/org/prebid/server/bidder/adnuntius/model/response/AdnuntiusResponse.java @@ -8,6 +8,6 @@ @Value(staticConstructor = "of") public class AdnuntiusResponse { - @JsonProperty("ad_units") + @JsonProperty("adUnits") List adsUnits; } diff --git a/src/test/resources/org/prebid/server/it/openrtb2/adnuntius/test-adnuntius-bid-response.json b/src/test/resources/org/prebid/server/it/openrtb2/adnuntius/test-adnuntius-bid-response.json index 896e5947bb7..8b1246eb133 100644 --- a/src/test/resources/org/prebid/server/it/openrtb2/adnuntius/test-adnuntius-bid-response.json +++ b/src/test/resources/org/prebid/server/it/openrtb2/adnuntius/test-adnuntius-bid-response.json @@ -1,8 +1,8 @@ { - "ad_units": [ + "adUnits": [ { - "au_id": "some_au_id", - "target_id": "some_au_id-imp_id", + "auId": "some_au_id", + "targetId": "some_au_id-imp_id", "html": "some_html", "ads": [ { @@ -10,12 +10,12 @@ "amount": 42.42, "currency": "USD" }, - "ad_id": "some_ad_id", - "creative_width": "140", - "creative_height": "90", - "creative_id": "some_creative_id", - "line_item_id": "some_line_item_id", - "destination_urls": { + "adId": "some_ad_id", + "creativeWidth": "140", + "creativeHeight": "90", + "creativeId": "some_creative_id", + "lineItemId": "some_line_item_id", + "destinationUrls": { "some_url": "https://www.domain.dm/uri" } } From e95d4814680f4b1bdaac565e4a810bdb59b951fa Mon Sep 17 00:00:00 2001 From: Alex Maltsev Date: Tue, 17 May 2022 15:27:05 +0300 Subject: [PATCH 2/2] Fixed impId resolution bug --- .../bidder/adnuntius/AdnuntiusBidder.java | 44 +++++++++--------- .../model/util/AdsUnitWithImpId.java | 12 +++++ .../bidder/adnuntius/AdnuntiusBidderTest.java | 46 +++++++++++++++---- 3 files changed, 72 insertions(+), 30 deletions(-) create mode 100644 src/main/java/org/prebid/server/bidder/adnuntius/model/util/AdsUnitWithImpId.java diff --git a/src/main/java/org/prebid/server/bidder/adnuntius/AdnuntiusBidder.java b/src/main/java/org/prebid/server/bidder/adnuntius/AdnuntiusBidder.java index 1ffd6ebc8df..84b2304b8b1 100644 --- a/src/main/java/org/prebid/server/bidder/adnuntius/AdnuntiusBidder.java +++ b/src/main/java/org/prebid/server/bidder/adnuntius/AdnuntiusBidder.java @@ -11,7 +11,6 @@ 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.apache.http.client.utils.URIBuilder; import org.prebid.server.bidder.Bidder; @@ -22,6 +21,7 @@ import org.prebid.server.bidder.adnuntius.model.response.AdnuntiusAdsUnit; import org.prebid.server.bidder.adnuntius.model.response.AdnuntiusBid; import org.prebid.server.bidder.adnuntius.model.response.AdnuntiusResponse; +import org.prebid.server.bidder.adnuntius.model.util.AdsUnitWithImpId; import org.prebid.server.bidder.model.BidderBid; import org.prebid.server.bidder.model.BidderError; import org.prebid.server.bidder.model.HttpCall; @@ -49,6 +49,7 @@ import java.util.Map; import java.util.Objects; import java.util.stream.Collectors; +import java.util.stream.IntStream; public class AdnuntiusBidder implements Bidder { @@ -192,26 +193,36 @@ public Result> makeBids(HttpCall httpCall, Bid try { final String body = httpCall.getResponse().getBody(); final AdnuntiusResponse bidResponse = mapper.decodeValue(body, AdnuntiusResponse.class); - return Result.withValues(extractBids(bidResponse)); + return Result.withValues(extractBids(bidRequest, bidResponse)); } catch (DecodeException | PreBidException e) { return Result.withError(BidderError.badServerResponse(e.getMessage())); } } - private static List extractBids(AdnuntiusResponse bidResponse) { + private static List extractBids(BidRequest bidRequest, AdnuntiusResponse bidResponse) { if (bidResponse == null || CollectionUtils.isEmpty(bidResponse.getAdsUnits())) { return Collections.emptyList(); } - final List validAdsUnits = bidResponse.getAdsUnits().stream() - .filter(AdnuntiusBidder::validateAdsUnit).collect(Collectors.toList()); + final List adsUnits = bidResponse.getAdsUnits(); + final List imps = bidRequest.getImp(); + if (adsUnits.size() > imps.size()) { + throw new PreBidException("Impressions count is less then ads units count."); + } + + final List validAdsUnitToImpId = IntStream.range(0, adsUnits.size()) + .mapToObj(i -> AdsUnitWithImpId.of(adsUnits.get(i), imps.get(i).getId())) + .filter(adsUnitWithImpId -> validateAdsUnit(adsUnitWithImpId.getAdsUnit())) + .collect(Collectors.toList()); - if (validAdsUnits.isEmpty()) { + if (validAdsUnitToImpId.isEmpty()) { return Collections.emptyList(); } - final String currency = extractCurrency(validAdsUnits); - return validAdsUnits.stream().map(adsUnit -> makeBid(adsUnit, currency)).collect(Collectors.toList()); + final String currency = extractCurrency(validAdsUnitToImpId); + return validAdsUnitToImpId.stream() + .map(adsUnitWithImpId -> makeBid(adsUnitWithImpId.getAdsUnit(), adsUnitWithImpId.getImpId(), currency)) + .collect(Collectors.toList()); } private static boolean validateAdsUnit(AdnuntiusAdsUnit adsUnit) { @@ -219,17 +230,17 @@ private static boolean validateAdsUnit(AdnuntiusAdsUnit adsUnit) { return CollectionUtils.isNotEmpty(ads) && ads.get(0) != null; } - private static String extractCurrency(List adsUnits) { - final AdnuntiusBid bid = adsUnits.get(adsUnits.size() - 1).getAds().get(0).getBid(); + private static String extractCurrency(List adsUnits) { + final AdnuntiusBid bid = adsUnits.get(adsUnits.size() - 1).getAdsUnit().getAds().get(0).getBid(); return ObjectUtil.getIfNotNull(bid, AdnuntiusBid::getCurrency); } - private static BidderBid makeBid(AdnuntiusAdsUnit adsUnit, String currency) { + private static BidderBid makeBid(AdnuntiusAdsUnit adsUnit, String impId, String currency) { final AdnuntiusAd ad = adsUnit.getAds().get(0); final String adId = ad.getAdId(); final Bid bid = Bid.builder() .id(adId) - .impid(extractImpId(adsUnit)) + .impid(impId) .w(parseMeasure(ad.getCreativeWidth())) .h(parseMeasure(ad.getCreativeHeight())) .adid(adId) @@ -243,15 +254,6 @@ private static BidderBid makeBid(AdnuntiusAdsUnit adsUnit, String currency) { return BidderBid.of(bid, BidType.banner, currency); } - private static String extractImpId(AdnuntiusAdsUnit adsUnit) { - final String targetId = adsUnit.getTargetId(); - final String auId = adsUnit.getAuId(); - - return ObjectUtils.allNotNull(targetId, auId) && targetId.startsWith(auId + TARGET_ID_DELIMITER) - ? targetId.substring(auId.length() + TARGET_ID_DELIMITER.length()) - : null; - } - private static Integer parseMeasure(String measure) { try { return Integer.valueOf(measure); diff --git a/src/main/java/org/prebid/server/bidder/adnuntius/model/util/AdsUnitWithImpId.java b/src/main/java/org/prebid/server/bidder/adnuntius/model/util/AdsUnitWithImpId.java new file mode 100644 index 00000000000..286573544a8 --- /dev/null +++ b/src/main/java/org/prebid/server/bidder/adnuntius/model/util/AdsUnitWithImpId.java @@ -0,0 +1,12 @@ +package org.prebid.server.bidder.adnuntius.model.util; + +import lombok.Value; +import org.prebid.server.bidder.adnuntius.model.response.AdnuntiusAdsUnit; + +@Value(staticConstructor = "of") +public class AdsUnitWithImpId { + + AdnuntiusAdsUnit adsUnit; + + String impId; +} diff --git a/src/test/java/org/prebid/server/bidder/adnuntius/AdnuntiusBidderTest.java b/src/test/java/org/prebid/server/bidder/adnuntius/AdnuntiusBidderTest.java index 854a4415a73..19830d65677 100644 --- a/src/test/java/org/prebid/server/bidder/adnuntius/AdnuntiusBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/adnuntius/AdnuntiusBidderTest.java @@ -73,7 +73,7 @@ public void makeHttpRequestsShouldReturnErrorWhenSomeImpBannerIsAbsent() { // then assertThat(result.getValue()).hasSize(0); assertThat(result.getErrors()).extracting(BidderError::getMessage) - .containsExactly("Fail on Imp.Id=null: Adnuntius supports only Banner"); + .containsExactly("Fail on Imp.Id=test: Adnuntius supports only Banner"); } @Test @@ -115,8 +115,9 @@ public void makeHttpRequestsShouldReturnRequestsWithAdUnitsSeparatedByImpExtNetw @Test public void makeHttpRequestsShouldReturnRequestsWithCorrectAdUnits() { // given - final BidRequest bidRequest = givenBidRequest(givenImp(identity()), - givenImp(ExtImpAdnuntius.of("auId", null), identity()), + final BidRequest bidRequest = givenBidRequest( + givenImp(imp -> imp.id(null)), + givenImp(ExtImpAdnuntius.of("auId", null), imp -> imp.id(null)), givenImp(ExtImpAdnuntius.of("auId", null), imp -> imp.id("impId"))); // when @@ -289,9 +290,10 @@ public void makeBidsShouldReturnEmptyListIfResponseAdsUnitsIsEmpty() throws Json public void makeBidsShouldSkipInvalidAdsUnits() throws JsonProcessingException { // given final HttpCall httpCall = givenHttpCall(givenAdsUnit()); + final BidRequest bidRequest = givenBidRequest(givenImp(identity())); // when - final Result> result = bidder.makeBids(httpCall, null); + final Result> result = bidder.makeBids(httpCall, bidRequest); // then assertThat(result.getValue()).isEmpty(); @@ -307,8 +309,10 @@ public void makeBidsShouldUseCurrencyOfFirstBidOfLastAdsUnit() throws JsonProces givenAdsUnit(givenAd(ad -> ad.bid(AdnuntiusBid.of(null, "2.1"))), givenAd(ad -> ad.bid(AdnuntiusBid.of(null, "2.2"))))); + final BidRequest bidRequest = givenBidRequest(givenImp(identity()), givenImp(identity())); + // when - final Result> result = bidder.makeBids(httpCall, null); + final Result> result = bidder.makeBids(httpCall, bidRequest); // then assertThat(result.getValue()).extracting(BidderBid::getBidCurrency) @@ -323,8 +327,10 @@ public void makeBidsShouldReturnErrorIfCreativeHeightOfSomeAdIsAbsent() throws J givenAdsUnit(givenAd(ad -> ad.bid(AdnuntiusBid.of(null, "CUR")))), givenAdsUnit(givenAd(ad -> ad.creativeHeight(null)))); + final BidRequest bidRequest = givenBidRequest(givenImp(identity()), givenImp(identity())); + // when - final Result> result = bidder.makeBids(httpCall, null); + final Result> result = bidder.makeBids(httpCall, bidRequest); // then assertThat(result.getValue()).isEmpty(); @@ -339,8 +345,10 @@ public void makeBidsShouldReturnErrorIfCreativeWidthtOfSomeAdIsAbsent() throws J givenAdsUnit(givenAd(ad -> ad.bid(AdnuntiusBid.of(null, "CUR")))), givenAdsUnit(givenAd(ad -> ad.creativeWidth(null)))); + final BidRequest bidRequest = givenBidRequest(givenImp(identity()), givenImp(identity())); + // when - final Result> result = bidder.makeBids(httpCall, null); + final Result> result = bidder.makeBids(httpCall, bidRequest); // then assertThat(result.getValue()).isEmpty(); @@ -359,8 +367,10 @@ public void makeBidsShouldReturnCorrectSeatBids() throws JsonProcessingException .destinationUrls(Map.of("key1", "https://www.domain1.com/uri", "key2", "http://www.domain2.dt/uri"))))); + final BidRequest bidRequest = givenBidRequest(givenImp(imp -> imp.id("impId"))); + // when - final Result> result = bidder.makeBids(httpCall, null); + final Result> result = bidder.makeBids(httpCall, bidRequest); // then assertThat(result.getValue()).hasSize(1).allSatisfy(bidderBid -> { @@ -383,6 +393,24 @@ public void makeBidsShouldReturnCorrectSeatBids() throws JsonProcessingException assertThat(result.getErrors()).isEmpty(); } + @Test + public void makeBidsShouldReturnErrorWhenAdsUnitsCountGreaterThanImpsCount() throws JsonProcessingException { + // given + final HttpCall httpCall = givenHttpCall( + givenAdsUnit(givenAd(identity())), + givenAdsUnit(givenAd(identity()))); + + final BidRequest bidRequest = givenBidRequest(givenImp(identity())); + + // when + final Result> result = bidder.makeBids(httpCall, bidRequest); + + // then + assertThat(result.getValue()).isEmpty(); + assertThat(result.getErrors()).extracting(BidderError::getMessage) + .containsExactly("Impressions count is less then ads units count."); + } + private BidRequest givenBidRequest(UnaryOperator bidRequestCustomizer, Imp... imps) { return bidRequestCustomizer.apply(BidRequest.builder()).imp(List.of(imps)).build(); } @@ -394,7 +422,7 @@ private BidRequest givenBidRequest(Imp... imps) { private Imp givenImp(ExtImpAdnuntius extImpAdnuntius, UnaryOperator impCustomizer) { final Banner banner = Banner.builder().build(); final ObjectNode ext = mapper.valueToTree(ExtPrebid.of(null, extImpAdnuntius)); - return impCustomizer.apply(Imp.builder().banner(banner).ext(ext)).build(); + return impCustomizer.apply(Imp.builder().id("test").banner(banner).ext(ext)).build(); } private Imp givenImp(UnaryOperator impCustomizer) {