diff --git a/src/main/java/org/prebid/server/auction/BidderAliases.java b/src/main/java/org/prebid/server/auction/BidderAliases.java index b9108f589c1..87ad01efe3b 100644 --- a/src/main/java/org/prebid/server/auction/BidderAliases.java +++ b/src/main/java/org/prebid/server/auction/BidderAliases.java @@ -39,7 +39,9 @@ public boolean isAliasDefined(String alias) { } public String resolveBidder(String aliasOrBidder) { - return aliasToBidder.getOrDefault(aliasOrBidder, aliasOrBidder); + return bidderCatalog.isValidName(aliasOrBidder) + ? aliasOrBidder + : aliasToBidder.getOrDefault(aliasOrBidder, aliasOrBidder); } public boolean isSame(String bidder1, String bidder2) { @@ -47,9 +49,8 @@ public boolean isSame(String bidder1, String bidder2) { } public Integer resolveAliasVendorId(String alias) { - return aliasToVendorId.containsKey(alias) - ? aliasToVendorId.get(alias) - : resolveAliasVendorIdViaCatalog(alias); + final Integer vendorId = resolveAliasVendorIdViaCatalog(alias); + return vendorId == null ? aliasToVendorId.get(alias) : vendorId; } private Integer resolveAliasVendorIdViaCatalog(String alias) { diff --git a/src/main/java/org/prebid/server/auction/privacy/enforcement/TcfEnforcement.java b/src/main/java/org/prebid/server/auction/privacy/enforcement/TcfEnforcement.java index 86602099401..933ba724f0f 100644 --- a/src/main/java/org/prebid/server/auction/privacy/enforcement/TcfEnforcement.java +++ b/src/main/java/org/prebid/server/auction/privacy/enforcement/TcfEnforcement.java @@ -73,7 +73,7 @@ public Future> enforce(AuctionContext auctionContext, return tcfDefinerService.resultForBidderNames( bidders, - VendorIdResolver.of(aliases, bidderCatalog), + VendorIdResolver.of(aliases), auctionContext.getPrivacyContext().getTcfContext(), accountGdprConfig(auctionContext.getAccount())) .map(TcfResponse::getActions) diff --git a/src/main/java/org/prebid/server/privacy/gdpr/VendorIdResolver.java b/src/main/java/org/prebid/server/privacy/gdpr/VendorIdResolver.java index 3076d653030..5c646d3b925 100644 --- a/src/main/java/org/prebid/server/privacy/gdpr/VendorIdResolver.java +++ b/src/main/java/org/prebid/server/privacy/gdpr/VendorIdResolver.java @@ -6,30 +6,20 @@ public class VendorIdResolver { private final BidderAliases aliases; - private final BidderCatalog bidderCatalog; - private VendorIdResolver(BidderAliases aliases, BidderCatalog bidderCatalog) { + private VendorIdResolver(BidderAliases aliases) { this.aliases = aliases; - this.bidderCatalog = bidderCatalog; } - public static VendorIdResolver of(BidderAliases aliases, BidderCatalog bidderCatalog) { - return new VendorIdResolver(aliases, bidderCatalog); + public static VendorIdResolver of(BidderAliases aliases) { + return new VendorIdResolver(aliases); } public static VendorIdResolver of(BidderCatalog bidderCatalog) { - return of(null, bidderCatalog); + return of(BidderAliases.of(null, null, bidderCatalog)); } public Integer resolve(String aliasOrBidder) { - final Integer requestAliasVendorId = aliases != null ? aliases.resolveAliasVendorId(aliasOrBidder) : null; - - return requestAliasVendorId != null ? requestAliasVendorId : resolveViaCatalog(aliasOrBidder); - } - - private Integer resolveViaCatalog(String aliasOrBidder) { - final String bidderName = aliases != null ? aliases.resolveBidder(aliasOrBidder) : aliasOrBidder; - - return bidderCatalog.isActive(bidderName) ? bidderCatalog.vendorIdByName(bidderName) : null; + return aliases != null ? aliases.resolveAliasVendorId(aliasOrBidder) : null; } } diff --git a/src/test/groovy/org/prebid/server/functional/model/bidder/BidderName.groovy b/src/test/groovy/org/prebid/server/functional/model/bidder/BidderName.groovy index f91f209395c..352080844ab 100644 --- a/src/test/groovy/org/prebid/server/functional/model/bidder/BidderName.groovy +++ b/src/test/groovy/org/prebid/server/functional/model/bidder/BidderName.groovy @@ -13,6 +13,7 @@ enum BidderName { ALIAS_CAMEL_CASE("AlIaS"), GENERIC_CAMEL_CASE("GeNerIc"), GENERIC("generic"), + GENER_X("gener_x"), RUBICON("rubicon"), APPNEXUS("appnexus"), RUBICON_ALIAS("rubiconAlias"), 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 a1078731f44..605f286c803 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 @@ -13,6 +13,8 @@ class Bidder { Generic alias Generic generic + @JsonProperty("gener_x") + Generic generX @JsonProperty("GeNerIc") Generic genericCamelCase Rubicon rubicon diff --git a/src/test/groovy/org/prebid/server/functional/tests/AliasSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/AliasSpec.groovy index 15cd0668f6c..d4b8f84c826 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/AliasSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/AliasSpec.groovy @@ -1,16 +1,20 @@ package org.prebid.server.functional.tests 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.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.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.testcontainers.Dependencies.getNetworkServiceContainer +import static org.prebid.server.functional.testcontainers.Dependencies.networkServiceContainer import static org.prebid.server.functional.util.HttpUtil.CONTENT_ENCODING_HEADER class AliasSpec extends BaseSpec { @@ -144,4 +148,101 @@ class AliasSpec extends BaseSpec { 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", + "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.openx = Openx.defaultOpenx + imp[0].ext.prebid.bidder.generic = new Generic() + ext.prebid.aliases = [(OPENX.value): GENERIC] + } + + when: "PBS processes auction request" + def bidResponse = pbsService.sendAuctionRequest(bidRequest) + + then: "PBS contain two http calls and the different url for both" + def responseDebug = bidResponse.ext.debug + assert responseDebug.httpcalls.size() == 2 + assert responseDebug.httpcalls[OPENX.value]*.uri == ["$networkServiceContainer.rootUri/openx/auction"] + assert responseDebug.httpcalls[GENERIC.value]*.uri == ["$networkServiceContainer.rootUri/auction"] + + cleanup: "Stop and remove pbs container" + pbsServiceFactory.removeContainer(pbsConfig) + } + + 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 { + imp[0].ext.prebid.bidder.generX = new Generic() + ext.prebid.aliases = [(GENER_X.value): GENERIC] + } + + when: "PBS processes auction request" + def bidResponse = defaultPbsService.sendAuctionRequest(bidRequest) + + then: "PBS contain two http calls and the same url for both" + def responseDebug = bidResponse.ext.debug + assert responseDebug.httpcalls.size() == 2 + assert responseDebug.httpcalls[GENER_X.value]*.uri == responseDebug.httpcalls[GENERIC.value]*.uri + + and: "Bidder request should contain request per-alies" + def bidderRequests = bidder.getBidderRequests(bidRequest.id) + assert bidderRequests.size() == 2 + } + + def "PBS should invoke aliases when alias is unknown and no core bidder is specified"() { + given: "Default bid request with generic and alias bidder" + def bidRequest = BidRequest.defaultBidRequest.tap { + imp[0].ext.prebid.bidder.generX = new Generic() + imp[0].ext.prebid.bidder.generic = null + ext.prebid.aliases = [(GENER_X.value): GENERIC] + } + + when: "PBS processes auction request" + def bidResponse = defaultPbsService.sendAuctionRequest(bidRequest) + + then: "PBS contain two http calls and the same url for both" + def responseDebug = bidResponse.ext.debug + assert responseDebug.httpcalls.size() == 1 + assert responseDebug.httpcalls[GENER_X.value] + } } diff --git a/src/test/groovy/org/prebid/server/functional/tests/ImpRequestSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/ImpRequestSpec.groovy index 085bd19a690..4cc3b728449 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/ImpRequestSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/ImpRequestSpec.groovy @@ -1,5 +1,6 @@ package org.prebid.server.functional.tests +import org.prebid.server.functional.model.bidder.Generic import org.prebid.server.functional.model.bidder.Openx import org.prebid.server.functional.model.db.StoredImp import org.prebid.server.functional.model.request.auction.BidRequest @@ -103,8 +104,10 @@ class ImpRequestSpec extends BaseSpec { imp.first.tap { pmp = Pmp.defaultPmp ext.prebid.imp = [(aliasName): new Imp(pmp: extPmp)] + ext.prebid.bidder.generic = null + ext.prebid.bidder.alias = new Generic() } - ext.prebid.aliases = [(aliasName.value): GENERIC] + ext.prebid.aliases = [(aliasName.value): bidderName] } when: "Requesting PBS auction" diff --git a/src/test/java/org/prebid/server/auction/BidderAliasesTest.java b/src/test/java/org/prebid/server/auction/BidderAliasesTest.java index e75a3c9f0f4..30fc23a21fa 100644 --- a/src/test/java/org/prebid/server/auction/BidderAliasesTest.java +++ b/src/test/java/org/prebid/server/auction/BidderAliasesTest.java @@ -1,5 +1,6 @@ package org.prebid.server.auction; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -10,13 +11,22 @@ import static java.util.Collections.singletonMap; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mock.Strictness.LENIENT; @ExtendWith(MockitoExtension.class) public class BidderAliasesTest { - @Mock + @Mock(strictness = LENIENT) private BidderCatalog bidderCatalog; + @BeforeEach + public void before() { + given(bidderCatalog.isValidName(any())).willReturn(false); + given(bidderCatalog.isActive(any())).willReturn(false); + } + @Test public void isAliasDefinedShouldReturnFalseWhenNoAliasesInRequest() { // given @@ -53,6 +63,16 @@ public void resolveBidderShouldReturnInputWhenNoAliasesInRequest() { assertThat(aliases.resolveBidder("alias")).isEqualTo("alias"); } + @Test + public void resolveBidderShouldReturnInputWhenNoAliasesInRequestButAliasIsValidInBidderCatalog() { + // given + given(bidderCatalog.isValidName("alias")).willReturn(true); + final BidderAliases aliases = BidderAliases.of(null, null, bidderCatalog); + + // when and then + assertThat(aliases.resolveBidder("alias")).isEqualTo("alias"); + } + @Test public void resolveBidderShouldReturnInputWhenAliasIsNotDefinedInRequest() { // given @@ -62,6 +82,16 @@ public void resolveBidderShouldReturnInputWhenAliasIsNotDefinedInRequest() { assertThat(aliases.resolveBidder("alias")).isEqualTo("alias"); } + @Test + public void resolveBidderShouldReturnInputWhenAliasIsNotDefinedInRequestButAliasIsValidInBidderCatalog() { + // given + given(bidderCatalog.isValidName("alias")).willReturn(true); + final BidderAliases aliases = BidderAliases.of(singletonMap("anotherAlias", "bidder"), null, bidderCatalog); + + // when and then + assertThat(aliases.resolveBidder("alias")).isEqualTo("alias"); + } + @Test public void resolveBidderShouldDetectAliasInRequest() { // given @@ -71,6 +101,16 @@ public void resolveBidderShouldDetectAliasInRequest() { assertThat(aliases.resolveBidder("alias")).isEqualTo("bidder"); } + @Test + public void resolveBidderShouldDetectInBidderCatalogWhenItIsValid() { + // given + given(bidderCatalog.isValidName("alias")).willReturn(true); + final BidderAliases aliases = BidderAliases.of(singletonMap("alias", "bidder"), null, bidderCatalog); + + // when and then + assertThat(aliases.resolveBidder("alias")).isEqualTo("alias"); + } + @Test public void isSameShouldReturnTrueIfBiddersSameConsideringAliases() { // given @@ -110,6 +150,17 @@ public void resolveAliasVendorIdShouldReturnNullWhenNoVendorIdsInRequest() { assertThat(aliases.resolveAliasVendorId("alias")).isNull(); } + @Test + public void resolveAliasVendorIdShouldReturnVendorIdFromBidderCatalogWhenNoVendorIdsInRequest() { + // given + given(bidderCatalog.isActive("alias")).willReturn(true); + given(bidderCatalog.vendorIdByName("alias")).willReturn(3); + final BidderAliases aliases = BidderAliases.of(null, null, bidderCatalog); + + // when and then + assertThat(aliases.resolveAliasVendorId("alias")).isEqualTo(3); + } + @Test public void resolveAliasVendorIdShouldReturnNullWhenVendorIdIsNotDefinedInRequest() { // given @@ -120,11 +171,24 @@ public void resolveAliasVendorIdShouldReturnNullWhenVendorIdIsNotDefinedInReques } @Test - public void resolveAliasVendorIdShouldDetectVendorIdInRequest() { + public void resolveAliasVendorIdShouldReturnVendorIdFromBidderCatalogWhenVendorIdIsNotDefinedInRequest() { + // given + given(bidderCatalog.isActive("alias")).willReturn(true); + given(bidderCatalog.vendorIdByName("alias")).willReturn(3); + final BidderAliases aliases = BidderAliases.of(null, singletonMap("anotherAlias", 2), bidderCatalog); + + // when and then + assertThat(aliases.resolveAliasVendorId("alias")).isEqualTo(3); + } + + @Test + public void resolveAliasVendorIdShouldReturnVendorIdFromBidderCatalogWhenVendorIdIsInRequest() { // given + given(bidderCatalog.isActive("alias")).willReturn(true); + given(bidderCatalog.vendorIdByName("alias")).willReturn(3); final BidderAliases aliases = BidderAliases.of(null, singletonMap("alias", 2), bidderCatalog); // when and then - assertThat(aliases.resolveAliasVendorId("alias")).isEqualTo(2); + assertThat(aliases.resolveAliasVendorId("alias")).isEqualTo(3); } } diff --git a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java index bfecf6632f9..1c6c3ff1d3e 100644 --- a/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java +++ b/src/test/java/org/prebid/server/auction/ExchangeServiceTest.java @@ -1086,6 +1086,8 @@ public void shouldReturnProperStoredResponseIfAvailableOnlySingleImpRequests() { @Test public void shouldExtractRequestByAliasForCorrectBidder() { // given + given(bidderCatalog.isValidName("bidderAlias")).willReturn(false); + final Bidder bidder = mock(Bidder.class); givenBidder("bidder", bidder, givenEmptySeatBid()); @@ -1108,9 +1110,36 @@ public void shouldExtractRequestByAliasForCorrectBidder() { .contains(1); } + @Test + public void shouldExtractRequestByAliasForHardcodedBidderAlias() { + // given + final Bidder bidder = mock(Bidder.class); + givenBidder("bidderAlias", bidder, givenEmptySeatBid()); + + final BidRequest bidRequest = givenBidRequest(singletonList( + givenImp(singletonMap("bidderAlias", 1), identity())), + builder -> builder.ext(ExtRequest.of(ExtRequestPrebid.builder() + .aliases(singletonMap("bidderAlias", "bidder")) + .auctiontimestamp(1000L) + .build()))); + + // when + target.holdAuction(givenRequestContext(bidRequest)); + + // then + final ArgumentCaptor bidRequestCaptor = ArgumentCaptor.forClass(BidderRequest.class); + verify(httpBidderRequester) + .requestBids(same(bidder), bidRequestCaptor.capture(), any(), any(), any(), any(), anyBoolean()); + assertThat(bidRequestCaptor.getValue().getBidRequest().getImp()).hasSize(1) + .extracting(imp -> imp.getExt().get("bidder").asInt()) + .contains(1); + } + @Test public void shouldExtractMultipleRequestsForTheSameBidderIfAliasesWereUsed() { // given + given(bidderCatalog.isValidName("bidderAlias")).willReturn(false); + final Bidder bidder = mock(Bidder.class); givenBidder("bidder", bidder, givenEmptySeatBid()); @@ -1136,6 +1165,39 @@ public void shouldExtractMultipleRequestsForTheSameBidderIfAliasesWereUsed() { .containsOnly(2, 1); } + @Test + public void shouldExtractMultipleRequestsForBidderAndItsHardcodedAlias() { + // given + final Bidder bidder = mock(Bidder.class); + final Bidder bidderAlias = mock(Bidder.class); + givenBidder("bidder", bidder, givenEmptySeatBid()); + givenBidder("bidderAlias", bidderAlias, givenEmptySeatBid()); + + final BidRequest bidRequest = givenBidRequest(singletonList( + givenImp(doubleMap("bidder", 1, "bidderAlias", 2), identity())), + builder -> builder.ext(ExtRequest.of(ExtRequestPrebid.builder() + .aliases(singletonMap("bidderAlias", "bidder")) + .auctiontimestamp(1000L) + .build()))); + + // when + target.holdAuction(givenRequestContext(bidRequest)); + + // then + final ArgumentCaptor bidRequestCaptor = ArgumentCaptor.forClass(BidderRequest.class); + verify(httpBidderRequester) + .requestBids(same(bidder), bidRequestCaptor.capture(), any(), any(), any(), any(), anyBoolean()); + verify(httpBidderRequester) + .requestBids(same(bidderAlias), bidRequestCaptor.capture(), any(), any(), any(), any(), anyBoolean()); + + final List capturedBidderRequests = bidRequestCaptor.getAllValues(); + + assertThat(capturedBidderRequests).hasSize(2) + .extracting(BidderRequest::getBidRequest) + .extracting(capturedBidRequest -> capturedBidRequest.getImp().getFirst().getExt().get("bidder").asInt()) + .containsOnly(2, 1); + } + @Test public void shouldTolerateBidderResultWithoutBids() { // given @@ -2962,6 +3024,8 @@ public void shouldNotAddExtPrebidEventsWhenEventsServiceReturnsEmptyEventsServic @Test public void shouldIncrementCommonMetrics() { // given + given(bidderCatalog.isValidName("someAlias")).willReturn(false); + given(httpBidderRequester.requestBids(any(), any(), any(), any(), any(), any(), anyBoolean())) .willReturn(Future.succeededFuture(givenSeatBid(singletonList( givenBidderBid(Bid.builder().impid("impId").price(TEN).build())))));