From 146513aeb02a41cac7bde066f8501ecf5f5f0703 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Fri, 8 Sep 2017 10:19:01 -0400 Subject: [PATCH] Set Ribbon connect and read timeout values to equal the Hystrix timeout. Fixes #1827. --- .../ribbon/RibbonClientConfiguration.java | 6 +++ .../support/AbstractLoadBalancingClient.java | 5 +- .../route/support/AbstractRibbonCommand.java | 4 +- .../RibbonLoadBalancingHttpClientTests.java | 7 +++ .../OkHttpLoadBalancingClientTests.java | 7 +++ .../HttpClientRibbonCommandFactoryTest.java | 50 +++++++++++++++++++ .../OkHttpRibbonCommandFactoryTest.java | 50 +++++++++++++++++++ 7 files changed, 126 insertions(+), 3 deletions(-) create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactoryTest.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactoryTest.java diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java index a80816d8..6a38e720 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java @@ -35,6 +35,7 @@ import org.springframework.context.annotation.Import; import com.netflix.client.DefaultLoadBalancerRetryHandler; import com.netflix.client.RetryHandler; +import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.DefaultClientConfigImpl; import com.netflix.client.config.IClientConfig; import com.netflix.loadbalancer.ConfigurationBasedServerList; @@ -68,6 +69,9 @@ import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttps @Import({HttpClientConfiguration.class, OkHttpRibbonConfiguration.class, RestClientRibbonConfiguration.class, HttpClientRibbonConfiguration.class}) public class RibbonClientConfiguration { + public static final int DEFAULT_CONNECT_TIMEOUT = 1000; + public static final int DEFAULT_READ_TIMEOUT = 1000; + @Value("${ribbon.client.name}") private String name = "client"; @@ -82,6 +86,8 @@ public class RibbonClientConfiguration { public IClientConfig ribbonClientConfig() { DefaultClientConfigImpl config = new DefaultClientConfigImpl(); config.loadProperties(this.name); + config.set(CommonClientConfigKey.ConnectTimeout, DEFAULT_CONNECT_TIMEOUT); + config.set(CommonClientConfigKey.ReadTimeout, DEFAULT_READ_TIMEOUT); return config; } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/support/AbstractLoadBalancingClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/support/AbstractLoadBalancingClient.java index 3c9abf0c..22659d30 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/support/AbstractLoadBalancingClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/support/AbstractLoadBalancingClient.java @@ -18,6 +18,7 @@ package org.springframework.cloud.netflix.ribbon.support; import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; +import org.springframework.cloud.netflix.ribbon.RibbonClientConfiguration; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import com.netflix.client.AbstractLoadBalancerAwareClient; @@ -92,10 +93,10 @@ public abstract class AbstractLoadBalancingClient()); + } + + @Test + public void testHystrixTimeoutValue() throws Exception { + RibbonCommandContext context = mock(RibbonCommandContext.class); + doReturn("service").when(context).getServiceId(); + HttpClientRibbonCommand ribbonCommand = this.ribbonCommandFactory.create(context); + assertEquals(2000, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue()); + } + +} \ No newline at end of file diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactoryTest.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactoryTest.java new file mode 100644 index 00000000..392a182b --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactoryTest.java @@ -0,0 +1,50 @@ +package org.springframework.cloud.netflix.zuul.filters.route.okhttp; + +import java.util.HashSet; +import org.junit.Before; +import org.junit.Test; +import org.springframework.cloud.netflix.ribbon.SpringClientFactory; +import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpLoadBalancingClient; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; +import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; +import org.springframework.cloud.netflix.zuul.filters.route.ZuulFallbackProvider; + +import com.netflix.client.config.DefaultClientConfigImpl; +import com.netflix.client.config.IClientConfig; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; + +/** + * @author Ryan Baxter + */ +public class OkHttpRibbonCommandFactoryTest { + + SpringClientFactory springClientFactory; + ZuulProperties zuulProperties; + OkHttpRibbonCommandFactory commandFactory; + + @Before + public void setup() { + this.springClientFactory = mock(SpringClientFactory.class); + this.zuulProperties = new ZuulProperties(); + OkHttpLoadBalancingClient loadBalancingHttpClient = mock(OkHttpLoadBalancingClient.class); + IClientConfig clientConfig = new DefaultClientConfigImpl(); + doReturn(loadBalancingHttpClient).when(this.springClientFactory).getClient(anyString(), + eq(OkHttpLoadBalancingClient.class)); + doReturn(clientConfig).when(this.springClientFactory).getClientConfig(anyString()); + commandFactory = new OkHttpRibbonCommandFactory(springClientFactory, zuulProperties, new HashSet()); + } + + @Test + public void testHystrixTimeoutValue() throws Exception { + RibbonCommandContext context = mock(RibbonCommandContext.class); + doReturn("service").when(context).getServiceId(); + OkHttpRibbonCommand ribbonCommand = this.commandFactory.create(context); + assertEquals(2000, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue()); + } + +} \ No newline at end of file