[bug] set connection time out and request time out incorrectly

change unit of time to millisecond and remove timeToLiveUnit
Tosan · May 13, 2024 · 017b530 · 017b530
1 parent 4100902
commit 017b530
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 56 deletions.
diff --git a/pom.xml b/pom.xml
@@ -9,7 +9,7 @@
     <parent>
         <groupId>org.springframework.boot</groupId>
         <artifactId>spring-boot-starter-parent</artifactId>
-        <version>3.2.2</version>
+        <version>3.2.4</version>
         <relativePath />
     </parent>
 
@@ -52,7 +52,7 @@
 
     <properties>
         <java.version>17</java.version>
-        <spring-cloud.version>2023.0.0</spring-cloud.version>
+        <spring-cloud.version>2023.0.1</spring-cloud.version>
         <tosan.mask.version>2.2.0</tosan.mask.version>
     </properties>
 

diff --git a/...lient-spring-boot-core/src/main/java/com/tosan/client/http/core/HttpClientProperties.java b/...lient-spring-boot-core/src/main/java/com/tosan/client/http/core/HttpClientProperties.java
@@ -4,8 +4,6 @@
 import org.springframework.boot.context.properties.NestedConfigurationProperty;
 import org.springframework.validation.annotation.Validated;
 
-import java.util.concurrent.TimeUnit;
-
 /**
  * @author Ali Alimohammadi
  * @since 1/22/2021
@@ -260,22 +258,17 @@ public static class ConnectionConfiguration {
         public static final int DEFAULT_MAX_CONNECTIONS_PER_ROUTE = 50;
 
         /**
-         * Default value for time to live.
-         */
-        public static final long DEFAULT_TIME_TO_LIVE = 900L;
-
-        /**
-         * Default time to live unit.
+         * Default value for time to live in milliseconds.
          */
-        public static final TimeUnit DEFAULT_TIME_TO_LIVE_UNIT = TimeUnit.SECONDS;
+        public static final long DEFAULT_TIME_TO_LIVE = 900000L;
 
         /**
          * Default value for following redirects.
          */
         public static final boolean DEFAULT_FOLLOW_REDIRECTS = true;
 
         /**
-         * Default value for connection timeout.
+         * Default value for connection timeout in milliseconds.
          * A timeout value of zero is interpreted as an infinite timeout.
          * A negative value is interpreted as undefined (system default if applicable).
          */
