diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java index 420bd2cd..1a98f8c2 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java +++ b/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); } } } \ No newline at end of file diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClient.java index 4060fc04..37d7f662 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClient.java +++ b/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; diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RibbonLoadBalancerTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancerTests.java similarity index 99% rename from spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RibbonLoadBalancerTests.java rename to spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancerTests.java index 734bc651..cb9ad67a 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RibbonLoadBalancerTests.java +++ b/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; diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java index 714991b0..63e8753a 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java +++ b/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() { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClientOverrideTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClientOverrideTests.java new file mode 100644 index 00000000..9bcb1d14 --- /dev/null +++ b/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(); + + } +}