Skip to content
This repository has been archived by the owner on Nov 26, 2024. It is now read-only.

Commit

Permalink
Merge branch 'master' into feature/support-json-operators-in-dao
Browse files Browse the repository at this point in the history
  • Loading branch information
splitfeed authored Sep 19, 2024
2 parents f793988 + 9d882b9 commit e2ee33a
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.reactivestreams.Publisher;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.event.Level;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.util.retry.Retry;
Expand Down Expand Up @@ -79,6 +80,7 @@
import static javax.ws.rs.core.MediaType.APPLICATION_JSON;
import static reactor.core.Exceptions.isRetryExhausted;
import static reactor.core.publisher.Mono.just;
import static se.fortnox.reactivewizard.client.HttpClientConfig.DEFAULT_TIMEOUT_MS;

public class HttpClient implements InvocationHandler {
private static final Logger LOG = LoggerFactory.getLogger(HttpClient.class);
Expand All @@ -103,8 +105,8 @@ public class HttpClient implements InvocationHandler {
private final RequestLogger requestLogger;
private final Map<Class<?>, List<HttpClient.BeanParamProperty>> beanParamCache = new ConcurrentHashMap<>();
private final Map<Method, JaxRsMeta> jaxRsMetaMap = new ConcurrentHashMap<>();
private int timeout = 10;
private TemporalUnit timeoutUnit = ChronoUnit.SECONDS;
private int timeout = DEFAULT_TIMEOUT_MS;
private TemporalUnit timeoutUnit = ChronoUnit.MILLIS;
private final Duration retryDuration;

@Inject
Expand All @@ -131,6 +133,7 @@ public HttpClient(HttpClientConfig config,
collector = new ByteBufCollector(config.getMaxResponseSize());
this.preRequestHooks = preRequestHooks;
this.retryDuration = Duration.ofMillis(config.getRetryDelayMs());
setTimeout(config.getTimeoutMs(), ChronoUnit.MILLIS);
}

public HttpClient(HttpClientConfig config) {
Expand Down Expand Up @@ -259,7 +262,7 @@ public static <T> Mono<Response<T>> getFullResponse(Mono<T> source) {

private <T> Mono<T> convertError(RequestBuilder fullReq, Throwable throwable) {
String request = format("%s, headers: %s", fullReq.getFullUrl(), requestLogger.getHeaderValuesOrRedactClient(fullReq.getHeaders()));
LOG.warn("Failed request. Url: {}", request, throwable);
logFailedRequest(throwable, request);

if (isRetryExhausted(throwable)) {
throwable = throwable.getCause();
Expand All @@ -275,6 +278,15 @@ private <T> Mono<T> convertError(RequestBuilder fullReq, Throwable throwable) {
return Mono.error(throwable);
}

private static void logFailedRequest(Throwable throwable, String request) {
var isExpectedError = throwable instanceof WebException webException &&
webException.getStatus().code() == HttpResponseStatus.NOT_FOUND.code() &&
!"resource.not.found".equals(webException.getError());

var level = isExpectedError ? Level.INFO : Level.WARN;
LOG.atLevel(level).setCause(throwable).log("Failed request. Url: {}", request);
}

protected Flux<Object> parseResponseSingle(Method method, RwHttpClientResponse response) {
if (expectsByteArrayResponse(method)) {
return Flux.from(collector.collectBytes(response.getContent()));
Expand Down Expand Up @@ -534,7 +546,22 @@ protected RequestBuilder createRequest(Method method, Object[] arguments) {
JaxRsMeta meta = getJaxRsMeta(method);

RequestBuilder request = new RequestBuilder(serverInfo, meta.getHttpMethod(), meta.getFullPath());
request.setUri(getPath(method, arguments, meta));

String root = config.getRoot();
String path = getPath(method, arguments, meta);
if (root == null || root.isEmpty()) {
request.setUri(path);
} else {
if (root.endsWith("/")) {
root = root.substring(0, root.length() - 1);
}
if (path.startsWith("/")) {
request.setUri(root + path);
} else {
request.setUri(root + "/" + path);
}
}

setHeaderParams(request, method, arguments);
addCustomParams(request, method, arguments);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
@Config("httpClient")
public class HttpClientConfig {

public static final int DEFAULT_TIMEOUT_MS = 10_000;

@Valid
@JsonProperty("port")
private int port = 80;
Expand All @@ -33,6 +35,7 @@ public class HttpClientConfig {
private int maxConnections = 1000;

private String url;
private String root;
private InetSocketAddress devServerInfo;
@Size(min = 1)
private String devCookie;
Expand All @@ -44,10 +47,12 @@ public class HttpClientConfig {
private boolean isHttps;
private int retryCount = 3;
private int retryDelayMs = 1000;
private int timeoutMs = DEFAULT_TIMEOUT_MS;
private int readTimeoutMs = 10000;
private int poolAcquireTimeoutMs = 10000;
@JsonProperty("validateCertificates")
private boolean isValidateCertificates = true;
private boolean followRedirect = false;

private long connectionMaxIdleTimeInMs = TimeUnit.MILLISECONDS.convert(10, MINUTES);
private int numberOfConnectionFailuresAllowed = 10;
Expand Down Expand Up @@ -88,6 +93,7 @@ public void setUrl(String url) throws URISyntaxException {
}
URI uri = new URI(this.url);
setHost(uri.getHost());
setRoot(uri.getPath());

isHttps = "https".equals(uri.getScheme());
port = uri.getPort();
Expand All @@ -100,6 +106,14 @@ public void setUrl(String url) throws URISyntaxException {
}
}

public void setRoot(String root) {
this.root = root;
}

public String getRoot() {
return root;
}

public InetSocketAddress getDevServerInfo() {
return devServerInfo;
}
Expand Down Expand Up @@ -161,6 +175,14 @@ public void setHost(String host) {
}
}

public int getTimeoutMs() {
return timeoutMs;
}

public void setTimeoutMs(int timeoutMs) {
this.timeoutMs = timeoutMs;
}

public int getReadTimeoutMs() {
return readTimeoutMs;
}
Expand All @@ -177,6 +199,14 @@ public void setValidateCertificates(boolean value) {
isValidateCertificates = value;
}

public boolean isFollowRedirect() {
return followRedirect;
}

public void setFollowRedirect(boolean value) {
followRedirect = value;
}

public BasicAuthConfig getBasicAuth() {
return basicAuth;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ private HttpClient buildClient(InetSocketAddress socketAddress) {
.doOnError((httpClientRequest, throwable) -> {
healthRecorder.logStatus(connectionProvider, errorCount.incrementAndGet() <= config.getNumberOfConnectionFailuresAllowed());
}, (httpClientResponse, throwable) -> { })
.followRedirect(false);
.followRedirect(config.isFollowRedirect());

if (config.isHttps()) {
return setupSsl(client, config.isValidateCertificates());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,14 @@ void shouldInitializeFromConfig() {
assertThat(config.getRetryCount()).isEqualTo(15);
assertThat(config.getRetryDelayMs()).isEqualTo(200);
assertThat(config.getMaxResponseSize()).isEqualTo(512);
assertThat(config.getTimeoutMs()).isEqualTo(60 * 1000);
assertThat(config.isFollowRedirect()).isTrue();

assertThat(config.getDevCookie()).isEqualTo("TEST=123");
assertThat(config.getDevServerInfo().getHostString()).isEqualTo("mymachine");
assertThat(config.getDevServerInfo().getPort()).isEqualTo(9090);
assertThat(config.getDevHeaders()).contains(entry("Host", "localhost"));

}

@Test
Expand Down Expand Up @@ -83,6 +86,14 @@ void shouldSetPort() throws URISyntaxException {
assertThat(config.getHost()).isEqualTo("localhost");
}

@Test
void shouldSetRoot() throws URISyntaxException {
assertThat(new HttpClientConfig("http://localhost:8080").getRoot()).isEqualTo("");
assertThat(new HttpClientConfig("http://localhost:8080/").getRoot()).isEqualTo("/");
assertThat(new HttpClientConfig("http://localhost:8080/root/path").getRoot()).isEqualTo("/root/path");
assertThat(new HttpClientConfig("http://localhost:8080/root/path/").getRoot()).isEqualTo("/root/path/");
}

@Test
void shouldSetHttpsFromProtocolEvenIfPortIsSupplied() throws URISyntaxException {
HttpClientConfig config = new HttpClientConfig("https://localhost:8080");
Expand Down Expand Up @@ -121,4 +132,16 @@ void shouldNotBeInsecureByDefault() throws URISyntaxException {
assertThat(httpClientConfig.isHttps()).isTrue();
assertThat(httpClientConfig.isValidateCertificates()).isTrue();
}

@Test
void shouldNotFollowRedirectByDefault() throws URISyntaxException {
HttpClientConfig httpClientConfig = new HttpClientConfig("http://example.com");
assertThat(httpClientConfig.isFollowRedirect()).isFalse();
}

@Test
void shouldDefaultToTenSecondTimeout() throws URISyntaxException {
HttpClientConfig httpClientConfig = new HttpClientConfig("http://example.com");
assertThat(httpClientConfig.getTimeoutMs()).isEqualTo(10 * 1000);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,23 @@ protected JaxRsMeta getJaxRsMeta(Method method) {
assertThat(jaxRsMetas.get(2)).isSameAs(jaxRsMetas.get(3));
}

@Test
void shouldAddRootToRequestUri() throws URISyntaxException, NoSuchMethodException {

assertAddedRoot("localhost", "/hello");
assertAddedRoot("localhost/", "/hello");
assertAddedRoot("localhost/root", "/root/hello");
assertAddedRoot("localhost/root/", "/root/hello");
}

private void assertAddedRoot(String url, String expected) throws URISyntaxException, NoSuchMethodException {
HttpClient httpClient = new HttpClient(new HttpClientConfig(url));

Method getHello = TestResource.class.getMethod("getHello");

assertThat(httpClient.createRequest(getHello, new Object[0]).getUri()).isEqualTo(expected);
}

@Test
void shouldRetryIfEmptyReturnedOnGet() {

Expand Down Expand Up @@ -685,6 +702,49 @@ void shouldApplyHeaderTransformerInLogsAndExceptionMessage() throws Exception {
loggingVerifier.verify(WARN, "Failed request. Url: localhost:" + server.port() + "/hello, headers: [Host=localhost, someHeader=ABCD]");
}

@Test
void shouldLogWarnOnErrorResponse() throws Exception {
server = startServer(BAD_REQUEST, "");
HttpClientConfig config = new HttpClientConfig("localhost:" + server.port());
TestResource resource = getHttpProxy(config);

assertThatExceptionOfType(WebException.class)
.isThrownBy(() -> resource.getHello()
.toBlocking()
.single());

loggingVerifier.verify(WARN, "Failed request. Url: localhost:" + server.port() + "/hello, headers: [Host=localhost]");
}

@Test
void shouldLogInfoOnNotFoundResponse() throws Exception {
server = startServer(NOT_FOUND, "");
HttpClientConfig config = new HttpClientConfig("localhost:" + server.port());
TestResource resource = getHttpProxy(config);

assertThatExceptionOfType(WebException.class)
.isThrownBy(() -> resource.getHello()
.toBlocking()
.single());

loggingVerifier.verify(INFO, "Failed request. Url: localhost:" + server.port() + "/hello, headers: [Host=localhost]");
}

@Test
void shouldLogWarnOnNotFoundResource() throws Exception {
server = startServer(NOT_FOUND, "{\"error\":\"resource.not.found\"}");
HttpClientConfig config = new HttpClientConfig("localhost:" + server.port());
TestResource resource = getHttpProxy(config);

assertThatExceptionOfType(WebException.class)
.isThrownBy(() -> resource.getHello()
.toBlocking()
.single());

loggingVerifier.verify(WARN, "Failed request. Url: localhost:" + server.port() + "/hello, headers: [Host=localhost]");
}


@Test
void shouldRedactSensitiveHeaderInLogsAndExceptionMessage() throws Exception {
server = startServer(BAD_REQUEST, "someError");
Expand Down Expand Up @@ -1611,7 +1671,7 @@ void shouldLogConfiguredIpAddressOnFailedRequest() throws URISyntaxException {
.cause().cause()
.satisfies(exception -> assertThat(exception)
.hasMessageContaining("URL: %s:%s/hello", host, server.port())));
loggingVerifier.verify(WARN, "Failed request. Url: %1$s:%2$s/hello, headers: [Host=%1$s]".formatted(host, server.port()));
loggingVerifier.verify(INFO, "Failed request. Url: %1$s:%2$s/hello, headers: [Host=%1$s]".formatted(host, server.port()));
}

@Test
Expand Down
2 changes: 2 additions & 0 deletions client/src/test/resources/httpconfig.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,5 @@ httpClient:
retryCount: 15
retryDelayMs: 200
maxResponseSize: 512
timeoutMs: 60000
followRedirect: true
18 changes: 9 additions & 9 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -80,34 +80,34 @@
<reactor-bom.version>2023.0.8</reactor-bom.version>
<netty.version>4.1.111.Final</netty.version>
<jackson-bom.version>2.17.2</jackson-bom.version>
<guava.version>33.2.1-jre</guava.version>
<guava.version>33.3.0-jre</guava.version>
<rxjava.version>1.3.8</rxjava.version>
<hikaricp.version>5.1.0</hikaricp.version>
<slf4j.version>2.0.13</slf4j.version>
<slf4j.version>2.0.16</slf4j.version>
<junit.version>4.13.2</junit.version>
<junit5.version>5.10.3</junit5.version>
<h2.version>2.2.224</h2.version>
<junit5.version>5.11.0</junit5.version>
<h2.version>2.3.232</h2.version>
<snakeyaml.version>2.2</snakeyaml.version>
<hibernate-validator.version>8.0.1.Final</hibernate-validator.version>
<jakarta.validation-api.version>3.1.0</jakarta.validation-api.version>
<log4j.version>2.23.1</log4j.version>

<guice.version>7.0.0</guice.version>
<auto-service.version>1.1.1</auto-service.version>
<assertj.version>3.26.0</assertj.version>
<assertj.version>3.26.3</assertj.version>
<mockito.version>5.12.0</mockito.version>
<jacoco.version>0.8.12</jacoco.version>
<reactiversecontexts.version>1.0.1</reactiversecontexts.version>
<classgraph.version>4.8.174</classgraph.version>
<blockhound.version>1.0.9.RELEASE</blockhound.version>

<maven-gpg-plugin.version>3.2.4</maven-gpg-plugin.version>
<surefire.version>3.3.0</surefire.version>
<surefire.version>3.5.0</surefire.version>
<nexus-staging-maven-plugin.version>1.7.0</nexus-staging-maven-plugin.version>
<maven-compiler-plugin.version>3.13.0</maven-compiler-plugin.version>
<maven-enforcer-plugin.version>3.5.0</maven-enforcer-plugin.version>
<maven-source-plugin.version>3.3.1</maven-source-plugin.version>
<maven-jxr-plugin.version>3.4.0</maven-jxr-plugin.version>
<maven-jxr-plugin.version>3.5.0</maven-jxr-plugin.version>
<maven-javadoc-plugin.version>3.7.0</maven-javadoc-plugin.version>
<maven-checkstyle-plugin.version>3.4.0</maven-checkstyle-plugin.version>
<maven-jar-plugin.version>3.4.2</maven-jar-plugin.version>
Expand All @@ -120,10 +120,10 @@
<jakarta.el.version>4.0.2</jakarta.el.version>
<javax.ws.rs-api.version>2.1.1</javax.ws.rs-api.version>
<maven-deploy-plugin.version>3.1.2</maven-deploy-plugin.version>
<byte-buddy.version>1.14.18</byte-buddy.version>
<byte-buddy.version>1.15.0</byte-buddy.version>
<jakarta.inject-api.version>2.0.1</jakarta.inject-api.version>
<bcpkix-jdk18on.version>1.78.1</bcpkix-jdk18on.version>
<swagger-annotations.version>2.2.22</swagger-annotations.version>
<swagger-annotations.version>2.2.23</swagger-annotations.version>

<sonar.host.url>https://sonarcloud.io</sonar.host.url>
<sonar.organization>fortnoxab</sonar.organization>
Expand Down

0 comments on commit e2ee33a

Please sign in to comment.