Skip to content

Commit

Permalink
Core: Remove all imp.ext.prebid.imp (#3378)
Browse files Browse the repository at this point in the history
  • Loading branch information
@osulzhenko
osulzhenko authored Aug 21, 2024
1 parent 796ee57 commit 39c7a9f
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 45 deletions.
11 changes: 5 additions & 6 deletions src/main/java/org/prebid/server/auction/ImpAdjuster.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,17 @@ public Imp adjust(Imp originalImp, String bidder, BidderAliases bidderAliases, L
final JsonNode bidderNode = getBidderNode(bidder, bidderAliases, impExtPrebidImp);

if (bidderNode == null || bidderNode.isEmpty()) {
removeImpExtPrebidImp(originalImp.getExt());
return originalImp;
}

// remove circular references according to the requirements
removeExtPrebidBidder(bidderNode);

try {
final JsonNode originalImpNode = jacksonMapper.mapper().valueToTree(originalImp);
final JsonNode mergedImpNode = jsonMerger.merge(bidderNode, originalImpNode);

// clean up merged imp.ext.prebid.imp
removeImpExtPrebidImp(mergedImpNode);
removeImpExtPrebidImp(mergedImpNode.get(IMP_EXT));

final Imp resultImp = jacksonMapper.mapper().convertValue(mergedImpNode, Imp.class);

Expand All @@ -61,6 +60,7 @@ public Imp adjust(Imp originalImp, String bidder, BidderAliases bidderAliases, L
} catch (Exception e) {
debugMessages.add("imp.ext.prebid.imp.%s can not be merged into original imp [id=%s], reason: %s"
.formatted(bidder, originalImp.getId(), e.getMessage()));
removeImpExtPrebidImp(originalImp.getExt());
return originalImp;
}
}
Expand Down Expand Up @@ -90,9 +90,8 @@ private static void removeExtPrebidBidder(JsonNode bidderNode) {
.ifPresent(ext -> ext.remove(EXT_PREBID_BIDDER));
}

private static void removeImpExtPrebidImp(JsonNode mergedImpNode) {
Optional.ofNullable(mergedImpNode.get(IMP_EXT))
.map(extNode -> extNode.get(EXT_PREBID))
private static void removeImpExtPrebidImp(JsonNode impExt) {
Optional.ofNullable(impExt.get(EXT_PREBID))
.map(ObjectNode.class::cast)
.ifPresent(prebid -> prebid.remove(EXT_PREBID_IMP));
}
Expand Down
28 changes: 28 additions & 0 deletions src/test/groovy/org/prebid/server/functional/tests/ImpRequestSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ 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.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
Expand Down Expand Up @@ -156,6 +157,33 @@ class ImpRequestSpec extends BaseSpec {
bidderName << [WILDCARD, UNKNOWN]
}

def "PBS shouldn't update imp fields and without warning when imp.ext.prebid.imp contain not applicable bidder"() {
given: "Default basic BidRequest"
def impPmp = Pmp.defaultPmp
def bidRequest = BidRequest.defaultBidRequest.tap {
imp.first.tap {
pmp = impPmp
ext.prebid.imp = [(RUBICON): new Imp(pmp: Pmp.defaultPmp)]
}
}

when: "Requesting PBS auction"
def response = defaultPbsServiceWithAlias.sendAuctionRequest(bidRequest)

then: "Bid response should not contain warning"
assert !response?.ext?.warnings

and: "BidderRequest should contain pmp from original imp"
def bidderRequest = bidder.getBidderRequest(bidRequest.id)
assert bidderRequest.imp.pmp == [impPmp]

and: "PBS should remove imp.ext.prebid.imp from bidderRequest"
assert !bidderRequest?.imp?.first?.ext?.prebid?.imp

and: "PBS should remove imp.ext.prebid.bidder from bidderRequest"
assert !bidderRequest?.imp?.first?.ext?.prebid?.bidder
}

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
57 changes: 18 additions & 39 deletions src/test/java/org/prebid/server/auction/ImpAdjusterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,24 +78,7 @@ public void adjustShouldReturnOriginalImpWhenImpExtPrebidImpIsAbsent() {
}

@Test
public void adjustShouldReturnOriginalImpWhenImpExtPrebidImpDoesNotHaveRequestedBidder() {
// given
final Imp givenImp = Imp.builder()
.ext(mapper.createObjectNode().set("prebid", mapper.createObjectNode()
.set("imp", mapper.createObjectNode().set("anotherBidder", mapper.createObjectNode()))))
.build();
final List<String> debugMessages = new ArrayList<>();

// when
final Imp result = target.adjust(givenImp, "someBidder", bidderAliases, debugMessages);

// then
assertThat(result).isSameAs(givenImp);
assertThat(debugMessages).isEmpty();
}

@Test
public void adjustShouldReturnOriginalImpWhenImpExtPrebidImpHasEmptyBidder() {
public void adjustShouldRemoveExpImpFromOriginalImpWhenImpExtPrebidImpHasEmptyBidder() {
// given
final Imp givenImp = Imp.builder()
.ext(mapper.createObjectNode().set("prebid", mapper.createObjectNode()
Expand All @@ -107,24 +90,10 @@ public void adjustShouldReturnOriginalImpWhenImpExtPrebidImpHasEmptyBidder() {
final Imp result = target.adjust(givenImp, "someBidder", bidderAliases, debugMessages);

// then
assertThat(result).isSameAs(givenImp);
assertThat(debugMessages).isEmpty();
}

@Test
public void adjustShouldReturnOriginalImpWhenMergedImpNodeIsEmpty() {
// given
final Imp givenImp = Imp.builder()
.ext(mapper.createObjectNode().set("prebid", mapper.createObjectNode()
.set("imp", mapper.createObjectNode().set("someBidder", mapper.createObjectNode()))))
.build();
final List<String> debugMessages = new ArrayList<>();

// when
final Imp result = target.adjust(givenImp, "someBidder", bidderAliases, debugMessages);
final Imp expectedImp = givenImp.toBuilder()
.ext(mapper.createObjectNode().set("prebid", mapper.createObjectNode())).build();

// then
assertThat(result).isSameAs(givenImp);
assertThat(result).isEqualTo(expectedImp);
assertThat(debugMessages).isEmpty();
}

Expand Down Expand Up @@ -214,7 +183,7 @@ public void resolveImpShouldMergeBidderSpecificImpIntoOriginalImpCaseAliasBidder
}

@Test
public void resolveImpShouldReturnOriginalImpWhenResultingImpValidationFailed() throws ValidationException {
public void resolveImpShouldReturnImpWithoutExpImpWhenResultingImpValidationFailed() throws ValidationException {
// given
doThrow(new ValidationException("imp validation failed")).when(impValidator).validateImp(any());

Expand All @@ -231,14 +200,19 @@ public void resolveImpShouldReturnOriginalImpWhenResultingImpValidationFailed()
final Imp result = target.adjust(givenImp, "someBidder", bidderAliases, debugMessages);

// then
assertThat(result).isSameAs(givenImp);
final Imp expectedImp = givenImp.toBuilder()
.ext(mapper.createObjectNode().put("originAttr", "originValue")
.set("prebid", mapper.createObjectNode().put("prebidOriginAttr", "prebidOriginValue")))
.build();

assertThat(result).isEqualTo(expectedImp);
assertThat(debugMessages).containsOnly(
"imp.ext.prebid.imp.someBidder can not be merged into original imp [id=impId], "
+ "reason: imp validation failed");
}

@Test
public void resolveImpShouldReturnOriginalImpWhenMergingFailed() {
public void resolveImpShouldReturnImpWithoutExpWhenMergingFailed() {
// given
final ObjectNode invalidBidderImp = mapper.createObjectNode()
.put("bidfloor", "2.0")
Expand All @@ -251,7 +225,12 @@ public void resolveImpShouldReturnOriginalImpWhenMergingFailed() {
final Imp result = target.adjust(givenImp, "someBidder", bidderAliases, debugMessages);

// then
assertThat(result).isSameAs(givenImp);
final Imp expectedImp = givenImp.toBuilder()
.ext(mapper.createObjectNode().put("originAttr", "originValue")
.set("prebid", mapper.createObjectNode().put("prebidOriginAttr", "prebidOriginValue")))
.build();

assertThat(result).isEqualTo(expectedImp);
assertThat(debugMessages).hasSize(1).first()
.satisfies(message -> assertThat(message).startsWith(
"imp.ext.prebid.imp.someBidder can not be merged into original imp [id=impId],"
Expand Down

0 comments on commit 39c7a9f

Please sign in to comment.