Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Case Insensitive Check for Bidder Controls #3352

Merged
merged 2 commits into from
Aug 12, 2024
Merged

Case Insensitive Check for Bidder Controls #3352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f4fe94
Case Insensitive Check for Bidder Controls
AntoxaAntoxic Aug 6, 2024
cc9964b
tests: Increase test coverage for naming and alias cases in bidderCon…
osulzhenko Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion ...main/java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.fasterxml.jackson.databind.JsonNode;
import com.iab.openrtb.request.BidRequest;
import com.iab.openrtb.request.Imp;
import org.apache.commons.lang3.StringUtils;
import org.prebid.server.auction.BidderAliases;
import org.prebid.server.bidder.BidderCatalog;
import org.prebid.server.bidder.model.BidderError;
Expand All @@ -14,6 +15,7 @@

import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
Expand Down Expand Up @@ -78,10 +80,11 @@ 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.get(originalBidderName))
.map(bidders -> getBidder(originalBidderName, bidders))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If getBidder returns null, it will cause an NPE on the next line.
While project logic suggests this shouldn’t happen, it would be better for readability if you added a nonNull filter or some other safeguard.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it won't since it's nullable optional that execute orElse in that case

.map(bidder -> bidder.get(PREF_MTYPE_FIELD))
.filter(JsonNode::isTextual)
.map(JsonNode::textValue)
Expand All @@ -92,6 +95,17 @@ private MediaType preferredMediaType(BidRequest bidRequest,
.orElse(null);
}

private static JsonNode getBidder(String bidderName, JsonNode biddersNode) {
final Iterator<String> fieldNames = biddersNode.fieldNames();
while (fieldNames.hasNext()) {
final String fieldName = fieldNames.next();
if (StringUtils.equalsIgnoreCase(bidderName, fieldName)) {
return biddersNode.get(fieldName);
}
}
return null;
}

private static Imp processImp(Imp imp, MediaType preferredMediaType, List<BidderError> errors) {
if (!isMultiFormat(imp)) {
return imp;
Expand Down
3 changes: 3 additions & 0 deletions src/test/groovy/org/prebid/server/functional/model/request/auction/BidderControls.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package org.prebid.server.functional.model.request.auction

import com.fasterxml.jackson.annotation.JsonProperty
import groovy.transform.ToString

@ToString(includeNames = true, ignoreNulls = true)
class BidderControls {

GenericPreferredBidder generic
@JsonProperty("GeNeRiC")
GenericPreferredBidder genericAnyCase
}
108 changes: 100 additions & 8 deletions src/test/groovy/org/prebid/server/functional/tests/FilterMultiFormatSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.prebid.server.functional.tests

import org.prebid.server.functional.model.bidder.BidderName
import org.prebid.server.functional.model.bidder.Generic
import org.prebid.server.functional.model.config.AccountAuctionConfig
import org.prebid.server.functional.model.config.AccountConfig
import org.prebid.server.functional.model.db.Account
Expand All @@ -11,6 +12,7 @@ import org.prebid.server.functional.model.request.auction.BidderControls
import org.prebid.server.functional.model.request.auction.GenericPreferredBidder
import org.prebid.server.functional.model.request.auction.Native

import static org.prebid.server.functional.model.bidder.BidderName.ALIAS
import static org.prebid.server.functional.model.response.auction.ErrorType.GENERIC
import static org.prebid.server.functional.model.response.auction.MediaType.AUDIO
import static org.prebid.server.functional.model.response.auction.MediaType.BANNER
Expand Down Expand Up @@ -52,7 +54,7 @@ class FilterMultiFormatSpec extends BaseSpec {
def bidRequest = BidRequest.defaultBidRequest.tap {
imp[0].banner = Banner.defaultBanner
imp[0].audio = Audio.defaultAudio
ext.prebid.bidderControls = new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER))
ext.prebid.bidderControls = bidderControls
}

when: "PBS processes auction request"
Expand All @@ -62,6 +64,12 @@ class FilterMultiFormatSpec extends BaseSpec {
def bidderRequest = bidder.getBidderRequest(bidRequest.id)
assert bidderRequest.imp[0].banner
assert bidderRequest.imp[0].audio

where:
bidderControls << [
new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER)),
new BidderControls(genericAnyCase: new GenericPreferredBidder(preferredMediaType: BANNER))
]
}

def "PBS should respond with one requested preferred media type when default adapters multi format is false in config and preferred media type specified at account level"() {
Expand Down Expand Up @@ -98,7 +106,7 @@ class FilterMultiFormatSpec extends BaseSpec {
def bidRequest = BidRequest.defaultBidRequest.tap {
imp[0].banner = Banner.defaultBanner
imp[0].audio = Audio.defaultAudio
ext.prebid.bidderControls = new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER))
ext.prebid.bidderControls = bidderControls
}

