Skip to content

Commit 9e04626

Browse files
AntoxaAntoxicAntoxaAntoxic
authored
Fix Bidder Aliases Validation (#3696)
1 parent 8b6d9e9 commit 9e04626

File tree

3 files changed

+133
-56
lines changed

3 files changed

+133
-56
lines changed

‎src/main/java/org/prebid/server/validation/ImpValidator.java‎

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.apache.commons.collections4.CollectionUtils;
2929
import org.apache.commons.lang3.StringUtils;
3030
import org.apache.commons.lang3.exception.ExceptionUtils;
31+
import org.prebid.server.auction.BidderAliases;
3132
import org.prebid.server.bidder.BidderCatalog;
3233
import org.prebid.server.json.JacksonMapper;
3334
import org.prebid.server.proto.openrtb.ext.request.ExtImpPrebid;
@@ -365,9 +366,10 @@ private void validateImpExt(ObjectNode ext, Map<String, String> aliases, int imp
365366
validateImpExtPrebid(ext != null ? ext.get(PREBID_EXT) : null, aliases, impIndex, warnings);
366367
}
367368

368-
private void validateImpExtPrebid(JsonNode extPrebidNode, Map<String, String> aliases, int impIndex,
369-
List<String> warnings)
370-
throws ValidationException {
369+
private void validateImpExtPrebid(JsonNode extPrebidNode,
370+
Map<String, String> requestAliases,
371+
int impIndex,
372+
List<String> warnings) throws ValidationException {
371373

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

390-
validateImpExtPrebidBidder(extPrebidBidderNode, extPrebid.getStoredAuctionResponse(),
391-
aliases, impIndex, warnings);
392-
validateImpExtPrebidStoredResponses(extPrebid, aliases, impIndex, warnings);
392+
final BidderAliases aliases = BidderAliases.of(requestAliases, null, bidderCatalog);
393393

394+
validateImpExtPrebidBidder(
395+
extPrebidBidderNode,
396+
extPrebid.getStoredAuctionResponse(),
397+
aliases,
398+
impIndex,
399+
warnings);
400+
401+
validateImpExtPrebidStoredResponses(extPrebid, aliases, impIndex, warnings);
394402
validateImpExtPrebidImp(extPrebidNode.get(IMP_EXT), aliases, impIndex, warnings);
395403
}
396404

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

418426
private void validateImpExtPrebidBidder(JsonNode extPrebidBidder,
419427
ExtStoredAuctionResponse storedAuctionResponse,
420-
Map<String, String> aliases,
428+
BidderAliases aliases,
421429
int impIndex,
422430
List<String> warnings) throws ValidationException {
423431
if (extPrebidBidder == null) {
@@ -433,7 +441,7 @@ private void validateImpExtPrebidBidder(JsonNode extPrebidBidder,
433441
final Map.Entry<String, JsonNode> bidderExtension = bidderExtensions.next();
434442
final String bidder = bidderExtension.getKey();
435443
try {
436-
validateImpBidderExtName(impIndex, bidderExtension, aliases.getOrDefault(bidder, bidder));
444+
validateImpBidderExtName(impIndex, bidderExtension, aliases.resolveBidder(bidder));
437445
} catch (ValidationException ex) {
438446
bidderExtensions.remove();
439447
warnings.add("WARNING: request.imp[%d].ext.prebid.bidder.%s was dropped with a reason: %s"
@@ -447,7 +455,7 @@ private void validateImpExtPrebidBidder(JsonNode extPrebidBidder,
447455
}
448456

449457
private void validateImpExtPrebidStoredResponses(ExtImpPrebid extPrebid,
450-
Map<String, String> aliases,
458+
BidderAliases aliases,
451459
int impIndex,
452460
List<String> warnings) throws ValidationException {
453461
final ExtStoredAuctionResponse extStoredAuctionResponse = extPrebid.getStoredAuctionResponse();
@@ -479,8 +487,11 @@ private void validateImpExtPrebidStoredResponses(ExtImpPrebid extPrebid,
479487
}
480488
}
481489

482-
private void validateStoredBidResponse(ExtStoredBidResponse extStoredBidResponse, ObjectNode bidderNode,
483-
Map<String, String> aliases, int impIndex) throws ValidationException {
490+
private void validateStoredBidResponse(ExtStoredBidResponse extStoredBidResponse,
491+
ObjectNode bidderNode,
492+
BidderAliases aliases,
493+
int impIndex) throws ValidationException {
494+
484495
final String bidder = extStoredBidResponse.getBidder();
485496
final String id = extStoredBidResponse.getId();
486497
if (StringUtils.isEmpty(bidder)) {
@@ -493,7 +504,7 @@ private void validateStoredBidResponse(ExtStoredBidResponse extStoredBidResponse
493504
"Id was not defined for request.imp[%d].ext.prebid.storedbidresponse.id".formatted(impIndex));
494505
}
495506

496-
final String resolvedBidder = aliases.getOrDefault(bidder, bidder);
507+
final String resolvedBidder = aliases.resolveBidder(bidder);
497508

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

521-
private void validateImpBidderExtName(int impIndex, Map.Entry<String, JsonNode> bidderExtension, String bidderName)
522-
throws ValidationException {
532+
private void validateImpBidderExtName(int impIndex,
533+
Map.Entry<String, JsonNode> bidderExtension,
534+
String bidderName) throws ValidationException {
535+
523536
if (bidderCatalog.isValidName(bidderName)) {
524537
final Set<String> messages = bidderParamValidator.validate(bidderName, bidderExtension.getValue());
525538
if (!messages.isEmpty()) {

‎src/test/groovy/org/prebid/server/functional/tests/AliasSpec.groovy‎

Lines changed: 67 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,41 @@
11
package org.prebid.server.functional.tests
22

3+
import org.prebid.server.functional.model.bidder.AppNexus
34
import org.prebid.server.functional.model.bidder.Generic
45
import org.prebid.server.functional.model.bidder.Openx
56
import org.prebid.server.functional.model.request.auction.BidRequest
67
import org.prebid.server.functional.service.PrebidServerException
7-
import org.prebid.server.functional.testcontainers.Dependencies
8+
import org.prebid.server.functional.service.PrebidServerService
89
import org.prebid.server.functional.util.PBSUtils
910

1011
import static io.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST
1112
import static org.prebid.server.functional.model.bidder.BidderName.ALIAS
13+
import static org.prebid.server.functional.model.bidder.BidderName.APPNEXUS
1214
import static org.prebid.server.functional.model.bidder.BidderName.BOGUS
1315
import static org.prebid.server.functional.model.bidder.BidderName.GENERIC
1416
import static org.prebid.server.functional.model.bidder.BidderName.GENER_X
1517
import static org.prebid.server.functional.model.bidder.BidderName.OPENX
1618
import static org.prebid.server.functional.model.bidder.CompressionType.GZIP
19+
import static org.prebid.server.functional.model.response.auction.ErrorType.PREBID
1720
import static org.prebid.server.functional.testcontainers.Dependencies.networkServiceContainer
1821
import static org.prebid.server.functional.util.HttpUtil.CONTENT_ENCODING_HEADER
1922

2023
class AliasSpec extends BaseSpec {
2124

25+
private static final Map<String, String> ADDITIONAL_BIDDERS_CONFIG = ["adapters.${OPENX.value}.enabled" : "true",
26+
"adapters.${OPENX.value}.endpoint" : "$networkServiceContainer.rootUri/${OPENX.value}/auction".toString(),
27+
"adapters.${APPNEXUS.value}.enabled" : "true",
28+
"adapters.${APPNEXUS.value}.endpoint": "$networkServiceContainer.rootUri/${APPNEXUS.value}/auction".toString()]
29+
private static PrebidServerService pbsServiceWithAdditionalBidders
30+
31+
def setupSpec() {
32+
pbsServiceWithAdditionalBidders = pbsServiceFactory.getService(ADDITIONAL_BIDDERS_CONFIG + GENERIC_ALIAS_CONFIG)
33+
}
34+
35+
def cleanupSpec() {
36+
pbsServiceFactory.removeContainer(ADDITIONAL_BIDDERS_CONFIG + GENERIC_ALIAS_CONFIG)
37+
}
38+
2239
def "PBS should be able to take alias from request"() {
2340
given: "Default bid request with alias"
2441
def bidRequest = BidRequest.defaultBidRequest.tap {
@@ -133,56 +150,19 @@ class AliasSpec extends BaseSpec {
133150
}
134151

135152
def "PBS aliased bidder config should be independently from parent"() {
136-
given: "Pbs config"
137-
def prebidServerService = pbsServiceFactory.getService(GENERIC_ALIAS_CONFIG)
138-
139-
and: "Default bid request with alias"
153+
given: "Default bid request with alias"
140154
def bidRequest = BidRequest.defaultBidRequest.tap {
141155
imp[0].ext.prebid.bidder.alias = new Generic()
142156
}
143157

144158
when: "PBS processes auction request"
145-
prebidServerService.sendAuctionRequest(bidRequest)
159+
pbsServiceWithAdditionalBidders.sendAuctionRequest(bidRequest)
146160

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

152-
def "PBS should ignore alias logic when hardcoded alias endpoints are present"() {
153-
given: "PBs server with aliases config"
154-
def pbsConfig = ["adapters.generic.aliases.alias.enabled" : "true",
155-
"adapters.generic.aliases.alias.endpoint": "$networkServiceContainer.rootUri/alias/auction".toString(),
156-
"adapters.openx.enabled" : "true",
157-
"adapters.openx.endpoint" : "$networkServiceContainer.rootUri/openx/auction".toString()]
158-
def pbsService = pbsServiceFactory.getService(pbsConfig)
159-
160-
and: "Default bid request with openx and alias bidder"
161-
def bidRequest = BidRequest.defaultBidRequest.tap {
162-
imp[0].ext.prebid.bidder.alias = new Generic()
163-
imp[0].ext.prebid.bidder.generic = new Generic()
164-
imp[0].ext.prebid.bidder.openx = new Openx()
165-
ext.prebid.aliases = [(ALIAS.value): OPENX]
166-
}
167-
168-
when: "PBS processes auction request"
169-
def bidResponse = pbsService.sendAuctionRequest(bidRequest)
170-
171-
then: "PBS should call only generic bidder"
172-
def responseDebug = bidResponse.ext.debug
173-
assert responseDebug.httpcalls[GENERIC.value]
174-
175-
and: "PBS shouldn't call only opexn,alias bidder"
176-
assert !responseDebug.httpcalls[OPENX.value]
177-
assert !responseDebug.httpcalls[ALIAS.value]
178-
179-
and: "PBS should call only generic bidder"
180-
assert bidder.getBidderRequest(bidRequest.id)
181-
182-
cleanup: "Stop and remove pbs container"
183-
pbsServiceFactory.removeContainer(pbsConfig)
184-
}
185-
186166
def "PBS should ignore aliases for requests with a base adapter"() {
187167
given: "PBs server with aliases config"
188168
def pbsConfig = ["adapters.openx.enabled" : "true",
@@ -209,6 +189,53 @@ class AliasSpec extends BaseSpec {
209189
pbsServiceFactory.removeContainer(pbsConfig)
210190
}
211191

192+
def "PBS should validate request as alias request and without any warnings when required properties in place"() {
193+
given: "Default bid request with openx and alias bidder"
194+
def bidRequest = BidRequest.defaultBidRequest.tap {
195+
imp[0].ext.prebid.bidder.appNexus = AppNexus.default
196+
imp[0].ext.prebid.bidder.generic = null
197+
ext.prebid.aliases = [(APPNEXUS.value): OPENX]
198+
}
199+
200+
when: "PBS processes auction request"
201+
def bidResponse = pbsServiceWithAdditionalBidders.sendAuctionRequest(bidRequest)
202+
203+
then: "PBS contain http call for specific bidder"
204+
def responseDebug = bidResponse.ext.debug
205+
assert responseDebug.httpcalls.size() == 1
206+
assert responseDebug.httpcalls[APPNEXUS.value]*.uri == ["$networkServiceContainer.rootUri/${APPNEXUS.value}/auction"]
207+
208+
and: "PBS should not contain any warnings"
209+
assert !bidResponse.ext.warnings
210+
}
211+
212+
def "PBS should validate request as alias request and emit proper warnings when validation fails for request"() {
213+
given: "Default bid request with openx and alias bidder"
214+
def bidRequest = BidRequest.defaultBidRequest.tap {
215+
imp[0].ext.prebid.bidder.appNexus = new AppNexus()
216+
imp[0].ext.prebid.bidder.generic = null
217+
ext.prebid.aliases = [(APPNEXUS.value): OPENX]
218+
}
219+
220+
when: "PBS processes auction request"
221+
def bidResponse = pbsServiceWithAdditionalBidders.sendAuctionRequest(bidRequest)
222+
223+
then: "PBS shouldn't contain http calls"
224+
assert !bidResponse.ext.debug.httpcalls
225+
226+
and: "Bid response should contain warning"
227+
assert bidResponse.ext?.warnings[PREBID]?.code == [999, 999]
228+
assert bidResponse.ext?.warnings[PREBID]*.message ==
229+
["WARNING: request.imp[0].ext.prebid.bidder.${APPNEXUS.value} was dropped with a reason: " +
230+
"request.imp[0].ext.prebid.bidder.${APPNEXUS.value} failed validation.\n" +
231+
"\$.placement_id: is missing but it is required\n" +
232+
"\$.member: is missing but it is required\n" +
233+
"\$.placementId: is missing but it is required\n" +
234+
"\$.inv_code: is missing but it is required\n" +
235+
"\$.invCode: is missing but it is required",
236+
"WARNING: request.imp[0].ext must contain at least one valid bidder"]
237+
}
238+
212239
def "PBS should invoke as aliases when alias is unknown and core bidder is specified"() {
213240
given: "Default bid request with generic and alias bidder"
214241
def bidRequest = BidRequest.defaultBidRequest.tap {

‎src/test/java/org/prebid/server/validation/ImpValidatorTest.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import static org.mockito.ArgumentMatchers.eq;
5050
import static org.mockito.BDDMockito.given;
5151
import static org.mockito.Mock.Strictness.LENIENT;
52+
import static org.mockito.Mockito.verify;
5253

5354
@ExtendWith(MockitoExtension.class)
5455
public class ImpValidatorTest extends VertxTest {
@@ -1516,6 +1517,42 @@ public void validateImpsShouldReturnWarningMessageAndDropBidderWhenBidderExtIsIn
15161517
.containsOnly(mapper.createObjectNode());
15171518
}
15181519

1520+
@Test
1521+
public void validateImpsShouldReturnWarningMessageAndDropBidderWhenBidderExtIsInvalidAndBidderIsHardcodedAlias()
1522+
throws ValidationException {
1523+
1524+
// given
1525+
final List<Imp> givenImps = singletonList(validImpBuilder()
1526+
.ext(mapper.valueToTree(singletonMap("prebid", singletonMap("bidder", singletonMap("someAlias", 0)))))
1527+
.build());
1528+
given(bidderParamValidator.validate(any(), any()))
1529+
.willReturn(new LinkedHashSet<>(asList("errorMessage1", "errorMessage2")));
1530+
given(bidderCatalog.isValidName("someAlias")).willReturn(true);
1531+
1532+
final List<String> debugMessages = new ArrayList<>();
1533+
1534+
// when
1535+
target.validateImps(givenImps, Map.of("someAlias", "rubicon"), debugMessages);
1536+
1537+
// then
1538+
assertThat(debugMessages)
1539+
.containsExactly(
1540+
"""
1541+
WARNING: request.imp[0].ext.prebid.bidder.someAlias was dropped with a reason: \
1542+
request.imp[0].ext.prebid.bidder.someAlias failed validation.
1543+
errorMessage1
1544+
errorMessage2""",
1545+
"WARNING: request.imp[0].ext must contain at least one valid bidder");
1546+
1547+
assertThat(givenImps)
1548+
.extracting(Imp::getExt)
1549+
.extracting(impExt -> impExt.get("prebid"))
1550+
.extracting(prebid -> prebid.get("bidder"))
1551+
.containsOnly(mapper.createObjectNode());
1552+
1553+
verify(bidderParamValidator).validate(eq("someAlias"), any());
1554+
}
1555+
15191556
@Test
15201557
public void validateImpsShouldReturnWarningMessageAndDropBidderWhenImpExtPrebidImpBidderIsUnknown()
15211558
throws ValidationException {

0 commit comments

Comments
 (0)