From 05362cfcd761ea6cd47f1f6b05de15f169223dbc Mon Sep 17 00:00:00 2001 From: antonbabak Date: Wed, 3 Jan 2024 11:51:04 +0100 Subject: [PATCH 1/6] Move prefmtype to ext.prebid.biddercontrols --- .../mediatypeprocessor/MultiFormatMediaTypeProcessor.java | 2 +- .../server/proto/openrtb/ext/request/ExtRequestPrebid.java | 5 +++++ .../MultiFormatMediaTypeProcessorTest.java | 6 +++--- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java b/src/main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java index 7786d01dc42..d6d1d263ac9 100644 --- a/src/main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java +++ b/src/main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java @@ -60,7 +60,7 @@ private boolean isMultiFormatSupported(String bidder) { private MediaType preferredMediaType(BidRequest bidRequest, Account account, String bidderName) { return Optional.ofNullable(bidRequest.getExt()) .map(ExtRequest::getPrebid) - .map(ExtRequestPrebid::getBidders) + .map(ExtRequestPrebid::getBiddercontrols) .map(bidders -> bidders.at(PREF_MTYPE_FIELD)) .filter(JsonNode::isTextual) .map(JsonNode::textValue) diff --git a/src/main/java/org/prebid/server/proto/openrtb/ext/request/ExtRequestPrebid.java b/src/main/java/org/prebid/server/proto/openrtb/ext/request/ExtRequestPrebid.java index 94032078663..0e7005b81b8 100644 --- a/src/main/java/org/prebid/server/proto/openrtb/ext/request/ExtRequestPrebid.java +++ b/src/main/java/org/prebid/server/proto/openrtb/ext/request/ExtRequestPrebid.java @@ -110,6 +110,11 @@ public class ExtRequestPrebid { */ ObjectNode bidders; + /** + * Defines the contract for bidrequest.ext.prebid.biddercontrols + */ + ObjectNode biddercontrols; + /** * Defines the contract for bidrequest.ext.prebid.amp */ diff --git a/src/test/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessorTest.java b/src/test/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessorTest.java index 240026b703b..55ee5964cbe 100644 --- a/src/test/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessorTest.java +++ b/src/test/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessorTest.java @@ -113,12 +113,12 @@ public void processShouldUseRequestLevelPreferredMediaTypeFirst() { // given given(bidderCatalog.bidderInfoByName(BIDDER)).willReturn(givenBidderInfo(false)); - final ObjectNode bidders = mapper.createObjectNode(); - bidders.putObject("bidder").put("prefmtype", "video"); + final ObjectNode bidderControls = mapper.createObjectNode(); + bidderControls.putObject("bidder").put("prefmtype", "video"); final BidRequest bidRequest = givenBidRequest( request -> request.ext(ExtRequest.of(ExtRequestPrebid.builder() - .bidders(bidders) + .biddercontrols(bidderControls) .build())), givenImp(BANNER, VIDEO, AUDIO, NATIVE)); From cf32a694d3e4b4b1b3989d3f99bb3df43f87908a Mon Sep 17 00:00:00 2001 From: Markiyan Mykush <95693607+marki1an@users.noreply.github.com> Date: Wed, 3 Jan 2024 15:07:23 +0200 Subject: [PATCH 2/6] Tests: Update path for preferredMediaType feature (#2876) --- ...redBidders.groovy => BidderControls.groovy} | 3 +-- .../model/request/auction/Prebid.groovy | 2 +- .../tests/FilterMultiFormatSpec.groovy | 18 +++++++++--------- 3 files changed, 11 insertions(+), 12 deletions(-) rename src/test/groovy/org/prebid/server/functional/model/request/auction/{PreferredBidders.groovy => BidderControls.groovy} (75%) diff --git a/src/test/groovy/org/prebid/server/functional/model/request/auction/PreferredBidders.groovy b/src/test/groovy/org/prebid/server/functional/model/request/auction/BidderControls.groovy similarity index 75% rename from src/test/groovy/org/prebid/server/functional/model/request/auction/PreferredBidders.groovy rename to src/test/groovy/org/prebid/server/functional/model/request/auction/BidderControls.groovy index 8defea34aa5..dc01fb12b3a 100644 --- a/src/test/groovy/org/prebid/server/functional/model/request/auction/PreferredBidders.groovy +++ b/src/test/groovy/org/prebid/server/functional/model/request/auction/BidderControls.groovy @@ -3,8 +3,7 @@ package org.prebid.server.functional.model.request.auction import groovy.transform.ToString @ToString(includeNames = true, ignoreNulls = true) -class PreferredBidders { +class BidderControls { GenericPreferredBidder generic - GenericPreferredBidder bidder } diff --git a/src/test/groovy/org/prebid/server/functional/model/request/auction/Prebid.groovy b/src/test/groovy/org/prebid/server/functional/model/request/auction/Prebid.groovy index 15bc90a72d9..9afc9551a06 100644 --- a/src/test/groovy/org/prebid/server/functional/model/request/auction/Prebid.groovy +++ b/src/test/groovy/org/prebid/server/functional/model/request/auction/Prebid.groovy @@ -35,7 +35,7 @@ class Prebid { Boolean createTids Sdk sdk List adServerTargeting - PreferredBidders bidders + BidderControls bidderControls static class Channel { diff --git a/src/test/groovy/org/prebid/server/functional/tests/FilterMultiFormatSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/FilterMultiFormatSpec.groovy index 1940c3ba805..c105aaef867 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/FilterMultiFormatSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/FilterMultiFormatSpec.groovy @@ -9,7 +9,7 @@ import org.prebid.server.functional.model.request.auction.Banner import org.prebid.server.functional.model.request.auction.BidRequest import org.prebid.server.functional.model.request.auction.GenericPreferredBidder import org.prebid.server.functional.model.request.auction.Native -import org.prebid.server.functional.model.request.auction.PreferredBidders +import org.prebid.server.functional.model.request.auction.BidderControls import static org.prebid.server.functional.model.response.auction.ErrorType.GENERIC import static org.prebid.server.functional.model.response.auction.MediaType.AUDIO @@ -52,7 +52,7 @@ class FilterMultiFormatSpec extends BaseSpec { def bidRequest = BidRequest.defaultBidRequest.tap { imp[0].banner = Banner.defaultBanner imp[0].audio = Audio.defaultAudio - ext.prebid.bidders = new PreferredBidders(generic: new GenericPreferredBidder(preferredMediaType: BANNER)) + ext.prebid.bidderControls = new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER)) } when: "PBS processes auction request" @@ -98,7 +98,7 @@ class FilterMultiFormatSpec extends BaseSpec { def bidRequest = BidRequest.defaultBidRequest.tap { imp[0].banner = Banner.defaultBanner imp[0].audio = Audio.defaultAudio - ext.prebid.bidders = new PreferredBidders(generic: new GenericPreferredBidder(preferredMediaType: BANNER)) + ext.prebid.bidderControls = new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER)) } when: "PBS processes auction request" @@ -119,7 +119,7 @@ class FilterMultiFormatSpec extends BaseSpec { def bidRequest = BidRequest.defaultBidRequest.tap { imp[0].banner = Banner.defaultBanner imp[0].audio = Audio.defaultAudio - ext.prebid.bidders = new PreferredBidders(generic: new GenericPreferredBidder(preferredMediaType: BANNER)) + ext.prebid.bidderControls = new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER)) } when: "PBS processes auction request" @@ -190,7 +190,7 @@ class FilterMultiFormatSpec extends BaseSpec { def bidRequest = BidRequest.defaultBidRequest.tap { imp[0].banner = Banner.defaultBanner imp[0].audio = Audio.defaultAudio - ext.prebid.bidders = new PreferredBidders(generic: new GenericPreferredBidder(preferredMediaType: BANNER)) + ext.prebid.bidderControls = new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER)) } when: "PBS processes auction request" @@ -241,7 +241,7 @@ class FilterMultiFormatSpec extends BaseSpec { imp[0].banner = null imp[0].audio = Audio.defaultAudio imp[0].nativeObj = Native.defaultNative - ext.prebid.bidders = new PreferredBidders(generic: new GenericPreferredBidder(preferredMediaType: BANNER)) + ext.prebid.bidderControls = new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER)) } when: "PBS processes auction request" @@ -292,7 +292,7 @@ class FilterMultiFormatSpec extends BaseSpec { def bidRequest = BidRequest.defaultBidRequest.tap { imp[0].banner = null imp[0].audio = Audio.defaultAudio - ext.prebid.bidders = new PreferredBidders(generic: new GenericPreferredBidder(preferredMediaType: BANNER)) + ext.prebid.bidderControls = new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER)) } when: "PBS processes auction request" @@ -315,7 +315,7 @@ class FilterMultiFormatSpec extends BaseSpec { def bidRequest = BidRequest.defaultBidRequest.tap { imp[0].banner = Banner.getDefaultBanner() imp[0].audio = Audio.defaultAudio - ext.prebid.bidders = new PreferredBidders(generic: new GenericPreferredBidder(preferredMediaType: NULL)) + ext.prebid.bidderControls = new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: NULL)) } when: "PBS processes auction request" @@ -364,7 +364,7 @@ class FilterMultiFormatSpec extends BaseSpec { def bidRequest = BidRequest.defaultBidRequest.tap { imp[0].banner = Banner.defaultBanner imp[0].audio = Audio.defaultAudio - ext.prebid.bidders = new PreferredBidders(generic: new GenericPreferredBidder(preferredMediaType: BANNER)) + ext.prebid.bidderControls = new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER)) } and: "Account in the DB with preferred media type" From f987a303b064f7cdd9d7b2493f86ee825b66a0e7 Mon Sep 17 00:00:00 2001 From: antonbabak Date: Wed, 3 Jan 2024 15:44:49 +0100 Subject: [PATCH 3/6] Fixes --- checkstyle.xml | 4 +- .../server/auction/ExchangeService.java | 76 ++++++++++++------- .../server/auction/ExchangeServiceTest.java | 22 ++++-- 3 files changed, 69 insertions(+), 33 deletions(-) diff --git a/checkstyle.xml b/checkstyle.xml index f20b1bb51b1..aac9ec01cfe 100644 --- a/checkstyle.xml +++ b/checkstyle.xml @@ -8,7 +8,9 @@ - + + + diff --git a/src/main/java/org/prebid/server/auction/ExchangeService.java b/src/main/java/org/prebid/server/auction/ExchangeService.java index 8f5e108cb8d..571620c1582 100644 --- a/src/main/java/org/prebid/server/auction/ExchangeService.java +++ b/src/main/java/org/prebid/server/auction/ExchangeService.java @@ -758,6 +758,23 @@ private boolean isUserEidAllowed(String source, Map> eidPer || EID_ALLOWED_FOR_ALL_BIDDERS.equals(allowedBidder)); } + /** + * Extracts a map of bidders to their arguments from {@link ObjectNode} prebid.bidders. or prebid.biddercontrols. + */ + private static Map bidderToPrebidBidders(ObjectNode bidders) { + if (bidders == null || bidders.isNull()) { + return Collections.emptyMap(); + } + + final Map bidderToPrebidParameters = new HashMap<>(); + final Iterator> biddersToParams = bidders.fields(); + while (biddersToParams.hasNext()) { + final Map.Entry bidderToParam = biddersToParams.next(); + bidderToPrebidParameters.put(bidderToParam.getKey(), bidderToParam.getValue()); + } + return bidderToPrebidParameters; + } + /** * Returns shuffled list of {@link AuctionParticipation} with {@link BidRequest}. */ @@ -771,7 +788,11 @@ private List getAuctionParticipation( BidderAliases aliases, AuctionContext context) { - final Map bidderToPrebidBidders = bidderToPrebidBidders(bidRequest); + final ExtRequestPrebid prebid = extRequestPrebid(bidRequest); + final Map bidderToPrebidBidders = bidderToPrebidBidders( + prebid == null ? null : prebid.getBidders()); + final Map bidderToPrebidBidderControls = bidderToPrebidBidders( + prebid == null ? null : prebid.getBiddercontrols()); final List bidderRequests = bidderPrivacyResults.stream() // for each bidder create a new request that is a copy of original request except buyerid, imp // extensions, ext.prebid.data.bidders and ext.prebid.bidders. @@ -783,6 +804,7 @@ private List getAuctionParticipation( bidderToMultiBid, biddersToConfigs, bidderToPrebidBidders, + bidderToPrebidBidderControls, aliases, context)) // Can't be removed after we prepare workflow to filter blocked @@ -793,26 +815,6 @@ private List getAuctionParticipation( return bidderRequests; } - /** - * Extracts a map of bidders to their arguments from {@link ObjectNode} prebid.bidders. - */ - private static Map bidderToPrebidBidders(BidRequest bidRequest) { - final ExtRequestPrebid prebid = extRequestPrebid(bidRequest); - final ObjectNode bidders = prebid == null ? null : prebid.getBidders(); - - if (bidders == null || bidders.isNull()) { - return Collections.emptyMap(); - } - - final Map bidderToPrebidParameters = new HashMap<>(); - final Iterator> biddersToParams = bidders.fields(); - while (biddersToParams.hasNext()) { - final Map.Entry bidderToParam = biddersToParams.next(); - bidderToPrebidParameters.put(bidderToParam.getKey(), bidderToParam.getValue()); - } - return bidderToPrebidParameters; - } - /** * Returns {@link AuctionParticipation} for the given bidder. */ @@ -823,6 +825,7 @@ private AuctionParticipation createAuctionParticipation( Map bidderToMultiBid, Map biddersToConfigs, Map bidderToPrebidBidders, + Map bidderToPrebidBidderControls, BidderAliases bidderAliases, AuctionContext context) { @@ -852,6 +855,7 @@ private AuctionParticipation createAuctionParticipation( bidderToMultiBid, biddersToConfigs, bidderToPrebidBidders, + bidderToPrebidBidderControls, context); final BidderRequest bidderRequest = BidderRequest.builder() @@ -878,6 +882,7 @@ private BidRequest prepareBidRequest(BidderPrivacyResult bidderPrivacyResult, Map bidderToMultiBid, Map biddersToConfigs, Map bidderToPrebidBidders, + Map bidderToPrebidBidderControls, AuctionContext context) { final BidRequest bidRequest = context.getBidRequest(); @@ -927,6 +932,13 @@ private BidRequest prepareBidRequest(BidderPrivacyResult bidderPrivacyResult, final boolean isDooh = !isApp && preparedDooh != null; final boolean isSite = !isApp && !isDooh && preparedSite != null; + final ExtRequest extRequest = prepareExt( + bidder, + bidderToPrebidBidders, + bidderToPrebidBidderControls, + bidderToMultiBid, + bidRequest.getExt()); + return bidRequest.toBuilder() // User was already prepared above .user(bidderPrivacyResult.getUser()) @@ -936,7 +948,7 @@ private BidRequest prepareBidRequest(BidderPrivacyResult bidderPrivacyResult, .dooh(isDooh ? preparedDooh : null) .site(isSite ? preparedSite : null) .source(prepareSource(bidder, bidRequest, transmitTid)) - .ext(prepareExt(bidder, bidderToPrebidBidders, bidderToMultiBid, bidRequest.getExt())) + .ext(extRequest) .build(); } @@ -1150,13 +1162,15 @@ private Source prepareSource(String bidder, BidRequest bidRequest, boolean trans } /** - * Removes all bidders except the given bidder from bidrequest.ext.prebid.bidders to hide list of allowed bidders - * from initial request. + * Removes all bidders except the given bidder + * from bidrequest.ext.prebid.bidders and bidrequest.ext.prebid.biddercontrols + * to hide list of allowed bidders from initial request. *

* Also masks bidrequest.ext.prebid.schains. */ private ExtRequest prepareExt(String bidder, Map bidderToPrebidBidders, + Map bidderToPrebidBidderControls, Map bidderToMultiBid, ExtRequest requestExt) { @@ -1173,6 +1187,7 @@ private ExtRequest prepareExt(String bidder, final boolean suppressAliases = extPrebid != null && extPrebid.getAliases() != null; if (bidderToPrebidBidders.isEmpty() + && bidderToPrebidBidderControls.isEmpty() && bidderToMultiBid.isEmpty() && !suppressSchains && !suppressBidderConfig @@ -1183,10 +1198,16 @@ private ExtRequest prepareExt(String bidder, return requestExt; } - final JsonNode prebidParameters = bidderToPrebidBidders.get(bidder); - final ObjectNode bidders = prebidParameters != null - ? mapper.mapper().valueToTree(ExtPrebidBidders.of(prebidParameters)) + final JsonNode bidderParameter = bidderToPrebidBidders.get(bidder); + final ObjectNode bidders = bidderParameter != null + ? mapper.mapper().valueToTree(ExtPrebidBidders.of(bidderParameter)) + : null; + + final JsonNode bidderControlsParameter = bidderToPrebidBidderControls.get(bidder); + final ObjectNode bidderControls = bidderControlsParameter != null + ? mapper.mapper().valueToTree(ExtPrebidBidders.of(bidderControlsParameter)) : null; + final ExtRequestPrebid.ExtRequestPrebidBuilder extPrebidBuilder = Optional.ofNullable(extPrebid) .map(ExtRequestPrebid::toBuilder) .orElse(ExtRequestPrebid.builder()); @@ -1194,6 +1215,7 @@ private ExtRequest prepareExt(String bidder, return ExtRequest.of(extPrebidBuilder .multibid(resolveExtRequestMultiBids(bidderToMultiBid.get(bidder), bidder)) .bidders(bidders) + .biddercontrols(bidderControls) .bidderparams(prepareBidderParameters(extPrebid, bidder)) .schains(null) .data(null) diff --git a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java index 50f1b56285d..98305d8e37b 100644 --- a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java +++ b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java @@ -854,11 +854,15 @@ public void shouldPassRequestWithExtPrebidToDefinedBidder() { givenBidder(bidder2Name, bidder2, givenEmptySeatBid()); final ExtRequest extRequest = ExtRequest.of( - ExtRequestPrebid.builder() - .bidders(mapper.createObjectNode() - .putPOJO(bidder1Name, mapper.createObjectNode().put("test1", "test1")) - .putPOJO(bidder2Name, mapper.createObjectNode().put("test2", "test2")) - .putPOJO("spam", mapper.createObjectNode().put("spam", "spam"))) + ExtRequestPrebid.builder() + .bidders(mapper.createObjectNode() + .putPOJO(bidder1Name, mapper.createObjectNode().put("test1", "test1")) + .putPOJO(bidder2Name, mapper.createObjectNode().put("test2", "test2")) + .putPOJO("spam", mapper.createObjectNode().put("spam", "spam"))) + .biddercontrols(mapper.createObjectNode() + .putPOJO(bidder1Name, mapper.createObjectNode().put("test1", "test1")) + .putPOJO(bidder2Name, mapper.createObjectNode().put("test2", "test2")) + .putPOJO("spam", mapper.createObjectNode().put("spam", "spam"))) .auctiontimestamp(1000L) .build()); @@ -882,6 +886,10 @@ public void shouldPassRequestWithExtPrebidToDefinedBidder() { assertThat(bidders1).isNotNull(); assertThat(bidders1.fields()).toIterable().hasSize(1) .containsOnly(entry("bidder", mapper.createObjectNode().put("test1", "test1"))); + final JsonNode bidderControls1 = prebid1.getBiddercontrols(); + assertThat(bidderControls1).isNotNull(); + assertThat(bidderControls1.fields()).toIterable().hasSize(1) + .containsOnly(entry("bidder", mapper.createObjectNode().put("test1", "test1"))); final ArgumentCaptor bidRequest2Captor = ArgumentCaptor.forClass(BidderRequest.class); verify(httpBidderRequester) @@ -893,6 +901,10 @@ public void shouldPassRequestWithExtPrebidToDefinedBidder() { assertThat(bidders2).isNotNull(); assertThat(bidders2.fields()).toIterable().hasSize(1) .containsOnly(entry("bidder", mapper.createObjectNode().put("test2", "test2"))); + final JsonNode bidderControls2 = prebid2.getBiddercontrols(); + assertThat(bidderControls2).isNotNull(); + assertThat(bidderControls2.fields()).toIterable().hasSize(1) + .containsOnly(entry("bidder", mapper.createObjectNode().put("test2", "test2"))); } @Test From 879a09c1c327a753fde38253fdb8e279574b0801 Mon Sep 17 00:00:00 2001 From: antonbabak Date: Wed, 3 Jan 2024 17:05:14 +0100 Subject: [PATCH 4/6] Fixes 2 --- .../MultiFormatMediaTypeProcessor.java | 17 ++++++++++++++--- .../MultiFormatMediaTypeProcessorTest.java | 9 +++++++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java b/src/main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java index d6d1d263ac9..8b6ee49c7d0 100644 --- a/src/main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java +++ b/src/main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java @@ -30,13 +30,24 @@ public MultiFormatMediaTypeProcessor(BidderCatalog bidderCatalog) { @Override public MediaTypeProcessingResult process(BidRequest bidRequest, String bidderName, Account account) { + final BidRequest.BidRequestBuilder bidRequestBuilder = Optional.ofNullable(bidRequest.getExt()) + .map(ExtRequest::getPrebid) + .map(prebid -> prebid.toBuilder().biddercontrols(null).build()) + .map(prebid -> { + final ExtRequest extRequest = ExtRequest.of(prebid); + extRequest.addProperties(bidRequest.getExt().getProperties()); + return extRequest; + }) + .map(extRequest -> bidRequest.toBuilder().ext(extRequest)) + .orElse(bidRequest.toBuilder()); + if (isMultiFormatSupported(bidderName)) { - return MediaTypeProcessingResult.succeeded(bidRequest, Collections.emptyList()); + return MediaTypeProcessingResult.succeeded(bidRequestBuilder.build(), Collections.emptyList()); } final MediaType preferredMediaType = preferredMediaType(bidRequest, account, bidderName); if (preferredMediaType == null) { - return MediaTypeProcessingResult.succeeded(bidRequest, Collections.emptyList()); + return MediaTypeProcessingResult.succeeded(bidRequestBuilder.build(), Collections.emptyList()); } final List errors = new ArrayList<>(); @@ -50,7 +61,7 @@ public MediaTypeProcessingResult process(BidRequest bidRequest, String bidderNam return MediaTypeProcessingResult.rejected(errors); } - return MediaTypeProcessingResult.succeeded(bidRequest.toBuilder().imp(updatedImps).build(), errors); + return MediaTypeProcessingResult.succeeded(bidRequestBuilder.imp(updatedImps).build(), errors); } private boolean isMultiFormatSupported(String bidder) { diff --git a/src/test/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessorTest.java b/src/test/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessorTest.java index 55ee5964cbe..4a4b4da4a60 100644 --- a/src/test/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessorTest.java +++ b/src/test/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessorTest.java @@ -69,7 +69,7 @@ public void processShouldReturnSameBidRequestIfMultiFormatSupported() { // then assertThat(result.isRejected()).isFalse(); - assertThat(result.getBidRequest()).isSameAs(bidRequest); + assertThat(result.getBidRequest()).isEqualTo(bidRequest); assertThat(result.getErrors()).isEmpty(); } @@ -85,7 +85,7 @@ public void processShouldReturnSameBidRequestIfPreferredMediaTypeNotSpecified() // then assertThat(result.isRejected()).isFalse(); - assertThat(result.getBidRequest()).isSameAs(bidRequest); + assertThat(result.getBidRequest()).isEqualTo(bidRequest); assertThat(result.getErrors()).isEmpty(); } @@ -133,6 +133,11 @@ public void processShouldUseRequestLevelPreferredMediaTypeFirst() { .extracting(BidRequest::getImp) .asInstanceOf(InstanceOfAssertFactories.list(Imp.class)) .containsExactly(givenImp(VIDEO)); + assertThat(result.getBidRequest()) + .extracting(BidRequest::getExt) + .extracting(ExtRequest::getPrebid) + .extracting(ExtRequestPrebid::getBidderconfig) + .isNull(); assertThat(result.getErrors()).isEmpty(); } From 8f42d65ae1afa8bc50300539bde0717ee307b665 Mon Sep 17 00:00:00 2001 From: antonbabak Date: Thu, 4 Jan 2024 10:49:02 +0100 Subject: [PATCH 5/6] Comments Fixes --- .../server/auction/ExchangeService.java | 80 +++++++------------ .../BidderMediaTypeProcessor.java | 9 ++- .../CompositeMediaTypeProcessor.java | 12 ++- .../MediaTypeProcessor.java | 3 +- .../MultiFormatMediaTypeProcessor.java | 23 ++++-- .../server/auction/ExchangeServiceTest.java | 26 ++---- .../BidderMediaTypeProcessorTest.java | 16 ++-- .../CompositeMediaTypeProcessorTest.java | 23 +++++- .../MultiFormatMediaTypeProcessorTest.java | 29 ++++--- 9 files changed, 117 insertions(+), 104 deletions(-) diff --git a/src/main/java/org/prebid/server/auction/ExchangeService.java b/src/main/java/org/prebid/server/auction/ExchangeService.java index 571620c1582..b9235895428 100644 --- a/src/main/java/org/prebid/server/auction/ExchangeService.java +++ b/src/main/java/org/prebid/server/auction/ExchangeService.java @@ -758,23 +758,6 @@ private boolean isUserEidAllowed(String source, Map> eidPer || EID_ALLOWED_FOR_ALL_BIDDERS.equals(allowedBidder)); } - /** - * Extracts a map of bidders to their arguments from {@link ObjectNode} prebid.bidders. or prebid.biddercontrols. - */ - private static Map bidderToPrebidBidders(ObjectNode bidders) { - if (bidders == null || bidders.isNull()) { - return Collections.emptyMap(); - } - - final Map bidderToPrebidParameters = new HashMap<>(); - final Iterator> biddersToParams = bidders.fields(); - while (biddersToParams.hasNext()) { - final Map.Entry bidderToParam = biddersToParams.next(); - bidderToPrebidParameters.put(bidderToParam.getKey(), bidderToParam.getValue()); - } - return bidderToPrebidParameters; - } - /** * Returns shuffled list of {@link AuctionParticipation} with {@link BidRequest}. */ @@ -788,11 +771,7 @@ private List getAuctionParticipation( BidderAliases aliases, AuctionContext context) { - final ExtRequestPrebid prebid = extRequestPrebid(bidRequest); - final Map bidderToPrebidBidders = bidderToPrebidBidders( - prebid == null ? null : prebid.getBidders()); - final Map bidderToPrebidBidderControls = bidderToPrebidBidders( - prebid == null ? null : prebid.getBiddercontrols()); + final Map bidderToPrebidBidders = bidderToPrebidBidders(bidRequest); final List bidderRequests = bidderPrivacyResults.stream() // for each bidder create a new request that is a copy of original request except buyerid, imp // extensions, ext.prebid.data.bidders and ext.prebid.bidders. @@ -804,7 +783,6 @@ private List getAuctionParticipation( bidderToMultiBid, biddersToConfigs, bidderToPrebidBidders, - bidderToPrebidBidderControls, aliases, context)) // Can't be removed after we prepare workflow to filter blocked @@ -815,6 +793,26 @@ private List getAuctionParticipation( return bidderRequests; } + /** + * Extracts a map of bidders to their arguments from {@link ObjectNode} prebid.bidders. + */ + private static Map bidderToPrebidBidders(BidRequest bidRequest) { + final ExtRequestPrebid prebid = extRequestPrebid(bidRequest); + final ObjectNode bidders = prebid == null ? null : prebid.getBidders(); + + if (bidders == null || bidders.isNull()) { + return Collections.emptyMap(); + } + + final Map bidderToPrebidParameters = new HashMap<>(); + final Iterator> biddersToParams = bidders.fields(); + while (biddersToParams.hasNext()) { + final Map.Entry bidderToParam = biddersToParams.next(); + bidderToPrebidParameters.put(bidderToParam.getKey(), bidderToParam.getValue()); + } + return bidderToPrebidParameters; + } + /** * Returns {@link AuctionParticipation} for the given bidder. */ @@ -825,7 +823,6 @@ private AuctionParticipation createAuctionParticipation( Map bidderToMultiBid, Map biddersToConfigs, Map bidderToPrebidBidders, - Map bidderToPrebidBidderControls, BidderAliases bidderAliases, AuctionContext context) { @@ -855,7 +852,6 @@ private AuctionParticipation createAuctionParticipation( bidderToMultiBid, biddersToConfigs, bidderToPrebidBidders, - bidderToPrebidBidderControls, context); final BidderRequest bidderRequest = BidderRequest.builder() @@ -882,7 +878,6 @@ private BidRequest prepareBidRequest(BidderPrivacyResult bidderPrivacyResult, Map bidderToMultiBid, Map biddersToConfigs, Map bidderToPrebidBidders, - Map bidderToPrebidBidderControls, AuctionContext context) { final BidRequest bidRequest = context.getBidRequest(); @@ -932,13 +927,6 @@ private BidRequest prepareBidRequest(BidderPrivacyResult bidderPrivacyResult, final boolean isDooh = !isApp && preparedDooh != null; final boolean isSite = !isApp && !isDooh && preparedSite != null; - final ExtRequest extRequest = prepareExt( - bidder, - bidderToPrebidBidders, - bidderToPrebidBidderControls, - bidderToMultiBid, - bidRequest.getExt()); - return bidRequest.toBuilder() // User was already prepared above .user(bidderPrivacyResult.getUser()) @@ -948,7 +936,7 @@ private BidRequest prepareBidRequest(BidderPrivacyResult bidderPrivacyResult, .dooh(isDooh ? preparedDooh : null) .site(isSite ? preparedSite : null) .source(prepareSource(bidder, bidRequest, transmitTid)) - .ext(extRequest) + .ext(prepareExt(bidder, bidderToPrebidBidders, bidderToMultiBid, bidRequest.getExt())) .build(); } @@ -1162,15 +1150,13 @@ private Source prepareSource(String bidder, BidRequest bidRequest, boolean trans } /** - * Removes all bidders except the given bidder - * from bidrequest.ext.prebid.bidders and bidrequest.ext.prebid.biddercontrols - * to hide list of allowed bidders from initial request. + * Removes all bidders except the given bidder from bidrequest.ext.prebid.bidders to hide list of allowed bidders + * from initial request. *

* Also masks bidrequest.ext.prebid.schains. */ private ExtRequest prepareExt(String bidder, Map bidderToPrebidBidders, - Map bidderToPrebidBidderControls, Map bidderToMultiBid, ExtRequest requestExt) { @@ -1187,7 +1173,6 @@ private ExtRequest prepareExt(String bidder, final boolean suppressAliases = extPrebid != null && extPrebid.getAliases() != null; if (bidderToPrebidBidders.isEmpty() - && bidderToPrebidBidderControls.isEmpty() && bidderToMultiBid.isEmpty() && !suppressSchains && !suppressBidderConfig @@ -1198,16 +1183,10 @@ private ExtRequest prepareExt(String bidder, return requestExt; } - final JsonNode bidderParameter = bidderToPrebidBidders.get(bidder); - final ObjectNode bidders = bidderParameter != null - ? mapper.mapper().valueToTree(ExtPrebidBidders.of(bidderParameter)) + final JsonNode prebidParameters = bidderToPrebidBidders.get(bidder); + final ObjectNode bidders = prebidParameters != null + ? mapper.mapper().valueToTree(ExtPrebidBidders.of(prebidParameters)) : null; - - final JsonNode bidderControlsParameter = bidderToPrebidBidderControls.get(bidder); - final ObjectNode bidderControls = bidderControlsParameter != null - ? mapper.mapper().valueToTree(ExtPrebidBidders.of(bidderControlsParameter)) - : null; - final ExtRequestPrebid.ExtRequestPrebidBuilder extPrebidBuilder = Optional.ofNullable(extPrebid) .map(ExtRequestPrebid::toBuilder) .orElse(ExtRequestPrebid.builder()); @@ -1215,7 +1194,6 @@ private ExtRequest prepareExt(String bidder, return ExtRequest.of(extPrebidBuilder .multibid(resolveExtRequestMultiBids(bidderToMultiBid.get(bidder), bidder)) .bidders(bidders) - .biddercontrols(bidderControls) .bidderparams(prepareBidderParameters(extPrebid, bidder)) .schains(null) .data(null) @@ -1328,18 +1306,16 @@ private Future processAndRequestBids(AuctionContext auctionConte final String bidderName = bidderRequest.getBidder(); final MediaTypeProcessingResult mediaTypeProcessingResult = mediaTypeProcessor.process( - bidderRequest.getBidRequest(), aliases.resolveBidder(bidderName), auctionContext.getAccount()); + bidderRequest.getBidRequest(), bidderName, aliases, auctionContext.getAccount()); final List mediaTypeProcessingErrors = mediaTypeProcessingResult.getErrors(); if (mediaTypeProcessingResult.isRejected()) { auctionContext.getBidRejectionTrackers() .get(bidderName) .rejectAll(BidRejectionReason.REJECTED_BY_MEDIA_TYPE); - final BidderSeatBid bidderSeatBid = BidderSeatBid.builder() .warnings(mediaTypeProcessingErrors) .build(); - return Future.succeededFuture(BidderResponse.of(bidderName, bidderSeatBid, 0)); } diff --git a/src/main/java/org/prebid/server/auction/mediatypeprocessor/BidderMediaTypeProcessor.java b/src/main/java/org/prebid/server/auction/mediatypeprocessor/BidderMediaTypeProcessor.java index 8f89f7fe498..0171ff90b68 100644 --- a/src/main/java/org/prebid/server/auction/mediatypeprocessor/BidderMediaTypeProcessor.java +++ b/src/main/java/org/prebid/server/auction/mediatypeprocessor/BidderMediaTypeProcessor.java @@ -3,6 +3,7 @@ import com.iab.openrtb.request.BidRequest; import com.iab.openrtb.request.Imp; import org.apache.commons.collections4.SetUtils; +import org.prebid.server.auction.BidderAliases; import org.prebid.server.bidder.BidderCatalog; import org.prebid.server.bidder.BidderInfo; import org.prebid.server.bidder.model.BidderError; @@ -35,8 +36,12 @@ public BidderMediaTypeProcessor(BidderCatalog bidderCatalog) { } @Override - public MediaTypeProcessingResult process(BidRequest bidRequest, String bidderName, Account account) { - final Set supportedMediaTypes = extractSupportedMediaTypes(bidRequest, bidderName); + public MediaTypeProcessingResult process(BidRequest bidRequest, + String bidderName, + BidderAliases aliases, + Account account) { + final String resolvedBidderName = aliases.resolveBidder(bidderName); + final Set supportedMediaTypes = extractSupportedMediaTypes(bidRequest, resolvedBidderName); if (supportedMediaTypes.isEmpty()) { return MediaTypeProcessingResult.rejected(Collections.singletonList( BidderError.badInput("Bidder does not support any media types."))); diff --git a/src/main/java/org/prebid/server/auction/mediatypeprocessor/CompositeMediaTypeProcessor.java b/src/main/java/org/prebid/server/auction/mediatypeprocessor/CompositeMediaTypeProcessor.java index b1376052562..c51d35eb33f 100644 --- a/src/main/java/org/prebid/server/auction/mediatypeprocessor/CompositeMediaTypeProcessor.java +++ b/src/main/java/org/prebid/server/auction/mediatypeprocessor/CompositeMediaTypeProcessor.java @@ -1,6 +1,7 @@ package org.prebid.server.auction.mediatypeprocessor; import com.iab.openrtb.request.BidRequest; +import org.prebid.server.auction.BidderAliases; import org.prebid.server.bidder.model.BidderError; import org.prebid.server.settings.model.Account; @@ -17,12 +18,19 @@ public CompositeMediaTypeProcessor(List mediaTypeProcessors) } @Override - public MediaTypeProcessingResult process(BidRequest originalBidRequest, String bidderName, Account account) { + public MediaTypeProcessingResult process(BidRequest originalBidRequest, + String bidderName, + BidderAliases aliases, + Account account) { BidRequest bidRequest = originalBidRequest; final List errors = new ArrayList<>(); for (MediaTypeProcessor mediaTypeProcessor : mediaTypeProcessors) { - final MediaTypeProcessingResult result = mediaTypeProcessor.process(bidRequest, bidderName, account); + final MediaTypeProcessingResult result = mediaTypeProcessor.process( + bidRequest, + bidderName, + aliases, + account); bidRequest = result.getBidRequest(); errors.addAll(result.getErrors()); diff --git a/src/main/java/org/prebid/server/auction/mediatypeprocessor/MediaTypeProcessor.java b/src/main/java/org/prebid/server/auction/mediatypeprocessor/MediaTypeProcessor.java index fb15437d92b..89e7f07589b 100644 --- a/src/main/java/org/prebid/server/auction/mediatypeprocessor/MediaTypeProcessor.java +++ b/src/main/java/org/prebid/server/auction/mediatypeprocessor/MediaTypeProcessor.java @@ -1,9 +1,10 @@ package org.prebid.server.auction.mediatypeprocessor; import com.iab.openrtb.request.BidRequest; +import org.prebid.server.auction.BidderAliases; import org.prebid.server.settings.model.Account; public interface MediaTypeProcessor { - MediaTypeProcessingResult process(BidRequest bidRequest, String bidderName, Account account); + MediaTypeProcessingResult process(BidRequest bidRequest, String bidderName, BidderAliases aliases, Account account); } diff --git a/src/main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java b/src/main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java index 8b6ee49c7d0..15459823d9a 100644 --- a/src/main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java +++ b/src/main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java @@ -4,6 +4,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.iab.openrtb.request.BidRequest; import com.iab.openrtb.request.Imp; +import org.prebid.server.auction.BidderAliases; import org.prebid.server.bidder.BidderCatalog; import org.prebid.server.bidder.model.BidderError; import org.prebid.server.proto.openrtb.ext.request.ExtRequest; @@ -20,7 +21,7 @@ public class MultiFormatMediaTypeProcessor implements MediaTypeProcessor { - private static final JsonPointer PREF_MTYPE_FIELD = JsonPointer.compile("/bidder/prefmtype"); + private static final String PREF_MTYPE_FIELD = "prefmtype"; private final BidderCatalog bidderCatalog; @@ -29,7 +30,11 @@ public MultiFormatMediaTypeProcessor(BidderCatalog bidderCatalog) { } @Override - public MediaTypeProcessingResult process(BidRequest bidRequest, String bidderName, Account account) { + public MediaTypeProcessingResult process(BidRequest bidRequest, + String bidderName, + BidderAliases aliases, + Account account) { + final String resolvedBidderName = aliases.resolveBidder(bidderName); final BidRequest.BidRequestBuilder bidRequestBuilder = Optional.ofNullable(bidRequest.getExt()) .map(ExtRequest::getPrebid) .map(prebid -> prebid.toBuilder().biddercontrols(null).build()) @@ -40,12 +45,11 @@ public MediaTypeProcessingResult process(BidRequest bidRequest, String bidderNam }) .map(extRequest -> bidRequest.toBuilder().ext(extRequest)) .orElse(bidRequest.toBuilder()); - - if (isMultiFormatSupported(bidderName)) { + if (isMultiFormatSupported(resolvedBidderName)) { return MediaTypeProcessingResult.succeeded(bidRequestBuilder.build(), Collections.emptyList()); } - final MediaType preferredMediaType = preferredMediaType(bidRequest, account, bidderName); + final MediaType preferredMediaType = preferredMediaType(bidRequest, account, bidderName, resolvedBidderName); if (preferredMediaType == null) { return MediaTypeProcessingResult.succeeded(bidRequestBuilder.build(), Collections.emptyList()); } @@ -68,17 +72,20 @@ private boolean isMultiFormatSupported(String bidder) { return bidderCatalog.bidderInfoByName(bidder).getOrtb().isMultiFormatSupported(); } - private MediaType preferredMediaType(BidRequest bidRequest, Account account, String bidderName) { + private MediaType preferredMediaType(BidRequest bidRequest, + Account account, + String originalBidderName, + String resolvedBidderName) { return Optional.ofNullable(bidRequest.getExt()) .map(ExtRequest::getPrebid) .map(ExtRequestPrebid::getBiddercontrols) - .map(bidders -> bidders.at(PREF_MTYPE_FIELD)) + .map(bidders -> bidders.at(JsonPointer.compile("/" + originalBidderName + "/" + PREF_MTYPE_FIELD))) .filter(JsonNode::isTextual) .map(JsonNode::textValue) .map(MediaType::of) .or(() -> Optional.ofNullable(account.getAuction()) .map(AccountAuctionConfig::getPreferredMediaTypes) - .map(preferredMediaTypes -> preferredMediaTypes.get(bidderName))) + .map(preferredMediaTypes -> preferredMediaTypes.get(resolvedBidderName))) .orElse(null); } diff --git a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java index 98305d8e37b..10b33eb246c 100644 --- a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java +++ b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java @@ -368,7 +368,7 @@ public void setUp() { false, AuctionResponsePayloadImpl.of(invocation.getArgument(0))))); - given(mediaTypeProcessor.process(any(), anyString(), any())) + given(mediaTypeProcessor.process(any(), anyString(), any(), any())) .willAnswer(invocation -> MediaTypeProcessingResult.succeeded(invocation.getArgument(0), emptyList())); given(responseBidValidator.validate(any(), any(), any(), any())).willReturn(ValidationResult.success()); @@ -854,15 +854,11 @@ public void shouldPassRequestWithExtPrebidToDefinedBidder() { givenBidder(bidder2Name, bidder2, givenEmptySeatBid()); final ExtRequest extRequest = ExtRequest.of( - ExtRequestPrebid.builder() - .bidders(mapper.createObjectNode() - .putPOJO(bidder1Name, mapper.createObjectNode().put("test1", "test1")) - .putPOJO(bidder2Name, mapper.createObjectNode().put("test2", "test2")) - .putPOJO("spam", mapper.createObjectNode().put("spam", "spam"))) - .biddercontrols(mapper.createObjectNode() - .putPOJO(bidder1Name, mapper.createObjectNode().put("test1", "test1")) - .putPOJO(bidder2Name, mapper.createObjectNode().put("test2", "test2")) - .putPOJO("spam", mapper.createObjectNode().put("spam", "spam"))) + ExtRequestPrebid.builder() + .bidders(mapper.createObjectNode() + .putPOJO(bidder1Name, mapper.createObjectNode().put("test1", "test1")) + .putPOJO(bidder2Name, mapper.createObjectNode().put("test2", "test2")) + .putPOJO("spam", mapper.createObjectNode().put("spam", "spam"))) .auctiontimestamp(1000L) .build()); @@ -886,10 +882,6 @@ public void shouldPassRequestWithExtPrebidToDefinedBidder() { assertThat(bidders1).isNotNull(); assertThat(bidders1.fields()).toIterable().hasSize(1) .containsOnly(entry("bidder", mapper.createObjectNode().put("test1", "test1"))); - final JsonNode bidderControls1 = prebid1.getBiddercontrols(); - assertThat(bidderControls1).isNotNull(); - assertThat(bidderControls1.fields()).toIterable().hasSize(1) - .containsOnly(entry("bidder", mapper.createObjectNode().put("test1", "test1"))); final ArgumentCaptor bidRequest2Captor = ArgumentCaptor.forClass(BidderRequest.class); verify(httpBidderRequester) @@ -901,10 +893,6 @@ public void shouldPassRequestWithExtPrebidToDefinedBidder() { assertThat(bidders2).isNotNull(); assertThat(bidders2.fields()).toIterable().hasSize(1) .containsOnly(entry("bidder", mapper.createObjectNode().put("test2", "test2"))); - final JsonNode bidderControls2 = prebid2.getBiddercontrols(); - assertThat(bidderControls2).isNotNull(); - assertThat(bidderControls2.fields()).toIterable().hasSize(1) - .containsOnly(entry("bidder", mapper.createObjectNode().put("test2", "test2"))); } @Test @@ -4770,7 +4758,7 @@ public void shouldResponseWithEmptySeatBidIfBidderNotSupportProvidedMediaTypes() final BidRequest bidRequest = givenBidRequest(singletonList(imp), identity()); final AuctionContext auctionContext = givenRequestContext(bidRequest).toBuilder().build(); - given(mediaTypeProcessor.process(any(), anyString(), any())) + given(mediaTypeProcessor.process(any(), anyString(), any(), any())) .willReturn(MediaTypeProcessingResult.rejected(Collections.singletonList( BidderError.badInput("MediaTypeProcessor error.")))); given(bidResponseCreator.create( diff --git a/src/test/java/org/prebid/server/auction/mediatypeprocessor/BidderMediaTypeProcessorTest.java b/src/test/java/org/prebid/server/auction/mediatypeprocessor/BidderMediaTypeProcessorTest.java index ca5e00963be..165e9c63089 100644 --- a/src/test/java/org/prebid/server/auction/mediatypeprocessor/BidderMediaTypeProcessorTest.java +++ b/src/test/java/org/prebid/server/auction/mediatypeprocessor/BidderMediaTypeProcessorTest.java @@ -15,6 +15,7 @@ import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; import org.prebid.server.VertxTest; +import org.prebid.server.auction.BidderAliases; import org.prebid.server.auction.versionconverter.OrtbVersion; import org.prebid.server.bidder.BidderCatalog; import org.prebid.server.bidder.BidderInfo; @@ -32,7 +33,9 @@ import static java.util.Collections.singletonList; import static java.util.function.UnaryOperator.identity; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.when; import static org.prebid.server.spring.config.bidder.model.MediaType.AUDIO; import static org.prebid.server.spring.config.bidder.model.MediaType.BANNER; import static org.prebid.server.spring.config.bidder.model.MediaType.NATIVE; @@ -47,12 +50,15 @@ public class BidderMediaTypeProcessorTest extends VertxTest { @Mock private BidderCatalog bidderCatalog; + @Mock + private BidderAliases bidderAliases; private BidderMediaTypeProcessor target; @Before public void setUp() { target = new BidderMediaTypeProcessor(bidderCatalog); + when(bidderAliases.resolveBidder(anyString())).thenAnswer(invocation -> invocation.getArgument(0)); } @Test @@ -63,7 +69,7 @@ public void processShouldReturnRejectedResultAndErrorIfBidderNotSupportAnyMediaT final BidRequest bidRequest = givenBidRequest(identity(), givenImp(BANNER)); // when - final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, null); + final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, bidderAliases, null); // then assertThat(result.isRejected()).isTrue(); @@ -79,7 +85,7 @@ public void processShouldUseAppMediaTypesIfAppPresent() { .willReturn(givenBidderInfo(singletonList(BANNER), singletonList(AUDIO), singletonList(NATIVE))); // when - final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, null); + final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, bidderAliases, null); // then assertThat(result.getBidRequest()).isEqualTo(bidRequest); @@ -97,7 +103,7 @@ public void processShouldRemoveUnsupportedMediaTypes() { givenImp(BANNER, VIDEO)); // when - final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, null); + final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, bidderAliases, null); // then assertThat(result.getBidRequest()) @@ -118,7 +124,7 @@ public void processShouldRemoveImpWithOnlyUnsupportedMediaTypes() { givenImp(BANNER, AUDIO)); // when - final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, null); + final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, bidderAliases, null); // then assertThat(result.getBidRequest()) @@ -141,7 +147,7 @@ public void processShouldReturnRejectedResultIfRequestDoesNotContainsAnyImpWithS givenImp(NATIVE)); // when - final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, null); + final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, bidderAliases, null); // then assertThat(result.isRejected()).isTrue(); diff --git a/src/test/java/org/prebid/server/auction/mediatypeprocessor/CompositeMediaTypeProcessorTest.java b/src/test/java/org/prebid/server/auction/mediatypeprocessor/CompositeMediaTypeProcessorTest.java index 8a72cc28ffd..b0081898311 100644 --- a/src/test/java/org/prebid/server/auction/mediatypeprocessor/CompositeMediaTypeProcessorTest.java +++ b/src/test/java/org/prebid/server/auction/mediatypeprocessor/CompositeMediaTypeProcessorTest.java @@ -7,6 +7,7 @@ import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; +import org.prebid.server.auction.BidderAliases; import org.prebid.server.bidder.model.BidderError; import static java.util.Arrays.asList; @@ -17,6 +18,7 @@ import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; public class CompositeMediaTypeProcessorTest { @@ -29,17 +31,21 @@ public class CompositeMediaTypeProcessorTest { @Mock private MediaTypeProcessor mediaTypeProcessor2; + @Mock + private BidderAliases bidderAliases; + private CompositeMediaTypeProcessor target; @Before public void setUp() { target = new CompositeMediaTypeProcessor(asList(mediaTypeProcessor1, mediaTypeProcessor2)); + when(bidderAliases.resolveBidder(anyString())).thenAnswer(invocation -> invocation.getArgument(0)); } @Test public void processShouldReturnExpectedResult() { // given - given(mediaTypeProcessor1.process(any(), anyString(), any())) + given(mediaTypeProcessor1.process(any(), anyString(), any(), any())) .willReturn(MediaTypeProcessingResult.succeeded( BidRequest.builder().id("processed by mediaTypeProcessor1").build(), singletonList(BidderError.badInput("Error from mediaTypeProcessor1")))); @@ -47,13 +53,18 @@ public void processShouldReturnExpectedResult() { given(mediaTypeProcessor2.process( argThat(request -> request.getId().equals("processed by mediaTypeProcessor1")), anyString(), + any(), any())) .willReturn(MediaTypeProcessingResult.succeeded( BidRequest.builder().id("processed by mediaTypeProcessor2").build(), singletonList(BidderError.badInput("Error from mediaTypeProcessor2")))); // when - final MediaTypeProcessingResult result = target.process(BidRequest.builder().build(), "bidder", null); + final MediaTypeProcessingResult result = target.process( + BidRequest.builder().build(), + "bidder", + bidderAliases, + null); // then assertThat(result.isRejected()).isFalse(); @@ -68,12 +79,16 @@ public void processShouldReturnExpectedResult() { @Test public void processShouldReturnExpectedResultIfRejectedBySomeOfProcessors() { // given - given(mediaTypeProcessor1.process(any(), anyString(), any())) + given(mediaTypeProcessor1.process(any(), anyString(), any(), any())) .willReturn(MediaTypeProcessingResult.rejected( singletonList(BidderError.badInput("Error from mediaTypeProcessor1")))); // when - final MediaTypeProcessingResult result = target.process(BidRequest.builder().build(), "bidder", null); + final MediaTypeProcessingResult result = target.process( + BidRequest.builder().build(), + "bidder", + bidderAliases, + null); // then assertThat(result.isRejected()).isTrue(); diff --git a/src/test/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessorTest.java b/src/test/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessorTest.java index 4a4b4da4a60..430c67b9ff9 100644 --- a/src/test/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessorTest.java +++ b/src/test/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessorTest.java @@ -15,6 +15,7 @@ import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; import org.prebid.server.VertxTest; +import org.prebid.server.auction.BidderAliases; import org.prebid.server.auction.versionconverter.OrtbVersion; import org.prebid.server.bidder.BidderCatalog; import org.prebid.server.bidder.BidderInfo; @@ -35,7 +36,9 @@ import static java.util.Collections.emptyList; import static java.util.function.UnaryOperator.identity; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.when; import static org.prebid.server.spring.config.bidder.model.MediaType.AUDIO; import static org.prebid.server.spring.config.bidder.model.MediaType.BANNER; import static org.prebid.server.spring.config.bidder.model.MediaType.NATIVE; @@ -50,12 +53,15 @@ public class MultiFormatMediaTypeProcessorTest extends VertxTest { @Mock private BidderCatalog bidderCatalog; + @Mock + private BidderAliases bidderAliases; private MultiFormatMediaTypeProcessor target; @Before public void setUp() { target = new MultiFormatMediaTypeProcessor(bidderCatalog); + when(bidderAliases.resolveBidder(anyString())).thenAnswer(invocation -> invocation.getArgument(0)); } @Test @@ -65,7 +71,7 @@ public void processShouldReturnSameBidRequestIfMultiFormatSupported() { final BidRequest bidRequest = givenBidRequest(identity(), givenImp(BANNER, VIDEO)); // when - final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, null); + final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, bidderAliases, null); // then assertThat(result.isRejected()).isFalse(); @@ -81,7 +87,7 @@ public void processShouldReturnSameBidRequestIfPreferredMediaTypeNotSpecified() final Account account = givenAccount(null); // when - final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, account); + final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, bidderAliases, account); // then assertThat(result.isRejected()).isFalse(); @@ -97,7 +103,7 @@ public void processShouldReturnImpWithPreferredMediaType() { final Account account = givenAccount(Map.of(BIDDER, VIDEO)); // when - final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, account); + final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, bidderAliases, account); // then assertThat(result.isRejected()).isFalse(); @@ -111,10 +117,11 @@ public void processShouldReturnImpWithPreferredMediaType() { @Test public void processShouldUseRequestLevelPreferredMediaTypeFirst() { // given - given(bidderCatalog.bidderInfoByName(BIDDER)).willReturn(givenBidderInfo(false)); + given(bidderAliases.resolveBidder(BIDDER)).willReturn("resolvedBidderName"); + given(bidderCatalog.bidderInfoByName("resolvedBidderName")).willReturn(givenBidderInfo(false)); final ObjectNode bidderControls = mapper.createObjectNode(); - bidderControls.putObject("bidder").put("prefmtype", "video"); + bidderControls.putObject(BIDDER).put("prefmtype", "video"); final BidRequest bidRequest = givenBidRequest( request -> request.ext(ExtRequest.of(ExtRequestPrebid.builder() @@ -122,10 +129,10 @@ public void processShouldUseRequestLevelPreferredMediaTypeFirst() { .build())), givenImp(BANNER, VIDEO, AUDIO, NATIVE)); - final Account account = givenAccount(Map.of(BIDDER, AUDIO)); + final Account account = givenAccount(Map.of("resolvedBidderName", AUDIO)); // when - final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, account); + final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, bidderAliases, account); // then assertThat(result.isRejected()).isFalse(); @@ -136,7 +143,7 @@ public void processShouldUseRequestLevelPreferredMediaTypeFirst() { assertThat(result.getBidRequest()) .extracting(BidRequest::getExt) .extracting(ExtRequest::getPrebid) - .extracting(ExtRequestPrebid::getBidderconfig) + .extracting(ExtRequestPrebid::getBiddercontrols) .isNull(); assertThat(result.getErrors()).isEmpty(); } @@ -155,7 +162,7 @@ public void processShouldSkipImpsWithSingleMediaType() { final Account account = givenAccount(Map.of(BIDDER, VIDEO)); // when - final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, account); + final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, bidderAliases, account); // then assertThat(result.isRejected()).isFalse(); @@ -186,7 +193,7 @@ public void processShouldFilterMultiFormatImpsWithoutPreferredMediaType() { final Account account = givenAccount(Map.of(BIDDER, VIDEO)); // when - final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, account); + final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, bidderAliases, account); // then assertThat(result.isRejected()).isFalse(); @@ -214,7 +221,7 @@ public void processShouldRejectEmptyRequestAfterFiltering() { final Account account = givenAccount(Map.of(BIDDER, VIDEO)); // when - final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, account); + final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, bidderAliases, account); // then assertThat(result.isRejected()).isTrue(); From 121461268c9229d6ca848e306935df5a65ab6f70 Mon Sep 17 00:00:00 2001 From: antonbabak Date: Thu, 4 Jan 2024 11:44:54 +0100 Subject: [PATCH 6/6] Comments Fixes 2 --- .../mediatypeprocessor/MultiFormatMediaTypeProcessor.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java b/src/main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java index 15459823d9a..f4e89ea2f76 100644 --- a/src/main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java +++ b/src/main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java @@ -1,6 +1,5 @@ package org.prebid.server.auction.mediatypeprocessor; -import com.fasterxml.jackson.core.JsonPointer; import com.fasterxml.jackson.databind.JsonNode; import com.iab.openrtb.request.BidRequest; import com.iab.openrtb.request.Imp; @@ -35,6 +34,9 @@ public MediaTypeProcessingResult process(BidRequest bidRequest, BidderAliases aliases, Account account) { final String resolvedBidderName = aliases.resolveBidder(bidderName); + //todo: ext.prebid.biddercontrols clean-up should NOT be here + // Suggestion: keep biddercontrols in the Auction Context + // and clean it up on the extraction auction participants step final BidRequest.BidRequestBuilder bidRequestBuilder = Optional.ofNullable(bidRequest.getExt()) .map(ExtRequest::getPrebid) .map(prebid -> prebid.toBuilder().biddercontrols(null).build()) @@ -79,7 +81,8 @@ private MediaType preferredMediaType(BidRequest bidRequest, return Optional.ofNullable(bidRequest.getExt()) .map(ExtRequest::getPrebid) .map(ExtRequestPrebid::getBiddercontrols) - .map(bidders -> bidders.at(JsonPointer.compile("/" + originalBidderName + "/" + PREF_MTYPE_FIELD))) + .map(bidders -> bidders.get(originalBidderName)) + .map(bidder -> bidder.get(PREF_MTYPE_FIELD)) .filter(JsonNode::isTextual) .map(JsonNode::textValue) .map(MediaType::of)