Merge remote-tracking branch 'origin/2.2.x'

# Conflicts:
#	spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java
This commit is contained in:
Olga Maciaszek-Sharma
2020-07-13 17:23:49 +02:00
4 changed files with 152 additions and 87 deletions

View File

@@ -18,6 +18,7 @@ package org.springframework.cloud.openfeign;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.TimeUnit;
import feign.Client;
import feign.Contract;
@@ -52,6 +53,7 @@ import org.springframework.util.StringUtils;
* @author Eko Kurniawan Khannedy
* @author Gregor Zurowski
* @author Matt King
* @author Olga Maciaszek-Sharma
*/
public class FeignClientFactoryBean
implements FactoryBean<Object>, InitializingBean, ApplicationContextAware {
@@ -81,15 +83,19 @@ public class FeignClientFactoryBean
private Class<?> fallbackFactory = void.class;
private int readTimeoutMillis = new Request.Options().readTimeoutMillis();
private int connectTimeoutMillis = new Request.Options().connectTimeoutMillis();
@Override
public void afterPropertiesSet() {
Assert.hasText(this.contextId, "Context id must be set");
Assert.hasText(this.name, "Name must be set");
Assert.hasText(contextId, "Context id must be set");
Assert.hasText(name, "Name must be set");
}
protected Feign.Builder feign(FeignContext context) {
FeignLoggerFactory loggerFactory = get(context, FeignLoggerFactory.class);
Logger logger = loggerFactory.create(this.type);
Logger logger = loggerFactory.create(type);
// @formatter:off
Feign.Builder builder = get(context, Feign.Builder.class)
@@ -119,7 +125,7 @@ public class FeignClientFactoryBean
}
protected void configureFeign(FeignContext context, Feign.Builder builder) {
FeignClientProperties properties = this.applicationContext
FeignClientProperties properties = applicationContext
.getBean(FeignClientProperties.class);
FeignClientConfigurer feignClientConfigurer = getOptional(context,
@@ -132,15 +138,13 @@ public class FeignClientFactoryBean
configureUsingProperties(
properties.getConfig().get(properties.getDefaultConfig()),
builder);
configureUsingProperties(properties.getConfig().get(this.contextId),
builder);
configureUsingProperties(properties.getConfig().get(contextId), builder);
}
else {
configureUsingProperties(
properties.getConfig().get(properties.getDefaultConfig()),
builder);
configureUsingProperties(properties.getConfig().get(this.contextId),
builder);
configureUsingProperties(properties.getConfig().get(contextId), builder);
configureUsingConfiguration(context, builder);
}
}
@@ -168,7 +172,7 @@ public class FeignClientFactoryBean
FeignErrorDecoderFactory errorDecoderFactory = getOptional(context,
FeignErrorDecoderFactory.class);
if (errorDecoderFactory != null) {
ErrorDecoder factoryErrorDecoder = errorDecoderFactory.create(this.type);
ErrorDecoder factoryErrorDecoder = errorDecoderFactory.create(type);
builder.errorDecoder(factoryErrorDecoder);
}
}
@@ -176,6 +180,8 @@ public class FeignClientFactoryBean
Request.Options.class);
if (options != null) {
builder.options(options);
readTimeoutMillis = options.readTimeoutMillis();
connectTimeoutMillis = options.connectTimeoutMillis();
}
Map<String, RequestInterceptor> requestInterceptors = getInheritedAwareInstances(
context, RequestInterceptor.class);
@@ -187,7 +193,7 @@ public class FeignClientFactoryBean
if (queryMapEncoder != null) {
builder.queryMapEncoder(queryMapEncoder);
}
if (this.decode404) {
if (decode404) {
builder.decode404();
}
ExceptionPropagationPolicy exceptionPropagationPolicy = getInheritedAwareOptional(
@@ -208,10 +214,13 @@ public class FeignClientFactoryBean
builder.logLevel(config.getLoggerLevel());
}
if (config.getConnectTimeout() != null && config.getReadTimeout() != null) {
builder.options(new Request.Options(config.getConnectTimeout(),
config.getReadTimeout()));
}
connectTimeoutMillis = config.getConnectTimeout() != null
? config.getConnectTimeout() : connectTimeoutMillis;
readTimeoutMillis = config.getReadTimeout() != null ? config.getReadTimeout()
: readTimeoutMillis;
builder.options(new Request.Options(connectTimeoutMillis, TimeUnit.MILLISECONDS,
readTimeoutMillis, TimeUnit.MILLISECONDS, true));
if (config.getRetryer() != null) {
Retryer retryer = getOrInstantiate(config.getRetryer());
@@ -257,7 +266,7 @@ public class FeignClientFactoryBean
private <T> T getOrInstantiate(Class<T> tClass) {
try {
return this.applicationContext.getBean(tClass);
return applicationContext.getBean(tClass);
}
catch (NoSuchBeanDefinitionException e) {
return BeanUtils.instantiateClass(tClass);
@@ -265,16 +274,16 @@ public class FeignClientFactoryBean
}
protected <T> T get(FeignContext context, Class<T> type) {
T instance = context.getInstance(this.contextId, type);
T instance = context.getInstance(contextId, type);
if (instance == null) {
throw new IllegalStateException(
"No bean found of type " + type + " for " + this.contextId);
"No bean found of type " + type + " for " + contextId);
}
return instance;
}
protected <T> T getOptional(FeignContext context, Class<T> type) {
return context.getInstance(this.contextId, type);
return context.getInstance(contextId, type);
}
protected <T> T getInheritedAwareOptional(FeignContext context, Class<T> type) {
@@ -282,17 +291,17 @@ public class FeignClientFactoryBean
return getOptional(context, type);
}
else {
return context.getInstanceWithoutAncestors(this.contextId, type);
return context.getInstanceWithoutAncestors(contextId, type);
}
}
protected <T> Map<String, T> getInheritedAwareInstances(FeignContext context,
Class<T> type) {
if (inheritParentContext) {
return context.getInstances(this.contextId, type);
return context.getInstances(contextId, type);
}
else {
return context.getInstancesWithoutAncestors(this.contextId, type);
return context.getInstancesWithoutAncestors(contextId, type);
}
}
@@ -320,22 +329,22 @@ public class FeignClientFactoryBean
* information
*/
<T> T getTarget() {
FeignContext context = this.applicationContext.getBean(FeignContext.class);
FeignContext context = applicationContext.getBean(FeignContext.class);
Feign.Builder builder = feign(context);
if (!StringUtils.hasText(this.url)) {
if (!this.name.startsWith("http")) {
this.url = "http://" + this.name;
if (!StringUtils.hasText(url)) {
if (!name.startsWith("http")) {
url = "http://" + name;
}
else {
this.url = this.name;
url = name;
}
this.url += cleanPath();
url += cleanPath();
return (T) loadBalance(builder, context,
new HardCodedTarget<>(this.type, this.name, this.url));
new HardCodedTarget<>(type, name, url));
}
if (StringUtils.hasText(this.url) && !this.url.startsWith("http")) {
this.url = "http://" + this.url;
if (StringUtils.hasText(url) && !url.startsWith("http")) {
url = "http://" + url;
}
String url = this.url + cleanPath();
Client client = getOptional(context, Client.class);
@@ -349,7 +358,7 @@ public class FeignClientFactoryBean
}
Targeter targeter = get(context, Targeter.class);
return (T) targeter.target(this, builder, context,
new HardCodedTarget<>(this.type, this.name, url));
new HardCodedTarget<>(type, name, url));
}
private String cleanPath() {
@@ -367,7 +376,7 @@ public class FeignClientFactoryBean
@Override
public Class<?> getObjectType() {
return this.type;
return type;
}
@Override
@@ -376,7 +385,7 @@ public class FeignClientFactoryBean
}
public Class<?> getType() {
return this.type;
return type;
}
public void setType(Class<?> type) {
@@ -384,7 +393,7 @@ public class FeignClientFactoryBean
}
public String getName() {
return this.name;
return name;
}
public void setName(String name) {
@@ -392,7 +401,7 @@ public class FeignClientFactoryBean
}
public String getContextId() {
return this.contextId;
return contextId;
}
public void setContextId(String contextId) {
@@ -400,7 +409,7 @@ public class FeignClientFactoryBean
}
public String getUrl() {
return this.url;
return url;
}
public void setUrl(String url) {
@@ -408,7 +417,7 @@ public class FeignClientFactoryBean
}
public String getPath() {
return this.path;
return path;
}
public void setPath(String path) {
@@ -416,7 +425,7 @@ public class FeignClientFactoryBean
}
public boolean isDecode404() {
return this.decode404;
return decode404;
}
public void setDecode404(boolean decode404) {
@@ -432,7 +441,7 @@ public class FeignClientFactoryBean
}
public ApplicationContext getApplicationContext() {
return this.applicationContext;
return applicationContext;
}
@Override
@@ -441,7 +450,7 @@ public class FeignClientFactoryBean
}
public Class<?> getFallback() {
return this.fallback;
return fallback;
}
public void setFallback(Class<?> fallback) {
@@ -449,7 +458,7 @@ public class FeignClientFactoryBean
}
public Class<?> getFallbackFactory() {
return this.fallbackFactory;
return fallbackFactory;
}
public void setFallbackFactory(Class<?> fallbackFactory) {
@@ -465,35 +474,31 @@ public class FeignClientFactoryBean
return false;
}
FeignClientFactoryBean that = (FeignClientFactoryBean) o;
return Objects.equals(this.applicationContext, that.applicationContext)
&& this.decode404 == that.decode404
&& this.inheritParentContext == that.inheritParentContext
&& Objects.equals(this.fallback, that.fallback)
&& Objects.equals(this.fallbackFactory, that.fallbackFactory)
&& Objects.equals(this.name, that.name)
&& Objects.equals(this.path, that.path)
&& Objects.equals(this.type, that.type)
&& Objects.equals(this.url, that.url);
return Objects.equals(applicationContext, that.applicationContext)
&& decode404 == that.decode404
&& inheritParentContext == that.inheritParentContext
&& Objects.equals(fallback, that.fallback)
&& Objects.equals(fallbackFactory, that.fallbackFactory)
&& Objects.equals(name, that.name) && Objects.equals(path, that.path)
&& Objects.equals(type, that.type) && Objects.equals(url, that.url);
}
@Override
public int hashCode() {
return Objects.hash(this.applicationContext, this.decode404,
this.inheritParentContext, this.fallback, this.fallbackFactory, this.name,
this.path, this.type, this.url);
return Objects.hash(applicationContext, decode404, inheritParentContext, fallback,
fallbackFactory, name, path, type, url);
}
@Override
public String toString() {
return new StringBuilder("FeignClientFactoryBean{").append("type=")
.append(this.type).append(", ").append("name='").append(this.name)
.append("', ").append("url='").append(this.url).append("', ")
.append("path='").append(this.path).append("', ").append("decode404=")
.append(this.decode404).append(", ").append("inheritParentContext=")
.append(this.inheritParentContext).append(", ")
.append("applicationContext=").append(this.applicationContext)
.append(", ").append("fallback=").append(this.fallback).append(", ")
.append("fallbackFactory=").append(this.fallbackFactory).append("}")
return new StringBuilder("FeignClientFactoryBean{").append("type=").append(type)
.append(", ").append("name='").append(name).append("', ").append("url='")
.append(url).append("', ").append("path='").append(path).append("', ")
.append("decode404=").append(decode404).append(", ")
.append("inheritParentContext=").append(inheritParentContext).append(", ")
.append("applicationContext=").append(applicationContext).append(", ")
.append("fallback=").append(fallback).append(", ")
.append("fallbackFactory=").append(fallbackFactory).append("}")
.toString();
}