@@ -287,7 +280,7 @@ public static class ConnectionConfiguration {
         public static final int DEFAULT_CONNECTION_TIMER_REPEAT = 3000;
 
         /**
-         * Default value for socket timeout.
+         * Default value for socket timeout in milliseconds.
          * A timeout value of zero is interpreted as an infinite timeout.
          * A negative value is interpreted as undefined (system default if applicable).
          */
@@ -299,8 +292,6 @@ public static class ConnectionConfiguration {
 
         private long timeToLive = DEFAULT_TIME_TO_LIVE;
 
-        private TimeUnit timeToLiveUnit = DEFAULT_TIME_TO_LIVE_UNIT;
-
         private boolean followRedirects = DEFAULT_FOLLOW_REDIRECTS;
 
         private int connectionTimeout = DEFAULT_CONNECTION_TIMEOUT;
@@ -332,18 +323,13 @@ public long getTimeToLive() {
             return timeToLive;
         }
 
+        /**
+         * @param timeToLive Defines the total span of time connections can be kept alive or execute requests in seconds.
+         */
         public void setTimeToLive(long timeToLive) {
             this.timeToLive = timeToLive;
         }
 
-        public TimeUnit getTimeToLiveUnit() {
-            return timeToLiveUnit;
-        }
-
-        public void setTimeToLiveUnit(TimeUnit timeToLiveUnit) {
-            this.timeToLiveUnit = timeToLiveUnit;
-        }
-
         public boolean isFollowRedirects() {
             return followRedirects;
         }

diff --git a/...src/main/java/com/tosan/client/http/core/factory/ConfigurableApacheHttpClientFactory.java b/...src/main/java/com/tosan/client/http/core/factory/ConfigurableApacheHttpClientFactory.java
@@ -5,6 +5,7 @@
 import org.apache.commons.lang3.StringUtils;
 import org.apache.hc.client5.http.auth.AuthScope;
 import org.apache.hc.client5.http.auth.UsernamePasswordCredentials;
+import org.apache.hc.client5.http.config.ConnectionConfig;
 import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy;
 import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider;
@@ -16,6 +17,7 @@
 import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.util.TimeValue;
+import org.apache.hc.core5.util.Timeout;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -24,7 +26,6 @@
 import java.security.KeyStore;
 import java.util.Timer;
 import java.util.TimerTask;
-import java.util.concurrent.TimeUnit;
 
 /**
  * Factory used to create a HttpClient Instance
@@ -48,7 +49,7 @@ public ConfigurableApacheHttpClientFactory(HttpClientBuilder httpClientBuilder,
     }
 
     public HttpClientBuilder createBuilder() {
-        configureTimeouts(httpClientBuilder);
+        configureRequest(httpClientBuilder);
         configureConnectionManager(httpClientBuilder);
         HttpClientProperties.ProxyConfiguration proxyConfig = httpClientProperties.getProxy();
         if (proxyConfig.isEnable()) {
@@ -59,22 +60,22 @@ public HttpClientBuilder createBuilder() {
         return httpClientBuilder;
     }
 
-    private void configureTimeouts(HttpClientBuilder builder) {
+    private void configureRequest(HttpClientBuilder builder) {
         builder.setDefaultRequestConfig(RequestConfig.custom()
-                        .setConnectTimeout(httpClientProperties.getConnection().getConnectionTimeout(), TimeUnit.MILLISECONDS)
-                        .setConnectionRequestTimeout(httpClientProperties.getConnection().getSocketTimeout(), TimeUnit.MILLISECONDS)
-                        .setRedirectsEnabled(httpClientProperties.getConnection().isFollowRedirects())
-                        .setCookieSpec(httpClientProperties.getConnection().getCookieSpecPolicy())
-                        .build());
+                .setRedirectsEnabled(httpClientProperties.getConnection().isFollowRedirects())
+                .setCookieSpec(httpClientProperties.getConnection().getCookieSpecPolicy())
+                .build());
     }
 
     private void configureConnectionManager(HttpClientBuilder builder) {
-        TimeValue timeToLive = TimeValue.of(httpClientProperties.getConnection().getTimeToLive(),
-                httpClientProperties.getConnection().getTimeToLiveUnit());
         connectionManagerBuilder
                 .setMaxConnTotal(httpClientProperties.getConnection().getMaxConnections())
                 .setMaxConnPerRoute(httpClientProperties.getConnection().getMaxConnectionsPerRoute())
-                .setConnectionTimeToLive(timeToLive);
+                .setDefaultConnectionConfig(ConnectionConfig.custom()
+                        .setConnectTimeout(Timeout.ofMilliseconds(httpClientProperties.getConnection().getConnectionTimeout()))
+                        .setSocketTimeout(Timeout.ofMilliseconds(httpClientProperties.getConnection().getSocketTimeout()))
+                        .setTimeToLive(TimeValue.ofMilliseconds(httpClientProperties.getConnection().getTimeToLive()))
+                        .build());
         String baseServiceUrl = httpClientProperties.getBaseServiceUrl();
         if (baseServiceUrl != null && baseServiceUrl.startsWith("https")) {
             configureSSL(connectionManagerBuilder);

diff --git a/...est/java/com/tosan/client/http/core/factory/ConfigurableApacheHttpClientFactoryUTest.java b/...est/java/com/tosan/client/http/core/factory/ConfigurableApacheHttpClientFactoryUTest.java
@@ -11,7 +11,6 @@
 import org.apache.hc.client5.http.impl.routing.SystemDefaultRoutePlanner;
 import org.apache.hc.client5.http.routing.HttpRoutePlanner;
 import org.apache.hc.core5.http.protocol.BasicHttpContext;
-import org.apache.hc.core5.util.Timeout;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -58,29 +57,10 @@ public void createBuilder_defaultConfiguration() {
         HttpClientBuilder builder = underTest.createBuilder();
         RequestConfig requestConfig = (RequestConfig) ReflectionTestUtils.getField(builder, HttpClientBuilder.class, "defaultRequestConfig");
         assertThat(requestConfig).isNotNull();
-        assertThat(requestConfig.getConnectTimeout())
-                .isEqualTo(Timeout.ofMilliseconds(HttpClientProperties.ConnectionConfiguration.DEFAULT_CONNECTION_TIMEOUT));
-        assertThat(requestConfig.getConnectionRequestTimeout())
-                .isEqualTo(Timeout.ofMilliseconds(HttpClientProperties.ConnectionConfiguration.DEFAULT_SOCKET_TIMEOUT));
-
         HttpRoutePlanner proxySelector = (SystemDefaultRoutePlanner) ReflectionTestUtils.getField(builder, HttpClientBuilder.class, "routePlanner");
         assertThat(proxySelector).isNull();
     }
 
-    @Test
-    public void createBuilder_timeoutConfiguration() {
-        connectionConfiguration.setConnectionTimeout(1234);
-        connectionConfiguration.setSocketTimeout(5678);
-        ConfigurableApacheHttpClientFactory underTest = new ConfigurableApacheHttpClientFactory(HttpClientBuilder.create(),
-                PoolingHttpClientConnectionManagerBuilder.create(), httpClientProperties);
-        HttpClientBuilder builder = underTest.createBuilder();
-
-        RequestConfig requestConfig = (RequestConfig) ReflectionTestUtils.getField(builder, HttpClientBuilder.class, "defaultRequestConfig");
-
-        assertThat(requestConfig.getConnectTimeout()).isEqualTo(Timeout.ofMilliseconds(1234));
-        assertThat(requestConfig.getConnectionRequestTimeout()).isEqualTo(Timeout.ofMilliseconds(5678));
-    }
-
     @Test
     public void createBuilder_proxyConfiguration_noAuthentication() {
         when(httpClientProperties.getProxy()).thenReturn(hostConfig);

diff --git a/...main/java/com/tosan/client/http/resttemplate/starter/util/HttpLoggingInterceptorUtil.java b/...main/java/com/tosan/client/http/resttemplate/starter/util/HttpLoggingInterceptorUtil.java
@@ -12,6 +12,7 @@
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpRequest;
 import org.springframework.http.client.ClientHttpResponse;
+import org.springframework.http.converter.json.Jackson2ObjectMapperBuilder;
 import org.springframework.util.StreamUtils;
 
 import java.io.IOException;
@@ -26,7 +27,7 @@
  * @since 8/3/2022
  */
 public class HttpLoggingInterceptorUtil {
-    private static final ObjectMapper mapper = new ObjectMapper();
+    private static final ObjectMapper mapper = new Jackson2ObjectMapperBuilder().build();
 
     static {
         mapper.enable(SerializationFeature.INDENT_OUTPUT)

diff --git a/...ot-starter/src/main/java/com/tosan/client/http/starter/impl/feign/CustomErrorDecoder.java b/...ot-starter/src/main/java/com/tosan/client/http/starter/impl/feign/CustomErrorDecoder.java
@@ -46,7 +46,8 @@ public Exception decode(String methodKey, Response response) {
             Response.Body body = response.body();
             String responseBody = StreamUtils.copyToString(body.asInputStream(), StandardCharsets.UTF_8);
             int status = response.status();
-            LOGGER.info("ServerErrorResponse:\n ResponseStatus: {}\n ResponseBody: {}", status, responseBody);
+            LOGGER.info("ServerErrorResponse :\n ResponseStatus:{}\n ResponseBody:{}", status,
+                    responseBody);
             Map<String, Class<? extends Exception>> exceptionMap = customErrorDecoderConfig.getExceptionMap();
             if (status >= 400 && status < 500) {
                 return extractBadRequestErrorException(responseBody, exceptionMap);