From 53382dd9486504413fd1dbaa4398554a3dacd5e1 Mon Sep 17 00:00:00 2001 From: Anton Babak <76536883+AntoxaAntoxic@users.noreply.github.com> Date: Mon, 26 Aug 2024 16:24:27 +0200 Subject: [PATCH] Bugfix: Fix merging imp.ext.prebid.imp into imp (#3396) --- .../server/auction/ExchangeService.java | 15 ++++---- .../functional/tests/ImpRequestSpec.groovy | 38 +++++++++++++++++++ .../server/auction/ExchangeServiceTest.java | 28 +++++++++++--- 3 files changed, 67 insertions(+), 14 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..8ccada983ac 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(); } @@ -981,6 +980,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(); @@ -1016,18 +1016,17 @@ private ObjectNode prepareImpExt(String bidder, BigDecimal adjustedFloor, boolean transmitTid, boolean useFirstPartyData) { - - final ObjectNode modifiedImpExt = impExt.deepCopy(); + final JsonNode bidderNode = bidderParamsFromImpExt(impExt).get(bidder); 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, bidderNode); 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) { 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 diff --git a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java index 40dd296723b..ffff9684fb1 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,14 +502,20 @@ 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)); + 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)); @@ -526,6 +533,15 @@ 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).isEqualTo(givenImp); + assertThat(actualImp.getExt()).isNotSameAs(givenImp.getExt()); + assertThat(actualImp.getExt()).isEqualTo(givenImp.getExt()); } @Test