FeignClient Configuration Properties (#1942)

* add feign client properties

* configure feign builder from properties if exists

* change string config to class config and add unit test for feign client using configuration properties

* refactoring and add primary flag configuration

* add decode404 in feign client configuration properties

* change unit test properties decode404 to true

* remove lombok from FeignClientProperties and change primary attribute to default-to-properties

fixes gh-1931

docs/src/main/asciidoc/spring-cloud-netflix.adoc

@@ -1042,8 +1042,46 @@ public class FooConfiguration {
 
 This replaces the `SpringMvcContract` with `feign.Contract.Default` and adds a `RequestInterceptor` to the collection of `RequestInterceptor`.
 
+`@FeignClient` also can be configured using configuration properties.
+
+application.yml
+[source,yaml]
+----
+feign:
+  client:
+    config:
+      feignName:
+        connectTimeout: 5000
+        readTimeout: 5000
+        loggerLevel: full
+        errorDecoder: com.example.SimpleErrorDecoder
+        retryer: com.example.SimpleRetryer
+        requestInterceptors:
+          - com.example.FooRequestInterceptor
+          - com.example.BarRequestInterceptor
+        decode404: false
+----
+
 Default configurations can be specified in the `@EnableFeignClients` attribute `defaultConfiguration` in a similar manner as described above. The difference is that this configuration will apply to _all_ feign clients.
 
+If you prefer using configuration properties to configured all `@FeignClient`, you can create configuration properties with `default` feign name.
+
+application.yml
+[source,yaml]
+----
+feign:
+  client:
+    config:
+      default:
+        connectTimeout: 5000
+        readTimeout: 5000
+        loggerLevel: basic
+----
+
+If we create both `@Configuration` bean and configuration properties, configuration properties will win.
+It will override `@Configuration` values. But if you want to change the priority to `@Configuration`,
+you can change `feign.client.default-to-properties` to `false`.
+
 NOTE: If you need to use `ThreadLocal` bound variables in your `RequestInterceptor`s you will need to either set the
 thread isolation strategy for Hystrix to `SEMAPHORE` or disable Hystrix in Feign.
 

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

@@ -25,6 +25,7 @@ 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.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.cloud.client.actuator.HasFeatures;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -40,6 +41,7 @@ import feign.okhttp.OkHttpClient;
  */
 @Configuration
 @ConditionalOnClass(Feign.class)
+@EnableConfigurationProperties({FeignClientProperties.class})
 public class FeignAutoConfiguration {
 
 	@Autowired(required = false)

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2016 the original author or authors.
+ * Copyright 2013-2017 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.
@@ -18,9 +18,11 @@ package org.springframework.cloud.netflix.feign;
 
 import java.util.Map;
 
+import org.springframework.beans.BeanUtils;
 import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.FactoryBean;
 import org.springframework.beans.factory.InitializingBean;
+import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.cloud.netflix.feign.ribbon.LoadBalancerFeignClient;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
@@ -44,6 +46,7 @@ import lombok.EqualsAndHashCode;
 /**
  * @author Spencer Gibb
  * @author Venil Noronha
+ * @author Eko Kurniawan Khannedy
  */
 @Data
 @EqualsAndHashCode(callSuper = false)
@@ -74,7 +77,6 @@ class FeignClientFactoryBean implements FactoryBean<Object>, InitializingBean,
 		Assert.hasText(this.name, "Name must be set");
 	}
 
-
 	@Override
 	public void setApplicationContext(ApplicationContext context) throws BeansException {
 		this.applicationContext = context;
@@ -93,7 +95,29 @@ class FeignClientFactoryBean implements FactoryBean<Object>, InitializingBean,
 				.contract(get(context, Contract.class));
 		// @formatter:on
 
-		// optional values
+		configureFeign(context, builder);
+
+		return builder;
+	}
+
+	protected void configureFeign(FeignContext context, Feign.Builder builder) {
+		FeignClientProperties properties = applicationContext.getBean(FeignClientProperties.class);
+		if (properties != null) {
+			if (properties.isDefaultToProperties()) {
+				configureUsingConfiguration(context, builder);
+				configureUsingProperties(properties.getConfig().get(properties.getDefaultConfig()), builder);
+				configureUsingProperties(properties.getConfig().get(this.name), builder);
+			} else {
+				configureUsingProperties(properties.getConfig().get(properties.getDefaultConfig()), builder);
+				configureUsingProperties(properties.getConfig().get(this.name), builder);
+				configureUsingConfiguration(context, builder);
+			}
+		} else {
+			configureUsingConfiguration(context, builder);
+		}
+	}
+
+	protected void configureUsingConfiguration(FeignContext context, Feign.Builder builder) {
 		Logger.Level level = getOptional(context, Logger.Level.class);
 		if (level != null) {
 			builder.logLevel(level);
@@ -119,8 +143,52 @@ class FeignClientFactoryBean implements FactoryBean<Object>, InitializingBean,
 		if (decode404) {
 			builder.decode404();
 		}
+	}
 
-		return builder;
+	protected void configureUsingProperties(FeignClientProperties.FeignClientConfiguration config, Feign.Builder builder) {
+		if (config == null) {
+			return;
+		}
+
+		if (config.getLoggerLevel() != null) {
+			builder.logLevel(config.getLoggerLevel());
+		}
+
+		if (config.getConnectTimeout() != null && config.getReadTimeout() != null) {
+			builder.options(new Request.Options(config.getConnectTimeout(), config.getReadTimeout()));
+		}
+
+		if (config.getRetryer() != null) {
+			Retryer retryer = getOrInstantiate(config.getRetryer());
+			builder.retryer(retryer);
+		}
+
+		if (config.getErrorDecoder() != null) {
+			ErrorDecoder errorDecoder = getOrInstantiate(config.getErrorDecoder());
+			builder.errorDecoder(errorDecoder);
+		}
+
+		if (config.getRequestInterceptors() != null && !config.getRequestInterceptors().isEmpty()) {
+			// this will add request interceptor to builder, not replace existing
+			for (Class<RequestInterceptor> bean : config.getRequestInterceptors()) {
+				RequestInterceptor interceptor = getOrInstantiate(bean);
+				builder.requestInterceptor(interceptor);
+			}
+		}
+
+		if (config.getDecode404() != null) {
+			if (config.getDecode404()) {
+				builder.decode404();
+			}
+		}
+	}
+
+	private <T> T getOrInstantiate(Class<T> tClass) {
+		try {
+			return applicationContext.getBean(tClass);
+		} catch (NoSuchBeanDefinitionException e) {
+			return BeanUtils.instantiateClass(tClass);
+		}
 	}
 
 	protected <T> T get(FeignContext context, Class<T> type) {

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

@@ -0,0 +1,174 @@
+/*
+ * Copyright 2013-2017 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.Logger;
+import feign.RequestInterceptor;
+import feign.Retryer;
+import feign.codec.ErrorDecoder;
+import lombok.Data;
+import org.springframework.boot.context.properties.ConfigurationProperties;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+/**
+ * @author Eko Kurniawan Khannedy
+ */
+@ConfigurationProperties("feign.client")
+public class FeignClientProperties {
+
+	private boolean defaultToProperties = true;
+
+	private String defaultConfig = "default";
+
+	private Map<String, FeignClientConfiguration> config = new HashMap<>();
+
+	public boolean isDefaultToProperties() {
+		return defaultToProperties;
+	}
+
+	public void setDefaultToProperties(boolean defaultToProperties) {
+		this.defaultToProperties = defaultToProperties;
+	}
+
+	public String getDefaultConfig() {
+		return defaultConfig;
+	}
+
+	public void setDefaultConfig(String defaultConfig) {
+		this.defaultConfig = defaultConfig;
+	}
+
+	public Map<String, FeignClientConfiguration> getConfig() {
+		return config;
+	}
+
+	public void setConfig(Map<String, FeignClientConfiguration> config) {
+		this.config = config;
+	}
+
+	@Override
+	public boolean equals(Object o) {
+		if (this == o) return true;
+		if (o == null || getClass() != o.getClass()) return false;
+		FeignClientProperties that = (FeignClientProperties) o;
+		return defaultToProperties == that.defaultToProperties &&
+				Objects.equals(defaultConfig, that.defaultConfig) &&
+				Objects.equals(config, that.config);
+	}
+
+	@Override
+	public int hashCode() {
+		return Objects.hash(defaultToProperties, defaultConfig, config);
+	}
+
+	public static class FeignClientConfiguration {
+
+		private Logger.Level loggerLevel;
+
+		private Integer connectTimeout;
+
+		private Integer readTimeout;
+
+		private Class<Retryer> retryer;
+
+		private Class<ErrorDecoder> errorDecoder;
+
+		private List<Class<RequestInterceptor>> requestInterceptors;
+
+		private Boolean decode404;
+
+		public Logger.Level getLoggerLevel() {
+			return loggerLevel;
+		}
+
+		public void setLoggerLevel(Logger.Level loggerLevel) {
+			this.loggerLevel = loggerLevel;
+		}
+
+		public Integer getConnectTimeout() {
+			return connectTimeout;
+		}
+
+		public void setConnectTimeout(Integer connectTimeout) {
+			this.connectTimeout = connectTimeout;
+		}
+
+		public Integer getReadTimeout() {
+			return readTimeout;
+		}
+
+		public void setReadTimeout(Integer readTimeout) {
+			this.readTimeout = readTimeout;
+		}
+
+		public Class<Retryer> getRetryer() {
+			return retryer;
+		}
+
+		public void setRetryer(Class<Retryer> retryer) {
+			this.retryer = retryer;
+		}
+
+		public Class<ErrorDecoder> getErrorDecoder() {
+			return errorDecoder;
+		}
+
+		public void setErrorDecoder(Class<ErrorDecoder> errorDecoder) {
+			this.errorDecoder = errorDecoder;
+		}
+
+		public List<Class<RequestInterceptor>> getRequestInterceptors() {
+			return requestInterceptors;
+		}
+
+		public void setRequestInterceptors(List<Class<RequestInterceptor>> requestInterceptors) {
+			this.requestInterceptors = requestInterceptors;
+		}
+
+		public Boolean getDecode404() {
+			return decode404;
+		}
+
+		public void setDecode404(Boolean decode404) {
+			this.decode404 = decode404;
+		}
+
+		@Override
+		public boolean equals(Object o) {
+			if (this == o) return true;
+			if (o == null || getClass() != o.getClass()) return false;
+			FeignClientConfiguration that = (FeignClientConfiguration) o;
+			return loggerLevel == that.loggerLevel &&
+					Objects.equals(connectTimeout, that.connectTimeout) &&
+					Objects.equals(readTimeout, that.readTimeout) &&
+					Objects.equals(retryer, that.retryer) &&
+					Objects.equals(errorDecoder, that.errorDecoder) &&
+					Objects.equals(requestInterceptors, that.requestInterceptors) &&
+					Objects.equals(decode404, that.decode404);
+		}
+
+		@Override
+		public int hashCode() {
+			return Objects.hash(loggerLevel, connectTimeout, readTimeout, retryer,
+					errorDecoder, requestInterceptors, decode404);
+		}
+	}
+
+}

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

@@ -0,0 +1,163 @@
+/*
+ * 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;
+
+import feign.RequestInterceptor;
+import feign.RequestTemplate;
+import feign.RetryableException;
+import feign.Retryer;
+import feign.codec.ErrorDecoder;
+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.test.context.SpringBootTest;
+import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.test.annotation.DirtiesContext;
+import org.springframework.test.context.TestPropertySource;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.servlet.http.HttpServletRequest;
+
+import static org.junit.Assert.*;
+
+/**
+ * @author Eko Kurniawan Khannedy
+ */
+@RunWith(SpringJUnit4ClassRunner.class)
+@SpringBootTest(classes = FeignClientUsingPropertiesTests.Application.class, webEnvironment = WebEnvironment.RANDOM_PORT)
+@TestPropertySource("classpath:feign-properties.properties")
+@DirtiesContext
+public class FeignClientUsingPropertiesTests {
+
+	@Autowired
+	FeignContext context;
+
+	@Autowired
+	private ApplicationContext applicationContext;
+
+	@Value("${local.server.port}")
+	private int port = 0;
+
+	private FeignClientFactoryBean fooFactoryBean;
+
+	private FeignClientFactoryBean barFactoryBean;
+
+	public FeignClientUsingPropertiesTests() {
+		fooFactoryBean = new FeignClientFactoryBean();
+		fooFactoryBean.setName("foo");
+		fooFactoryBean.setType(FeignClientFactoryBean.class);
+
+		barFactoryBean = new FeignClientFactoryBean();
+		barFactoryBean.setName("bar");
+		barFactoryBean.setType(FeignClientFactoryBean.class);
+	}
+
+	public FooClient fooClient() {
+		fooFactoryBean.setApplicationContext(applicationContext);
+		return fooFactoryBean.feign(context).target(FooClient.class, "http://localhost:" + this.port);
+	}
+
+	public BarClient barClient() {
+		barFactoryBean.setApplicationContext(applicationContext);
+		return barFactoryBean.feign(context).target(BarClient.class, "http://localhost:" + this.port);
+	}
+
+	@Test
+	public void testFoo() {
+		String response = fooClient().foo();
+		assertNotNull("OK", response);
+	}
+
+	@Test(expected = RetryableException.class)
+	public void testBar() {
+		barClient().bar();
+		fail("it should timeout");
+	}
+
+	protected interface FooClient {
+
+		@RequestMapping(method = RequestMethod.GET, value = "/foo")
+		String foo();
+	}
+
+	protected interface BarClient {
+
+		@RequestMapping(method = RequestMethod.GET, value = "/bar")
+		String bar();
+	}
+
+	@Configuration
+	@EnableAutoConfiguration
+	@RestController
+	protected static class Application {
+
+		@RequestMapping(method = RequestMethod.GET, value = "/foo")
+		public String foo(HttpServletRequest request) throws IllegalAccessException {
+			if ("Foo".equals(request.getHeader("Foo")) &&
+					"Bar".equals(request.getHeader("Bar"))) {
+				return "OK";
+			} else {
+				throw new IllegalAccessException("It should has Foo and Bar header");
+			}
+		}
+
+		@RequestMapping(method = RequestMethod.GET, value = "/bar")
+		public String bar() throws InterruptedException {
+			Thread.sleep(2000L);
+			return "OK";
+		}
+
+	}
+
+	public static class FooRequestInterceptor implements RequestInterceptor {
+		@Override
+		public void apply(RequestTemplate template) {
+			template.header("Foo", "Foo");
+		}
+	}
+
+	public static class BarRequestInterceptor implements RequestInterceptor {
+		@Override
+		public void apply(RequestTemplate template) {
+			template.header("Bar", "Bar");
+		}
+	}
+
+	public static class NoRetryer implements Retryer {
+
+		@Override
+		public void continueOrPropagate(RetryableException e) {
+			throw e;
+		}
+
+		@Override
+		public Retryer clone() {
+			return this;
+		}
+	}
+
+	public static class DefaultErrorDecoder extends ErrorDecoder.Default {
+	}
+
+}

spring-cloud-netflix-core/src/test/resources/feign-properties.properties

@@ -0,0 +1,19 @@
+# This configuration used by test class org.springframework.cloud.netflix.feign.FeignClientUsingPropertiesTests
+
+logging.level.org.springframework.cloud.netflix.feign=debug
+
+feign.client.default-to-properties=true
+feign.client.default-config=default
+
+feign.client.config.default.connectTimeout=5000
+feign.client.config.default.readTimeout=5000
+feign.client.config.default.loggerLevel=full
+feign.client.config.default.errorDecoder=org.springframework.cloud.netflix.feign.FeignClientUsingPropertiesTests.DefaultErrorDecoder
+feign.client.config.default.retryer=org.springframework.cloud.netflix.feign.FeignClientUsingPropertiesTests.NoRetryer
+feign.client.config.default.decode404=true
+
+feign.client.config.foo.requestInterceptors[0]=org.springframework.cloud.netflix.feign.FeignClientUsingPropertiesTests.FooRequestInterceptor
+feign.client.config.foo.requestInterceptors[1]=org.springframework.cloud.netflix.feign.FeignClientUsingPropertiesTests.BarRequestInterceptor
+
+feign.client.config.bar.connectTimeout=1000
+feign.client.config.bar.readTimeout=1000