when: "PBS processes auction request"
Expand All @@ -108,6 +116,12 @@ class FilterMultiFormatSpec extends BaseSpec {
def bidderRequest = bidder.getBidderRequest(bidRequest.id)
assert bidderRequest.imp[0].banner
assert !bidderRequest.imp[0].audio

where:
bidderControls << [
new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER)),
new BidderControls(genericAnyCase: new GenericPreferredBidder(preferredMediaType: BANNER))
]
}

def "PBS should respond with all requested media type when multi format is true in config and preferred media type specified at request level"() {
Expand All @@ -119,7 +133,7 @@ class FilterMultiFormatSpec extends BaseSpec {
def bidRequest = BidRequest.defaultBidRequest.tap {
imp[0].banner = Banner.defaultBanner
imp[0].audio = Audio.defaultAudio
ext.prebid.bidderControls = new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER))
ext.prebid.bidderControls = bidderControls
}

when: "PBS processes auction request"
Expand All @@ -129,6 +143,12 @@ class FilterMultiFormatSpec extends BaseSpec {
def bidderRequest = bidder.getBidderRequest(bidRequest.id)
assert bidderRequest.imp.banner
assert bidderRequest.imp.audio

where:
bidderControls << [
new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER)),
new BidderControls(genericAnyCase: new GenericPreferredBidder(preferredMediaType: BANNER))
]
}

def "PBS should respond with all requested media type when multi format is true in config and preferred media type specified at account level"() {
Expand Down Expand Up @@ -190,7 +210,7 @@ class FilterMultiFormatSpec extends BaseSpec {
def bidRequest = BidRequest.defaultBidRequest.tap {
imp[0].banner = Banner.defaultBanner
imp[0].audio = Audio.defaultAudio
ext.prebid.bidderControls = new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER))
ext.prebid.bidderControls = bidderControls
}

when: "PBS processes auction request"
Expand All @@ -200,6 +220,12 @@ class FilterMultiFormatSpec extends BaseSpec {
def bidderRequest = bidder.getBidderRequest(bidRequest.id)
assert bidderRequest.imp[0].banner
assert !bidderRequest.imp[0].audio

where:
bidderControls << [
new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER)),
new BidderControls(genericAnyCase: new GenericPreferredBidder(preferredMediaType: BANNER))
]
}

def "PBS should respond with warning and don't make a bidder call when multi format at request and preferred media type specified at account level with non requested media type"() {
Expand Down Expand Up @@ -241,7 +267,7 @@ class FilterMultiFormatSpec extends BaseSpec {
imp[0].banner = null
imp[0].audio = Audio.defaultAudio
imp[0].nativeObj = Native.defaultNative
ext.prebid.bidderControls = new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER))
ext.prebid.bidderControls = bidderControls
}

when: "PBS processes auction request"
Expand All @@ -254,6 +280,12 @@ class FilterMultiFormatSpec extends BaseSpec {
assert bidResponse.ext.warnings[GENERIC]?.message ==
["Imp ${bidRequest.imp[0].id} does not have a media type after filtering and has been removed from the request for this bidder.",
"Bid request contains 0 impressions after filtering."]

where:
bidderControls << [
new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER)),
new BidderControls(genericAnyCase: new GenericPreferredBidder(preferredMediaType: BANNER))
]
}

def "PBS shouldn't respond with warning and make a bidder call when request doesn't contain multi format and preferred media type specified at account level"() {
Expand Down Expand Up @@ -292,7 +324,7 @@ class FilterMultiFormatSpec extends BaseSpec {
def bidRequest = BidRequest.defaultBidRequest.tap {
imp[0].banner = null
imp[0].audio = Audio.defaultAudio
ext.prebid.bidderControls = new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER))
ext.prebid.bidderControls = bidderControls
}

when: "PBS processes auction request"
Expand All @@ -304,6 +336,12 @@ class FilterMultiFormatSpec extends BaseSpec {

and: "Bid response shouldn't contain warning"
assert !bidResponse.ext.warnings

where:
bidderControls << [
new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER)),
new BidderControls(genericAnyCase: new GenericPreferredBidder(preferredMediaType: BANNER))
]
}

def "PBS shouldn't respond with warning and make a bidder call when request doesn't contain multi format and multi format is false and preferred media type specified at request level with null"() {
Expand All @@ -315,7 +353,7 @@ class FilterMultiFormatSpec extends BaseSpec {
def bidRequest = BidRequest.defaultBidRequest.tap {
imp[0].banner = Banner.getDefaultBanner()
imp[0].audio = Audio.defaultAudio
ext.prebid.bidderControls = new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: NULL))
ext.prebid.bidderControls = bidderControls
}

