diff --git a/src/main/java/org/prebid/server/bidder/openx/OpenxBidder.java b/src/main/java/org/prebid/server/bidder/openx/OpenxBidder.java index 6483b491a61..1ddb092a5e7 100644 --- a/src/main/java/org/prebid/server/bidder/openx/OpenxBidder.java +++ b/src/main/java/org/prebid/server/bidder/openx/OpenxBidder.java @@ -16,7 +16,6 @@ import org.prebid.server.bidder.model.BidderError; import org.prebid.server.bidder.model.HttpRequest; import org.prebid.server.bidder.model.Result; -import org.prebid.server.bidder.openx.model.OpenxImpType; import org.prebid.server.bidder.openx.proto.OpenxBidExt; import org.prebid.server.bidder.openx.proto.OpenxRequestExt; import org.prebid.server.bidder.openx.proto.OpenxVideoExt; @@ -43,7 +42,6 @@ import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; -import java.util.stream.Stream; public class OpenxBidder implements Bidder { @@ -68,18 +66,17 @@ public OpenxBidder(String endpointUrl, JacksonMapper mapper) { @Override public Result>> makeHttpRequests(BidRequest bidRequest) { - final Map> differentiatedImps = bidRequest.getImp().stream() - .collect(Collectors.groupingBy(OpenxBidder::resolveImpType)); + final Map> partitionedImps = bidRequest.getImp().stream() + .filter(Objects::nonNull) + .collect(Collectors.partitioningBy(OpenxBidder::isSupportedImpType)); final List processingErrors = new ArrayList<>(); final List outgoingRequests = makeRequests( bidRequest, - differentiatedImps.get(OpenxImpType.banner), - differentiatedImps.get(OpenxImpType.video), - differentiatedImps.get(OpenxImpType.xNative), + partitionedImps.get(Boolean.TRUE), processingErrors); - final List errors = errors(differentiatedImps.get(OpenxImpType.other), processingErrors); + final List errors = errors(partitionedImps.get(Boolean.FALSE), processingErrors); return Result.of(createHttpRequests(outgoingRequests), errors); } @@ -94,45 +91,20 @@ public Result> makeBids(BidderCall httpCall, BidRequ } } + private static boolean isSupportedImpType(Imp imp) { + return imp.getBanner() != null || imp.getVideo() != null || imp.getXNative() != null; + } + private List makeRequests( BidRequest bidRequest, - List bannerImps, - List videoImps, - List nativeImps, + List imps, List errors) { - final List bidRequests = new ArrayList<>(); - // single request for all banner and native imps - final List bannerAndNativeImps = Stream.of(bannerImps, nativeImps) - .filter(Objects::nonNull) - .flatMap(Collection::stream) - .toList(); - final BidRequest bannerAndNativeImpsRequest = createSingleRequest(bannerAndNativeImps, bidRequest, errors); - if (bannerAndNativeImpsRequest != null) { - bidRequests.add(bannerAndNativeImpsRequest); - } - - if (CollectionUtils.isNotEmpty(videoImps)) { - // single request for each video imp - bidRequests.addAll(videoImps.stream() - .map(Collections::singletonList) - .map(imps -> createSingleRequest(imps, bidRequest, errors)) - .filter(Objects::nonNull) - .toList()); - } - return bidRequests; - } - private static OpenxImpType resolveImpType(Imp imp) { - if (imp.getBanner() != null) { - return OpenxImpType.banner; - } - if (imp.getVideo() != null) { - return OpenxImpType.video; - } - if (imp.getXNative() != null) { - return OpenxImpType.xNative; + final BidRequest request = createSingleRequest(imps, bidRequest, errors); + if (request != null) { + return Collections.singletonList(request); } - return OpenxImpType.other; + return Collections.emptyList(); } private static BidType resolveBidType(Imp imp) { @@ -178,23 +150,30 @@ private BidRequest createSingleRequest(List imps, BidRequest bidRequest, Li return null; } - List processedImps = null; - try { - processedImps = imps.stream().map(this::makeImp).toList(); - } catch (PreBidException e) { - errors.add(BidderError.badInput(e.getMessage())); + final List processedImps = new ArrayList<>(); + ExtRequest requestExt = null; + for (Imp imp : imps) { + try { + final ExtPrebid impExt = parseOpenxExt(imp); + processedImps.add(makeImp(imp, impExt)); + + if (requestExt == null) { + requestExt = makeReqExt(impExt.getBidder()); + } + } catch (PreBidException e) { + errors.add(BidderError.badInput("imp id=%s: %s".formatted(imp.getId(), e.getMessage()))); + } } return CollectionUtils.isNotEmpty(processedImps) ? bidRequest.toBuilder() .imp(processedImps) - .ext(makeReqExt(imps.getFirst())) + .ext(requestExt) .build() : null; } - private Imp makeImp(Imp imp) { - final ExtPrebid impExt = parseOpenxExt(imp); + private Imp makeImp(Imp imp, ExtPrebid impExt) { final ExtImpOpenx openxImpExt = impExt.getBidder(); final ExtImpPrebid prebidImpExt = impExt.getPrebid(); final Imp.ImpBuilder impBuilder = imp.toBuilder() @@ -202,7 +181,7 @@ private Imp makeImp(Imp imp) { .bidfloor(resolveBidFloor(imp.getBidfloor(), openxImpExt.getCustomFloor())) .ext(makeImpExt(imp.getExt(), MapUtils.isNotEmpty(openxImpExt.getCustomParams()))); - if (resolveImpType(imp) == OpenxImpType.video + if (imp.getVideo() != null && prebidImpExt != null && Objects.equals(prebidImpExt.getIsRewardedInventory(), 1)) { impBuilder.video(imp.getVideo().toBuilder() @@ -218,8 +197,7 @@ private static BigDecimal resolveBidFloor(BigDecimal impBidFloor, BigDecimal cus : impBidFloor; } - private ExtRequest makeReqExt(Imp imp) { - final ExtImpOpenx openxImpExt = parseOpenxExt(imp).getBidder(); + private ExtRequest makeReqExt(ExtImpOpenx openxImpExt) { return mapper.fillExtension( ExtRequest.empty(), OpenxRequestExt.of(openxImpExt.getDelDomain(), openxImpExt.getPlatform(), OPENX_CONFIG)); diff --git a/src/main/java/org/prebid/server/bidder/openx/model/OpenxImpType.java b/src/main/java/org/prebid/server/bidder/openx/model/OpenxImpType.java deleted file mode 100644 index c872e7f97e6..00000000000 --- a/src/main/java/org/prebid/server/bidder/openx/model/OpenxImpType.java +++ /dev/null @@ -1,9 +0,0 @@ -package org.prebid.server.bidder.openx.model; - -public enum OpenxImpType { - - // supported - banner, video, xNative, - // not supported - other -} diff --git a/src/test/java/org/prebid/server/bidder/openx/OpenxBidderTest.java b/src/test/java/org/prebid/server/bidder/openx/OpenxBidderTest.java index f5ba6094c17..74888aa1d68 100644 --- a/src/test/java/org/prebid/server/bidder/openx/OpenxBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/openx/OpenxBidderTest.java @@ -99,6 +99,7 @@ public void makeHttpRequestsShouldReturnResultWithErrorWhenImpExtOmitted() { // given final BidRequest bidRequest = BidRequest.builder() .imp(singletonList(Imp.builder() + .id("impId1") .banner(Banner.builder().build()) .build())) .build(); @@ -109,7 +110,7 @@ public void makeHttpRequestsShouldReturnResultWithErrorWhenImpExtOmitted() { // then assertThat(result.getValue()).isEmpty(); assertThat(result.getErrors()).hasSize(1) - .containsExactly(BidderError.badInput("openx parameters section is missing")); + .containsExactly(BidderError.badInput("imp id=impId1: openx parameters section is missing")); } @Test @@ -117,6 +118,7 @@ public void makeHttpRequestsShouldReturnResultWithErrorWhenImpExtMalformed() { // given final BidRequest bidRequest = BidRequest.builder() .imp(singletonList(Imp.builder() + .id("impId1") .banner(Banner.builder().build()) .ext(mapper.createObjectNode()) .build())) @@ -128,7 +130,7 @@ public void makeHttpRequestsShouldReturnResultWithErrorWhenImpExtMalformed() { // then assertThat(result.getValue()).isEmpty(); assertThat(result.getErrors()).hasSize(1) - .containsExactly(BidderError.badInput("openx parameters section is missing")); + .containsExactly(BidderError.badInput("imp id=impId1: openx parameters section is missing")); } @Test @@ -136,6 +138,7 @@ public void makeHttpRequestsShouldReturnResultWithErrorWhenImpExtOpenxEmpty() { // given final BidRequest bidRequest = BidRequest.builder() .imp(singletonList(Imp.builder() + .id("impId1") .video(Video.builder().build()) .ext(mapper.valueToTree( ExtPrebid.of(null, null))) @@ -148,7 +151,7 @@ public void makeHttpRequestsShouldReturnResultWithErrorWhenImpExtOpenxEmpty() { // then assertThat(result.getValue()).isEmpty(); assertThat(result.getErrors()).hasSize(1) - .containsExactly(BidderError.badInput("openx parameters section is missing")); + .containsExactly(BidderError.badInput("imp id=impId1: openx parameters section is missing")); } @Test @@ -156,6 +159,7 @@ public void makeHttpRequestsShouldReturnResultWithErrorWhenImpExtOpenxMalformed( // given final BidRequest bidRequest = BidRequest.builder() .imp(singletonList(Imp.builder() + .id("impId1") .banner(Banner.builder().build()) .ext(mapper.valueToTree(ExtPrebid.of(null, mapper.createArrayNode()))) .build())) @@ -167,7 +171,7 @@ public void makeHttpRequestsShouldReturnResultWithErrorWhenImpExtOpenxMalformed( // then assertThat(result.getValue()).isEmpty(); assertThat(result.getErrors().getFirst().getMessage()) - .startsWith("Cannot deserialize value of"); + .startsWith("imp id=impId1: Cannot deserialize value of"); } @Test @@ -233,10 +237,9 @@ public void makeHttpRequestsShouldReturnResultWithExpectedFieldsSet() { .containsExactly(BidderError.badInput( "OpenX only supports banner, video and native imps. Ignoring imp id=impId1")); - assertThat(result.getValue()).hasSize(3) + assertThat(result.getValue()).hasSize(1) .extracting(httpRequest -> mapper.readValue(httpRequest.getBody(), BidRequest.class)) .containsExactly( - // check if all banner imps are part of single bidRequest BidRequest.builder() .id("bidRequestId") .imp(asList( @@ -262,46 +265,20 @@ public void makeHttpRequestsShouldReturnResultWithExpectedFieldsSet() { .customParams( givenCustomParams("foo2", "bar2")) .build())) - .build())) - .ext(jacksonMapper.fillExtension( - ExtRequest.empty(), - OpenxRequestExt.of("se-demo-d.openx.net", null, "hb_pbs_1.0.0"))) - .user(User.builder() - .ext(ExtUser.builder().consent("consent").build()) - .build()) - .regs(Regs.builder().coppa(0).ext(ExtRegs.of(1, null, null, null)).build()) - .build(), - // check if each of video imps is a part of separate bidRequest and impId3 is rewarded video - BidRequest.builder() - .id("bidRequestId") - .imp(singletonList( + .build(), Imp.builder() .id("impId3") .video(Video.builder() .ext(mapper.valueToTree(OpenxVideoExt.of(1))) .build()) .tagid("555555") - // check if each of video imps is a part of separate bidRequest .bidfloor(BigDecimal.valueOf(0.1)) .ext(mapper.valueToTree( ExtImpOpenx.builder() .customParams( givenCustomParams("foo3", "bar3")) .build())) - .build())) - - .ext(jacksonMapper.fillExtension( - ExtRequest.empty(), - OpenxRequestExt.of("se-demo-d.openx.net", null, "hb_pbs_1.0.0"))) - .user(User.builder() - .ext(ExtUser.builder().consent("consent").build()) - .build()) - .regs(Regs.builder().coppa(0).ext(ExtRegs.of(1, null, null, null)).build()) - .build(), - // check if each of video imps is a part of separate bidRequest - BidRequest.builder() - .id("bidRequestId") - .imp(singletonList( + .build(), Imp.builder() .id("impId4") .video(Video.builder().build()) @@ -313,7 +290,8 @@ public void makeHttpRequestsShouldReturnResultWithExpectedFieldsSet() { .build())) .build())) .ext(jacksonMapper.fillExtension( - ExtRequest.empty(), OpenxRequestExt.of(null, "PLATFORM", "hb_pbs_1.0.0"))) + ExtRequest.empty(), + OpenxRequestExt.of("se-demo-d.openx.net", null, "hb_pbs_1.0.0"))) .user(User.builder() .ext(ExtUser.builder().consent("consent").build()) .build()) @@ -322,38 +300,29 @@ public void makeHttpRequestsShouldReturnResultWithExpectedFieldsSet() { } @Test - public void makeHttpRequestsShouldReturnResultWithSingleBidRequestForMultipleBannerAndNativeImps() { + public void makeHttpRequestsShouldReturnResultWithSingleBidRequestForMultipleImpsWithDifferentFormat() { // given final BidRequest bidRequest = BidRequest.builder() .id("bidRequestId") .imp(asList( Imp.builder() - .id("impId4") - .banner(Banner.builder().build()) + .id("impId1") + .banner(Banner.builder().w(320).h(200).build()) .ext(mapper.valueToTree( - ExtPrebid.of(null, - ExtImpOpenx.builder() - .customParams(givenCustomParams("foo4", "bar4")) - .delDomain("se-demo-d.openx.net") - .unit("4").build()))).build(), + ExtPrebid.of(null, ExtImpOpenx.builder().unit("1").build()))) + .build(), Imp.builder() - .id("impId5") - .xNative(Native.builder().request("{\"testreq\":1}").build()) + .id("impId2") + .xNative(Native.builder().request("{\"version\":1}").build()) .ext(mapper.valueToTree( - ExtPrebid.of(null, - ExtImpOpenx.builder() - .customParams(givenCustomParams("foo5", "bar5")) - .delDomain("se-demo-d.openx.net") - .unit("5").build()))).build(), + ExtPrebid.of(null, ExtImpOpenx.builder().unit("2").build()))) + .build(), Imp.builder() - .id("impId6") - .xNative(Native.builder().build()) + .id("impId3") + .video(Video.builder().maxduration(10).build()) .ext(mapper.valueToTree( - ExtPrebid.of(null, - ExtImpOpenx.builder() - .customParams(givenCustomParams("foo6", "bar6")) - .delDomain("se-demo-d.openx.net") - .unit("6").build()))).build())) + ExtPrebid.of(null, ExtImpOpenx.builder().unit("3").build()))) + .build())) .user(User.builder().ext(ExtUser.builder().consent("consent").build()).build()) .regs(Regs.builder().coppa(0).ext(ExtRegs.of(1, null, null, null)).build()) .build(); @@ -367,43 +336,29 @@ public void makeHttpRequestsShouldReturnResultWithSingleBidRequestForMultipleBan assertThat(result.getValue()).hasSize(1) .extracting(httpRequest -> mapper.readValue(httpRequest.getBody(), BidRequest.class)) .containsExactly( - // check if all native and banner imps are part of single bidRequest BidRequest.builder() .id("bidRequestId") .imp(asList( Imp.builder() - .id("impId4") - .tagid("4") - .banner(Banner.builder().build()) - .ext(mapper.valueToTree( - ExtImpOpenx.builder() - .customParams( - givenCustomParams("foo4", "bar4")) - .build())) - .build(), + .id("impId1") + .tagid("1") + .banner(Banner.builder().w(320).h(200).build()) + .ext(mapper.valueToTree(ExtImpOpenx.builder().build())).build(), Imp.builder() - .id("impId5") - .tagid("5") - .xNative(Native.builder().request("{\"testreq\":1}").build()) - .ext(mapper.valueToTree( - ExtImpOpenx.builder() - .customParams( - givenCustomParams("foo5", "bar5")) - .build())) + .id("impId2") + .tagid("2") + .xNative(Native.builder().request("{\"version\":1}").build()) + .ext(mapper.valueToTree(ExtImpOpenx.builder().build())) .build(), Imp.builder() - .id("impId6") - .tagid("6") - .xNative(Native.builder().build()) - .ext(mapper.valueToTree( - ExtImpOpenx.builder() - .customParams( - givenCustomParams("foo6", "bar6")) - .build())) + .id("impId3") + .tagid("3") + .video(Video.builder().maxduration(10).build()) + .ext(mapper.valueToTree(ExtImpOpenx.builder().build())) .build())) .ext(jacksonMapper.fillExtension( ExtRequest.empty(), - OpenxRequestExt.of("se-demo-d.openx.net", null, "hb_pbs_1.0.0"))) + OpenxRequestExt.of(null, null, "hb_pbs_1.0.0"))) .user(User.builder() .ext(ExtUser.builder().consent("consent").build()) .build()) @@ -412,67 +367,122 @@ public void makeHttpRequestsShouldReturnResultWithSingleBidRequestForMultipleBan } @Test - public void makeHttpRequestsShouldReturnResultWithSingleBidRequestForMultiFormatImps() { + public void makeHttpRequestsShouldSkipMalformedFirstImpAndDeriveRequestExtFromLaterValidImp() { // given final BidRequest bidRequest = BidRequest.builder() .id("bidRequestId") .imp(asList( Imp.builder() - .id("impId1") - .banner(Banner.builder().w(320).h(200).build()) - .video(Video.builder().maxduration(10).build()) - .ext(mapper.valueToTree( - ExtPrebid.of(null, ExtImpOpenx.builder().unit("1").build()))) + .id("badImp") + .banner(Banner.builder().build()) .build(), Imp.builder() - .id("impId2") - .banner(Banner.builder().w(300).h(150).build()) - .xNative(Native.builder().request("{\"version\":1}").build()) + .id("anotherBadImp") + .banner(Banner.builder().build()) + .build(), + Imp.builder() + .id("goodImp") + .banner(Banner.builder().build()) .ext(mapper.valueToTree( - ExtPrebid.of(null, ExtImpOpenx.builder().unit("2").build()))) + ExtPrebid.of(null, + ExtImpOpenx.builder() + .delDomain("se-demo-d.openx.net") + .platform("PLATFORM") + .unit("555555").build()))) .build())) - .user(User.builder().ext(ExtUser.builder().consent("consent").build()).build()) - .regs(Regs.builder().coppa(0).ext(ExtRegs.of(1, null, null, null)).build()) .build(); // when final Result>> result = target.makeHttpRequests(bidRequest); // then - assertThat(result.getErrors()).isEmpty(); + assertThat(result.getErrors()).hasSize(2) + .containsExactly( + BidderError.badInput("imp id=badImp: openx parameters section is missing"), + BidderError.badInput("imp id=anotherBadImp: openx parameters section is missing")); assertThat(result.getValue()).hasSize(1) .extracting(httpRequest -> mapper.readValue(httpRequest.getBody(), BidRequest.class)) .containsExactly( - // check if all native and banner imps are part of single bidRequest BidRequest.builder() .id("bidRequestId") - .imp(asList( - // verify banner and video media types are preserved in a single imp - Imp.builder() - .id("impId1") - .tagid("1") - .banner(Banner.builder().w(320).h(200).build()) - .video(Video.builder().maxduration(10).build()) - .ext(mapper.valueToTree(ExtImpOpenx.builder().build())).build(), - // verify banner and native media types are preserved in a single imp + .imp(singletonList( Imp.builder() - .id("impId2") - .tagid("2") - .banner(Banner.builder().w(300).h(150).build()) - .xNative(Native.builder().request("{\"version\":1}").build()) + .id("goodImp") + .banner(Banner.builder().build()) + .tagid("555555") .ext(mapper.valueToTree(ExtImpOpenx.builder().build())) .build())) .ext(jacksonMapper.fillExtension( ExtRequest.empty(), - OpenxRequestExt.of(null, null, "hb_pbs_1.0.0"))) - .user(User.builder() - .ext(ExtUser.builder().consent("consent").build()) - .build()) - .regs(Regs.builder().coppa(0).ext(ExtRegs.of(1, null, null, null)).build()) + OpenxRequestExt.of("se-demo-d.openx.net", "PLATFORM", "hb_pbs_1.0.0"))) .build()); } + @Test + public void makeHttpRequestsShouldAttachRewardedVideoExtWhenImpHasBothBannerAndVideo() { + // given + final BidRequest bidRequest = BidRequest.builder() + .id("bidRequestId") + .imp(singletonList(Imp.builder() + .id("impId1") + .banner(Banner.builder().build()) + .video(Video.builder().build()) + .ext(mapper.valueToTree( + ExtPrebid.of( + ExtImpPrebid.builder().isRewardedInventory(1).build(), + ExtImpOpenx.builder().unit("1").build()))) + .build())) + .build(); + + // when + final Result>> result = target.makeHttpRequests(bidRequest); + + // then + assertThat(result.getErrors()).isEmpty(); + assertThat(result.getValue()).hasSize(1) + .extracting(httpRequest -> mapper.readValue(httpRequest.getBody(), BidRequest.class)) + .flatExtracting(BidRequest::getImp) + .containsExactly(Imp.builder() + .id("impId1") + .tagid("1") + .banner(Banner.builder().build()) + .video(Video.builder().ext(mapper.valueToTree(OpenxVideoExt.of(1))).build()) + .ext(mapper.valueToTree(ExtImpOpenx.builder().build())) + .build()); + } + + @Test + public void makeHttpRequestsShouldNotAttachRewardedVideoExtWhenImpHasNoVideo() { + // given + final BidRequest bidRequest = BidRequest.builder() + .id("bidRequestId") + .imp(singletonList(Imp.builder() + .id("impId1") + .banner(Banner.builder().build()) + .ext(mapper.valueToTree( + ExtPrebid.of( + ExtImpPrebid.builder().isRewardedInventory(1).build(), + ExtImpOpenx.builder().unit("1").build()))) + .build())) + .build(); + + // when + final Result>> result = target.makeHttpRequests(bidRequest); + + // then + assertThat(result.getErrors()).isEmpty(); + assertThat(result.getValue()).hasSize(1) + .extracting(httpRequest -> mapper.readValue(httpRequest.getBody(), BidRequest.class)) + .flatExtracting(BidRequest::getImp) + .containsExactly(Imp.builder() + .id("impId1") + .tagid("1") + .banner(Banner.builder().build()) + .ext(mapper.valueToTree(ExtImpOpenx.builder().build())) + .build()); + } + @Test public void makeHttpRequestsShouldPassThroughImpExt() { // given