- * Expected the endpoint to satisfy the following API: + * Expected the endpoint to satisfy the following API (URL is encoded): *
* GET {endpoint}?request-ids=["req1","req2"]&imp-ids=["imp1","imp2","imp3"] *
+ * or settings.http.rfc3986-compatible is set to true + *
+ * * GET {endpoint}?request-id=req1&request-id=req2&imp-id=imp1&imp-id=imp2&imp-id=imp3 + * *
* This endpoint should return a payload like: *
* {
@@ -76,20 +82,27 @@ public class HttpApplicationSettings implements ApplicationSettings {
private final String categoryEndpoint;
private final HttpClient httpClient;
private final JacksonMapper mapper;
+ private final boolean isRfc3986Compatible;
+
+ public HttpApplicationSettings(HttpClient httpClient,
+ JacksonMapper mapper,
+ String endpoint,
+ String ampEndpoint,
+ String videoEndpoint,
+ String categoryEndpoint,
+ boolean isRfc3986Compatible) {
- public HttpApplicationSettings(HttpClient httpClient, JacksonMapper mapper, String endpoint, String ampEndpoint,
- String videoEndpoint, String categoryEndpoint) {
this.httpClient = Objects.requireNonNull(httpClient);
this.mapper = Objects.requireNonNull(mapper);
- this.endpoint = HttpUtil.validateUrl(Objects.requireNonNull(endpoint));
- this.ampEndpoint = HttpUtil.validateUrl(Objects.requireNonNull(ampEndpoint));
- this.videoEndpoint = HttpUtil.validateUrl(Objects.requireNonNull(videoEndpoint));
- this.categoryEndpoint = HttpUtil.validateUrl(Objects.requireNonNull(categoryEndpoint));
+ this.endpoint = HttpUtil.validateUrlSyntax(Objects.requireNonNull(endpoint));
+ this.ampEndpoint = HttpUtil.validateUrlSyntax(Objects.requireNonNull(ampEndpoint));
+ this.videoEndpoint = HttpUtil.validateUrlSyntax(Objects.requireNonNull(videoEndpoint));
+ this.categoryEndpoint = HttpUtil.validateUrlSyntax(Objects.requireNonNull(categoryEndpoint));
+ this.isRfc3986Compatible = isRfc3986Compatible;
}
@Override
public Future getAccountById(String accountId, Timeout timeout) {
-
return fetchAccountsByIds(Collections.singleton(accountId), timeout)
.map(accounts -> accounts.stream()
.findFirst()
@@ -111,15 +124,20 @@ private Future> fetchAccountsByIds(Set accountIds, Timeout
.recover(Future::failedFuture);
}
- private static String accountsRequestUrlFrom(String endpoint, Set accountIds) {
- final StringBuilder url = new StringBuilder(endpoint);
- url.append(endpoint.contains("?") ? "&" : "?");
-
- if (!accountIds.isEmpty()) {
- url.append("account-ids=[\"").append(joinIds(accountIds)).append("\"]");
+ private String accountsRequestUrlFrom(String endpoint, Set accountIds) {
+ try {
+ final URIBuilder uriBuilder = new URIBuilder(endpoint);
+ if (!accountIds.isEmpty()) {
+ if (isRfc3986Compatible) {
+ accountIds.forEach(accountId -> uriBuilder.addParameter("account-id", accountId));
+ } else {
+ uriBuilder.addParameter("account-ids", "[\"%s\"]".formatted(joinIds(accountIds)));
+ }
+ }
+ return uriBuilder.build().toString();
+ } catch (URISyntaxException e) {
+ throw new PreBidException("URL %s has bad syntax".formatted(endpoint));
}
-
- return url.toString();
}
private Future> processAccountsResponse(HttpClientResponse response, Set accountIds) {
@@ -165,9 +183,6 @@ public Future getAmpStoredData(String accountId, Set r
return fetchStoredData(ampEndpoint, requestIds, Collections.emptySet(), timeout);
}
- /**
- * Not supported and returns failed result.
- */
@Override
public Future getVideoStoredData(String accountId, Set requestIds, Set impIds,
Timeout timeout) {
@@ -240,22 +255,27 @@ private Future fetchStoredData(String endpoint, Set re
.recover(exception -> failStoredDataResponse(exception, requestIds, impIds));
}
- private static String storeRequestUrlFrom(String endpoint, Set requestIds, Set impIds) {
- final StringBuilder url = new StringBuilder(endpoint);
- url.append(endpoint.contains("?") ? "&" : "?");
-
- if (!requestIds.isEmpty()) {
- url.append("request-ids=[\"").append(joinIds(requestIds)).append("\"]");
- }
-
- if (!impIds.isEmpty()) {
+ private String storeRequestUrlFrom(String endpoint, Set requestIds, Set impIds) {
+ try {
+ final URIBuilder uriBuilder = new URIBuilder(endpoint);
if (!requestIds.isEmpty()) {
- url.append("&");
+ if (isRfc3986Compatible) {
+ requestIds.forEach(requestId -> uriBuilder.addParameter("request-id", requestId));
+ } else {
+ uriBuilder.addParameter("request-ids", "[\"%s\"]".formatted(joinIds(requestIds)));
+ }
+ }
+ if (!impIds.isEmpty()) {
+ if (isRfc3986Compatible) {
+ impIds.forEach(impId -> uriBuilder.addParameter("imp-id", impId));
+ } else {
+ uriBuilder.addParameter("imp-ids", "[\"%s\"]".formatted(joinIds(impIds)));
+ }
}
- url.append("imp-ids=[\"").append(joinIds(impIds)).append("\"]");
+ return uriBuilder.build().toString();
+ } catch (URISyntaxException e) {
+ throw new PreBidException("URL %s has bad syntax".formatted(endpoint));
}
-
- return url.toString();
}
private static String joinIds(Set ids) {
diff --git a/src/main/java/org/prebid/server/spring/config/SettingsConfiguration.java b/src/main/java/org/prebid/server/spring/config/SettingsConfiguration.java
index 4e883ba2495..3f674ae814d 100644
--- a/src/main/java/org/prebid/server/spring/config/SettingsConfiguration.java
+++ b/src/main/java/org/prebid/server/spring/config/SettingsConfiguration.java
@@ -122,10 +122,17 @@ HttpApplicationSettings httpApplicationSettings(
@Value("${settings.http.endpoint}") String endpoint,
@Value("${settings.http.amp-endpoint}") String ampEndpoint,
@Value("${settings.http.video-endpoint}") String videoEndpoint,
- @Value("${settings.http.category-endpoint}") String categoryEndpoint) {
+ @Value("${settings.http.category-endpoint}") String categoryEndpoint,
+ @Value("${settings.http.rfc3986-compatible:false}") boolean isRfc3986Compatible) {
- return new HttpApplicationSettings(httpClient, mapper, endpoint, ampEndpoint, videoEndpoint,
- categoryEndpoint);
+ return new HttpApplicationSettings(
+ httpClient,
+ mapper,
+ endpoint,
+ ampEndpoint,
+ videoEndpoint,
+ categoryEndpoint,
+ isRfc3986Compatible);
}
}
diff --git a/src/main/java/org/prebid/server/util/HttpUtil.java b/src/main/java/org/prebid/server/util/HttpUtil.java
index ad9dd8a9238..47a5f24eda8 100644
--- a/src/main/java/org/prebid/server/util/HttpUtil.java
+++ b/src/main/java/org/prebid/server/util/HttpUtil.java
@@ -8,6 +8,7 @@
import io.vertx.core.http.HttpServerResponse;
import io.vertx.ext.web.RoutingContext;
import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.validator.routines.UrlValidator;
import org.prebid.server.log.ConditionalLogger;
import org.prebid.server.log.Logger;
import org.prebid.server.log.LoggerFactory;
@@ -78,12 +79,15 @@ public final class HttpUtil {
public static final String MACROS_OPEN = "{{";
public static final String MACROS_CLOSE = "}}";
+ private static final UrlValidator URL_VALIDAROR = UrlValidator.getInstance();
+
private HttpUtil() {
}
/**
* Checks the input string for using as URL.
*/
+ @Deprecated
public static String validateUrl(String url) {
if (containsMacrosses(url)) {
return url;
@@ -96,6 +100,14 @@ public static String validateUrl(String url) {
}
}
+ public static String validateUrlSyntax(String url) {
+ if (containsMacrosses(url) || URL_VALIDAROR.isValid(url)) {
+ return url;
+ }
+
+ throw new IllegalArgumentException("URL supplied is not valid: " + url);
+ }
+
// TODO: We need our own way to work with url macrosses
private static boolean containsMacrosses(String url) {
return StringUtils.contains(url, MACROS_OPEN) && StringUtils.contains(url, MACROS_CLOSE);
diff --git a/src/test/groovy/org/prebid/server/functional/testcontainers/container/NetworkServiceContainer.groovy b/src/test/groovy/org/prebid/server/functional/testcontainers/container/NetworkServiceContainer.groovy
index 53faa7165fa..8022f2e8dcc 100644
--- a/src/test/groovy/org/prebid/server/functional/testcontainers/container/NetworkServiceContainer.groovy
+++ b/src/test/groovy/org/prebid/server/functional/testcontainers/container/NetworkServiceContainer.groovy
@@ -8,6 +8,9 @@ class NetworkServiceContainer extends MockServerContainer {
NetworkServiceContainer(String version) {
super(DockerImageName.parse("mockserver/mockserver:mockserver-$version"))
+ def aliasWithTopLevelDomain = "${getNetworkAliases().first()}.com".toString()
+ withCreateContainerCmdModifier { it.withHostName(aliasWithTopLevelDomain) }
+ setNetworkAliases([aliasWithTopLevelDomain])
}
String getHostAndPort() {
diff --git a/src/test/groovy/org/prebid/server/functional/testcontainers/scaffolding/HttpSettings.groovy b/src/test/groovy/org/prebid/server/functional/testcontainers/scaffolding/HttpSettings.groovy
index de271b4123a..5af648b2bc0 100644
--- a/src/test/groovy/org/prebid/server/functional/testcontainers/scaffolding/HttpSettings.groovy
+++ b/src/test/groovy/org/prebid/server/functional/testcontainers/scaffolding/HttpSettings.groovy
@@ -1,13 +1,21 @@
package org.prebid.server.functional.testcontainers.scaffolding
+import org.mockserver.matchers.Times
+import org.mockserver.model.Header
import org.mockserver.model.HttpRequest
+import org.mockserver.model.HttpStatusCode
+import org.prebid.server.functional.model.ResponseModel
import org.testcontainers.containers.MockServerContainer
import static org.mockserver.model.HttpRequest.request
+import static org.mockserver.model.HttpResponse.response
+import static org.mockserver.model.HttpStatusCode.OK_200
+import static org.mockserver.model.MediaType.APPLICATION_JSON
class HttpSettings extends NetworkScaffolding {
private static final String ENDPOINT = "/stored-requests"
+ private static final String RFC_ENDPOINT = "/stored-requests-rfc"
private static final String AMP_ENDPOINT = "/amp-stored-requests"
HttpSettings(MockServerContainer mockServerContainer) {
@@ -27,12 +35,47 @@ class HttpSettings extends NetworkScaffolding {
@Override
void setResponse() {
+ }
+
+ protected HttpRequest getRfcRequest(String accountId) {
+ request().withPath(RFC_ENDPOINT)
+ .withQueryStringParameter("account-id", accountId)
+ }
+
+
+ void setRfcResponse(String value,
+ ResponseModel responseModel,
+ HttpStatusCode statusCode = OK_200,
+ Map headers = [:]) {
+ def responseHeaders = headers.collect { new Header(it.key, it.value) }
+ def mockResponse = encode(responseModel)
+ mockServerClient.when(getRfcRequest(value), Times.unlimited())
+ .respond(response().withStatusCode(statusCode.code())
+ .withBody(mockResponse, APPLICATION_JSON)
+ .withHeaders(responseHeaders))
+ }
+ int getRfcRequestCount(String value) {
+ mockServerClient.retrieveRecordedRequests(getRfcRequest(value))
+ .size()
}
@Override
void reset() {
super.reset(ENDPOINT)
+ super.reset(RFC_ENDPOINT)
super.reset(AMP_ENDPOINT)
}
+
+ static String getEndpoint() {
+ return ENDPOINT
+ }
+
+ static String getAmpEndpoint() {
+ return AMP_ENDPOINT
+ }
+
+ static String getRfcEndpoint() {
+ return RFC_ENDPOINT
+ }
}
diff --git a/src/test/groovy/org/prebid/server/functional/tests/HttpSettingsSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/HttpSettingsSpec.groovy
index 2c6d1556a81..01f60ac2808 100644
--- a/src/test/groovy/org/prebid/server/functional/tests/HttpSettingsSpec.groovy
+++ b/src/test/groovy/org/prebid/server/functional/tests/HttpSettingsSpec.groovy
@@ -15,7 +15,6 @@ import org.prebid.server.functional.testcontainers.PbsConfig
import org.prebid.server.functional.testcontainers.scaffolding.HttpSettings
import org.prebid.server.functional.util.PBSUtils
import org.prebid.server.util.ResourceUtil
-import spock.lang.Shared
import static org.prebid.server.functional.model.bidder.BidderName.GENERIC
import static org.prebid.server.functional.testcontainers.Dependencies.networkServiceContainer
@@ -23,11 +22,25 @@ import static org.prebid.server.functional.testcontainers.Dependencies.networkSe
class HttpSettingsSpec extends BaseSpec {
// Check that PBS actually applied account config only possible by relying on side effects.
- @Shared
- HttpSettings httpSettings = new HttpSettings(networkServiceContainer)
+ static PrebidServerService prebidServerService
+ static PrebidServerService prebidServerServiceWithRfc
- @Shared
- PrebidServerService prebidServerService = pbsServiceFactory.getService(PbsConfig.httpSettingsConfig)
+ private static final HttpSettings httpSettings = new HttpSettings(networkServiceContainer)
+ private static final Map PBS_CONFIG_WITH_RFC = new HashMap<>(PbsConfig.httpSettingsConfig) +
+ ['settings.http.endpoint': "${networkServiceContainer.rootUri}${HttpSettings.rfcEndpoint}".toString(),
+ 'settings.http.rfc3986-compatible': 'true']
+
+ def setupSpec() {
+ prebidServerService = pbsServiceFactory.getService(PbsConfig.httpSettingsConfig)
+ prebidServerServiceWithRfc = pbsServiceFactory.getService(PBS_CONFIG_WITH_RFC)
+ bidder.setResponse()
+ vendorList.setResponse()
+ }
+
+ def cleanupSpec() {
+ prebidServerService = pbsServiceFactory.removeContainer(PbsConfig.httpSettingsConfig)
+ prebidServerService = pbsServiceFactory.removeContainer(PBS_CONFIG_WITH_RFC)
+ }
def "PBS should take account information from http data source on auction request"() {
given: "Get basic BidRequest with generic bidder and set gdpr = 1"
@@ -35,8 +48,8 @@ class HttpSettingsSpec extends BaseSpec {
bidRequest.regs.gdpr = 1
and: "Prepare default account response with gdpr = 0"
- def httpSettingsResponse = HttpAccountsResponse.getDefaultHttpAccountsResponse(bidRequest?.site?.publisher?.id)
- httpSettings.setResponse(bidRequest?.site?.publisher?.id, httpSettingsResponse)
+ def httpSettingsResponse = HttpAccountsResponse.getDefaultHttpAccountsResponse(bidRequest.accountId)
+ httpSettings.setResponse(bidRequest.accountId, httpSettingsResponse)
when: "PBS processes auction request"
def response = prebidServerService.sendAuctionRequest(bidRequest)
@@ -51,7 +64,32 @@ class HttpSettingsSpec extends BaseSpec {
assert bidder.getRequestCount(bidRequest.id) == 1
and: "There should be only one account request"
- assert httpSettings.getRequestCount(bidRequest?.site?.publisher?.id) == 1
+ assert httpSettings.getRequestCount(bidRequest.accountId) == 1
+ }
+
+ def "PBS should take account information from http data source on auction request when rfc3986 enabled"() {
+ given: "Get basic BidRequest with generic bidder and set gdpr = 1"
+ def bidRequest = BidRequest.defaultBidRequest
+ bidRequest.regs.gdpr = 1
+
+ and: "Prepare default account response with gdpr = 0"
+ def httpSettingsResponse = HttpAccountsResponse.getDefaultHttpAccountsResponse(bidRequest.accountId)
+ httpSettings.setRfcResponse(bidRequest.accountId, httpSettingsResponse)
+
+ when: "PBS processes auction request"
+ def response = prebidServerServiceWithRfc.sendAuctionRequest(bidRequest)
+
+ then: "Response should contain basic fields"
+ assert response.id
+ assert response.seatbid?.size() == 1
+ assert response.seatbid.first().seat == GENERIC
+ assert response.seatbid?.first()?.bid?.size() == 1
+
+ and: "There should be only one call to bidder"
+ assert bidder.getRequestCount(bidRequest.id) == 1
+
+ and: "There should be only one account request"
+ assert httpSettings.getRfcRequestCount(bidRequest.accountId) == 1
}
def "PBS should take account information from http data source on AMP request"() {
@@ -84,6 +122,36 @@ class HttpSettingsSpec extends BaseSpec {
assert !response.ext?.debug?.httpcalls?.isEmpty()
}
+ def "PBS should take account information from http data source on AMP request when rfc3986 enabled"() {
+ given: "Default AmpRequest"
+ def ampRequest = AmpRequest.defaultAmpRequest
+
+ and: "Get basic stored request and set gdpr = 1"
+ def ampStoredRequest = BidRequest.defaultBidRequest
+ ampStoredRequest.site.publisher.id = ampRequest.account
+ ampStoredRequest.regs.gdpr = 1
+
+ and: "Save storedRequest into DB"
+ def storedRequest = StoredRequest.getStoredRequest(ampRequest, ampStoredRequest)
+ storedRequestDao.save(storedRequest)
+
+ and: "Prepare default account response with gdpr = 0"
+ def httpSettingsResponse = HttpAccountsResponse.getDefaultHttpAccountsResponse(ampRequest.account.toString())
+ httpSettings.setRfcResponse(ampRequest.account.toString(), httpSettingsResponse)
+
+ when: "PBS processes amp request"
+ def response = prebidServerServiceWithRfc.sendAmpRequest(ampRequest)
+
+ then: "Response should contain httpcalls"
+ assert !response.ext?.debug?.httpcalls?.isEmpty()
+
+ and: "There should be only one account request"
+ assert httpSettings.getRfcRequestCount(ampRequest.account.toString()) == 1
+
+ then: "Response should contain targeting"
+ assert !response.ext?.debug?.httpcalls?.isEmpty()
+ }
+
def "PBS should take account information from http data source on event request"() {
given: "Default EventRequest"
def eventRequest = EventRequest.defaultEventRequest
@@ -103,6 +171,25 @@ class HttpSettingsSpec extends BaseSpec {
assert httpSettings.getRequestCount(eventRequest.accountId.toString()) == 1
}
+ def "PBS should take account information from http data source on event request when rfc3986 enabled"() {
+ given: "Default EventRequest"
+ def eventRequest = EventRequest.defaultEventRequest
+
+ and: "Prepare default account response"
+ def httpSettingsResponse = HttpAccountsResponse.getDefaultHttpAccountsResponse(eventRequest.accountId.toString())
+ httpSettings.setRfcResponse(eventRequest.accountId.toString(), httpSettingsResponse)
+
+ when: "PBS processes event request"
+ def responseBody = prebidServerServiceWithRfc.sendEventRequest(eventRequest)
+
+ then: "Event response should contain and corresponding content-type"
+ assert responseBody ==
+ ResourceUtil.readByteArrayFromClassPath("org/prebid/server/functional/tracking-pixel.png")
+
+ and: "There should be only one account request"
+ assert httpSettings.getRfcRequestCount(eventRequest.accountId.toString()) == 1
+ }
+
def "PBS should take account information from http data source on setuid request"() {
given: "Pbs config with adapters.generic.usersync.redirect.*"
def pbsConfig = PbsConfig.httpSettingsConfig +
@@ -137,6 +224,42 @@ class HttpSettingsSpec extends BaseSpec {
pbsServiceFactory.removeContainer(pbsConfig)
}
+ def "PBS should take account information from http data source on setuid request when rfc3986 enabled"() {
+ given: "Pbs config with adapters.generic.usersync.redirect.*"
+ def pbsConfig = new HashMap<>(PbsConfig.httpSettingsConfig) +
+ ['settings.http.endpoint': "${networkServiceContainer.rootUri}${HttpSettings.rfcEndpoint}".toString(),
+ 'settings.http.rfc3986-compatible': 'true',
+ 'adapters.generic.usersync.redirect.url' : "$networkServiceContainer.rootUri/generic-usersync&redir={{redirect_url}}".toString(),
+ 'adapters.generic.usersync.redirect.support-cors' : 'false',
+ 'adapters.generic.usersync.redirect.format-override': 'blank']
+ def prebidServerService = pbsServiceFactory.getService(pbsConfig)
+
+ and: "Get default SetuidRequest and set account, gdpr=1 "
+ def request = SetuidRequest.defaultSetuidRequest
+ request.gdpr = 1
+ request.account = PBSUtils.randomNumber.toString()
+ def uidsCookie = UidsCookie.defaultUidsCookie
+
+ and: "Prepare default account response"
+ def httpSettingsResponse = HttpAccountsResponse.getDefaultHttpAccountsResponse(request.account)
+ httpSettings.setRfcResponse(request.account, httpSettingsResponse)
+
+ when: "PBS processes setuid request"
+ def response = prebidServerService.sendSetUidRequest(request, uidsCookie)
+
+ then: "Response should contain tempUIDs cookie"
+ assert !response.uidsCookie.uids
+ assert response.uidsCookie.tempUIDs
+ assert response.responseBody ==
+ ResourceUtil.readByteArrayFromClassPath("org/prebid/server/functional/tracking-pixel.png")
+
+ and: "There should be only one account request"
+ assert httpSettings.getRfcRequestCount(request.account) == 1
+
+ cleanup: "Stop and remove pbs container"
+ pbsServiceFactory.removeContainer(pbsConfig)
+ }
+
def "PBS should take account information from http data source on vtrack request"() {
given: "Default VtrackRequest"
String payload = PBSUtils.randomString
@@ -162,6 +285,31 @@ class HttpSettingsSpec extends BaseSpec {
assert prebidCacheRequest.contains("/event?t=imp&b=${request.puts[0].bidid}&a=$accountId&bidder=${request.puts[0].bidder}")
}
+ def "PBS should take account information from http data source on vtrack request when rfc3986 enabled"() {
+ given: "Default VtrackRequest"
+ String payload = PBSUtils.randomString
+ def request = VtrackRequest.getDefaultVtrackRequest(encodeXml(Vast.getDefaultVastModel(payload)))
+ def accountId = PBSUtils.randomNumber.toString()
+
+ and: "Prepare default account response"
+ def httpSettingsResponse = HttpAccountsResponse.getDefaultHttpAccountsResponse(accountId)
+ httpSettings.setRfcResponse(accountId, httpSettingsResponse)
+
+ when: "PBS processes vtrack request"
+ def response = prebidServerServiceWithRfc.sendVtrackRequest(request, accountId)
+
+ then: "Response should contain uid"
+ assert response.responses[0]?.uuid
+
+ and: "There should be only one account request and pbc request"
+ assert httpSettings.getRfcRequestCount(accountId.toString()) == 1
+ assert prebidCache.getXmlRequestCount(payload) == 1
+
+ and: "VastXml that was send to PrebidCache must contain event url"
+ def prebidCacheRequest = prebidCache.getXmlRecordedRequestsBody(payload)[0]
+ assert prebidCacheRequest.contains("/event?t=imp&b=${request.puts[0].bidid}&a=$accountId&bidder=${request.puts[0].bidder}")
+ }
+
def "PBS should return error if account settings isn't found"() {
given: "Default EventRequest"
def eventRequest = EventRequest.defaultEventRequest
@@ -174,4 +322,17 @@ class HttpSettingsSpec extends BaseSpec {
assert exception.statusCode == 401
assert exception.responseBody.contains("Account '$eventRequest.accountId' doesn't support events")
}
+
+ def "PBS should return error if account settings isn't found when rfc3986 enabled"() {
+ given: "Default EventRequest"
+ def eventRequest = EventRequest.defaultEventRequest
+
+ when: "PBS processes event request"
+ prebidServerServiceWithRfc.sendEventRequest(eventRequest)
+
+ then: "Request should fail with error"
+ def exception = thrown(PrebidServerException)
+ assert exception.statusCode == 401
+ assert exception.responseBody.contains("Account '$eventRequest.accountId' doesn't support events")
+ }
}
diff --git a/src/test/java/org/prebid/server/settings/HttpApplicationSettingsTest.java b/src/test/java/org/prebid/server/settings/HttpApplicationSettingsTest.java
index e3076ddbdfd..60627cf2571 100644
--- a/src/test/java/org/prebid/server/settings/HttpApplicationSettingsTest.java
+++ b/src/test/java/org/prebid/server/settings/HttpApplicationSettingsTest.java
@@ -2,6 +2,8 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import io.vertx.core.Future;
+import org.apache.http.NameValuePair;
+import org.apache.http.client.utils.URIBuilder;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@@ -22,12 +24,14 @@
import org.prebid.server.vertx.httpclient.HttpClient;
import org.prebid.server.vertx.httpclient.model.HttpClientResponse;
+import java.net.URISyntaxException;
import java.time.Clock;
import java.time.Instant;
import java.time.ZoneId;
import java.util.Collections;
import java.util.HashSet;
import java.util.Map;
+import java.util.Set;
import java.util.concurrent.TimeoutException;
import static java.util.Arrays.asList;
@@ -49,10 +53,10 @@
@ExtendWith(MockitoExtension.class)
public class HttpApplicationSettingsTest extends VertxTest {
- private static final String ENDPOINT = "http://stored-requests";
- private static final String AMP_ENDPOINT = "http://amp-stored-requests";
- private static final String VIDEO_ENDPOINT = "http://video-stored-requests";
- private static final String CATEGORY_ENDPOINT = "http://category-requests";
+ private static final String ENDPOINT = "http://stored-requests.com/something?id=1";
+ private static final String AMP_ENDPOINT = "http://amp-stored-requests.com/something?id=2";
+ private static final String VIDEO_ENDPOINT = "http://video-stored-requests.com/something?id=3";
+ private static final String CATEGORY_ENDPOINT = "http://category-requests.com/something";
@Mock(strictness = LENIENT)
private HttpClient httpClient;
@@ -65,7 +69,7 @@ public class HttpApplicationSettingsTest extends VertxTest {
@BeforeEach
public void setUp() {
httpApplicationSettings = new HttpApplicationSettings(httpClient, jacksonMapper, ENDPOINT, AMP_ENDPOINT,
- VIDEO_ENDPOINT, CATEGORY_ENDPOINT);
+ VIDEO_ENDPOINT, CATEGORY_ENDPOINT, false);
final Clock clock = Clock.fixed(Instant.now(), ZoneId.systemDefault());
final TimeoutFactory timeoutFactory = new TimeoutFactory(clock);
@@ -77,7 +81,7 @@ public void setUp() {
public void creationShouldFailsOnInvalidEndpoint() {
assertThatIllegalArgumentException()
.isThrownBy(() -> new HttpApplicationSettings(httpClient, jacksonMapper, "invalid_url", AMP_ENDPOINT,
- VIDEO_ENDPOINT, CATEGORY_ENDPOINT))
+ VIDEO_ENDPOINT, CATEGORY_ENDPOINT, false))
.withMessage("URL supplied is not valid: invalid_url");
}
@@ -85,7 +89,7 @@ public void creationShouldFailsOnInvalidEndpoint() {
public void creationShouldFailsOnInvalidAmpEndpoint() {
assertThatIllegalArgumentException()
.isThrownBy(() -> new HttpApplicationSettings(httpClient, jacksonMapper, ENDPOINT, "invalid_url",
- VIDEO_ENDPOINT, CATEGORY_ENDPOINT))
+ VIDEO_ENDPOINT, CATEGORY_ENDPOINT, false))
.withMessage("URL supplied is not valid: invalid_url");
}
@@ -93,7 +97,7 @@ public void creationShouldFailsOnInvalidAmpEndpoint() {
public void creationShouldFailsOnInvalidVideoEndpoint() {
assertThatIllegalArgumentException()
.isThrownBy(() -> new HttpApplicationSettings(httpClient, jacksonMapper, ENDPOINT, AMP_ENDPOINT,
- "invalid_url", CATEGORY_ENDPOINT))
+ "invalid_url", CATEGORY_ENDPOINT, false))
.withMessage("URL supplied is not valid: invalid_url");
}
@@ -118,7 +122,40 @@ public void getAccountByIdShouldReturnFetchedAccount() throws JsonProcessingExce
assertThat(future.result().getId()).isEqualTo("someId");
assertThat(future.result().getAuction().getPriceGranularity()).isEqualTo("testPriceGranularity");
- verify(httpClient).get(eq("http://stored-requests?account-ids=[\"someId\"]"), any(),
+ verify(httpClient).get(
+ eq("http://stored-requests.com/something?id=1&account-ids=%5B%22someId%22%5D"),
+ any(),
+ anyLong());
+ }
+
+ @Test
+ public void getAccountByIdShouldReturnFetchedAccountWithRfc3986CompatibleParams() throws JsonProcessingException {
+ // given
+ givenHttpClientReturnsResponse(200, null);
+ httpApplicationSettings = new HttpApplicationSettings(httpClient, jacksonMapper,
+ ENDPOINT, AMP_ENDPOINT, VIDEO_ENDPOINT, CATEGORY_ENDPOINT, true);
+
+ final Account account = Account.builder()
+ .id("someId")
+ .auction(AccountAuctionConfig.builder()
+ .priceGranularity("testPriceGranularity")
+ .build())
+ .privacy(AccountPrivacyConfig.builder().build())
+ .build();
+ final HttpAccountsResponse response = HttpAccountsResponse.of(Collections.singletonMap("someId", account));
+ givenHttpClientReturnsResponse(200, mapper.writeValueAsString(response));
+
+ // when
+ final Future future = httpApplicationSettings.getAccountById("someId", timeout);
+
+ // then
+ assertThat(future.succeeded()).isTrue();
+ assertThat(future.result().getId()).isEqualTo("someId");
+ assertThat(future.result().getAuction().getPriceGranularity()).isEqualTo("testPriceGranularity");
+
+ verify(httpClient).get(
+ eq("http://stored-requests.com/something?id=1&account-id=someId"),
+ any(),
anyLong());
}
@@ -234,8 +271,11 @@ public void getStoredDataShouldSendHttpRequestWithExpectedNewParams() {
new HashSet<>(asList("id3", "id4")), timeout);
// then
- verify(httpClient).get(eq("http://stored-requests?request-ids=[\"id2\",\"id1\"]&imp-ids=[\"id4\",\"id3\"]"),
- any(), anyLong());
+ verify(httpClient).get(
+ eq("http://stored-requests.com/something"
+ + "?id=1&request-ids=%5B%22id2%22%2C%22id1%22%5D&imp-ids=%5B%22id4%22%2C%22id3%22%5D"),
+ any(),
+ anyLong());
}
@Test
@@ -243,16 +283,45 @@ public void getStoredDataShouldSendHttpRequestWithExpectedAppendedParams() {
// given
givenHttpClientReturnsResponse(200, null);
httpApplicationSettings = new HttpApplicationSettings(httpClient, jacksonMapper,
- "http://some-domain?param1=value1", AMP_ENDPOINT, VIDEO_ENDPOINT, CATEGORY_ENDPOINT);
+ "http://some-domain.com?param1=value1", AMP_ENDPOINT, VIDEO_ENDPOINT, CATEGORY_ENDPOINT, false);
// when
httpApplicationSettings.getStoredData(null, singleton("id1"), singleton("id2"), timeout);
// then
- verify(httpClient).get(eq("http://some-domain?param1=value1&request-ids=[\"id1\"]&imp-ids=[\"id2\"]"), any(),
+ verify(httpClient).get(
+ eq("http://some-domain.com?param1=value1&request-ids=%5B%22id1%22%5D&imp-ids=%5B%22id2%22%5D"),
+ any(),
anyLong());
}
+ @Test
+ public void getStoredDataShouldSendHttpRequestWithRfc3986CompatibleParams() throws URISyntaxException {
+ // given
+ givenHttpClientReturnsResponse(200, null);
+ httpApplicationSettings = new HttpApplicationSettings(httpClient, jacksonMapper,
+ ENDPOINT, AMP_ENDPOINT, VIDEO_ENDPOINT, CATEGORY_ENDPOINT, true);
+
+ // when
+ httpApplicationSettings.getStoredData(null, Set.of("id1", "id2"), Set.of("id1", "id2"), timeout);
+
+ // then
+ final ArgumentCaptor captor = ArgumentCaptor.forClass(String.class);
+ verify(httpClient).get(captor.capture(), any(), anyLong());
+
+ final URIBuilder uriBuilder = new URIBuilder(captor.getValue());
+ assertThat(uriBuilder.getHost()).isEqualTo("stored-requests.com");
+ assertThat(uriBuilder.getPath()).isEqualTo("/something");
+ assertThat(uriBuilder.getQueryParams())
+ .extracting(NameValuePair::getName, NameValuePair::getValue)
+ .containsExactlyInAnyOrder(
+ tuple("id", "1"),
+ tuple("request-id", "id1"),
+ tuple("request-id", "id2"),
+ tuple("imp-id", "id1"),
+ tuple("imp-id", "id2"));
+ }
+
@Test
public void getStoredDataShouldReturnResultWithErrorIfHttpClientFails() {
// given
@@ -416,7 +485,7 @@ public void getCategoriesShouldBuildUrlFromEndpointAdServerAndPublisher() {
// then
final ArgumentCaptor urlArgumentCaptor = ArgumentCaptor.forClass(String.class);
verify(httpClient).get(urlArgumentCaptor.capture(), anyLong());
- assertThat(urlArgumentCaptor.getValue()).isEqualTo("http://category-requests/primaryAdServer/publisher.json");
+ assertThat(urlArgumentCaptor.getValue()).isEqualTo("http://category-requests.com/something/primaryAdServer/publisher.json");
}
@Test
@@ -431,7 +500,8 @@ public void getCategoriesShouldBuildUrlFromEndpointAdServer() {
// then
final ArgumentCaptor urlArgumentCaptor = ArgumentCaptor.forClass(String.class);
verify(httpClient).get(urlArgumentCaptor.capture(), anyLong());
- assertThat(urlArgumentCaptor.getValue()).isEqualTo("http://category-requests/primaryAdServer.json");
+ assertThat(urlArgumentCaptor.getValue())
+ .isEqualTo("http://category-requests.com/something/primaryAdServer.json");
}
@Test
@@ -447,7 +517,7 @@ public void getCategoriesShouldReturnFailedFutureWithTimeoutException() {
// then
assertThat(result.failed()).isTrue();
assertThat(result.cause()).isInstanceOf(TimeoutException.class)
- .hasMessage("Failed to fetch categories from url 'http://category-requests/primaryAdServer.json'."
+ .hasMessage("Failed to fetch categories from url 'http://category-requests.com/something/primaryAdServer.json'."
+ " Reason: Timeout exceeded");
}
@@ -464,7 +534,7 @@ public void getCategoriesShouldReturnFailedFutureWhenResponseStatusIsNot200() {
// then
assertThat(result.failed()).isTrue();
assertThat(result.cause()).isInstanceOf(PreBidException.class)
- .hasMessage("Failed to fetch categories from url 'http://category-requests/primaryAdServer.json'."
+ .hasMessage("Failed to fetch categories from url 'http://category-requests.com/something/primaryAdServer.json'."
+ " Reason: Response status code is '400'");
}
@@ -481,7 +551,7 @@ public void getCategoriesShouldReturnFailedFutureWhenBodyIsNull() {
// then
assertThat(result.failed()).isTrue();
assertThat(result.cause()).isInstanceOf(PreBidException.class)
- .hasMessage("Failed to fetch categories from url 'http://category-requests/primaryAdServer.json'."
+ .hasMessage("Failed to fetch categories from url 'http://category-requests.com/something/primaryAdServer.json'."
+ " Reason: Response body is null or empty");
}
@@ -499,7 +569,7 @@ public void getCategoriesShouldReturnFailedFutureWhenBodyCantBeParsed() {
assertThat(result.failed()).isTrue();
assertThat(result.cause()).isInstanceOf(PreBidException.class)
.hasMessageStartingWith("Failed to fetch categories from url "
- + "'http://category-requests/primaryAdServer.json'. Reason: Failed to decode response body");
+ + "'http://category-requests.com/something/primaryAdServer.json'. Reason: Failed to decode response body");
}
@Test