From 3991c8c5f78b6c2d781c5c4b0e86c961378e6fc9 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Tue, 28 Jun 2016 20:54:42 -0600 Subject: [PATCH] Allow custom feign clients when not using ribbon. fixes gh-1143 --- .../cloud/netflix/feign/DefaultTargeter.java | 33 +++++ .../netflix/feign/FeignAutoConfiguration.java | 70 ++++++++++ .../netflix/feign/FeignClientFactoryBean.java | 72 ++-------- .../feign/FeignClientSpecification.java | 2 +- .../netflix/feign/FeignClientsRegistrar.java | 2 +- .../cloud/netflix/feign/HystrixTargeter.java | 54 ++++++++ .../cloud/netflix/feign/Targeter.java | 29 ++++ .../FeignRibbonClientAutoConfiguration.java | 4 +- .../feign/FeignHttpClientUrlTests.java | 129 ++++++++++++++++++ .../feign/valid/FeignHttpClientTests.java | 28 ++-- 10 files changed, 344 insertions(+), 79 deletions(-) create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/DefaultTargeter.java create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/HystrixTargeter.java create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/Targeter.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientUrlTests.java diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/DefaultTargeter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/DefaultTargeter.java new file mode 100644 index 00000000..a51a0793 --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/DefaultTargeter.java @@ -0,0 +1,33 @@ +/* + * Copyright 2013-2016 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; + +import feign.Feign; +import feign.Target; + +/** + * @author Spencer Gibb + */ +class DefaultTargeter implements Targeter { + + @Override + public T target(FeignClientFactoryBean factory, Feign.Builder feign, FeignContext context, + Target.HardCodedTarget target) { + return feign.target(target); + } +} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java index 1ec41430..73c314c6 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java @@ -19,13 +19,20 @@ package org.springframework.cloud.netflix.feign; import java.util.ArrayList; import java.util.List; +import org.apache.http.client.HttpClient; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.client.actuator.HasFeatures; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import feign.Client; import feign.Feign; +import feign.httpclient.ApacheHttpClient; +import feign.okhttp.OkHttpClient; /** * @author Spencer Gibb @@ -49,4 +56,67 @@ public class FeignAutoConfiguration { context.setConfigurations(this.configurations); return context; } + + @Configuration + @ConditionalOnClass(name = "feign.hystrix.HystrixFeign") + protected static class HystrixFeignTargeterConfiguration { + @Bean + @ConditionalOnMissingBean + public Targeter feignTargeter() { + return new HystrixTargeter(); + } + } + + @Configuration + @ConditionalOnMissingClass(name = "feign.hystrix.HystrixFeign") + protected static class DefaultFeignTargeterConfiguration { + @Bean + @ConditionalOnMissingBean + public Targeter feignTargeter() { + return new DefaultTargeter(); + } + } + + // the following configuration is for alternate feign clients if + // ribbon is not on the class path. + // see corresponding confiurations in FeignRibbonClientAutoConfiguration + // for load balanced ribbon clients. + @Configuration + @ConditionalOnClass(ApacheHttpClient.class) + @ConditionalOnMissingClass(name = "com.netflix.loadbalancer.ILoadBalancer") + @ConditionalOnProperty(value = "feign.httpclient.enabled", matchIfMissing = true) + protected static class HttpClientFeignConfiguration { + + @Autowired(required = false) + private HttpClient httpClient; + + @Bean + @ConditionalOnMissingBean(Client.class) + public Client feignClient() { + if (this.httpClient != null) { + return new ApacheHttpClient(this.httpClient); + } + return new ApacheHttpClient(); + } + } + + @Configuration + @ConditionalOnClass(OkHttpClient.class) + @ConditionalOnMissingClass(name = "com.netflix.loadbalancer.ILoadBalancer") + @ConditionalOnProperty(value = "feign.okhttp.enabled", matchIfMissing = true) + protected static class OkHttpFeignConfiguration { + + @Autowired(required = false) + private okhttp3.OkHttpClient okHttpClient; + + @Bean + @ConditionalOnMissingBean(Client.class) + public Client feignClient() { + if (this.okHttpClient != null) { + return new OkHttpClient(this.okHttpClient); + } + return new OkHttpClient(); + } + } + } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientFactoryBean.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientFactoryBean.java index 624d0132..fa345d88 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientFactoryBean.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientFactoryBean.java @@ -21,10 +21,11 @@ import java.util.Map; import org.springframework.beans.BeansException; import org.springframework.beans.factory.FactoryBean; import org.springframework.beans.factory.InitializingBean; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.cloud.netflix.feign.ribbon.LoadBalancerFeignClient; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; import feign.Client; @@ -51,19 +52,8 @@ import lombok.EqualsAndHashCode; class FeignClientFactoryBean implements FactoryBean, InitializingBean, ApplicationContextAware { - private static final Targeter targeter; - - static { - Targeter targeterToUse; - if (ClassUtils.isPresent("feign.hystrix.HystrixFeign", - FeignClientFactoryBean.class.getClassLoader())) { - targeterToUse = new HystrixTargeter(); - } - else { - targeterToUse = new DefaultTargeter(); - } - targeter = targeterToUse; - } + @Autowired + private Targeter targeter; private Class type; @@ -84,6 +74,7 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, Assert.hasText(this.name, "Name must be set"); } + @Override public void setApplicationContext(ApplicationContext context) throws BeansException { this.applicationContext = context; @@ -181,6 +172,15 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, this.url = "http://" + this.url; } String url = this.url + cleanPath(); + Client client = getOptional(context, Client.class); + if (client != null) { + if (client instanceof LoadBalancerFeignClient) { + // not lod balancing because we have a url, + // but ribbon is on the classpath, so unwrap + client = ((LoadBalancerFeignClient)client).getDelegate(); + } + builder.client(client); + } return targeter.target(this, builder, context, new HardCodedTarget<>( this.type, this.name, url)); } @@ -208,48 +208,4 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, return true; } - interface Targeter { - T target(FeignClientFactoryBean factory, Feign.Builder feign, FeignContext context, - HardCodedTarget target); - } - - static class DefaultTargeter implements Targeter { - - @Override - public T target(FeignClientFactoryBean factory, Feign.Builder feign, FeignContext context, - HardCodedTarget target) { - return feign.target(target); - } - } - - @SuppressWarnings("unchecked") - static class HystrixTargeter implements Targeter { - - @Override - public T target(FeignClientFactoryBean factory, Feign.Builder feign, FeignContext context, - HardCodedTarget target) { - if (factory.fallback == void.class - || !(feign instanceof feign.hystrix.HystrixFeign.Builder)) { - return feign.target(target); - } - - Object fallbackInstance = context.getInstance(factory.name, factory.fallback); - if (fallbackInstance == null) { - throw new IllegalStateException(String.format( - "No fallback instance of type %s found for feign client %s", - factory.fallback, factory.name)); - } - - if (!target.type().isAssignableFrom(factory.fallback)) { - throw new IllegalStateException( - String.format( - "Incompatible fallback instance. Fallback of type %s is not assignable to %s for feign client %s", - factory.fallback, target.type(), factory.name)); - } - - feign.hystrix.HystrixFeign.Builder builder = (feign.hystrix.HystrixFeign.Builder) feign; - return builder.target(target, (T) fallbackInstance); - } - } - } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientSpecification.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientSpecification.java index 07b0623b..dd93348a 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientSpecification.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientSpecification.java @@ -28,7 +28,7 @@ import lombok.NoArgsConstructor; @Data @AllArgsConstructor @NoArgsConstructor -public class FeignClientSpecification implements NamedContextFactory.Specification { +class FeignClientSpecification implements NamedContextFactory.Specification { private String name; diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java index 24564433..61feb10b 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java @@ -57,7 +57,7 @@ import org.springframework.util.StringUtils; * @author Jakub Narloch * @author Venil Noronha */ -public class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, +class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, ResourceLoaderAware, BeanClassLoaderAware { // patterned after Spring Integration IntegrationComponentScanRegistrar diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/HystrixTargeter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/HystrixTargeter.java new file mode 100644 index 00000000..5b9ed8d8 --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/HystrixTargeter.java @@ -0,0 +1,54 @@ +/* + * Copyright 2013-2016 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; + +import feign.Feign; +import feign.Target; + +/** + * @author Spencer Gibb + */ +@SuppressWarnings("unchecked") +class HystrixTargeter implements Targeter { + + @Override + public T target(FeignClientFactoryBean factory, Feign.Builder feign, FeignContext context, + Target.HardCodedTarget target) { + if (factory.getFallback() == void.class + || !(feign instanceof feign.hystrix.HystrixFeign.Builder)) { + return feign.target(target); + } + + Object fallbackInstance = context.getInstance(factory.getName(), factory.getFallback()); + if (fallbackInstance == null) { + throw new IllegalStateException(String.format( + "No fallback instance of type %s found for feign client %s", + factory.getFallback(), factory.getName())); + } + + if (!target.type().isAssignableFrom(factory.getFallback())) { + throw new IllegalStateException( + String.format( + "Incompatible fallback instance. Fallback of type %s is not assignable to %s for feign client %s", + factory.getFallback(), target.type(), factory.getName())); + } + + feign.hystrix.HystrixFeign.Builder builder = (feign.hystrix.HystrixFeign.Builder) feign; + return builder.target(target, (T) fallbackInstance); + } +} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/Targeter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/Targeter.java new file mode 100644 index 00000000..3ebd9a86 --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/Targeter.java @@ -0,0 +1,29 @@ +/* + * Copyright 2013-2016 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; + +import feign.Feign; +import feign.Target; + +/** + * @author Spencer Gibb + */ +interface Targeter { + T target(FeignClientFactoryBean factory, Feign.Builder feign, FeignContext context, + Target.HardCodedTarget target); +} 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 7622a60f..d1d8c821 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 @@ -71,7 +71,7 @@ public class FeignRibbonClientAutoConfiguration { @Configuration @ConditionalOnClass(ApacheHttpClient.class) @ConditionalOnProperty(value = "feign.httpclient.enabled", matchIfMissing = true) - protected static class HttpClientConfiguration { + protected static class HttpClientFeignLoadBalancedConfiguration { @Autowired(required = false) private HttpClient httpClient; @@ -94,7 +94,7 @@ public class FeignRibbonClientAutoConfiguration { @Configuration @ConditionalOnClass(OkHttpClient.class) @ConditionalOnProperty(value = "feign.okhttp.enabled", matchIfMissing = true) - protected static class OkHttpConfiguration { + protected static class OkHttpFeignLoadBalancedConfiguration { @Autowired(required = false) private okhttp3.OkHttpClient okHttpClient; diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientUrlTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientUrlTests.java new file mode 100644 index 00000000..b7a2e5b3 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientUrlTests.java @@ -0,0 +1,129 @@ +/* + * Copyright 2013-2016 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; + +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; + +import java.lang.reflect.Field; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +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.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.util.ReflectionUtils; +import org.springframework.util.SocketUtils; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.RestController; + +import feign.Client; +import feign.Feign; +import feign.Target; +import feign.httpclient.ApacheHttpClient; +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; + +/** + * @author Spencer Gibb + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration(classes = FeignHttpClientUrlTests.TestConfig.class) +@WebIntegrationTest(value = { + "spring.application.name=feignclienturltest", "feign.hystrix.enabled=false", + "feign.okhttp.enabled=false" }) +@DirtiesContext +public class FeignHttpClientUrlTests { + + @BeforeClass + public static void beforeClass() { + int port = SocketUtils.findAvailableTcpPort(); + System.setProperty("server.port", String.valueOf(port)); + } + + @AfterClass + public static void afterClass() { + System.clearProperty("server.port"); + } + + @Autowired + private UrlClient urlClient; + + // this tests that + @FeignClient(name = "localappurl", url = "http://localhost:${server.port}/") + protected interface UrlClient { + @RequestMapping(method = RequestMethod.GET, value = "/hello") + Hello getHello(); + } + + @Configuration + @EnableAutoConfiguration + @RestController + @EnableFeignClients(clients = { UrlClient.class }) + protected static class TestConfig { + + @RequestMapping(method = RequestMethod.GET, value = "/hello") + public Hello getHello() { + return new Hello("hello world 1"); + } + + @Bean + public Targeter feignTargeter() { + return new Targeter() { + @Override + public T target(FeignClientFactoryBean factory, Feign.Builder feign, FeignContext context, Target.HardCodedTarget target) { + Field field = ReflectionUtils.findField(Feign.Builder.class, "client"); + ReflectionUtils.makeAccessible(field); + Client client = (Client) ReflectionUtils.getField(field, feign); + if (target.name().equals("localappurl")) { + assertThat("client was wrong type", client, is(instanceOf(ApacheHttpClient.class))); + } + return feign.target(target); + } + }; + } + + } + + @Test + public void testUrlHttpClient() { + assertNotNull("UrlClient was null", this.urlClient); + Hello hello = this.urlClient.getHello(); + assertNotNull("hello was null", hello); + assertEquals("first hello didn't match", new Hello("hello world 1"), hello); + } + + @Data + @AllArgsConstructor + @NoArgsConstructor + public static class Hello { + private String message; + } +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignHttpClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignHttpClientTests.java index 43dc9c95..0bc72850 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignHttpClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignHttpClientTests.java @@ -16,15 +16,19 @@ package org.springframework.cloud.netflix.feign.valid; -import com.netflix.loadbalancer.Server; -import com.netflix.loadbalancer.ServerList; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; -import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.test.SpringApplicationConfiguration; import org.springframework.boot.test.WebIntegrationTest; import org.springframework.cloud.netflix.feign.EnableFeignClients; @@ -42,25 +46,20 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RestController; +import com.netflix.loadbalancer.Server; +import com.netflix.loadbalancer.ServerList; + import feign.Client; import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.notNullValue; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; - /** * @author Spencer Gibb */ @RunWith(SpringJUnit4ClassRunner.class) @SpringApplicationConfiguration(classes = FeignHttpClientTests.Application.class) -@WebIntegrationTest(randomPort = true, value = { "server.port=0", +@WebIntegrationTest(randomPort = true, value = { "spring.application.name=feignclienttest", "feign.hystrix.enabled=false", "feign.okhttp.enabled=false" }) @DirtiesContext @@ -121,11 +120,6 @@ public class FeignHttpClientTests { return new User("John Smith"); } - public static void main(String[] args) { - new SpringApplicationBuilder(Application.class).properties( - "spring.application.name=feignclienttest", - "management.contextPath=/admin").run(args); - } } @Test