From 1df9c8459e9a90f4956803dd144e713a42199d00 Mon Sep 17 00:00:00 2001 From: antonbabak Date: Thu, 22 Aug 2024 10:04:44 +0200 Subject: [PATCH 1/6] Fix merging imp.ext.prebid.imp into imp --- .../server/auction/ExchangeService.java | 1 + .../server/auction/ExchangeServiceTest.java | 23 ++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/prebid/server/auction/ExchangeService.java b/src/main/java/org/prebid/server/auction/ExchangeService.java index d97d0d582ab..8c243ee866e 100644 --- a/src/main/java/org/prebid/server/auction/ExchangeService.java +++ b/src/main/java/org/prebid/server/auction/ExchangeService.java @@ -981,6 +981,7 @@ private List prepareImps(String bidder, return bidRequest.getImp().stream() .filter(imp -> bidderParamsFromImpExt(imp.getExt()).hasNonNull(bidder)) + .map(imp -> imp.toBuilder().ext(imp.getExt().deepCopy()).build()) .map(imp -> impAdjuster.adjust(imp, bidder, bidderAliases, debugWarnings)) .map(imp -> prepareImp(imp, bidder, bidRequest, transmitTid, useFirstPartyData, account, debugWarnings)) .toList(); diff --git a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java index 40dd296723b..20a2535b2ea 100644 --- a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java +++ b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java @@ -186,6 +186,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; import static org.assertj.core.api.Assertions.tuple; +import static org.mockito.ArgumentCaptor.forClass; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; @@ -501,12 +502,14 @@ public void shouldExtractRequestWithBidderSpecificExtension() { // given givenBidder(givenEmptySeatBid()); - final BidRequest bidRequest = givenBidRequest(singletonList( - givenImp(singletonMap("someBidder", 1), builder -> builder - .id("impId") - .banner(Banner.builder() - .format(singletonList(Format.builder().w(400).h(300).build())) - .build()))), + final Imp givenImp = givenImp(singletonMap("someBidder", 1), builder -> builder + .id("impId") + .banner(Banner.builder() + .format(singletonList(Format.builder().w(400).h(300).build())) + .build())); + + final BidRequest bidRequest = givenBidRequest( + singletonList(givenImp), builder -> builder.id("requestId").tmax(500L)); // when @@ -526,6 +529,14 @@ public void shouldExtractRequestWithBidderSpecificExtension() { .build())) .tmax(500L) .build()); + + final ArgumentCaptor impCaptor = forClass(Imp.class); + verify(impAdjuster).adjust(impCaptor.capture(), eq("someBidder"), any(), any()); + + final Imp actualImp = impCaptor.getValue(); + assertThat(actualImp).isNotSameAs(givenImp); + assertThat(actualImp.getExt()).isNotSameAs(givenImp.getExt()); + assertThat(actualImp).isEqualTo(givenImp); } @Test From a8827b254f837c5951ee39a798bd264209a019eb Mon Sep 17 00:00:00 2001 From: antonbabak Date: Thu, 22 Aug 2024 10:08:57 +0200 Subject: [PATCH 2/6] Fix checkstyle --- src/main/java/org/prebid/server/auction/ExchangeService.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/prebid/server/auction/ExchangeService.java b/src/main/java/org/prebid/server/auction/ExchangeService.java index 8c243ee866e..7b1547cbc18 100644 --- a/src/main/java/org/prebid/server/auction/ExchangeService.java +++ b/src/main/java/org/prebid/server/auction/ExchangeService.java @@ -378,7 +378,6 @@ private static BidRequestCacheInfo bidRequestCacheInfo(BidRequest bidRequest) { .build(); } } - return BidRequestCacheInfo.noCache(); } From e5bae388d75f2506898370c4f884c39d37929d1b Mon Sep 17 00:00:00 2001 From: antonbabak Date: Thu, 22 Aug 2024 11:40:32 +0200 Subject: [PATCH 3/6] Remove unnecessary deep copy --- .../org/prebid/server/auction/ExchangeService.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/prebid/server/auction/ExchangeService.java b/src/main/java/org/prebid/server/auction/ExchangeService.java index 7b1547cbc18..a0bedea8cc7 100644 --- a/src/main/java/org/prebid/server/auction/ExchangeService.java +++ b/src/main/java/org/prebid/server/auction/ExchangeService.java @@ -1017,17 +1017,16 @@ private ObjectNode prepareImpExt(String bidder, boolean transmitTid, boolean useFirstPartyData) { - final ObjectNode modifiedImpExt = impExt.deepCopy(); final JsonNode impExtPrebid = prepareImpExt(impExt.get(PREBID_EXT), adjustedFloor); Optional.ofNullable(impExtPrebid).ifPresentOrElse( - ext -> modifiedImpExt.set(PREBID_EXT, ext), - () -> modifiedImpExt.remove(PREBID_EXT)); - modifiedImpExt.set(BIDDER_EXT, bidderParamsFromImpExt(impExt).get(bidder)); + ext -> impExt.set(PREBID_EXT, ext), + () -> impExt.remove(PREBID_EXT)); + impExt.set(BIDDER_EXT, bidderParamsFromImpExt(impExt).get(bidder)); if (!transmitTid) { - modifiedImpExt.remove(TID_EXT); + impExt.remove(TID_EXT); } - return fpdResolver.resolveImpExt(modifiedImpExt, useFirstPartyData); + return fpdResolver.resolveImpExt(impExt, useFirstPartyData); } private JsonNode prepareImpExt(JsonNode extImpPrebidNode, BigDecimal adjustedFloor) { From 233fbd14d5f4037e415c6846d2c03331720a0e37 Mon Sep 17 00:00:00 2001 From: antonbabak Date: Thu, 22 Aug 2024 11:48:56 +0200 Subject: [PATCH 4/6] Remove unnecessary deep copy --- src/main/java/org/prebid/server/auction/ExchangeService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/prebid/server/auction/ExchangeService.java b/src/main/java/org/prebid/server/auction/ExchangeService.java index a0bedea8cc7..8ccada983ac 100644 --- a/src/main/java/org/prebid/server/auction/ExchangeService.java +++ b/src/main/java/org/prebid/server/auction/ExchangeService.java @@ -1016,12 +1016,12 @@ private ObjectNode prepareImpExt(String bidder, BigDecimal adjustedFloor, boolean transmitTid, boolean useFirstPartyData) { - + final JsonNode bidderNode = bidderParamsFromImpExt(impExt).get(bidder); final JsonNode impExtPrebid = prepareImpExt(impExt.get(PREBID_EXT), adjustedFloor); Optional.ofNullable(impExtPrebid).ifPresentOrElse( ext -> impExt.set(PREBID_EXT, ext), () -> impExt.remove(PREBID_EXT)); - impExt.set(BIDDER_EXT, bidderParamsFromImpExt(impExt).get(bidder)); + impExt.set(BIDDER_EXT, bidderNode); if (!transmitTid) { impExt.remove(TID_EXT); } From bb2afc78426ba6cb8a2f747936d623fb27be64dc Mon Sep 17 00:00:00 2001 From: antonbabak Date: Thu, 22 Aug 2024 13:00:07 +0200 Subject: [PATCH 5/6] Fix unit tests --- .../org/prebid/server/auction/ExchangeServiceTest.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java index 20a2535b2ea..ffff9684fb1 100644 --- a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java +++ b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java @@ -512,6 +512,10 @@ public void shouldExtractRequestWithBidderSpecificExtension() { singletonList(givenImp), builder -> builder.id("requestId").tmax(500L)); + final ObjectNode adjustedExt = givenImp.getExt().deepCopy(); + final Imp adjustedImp = givenImp.toBuilder().ext(adjustedExt).build(); + given(impAdjuster.adjust(any(), any(), any(), any())).willReturn(adjustedImp); + // when target.holdAuction(givenRequestContext(bidRequest)); @@ -535,8 +539,9 @@ public void shouldExtractRequestWithBidderSpecificExtension() { final Imp actualImp = impCaptor.getValue(); assertThat(actualImp).isNotSameAs(givenImp); - assertThat(actualImp.getExt()).isNotSameAs(givenImp.getExt()); assertThat(actualImp).isEqualTo(givenImp); + assertThat(actualImp.getExt()).isNotSameAs(givenImp.getExt()); + assertThat(actualImp.getExt()).isEqualTo(givenImp.getExt()); } @Test From 161409e050d655ee5ba54928d495ed8f7dfb6f50 Mon Sep 17 00:00:00 2001 From: osulzhenko <125548596+osulzhenko@users.noreply.github.com> Date: Fri, 23 Aug 2024 15:18:01 +0300 Subject: [PATCH 6/6] Add a functional test to ensure that bidder implementation is always merged (#3397) * Add a functional test to ensure that the correct bidder implementation is always merged --- .../functional/tests/ImpRequestSpec.groovy | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/test/groovy/org/prebid/server/functional/tests/ImpRequestSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/ImpRequestSpec.groovy index 9071eea8194..085bd19a690 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/ImpRequestSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/ImpRequestSpec.groovy @@ -1,5 +1,6 @@ package org.prebid.server.functional.tests +import org.prebid.server.functional.model.bidder.Openx import org.prebid.server.functional.model.db.StoredImp import org.prebid.server.functional.model.request.auction.BidRequest import org.prebid.server.functional.model.request.auction.Imp @@ -13,10 +14,12 @@ import static org.prebid.server.functional.model.bidder.BidderName.ALIAS_CAMEL_C import static org.prebid.server.functional.model.bidder.BidderName.EMPTY import static org.prebid.server.functional.model.bidder.BidderName.GENERIC import static org.prebid.server.functional.model.bidder.BidderName.GENERIC_CAMEL_CASE +import static org.prebid.server.functional.model.bidder.BidderName.OPENX import static org.prebid.server.functional.model.bidder.BidderName.RUBICON import static org.prebid.server.functional.model.bidder.BidderName.UNKNOWN import static org.prebid.server.functional.model.bidder.BidderName.WILDCARD import static org.prebid.server.functional.model.response.auction.ErrorType.PREBID +import static org.prebid.server.functional.testcontainers.Dependencies.getNetworkServiceContainer class ImpRequestSpec extends BaseSpec { @@ -184,6 +187,41 @@ class ImpRequestSpec extends BaseSpec { assert !bidderRequest?.imp?.first?.ext?.prebid?.bidder } + def "PBS should always update specified bidder imp when imp.ext.prebid.imp contain such bidder"() { + given: "PBs with openx bidder" + def pbsService = pbsServiceFactory.getService( + ["adapters.openx.enabled" : "true", + "adapters.openx.endpoint": "$networkServiceContainer.rootUri/auction".toString()]) + + and: "Default basic BidRequest" + def impPmp = Pmp.defaultPmp + def extPrebidImpPmp = Pmp.defaultPmp + def bidRequest = BidRequest.defaultBidRequest.tap { + imp.first.tap { + pmp = impPmp + ext.prebid.bidder.openx = Openx.defaultOpenx + ext.prebid.imp = [(OPENX): new Imp(pmp: extPrebidImpPmp)] + } + } + + when: "Requesting PBS auction" + def response = pbsService.sendAuctionRequest(bidRequest) + + then: "Bid response should not contain warning" + assert !response?.ext?.warnings + + and: "Generic bidderRequest should contain pmp from original imp" + def bidderToBidderRequests = getRequests(response) + assert bidderToBidderRequests[GENERIC.value].first.imp.pmp == [impPmp] + + and: "OpenX bidderRequest should contain pmp from ext.prebid.imp" + assert bidderToBidderRequests[OPENX.value].first.imp.pmp == [extPrebidImpPmp] + + and: "PBS should remove imp.ext.prebid.bidder from bidderRequests" + def bidderRequests = bidder.getBidderRequests(bidRequest.id) + assert !bidderRequests?.imp?.ext?.prebid?.imp?.flatten() + } + def "PBS should validate imp and add proper warning when imp.ext.prebid.imp contain invalid ortb data"() { given: "BidRequest with invalid config for ext.prebid.imp" def impPmp = Pmp.defaultPmp