Skip to content

Commit

Permalink
Bugfix: Fix merging imp.ext.prebid.imp into imp (#3396)
Browse files Browse the repository at this point in the history
  • Loading branch information
AntoxaAntoxic authored Aug 26, 2024
1 parent 88bef61 commit e827e45
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 14 deletions.
15 changes: 7 additions & 8 deletions src/main/java/org/prebid/server/auction/ExchangeService.java
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,6 @@ private static BidRequestCacheInfo bidRequestCacheInfo(BidRequest bidRequest) {
.build();
}
}

return BidRequestCacheInfo.noCache();
}

Expand Down Expand Up @@ -981,6 +980,7 @@ private List<Imp> 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();
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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 {

Expand Down Expand Up @@ -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
Expand Down
28 changes: 22 additions & 6 deletions src/test/java/org/prebid/server/auction/ExchangeServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));

Expand All @@ -526,6 +533,15 @@ public void shouldExtractRequestWithBidderSpecificExtension() {
.build()))
.tmax(500L)
.build());

final ArgumentCaptor<Imp> 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
Expand Down

0 comments on commit e827e45

Please sign in to comment.