Merge pull request #513 from spring-cloud/feign-eager-instantiation-problem

Change from @Autowired to context.getBean

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<Object>, InitializingBean {
+class FeignClientFactoryBean implements FactoryBean<Object>, InitializingBean, ApplicationContextAware {
 
 	private Class<?> type;
 
@@ -53,35 +55,7 @@ class FeignClientFactoryBean implements FactoryBean<Object>, 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<RequestInterceptor> requestInterceptors;
+	private ApplicationContext context;
 
 	@Override
 	public void afterPropertiesSet() throws Exception {
@@ -91,40 +65,67 @@ class FeignClientFactoryBean implements FactoryBean<Object>, 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<String, RequestInterceptor> requestInterceptors = this.context.getBeansOfType(RequestInterceptor.class);
+		if (requestInterceptors != null) {
+			builder.requestInterceptors(requestInterceptors.values());
 		}
 
 		return builder;
 	}
 
+	protected <T> T get(Class<T> type) {
+		return this.context.getBean(type);
+	}
+
+	protected <T> T getOptional(Class<T> type) {
+		try {
+			return this.context.getBean(type);
+		} catch (NoSuchBeanDefinitionException e) {
+			//ignore
+		}
+		return null;
+	}
+
 	protected <T> T loadBalance(Feign.Builder builder, Class<T> 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

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 {