From e0c513d512e7a45d7d55566bdfb479d3cfac4262 Mon Sep 17 00:00:00 2001 From: antonbabak Date: Fri, 7 Feb 2025 16:23:19 +0100 Subject: [PATCH 1/4] Use a hard-alias schema if present --- .../validation/BidderParamValidator.java | 19 ++++++++++++---- .../bidder/adtrgtme/AdtrgtmeBidderTest.java | 5 ++--- .../org/prebid/server/it/ApplicationTest.java | 10 ++++++++- .../validation/BidderParamValidatorTest.java | 22 +++++++++++++++++-- 4 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/prebid/server/validation/BidderParamValidator.java b/src/main/java/org/prebid/server/validation/BidderParamValidator.java index 98dea900d5c..71a21b13341 100644 --- a/src/main/java/org/prebid/server/validation/BidderParamValidator.java +++ b/src/main/java/org/prebid/server/validation/BidderParamValidator.java @@ -7,7 +7,6 @@ import com.networknt.schema.SpecVersion; import com.networknt.schema.ValidationMessage; import org.apache.commons.collections4.map.CaseInsensitiveMap; -import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; import org.prebid.server.bidder.BidderCatalog; import org.prebid.server.json.EncodeException; @@ -88,7 +87,7 @@ public static BidderParamValidator create(BidderCatalog bidderCatalog, final Map bidderRawSchemas = new LinkedHashMap<>(); bidderCatalog.names().forEach(bidder -> bidderRawSchemas.put( - bidder, createSchemaNode(schemaDirectory, maybeResolveAlias(bidderCatalog, bidder), mapper))); + bidder, createSchemaNode(bidderCatalog, schemaDirectory, bidder, mapper))); return new BidderParamValidator(toBidderSchemas(bidderRawSchemas), toSchemas(bidderRawSchemas, mapper)); } @@ -120,8 +119,20 @@ private static JsonSchema toBidderSchema(JsonNode schema, String bidder) { return result; } - private static String maybeResolveAlias(BidderCatalog bidderCatalog, String bidder) { - return ObjectUtils.defaultIfNull(bidderCatalog.bidderInfoByName(bidder).getAliasOf(), bidder); + private static JsonNode createSchemaNode(BidderCatalog bidderCatalog, + String schemaDirectory, + String bidder, + JacksonMapper mapper) { + + try { + return createSchemaNode(schemaDirectory, bidder, mapper); + } catch (IllegalArgumentException e) { + final String parentBidder = bidderCatalog.bidderInfoByName(bidder).getAliasOf(); + if (parentBidder != null) { + return createSchemaNode(schemaDirectory, parentBidder, mapper); + } + throw e; + } } private static JsonNode createSchemaNode(String schemaDirectory, String bidder, JacksonMapper mapper) { diff --git a/src/test/java/org/prebid/server/bidder/adtrgtme/AdtrgtmeBidderTest.java b/src/test/java/org/prebid/server/bidder/adtrgtme/AdtrgtmeBidderTest.java index af2b0674f7b..55ae12b0c37 100644 --- a/src/test/java/org/prebid/server/bidder/adtrgtme/AdtrgtmeBidderTest.java +++ b/src/test/java/org/prebid/server/bidder/adtrgtme/AdtrgtmeBidderTest.java @@ -21,7 +21,6 @@ import org.prebid.server.bidder.model.HttpResponse; import org.prebid.server.bidder.model.Result; import org.prebid.server.proto.openrtb.ext.ExtPrebid; -import org.prebid.server.proto.openrtb.ext.request.adrino.ExtImpAdrino; import org.prebid.server.proto.openrtb.ext.request.adtrgtme.ExtImpAdtrgtme; import org.prebid.server.util.HttpUtil; @@ -129,9 +128,9 @@ public void makeHttpRequestsShouldSplitRequestIntoMultipleRequests() { // given final BidRequest bidRequest = BidRequest.builder().site(Site.builder().id("site_id").build()) .imp(asList(givenImp(impBuilder -> impBuilder.xNative(Native.builder().build()) - .ext(mapper.valueToTree(ExtPrebid.of(null, ExtImpAdrino.of("test"))))), + .ext(mapper.valueToTree(ExtPrebid.of(null, ExtImpAdtrgtme.of(1))))), givenImp(impBuilder -> impBuilder.xNative(Native.builder().build()) - .ext(mapper.valueToTree(ExtPrebid.of(null, ExtImpAdrino.of("test"))))))).build(); + .ext(mapper.valueToTree(ExtPrebid.of(null, ExtImpAdtrgtme.of(1))))))).build(); // when final Result>> result = target.makeHttpRequests(bidRequest); diff --git a/src/test/java/org/prebid/server/it/ApplicationTest.java b/src/test/java/org/prebid/server/it/ApplicationTest.java index 16c7a550c52..ca4414b8897 100644 --- a/src/test/java/org/prebid/server/it/ApplicationTest.java +++ b/src/test/java/org/prebid/server/it/ApplicationTest.java @@ -451,7 +451,7 @@ public void biddersParamsShouldReturnBidderSchemas() throws JSONException, IOExc final Map expectedMap = CollectionUtils.union(bidders, aliases.keySet()).stream() .collect(Collectors.toMap( Function.identity(), - bidderName -> jsonSchemaToJsonNode(aliases.getOrDefault(bidderName, bidderName)))); + bidderName -> jsonSchemaToJsonNode(bidderName, aliases))); assertThat(responseAsMap.keySet()).hasSameElementsAs(expectedMap.keySet()); assertThat(responseAsMap).containsAllEntriesOf(expectedMap); @@ -693,6 +693,14 @@ private static Map getBidderAliasesFromConfigFiles() throws IOEx return Collections.emptyMap(); } + private static JsonNode jsonSchemaToJsonNode(String bidderName, Map aliases) { + try { + return jsonSchemaToJsonNode(bidderName); + } catch (IllegalArgumentException e) { + return jsonSchemaToJsonNode(aliases.getOrDefault(bidderName, bidderName)); + } + } + private static JsonNode jsonSchemaToJsonNode(String bidderName) { final String path = "static/bidder-params/" + bidderName + ".json"; try { diff --git a/src/test/java/org/prebid/server/validation/BidderParamValidatorTest.java b/src/test/java/org/prebid/server/validation/BidderParamValidatorTest.java index 674c4026a72..c0b99d22f38 100644 --- a/src/test/java/org/prebid/server/validation/BidderParamValidatorTest.java +++ b/src/test/java/org/prebid/server/validation/BidderParamValidatorTest.java @@ -9,6 +9,7 @@ import org.prebid.server.VertxTest; import org.prebid.server.bidder.BidderCatalog; import org.prebid.server.bidder.BidderInfo; +import org.prebid.server.proto.openrtb.ext.request.adrino.ExtImpAdrino; import org.prebid.server.proto.openrtb.ext.request.adtelligent.ExtImpAdtelligent; import org.prebid.server.proto.openrtb.ext.request.appnexus.ExtImpAppnexus; import org.prebid.server.proto.openrtb.ext.request.audiencenetwork.ExtImpAudienceNetwork; @@ -33,6 +34,7 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; +import static org.mockito.Mock.Strictness.LENIENT; @ExtendWith(MockitoExtension.class) public class BidderParamValidatorTest extends VertxTest { @@ -47,8 +49,9 @@ public class BidderParamValidatorTest extends VertxTest { private static final String EPLANNING = "eplanning"; private static final String BEACHFRONT = "beachfront"; private static final String VISX = "visx"; + private static final String ADRINO = "adrino"; - @Mock + @Mock(strictness = LENIENT) private BidderCatalog bidderCatalog; private BidderParamValidator bidderParamValidator; @@ -65,7 +68,8 @@ public void setUp() { OPENX, EPLANNING, BEACHFRONT, - VISX))); + VISX, + ADRINO))); given(bidderCatalog.bidderInfoByName(anyString())).willReturn(givenBidderInfo()); given(bidderCatalog.bidderInfoByName(eq(APPNEXUS_ALIAS))).willReturn(givenBidderInfo(APPNEXUS)); @@ -381,6 +385,20 @@ public void validateShouldReturnNoValidationMessagesWhenVisxUidValid() { assertThat(messagesIntegerUid).isEmpty(); } + @Test + public void validateShouldReturnValidationMessagesWhenAdrinoImpExtNotValid() { + // given + final ExtImpAdrino ext = ExtImpAdrino.of(null); + + final JsonNode node = mapper.convertValue(ext, JsonNode.class); + + // when + final Set messages = bidderParamValidator.validate(ADRINO, node); + + // then + assertThat(messages.size()).isEqualTo(1); + } + private static BidderInfo givenBidderInfo(String aliasOf) { return BidderInfo.create( true, From 7790491f6f386ba377c7feb0222fcbd03da68216 Mon Sep 17 00:00:00 2001 From: markiian Date: Fri, 14 Feb 2025 17:22:08 +0200 Subject: [PATCH 2/4] Minor update --- .../model/request/auction/Adrino.groovy | 6 ++ .../model/request/auction/Bidder.groovy | 2 + .../functional/tests/BidderParamsSpec.groovy | 63 +++++++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 src/test/groovy/org/prebid/server/functional/model/request/auction/Adrino.groovy diff --git a/src/test/groovy/org/prebid/server/functional/model/request/auction/Adrino.groovy b/src/test/groovy/org/prebid/server/functional/model/request/auction/Adrino.groovy new file mode 100644 index 00000000000..f93fa6bd4be --- /dev/null +++ b/src/test/groovy/org/prebid/server/functional/model/request/auction/Adrino.groovy @@ -0,0 +1,6 @@ +package org.prebid.server.functional.model.request.auction + +class Adrino { + + Object hash +} diff --git a/src/test/groovy/org/prebid/server/functional/model/request/auction/Bidder.groovy b/src/test/groovy/org/prebid/server/functional/model/request/auction/Bidder.groovy index 05a43bbb528..125faa37c4b 100644 --- a/src/test/groovy/org/prebid/server/functional/model/request/auction/Bidder.groovy +++ b/src/test/groovy/org/prebid/server/functional/model/request/auction/Bidder.groovy @@ -24,6 +24,8 @@ class Bidder { Ix ix @JsonProperty("openxalias") Openx openxAlias + Adrino adrino + Generic nativo static Bidder getDefaultBidder() { new Bidder().tap { diff --git a/src/test/groovy/org/prebid/server/functional/tests/BidderParamsSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/BidderParamsSpec.groovy index 6f69bdda378..e8151762026 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/BidderParamsSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/BidderParamsSpec.groovy @@ -6,6 +6,7 @@ import org.prebid.server.functional.model.db.Account import org.prebid.server.functional.model.db.StoredImp import org.prebid.server.functional.model.db.StoredRequest import org.prebid.server.functional.model.request.amp.AmpRequest +import org.prebid.server.functional.model.request.auction.Adrino import org.prebid.server.functional.model.request.auction.AuctionEnvironment import org.prebid.server.functional.model.request.auction.Banner import org.prebid.server.functional.model.request.auction.BidRequest @@ -1298,4 +1299,66 @@ class BidderParamsSpec extends BaseSpec { assert bidderRequest.imp[0].ext.auctionEnvironment == extAuctionEnv assert bidderRequest.imp[0].ext.interestGroupAuctionSupports.auctionEnvironment == extIgsAuctionEnv } + + def "PBS should reject alias bidders when bidder params from request doesn't satisfy own json-schema"() { + given: "Default bid request" + def bidRequest = BidRequest.defaultBidRequest.tap { + imp[0].ext.prebid.bidder.tap { + it.generic.exampleProperty = PBSUtils.randomNumber + //Adrino hard coded bidder alias in generic.yaml + it.adrino = new Adrino(hash: PBSUtils.randomNumber) + } + } + + when: "PBS processes auction request" + def response = defaultPbsService.sendAuctionRequest(bidRequest) + + then: "Bidder should be dropped" + assert response.ext?.warnings[PREBID]*.code == [999, 999, 999] + assert response.ext?.warnings[PREBID]*.message == + ["WARNING: request.imp[0].ext.prebid.bidder.generic was dropped with a reason: " + + "request.imp[0].ext.prebid.bidder.generic failed validation.\n" + + "\$.exampleProperty: integer found, string expected", + "WARNING: request.imp[0].ext.prebid.bidder.adrino was dropped with a reason: " + + "request.imp[0].ext.prebid.bidder.adrino failed validation.\n" + + "\$.hash: integer found, string expected", + "WARNING: request.imp[0].ext must contain at least one valid bidder"] + + and: "PBS should not call bidder" + assert bidder.getRequestCount(bidRequest.id) == 0 + + and: "targeting should be empty" + assert response.seatbid.isEmpty() + } + + def "PBS should reject alias bidders when bidder params from request doesn't satisfy aliased json-schema"() { + given: "Default basic generic BidRequest" + def bidRequest = BidRequest.defaultBidRequest.tap { + imp[0].ext.prebid.bidder.tap { + it.generic.exampleProperty = PBSUtils.randomNumber + //Nativo hard coded bidder alias in generic.yaml + it.nativo = new Generic(exampleProperty: PBSUtils.randomNumber) + } + } + + when: "PBS processes auction request" + def response = defaultPbsService.sendAuctionRequest(bidRequest) + + then: "Bidder should be dropped" + assert response.ext?.warnings[PREBID]*.code == [999, 999, 999] + assert response.ext?.warnings[PREBID]*.message == + ["WARNING: request.imp[0].ext.prebid.bidder.generic was dropped with a reason: " + + "request.imp[0].ext.prebid.bidder.generic failed validation.\n" + + "\$.exampleProperty: integer found, string expected", + "WARNING: request.imp[0].ext.prebid.bidder.nativo was dropped with a reason: " + + "request.imp[0].ext.prebid.bidder.nativo failed validation.\n" + + "\$.exampleProperty: integer found, string expected", + "WARNING: request.imp[0].ext must contain at least one valid bidder"] + + and: "PBS should not call bidder" + assert bidder.getRequestCount(bidRequest.id) == 0 + + and: "targeting should be empty" + assert response.seatbid.isEmpty() + } } From 19c46b5be795885f0a620567900720b78cfb17ad Mon Sep 17 00:00:00 2001 From: markiian Date: Fri, 14 Feb 2025 17:23:39 +0200 Subject: [PATCH 3/4] Minor update --- .../server/functional/model/request/auction/Adrino.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/groovy/org/prebid/server/functional/model/request/auction/Adrino.groovy b/src/test/groovy/org/prebid/server/functional/model/request/auction/Adrino.groovy index f93fa6bd4be..2d1675194e9 100644 --- a/src/test/groovy/org/prebid/server/functional/model/request/auction/Adrino.groovy +++ b/src/test/groovy/org/prebid/server/functional/model/request/auction/Adrino.groovy @@ -2,5 +2,5 @@ package org.prebid.server.functional.model.request.auction class Adrino { - Object hash + Integer hash } From a617371978cef438329433873067262eeafc64f3 Mon Sep 17 00:00:00 2001 From: markiian Date: Mon, 17 Feb 2025 12:33:52 +0200 Subject: [PATCH 4/4] Minor update --- .../testcontainers/PbsConfig.groovy | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/test/groovy/org/prebid/server/functional/testcontainers/PbsConfig.groovy b/src/test/groovy/org/prebid/server/functional/testcontainers/PbsConfig.groovy index aa10ebaf7a4..91ec927b1c2 100644 --- a/src/test/groovy/org/prebid/server/functional/testcontainers/PbsConfig.groovy +++ b/src/test/groovy/org/prebid/server/functional/testcontainers/PbsConfig.groovy @@ -89,25 +89,25 @@ LIMIT 1 } static Map getMySqlConfig(MySQLContainer mysql = Dependencies.mysqlContainer) { - ["settings.database.type" : "mysql", - "settings.database.host" : mysql.getNetworkAliases().get(0), - "settings.database.port" : mysql.exposedPorts.get(0) as String, - "settings.database.dbname" : mysql.databaseName, - "settings.database.user" : mysql.username, - "settings.database.password" : mysql.password, - "settings.database.pool-size" : "2", // setting 2 here to leave some slack for the PBS + ["settings.database.type" : "mysql", + "settings.database.host" : mysql.getNetworkAliases().get(0), + "settings.database.port" : mysql.exposedPorts.get(0) as String, + "settings.database.dbname" : mysql.databaseName, + "settings.database.user" : mysql.username, + "settings.database.password" : mysql.password, + "settings.database.pool-size" : "2", // setting 2 here to leave some slack for the PBS "settings.database.idle-connection-timeout": "300" ].asImmutable() } static Map getPostgreSqlConfig(PostgreSQLContainer postgres = Dependencies.postgresqlContainer) { - ["settings.database.type" : "postgres", - "settings.database.host" : postgres.getNetworkAliases().get(0), - "settings.database.port" : postgres.exposedPorts.get(0) as String, - "settings.database.dbname" : postgres.databaseName, - "settings.database.user" : postgres.username, - "settings.database.password" : postgres.password, - "settings.database.pool-size" : "2", // setting 2 here to leave some slack for the PBS + ["settings.database.type" : "postgres", + "settings.database.host" : postgres.getNetworkAliases().get(0), + "settings.database.port" : postgres.exposedPorts.get(0) as String, + "settings.database.dbname" : postgres.databaseName, + "settings.database.user" : postgres.username, + "settings.database.password" : postgres.password, + "settings.database.pool-size" : "2", // setting 2 here to leave some slack for the PBS "settings.database.idle-connection-timeout": "300" ].asImmutable() } @@ -119,6 +119,7 @@ LIMIT 1 // due to a config validation we'll need to circumvent all future aliases this way static Map getBidderAliasConfig() { ["adapters.generic.aliases.cwire.meta-info.site-media-types" : "", + "adapters.generic.aliases.cwire.meta-info.app-media-types" : "", "adapters.generic.aliases.blue.meta-info.app-media-types" : "", "adapters.generic.aliases.blue.meta-info.site-media-types" : "", "adapters.generic.aliases.adsinteractive.meta-info.app-media-types" : "",