From f60cc6fb427020340af547ef6167d08776675ef3 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Fri, 4 Sep 2015 15:22:48 -0600 Subject: [PATCH] Change from @Autowired to context.getBean. Solves problems where FeignClientFactoryBean is run before auto-configuration. fixed gh-441 --- .../netflix/feign/FeignClientFactoryBean.java | 101 +++++++++--------- .../valid/FeignClientValidationTests.java | 33 +++++- 2 files changed, 80 insertions(+), 54 deletions(-) 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 b8af29be..7661f110 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 @@ -16,14 +16,17 @@ package org.springframework.cloud.netflix.feign; -import java.util.List; +import java.util.Map; import lombok.Data; import lombok.EqualsAndHashCode; +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.beans.factory.NoSuchBeanDefinitionException; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -37,7 +40,6 @@ import feign.Retryer; import feign.codec.Decoder; import feign.codec.Encoder; import feign.codec.ErrorDecoder; -import feign.ribbon.LoadBalancingTarget; import feign.slf4j.Slf4jLogger; /** @@ -45,7 +47,7 @@ import feign.slf4j.Slf4jLogger; */ @Data @EqualsAndHashCode(callSuper = false) -class FeignClientFactoryBean implements FactoryBean, InitializingBean { +class FeignClientFactoryBean implements FactoryBean, InitializingBean, ApplicationContextAware { private Class type; @@ -53,35 +55,7 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean { private String url; - @Autowired - private Decoder decoder; - - @Autowired - private Encoder encoder; - - @Autowired - private Logger logger; - - @Autowired - private Contract contract; - - @Autowired(required = false) - private Logger.Level logLevel; - - @Autowired(required = false) - private Retryer retryer; - - @Autowired(required = false) - private ErrorDecoder errorDecoder; - - @Autowired(required = false) - private Request.Options options; - - @Autowired(required = false) - private Client ribbonClient; - - @Autowired(required = false) - private List requestInterceptors; + private ApplicationContext context; @Override public void afterPropertiesSet() throws Exception { @@ -91,40 +65,67 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean { } } + @Override + public void setApplicationContext(ApplicationContext context) throws BeansException { + this.context = context; + } + protected Feign.Builder feign() { + // @formatter:off Feign.Builder builder = Feign.builder() // required values - .logger(this.logger).encoder(this.encoder).decoder(this.decoder) - .contract(this.contract); + .logger(get(Logger.class)) + .encoder(get(Encoder.class)) + .decoder(get(Decoder.class)) + .contract(get(Contract.class)); + // @formatter:on // optional values - if (this.logLevel != null) { - builder.logLevel(this.logLevel); + Logger.Level level = getOptional(Logger.Level.class); + if (level != null) { + builder.logLevel(level); } - if (this.retryer != null) { - builder.retryer(this.retryer); + Retryer retryer = getOptional(Retryer.class); + if (retryer != null) { + builder.retryer(retryer); } - if (this.errorDecoder != null) { - builder.errorDecoder(this.errorDecoder); + ErrorDecoder errorDecoder = getOptional(ErrorDecoder.class); + if (errorDecoder != null) { + builder.errorDecoder(errorDecoder); } - if (this.options != null) { - builder.options(this.options); + Request.Options options = getOptional(Request.Options.class); + if (options != null) { + builder.options(options); } - if (this.requestInterceptors != null) { - builder.requestInterceptors(this.requestInterceptors); + Map requestInterceptors = this.context.getBeansOfType(RequestInterceptor.class); + if (requestInterceptors != null) { + builder.requestInterceptors(requestInterceptors.values()); } return builder; } + protected T get(Class type) { + return this.context.getBean(type); + } + + protected T getOptional(Class type) { + try { + return this.context.getBean(type); + } catch (NoSuchBeanDefinitionException e) { + //ignore + } + return null; + } + protected T loadBalance(Feign.Builder builder, Class type, String schemeName) { builder.logger(new Slf4jLogger(type)); // TODO: how to have choice here? - if (this.ribbonClient != null) { - return builder.client(this.ribbonClient).target(type, schemeName); - } - else { - return builder.target(LoadBalancingTarget.create(type, schemeName)); + Client client = getOptional(Client.class); + if (client != null) { + return builder.client(client).target(type, schemeName); } + + throw new IllegalStateException("No Feign Client for loadBalancing defined. Did you forget to include spring-cloud-starter-ribbon?"); } @Override diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientValidationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientValidationTests.java index 3f4f5872..3236552d 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientValidationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientValidationTests.java @@ -19,6 +19,8 @@ package org.springframework.cloud.netflix.feign.valid; import org.junit.Test; import org.springframework.cloud.netflix.feign.EnableFeignClients; import org.springframework.cloud.netflix.feign.FeignClient; +import org.springframework.cloud.netflix.feign.ribbon.FeignRibbonClientAutoConfiguration; +import org.springframework.cloud.netflix.ribbon.RibbonAutoConfiguration; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Configuration; import org.springframework.web.bind.annotation.RequestMapping; @@ -32,16 +34,39 @@ import static org.junit.Assert.assertNotNull; public class FeignClientValidationTests { @Test - public void valid() { + public void validNotLoadBalanced() { AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( - GoodConfiguration.class); - assertNotNull(context.getBean(GoodConfiguration.Client.class)); + GoodUrlConfiguration.class); + assertNotNull(context.getBean(GoodUrlConfiguration.Client.class)); context.close(); } @Configuration @EnableFeignClients - protected static class GoodConfiguration { + protected static class GoodUrlConfiguration { + + @FeignClient(url="http://example.com") + interface Client { + @RequestMapping(method = RequestMethod.GET, value = "/") + @Deprecated + String get(); + } + + } + + @Test + public void validLoadBalanced() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( + RibbonAutoConfiguration.class, + FeignRibbonClientAutoConfiguration.class, + GoodServiceIdConfiguration.class); + assertNotNull(context.getBean(GoodServiceIdConfiguration.Client.class)); + context.close(); + } + + @Configuration + @EnableFeignClients + protected static class GoodServiceIdConfiguration { @FeignClient("foo") interface Client {