From d468a9320e0e1a043beb9530366abdc059b86bec Mon Sep 17 00:00:00 2001 From: Yevhenii Viktorov Date: Mon, 29 Nov 2021 14:56:35 +0200 Subject: [PATCH 01/15] First iteration of changes --- .../server/bidder/adview/AdviewBidder.java | 36 ++++++++++++++++++- .../config/bidder/AdviewConfiguration.java | 6 ++-- 2 files changed, 39 insertions(+), 3 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 5de8002241e..5995786af37 100644 --- a/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java +++ b/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java @@ -9,19 +9,24 @@ import com.iab.openrtb.response.SeatBid; import io.vertx.core.http.HttpMethod; import org.apache.commons.collections4.CollectionUtils; +import org.apache.commons.lang3.StringUtils; import org.prebid.server.bidder.Bidder; import org.prebid.server.bidder.model.BidderBid; import org.prebid.server.bidder.model.BidderError; 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.currency.CurrencyConversionService; +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.adview.ExtImpAdview; import org.prebid.server.proto.openrtb.ext.response.BidType; +import org.prebid.server.util.BidderUtil; import org.prebid.server.util.HttpUtil; +import java.math.BigDecimal; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -35,13 +40,18 @@ public class AdviewBidder implements Bidder { new TypeReference>() { }; private static final String ACCOUNT_ID_MACRO = "{{AccountId}}"; + private static final String BIDDER_CURRENCY = "USD"; private final String endpointUrl; private final JacksonMapper mapper; + private final CurrencyConversionService currencyConversionService; - public AdviewBidder(String endpointUrl, JacksonMapper mapper) { + public AdviewBidder(String endpointUrl, + JacksonMapper mapper, + CurrencyConversionService currencyConversionService) { this.endpointUrl = HttpUtil.validateUrl(Objects.requireNonNull(endpointUrl)); this.mapper = Objects.requireNonNull(mapper); + this.currencyConversionService = Objects.requireNonNull(currencyConversionService); } @Override @@ -66,6 +76,30 @@ public Result>> makeHttpRequests(BidRequest request .build()); } + private BigDecimal resolveBidFloor(BidRequest request, Imp imp) { + return shouldConvertBidFloor(imp.getBidfloor(), imp.getBidfloorcur()) + ? convertBidFloorCurrency(imp.getBidfloor(), request, imp.getId(), imp.getBidfloorcur()) + : imp.getBidfloor(); + } + + private static boolean shouldConvertBidFloor(BigDecimal bidFloor, String bidFloorCur) { + return BidderUtil.isValidPrice(bidFloor) && !StringUtils.equalsIgnoreCase(bidFloorCur, BIDDER_CURRENCY); + } + + private BigDecimal convertBidFloorCurrency(BigDecimal bidFloor, + BidRequest bidRequest, + String impId, + String bidFloorCur) { + try { + return currencyConversionService + .convertCurrency(bidFloor, bidRequest, bidFloorCur, BIDDER_CURRENCY); + } catch (PreBidException e) { + throw new PreBidException(String.format( + "Unable to convert provided bid floor currency from %s to %s for imp `%s`", + bidFloorCur, BIDDER_CURRENCY, impId)); + } + } + private static BidRequest modifyRequest(BidRequest bidRequest, String masterTagId) { return bidRequest.toBuilder() .imp(modifyImps(bidRequest.getImp(), masterTagId)) diff --git a/src/main/java/org/prebid/server/spring/config/bidder/AdviewConfiguration.java b/src/main/java/org/prebid/server/spring/config/bidder/AdviewConfiguration.java index 2dbbd154b63..486ba414424 100644 --- a/src/main/java/org/prebid/server/spring/config/bidder/AdviewConfiguration.java +++ b/src/main/java/org/prebid/server/spring/config/bidder/AdviewConfiguration.java @@ -2,6 +2,7 @@ import org.prebid.server.bidder.BidderDeps; import org.prebid.server.bidder.adview.AdviewBidder; +import org.prebid.server.currency.CurrencyConversionService; import org.prebid.server.json.JacksonMapper; import org.prebid.server.spring.config.bidder.model.BidderConfigurationProperties; import org.prebid.server.spring.config.bidder.util.BidderDepsAssembler; @@ -30,12 +31,13 @@ BidderConfigurationProperties configurationProperties() { @Bean BidderDeps adviewBidderDeps(BidderConfigurationProperties adviewConfigurationProperties, @NotBlank @Value("${external-url}") String externalUrl, - JacksonMapper mapper) { + JacksonMapper mapper, + CurrencyConversionService currencyConversionService) { return BidderDepsAssembler.forBidder(BIDDER_NAME) .withConfig(adviewConfigurationProperties) .usersyncerCreator(UsersyncerCreator.create(externalUrl)) - .bidderCreator(config -> new AdviewBidder(config.getEndpoint(), mapper)) + .bidderCreator(config -> new AdviewBidder(config.getEndpoint(), mapper, currencyConversionService)) .assemble(); } } From ba28422a984c4f231390c2865788b777c1ae675d Mon Sep 17 00:00:00 2001 From: Yevhenii Viktorov Date: Tue, 30 Nov 2021 14:42:02 +0200 Subject: [PATCH 02/15] add logic, several tests, refactor code in bidder, fix IT --- .../server/bidder/adview/AdviewBidder.java | 33 ++++-- .../bidder/adview/AdviewBidderTest.java | 104 +++++++++++++++--- .../adview/test-adview-bid-request.json | 1 + 3 files changed, 109 insertions(+), 29 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 5995786af37..78935cedf97 100644 --- a/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java +++ b/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java @@ -60,22 +60,30 @@ public Result>> makeHttpRequests(BidRequest request final ExtImpAdview extImpAdview; try { - extImpAdview = mapper.mapper().convertValue(firstImp.getExt(), ADVIEW_EXT_TYPE_REFERENCE).getBidder(); - } catch (IllegalArgumentException e) { - return Result.withError(BidderError.badInput("invalid imp.ext")); + extImpAdview = parseExtImp(firstImp); + request = modifyRequest(request, extImpAdview.getMasterTagId(), resolveBidFloor(request, firstImp)); + } catch (PreBidException e) { + return Result.withError(BidderError.badInput(e.getMessage())); } - final BidRequest modifiedRequest = modifyRequest(request, extImpAdview.getMasterTagId()); return Result.withValue( HttpRequest.builder() .method(HttpMethod.POST) .uri(resolveEndpoint(extImpAdview.getAccountId())) .headers(HttpUtil.headers()) - .body(mapper.encodeToBytes(modifiedRequest)) - .payload(modifiedRequest) + .body(mapper.encodeToBytes(request)) + .payload(request) .build()); } + private ExtImpAdview parseExtImp(Imp imp) { + try { + return mapper.mapper().convertValue(imp.getExt(), ADVIEW_EXT_TYPE_REFERENCE).getBidder(); + } catch (IllegalArgumentException e) { + throw new PreBidException("invalid imp.ext"); + } + } + private BigDecimal resolveBidFloor(BidRequest request, Imp imp) { return shouldConvertBidFloor(imp.getBidfloor(), imp.getBidfloorcur()) ? convertBidFloorCurrency(imp.getBidfloor(), request, imp.getId(), imp.getBidfloorcur()) @@ -100,21 +108,24 @@ private BigDecimal convertBidFloorCurrency(BigDecimal bidFloor, } } - private static BidRequest modifyRequest(BidRequest bidRequest, String masterTagId) { + private static BidRequest modifyRequest(BidRequest bidRequest, String masterTagId, BigDecimal resolvedBidFloor) { return bidRequest.toBuilder() - .imp(modifyImps(bidRequest.getImp(), masterTagId)) + .imp(modifyImps(bidRequest.getImp(), masterTagId, resolvedBidFloor)) + .cur(Collections.singletonList("USD")) .build(); } - private static List modifyImps(List imps, String masterTagId) { + private static List modifyImps(List imps, String masterTagId, BigDecimal resolvedBidFloor) { final List modifiedImps = new ArrayList<>(imps); - modifiedImps.set(0, modifyImp(imps.get(0), masterTagId)); + modifiedImps.set(0, modifyImp(imps.get(0), masterTagId, resolvedBidFloor)); return modifiedImps; } - private static Imp modifyImp(Imp imp, String masterTagId) { + private static Imp modifyImp(Imp imp, String masterTagId, BigDecimal resolvedBidFloor) { return imp.toBuilder() .tagid(masterTagId) + .bidfloor(resolvedBidFloor) + .bidfloorcur(BIDDER_CURRENCY) .banner(resolveBanner(imp.getBanner())) .build(); } 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 fa587b581df..d59bcd25aa5 100644 --- a/src/test/java/org/prebid/server/bidder/adview/AdviewBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/adview/AdviewBidderTest.java @@ -11,7 +11,11 @@ import com.iab.openrtb.response.BidResponse; import com.iab.openrtb.response.SeatBid; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; import org.prebid.server.VertxTest; import org.prebid.server.bidder.model.BidderBid; import org.prebid.server.bidder.model.BidderError; @@ -19,18 +23,23 @@ 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.currency.CurrencyConversionService; +import org.prebid.server.exception.PreBidException; import org.prebid.server.proto.openrtb.ext.ExtPrebid; import org.prebid.server.proto.openrtb.ext.request.adview.ExtImpAdview; +import java.math.BigDecimal; +import java.util.Collections; import java.util.List; -import java.util.function.Function; +import java.util.function.UnaryOperator; import java.util.stream.Collectors; -import static java.util.Arrays.asList; -import static java.util.Collections.singletonList; -import static java.util.function.Function.identity; +import static java.util.function.UnaryOperator.identity; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.BDDMockito.given; import static org.prebid.server.proto.openrtb.ext.response.BidType.banner; import static org.prebid.server.proto.openrtb.ext.response.BidType.video; import static org.prebid.server.proto.openrtb.ext.response.BidType.xNative; @@ -41,14 +50,21 @@ public class AdviewBidderTest extends VertxTest { private AdviewBidder adviewBidder; + @Rule + public final MockitoRule mockitoRule = MockitoJUnit.rule(); + + @Mock + private CurrencyConversionService currencyConversionService; + @Before public void setUp() { - adviewBidder = new AdviewBidder(ENDPOINT_URL, jacksonMapper); + adviewBidder = new AdviewBidder(ENDPOINT_URL, jacksonMapper, currencyConversionService); } @Test public void creationShouldFailOnInvalidEndpointUrl() { - assertThatIllegalArgumentException().isThrownBy(() -> new AdviewBidder("invalid_url", jacksonMapper)); + assertThatIllegalArgumentException().isThrownBy(() -> + new AdviewBidder("invalid_url", jacksonMapper, currencyConversionService)); } @Test @@ -88,7 +104,7 @@ public void makeHttpRequestsShouldModifyFirstImpBanner() { final Banner banner = Banner.builder() .w(2) .h(2) - .format(singletonList(Format.builder().w(1).h(1).build())) + .format(Collections.singletonList(Format.builder().w(1).h(1).build())) .build(); final BidRequest bidRequest = givenBidRequest(impBuilder -> impBuilder.banner(banner)); @@ -105,6 +121,55 @@ public void makeHttpRequestsShouldModifyFirstImpBanner() { .containsExactly(banner.toBuilder().w(1).h(1).build()); } + @Test + public void makeHttpRequestsShouldConvertCurrencyIfIncorrect() { + // given + given(currencyConversionService.convertCurrency(any(), any(), anyString(), anyString())) + .willReturn(BigDecimal.TEN); + + final BidRequest bidRequest = givenBidRequest( + impBuilder -> impBuilder.bidfloor(BigDecimal.ONE).bidfloorcur("EUR")); + + // when + final Result>> result = adviewBidder.makeHttpRequests(bidRequest); + + // then + assertThat(result.getErrors()).isEmpty(); + assertThat(result.getValue()) + .extracting(HttpRequest::getPayload) + .extracting(BidRequest::getCur) + .containsExactly(Collections.singletonList("USD")); + assertThat(result.getValue()) + .extracting(HttpRequest::getPayload) + .extracting(BidRequest::getImp) + .containsOnly(Collections.singletonList( + givenImp(impBuilder -> impBuilder + .bidfloorcur("USD") + .bidfloor(BigDecimal.TEN) + .tagid("publisherId")))); + } + + @Test + public void makeHttpRequestsShouldReturnErrorMessageOnFailedCurrencyConversion() { + // given + given(currencyConversionService.convertCurrency(any(), any(), anyString(), anyString())) + .willThrow(PreBidException.class); + + final BidRequest bidRequest = givenBidRequest( + impCustomizer -> impCustomizer.bidfloor(BigDecimal.ONE).bidfloorcur("EUR")); + + // when + Result>> result = adviewBidder.makeHttpRequests(bidRequest); + + // then + assertThat(result.getErrors()).allSatisfy(bidderError -> { + assertThat(bidderError.getType()) + .isEqualTo(BidderError.Type.bad_input); + assertThat(bidderError.getMessage()) + .isEqualTo("Unable to convert provided bid floor currency from EUR to USD for imp `123`"); + }); + } + @Test public void makeHttpRequestsShouldNotModifyFirstImpBannerIfFormatsAreAbsent() { // given @@ -126,8 +191,8 @@ public void makeHttpRequestsShouldNotModifyFirstImpBannerIfFormatsAreAbsent() { @Test public void makeHttpRequestsShouldNotModifyFirstImpBannerIfFirstFormatIsAbsent() { // given - final BidRequest bidRequest = givenBidRequest( - impBuilder -> impBuilder.banner(Banner.builder().format(singletonList(null)).w(2).h(2).build())); + final BidRequest bidRequest = givenBidRequest(impBuilder -> + impBuilder.banner(Banner.builder().format(Collections.singletonList(null)).w(2).h(2).build())); // when final Result>> result = adviewBidder.makeHttpRequests(bidRequest); @@ -138,13 +203,15 @@ public void makeHttpRequestsShouldNotModifyFirstImpBannerIfFirstFormatIsAbsent() .extracting(HttpRequest::getPayload) .flatExtracting(BidRequest::getImp) .extracting(Imp::getBanner) - .containsExactly(Banner.builder().format(singletonList(null)).w(2).h(2).build()); + .containsExactly(Banner.builder().format(Collections.singletonList(null)).w(2).h(2).build()); } @Test public void makeHttpRequestsShouldModifyOnlyFirstImp() { // given - final BidRequest bidRequest = givenBidRequest(identity(), impBuilder -> impBuilder.id("456").ext(null)); + final BidRequest bidRequest = givenBidRequest(identity(), List.of( + identity(), + impBuilder -> impBuilder.id("456").ext(null))); // when final Result>> result = adviewBidder.makeHttpRequests(bidRequest); @@ -270,8 +337,8 @@ public void makeBidsShouldReturnBannerBidIfBannerAndVideoAreAbsentInRequestImp() } private static BidRequest givenBidRequest( - Function bidRequestCustomizer, - List> impCustomizers) { + UnaryOperator bidRequestCustomizer, + List> impCustomizers) { return bidRequestCustomizer.apply(BidRequest.builder() .imp(impCustomizers.stream() @@ -280,11 +347,11 @@ private static BidRequest givenBidRequest( .build(); } - private static BidRequest givenBidRequest(Function... impCustomizers) { - return givenBidRequest(identity(), asList(impCustomizers)); + private static BidRequest givenBidRequest(UnaryOperator impCustomizer) { + return givenBidRequest(identity(), Collections.singletonList(impCustomizer)); } - private static Imp givenImp(Function impCustomizer) { + private static Imp givenImp(UnaryOperator impCustomizer) { return impCustomizer.apply(Imp.builder() .id("123") .banner(Banner.builder().w(23).h(25).build()) @@ -293,9 +360,10 @@ private static Imp givenImp(Function impCustomiz .build(); } - private static BidResponse givenBidResponse(Function bidCustomizer) { + private static BidResponse givenBidResponse(UnaryOperator bidCustomizer) { return BidResponse.builder() - .seatbid(singletonList(SeatBid.builder().bid(singletonList(bidCustomizer.apply(Bid.builder()).build())) + .seatbid(Collections.singletonList(SeatBid.builder() + .bid(Collections.singletonList(bidCustomizer.apply(Bid.builder()).build())) .build())) .build(); } diff --git a/src/test/resources/org/prebid/server/it/openrtb2/adview/test-adview-bid-request.json b/src/test/resources/org/prebid/server/it/openrtb2/adview/test-adview-bid-request.json index 772853fd9ac..8f6adb73f46 100644 --- a/src/test/resources/org/prebid/server/it/openrtb2/adview/test-adview-bid-request.json +++ b/src/test/resources/org/prebid/server/it/openrtb2/adview/test-adview-bid-request.json @@ -8,6 +8,7 @@ "h": 250 }, "tagid": "placementId", + "bidfloorcur": "USD", "ext": { "bidder": { "placementId": "placementId", From 7191134f7465e59654ab241e21783f56ffa9acb2 Mon Sep 17 00:00:00 2001 From: Yevhenii Viktorov Date: Wed, 1 Dec 2021 17:01:44 +0200 Subject: [PATCH 03/15] move modifiedBidRequest into separate variable --- .../java/org/prebid/server/bidder/adview/AdviewBidder.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 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 78935cedf97..c409f786269 100644 --- a/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java +++ b/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java @@ -58,10 +58,11 @@ public AdviewBidder(String endpointUrl, public Result>> makeHttpRequests(BidRequest request) { final Imp firstImp = request.getImp().get(0); final ExtImpAdview extImpAdview; + final BidRequest modifiedBidRequest; try { extImpAdview = parseExtImp(firstImp); - request = modifyRequest(request, extImpAdview.getMasterTagId(), resolveBidFloor(request, firstImp)); + modifiedBidRequest = modifyRequest(request, extImpAdview.getMasterTagId(), resolveBidFloor(request, firstImp)); } catch (PreBidException e) { return Result.withError(BidderError.badInput(e.getMessage())); } @@ -71,8 +72,8 @@ public Result>> makeHttpRequests(BidRequest request .method(HttpMethod.POST) .uri(resolveEndpoint(extImpAdview.getAccountId())) .headers(HttpUtil.headers()) - .body(mapper.encodeToBytes(request)) - .payload(request) + .body(mapper.encodeToBytes(modifiedBidRequest)) + .payload(modifiedBidRequest) .build()); } From 3d925afd5ae09b45fe00139725532e744d538bbf Mon Sep 17 00:00:00 2001 From: Yevhenii Viktorov Date: Thu, 2 Dec 2021 21:08:23 +0200 Subject: [PATCH 04/15] fix CR comments --- .../server/bidder/adview/AdviewBidder.java | 21 ++++++++------- .../config/bidder/AdviewConfiguration.java | 2 +- .../bidder/adview/AdviewBidderTest.java | 26 ++++++++++--------- 3 files changed, 27 insertions(+), 22 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 c409f786269..7bf28f92332 100644 --- a/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java +++ b/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java @@ -47,11 +47,11 @@ public class AdviewBidder implements Bidder { private final CurrencyConversionService currencyConversionService; public AdviewBidder(String endpointUrl, - JacksonMapper mapper, - CurrencyConversionService currencyConversionService) { + CurrencyConversionService currencyConversionService, + JacksonMapper mapper) { this.endpointUrl = HttpUtil.validateUrl(Objects.requireNonNull(endpointUrl)); - this.mapper = Objects.requireNonNull(mapper); this.currencyConversionService = Objects.requireNonNull(currencyConversionService); + this.mapper = Objects.requireNonNull(mapper); } @Override @@ -86,9 +86,12 @@ private ExtImpAdview parseExtImp(Imp imp) { } private BigDecimal resolveBidFloor(BidRequest request, Imp imp) { - return shouldConvertBidFloor(imp.getBidfloor(), imp.getBidfloorcur()) - ? convertBidFloorCurrency(imp.getBidfloor(), request, imp.getId(), imp.getBidfloorcur()) - : imp.getBidfloor(); + final BigDecimal bidFloor = imp.getBidfloor(); + final String bidFloorCur = imp.getBidfloorcur(); + + return shouldConvertBidFloor(bidFloor, bidFloorCur) + ? convertBidFloorCurrency(bidFloor, bidFloorCur, imp.getId(), request) + : bidFloor; } private static boolean shouldConvertBidFloor(BigDecimal bidFloor, String bidFloorCur) { @@ -96,9 +99,9 @@ private static boolean shouldConvertBidFloor(BigDecimal bidFloor, String bidFloo } private BigDecimal convertBidFloorCurrency(BigDecimal bidFloor, - BidRequest bidRequest, + String bidFloorCur, String impId, - String bidFloorCur) { + BidRequest bidRequest) { try { return currencyConversionService .convertCurrency(bidFloor, bidRequest, bidFloorCur, BIDDER_CURRENCY); @@ -112,7 +115,7 @@ private BigDecimal convertBidFloorCurrency(BigDecimal bidFloor, private static BidRequest modifyRequest(BidRequest bidRequest, String masterTagId, BigDecimal resolvedBidFloor) { return bidRequest.toBuilder() .imp(modifyImps(bidRequest.getImp(), masterTagId, resolvedBidFloor)) - .cur(Collections.singletonList("USD")) + .cur(Collections.singletonList(BIDDER_CURRENCY)) .build(); } diff --git a/src/main/java/org/prebid/server/spring/config/bidder/AdviewConfiguration.java b/src/main/java/org/prebid/server/spring/config/bidder/AdviewConfiguration.java index 486ba414424..40b81bf434f 100644 --- a/src/main/java/org/prebid/server/spring/config/bidder/AdviewConfiguration.java +++ b/src/main/java/org/prebid/server/spring/config/bidder/AdviewConfiguration.java @@ -37,7 +37,7 @@ BidderDeps adviewBidderDeps(BidderConfigurationProperties adviewConfigurationPro return BidderDepsAssembler.forBidder(BIDDER_NAME) .withConfig(adviewConfigurationProperties) .usersyncerCreator(UsersyncerCreator.create(externalUrl)) - .bidderCreator(config -> new AdviewBidder(config.getEndpoint(), mapper, currencyConversionService)) + .bidderCreator(config -> new AdviewBidder(config.getEndpoint(), currencyConversionService, mapper)) .assemble(); } } 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 d59bcd25aa5..d1cb5127c3d 100644 --- a/src/test/java/org/prebid/server/bidder/adview/AdviewBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/adview/AdviewBidderTest.java @@ -29,14 +29,15 @@ import org.prebid.server.proto.openrtb.ext.request.adview.ExtImpAdview; import java.math.BigDecimal; -import java.util.Collections; import java.util.List; import java.util.function.UnaryOperator; import java.util.stream.Collectors; +import static java.util.Collections.singletonList; import static java.util.function.UnaryOperator.identity; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.AssertionsForClassTypes.tuple; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.BDDMockito.given; @@ -58,13 +59,13 @@ public class AdviewBidderTest extends VertxTest { @Before public void setUp() { - adviewBidder = new AdviewBidder(ENDPOINT_URL, jacksonMapper, currencyConversionService); + adviewBidder = new AdviewBidder(ENDPOINT_URL, currencyConversionService, jacksonMapper); } @Test public void creationShouldFailOnInvalidEndpointUrl() { assertThatIllegalArgumentException().isThrownBy(() -> - new AdviewBidder("invalid_url", jacksonMapper, currencyConversionService)); + new AdviewBidder("invalid_url", currencyConversionService, jacksonMapper)); } @Test @@ -104,7 +105,7 @@ public void makeHttpRequestsShouldModifyFirstImpBanner() { final Banner banner = Banner.builder() .w(2) .h(2) - .format(Collections.singletonList(Format.builder().w(1).h(1).build())) + .format(singletonList(Format.builder().w(1).h(1).build())) .build(); final BidRequest bidRequest = givenBidRequest(impBuilder -> impBuilder.banner(banner)); @@ -137,12 +138,13 @@ public void makeHttpRequestsShouldConvertCurrencyIfIncorrect() { assertThat(result.getErrors()).isEmpty(); assertThat(result.getValue()) .extracting(HttpRequest::getPayload) - .extracting(BidRequest::getCur) - .containsExactly(Collections.singletonList("USD")); + .flatExtracting(BidRequest::getImp) + .extracting(Imp::getBidfloor, Imp::getBidfloorcur) + .containsOnly(tuple(BigDecimal.TEN, "USD")); assertThat(result.getValue()) .extracting(HttpRequest::getPayload) .extracting(BidRequest::getImp) - .containsOnly(Collections.singletonList( + .containsOnly(singletonList( givenImp(impBuilder -> impBuilder .bidfloorcur("USD") .bidfloor(BigDecimal.TEN) @@ -192,7 +194,7 @@ public void makeHttpRequestsShouldNotModifyFirstImpBannerIfFormatsAreAbsent() { public void makeHttpRequestsShouldNotModifyFirstImpBannerIfFirstFormatIsAbsent() { // given final BidRequest bidRequest = givenBidRequest(impBuilder -> - impBuilder.banner(Banner.builder().format(Collections.singletonList(null)).w(2).h(2).build())); + impBuilder.banner(Banner.builder().format(singletonList(null)).w(2).h(2).build())); // when final Result>> result = adviewBidder.makeHttpRequests(bidRequest); @@ -203,7 +205,7 @@ public void makeHttpRequestsShouldNotModifyFirstImpBannerIfFirstFormatIsAbsent() .extracting(HttpRequest::getPayload) .flatExtracting(BidRequest::getImp) .extracting(Imp::getBanner) - .containsExactly(Banner.builder().format(Collections.singletonList(null)).w(2).h(2).build()); + .containsExactly(Banner.builder().format(singletonList(null)).w(2).h(2).build()); } @Test @@ -348,7 +350,7 @@ private static BidRequest givenBidRequest( } private static BidRequest givenBidRequest(UnaryOperator impCustomizer) { - return givenBidRequest(identity(), Collections.singletonList(impCustomizer)); + return givenBidRequest(identity(), singletonList(impCustomizer)); } private static Imp givenImp(UnaryOperator impCustomizer) { @@ -362,8 +364,8 @@ private static Imp givenImp(UnaryOperator impCustomizer) { private static BidResponse givenBidResponse(UnaryOperator bidCustomizer) { return BidResponse.builder() - .seatbid(Collections.singletonList(SeatBid.builder() - .bid(Collections.singletonList(bidCustomizer.apply(Bid.builder()).build())) + .seatbid(singletonList(SeatBid.builder() + .bid(singletonList(bidCustomizer.apply(Bid.builder()).build())) .build())) .build(); } From e2e0a2b10941ebfdb9e6e89d488fccb1acfa8004 Mon Sep 17 00:00:00 2001 From: Yevhenii Viktorov Date: Thu, 2 Dec 2021 21:10:06 +0200 Subject: [PATCH 05/15] split a line to be less then 120 --- .../java/org/prebid/server/bidder/adview/AdviewBidder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 7bf28f92332..97ba664e4f6 100644 --- a/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java +++ b/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java @@ -62,7 +62,8 @@ public Result>> makeHttpRequests(BidRequest request try { extImpAdview = parseExtImp(firstImp); - modifiedBidRequest = modifyRequest(request, extImpAdview.getMasterTagId(), resolveBidFloor(request, firstImp)); + modifiedBidRequest = + modifyRequest(request, extImpAdview.getMasterTagId(), resolveBidFloor(request, firstImp)); } catch (PreBidException e) { return Result.withError(BidderError.badInput(e.getMessage())); } From b6df5cb56631dcba58ad9fc1c3ee38c62268d1cf Mon Sep 17 00:00:00 2001 From: Yevhenii Viktorov Date: Fri, 3 Dec 2021 12:51:16 +0200 Subject: [PATCH 06/15] change logic how imp updated, depending on bidfloor change --- .../server/bidder/adview/AdviewBidder.java | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 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 97ba664e4f6..d5cc2b714ac 100644 --- a/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java +++ b/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java @@ -127,12 +127,23 @@ private static List modifyImps(List imps, String masterTagId, BigDecim } private static Imp modifyImp(Imp imp, String masterTagId, BigDecimal resolvedBidFloor) { - return imp.toBuilder() - .tagid(masterTagId) - .bidfloor(resolvedBidFloor) - .bidfloorcur(BIDDER_CURRENCY) - .banner(resolveBanner(imp.getBanner())) - .build(); + final Banner banner = imp.getBanner(); + + if (resolvedBidFloor != null) { + return imp.toBuilder() + .tagid(masterTagId) + .bidfloor(resolvedBidFloor) + .bidfloorcur(resolvedBidFloor.compareTo(imp.getBidfloor()) != 0 + ? BIDDER_CURRENCY + : imp.getBidfloorcur()) + .banner(resolveBanner(banner)) + .build(); + } else { + return imp.toBuilder() + .tagid(masterTagId) + .banner(resolveBanner(banner)) + .build(); + } } private static Banner resolveBanner(Banner banner) { From 450875f75c2fcb439434904ee31cfda7e1215730 Mon Sep 17 00:00:00 2001 From: Yevhenii Viktorov Date: Fri, 3 Dec 2021 13:05:18 +0200 Subject: [PATCH 07/15] added check for case when imp.getBidfloor == null, fixed integration test --- src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java | 2 +- .../server/it/openrtb2/adview/test-adview-bid-request.json | 1 - 2 files changed, 1 insertion(+), 2 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 d5cc2b714ac..828fddd5e3d 100644 --- a/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java +++ b/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java @@ -129,7 +129,7 @@ private static List modifyImps(List imps, String masterTagId, BigDecim private static Imp modifyImp(Imp imp, String masterTagId, BigDecimal resolvedBidFloor) { final Banner banner = imp.getBanner(); - if (resolvedBidFloor != null) { + if (imp.getBidfloor() != null && resolvedBidFloor != null) { return imp.toBuilder() .tagid(masterTagId) .bidfloor(resolvedBidFloor) diff --git a/src/test/resources/org/prebid/server/it/openrtb2/adview/test-adview-bid-request.json b/src/test/resources/org/prebid/server/it/openrtb2/adview/test-adview-bid-request.json index 8f6adb73f46..772853fd9ac 100644 --- a/src/test/resources/org/prebid/server/it/openrtb2/adview/test-adview-bid-request.json +++ b/src/test/resources/org/prebid/server/it/openrtb2/adview/test-adview-bid-request.json @@ -8,7 +8,6 @@ "h": 250 }, "tagid": "placementId", - "bidfloorcur": "USD", "ext": { "bidder": { "placementId": "placementId", From 732c3d6326d097df4bd7be8d1ed7c41fabaee365 Mon Sep 17 00:00:00 2001 From: Yevhenii Viktorov Date: Fri, 3 Dec 2021 14:52:51 +0200 Subject: [PATCH 08/15] refactor bidder imp modification logic --- .../server/bidder/adview/AdviewBidder.java | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 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 828fddd5e3d..e89b33d60c7 100644 --- a/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java +++ b/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java @@ -128,22 +128,19 @@ private static List modifyImps(List imps, String masterTagId, BigDecim private static Imp modifyImp(Imp imp, String masterTagId, BigDecimal resolvedBidFloor) { final Banner banner = imp.getBanner(); + final Imp.ImpBuilder impBuilder = imp.toBuilder() + .tagid(masterTagId) + .banner(resolveBanner(banner)); if (imp.getBidfloor() != null && resolvedBidFloor != null) { - return imp.toBuilder() - .tagid(masterTagId) + impBuilder .bidfloor(resolvedBidFloor) .bidfloorcur(resolvedBidFloor.compareTo(imp.getBidfloor()) != 0 ? BIDDER_CURRENCY - : imp.getBidfloorcur()) - .banner(resolveBanner(banner)) - .build(); - } else { - return imp.toBuilder() - .tagid(masterTagId) - .banner(resolveBanner(banner)) - .build(); + : imp.getBidfloorcur()); } + + return impBuilder.build(); } private static Banner resolveBanner(Banner banner) { From 957193ce5c86f227ab007c9d4fb62017d78f1fba Mon Sep 17 00:00:00 2001 From: Yevhenii Viktorov Date: Fri, 10 Dec 2021 12:28:58 +0200 Subject: [PATCH 09/15] refactor bidfloor currency --- .../server/bidder/adview/AdviewBidder.java | 51 ++++++++++++------- 1 file changed, 32 insertions(+), 19 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 e89b33d60c7..4571e5a74b1 100644 --- a/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java +++ b/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java @@ -62,8 +62,10 @@ public Result>> makeHttpRequests(BidRequest request try { extImpAdview = parseExtImp(firstImp); + final BigDecimal resolvedBidFloor = resolveBidFloor(request, firstImp); modifiedBidRequest = - modifyRequest(request, extImpAdview.getMasterTagId(), resolveBidFloor(request, firstImp)); + modifyRequest(request, extImpAdview.getMasterTagId(), resolvedBidFloor, + resolveBidFloorCurrency(firstImp, resolvedBidFloor)); } catch (PreBidException e) { return Result.withError(BidderError.badInput(e.getMessage())); } @@ -86,6 +88,16 @@ private ExtImpAdview parseExtImp(Imp imp) { } } + private String resolveBidFloorCurrency(Imp firstImp, BigDecimal resolvedBidFloor) { + final BigDecimal impBidFloor = firstImp.getBidfloor(); + + if (impBidFloor != null && resolvedBidFloor != null && resolvedBidFloor.compareTo(impBidFloor) != 0) { + return BIDDER_CURRENCY; + } + + return firstImp.getBidfloorcur(); + } + private BigDecimal resolveBidFloor(BidRequest request, Imp imp) { final BigDecimal bidFloor = imp.getBidfloor(); final String bidFloorCur = imp.getBidfloorcur(); @@ -113,34 +125,35 @@ private BigDecimal convertBidFloorCurrency(BigDecimal bidFloor, } } - private static BidRequest modifyRequest(BidRequest bidRequest, String masterTagId, BigDecimal resolvedBidFloor) { + private static BidRequest modifyRequest(BidRequest bidRequest, + String masterTagId, + BigDecimal resolvedBidFloor, + String resolvedBidFloorCur) { return bidRequest.toBuilder() - .imp(modifyImps(bidRequest.getImp(), masterTagId, resolvedBidFloor)) + .imp(modifyImps(bidRequest.getImp(), masterTagId, resolvedBidFloor, resolvedBidFloorCur)) .cur(Collections.singletonList(BIDDER_CURRENCY)) .build(); } - private static List modifyImps(List imps, String masterTagId, BigDecimal resolvedBidFloor) { + private static List modifyImps(List imps, + String masterTagId, + BigDecimal resolvedBidFloor, + String resolvedBidFloorCur) { final List modifiedImps = new ArrayList<>(imps); - modifiedImps.set(0, modifyImp(imps.get(0), masterTagId, resolvedBidFloor)); + modifiedImps.set(0, modifyImp(imps.get(0), masterTagId, resolvedBidFloor, resolvedBidFloorCur)); return modifiedImps; } - private static Imp modifyImp(Imp imp, String masterTagId, BigDecimal resolvedBidFloor) { - final Banner banner = imp.getBanner(); - final Imp.ImpBuilder impBuilder = imp.toBuilder() + private static Imp modifyImp(Imp imp, + String masterTagId, + BigDecimal resolvedBidFloor, + String resolvedBidFloorCur) { + return imp.toBuilder() .tagid(masterTagId) - .banner(resolveBanner(banner)); - - if (imp.getBidfloor() != null && resolvedBidFloor != null) { - impBuilder - .bidfloor(resolvedBidFloor) - .bidfloorcur(resolvedBidFloor.compareTo(imp.getBidfloor()) != 0 - ? BIDDER_CURRENCY - : imp.getBidfloorcur()); - } - - return impBuilder.build(); + .bidfloor(resolvedBidFloor) + .bidfloorcur(resolvedBidFloorCur) + .banner(resolveBanner(imp.getBanner())) + .build(); } private static Banner resolveBanner(Banner banner) { From b903eaa4fd330439cff19bd2356a8847614278d0 Mon Sep 17 00:00:00 2001 From: Yevhenii Viktorov Date: Fri, 10 Dec 2021 14:24:29 +0200 Subject: [PATCH 10/15] resolve CR comments --- .../java/org/prebid/server/bidder/adview/AdviewBidder.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 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 4571e5a74b1..f5b256beb75 100644 --- a/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java +++ b/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java @@ -43,12 +43,13 @@ public class AdviewBidder implements Bidder { private static final String BIDDER_CURRENCY = "USD"; private final String endpointUrl; - private final JacksonMapper mapper; private final CurrencyConversionService currencyConversionService; + private final JacksonMapper mapper; public AdviewBidder(String endpointUrl, CurrencyConversionService currencyConversionService, JacksonMapper mapper) { + this.endpointUrl = HttpUtil.validateUrl(Objects.requireNonNull(endpointUrl)); this.currencyConversionService = Objects.requireNonNull(currencyConversionService); this.mapper = Objects.requireNonNull(mapper); @@ -63,9 +64,9 @@ public Result>> makeHttpRequests(BidRequest request try { extImpAdview = parseExtImp(firstImp); final BigDecimal resolvedBidFloor = resolveBidFloor(request, firstImp); + final String resolvedBidFloorCurrency = resolveBidFloorCurrency(firstImp, resolvedBidFloor); modifiedBidRequest = - modifyRequest(request, extImpAdview.getMasterTagId(), resolvedBidFloor, - resolveBidFloorCurrency(firstImp, resolvedBidFloor)); + modifyRequest(request, extImpAdview.getMasterTagId(), resolvedBidFloor, resolvedBidFloorCurrency); } catch (PreBidException e) { return Result.withError(BidderError.badInput(e.getMessage())); } From 2cda1b241eef258cdeee4009a2293b3a5c9b50b6 Mon Sep 17 00:00:00 2001 From: Yevhenii Viktorov Date: Fri, 10 Dec 2021 14:27:29 +0200 Subject: [PATCH 11/15] resolve CR comment I oversaw --- .../server/bidder/adview/AdviewBidder.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 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 f5b256beb75..2a0be29971e 100644 --- a/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java +++ b/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java @@ -89,16 +89,6 @@ private ExtImpAdview parseExtImp(Imp imp) { } } - private String resolveBidFloorCurrency(Imp firstImp, BigDecimal resolvedBidFloor) { - final BigDecimal impBidFloor = firstImp.getBidfloor(); - - if (impBidFloor != null && resolvedBidFloor != null && resolvedBidFloor.compareTo(impBidFloor) != 0) { - return BIDDER_CURRENCY; - } - - return firstImp.getBidfloorcur(); - } - private BigDecimal resolveBidFloor(BidRequest request, Imp imp) { final BigDecimal bidFloor = imp.getBidfloor(); final String bidFloorCur = imp.getBidfloorcur(); @@ -126,6 +116,16 @@ private BigDecimal convertBidFloorCurrency(BigDecimal bidFloor, } } + private String resolveBidFloorCurrency(Imp firstImp, BigDecimal resolvedBidFloor) { + final BigDecimal impBidFloor = firstImp.getBidfloor(); + + if (impBidFloor != null && resolvedBidFloor != null && resolvedBidFloor.compareTo(impBidFloor) != 0) { + return BIDDER_CURRENCY; + } + + return firstImp.getBidfloorcur(); + } + private static BidRequest modifyRequest(BidRequest bidRequest, String masterTagId, BigDecimal resolvedBidFloor, From 88d004825f159ec260b257b7a657c67c5ac0fc9c Mon Sep 17 00:00:00 2001 From: Yevhenii Viktorov Date: Mon, 13 Dec 2021 11:42:22 +0200 Subject: [PATCH 12/15] refactor, so that bidfloor is converted based on currency not other way around --- .../server/bidder/adview/AdviewBidder.java | 45 +++++++++---------- 1 file changed, 22 insertions(+), 23 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 2a0be29971e..cbba8c92a7d 100644 --- a/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java +++ b/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java @@ -63,8 +63,16 @@ public Result>> makeHttpRequests(BidRequest request try { extImpAdview = parseExtImp(firstImp); - final BigDecimal resolvedBidFloor = resolveBidFloor(request, firstImp); - final String resolvedBidFloorCurrency = resolveBidFloorCurrency(firstImp, resolvedBidFloor); + final String resolvedBidFloorCurrency; + final BigDecimal resolvedBidFloor; + if (shouldConvertBidFloor(firstImp)) { + resolvedBidFloorCurrency = BIDDER_CURRENCY; + resolvedBidFloor = resolveBidFloor(request, firstImp, resolvedBidFloorCurrency); + } else { + resolvedBidFloorCurrency = firstImp.getBidfloorcur(); + resolvedBidFloor = firstImp.getBidfloor(); + } + modifiedBidRequest = modifyRequest(request, extImpAdview.getMasterTagId(), resolvedBidFloor, resolvedBidFloorCurrency); } catch (PreBidException e) { @@ -89,23 +97,24 @@ private ExtImpAdview parseExtImp(Imp imp) { } } - private BigDecimal resolveBidFloor(BidRequest request, Imp imp) { + private static boolean shouldConvertBidFloor(Imp imp) { + return BidderUtil.isValidPrice(imp.getBidfloor()) + && !StringUtils.equalsIgnoreCase(imp.getBidfloorcur(), BIDDER_CURRENCY); + } + + private BigDecimal resolveBidFloor(BidRequest request, Imp imp, String bidFloorCurrency) { final BigDecimal bidFloor = imp.getBidfloor(); final String bidFloorCur = imp.getBidfloorcur(); - return shouldConvertBidFloor(bidFloor, bidFloorCur) - ? convertBidFloorCurrency(bidFloor, bidFloorCur, imp.getId(), request) + return !bidFloorCurrency.equals(bidFloorCur) + ? convertBidFloor(bidFloor, bidFloorCur, imp.getId(), request) : bidFloor; } - private static boolean shouldConvertBidFloor(BigDecimal bidFloor, String bidFloorCur) { - return BidderUtil.isValidPrice(bidFloor) && !StringUtils.equalsIgnoreCase(bidFloorCur, BIDDER_CURRENCY); - } - - private BigDecimal convertBidFloorCurrency(BigDecimal bidFloor, - String bidFloorCur, - String impId, - BidRequest bidRequest) { + private BigDecimal convertBidFloor(BigDecimal bidFloor, + String bidFloorCur, + String impId, + BidRequest bidRequest) { try { return currencyConversionService .convertCurrency(bidFloor, bidRequest, bidFloorCur, BIDDER_CURRENCY); @@ -116,16 +125,6 @@ private BigDecimal convertBidFloorCurrency(BigDecimal bidFloor, } } - private String resolveBidFloorCurrency(Imp firstImp, BigDecimal resolvedBidFloor) { - final BigDecimal impBidFloor = firstImp.getBidfloor(); - - if (impBidFloor != null && resolvedBidFloor != null && resolvedBidFloor.compareTo(impBidFloor) != 0) { - return BIDDER_CURRENCY; - } - - return firstImp.getBidfloorcur(); - } - private static BidRequest modifyRequest(BidRequest bidRequest, String masterTagId, BigDecimal resolvedBidFloor, From e9ea8164c0d20c5ff6f279932fb46aa567775621 Mon Sep 17 00:00:00 2001 From: Yevhenii Viktorov Date: Mon, 13 Dec 2021 12:29:07 +0200 Subject: [PATCH 13/15] refactor method signatures --- .../server/bidder/adview/AdviewBidder.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 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 cbba8c92a7d..9cf82aceba3 100644 --- a/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java +++ b/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java @@ -127,31 +127,31 @@ private BigDecimal convertBidFloor(BigDecimal bidFloor, private static BidRequest modifyRequest(BidRequest bidRequest, String masterTagId, - BigDecimal resolvedBidFloor, - String resolvedBidFloorCur) { + BigDecimal bidFloor, + String bidFloorCur) { return bidRequest.toBuilder() - .imp(modifyImps(bidRequest.getImp(), masterTagId, resolvedBidFloor, resolvedBidFloorCur)) + .imp(modifyImps(bidRequest.getImp(), masterTagId, bidFloor, bidFloorCur)) .cur(Collections.singletonList(BIDDER_CURRENCY)) .build(); } private static List modifyImps(List imps, String masterTagId, - BigDecimal resolvedBidFloor, - String resolvedBidFloorCur) { + BigDecimal bidFloor, + String bidFloorCur) { final List modifiedImps = new ArrayList<>(imps); - modifiedImps.set(0, modifyImp(imps.get(0), masterTagId, resolvedBidFloor, resolvedBidFloorCur)); + modifiedImps.set(0, modifyImp(imps.get(0), masterTagId, bidFloor, bidFloorCur)); return modifiedImps; } private static Imp modifyImp(Imp imp, String masterTagId, - BigDecimal resolvedBidFloor, - String resolvedBidFloorCur) { + BigDecimal bidFloor, + String bidFloorCur) { return imp.toBuilder() .tagid(masterTagId) - .bidfloor(resolvedBidFloor) - .bidfloorcur(resolvedBidFloorCur) + .bidfloor(bidFloor) + .bidfloorcur(bidFloorCur) .banner(resolveBanner(imp.getBanner())) .build(); } From c02be51ec39068e69b83e1d8e5475fb205505aa1 Mon Sep 17 00:00:00 2001 From: Alex Maltsev Date: Wed, 15 Dec 2021 15:10:16 +0200 Subject: [PATCH 14/15] Add currency conversion proposal (#1633) --- .../server/bidder/adview/AdviewBidder.java | 68 ++++++------------- .../org/prebid/server/bidder/model/Price.java | 13 ++++ .../org/prebid/server/util/BidderUtil.java | 7 ++ 3 files changed, 41 insertions(+), 47 deletions(-) create mode 100644 src/main/java/org/prebid/server/bidder/model/Price.java 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 9cf82aceba3..f77a9a5c58a 100644 --- a/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java +++ b/src/main/java/org/prebid/server/bidder/adview/AdviewBidder.java @@ -9,8 +9,8 @@ import com.iab.openrtb.response.SeatBid; import io.vertx.core.http.HttpMethod; import org.apache.commons.collections4.CollectionUtils; -import org.apache.commons.lang3.StringUtils; import org.prebid.server.bidder.Bidder; +import org.prebid.server.bidder.model.Price; import org.prebid.server.bidder.model.BidderBid; import org.prebid.server.bidder.model.BidderError; import org.prebid.server.bidder.model.HttpCall; @@ -63,18 +63,8 @@ public Result>> makeHttpRequests(BidRequest request try { extImpAdview = parseExtImp(firstImp); - final String resolvedBidFloorCurrency; - final BigDecimal resolvedBidFloor; - if (shouldConvertBidFloor(firstImp)) { - resolvedBidFloorCurrency = BIDDER_CURRENCY; - resolvedBidFloor = resolveBidFloor(request, firstImp, resolvedBidFloorCurrency); - } else { - resolvedBidFloorCurrency = firstImp.getBidfloorcur(); - resolvedBidFloor = firstImp.getBidfloor(); - } - - modifiedBidRequest = - modifyRequest(request, extImpAdview.getMasterTagId(), resolvedBidFloor, resolvedBidFloorCurrency); + final Price bidFloorPrice = resolveBidFloor(firstImp, request); + modifiedBidRequest = modifyRequest(request, extImpAdview.getMasterTagId(), bidFloorPrice); } catch (PreBidException e) { return Result.withError(BidderError.badInput(e.getMessage())); } @@ -97,27 +87,20 @@ private ExtImpAdview parseExtImp(Imp imp) { } } - private static boolean shouldConvertBidFloor(Imp imp) { - return BidderUtil.isValidPrice(imp.getBidfloor()) - && !StringUtils.equalsIgnoreCase(imp.getBidfloorcur(), BIDDER_CURRENCY); - } - - private BigDecimal resolveBidFloor(BidRequest request, Imp imp, String bidFloorCurrency) { - final BigDecimal bidFloor = imp.getBidfloor(); - final String bidFloorCur = imp.getBidfloorcur(); - - return !bidFloorCurrency.equals(bidFloorCur) - ? convertBidFloor(bidFloor, bidFloorCur, imp.getId(), request) - : bidFloor; + private Price resolveBidFloor(Imp imp, BidRequest bidRequest) { + final Price initialBidFloorPrice = Price.of(imp.getBidfloorcur(), imp.getBidfloor()); + return BidderUtil.isValidPrice(initialBidFloorPrice) + ? convertBidFloor(initialBidFloorPrice, imp.getId(), bidRequest) + : initialBidFloorPrice; } - private BigDecimal convertBidFloor(BigDecimal bidFloor, - String bidFloorCur, - String impId, - BidRequest bidRequest) { + private Price convertBidFloor(Price bidFloorPrice, String impId, BidRequest bidRequest) { + final String bidFloorCur = bidFloorPrice.getCurrency(); try { - return currencyConversionService - .convertCurrency(bidFloor, bidRequest, bidFloorCur, BIDDER_CURRENCY); + final BigDecimal convertedPrice = currencyConversionService + .convertCurrency(bidFloorPrice.getValue(), bidRequest, bidFloorCur, BIDDER_CURRENCY); + + return Price.of(BIDDER_CURRENCY, convertedPrice); } catch (PreBidException e) { throw new PreBidException(String.format( "Unable to convert provided bid floor currency from %s to %s for imp `%s`", @@ -125,33 +108,24 @@ private BigDecimal convertBidFloor(BigDecimal bidFloor, } } - private static BidRequest modifyRequest(BidRequest bidRequest, - String masterTagId, - BigDecimal bidFloor, - String bidFloorCur) { + private static BidRequest modifyRequest(BidRequest bidRequest, String masterTagId, Price bidFloorPrice) { return bidRequest.toBuilder() - .imp(modifyImps(bidRequest.getImp(), masterTagId, bidFloor, bidFloorCur)) + .imp(modifyImps(bidRequest.getImp(), masterTagId, bidFloorPrice)) .cur(Collections.singletonList(BIDDER_CURRENCY)) .build(); } - private static List modifyImps(List imps, - String masterTagId, - BigDecimal bidFloor, - String bidFloorCur) { + private static List modifyImps(List imps, String masterTagId, Price bidFloorPrice) { final List modifiedImps = new ArrayList<>(imps); - modifiedImps.set(0, modifyImp(imps.get(0), masterTagId, bidFloor, bidFloorCur)); + modifiedImps.set(0, modifyImp(imps.get(0), masterTagId, bidFloorPrice)); return modifiedImps; } - private static Imp modifyImp(Imp imp, - String masterTagId, - BigDecimal bidFloor, - String bidFloorCur) { + private static Imp modifyImp(Imp imp, String masterTagId, Price bidFloorPrice) { return imp.toBuilder() .tagid(masterTagId) - .bidfloor(bidFloor) - .bidfloorcur(bidFloorCur) + .bidfloor(bidFloorPrice.getValue()) + .bidfloorcur(bidFloorPrice.getCurrency()) .banner(resolveBanner(imp.getBanner())) .build(); } diff --git a/src/main/java/org/prebid/server/bidder/model/Price.java b/src/main/java/org/prebid/server/bidder/model/Price.java new file mode 100644 index 00000000000..c2e55939791 --- /dev/null +++ b/src/main/java/org/prebid/server/bidder/model/Price.java @@ -0,0 +1,13 @@ +package org.prebid.server.bidder.model; + +import lombok.Value; + +import java.math.BigDecimal; + +@Value(staticConstructor = "of") +public class Price { + + String currency; + + BigDecimal value; +} diff --git a/src/main/java/org/prebid/server/util/BidderUtil.java b/src/main/java/org/prebid/server/util/BidderUtil.java index 725400a1541..f806249c905 100644 --- a/src/main/java/org/prebid/server/util/BidderUtil.java +++ b/src/main/java/org/prebid/server/util/BidderUtil.java @@ -1,5 +1,8 @@ package org.prebid.server.util; +import org.apache.commons.lang3.StringUtils; +import org.prebid.server.bidder.model.Price; + import java.math.BigDecimal; public class BidderUtil { @@ -10,4 +13,8 @@ private BidderUtil() { public static boolean isValidPrice(BigDecimal price) { return price != null && price.compareTo(BigDecimal.ZERO) > 0; } + + public static boolean isValidPrice(Price price) { + return isValidPrice(price.getValue()) && StringUtils.isNotBlank(price.getCurrency()); + } } From 057a6b987f48a8a131e3fafedc564826625cb61f Mon Sep 17 00:00:00 2001 From: Yevhenii Viktorov Date: Mon, 20 Dec 2021 10:04:18 +0200 Subject: [PATCH 15/15] rename test name and remove unnecessary checks in that test --- .../prebid/server/bidder/adview/AdviewBidderTest.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) 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 d1cb5127c3d..71aa7d6bd44 100644 --- a/src/test/java/org/prebid/server/bidder/adview/AdviewBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/adview/AdviewBidderTest.java @@ -123,7 +123,7 @@ public void makeHttpRequestsShouldModifyFirstImpBanner() { } @Test - public void makeHttpRequestsShouldConvertCurrencyIfIncorrect() { + public void makeHttpRequestsShouldConvertCurrencyIfRequestCurrencyDoesNotMatchBidderCurrency() { // given given(currencyConversionService.convertCurrency(any(), any(), anyString(), anyString())) .willReturn(BigDecimal.TEN); @@ -141,14 +141,6 @@ public void makeHttpRequestsShouldConvertCurrencyIfIncorrect() { .flatExtracting(BidRequest::getImp) .extracting(Imp::getBidfloor, Imp::getBidfloorcur) .containsOnly(tuple(BigDecimal.TEN, "USD")); - assertThat(result.getValue()) - .extracting(HttpRequest::getPayload) - .extracting(BidRequest::getImp) - .containsOnly(singletonList( - givenImp(impBuilder -> impBuilder - .bidfloorcur("USD") - .bidfloor(BigDecimal.TEN) - .tagid("publisherId")))); } @Test