Skip to content

Commit

Permalink
Merge branch 'master' into fpd-update
Browse files Browse the repository at this point in the history
  • Loading branch information
rpanchyk committed Jun 2, 2020
2 parents 356a43d + fa0429c commit 5db231d
Show file tree
Hide file tree
Showing 14 changed files with 27 additions and 33 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

<groupId>org.prebid</groupId>
<artifactId>prebid-server</artifactId>
<version>1.35.0-SNAPSHOT</version>
<version>1.36.0-SNAPSHOT</version>

<name>prebid-server</name>
<description>Prebid Server (Server-side Header Bidding)</description>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ private BidRequest overrideParameters(BidRequest bidRequest, HttpServerRequest r
ccpaConsent = Ccpa.isValid(consentString) ? consentString : null;

if (StringUtils.isAllBlank(gdprConsent, ccpaConsent)) {
logger.warn("Amp request parameter consent_string or gdpr_consent have invalid format = {0}",
logger.debug("Amp request parameter consent_string or gdpr_consent have invalid format: {0}",
consentString);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ private Future<Account> accountFallback(Throwable exception, String accountId,
private Future<Account> responseForUnknownAccount(String accountId) {
return enforceValidAccount
? Future.failedFuture(new UnauthorizedAccountException(
String.format("Unauthorised account id %s", accountId), accountId))
String.format("Unauthorized account id: %s", accountId), accountId))
: Future.succeededFuture(Account.empty(accountId));
}
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
package org.prebid.server.exception;

import lombok.Getter;

import java.util.Collections;
import java.util.List;

@SuppressWarnings("serial")
public class InvalidRequestException extends RuntimeException {

@Getter
private final List<String> messages;

@Getter
private final boolean needEnhancedLogging;

public InvalidRequestException(String message) {
Expand Down Expand Up @@ -39,12 +43,4 @@ public InvalidRequestException(List<String> messages, boolean needEnhancedLoggin
this.messages = messages;
this.needEnhancedLogging = needEnhancedLogging;
}

public List<String> getMessages() {
return messages;
}

public boolean isNeedEnhancedLogging() {
return needEnhancedLogging;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

import lombok.Getter;

@Getter
@SuppressWarnings("serial")
public class UnauthorizedAccountException extends RuntimeException {

@Getter
private String accountId;

public UnauthorizedAccountException(String message, String accountId) {
super(String.format("%s", message));
super(message);
this.accountId = accountId;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -319,15 +319,15 @@ private void handleResult(AsyncResult<AmpResponse> responseResult, AmpEvent.AmpE
body = message;
} else if (exception instanceof UnauthorizedAccountException) {
metricRequestStatus = MetricName.badinput;
final String message = String.format("Unauthorized: %s", exception.getMessage());
final String message = exception.getMessage();
conditionalLogger.info(message, 100);

errorMessages = Collections.singletonList(message);

status = HttpResponseStatus.UNAUTHORIZED.code();
body = message;
String userId = ((UnauthorizedAccountException) exception).getAccountId();
metrics.updateAccountRequestRejectedMetrics(userId);
String accountId = ((UnauthorizedAccountException) exception).getAccountId();
metrics.updateAccountRequestRejectedMetrics(accountId);
} else if (exception instanceof BlacklistedAppException
|| exception instanceof BlacklistedAccountException) {
metricRequestStatus = exception instanceof BlacklistedAccountException
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ private void handleResult(AsyncResult<Tuple2<BidResponse, AuctionContext>> respo
body = message;
} else if (exception instanceof UnauthorizedAccountException) {
metricRequestStatus = MetricName.badinput;
final String message = String.format("Unauthorized: %s", exception.getMessage());
final String message = exception.getMessage();
conditionalLogger.info(message, 100);
errorMessages = Collections.singletonList(message);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
public class GdprService {

private static final Logger logger = LoggerFactory.getLogger(GdprService.class);

private static final int PURPOSE_ONE_ID = 1;

private final VendorListService<VendorListV1, VendorV1> vendorListService;
Expand Down Expand Up @@ -72,7 +73,7 @@ private VendorConsent vendorConsentFrom(String vendorConsentString) {
try {
return VendorConsentDecoder.fromBase64String(vendorConsentString);
} catch (IllegalArgumentException | IllegalStateException e) {
logger.warn("Parsing consent string failed with error: {0}", e.getMessage());
logger.info("Parsing consent string failed with error: {0}", e.getMessage());
return null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,10 @@ private GdprInfoWithCountry<String> createGdprInfoWithCountry(String gdprConsent
}

private GdprInfoWithCountry<String> updateMetricsAndReturnDefault(Throwable exception, String gdprConsent) {
logger.info("Geolocation lookup failed", exception);
final String message = String.format("Geolocation lookup failed: %s", exception.getMessage());
logger.warn(message);
logger.debug(message, exception);

metrics.updateGeoLocationMetric(false);
return defaultGdprInfoWithCountry(gdprConsent);
}
Expand Down Expand Up @@ -325,7 +328,7 @@ private static TCString decodeTcString(String consentString) {
try {
return StringUtils.isBlank(consentString) ? null : TCString.decode(consentString);
} catch (Throwable e) {
logger.warn("Parsing consent string failed with error: {0}", e.getMessage());
logger.info("Parsing consent string failed with error: {0}", e.getMessage());
return null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ private Future<Void> saveToFile(String content, int version) {
logger.info("Created new vendor list for version {0}, file: {1}", version, filepath);
promise.complete();
} else {
logger.warn("Could not create new vendor list for version {0}, file: {1}", result.cause(), version,
logger.error("Could not create new vendor list for version {0}, file: {1}", result.cause(), version,
filepath);
promise.fail(result.cause());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
import io.vertx.core.Vertx;
import io.vertx.core.http.HttpClientRequest;
import io.vertx.core.http.HttpMethod;
import io.vertx.core.logging.Logger;
import io.vertx.core.logging.LoggerFactory;
import org.prebid.server.vertx.http.model.HttpClientResponse;

import java.util.Objects;
Expand All @@ -18,8 +16,6 @@
*/
public class BasicHttpClient implements HttpClient {

private static final Logger logger = LoggerFactory.getLogger(BasicHttpClient.class);

private final Vertx vertx;
private final io.vertx.core.http.HttpClient httpClient;

Expand Down Expand Up @@ -95,8 +91,6 @@ private void failResponse(Throwable exception, Promise<HttpClientResponse> promi
}

private static void failResponse(Throwable exception, Promise<HttpClientResponse> promise) {
logger.warn("HTTP client error", exception);

promise.tryFail(exception);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ public void shouldReturnFailedFutureIfAccountIsEnforcedAndIdIsNotProvided() {
assertThat(future.failed()).isTrue();
assertThat(future.cause())
.isInstanceOf(UnauthorizedAccountException.class)
.hasMessage("Unauthorised account id ");
.hasMessage("Unauthorized account id: ");
}

@Test
Expand Down Expand Up @@ -232,7 +232,7 @@ public void shouldReturnFailedFutureIfAccountIsEnforcedAndFailedGetAccountById()
assertThat(future.failed()).isTrue();
assertThat(future.cause())
.isInstanceOf(UnauthorizedAccountException.class)
.hasMessage("Unauthorised account id absentId");
.hasMessage("Unauthorized account id: absentId");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ public void shouldRespondWithBadRequestIfRequestHasBlacklistedApp() {
public void shouldRespondWithUnauthorizedIfAccountIdIsInvalid() {
// given
given(ampRequestFactory.fromRequest(any(), anyLong()))
.willReturn(Future.failedFuture(new UnauthorizedAccountException("Account id is not provided 1", "1")));
.willReturn(Future.failedFuture(new UnauthorizedAccountException("Account id is not provided", null)));

// when
ampHandler.handle(routingContext);
Expand All @@ -272,7 +272,7 @@ public void shouldRespondWithUnauthorizedIfAccountIdIsInvalid() {
.containsOnly(
tuple("AMP-Access-Control-Allow-Source-Origin", "http://example.com"),
tuple("Access-Control-Expose-Headers", "AMP-Access-Control-Allow-Source-Origin"));
verify(httpResponse).end(eq("Unauthorized: Account id is not provided 1"));
verify(httpResponse).end(eq("Account id is not provided"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,15 +221,15 @@ public void shouldRespondWithBadRequestIfBidRequestIsInvalid() {
public void shouldRespondWithUnauthorizedIfAccountIdIsInvalid() {
// given
given(auctionRequestFactory.fromRequest(any(), anyLong()))
.willReturn(Future.failedFuture(new UnauthorizedAccountException("Account id is not provided 1", "1")));
.willReturn(Future.failedFuture(new UnauthorizedAccountException("Account id is not provided", null)));

// when
auctionHandler.handle(routingContext);

// then
verifyZeroInteractions(exchangeService);
verify(httpResponse).setStatusCode(eq(401));
verify(httpResponse).end(eq("Unauthorized: Account id is not provided 1"));
verify(httpResponse).end(eq("Account id is not provided"));
}

@Test
Expand Down

0 comments on commit 5db231d

Please sign in to comment.