Use ribbon configuration for ReadTimeout & ConnectTimeout.

Allows feign/ribbon users to use standard config files rather than creating a bean of Request.Options.

fixes gh-696

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java

@@ -32,6 +32,7 @@ import com.netflix.loadbalancer.ILoadBalancer;
 
 import feign.Client;
 import feign.Feign;
+import feign.Request;
 import feign.httpclient.ApacheHttpClient;
 import feign.okhttp.OkHttpClient;
 
@@ -55,9 +56,16 @@ public class FeignRibbonClientAutoConfiguration {
 
 	@Bean
 	@ConditionalOnMissingBean
-	public Client feignClient(CachingSpringLoadBalancerFactory cachingFactory) {
+	public Client feignClient(CachingSpringLoadBalancerFactory cachingFactory,
+			SpringClientFactory clientFactory) {
 		return new LoadBalancerFeignClient(new Client.Default(null, null),
-				cachingFactory);
+				cachingFactory, clientFactory);
+	}
+
+	@Bean
+	@ConditionalOnMissingBean
+	public Request.Options feignRequestOptions() {
+		return LoadBalancerFeignClient.DEFAULT_OPTIONS;
 	}
 
 	@Configuration
@@ -68,12 +76,10 @@ public class FeignRibbonClientAutoConfiguration {
 		@Autowired(required = false)
 		private HttpClient httpClient;
 
-		@Autowired
-		CachingSpringLoadBalancerFactory cachingFactory;
-
 		@Bean
 		@ConditionalOnMissingBean(Client.class)
-		public Client feignClient() {
+		public Client feignClient(CachingSpringLoadBalancerFactory cachingFactory,
+				SpringClientFactory clientFactory) {
 			ApacheHttpClient delegate;
 			if (this.httpClient != null) {
 				delegate = new ApacheHttpClient(this.httpClient);
@@ -81,7 +87,7 @@ public class FeignRibbonClientAutoConfiguration {
 			else {
 				delegate = new ApacheHttpClient();
 			}
-			return new LoadBalancerFeignClient(delegate, this.cachingFactory);
+			return new LoadBalancerFeignClient(delegate, cachingFactory, clientFactory);
 		}
 	}
 
@@ -93,12 +99,10 @@ public class FeignRibbonClientAutoConfiguration {
 		@Autowired(required = false)
 		private com.squareup.okhttp.OkHttpClient okHttpClient;
 
-		@Autowired
-		CachingSpringLoadBalancerFactory cachingFactory;
-
 		@Bean
 		@ConditionalOnMissingBean(Client.class)
-		public Client feignClient() {
+		public Client feignClient(CachingSpringLoadBalancerFactory cachingFactory,
+				SpringClientFactory clientFactory) {
 			OkHttpClient delegate;
 			if (this.okHttpClient != null) {
 				delegate = new OkHttpClient(this.okHttpClient);
@@ -106,7 +110,7 @@ public class FeignRibbonClientAutoConfiguration {
 			else {
 				delegate = new OkHttpClient();
 			}
-			return new LoadBalancerFeignClient(delegate, this.cachingFactory);
+			return new LoadBalancerFeignClient(delegate, cachingFactory, clientFactory);
 		}
 	}
 }

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClient.java

@@ -19,9 +19,12 @@ package org.springframework.cloud.netflix.feign.ribbon;
 import java.io.IOException;
 import java.net.URI;
 
+import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
+
 import com.netflix.client.ClientException;
 import com.netflix.client.config.CommonClientConfigKey;
 import com.netflix.client.config.DefaultClientConfigImpl;
+import com.netflix.client.config.IClientConfig;
 
 import feign.Client;
 import feign.Request;
@@ -33,13 +36,18 @@ import feign.Response;
  */
 public class LoadBalancerFeignClient implements Client {
 
+	static final Request.Options DEFAULT_OPTIONS = new Request.Options();
+
 	private final Client delegate;
 	private CachingSpringLoadBalancerFactory lbClientFactory;
+	private SpringClientFactory clientFactory;
 
 	public LoadBalancerFeignClient(Client delegate,
-			CachingSpringLoadBalancerFactory lbClientFactory) {
+								   CachingSpringLoadBalancerFactory lbClientFactory,
+								   SpringClientFactory clientFactory) {
 		this.delegate = delegate;
 		this.lbClientFactory = lbClientFactory;
+		this.clientFactory = clientFactory;
 	}
 
 	@Override
@@ -50,8 +58,10 @@ public class LoadBalancerFeignClient implements Client {
 			URI uriWithoutHost = cleanUrl(request.url(), clientName);
 			FeignLoadBalancer.RibbonRequest ribbonRequest = new FeignLoadBalancer.RibbonRequest(
 					this.delegate, request, uriWithoutHost);
+
+			IClientConfig requestConfig = getClientConfig(options, clientName);
 			return lbClient(clientName).executeWithLoadBalancer(ribbonRequest,
-					new FeignOptionsClientConfig(options)).toResponse();
+					requestConfig).toResponse();
 		}
 		catch (ClientException e) {
 			IOException io = findIOException(e);
@@ -62,6 +72,16 @@ public class LoadBalancerFeignClient implements Client {
 		}
 	}
 
+	IClientConfig getClientConfig(Request.Options options, String clientName) {
+		IClientConfig requestConfig;
+		if (options == DEFAULT_OPTIONS) {
+			requestConfig = this.clientFactory.getClientConfig(clientName);
+		} else {
+			requestConfig = new FeignOptionsClientConfig(options);
+		}
+		return requestConfig;
+	}
+
 	protected IOException findIOException(Throwable t) {
 		if (t == null) {
 			return null;

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancerTests.java

@@ -40,7 +40,7 @@ import feign.RequestTemplate;
 import feign.Response;
 import lombok.SneakyThrows;
 
-public class RibbonLoadBalancerTests {
+public class FeignLoadBalancerTests {
 
 	@Mock
 	private Client delegate;

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java

@@ -79,7 +79,7 @@ public class FeignRibbonClientTests {
 
 	// Even though we don't maintain FeignRibbonClient, keep these tests
 	// around to make sure the expected behaviour doesn't break
-	private Client client = new LoadBalancerFeignClient(this.delegate, new CachingSpringLoadBalancerFactory(this.factory));
+	private Client client = new LoadBalancerFeignClient(this.delegate, new CachingSpringLoadBalancerFactory(this.factory), this.factory);
 
 	@Before
 	public void init() {

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClientOverrideTests.java

@@ -0,0 +1,120 @@
+/*
+ * Copyright 2013-2015 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.cloud.netflix.feign.ribbon;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
+import org.springframework.boot.test.SpringApplicationConfiguration;
+import org.springframework.boot.test.WebIntegrationTest;
+import org.springframework.cloud.netflix.feign.EnableFeignClients;
+import org.springframework.cloud.netflix.feign.FeignClient;
+import org.springframework.cloud.netflix.feign.FeignContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.test.annotation.DirtiesContext;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.web.bind.annotation.RequestMapping;
+
+import com.netflix.client.config.CommonClientConfigKey;
+import com.netflix.client.config.IClientConfig;
+
+import feign.Request;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author Spencer Gibb
+ */
+@RunWith(SpringJUnit4ClassRunner.class)
+@SpringApplicationConfiguration(classes = LoadBalancerFeignClientOverrideTests.TestConfiguration.class)
+@WebIntegrationTest(randomPort = true, value = {
+		"spring.application.name=loadBalancerFeignClientTests",
+		"ribbon.ConnectTimeout=5", "ribbon.ReadTimeout=10",
+		"foo.ribbon.ConnectTimeout=7", "foo.ribbon.ReadTimeout=17",
+		"feign.httpclient.enabled=false", "feign.okhttp.enabled=false"})
+@DirtiesContext
+public class LoadBalancerFeignClientOverrideTests {
+
+	@Autowired
+	private FeignContext context;
+
+	@Test
+	public void overrideRequestOptions() {
+		// specific ribbon 'bar' configuration via spring bean
+		Request.Options barOptions = this.context.getInstance("bar", Request.Options.class);
+		assertEquals(1, barOptions.connectTimeoutMillis());
+		assertEquals(2, barOptions.readTimeoutMillis());
+		assertOptions(barOptions, "bar", 1, 2);
+
+		// specific ribbon 'foo' configuration
+		Request.Options fooOptions = this.context.getInstance("foo", Request.Options.class);
+		assertEquals(LoadBalancerFeignClient.DEFAULT_OPTIONS, fooOptions);
+		assertOptions(fooOptions, "foo", 7, 17);
+
+		// generic ribbon default configuration
+		Request.Options bazOptions = this.context.getInstance("baz", Request.Options.class);
+		assertEquals(LoadBalancerFeignClient.DEFAULT_OPTIONS, fooOptions);
+		assertOptions(bazOptions, "baz", 5, 10);
+	}
+
+	void assertOptions(Request.Options options, String name, int expectedConnect, int expectedRead) {
+		LoadBalancerFeignClient client = this.context.getInstance(name, LoadBalancerFeignClient.class);
+		IClientConfig config = client.getClientConfig(options, name);
+		assertEquals("connect was wrong for "+name, expectedConnect, config.get(CommonClientConfigKey.ConnectTimeout, -1).intValue());
+		assertEquals("read was wrong for "+name, expectedRead, config.get(CommonClientConfigKey.ReadTimeout, -1).intValue());
+	}
+
+	@Configuration
+	@EnableFeignClients(clients = { FooClient.class, BarClient.class, BazClient.class })
+	@EnableAutoConfiguration
+	//@Import({ PropertyPlaceholderAutoConfiguration.class, ArchaiusAutoConfiguration.class,
+	//		FeignAutoConfiguration.class })
+	protected static class TestConfiguration {
+	}
+
+	@FeignClient(value = "foo", configuration = FooConfiguration.class)
+	interface FooClient {
+		@RequestMapping("/")
+		String get();
+
+	}
+
+	public static class FooConfiguration {
+	}
+
+	@FeignClient(value = "bar", configuration = BarConfiguration.class)
+	interface BarClient {
+		@RequestMapping("/")
+		String get();
+	}
+
+	public static class BarConfiguration {
+		@Bean
+		public Request.Options feignRequestOptions() {
+			return new Request.Options(1, 2);
+		}
+	}
+
+	@FeignClient(value = "baz")
+	interface BazClient {
+		@RequestMapping("/")
+		String get();
+
+	}
+}