when: "PBS processes auction request"
Expand All @@ -326,6 +364,12 @@ class FilterMultiFormatSpec extends BaseSpec {

and: "Bid response shouldn't contain warning"
assert !bidResponse.ext?.warnings

where:
bidderControls << [
new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: NULL)),
new BidderControls(genericAnyCase: new GenericPreferredBidder(preferredMediaType: NULL)),
]
}

def "PBS shouldn't respond with warning and make a bidder call when request doesn't contain multi format and multi format is false and preferred media type specified at account level with null"() {
Expand Down Expand Up @@ -364,7 +408,7 @@ class FilterMultiFormatSpec extends BaseSpec {
def bidRequest = BidRequest.defaultBidRequest.tap {
imp[0].banner = Banner.defaultBanner
imp[0].audio = Audio.defaultAudio
ext.prebid.bidderControls = new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER))
ext.prebid.bidderControls = bidderControls
}

and: "Account in the DB with preferred media type"
Expand All @@ -379,5 +423,53 @@ class FilterMultiFormatSpec extends BaseSpec {
def bidderRequest = bidder.getBidderRequest(bidRequest.id)
assert bidderRequest.imp[0].banner
assert !bidderRequest.imp[0].audio

where:
bidderControls << [
new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER)),
new BidderControls(genericAnyCase: new GenericPreferredBidder(preferredMediaType: BANNER))
]
}

def "PBS should not preferred media type specified at request level when it's alias bidder"() {
given: "PBS with adapter configuration"
def pbsService = pbsServiceFactory.getService(
"adapter-defaults.ortb.multiformat-supported": "false",
"adapters.generic.ortb.multiformat-supported": "false")

and: "Default bid request with alias"
def bidRequest = BidRequest.defaultBidRequest.tap {
imp[0].tap {
banner = Banner.defaultBanner
audio = Audio.defaultAudio
ext.prebid.bidder.tap {
alias = new Generic()
generic = null
}
}
ext.prebid.tap {
it.aliases = [(ALIAS.value): BidderName.GENERIC]
it.bidderControls = bidderControls
}
}

and: "Account in the DB with preferred media type"
def accountConfig = new AccountAuctionConfig(preferredMediaType: [(BidderName.GENERIC): AUDIO])
def account = new Account(uuid: bidRequest.accountId, config: new AccountConfig(auction: accountConfig))
accountDao.save(account)

when: "PBS processes auction request"
pbsService.sendAuctionRequest(bidRequest)

then: "Bidder request should contain preferred media type from account config"
def bidderRequest = bidder.getBidderRequest(bidRequest.id)
assert !bidderRequest.imp[0].banner
assert bidderRequest.imp[0].audio

where:
bidderControls << [
new BidderControls(generic: new GenericPreferredBidder(preferredMediaType: BANNER)),
new BidderControls(genericAnyCase: new GenericPreferredBidder(preferredMediaType: BANNER))
]
}
}
34 changes: 34 additions & 0 deletions .../java/org/prebid/server/auction/mediatypeprocessor/MultiFormatMediaTypeProcessorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,40 @@ public void processShouldUseRequestLevelPreferredMediaTypeFirst() {
assertThat(result.getErrors()).isEmpty();
}

@Test
public void processShouldUseRequestLevelPreferredMediaTypeFirstCaseInsensitive() {
// given
given(bidderAliases.resolveBidder(BIDDER)).willReturn("resolvedBidderName");
given(bidderCatalog.bidderInfoByName("resolvedBidderName")).willReturn(givenBidderInfo(false));

final ObjectNode bidderControls = mapper.createObjectNode();
bidderControls.putObject(BIDDER.toUpperCase()).put("prefmtype", "video");

final BidRequest bidRequest = givenBidRequest(
request -> request.ext(ExtRequest.of(ExtRequestPrebid.builder()
.biddercontrols(bidderControls)
.build())),
givenImp(BANNER, VIDEO, AUDIO, NATIVE));

final Account account = givenAccount(Map.of("resolvedBidderName", AUDIO));

// when
final MediaTypeProcessingResult result = target.process(bidRequest, BIDDER, bidderAliases, account);

// then
assertThat(result.isRejected()).isFalse();
assertThat(result.getBidRequest())
.extracting(BidRequest::getImp)
.asInstanceOf(InstanceOfAssertFactories.list(Imp.class))
.containsExactly(givenImp(VIDEO));
assertThat(result.getBidRequest())
.extracting(BidRequest::getExt)
.extracting(ExtRequest::getPrebid)
.extracting(ExtRequestPrebid::getBiddercontrols)
.isNull();
assertThat(result.getErrors()).isEmpty();
}

@Test
public void processShouldSkipImpsWithSingleMediaType() {
// given
Expand Down
Loading