View File

@@ -17,13 +17,19 @@
package org.springframework.cloud.openfeign;
import java.io.IOException;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.lang.reflect.Type;
import java.net.SocketTimeoutException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Map;
import java.util.Objects;
import javax.servlet.http.HttpServletRequest;
import feign.InvocationHandlerFactory;
import feign.Request;
import feign.RequestInterceptor;
import feign.RequestTemplate;
import feign.RetryableException;
@@ -47,6 +53,8 @@ import org.springframework.http.MediaType;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.TestPropertySource;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.bind.annotation.RestController;
@@ -57,7 +65,9 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen
/**
* @author Eko Kurniawan Khannedy
* @author Olga Maciaszek-Sharma
*/
@SuppressWarnings("FieldMayBeFinal")
@RunWith(SpringJUnit4ClassRunner.class)
@SpringBootTest(classes = FeignClientUsingPropertiesTests.Application.class,
webEnvironment = RANDOM_PORT)
@@ -83,45 +93,45 @@ public class FeignClientUsingPropertiesTests {
private FeignClientFactoryBean formFactoryBean;
public FeignClientUsingPropertiesTests() {
this.fooFactoryBean = new FeignClientFactoryBean();
this.fooFactoryBean.setContextId("foo");
this.fooFactoryBean.setType(FeignClientFactoryBean.class);
fooFactoryBean = new FeignClientFactoryBean();
fooFactoryBean.setContextId("foo");
fooFactoryBean.setType(FeignClientFactoryBean.class);
this.barFactoryBean = new FeignClientFactoryBean();
this.barFactoryBean.setContextId("bar");
this.barFactoryBean.setType(FeignClientFactoryBean.class);
barFactoryBean = new FeignClientFactoryBean();
barFactoryBean.setContextId("bar");
barFactoryBean.setType(FeignClientFactoryBean.class);
this.unwrapFactoryBean = new FeignClientFactoryBean();
this.unwrapFactoryBean.setContextId("unwrap");
this.unwrapFactoryBean.setType(FeignClientFactoryBean.class);
unwrapFactoryBean = new FeignClientFactoryBean();
unwrapFactoryBean.setContextId("unwrap");
unwrapFactoryBean.setType(FeignClientFactoryBean.class);
this.formFactoryBean = new FeignClientFactoryBean();
this.formFactoryBean.setContextId("form");
this.formFactoryBean.setType(FeignClientFactoryBean.class);
formFactoryBean = new FeignClientFactoryBean();
formFactoryBean.setContextId("form");
formFactoryBean.setType(FeignClientFactoryBean.class);
}
public FooClient fooClient() {
this.fooFactoryBean.setApplicationContext(this.applicationContext);
return this.fooFactoryBean.feign(this.context).target(FooClient.class,
"http://localhost:" + this.port);
fooFactoryBean.setApplicationContext(applicationContext);
return fooFactoryBean.feign(context).target(FooClient.class,
"http://localhost:" + port);
}
public BarClient barClient() {
this.barFactoryBean.setApplicationContext(this.applicationContext);
return this.barFactoryBean.feign(this.context).target(BarClient.class,
"http://localhost:" + this.port);
barFactoryBean.setApplicationContext(applicationContext);
return barFactoryBean.feign(context).target(BarClient.class,
"http://localhost:" + port);
}
public UnwrapClient unwrapClient() {
this.unwrapFactoryBean.setApplicationContext(this.applicationContext);
return this.unwrapFactoryBean.feign(this.context).target(UnwrapClient.class,
"http://localhost:" + this.port);
unwrapFactoryBean.setApplicationContext(applicationContext);
return unwrapFactoryBean.feign(context).target(UnwrapClient.class,
"http://localhost:" + port);
}
public FormClient formClient() {
this.formFactoryBean.setApplicationContext(this.applicationContext);
return this.formFactoryBean.feign(this.context).target(FormClient.class,
"http://localhost:" + this.port);
formFactoryBean.setApplicationContext(applicationContext);
return formFactoryBean.feign(context).target(FormClient.class,
"http://localhost:" + port);
}
@Test
@@ -149,6 +159,47 @@ public class FeignClientUsingPropertiesTests {
assertThat(response).isEqualTo("Data");
}
@Test
public void readTimeoutShouldWorkWhenConnectTimeoutNotSet() {
FeignClientFactoryBean readTimeoutFactoryBean = new FeignClientFactoryBean();
readTimeoutFactoryBean.setContextId("readTimeout");
readTimeoutFactoryBean.setType(FeignClientFactoryBean.class);
readTimeoutFactoryBean.setApplicationContext(applicationContext);
TimeoutClient client = readTimeoutFactoryBean.feign(context)
.target(TimeoutClient.class, "http://localhost:" + port);
Request.Options options = getRequestOptions((Proxy) client);
assertThat(options.readTimeoutMillis()).isEqualTo(1000);
assertThat(options.connectTimeoutMillis()).isEqualTo(5000);
}
@Test
public void connectTimeoutShouldWorkWhenReadTimeoutNotSet() {
FeignClientFactoryBean readTimeoutFactoryBean = new FeignClientFactoryBean();
readTimeoutFactoryBean.setContextId("connectTimeout");
readTimeoutFactoryBean.setType(FeignClientFactoryBean.class);
readTimeoutFactoryBean.setApplicationContext(applicationContext);
TimeoutClient client = readTimeoutFactoryBean.feign(context)
.target(TimeoutClient.class, "http://localhost:" + port);
Request.Options options = getRequestOptions((Proxy) client);
assertThat(options.connectTimeoutMillis()).isEqualTo(1000);
assertThat(options.readTimeoutMillis()).isEqualTo(5000);
}
private Request.Options getRequestOptions(Proxy client) {
Object invocationHandler = ReflectionTestUtils.getField(client, "h");
Map<Method, InvocationHandlerFactory.MethodHandler> dispatch = (Map<Method, InvocationHandlerFactory.MethodHandler>) ReflectionTestUtils
.getField(Objects.requireNonNull(invocationHandler), "dispatch");
Method key = new ArrayList<>(dispatch.keySet()).get(0);
return (Request.Options) ReflectionTestUtils.getField(dispatch.get(key),
"options");
}
protected interface FooClient {
@RequestMapping(method = RequestMethod.GET, value = "/foo")
@@ -178,6 +229,13 @@ public class FeignClientUsingPropertiesTests {
}
protected interface TimeoutClient {
@GetMapping("/timeouts")
String timeouts();
}
@Configuration(proxyBeanMethods = false)
@EnableAutoConfiguration
@RestController

View File

@@ -429,7 +429,7 @@ public class SpringMvcContractTests {
assertThat(data.template().url()).isEqualTo("/test/{id}");
assertThat(data.template().method()).isEqualTo("GET");
assertThat(data.template().headers().get("X-Foo").iterator().next())
assertThat(data.template().headers().get("x-Foo").iterator().next())
.isEqualTo("bar");
}

View File

@@ -16,3 +16,5 @@ feign.client.config.form.encoder=org.springframework.cloud.openfeign.FeignClient
feign.client.config.unwrap.connectTimeout=1000
feign.client.config.unwrap.readTimeout=1000
feign.client.config.unwrap.exceptionPropagationPolicy=unwrap
feign.client.config.readTimeout.readTimeout=1000
feign.client.config.connectTimeout.connectTimeout=1000