From c2c634707e2e8ebe5770a13cc7bbc0b02d49e5e3 Mon Sep 17 00:00:00 2001 From: Eko Kurniawan Khannedy Date: Sat, 8 Jul 2017 02:31:56 +0700 Subject: [PATCH] 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 --- .../main/asciidoc/spring-cloud-netflix.adoc | 38 ++++ .../netflix/feign/FeignAutoConfiguration.java | 2 + .../netflix/feign/FeignClientFactoryBean.java | 76 +++++++- .../netflix/feign/FeignClientProperties.java | 174 ++++++++++++++++++ .../FeignClientUsingPropertiesTests.java | 163 ++++++++++++++++ .../resources/feign-properties.properties | 19 ++ 6 files changed, 468 insertions(+), 4 deletions(-) create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientProperties.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignClientUsingPropertiesTests.java create mode 100644 spring-cloud-netflix-core/src/test/resources/feign-properties.properties diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index dd81ce0d..bec3f6da 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/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. 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 c643df8a..15facdff 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 @@ -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) 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 48370e41..2c61e8dd 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 @@ -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, 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, 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, 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 bean : config.getRequestInterceptors()) { + RequestInterceptor interceptor = getOrInstantiate(bean); + builder.requestInterceptor(interceptor); + } + } + + if (config.getDecode404() != null) { + if (config.getDecode404()) { + builder.decode404(); + } + } + } + + private T getOrInstantiate(Class tClass) { + try { + return applicationContext.getBean(tClass); + } catch (NoSuchBeanDefinitionException e) { + return BeanUtils.instantiateClass(tClass); + } } protected T get(FeignContext context, Class type) { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientProperties.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientProperties.java new file mode 100644 index 00000000..bdb51958 --- /dev/null +++ b/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 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 getConfig() { + return config; + } + + public void setConfig(Map 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; + + private Class errorDecoder; + + private List> 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 getRetryer() { + return retryer; + } + + public void setRetryer(Class retryer) { + this.retryer = retryer; + } + + public Class getErrorDecoder() { + return errorDecoder; + } + + public void setErrorDecoder(Class errorDecoder) { + this.errorDecoder = errorDecoder; + } + + public List> getRequestInterceptors() { + return requestInterceptors; + } + + public void setRequestInterceptors(List> 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); + } + } + +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignClientUsingPropertiesTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignClientUsingPropertiesTests.java new file mode 100644 index 00000000..5822444d --- /dev/null +++ b/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 { + } + +} diff --git a/spring-cloud-netflix-core/src/test/resources/feign-properties.properties b/spring-cloud-netflix-core/src/test/resources/feign-properties.properties new file mode 100644 index 00000000..bc045d17 --- /dev/null +++ b/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 \ No newline at end of file