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

Fix Bidder Aliases Validation #3696

Merged
merged 3 commits into from
Jan 28, 2025
+133 −56
Merged

Fix Bidder Aliases Validation #3696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d188ada
Fix Bidder Aliases Validation
AntoxaAntoxic Jan 23, 2025
c8b100d
Tests: Fix Bidder Aliases Validation (#3706)
osulzhenko Jan 28, 2025
4f157d0
Update functional tests
marki1an Jan 28, 2025
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
45 changes: 29 additions & 16 deletions src/main/java/org/prebid/server/validation/ImpValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.prebid.server.auction.BidderAliases;
import org.prebid.server.bidder.BidderCatalog;
import org.prebid.server.json.JacksonMapper;
import org.prebid.server.proto.openrtb.ext.request.ExtImpPrebid;
Expand Down Expand Up @@ -365,9 +366,10 @@ private void validateImpExt(ObjectNode ext, Map<String, String> aliases, int imp
validateImpExtPrebid(ext != null ? ext.get(PREBID_EXT) : null, aliases, impIndex, warnings);
}

private void validateImpExtPrebid(JsonNode extPrebidNode, Map<String, String> aliases, int impIndex,
List<String> warnings)
throws ValidationException {
private void validateImpExtPrebid(JsonNode extPrebidNode,
Map<String, String> requestAliases,
int impIndex,
List<String> warnings) throws ValidationException {

if (extPrebidNode == null) {
throw new ValidationException(
Expand All @@ -387,15 +389,21 @@ private void validateImpExtPrebid(JsonNode extPrebidNode, Map<String, String> al
}
final ExtImpPrebid extPrebid = parseExtImpPrebid((ObjectNode) extPrebidNode, impIndex);

validateImpExtPrebidBidder(extPrebidBidderNode, extPrebid.getStoredAuctionResponse(),
aliases, impIndex, warnings);
validateImpExtPrebidStoredResponses(extPrebid, aliases, impIndex, warnings);
final BidderAliases aliases = BidderAliases.of(requestAliases, null, bidderCatalog);

validateImpExtPrebidBidder(
extPrebidBidderNode,
extPrebid.getStoredAuctionResponse(),
aliases,
impIndex,
warnings);

validateImpExtPrebidStoredResponses(extPrebid, aliases, impIndex, warnings);
validateImpExtPrebidImp(extPrebidNode.get(IMP_EXT), aliases, impIndex, warnings);
}

private void validateImpExtPrebidImp(JsonNode imp,
Map<String, String> aliases,
BidderAliases aliases,
int impIndex,
List<String> warnings) {
if (imp == null) {
Expand All @@ -406,7 +414,7 @@ private void validateImpExtPrebidImp(JsonNode imp,
while (bidders.hasNext()) {
final Map.Entry<String, JsonNode> bidder = bidders.next();
final String bidderName = bidder.getKey();
final String resolvedBidderName = aliases.getOrDefault(bidderName, bidderName);
final String resolvedBidderName = aliases.resolveBidder(bidderName);
if (!bidderCatalog.isValidName(resolvedBidderName) && !bidderCatalog.isDeprecatedName(resolvedBidderName)) {
bidders.remove();
warnings.add("WARNING: request.imp[%d].ext.prebid.imp.%s was dropped with the reason: invalid bidder"
Expand All @@ -417,7 +425,7 @@ private void validateImpExtPrebidImp(JsonNode imp,

private void validateImpExtPrebidBidder(JsonNode extPrebidBidder,
ExtStoredAuctionResponse storedAuctionResponse,
Map<String, String> aliases,
BidderAliases aliases,
int impIndex,
List<String> warnings) throws ValidationException {
if (extPrebidBidder == null) {
Expand All @@ -433,7 +441,7 @@ private void validateImpExtPrebidBidder(JsonNode extPrebidBidder,
final Map.Entry<String, JsonNode> bidderExtension = bidderExtensions.next();
final String bidder = bidderExtension.getKey();
try {
validateImpBidderExtName(impIndex, bidderExtension, aliases.getOrDefault(bidder, bidder));
validateImpBidderExtName(impIndex, bidderExtension, aliases.resolveBidder(bidder));
} catch (ValidationException ex) {
bidderExtensions.remove();
warnings.add("WARNING: request.imp[%d].ext.prebid.bidder.%s was dropped with a reason: %s"
Expand All @@ -447,7 +455,7 @@ private void validateImpExtPrebidBidder(JsonNode extPrebidBidder,
}

private void validateImpExtPrebidStoredResponses(ExtImpPrebid extPrebid,
Map<String, String> aliases,
BidderAliases aliases,
int impIndex,
List<String> warnings) throws ValidationException {
final ExtStoredAuctionResponse extStoredAuctionResponse = extPrebid.getStoredAuctionResponse();
Expand Down Expand Up @@ -479,8 +487,11 @@ private void validateImpExtPrebidStoredResponses(ExtImpPrebid extPrebid,
}
}

private void validateStoredBidResponse(ExtStoredBidResponse extStoredBidResponse, ObjectNode bidderNode,
Map<String, String> aliases, int impIndex) throws ValidationException {
private void validateStoredBidResponse(ExtStoredBidResponse extStoredBidResponse,
ObjectNode bidderNode,
BidderAliases aliases,
int impIndex) throws ValidationException {

final String bidder = extStoredBidResponse.getBidder();
final String id = extStoredBidResponse.getId();
if (StringUtils.isEmpty(bidder)) {
Expand All @@ -493,7 +504,7 @@ private void validateStoredBidResponse(ExtStoredBidResponse extStoredBidResponse
"Id was not defined for request.imp[%d].ext.prebid.storedbidresponse.id".formatted(impIndex));
}

final String resolvedBidder = aliases.getOrDefault(bidder, bidder);
final String resolvedBidder = aliases.resolveBidder(bidder);

if (!bidderCatalog.isValidName(resolvedBidder)) {
throw new ValidationException(
Expand All @@ -518,8 +529,10 @@ private ExtImpPrebid parseExtImpPrebid(ObjectNode extImpPrebid, int impIndex) th
}
}

private void validateImpBidderExtName(int impIndex, Map.Entry<String, JsonNode> bidderExtension, String bidderName)
throws ValidationException {
private void validateImpBidderExtName(int impIndex,
Map.Entry<String, JsonNode> bidderExtension,
String bidderName) throws ValidationException {

if (bidderCatalog.isValidName(bidderName)) {
final Set<String> messages = bidderParamValidator.validate(bidderName, bidderExtension.getValue());
if (!messages.isEmpty()) {
Expand Down
107 changes: 67 additions & 40 deletions src/test/groovy/org/prebid/server/functional/tests/AliasSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,41 @@
package org.prebid.server.functional.tests

import org.prebid.server.functional.model.bidder.AppNexus
import org.prebid.server.functional.model.bidder.Generic
import org.prebid.server.functional.model.bidder.Openx
import org.prebid.server.functional.model.request.auction.BidRequest
import org.prebid.server.functional.service.PrebidServerException
import org.prebid.server.functional.testcontainers.Dependencies
import org.prebid.server.functional.service.PrebidServerService
import org.prebid.server.functional.util.PBSUtils

import static io.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST
import static org.prebid.server.functional.model.bidder.BidderName.ALIAS
import static org.prebid.server.functional.model.bidder.BidderName.APPNEXUS
import static org.prebid.server.functional.model.bidder.BidderName.BOGUS
import static org.prebid.server.functional.model.bidder.BidderName.GENERIC
import static org.prebid.server.functional.model.bidder.BidderName.GENER_X
import static org.prebid.server.functional.model.bidder.BidderName.OPENX
import static org.prebid.server.functional.model.bidder.CompressionType.GZIP
import static org.prebid.server.functional.model.response.auction.ErrorType.PREBID
import static org.prebid.server.functional.testcontainers.Dependencies.networkServiceContainer
import static org.prebid.server.functional.util.HttpUtil.CONTENT_ENCODING_HEADER

class AliasSpec extends BaseSpec {

private static final Map<String, String> ADDITIONAL_BIDDERS_CONFIG = ["adapters.${OPENX.value}.enabled" : "true",
"adapters.${OPENX.value}.endpoint" : "$networkServiceContainer.rootUri/${OPENX.value}/auction".toString(),
"adapters.${APPNEXUS.value}.enabled" : "true",
"adapters.${APPNEXUS.value}.endpoint": "$networkServiceContainer.rootUri/${APPNEXUS.value}/auction".toString()]
private static PrebidServerService pbsServiceWithAdditionalBidders

def setupSpec() {
pbsServiceWithAdditionalBidders = pbsServiceFactory.getService(ADDITIONAL_BIDDERS_CONFIG + GENERIC_ALIAS_CONFIG)
}

def cleanupSpec() {
pbsServiceFactory.removeContainer(ADDITIONAL_BIDDERS_CONFIG + GENERIC_ALIAS_CONFIG)
}

def "PBS should be able to take alias from request"() {
given: "Default bid request with alias"
def bidRequest = BidRequest.defaultBidRequest.tap {
Expand Down Expand Up @@ -133,56 +150,19 @@ class AliasSpec extends BaseSpec {
}

def "PBS aliased bidder config should be independently from parent"() {
given: "Pbs config"
def prebidServerService = pbsServiceFactory.getService(GENERIC_ALIAS_CONFIG)

and: "Default bid request with alias"
given: "Default bid request with alias"
def bidRequest = BidRequest.defaultBidRequest.tap {
imp[0].ext.prebid.bidder.alias = new Generic()
}

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

then: "Bidder request should contain request per-alies"
def bidderRequests = bidder.getBidderRequests(bidRequest.id)
assert bidderRequests.size() == 2
}

def "PBS should ignore alias logic when hardcoded alias endpoints are present"() {
given: "PBs server with aliases config"
def pbsConfig = ["adapters.generic.aliases.alias.enabled" : "true",
"adapters.generic.aliases.alias.endpoint": "$networkServiceContainer.rootUri/alias/auction".toString(),
"adapters.openx.enabled" : "true",
"adapters.openx.endpoint" : "$networkServiceContainer.rootUri/openx/auction".toString()]
def pbsService = pbsServiceFactory.getService(pbsConfig)

and: "Default bid request with openx and alias bidder"
def bidRequest = BidRequest.defaultBidRequest.tap {
imp[0].ext.prebid.bidder.alias = new Generic()
imp[0].ext.prebid.bidder.generic = new Generic()
imp[0].ext.prebid.bidder.openx = new Openx()
ext.prebid.aliases = [(ALIAS.value): OPENX]
}

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

then: "PBS should call only generic bidder"
def responseDebug = bidResponse.ext.debug
assert responseDebug.httpcalls[GENERIC.value]

and: "PBS shouldn't call only opexn,alias bidder"
assert !responseDebug.httpcalls[OPENX.value]
assert !responseDebug.httpcalls[ALIAS.value]

and: "PBS should call only generic bidder"
assert bidder.getBidderRequest(bidRequest.id)

cleanup: "Stop and remove pbs container"
pbsServiceFactory.removeContainer(pbsConfig)
}

def "PBS should ignore aliases for requests with a base adapter"() {
given: "PBs server with aliases config"
def pbsConfig = ["adapters.openx.enabled" : "true",
Expand All @@ -209,6 +189,53 @@ class AliasSpec extends BaseSpec {
pbsServiceFactory.removeContainer(pbsConfig)
}

def "PBS should validate request as alias request and without any warnings when required properties in place"() {
given: "Default bid request with openx and alias bidder"
def bidRequest = BidRequest.defaultBidRequest.tap {
imp[0].ext.prebid.bidder.appNexus = AppNexus.default
imp[0].ext.prebid.bidder.generic = null
ext.prebid.aliases = [(APPNEXUS.value): OPENX]
}

when: "PBS processes auction request"
def bidResponse = pbsServiceWithAdditionalBidders.sendAuctionRequest(bidRequest)

then: "PBS contain http call for specific bidder"
def responseDebug = bidResponse.ext.debug
assert responseDebug.httpcalls.size() == 1
assert responseDebug.httpcalls[APPNEXUS.value]*.uri == ["$networkServiceContainer.rootUri/${APPNEXUS.value}/auction"]

and: "PBS should not contain any warnings"
assert !bidResponse.ext.warnings
}

def "PBS should validate request as alias request and emit proper warnings when validation fails for request"() {
given: "Default bid request with openx and alias bidder"
def bidRequest = BidRequest.defaultBidRequest.tap {
imp[0].ext.prebid.bidder.appNexus = new AppNexus()
imp[0].ext.prebid.bidder.generic = null
ext.prebid.aliases = [(APPNEXUS.value): OPENX]
}

when: "PBS processes auction request"
def bidResponse = pbsServiceWithAdditionalBidders.sendAuctionRequest(bidRequest)

then: "PBS shouldn't contain http calls"
assert !bidResponse.ext.debug.httpcalls

and: "Bid response should contain warning"
assert bidResponse.ext?.warnings[PREBID]?.code == [999, 999]
assert bidResponse.ext?.warnings[PREBID]*.message ==
["WARNING: request.imp[0].ext.prebid.bidder.${APPNEXUS.value} was dropped with a reason: " +
"request.imp[0].ext.prebid.bidder.${APPNEXUS.value} failed validation.\n" +
"\$.placement_id: is missing but it is required\n" +
"\$.member: is missing but it is required\n" +
"\$.placementId: is missing but it is required\n" +
"\$.inv_code: is missing but it is required\n" +
"\$.invCode: is missing but it is required",
"WARNING: request.imp[0].ext must contain at least one valid bidder"]
}

def "PBS should invoke as aliases when alias is unknown and core bidder is specified"() {
given: "Default bid request with generic and alias bidder"
def bidRequest = BidRequest.defaultBidRequest.tap {
Expand Down
37 changes: 37 additions & 0 deletions src/test/java/org/prebid/server/validation/ImpValidatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mock.Strictness.LENIENT;
import static org.mockito.Mockito.verify;

@ExtendWith(MockitoExtension.class)
public class ImpValidatorTest extends VertxTest {
Expand Down Expand Up @@ -1516,6 +1517,42 @@ public void validateImpsShouldReturnWarningMessageAndDropBidderWhenBidderExtIsIn
.containsOnly(mapper.createObjectNode());
}

@Test
public void validateImpsShouldReturnWarningMessageAndDropBidderWhenBidderExtIsInvalidAndBidderIsHardcodedAlias()
throws ValidationException {

// given
final List<Imp> givenImps = singletonList(validImpBuilder()
.ext(mapper.valueToTree(singletonMap("prebid", singletonMap("bidder", singletonMap("someAlias", 0)))))
.build());
given(bidderParamValidator.validate(any(), any()))
.willReturn(new LinkedHashSet<>(asList("errorMessage1", "errorMessage2")));
given(bidderCatalog.isValidName("someAlias")).willReturn(true);

final List<String> debugMessages = new ArrayList<>();

// when
target.validateImps(givenImps, Map.of("someAlias", "rubicon"), debugMessages);

// then
assertThat(debugMessages)
.containsExactly(
"""
WARNING: request.imp[0].ext.prebid.bidder.someAlias was dropped with a reason: \
request.imp[0].ext.prebid.bidder.someAlias failed validation.
errorMessage1
errorMessage2""",
"WARNING: request.imp[0].ext must contain at least one valid bidder");

assertThat(givenImps)
.extracting(Imp::getExt)
.extracting(impExt -> impExt.get("prebid"))
.extracting(prebid -> prebid.get("bidder"))
.containsOnly(mapper.createObjectNode());

verify(bidderParamValidator).validate(eq("someAlias"), any());
}

@Test
public void validateImpsShouldReturnWarningMessageAndDropBidderWhenImpExtPrebidImpBidderIsUnknown()
throws ValidationException {
Expand Down
Loading