From 573fa334c4fdec84df6dbb9034f7ca1ccfbfbfe6 Mon Sep 17 00:00:00 2001 From: markiian Date: Fri, 31 Dec 2021 15:57:48 +0200 Subject: [PATCH 1/5] Consolidate Unruly Adapter: Refactor --- .../server/bidder/unruly/UnrulyBidder.java | 30 ++++++----- .../bidder/unruly/proto/ImpExtUnruly.java | 12 ----- .../ext/request/unruly/ExtImpUnruly.java | 4 +- src/main/resources/bidder-config/unruly.yaml | 8 +-- .../static/bidder-params/unruly.json | 7 +-- .../bidder/unruly/UnrulyBidderTest.java | 52 ++++++++++++++++--- .../unruly/test-auction-unruly-request.json | 5 +- .../unruly/test-unruly-bid-request.json | 5 +- .../unruly/test-unruly-bid-response.json | 7 ++- 9 files changed, 79 insertions(+), 51 deletions(-) delete mode 100644 src/main/java/org/prebid/server/bidder/unruly/proto/ImpExtUnruly.java diff --git a/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java b/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java index face5d03bc4..76aba9efdff 100644 --- a/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java +++ b/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java @@ -1,6 +1,8 @@ package org.prebid.server.bidder.unruly; 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; import com.iab.openrtb.request.Imp; import com.iab.openrtb.response.BidResponse; @@ -14,7 +16,6 @@ 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.unruly.proto.ImpExtUnruly; import org.prebid.server.exception.PreBidException; import org.prebid.server.json.DecodeException; import org.prebid.server.json.JacksonMapper; @@ -59,7 +60,7 @@ public Result>> makeHttpRequests(BidRequest request } final List> outgoingRequests = modifiedImps.stream() - .map(imp -> createSingleRequest(imp, request, endpointUrl)) + .map(imp -> createSingleRequest(imp, request)) .collect(Collectors.toList()); return Result.of(outgoingRequests, errors); @@ -74,19 +75,12 @@ private ExtImpUnruly parseImpExt(Imp imp) { } private Imp modifyImp(Imp imp, ExtImpUnruly extImpUnruly) { - final Imp.ImpBuilder modifiedImp = imp.toBuilder(); - - try { - modifiedImp.ext(mapper.mapper().valueToTree(ImpExtUnruly.of(extImpUnruly))); - } catch (IllegalArgumentException e) { - throw new PreBidException(e.getMessage(), e); - } - - return modifiedImp.build(); + final ObjectNode modifiedExt = mapper.mapper().createObjectNode(); + modifiedExt.set("unruly", mapper.mapper().convertValue(extImpUnruly, JsonNode.class)); + return imp.toBuilder().ext(modifiedExt).build(); } - private HttpRequest createSingleRequest(Imp modifiedImp, BidRequest request, - String endpointUrl) { + private HttpRequest createSingleRequest(Imp modifiedImp, BidRequest request) { final BidRequest outgoingRequest = request.toBuilder().imp(Collections.singletonList(modifiedImp)).build(); return HttpRequest.builder() @@ -132,7 +126,15 @@ private static List bidsFromResponse(BidRequest bidRequest, BidRespon private static BidType getBidType(String impId, List imps) { for (Imp imp : imps) { if (imp.getId().equals(impId)) { - return BidType.video; + if (imp.getBanner() != null) { + return BidType.banner; + } else if (imp.getVideo() != null) { + return BidType.video; + } else { + return BidType.banner; + } + } else { + throw new PreBidException(String.format("Unknown impression type for ID: %s", impId)); } } throw new PreBidException(String.format("Failed to find impression %s", impId)); diff --git a/src/main/java/org/prebid/server/bidder/unruly/proto/ImpExtUnruly.java b/src/main/java/org/prebid/server/bidder/unruly/proto/ImpExtUnruly.java deleted file mode 100644 index c4e388c7145..00000000000 --- a/src/main/java/org/prebid/server/bidder/unruly/proto/ImpExtUnruly.java +++ /dev/null @@ -1,12 +0,0 @@ -package org.prebid.server.bidder.unruly.proto; - -import lombok.AllArgsConstructor; -import lombok.Value; -import org.prebid.server.proto.openrtb.ext.request.unruly.ExtImpUnruly; - -@Value -@AllArgsConstructor(staticName = "of") -public class ImpExtUnruly { - - ExtImpUnruly unruly; -} diff --git a/src/main/java/org/prebid/server/proto/openrtb/ext/request/unruly/ExtImpUnruly.java b/src/main/java/org/prebid/server/proto/openrtb/ext/request/unruly/ExtImpUnruly.java index 50c2d047781..b20b52250c7 100644 --- a/src/main/java/org/prebid/server/proto/openrtb/ext/request/unruly/ExtImpUnruly.java +++ b/src/main/java/org/prebid/server/proto/openrtb/ext/request/unruly/ExtImpUnruly.java @@ -11,8 +11,6 @@ @AllArgsConstructor(staticName = "of") public class ExtImpUnruly { - String uuid; - @JsonProperty("siteid") - String siteId; + Integer siteId; } diff --git a/src/main/resources/bidder-config/unruly.yaml b/src/main/resources/bidder-config/unruly.yaml index 17034bd23c8..815e45f4ecd 100644 --- a/src/main/resources/bidder-config/unruly.yaml +++ b/src/main/resources/bidder-config/unruly.yaml @@ -1,14 +1,16 @@ adapters: unruly: - endpoint: http://targeting.unrulymedia.com/openrtb/2.2 + endpoint: https://targeting.unrulymedia.com/unruly_prebid_server meta-info: - maintainer-email: adspaces@unrulygroup.com + maintainer-email: prebidsupport@unrulygroup.com app-media-types: + - banner - video site-media-types: + - banner - video supported-vendors: - vendor-id: 162 + vendor-id: 36 usersync: url: https://sync.1rx.io/usersync2/rmphb?gdpr={{gdpr}}&gdpr_consent={{gdpr_consent}}&us_privacy={{us_privacy}}&redir= redirect-url: /setuid?bidder=unruly&gdpr={{gdpr}}&gdpr_consent={{gdpr_consent}}&us_privacy={{us_privacy}}&uid=[RX_UUID] diff --git a/src/main/resources/static/bidder-params/unruly.json b/src/main/resources/static/bidder-params/unruly.json index b4d630a1c52..42a76c3f7f3 100644 --- a/src/main/resources/static/bidder-params/unruly.json +++ b/src/main/resources/static/bidder-params/unruly.json @@ -4,17 +4,12 @@ "description": "A schema which validates params accepted by the Unruly adapter", "type": "object", "properties": { - "uuid": { - "type": "string", - "description": "uuid" - }, "siteid": { - "type": "string", + "type": "integer", "description": "ID for publisher site" } }, "required": [ - "uuid", "siteid" ] } diff --git a/src/test/java/org/prebid/server/bidder/unruly/UnrulyBidderTest.java b/src/test/java/org/prebid/server/bidder/unruly/UnrulyBidderTest.java index 190fac300b9..4322a68238c 100644 --- a/src/test/java/org/prebid/server/bidder/unruly/UnrulyBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/unruly/UnrulyBidderTest.java @@ -1,6 +1,7 @@ package org.prebid.server.bidder.unruly; import com.fasterxml.jackson.core.JsonProcessingException; +import com.iab.openrtb.request.Banner; import com.iab.openrtb.request.BidRequest; import com.iab.openrtb.request.Imp; import com.iab.openrtb.request.Video; @@ -29,6 +30,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.tuple; +import static org.prebid.server.proto.openrtb.ext.response.BidType.banner; import static org.prebid.server.proto.openrtb.ext.response.BidType.video; public class UnrulyBidderTest extends VertxTest { @@ -128,6 +130,25 @@ public void makeBidsShouldReturnEmptyListIfBidResponseSeatBidIsNull() throws Jso @Test public void makeBidsShouldReturnVideoBid() throws JsonProcessingException { + // given + 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 = unrulyBidder.makeBids(httpCall, null); + + // then + assertThat(result.getErrors()).isEmpty(); + assertThat(result.getValue()) + .containsExactly(BidderBid.of(Bid.builder().impid("123").build(), video, "USD")); + } + + @Test + public void makeBidsShouldReturnDefaultBannerBid() throws JsonProcessingException { // given final HttpCall httpCall = givenHttpCall( BidRequest.builder() @@ -142,7 +163,26 @@ public void makeBidsShouldReturnVideoBid() throws JsonProcessingException { // 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(), banner, "USD")); + } + + @Test + public void makeBidsShouldReturnBannerBid() throws JsonProcessingException { + // given + final HttpCall httpCall = givenHttpCall( + BidRequest.builder() + .imp(singletonList(Imp.builder().banner(Banner.builder().build()).id("123").build())) + .build(), + mapper.writeValueAsString( + givenBidResponse(bidBuilder -> bidBuilder.impid("123")))); + + // when + final Result> result = unrulyBidder.makeBids(httpCall, null); + + // then + assertThat(result.getErrors()).isEmpty(); + assertThat(result.getValue()) + .containsExactly(BidderBid.of(Bid.builder().impid("123").build(), banner, "USD")); } @Test @@ -161,7 +201,7 @@ public void makeBidsShouldReturnErrorIfImpressionWasNotFound() throws JsonProces // then assertThat(result.getValue()).isEmpty(); assertThat(result.getErrors()).hasSize(1) - .containsOnly(BidderError.badServerResponse("Failed to find impression 123")); + .containsExactly(BidderError.badServerResponse("Failed to find impression 123")); } private static BidRequest givenBidRequest( @@ -169,7 +209,7 @@ private static BidRequest givenBidRequest( Function impCustomizer) { return bidRequestCustomizer.apply(BidRequest.builder() - .imp(singletonList(givenImp(impCustomizer)))) + .imp(singletonList(givenImp(impCustomizer)))) .build(); } @@ -179,9 +219,9 @@ private static BidRequest givenBidRequest(Function impCustomizer) { return impCustomizer.apply(Imp.builder() - .id("123") - .video(Video.builder().build()) - .ext(mapper.valueToTree(ExtPrebid.of(null, ExtImpUnruly.of("uuid", "site_id"))))) + .id("123") + .video(Video.builder().build()) + .ext(mapper.valueToTree(ExtPrebid.of(null, ExtImpUnruly.of(123))))) .build(); } diff --git a/src/test/resources/org/prebid/server/it/openrtb2/unruly/test-auction-unruly-request.json b/src/test/resources/org/prebid/server/it/openrtb2/unruly/test-auction-unruly-request.json index 66c0070a37a..9842f5584fc 100644 --- a/src/test/resources/org/prebid/server/it/openrtb2/unruly/test-auction-unruly-request.json +++ b/src/test/resources/org/prebid/server/it/openrtb2/unruly/test-auction-unruly-request.json @@ -12,8 +12,7 @@ }, "ext": { "unruly": { - "uuid": "uu_id_1", - "siteid": "site_id_1" + "siteid": 123 } } } @@ -24,4 +23,4 @@ "gdpr": 0 } } -} \ No newline at end of file +} diff --git a/src/test/resources/org/prebid/server/it/openrtb2/unruly/test-unruly-bid-request.json b/src/test/resources/org/prebid/server/it/openrtb2/unruly/test-unruly-bid-request.json index f334cef507c..3bf8946422e 100644 --- a/src/test/resources/org/prebid/server/it/openrtb2/unruly/test-unruly-bid-request.json +++ b/src/test/resources/org/prebid/server/it/openrtb2/unruly/test-unruly-bid-request.json @@ -12,8 +12,7 @@ }, "ext": { "unruly": { - "uuid": "uu_id_1", - "siteid": "site_id_1" + "siteid": 123 } } } @@ -42,4 +41,4 @@ "gdpr": 0 } } -} \ No newline at end of file +} diff --git a/src/test/resources/org/prebid/server/it/openrtb2/unruly/test-unruly-bid-response.json b/src/test/resources/org/prebid/server/it/openrtb2/unruly/test-unruly-bid-response.json index 6c59f41f84e..c43a8ca5bac 100644 --- a/src/test/resources/org/prebid/server/it/openrtb2/unruly/test-unruly-bid-response.json +++ b/src/test/resources/org/prebid/server/it/openrtb2/unruly/test-unruly-bid-response.json @@ -9,6 +9,11 @@ "price": 1.25, "crid": "crid", "adm": "adm001", + "ext": { + "prebid": { + "type": "video" + } + }, "h": 600, "w": 800 } @@ -16,4 +21,4 @@ } ], "bidid": "bid001" -} \ No newline at end of file +} From 522044c4e5eeb0d1099f94c223c29e3849005a2a Mon Sep 17 00:00:00 2001 From: markiian Date: Wed, 12 Jan 2022 15:46:59 +0200 Subject: [PATCH 2/5] Consolidate Unruly Adapter: Refactor --- .../server/bidder/unruly/UnrulyBidder.java | 53 +++++++++++++------ .../bidder/unruly/UnrulyBidderTest.java | 26 +++++---- .../unruly/test-unruly-bid-request.json | 2 +- 3 files changed, 53 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java b/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java index 76aba9efdff..383a5ae11a3 100644 --- a/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java +++ b/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java @@ -5,9 +5,9 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import com.iab.openrtb.request.BidRequest; 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 io.vertx.core.MultiMap; import io.vertx.core.http.HttpMethod; import org.apache.commons.collections4.CollectionUtils; import org.prebid.server.bidder.Bidder; @@ -56,6 +56,7 @@ public Result>> makeHttpRequests(BidRequest request modifiedImps.add(modifyImp(imp, extImpUnruly)); } catch (PreBidException e) { errors.add(BidderError.badInput(e.getMessage())); + return Result.withErrors(errors); } } @@ -70,13 +71,14 @@ private ExtImpUnruly parseImpExt(Imp imp) { try { return mapper.mapper().convertValue(imp.getExt(), UNRULY_EXT_TYPE_REFERENCE).getBidder(); } catch (IllegalArgumentException e) { - throw new PreBidException(e.getMessage(), e); + throw new PreBidException(String.format("ext data not provided in imp id=%s. Abort all Request", + imp.getId())); } } private Imp modifyImp(Imp imp, ExtImpUnruly extImpUnruly) { final ObjectNode modifiedExt = mapper.mapper().createObjectNode(); - modifiedExt.set("unruly", mapper.mapper().convertValue(extImpUnruly, JsonNode.class)); + modifiedExt.set("bidder", mapper.mapper().convertValue(extImpUnruly, JsonNode.class)); return imp.toBuilder().ext(modifiedExt).build(); } @@ -86,44 +88,58 @@ private HttpRequest createSingleRequest(Imp modifiedImp, BidRequest return HttpRequest.builder() .method(HttpMethod.POST) .uri(endpointUrl) - .headers(getHeaders()) + .headers(HttpUtil.headers()) .body(mapper.encodeToBytes(outgoingRequest)) .payload(outgoingRequest) .build(); } - private static MultiMap getHeaders() { - return HttpUtil.headers() - .add("X-Unruly-Origin", "Prebid-Server"); - } - @Override public Result> makeBids(HttpCall httpCall, BidRequest bidRequest) { + final List errors = new ArrayList<>(); try { final BidResponse bidResponse = mapper.decodeValue(httpCall.getResponse().getBody(), BidResponse.class); - return Result.withValues(extractBids(httpCall.getRequest().getPayload(), bidResponse)); + return Result.of(extractBids(httpCall.getRequest().getPayload(), 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(BidRequest bidRequest, BidResponse bidResponse, + List errors) { return bidResponse == null || CollectionUtils.isEmpty(bidResponse.getSeatbid()) ? Collections.emptyList() - : bidsFromResponse(bidRequest, bidResponse); + : bidsFromResponse(bidRequest, bidResponse, errors); } - private static List bidsFromResponse(BidRequest bidRequest, BidResponse bidResponse) { - return bidResponse.getSeatbid().stream() + private static List bidsFromResponse(BidRequest bidRequest, 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, getBidType(bid.getImpid(), bidRequest.getImp()), bidResponse.getCur())) + .filter(Objects::nonNull) + .map(bid -> resolveBidderBid(bid, bidResponse.getCur(), bidRequest.getImp(), errors)) + .filter(Objects::nonNull) .collect(Collectors.toList()); } + private static BidderBid resolveBidderBid(Bid bid, String currency, List imps, List errors) { + final BidType bidType; + try { + bidType = getBidType(bid.getImpid(), imps); + } catch (PreBidException e) { + errors.add(BidderError.badServerResponse(e.getMessage())); + return null; + } + return BidderBid.of(bid, bidType, currency); + } + private static BidType getBidType(String impId, List imps) { + final List noMatchingImps = new ArrayList<>(); + for (Imp imp : imps) { if (imp.getId().equals(impId)) { if (imp.getBanner() != null) { @@ -134,9 +150,12 @@ private static BidType getBidType(String impId, List imps) { return BidType.banner; } } else { - throw new PreBidException(String.format("Unknown impression type for ID: %s", impId)); + noMatchingImps.add(imp.getId()); } } - throw new PreBidException(String.format("Failed to find impression %s", impId)); + + throw new PreBidException(String + .format("Bid response imp ID %s not found in bid request containing imps %s", + impId, noMatchingImps)); } } diff --git a/src/test/java/org/prebid/server/bidder/unruly/UnrulyBidderTest.java b/src/test/java/org/prebid/server/bidder/unruly/UnrulyBidderTest.java index 4322a68238c..66fb8e52342 100644 --- a/src/test/java/org/prebid/server/bidder/unruly/UnrulyBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/unruly/UnrulyBidderTest.java @@ -30,6 +30,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.tuple; +import static org.prebid.server.bidder.model.BidderError.Type.bad_input; +import static org.prebid.server.bidder.model.BidderError.Type.bad_server_response; import static org.prebid.server.proto.openrtb.ext.response.BidType.banner; import static org.prebid.server.proto.openrtb.ext.response.BidType.video; @@ -60,9 +62,11 @@ public void makeHttpRequestsShouldReturnErrorIfImpExtCouldNotBeParsed() { final Result>> result = unrulyBidder.makeHttpRequests(bidRequest); // then - assertThat(result.getErrors()).hasSize(1); - assertThat(result.getErrors().get(0).getMessage()).startsWith("Cannot deserialize value"); - assertThat(result.getValue()).isEmpty(); + assertThat(result.getErrors()).hasSize(1) + .allSatisfy(error -> { + assertThat(error.getMessage()).startsWith("ext data not provided in imp id=123"); + assertThat(error.getType()).isEqualTo(bad_input); + }); } @Test @@ -81,8 +85,7 @@ public void makeHttpRequestsShouldReturnOneRequestPerImpWithExpectedHeaders() { .extracting(Map.Entry::getKey, Map.Entry::getValue) .containsOnly( tuple("Content-Type", "application/json;charset=utf-8"), - tuple("Accept", "application/json"), - tuple("X-Unruly-Origin", "Prebid-Server")); + tuple("Accept", "application/json")); } @Test @@ -186,22 +189,25 @@ public void makeBidsShouldReturnBannerBid() throws JsonProcessingException { } @Test - public void makeBidsShouldReturnErrorIfImpressionWasNotFound() throws JsonProcessingException { + public void makeBidsShouldReturnErrorNotFoundBidRequest() throws JsonProcessingException { // given final HttpCall httpCall = givenHttpCall( BidRequest.builder() - .imp(singletonList(Imp.builder().id("321").build())) + .imp(singletonList(Imp.builder().id("112").build())) .build(), mapper.writeValueAsString( - givenBidResponse(bidBuilder -> bidBuilder.impid("123")))); + givenBidResponse(bidBuilder -> bidBuilder.impid("111")))); // when final Result> result = unrulyBidder.makeBids(httpCall, null); // then assertThat(result.getValue()).isEmpty(); - assertThat(result.getErrors()).hasSize(1) - .containsExactly(BidderError.badServerResponse("Failed to find impression 123")); + assertThat(result.getErrors()) + .allSatisfy(error -> { + assertThat(error.getMessage()).startsWith("Bid response imp ID 111 not found"); + assertThat(error.getType()).isEqualTo(bad_server_response); + }); } private static BidRequest givenBidRequest( diff --git a/src/test/resources/org/prebid/server/it/openrtb2/unruly/test-unruly-bid-request.json b/src/test/resources/org/prebid/server/it/openrtb2/unruly/test-unruly-bid-request.json index 3bf8946422e..6f70d15577d 100644 --- a/src/test/resources/org/prebid/server/it/openrtb2/unruly/test-unruly-bid-request.json +++ b/src/test/resources/org/prebid/server/it/openrtb2/unruly/test-unruly-bid-request.json @@ -11,7 +11,7 @@ "h": 600 }, "ext": { - "unruly": { + "bidder": { "siteid": 123 } } From 6e4792243ce3be1d868b1be7da4c7a4bd1bb9427 Mon Sep 17 00:00:00 2001 From: Alex Maltsev Date: Fri, 11 Feb 2022 19:17:31 +0200 Subject: [PATCH 3/5] Refactoring --- .../server/bidder/unruly/UnrulyBidder.java | 62 +++++-------------- .../ext/request/unruly/ExtImpUnruly.java | 16 ----- .../bidder/unruly/UnrulyBidderTest.java | 44 +++++-------- 3 files changed, 32 insertions(+), 90 deletions(-) delete mode 100644 src/main/java/org/prebid/server/proto/openrtb/ext/request/unruly/ExtImpUnruly.java diff --git a/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java b/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java index 383a5ae11a3..90177042c64 100644 --- a/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java +++ b/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java @@ -1,7 +1,5 @@ package org.prebid.server.bidder.unruly; -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; import com.iab.openrtb.request.Imp; @@ -19,8 +17,6 @@ import org.prebid.server.exception.PreBidException; import org.prebid.server.json.DecodeException; import org.prebid.server.json.JacksonMapper; -import org.prebid.server.proto.openrtb.ext.ExtPrebid; -import org.prebid.server.proto.openrtb.ext.request.unruly.ExtImpUnruly; import org.prebid.server.proto.openrtb.ext.response.BidType; import org.prebid.server.util.HttpUtil; @@ -33,10 +29,6 @@ public class UnrulyBidder implements Bidder { - private static final TypeReference> UNRULY_EXT_TYPE_REFERENCE = - new TypeReference<>() { - }; - private final String endpointUrl; private final JacksonMapper mapper; @@ -47,43 +39,23 @@ public UnrulyBidder(String endpointUrl, JacksonMapper mapper) { @Override public Result>> makeHttpRequests(BidRequest request) { - final List errors = new ArrayList<>(); - - final List modifiedImps = new ArrayList<>(); - for (Imp imp : request.getImp()) { - try { - final ExtImpUnruly extImpUnruly = parseImpExt(imp); - modifiedImps.add(modifyImp(imp, extImpUnruly)); - } catch (PreBidException e) { - errors.add(BidderError.badInput(e.getMessage())); - return Result.withErrors(errors); - } - } - - final List> outgoingRequests = modifiedImps.stream() + final List> httpRequests = request.getImp().stream() + .map(this::modifyImp) .map(imp -> createSingleRequest(imp, request)) .collect(Collectors.toList()); - return Result.of(outgoingRequests, errors); + return Result.withValues(httpRequests); } - private ExtImpUnruly parseImpExt(Imp imp) { - try { - return mapper.mapper().convertValue(imp.getExt(), UNRULY_EXT_TYPE_REFERENCE).getBidder(); - } catch (IllegalArgumentException e) { - throw new PreBidException(String.format("ext data not provided in imp id=%s. Abort all Request", - imp.getId())); - } - } + private Imp modifyImp(Imp imp) { + final ObjectNode modifiedExt = mapper.mapper().createObjectNode() + .set("bidder", imp.getExt().get("bidder")); - private Imp modifyImp(Imp imp, ExtImpUnruly extImpUnruly) { - final ObjectNode modifiedExt = mapper.mapper().createObjectNode(); - modifiedExt.set("bidder", mapper.mapper().convertValue(extImpUnruly, JsonNode.class)); return imp.toBuilder().ext(modifiedExt).build(); } - private HttpRequest createSingleRequest(Imp modifiedImp, BidRequest request) { - final BidRequest outgoingRequest = request.toBuilder().imp(Collections.singletonList(modifiedImp)).build(); + private HttpRequest createSingleRequest(Imp imp, BidRequest request) { + final BidRequest outgoingRequest = request.toBuilder().imp(Collections.singletonList(imp)).build(); return HttpRequest.builder() .method(HttpMethod.POST) @@ -127,18 +99,16 @@ private static List bidsFromResponse(BidRequest bidRequest, BidRespon } private static BidderBid resolveBidderBid(Bid bid, String currency, List imps, List errors) { - final BidType bidType; try { - bidType = getBidType(bid.getImpid(), imps); + return BidderBid.of(bid, getBidType(bid.getImpid(), imps), currency); } catch (PreBidException e) { errors.add(BidderError.badServerResponse(e.getMessage())); - return null; + return BidderBid.of(bid, BidType.banner, currency); } - return BidderBid.of(bid, bidType, currency); } private static BidType getBidType(String impId, List imps) { - final List noMatchingImps = new ArrayList<>(); + final List unmatchedImpIds = new ArrayList<>(); for (Imp imp : imps) { if (imp.getId().equals(impId)) { @@ -146,16 +116,14 @@ private static BidType getBidType(String impId, List imps) { return BidType.banner; } else if (imp.getVideo() != null) { return BidType.video; - } else { - return BidType.banner; } + throw new PreBidException("bid responses mediaType didn't match supported mediaTypes"); } else { - noMatchingImps.add(imp.getId()); + unmatchedImpIds.add(imp.getId()); } } - throw new PreBidException(String - .format("Bid response imp ID %s not found in bid request containing imps %s", - impId, noMatchingImps)); + throw new PreBidException( + "Bid response imp ID " + impId + " not found in bid request containing imps" + unmatchedImpIds); } } diff --git a/src/main/java/org/prebid/server/proto/openrtb/ext/request/unruly/ExtImpUnruly.java b/src/main/java/org/prebid/server/proto/openrtb/ext/request/unruly/ExtImpUnruly.java deleted file mode 100644 index b20b52250c7..00000000000 --- a/src/main/java/org/prebid/server/proto/openrtb/ext/request/unruly/ExtImpUnruly.java +++ /dev/null @@ -1,16 +0,0 @@ -package org.prebid.server.proto.openrtb.ext.request.unruly; - -import com.fasterxml.jackson.annotation.JsonProperty; -import lombok.AllArgsConstructor; -import lombok.Value; - -/** - * Defines the contract for bidrequest.imp[i].ext.unruly - */ -@Value -@AllArgsConstructor(staticName = "of") -public class ExtImpUnruly { - - @JsonProperty("siteid") - Integer siteId; -} diff --git a/src/test/java/org/prebid/server/bidder/unruly/UnrulyBidderTest.java b/src/test/java/org/prebid/server/bidder/unruly/UnrulyBidderTest.java index 66fb8e52342..6b872b9eff3 100644 --- a/src/test/java/org/prebid/server/bidder/unruly/UnrulyBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/unruly/UnrulyBidderTest.java @@ -1,6 +1,8 @@ package org.prebid.server.bidder.unruly; import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.node.IntNode; +import com.fasterxml.jackson.databind.node.ObjectNode; import com.iab.openrtb.request.Banner; import com.iab.openrtb.request.BidRequest; import com.iab.openrtb.request.Imp; @@ -18,7 +20,6 @@ import org.prebid.server.bidder.model.HttpResponse; import org.prebid.server.bidder.model.Result; import org.prebid.server.proto.openrtb.ext.ExtPrebid; -import org.prebid.server.proto.openrtb.ext.request.unruly.ExtImpUnruly; import java.util.List; import java.util.Map; @@ -30,7 +31,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.tuple; -import static org.prebid.server.bidder.model.BidderError.Type.bad_input; import static org.prebid.server.bidder.model.BidderError.Type.bad_server_response; import static org.prebid.server.proto.openrtb.ext.response.BidType.banner; import static org.prebid.server.proto.openrtb.ext.response.BidType.video; @@ -51,24 +51,6 @@ public void creationShouldFailOnInvalidEndpointUrl() { assertThatIllegalArgumentException().isThrownBy(() -> new UnrulyBidder("invalid_url", jacksonMapper)); } - @Test - public void makeHttpRequestsShouldReturnErrorIfImpExtCouldNotBeParsed() { - // given - final BidRequest bidRequest = givenBidRequest( - impBuilder -> impBuilder - .ext(mapper.valueToTree(ExtPrebid.of(null, mapper.createArrayNode())))); - - // when - final Result>> result = unrulyBidder.makeHttpRequests(bidRequest); - - // then - assertThat(result.getErrors()).hasSize(1) - .allSatisfy(error -> { - assertThat(error.getMessage()).startsWith("ext data not provided in imp id=123"); - assertThat(error.getType()).isEqualTo(bad_input); - }); - } - @Test public void makeHttpRequestsShouldReturnOneRequestPerImpWithExpectedHeaders() { // given @@ -164,7 +146,9 @@ public void makeBidsShouldReturnDefaultBannerBid() throws JsonProcessingExceptio final Result> result = unrulyBidder.makeBids(httpCall, null); // then - assertThat(result.getErrors()).isEmpty(); + assertThat(result.getErrors()).containsExactly( + BidderError.badServerResponse("bid responses mediaType didn't match supported mediaTypes")); + assertThat(result.getValue()) .containsExactly(BidderBid.of(Bid.builder().impid("123").build(), banner, "USD")); } @@ -202,12 +186,14 @@ public void makeBidsShouldReturnErrorNotFoundBidRequest() throws JsonProcessingE final Result> result = unrulyBidder.makeBids(httpCall, null); // then - assertThat(result.getValue()).isEmpty(); - assertThat(result.getErrors()) + assertThat(result.getErrors()).hasSize(1) .allSatisfy(error -> { assertThat(error.getMessage()).startsWith("Bid response imp ID 111 not found"); assertThat(error.getType()).isEqualTo(bad_server_response); }); + + assertThat(result.getValue()) + .containsExactly(BidderBid.of(Bid.builder().impid("111").build(), banner, "USD")); } private static BidRequest givenBidRequest( @@ -224,10 +210,14 @@ private static BidRequest givenBidRequest(Function impCustomizer) { - return impCustomizer.apply(Imp.builder() - .id("123") - .video(Video.builder().build()) - .ext(mapper.valueToTree(ExtPrebid.of(null, ExtImpUnruly.of(123))))) + final ObjectNode impExt = mapper.valueToTree( + ExtPrebid.of(null, mapper.createObjectNode().set("siteId", IntNode.valueOf(123)))); + + return impCustomizer.apply( + Imp.builder() + .id("123") + .video(Video.builder().build()) + .ext(impExt)) .build(); } From f21c2ce1961e81b11b057c7c58cb613bf199defd Mon Sep 17 00:00:00 2001 From: Alex Maltsev Date: Fri, 11 Feb 2022 19:23:31 +0200 Subject: [PATCH 4/5] Refactoring --- .../java/org/prebid/server/bidder/unruly/UnrulyBidder.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java b/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java index 90177042c64..9b7ceaf3fcf 100644 --- a/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java +++ b/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java @@ -79,6 +79,7 @@ public Result> makeBids(HttpCall httpCall, BidReques private static List extractBids(BidRequest bidRequest, BidResponse bidResponse, List errors) { + return bidResponse == null || CollectionUtils.isEmpty(bidResponse.getSeatbid()) ? Collections.emptyList() : bidsFromResponse(bidRequest, bidResponse, errors); @@ -86,15 +87,14 @@ private static List extractBids(BidRequest bidRequest, BidResponse bi private static List bidsFromResponse(BidRequest bidRequest, BidResponse bidResponse, List errors) { - return bidResponse.getSeatbid() - .stream() + + return bidResponse.getSeatbid().stream() .filter(Objects::nonNull) .map(SeatBid::getBid) .filter(Objects::nonNull) .flatMap(Collection::stream) .filter(Objects::nonNull) .map(bid -> resolveBidderBid(bid, bidResponse.getCur(), bidRequest.getImp(), errors)) - .filter(Objects::nonNull) .collect(Collectors.toList()); } From e8168573438adb1d0b2df83759b3edd0f3cc30ea Mon Sep 17 00:00:00 2001 From: Alex Maltsev Date: Fri, 11 Feb 2022 19:28:13 +0200 Subject: [PATCH 5/5] Refactoring --- .../java/org/prebid/server/bidder/unruly/UnrulyBidder.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java b/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java index 9b7ceaf3fcf..2ca51aaa847 100644 --- a/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java +++ b/src/main/java/org/prebid/server/bidder/unruly/UnrulyBidder.java @@ -77,7 +77,8 @@ public Result> makeBids(HttpCall httpCall, BidReques } } - private static List extractBids(BidRequest bidRequest, BidResponse bidResponse, + private static List extractBids(BidRequest bidRequest, + BidResponse bidResponse, List errors) { return bidResponse == null || CollectionUtils.isEmpty(bidResponse.getSeatbid()) @@ -85,7 +86,8 @@ private static List extractBids(BidRequest bidRequest, BidResponse bi : bidsFromResponse(bidRequest, bidResponse, errors); } - private static List bidsFromResponse(BidRequest bidRequest, BidResponse bidResponse, + private static List bidsFromResponse(BidRequest bidRequest, + BidResponse bidResponse, List errors) { return bidResponse.getSeatbid().stream()