diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java index b623652a..75e8f4e4 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java @@ -75,12 +75,13 @@ public class RibbonLoadBalancingHttpClient public RibbonApacheHttpResponse execute(RibbonApacheHttpRequest request, final IClientConfig configOverride) throws Exception { final RequestConfig.Builder builder = RequestConfig.custom(); - if (configOverride != null) { - builder.setConnectTimeout(configOverride.get( + IClientConfig config = configOverride != null ? configOverride : this.config; + if (config != null) { + builder.setConnectTimeout(config.get( CommonClientConfigKey.ConnectTimeout, this.connectTimeout)); - builder.setSocketTimeout(configOverride.get( + builder.setSocketTimeout(config.get( CommonClientConfigKey.ReadTimeout, this.readTimeout)); - builder.setRedirectsEnabled(configOverride.get( + builder.setRedirectsEnabled(config.get( CommonClientConfigKey.FollowRedirects, this.followRedirects)); } else { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClient.java index 33851327..a01db69b 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClient.java @@ -16,22 +16,20 @@ package org.springframework.cloud.netflix.ribbon.okhttp; +import okhttp3.OkHttpClient; +import okhttp3.Request; +import okhttp3.Response; + import java.net.URI; import java.util.concurrent.TimeUnit; - import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.cloud.netflix.ribbon.support.AbstractLoadBalancingClient; import org.springframework.web.util.UriComponentsBuilder; - import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.IClientConfig; import com.netflix.loadbalancer.ILoadBalancer; import com.netflix.loadbalancer.Server; -import okhttp3.OkHttpClient; -import okhttp3.Request; -import okhttp3.Response; - import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttpsIfNeeded; /** @@ -85,12 +83,13 @@ public class OkHttpLoadBalancingClient OkHttpClient getOkHttpClient(IClientConfig configOverride, boolean secure) { OkHttpClient.Builder builder = this.delegate.newBuilder(); - if (configOverride != null) { - builder.connectTimeout(configOverride.get( + IClientConfig config = configOverride != null ? configOverride : this.config; + if (config != null) { + builder.connectTimeout(config.get( CommonClientConfigKey.ConnectTimeout, this.connectTimeout), TimeUnit.MILLISECONDS); - builder.readTimeout(configOverride.get( + builder.readTimeout(config.get( CommonClientConfigKey.ReadTimeout, this.readTimeout), TimeUnit.MILLISECONDS); - builder.followRedirects(configOverride.get( + builder.followRedirects(config.get( CommonClientConfigKey.FollowRedirects, this.followRedirects)); if (secure) { builder.followSslRedirects(configOverride.get( diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java index cc372125..76d48c06 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java @@ -114,6 +114,21 @@ public class RibbonLoadBalancingHttpClientTests { assertThat(result.isRedirectsEnabled(), is(false)); } + @Test + public void testUpdatedTimeouts() + throws Exception { + SpringClientFactory factory = new SpringClientFactory(); + RequestConfig result = getBuiltRequestConfig(Timeouts.class, null, factory); + assertThat(result.getConnectTimeout(), is(60000)); + assertThat(result.getSocketTimeout(), is (50000)); + IClientConfig config = factory.getClientConfig("service"); + config.set(CommonClientConfigKey.ConnectTimeout, 60); + config.set(CommonClientConfigKey.ReadTimeout, 50); + result = getBuiltRequestConfig(Timeouts.class, null, factory); + assertThat(result.getConnectTimeout(), is(60)); + assertThat(result.getSocketTimeout(), is (50)); + } + @Configuration protected static class UseDefaults { @@ -164,8 +179,13 @@ public class RibbonLoadBalancingHttpClientTests { private RequestConfig getBuiltRequestConfig(Class defaultConfigurationClass, IClientConfig configOverride) throws Exception { + return getBuiltRequestConfig(defaultConfigurationClass, configOverride, new SpringClientFactory()); + } + + private RequestConfig getBuiltRequestConfig(Class defaultConfigurationClass, + IClientConfig configOverride, SpringClientFactory factory) + throws Exception { - SpringClientFactory factory = new SpringClientFactory(); factory.setApplicationContext(new AnnotationConfigApplicationContext( RibbonAutoConfiguration.class, defaultConfigurationClass)); HttpClient delegate = mock(HttpClient.class); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClientTests.java index fb8fab92..750163ea 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClientTests.java @@ -16,8 +16,7 @@ package org.springframework.cloud.netflix.ribbon.okhttp; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertThat; +import okhttp3.OkHttpClient; import org.junit.Test; import org.springframework.cloud.netflix.ribbon.RibbonAutoConfiguration; @@ -25,12 +24,12 @@ import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; - import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.DefaultClientConfigImpl; import com.netflix.client.config.IClientConfig; -import okhttp3.OkHttpClient; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; /** * @author Spencer Gibb @@ -84,9 +83,45 @@ public class OkHttpLoadBalancingClientTests { assertThat(result.followRedirects(), is(false)); } + @Test + public void testTimeouts() throws Exception { + OkHttpClient result = getHttpClient(Timeouts.class, null); + assertThat(result.readTimeoutMillis(), is(50000)); + assertThat(result.connectTimeoutMillis(), is(60000)); + } + + @Test + public void testTimeoutsOverride() throws Exception { + DefaultClientConfigImpl override = new DefaultClientConfigImpl(); + override.set(CommonClientConfigKey.ConnectTimeout, 60); + override.set(CommonClientConfigKey.ReadTimeout, 50); + OkHttpClient result = getHttpClient(Timeouts.class, override); + assertThat(result.readTimeoutMillis(), is(50)); + assertThat(result.connectTimeoutMillis(), is(60)); + } + + @Test + public void testUpdatedTimeouts() throws Exception { + SpringClientFactory factory = new SpringClientFactory(); + OkHttpClient result = getHttpClient(Timeouts.class, null, factory); + assertThat(result.readTimeoutMillis(), is(50000)); + assertThat(result.connectTimeoutMillis(), is(60000)); + IClientConfig config = factory.getClientConfig("service"); + config.set(CommonClientConfigKey.ConnectTimeout, 60); + config.set(CommonClientConfigKey.ReadTimeout, 50); + result = getHttpClient(Timeouts.class, null, factory); + assertThat(result.readTimeoutMillis(), is(50)); + assertThat(result.connectTimeoutMillis(), is(60)); + } + private OkHttpClient getHttpClient(Class defaultConfigurationClass, IClientConfig configOverride) throws Exception { - SpringClientFactory factory = new SpringClientFactory(); + return getHttpClient(defaultConfigurationClass, configOverride, new SpringClientFactory()); + } + + private OkHttpClient getHttpClient(Class defaultConfigurationClass, + IClientConfig configOverride, + SpringClientFactory factory) throws Exception { factory.setApplicationContext(new AnnotationConfigApplicationContext( RibbonAutoConfiguration.class, defaultConfigurationClass)); @@ -121,4 +156,15 @@ public class OkHttpLoadBalancingClientTests { } } + @Configuration + protected static class Timeouts { + @Bean + public IClientConfig clientConfig() { + DefaultClientConfigImpl config = new DefaultClientConfigImpl(); + config.set(CommonClientConfigKey.ConnectTimeout, 60000); + config.set(CommonClientConfigKey.ReadTimeout, 50000); + return config; + } + } + }