diff --git a/client/src/main/java/se/fortnox/reactivewizard/client/HttpClient.java b/client/src/main/java/se/fortnox/reactivewizard/client/HttpClient.java index 1b8414d3..66a8ad82 100644 --- a/client/src/main/java/se/fortnox/reactivewizard/client/HttpClient.java +++ b/client/src/main/java/se/fortnox/reactivewizard/client/HttpClient.java @@ -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; @@ -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); @@ -103,8 +105,8 @@ public class HttpClient implements InvocationHandler { private final RequestLogger requestLogger; private final Map, List> beanParamCache = new ConcurrentHashMap<>(); private final Map 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 @@ -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) { @@ -259,7 +262,7 @@ public static Mono> getFullResponse(Mono source) { private Mono 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(); @@ -275,6 +278,15 @@ private Mono 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 parseResponseSingle(Method method, RwHttpClientResponse response) { if (expectsByteArrayResponse(method)) { return Flux.from(collector.collectBytes(response.getContent())); @@ -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); diff --git a/client/src/main/java/se/fortnox/reactivewizard/client/HttpClientConfig.java b/client/src/main/java/se/fortnox/reactivewizard/client/HttpClientConfig.java index f31355b9..034831fe 100644 --- a/client/src/main/java/se/fortnox/reactivewizard/client/HttpClientConfig.java +++ b/client/src/main/java/se/fortnox/reactivewizard/client/HttpClientConfig.java @@ -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; @@ -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; @@ -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; @@ -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(); @@ -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; } @@ -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; } @@ -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; } diff --git a/client/src/main/java/se/fortnox/reactivewizard/client/ReactorRxClientProvider.java b/client/src/main/java/se/fortnox/reactivewizard/client/ReactorRxClientProvider.java index ca5b5ea6..2755f1b3 100644 --- a/client/src/main/java/se/fortnox/reactivewizard/client/ReactorRxClientProvider.java +++ b/client/src/main/java/se/fortnox/reactivewizard/client/ReactorRxClientProvider.java @@ -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()); diff --git a/client/src/test/java/se/fortnox/reactivewizard/client/HttpClientConfigTest.java b/client/src/test/java/se/fortnox/reactivewizard/client/HttpClientConfigTest.java index 25fae64b..db14640b 100644 --- a/client/src/test/java/se/fortnox/reactivewizard/client/HttpClientConfigTest.java +++ b/client/src/test/java/se/fortnox/reactivewizard/client/HttpClientConfigTest.java @@ -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 @@ -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"); @@ -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); + } } diff --git a/client/src/test/java/se/fortnox/reactivewizard/client/HttpClientTest.java b/client/src/test/java/se/fortnox/reactivewizard/client/HttpClientTest.java index 6735ca50..b2bc78f4 100644 --- a/client/src/test/java/se/fortnox/reactivewizard/client/HttpClientTest.java +++ b/client/src/test/java/se/fortnox/reactivewizard/client/HttpClientTest.java @@ -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() { @@ -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"); @@ -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 diff --git a/client/src/test/resources/httpconfig.yml b/client/src/test/resources/httpconfig.yml index 817897a1..1cd1438d 100644 --- a/client/src/test/resources/httpconfig.yml +++ b/client/src/test/resources/httpconfig.yml @@ -9,3 +9,5 @@ httpClient: retryCount: 15 retryDelayMs: 200 maxResponseSize: 512 + timeoutMs: 60000 + followRedirect: true diff --git a/pom.xml b/pom.xml index 6e893904..d8672121 100644 --- a/pom.xml +++ b/pom.xml @@ -80,13 +80,13 @@ 2023.0.8 4.1.111.Final 2.17.2 - 33.2.1-jre + 33.3.0-jre 1.3.8 5.1.0 - 2.0.13 + 2.0.16 4.13.2 - 5.10.3 - 2.2.224 + 5.11.0 + 2.3.232 2.2 8.0.1.Final 3.1.0 @@ -94,7 +94,7 @@ 7.0.0 1.1.1 - 3.26.0 + 3.26.3 5.12.0 0.8.12 1.0.1 @@ -102,12 +102,12 @@ 1.0.9.RELEASE 3.2.4 - 3.3.0 + 3.5.0 1.7.0 3.13.0 3.5.0 3.3.1 - 3.4.0 + 3.5.0 3.7.0 3.4.0 3.4.2 @@ -120,10 +120,10 @@ 4.0.2 2.1.1 3.1.2 - 1.14.18 + 1.15.0 2.0.1 1.78.1 - 2.2.22 + 2.2.23 https://sonarcloud.io fortnoxab