diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsConfiguration.java index 606cbbb522..e37ca43049 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsConfiguration.java @@ -16,9 +16,16 @@ package org.springframework.cloud.netflix.feign; +import feign.Contract; +import feign.Feign; +import feign.Logger; +import feign.Retryer; +import feign.codec.Decoder; +import feign.codec.Encoder; +import feign.hystrix.HystrixFeign; + import java.util.ArrayList; import java.util.List; - import org.springframework.beans.factory.ObjectFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; @@ -35,16 +42,8 @@ import org.springframework.core.convert.ConversionService; import org.springframework.format.support.DefaultFormattingConversionService; import org.springframework.format.support.FormattingConversionService; - import com.netflix.hystrix.HystrixCommand; -import feign.Contract; -import feign.Feign; -import feign.Logger; -import feign.codec.Decoder; -import feign.codec.Encoder; -import feign.hystrix.HystrixFeign; - /** * @author Dave Syer * @author Venil Noronha @@ -103,11 +102,17 @@ public Feign.Builder feignHystrixBuilder() { } } + @Bean + @ConditionalOnMissingBean + public Retryer feignRetryer() { + return Retryer.NEVER_RETRY; + } + @Bean @Scope("prototype") @ConditionalOnMissingBean - public Feign.Builder feignBuilder() { - return Feign.builder(); + public Feign.Builder feignBuilder(Retryer retryer) { + return Feign.builder().retryer(retryer); } @Bean diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactory.java index 295f507ad2..05b18dab76 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactory.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactory.java @@ -18,8 +18,11 @@ import java.util.Map; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; +import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; +import org.springframework.retry.support.RetryTemplate; import org.springframework.util.ConcurrentReferenceHashMap; import com.netflix.client.config.IClientConfig; @@ -34,11 +37,19 @@ public class CachingSpringLoadBalancerFactory { private final SpringClientFactory factory; + private final LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; private volatile Map cache = new ConcurrentReferenceHashMap<>(); public CachingSpringLoadBalancerFactory(SpringClientFactory factory) { this.factory = factory; + this.loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(factory); + } + + public CachingSpringLoadBalancerFactory(SpringClientFactory factory, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { + this.factory = factory; + this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; } public FeignLoadBalancer create(String clientName) { @@ -48,7 +59,8 @@ public FeignLoadBalancer create(String clientName) { IClientConfig config = this.factory.getClientConfig(clientName); ILoadBalancer lb = this.factory.getLoadBalancer(clientName); ServerIntrospector serverIntrospector = this.factory.getInstance(clientName, ServerIntrospector.class); - FeignLoadBalancer client = new FeignLoadBalancer(lb, config, serverIntrospector); + FeignLoadBalancer client = new FeignLoadBalancer(lb, config, serverIntrospector, + loadBalancedRetryPolicyFactory); this.cache.put(clientName, client); return client; } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancer.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancer.java index 1c3ed45a7b..7804b373af 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancer.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancer.java @@ -16,44 +16,61 @@ package org.springframework.cloud.netflix.feign.ribbon; +import feign.Client; +import feign.Request; +import feign.Response; +import feign.Util; + import java.io.IOException; import java.net.URI; +import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; - +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; +import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser; +import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClient; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; - +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpRequest; +import org.springframework.retry.RetryCallback; +import org.springframework.retry.RetryContext; +import org.springframework.retry.policy.NeverRetryPolicy; +import org.springframework.retry.support.RetryTemplate; import com.netflix.client.AbstractLoadBalancerAwareClient; import com.netflix.client.ClientException; import com.netflix.client.ClientRequest; +import com.netflix.client.DefaultLoadBalancerRetryHandler; import com.netflix.client.IResponse; import com.netflix.client.RequestSpecificRetryHandler; -import com.netflix.client.RetryHandler; import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.IClientConfig; import com.netflix.loadbalancer.ILoadBalancer; import com.netflix.loadbalancer.Server; -import feign.Client; -import feign.Request; -import feign.Response; -import feign.Util; - import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttpsIfNeeded; public class FeignLoadBalancer extends - AbstractLoadBalancerAwareClient { + AbstractLoadBalancerAwareClient implements + ServiceInstanceChooser { private final int connectTimeout; private final int readTimeout; private final IClientConfig clientConfig; private final ServerIntrospector serverIntrospector; + private final LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; public FeignLoadBalancer(ILoadBalancer lb, IClientConfig clientConfig, - ServerIntrospector serverIntrospector) { + ServerIntrospector serverIntrospector, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { super(lb, clientConfig); - this.setRetryHandler(RetryHandler.DEFAULT); + this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; + this.setRetryHandler(new DefaultLoadBalancerRetryHandler(clientConfig)); this.clientConfig = clientConfig; this.connectTimeout = clientConfig.get(CommonClientConfigKey.ConnectTimeout); this.readTimeout = clientConfig.get(CommonClientConfigKey.ReadTimeout); @@ -61,9 +78,9 @@ public FeignLoadBalancer(ILoadBalancer lb, IClientConfig clientConfig, } @Override - public RibbonResponse execute(RibbonRequest request, IClientConfig configOverride) + public RibbonResponse execute(final RibbonRequest request, IClientConfig configOverride) throws IOException { - Request.Options options; + final Request.Options options; if (configOverride != null) { options = new Request.Options( configOverride.get(CommonClientConfigKey.ConnectTimeout, @@ -74,26 +91,35 @@ public RibbonResponse execute(RibbonRequest request, IClientConfig configOverrid else { options = new Request.Options(this.connectTimeout, this.readTimeout); } - Response response = request.client().execute(request.toRequest(), options); - return new RibbonResponse(request.getUri(), response); + LoadBalancedRetryPolicy retryPolicy = loadBalancedRetryPolicyFactory.create(this.getClientName(), this); + RetryTemplate retryTemplate = new RetryTemplate(); + retryTemplate.setRetryPolicy(retryPolicy == null ? new NeverRetryPolicy() + : new FeignRetryPolicy(request.toHttpRequest(), retryPolicy, this, this.getClientName())); + return retryTemplate.execute(new RetryCallback() { + @Override + public RibbonResponse doWithRetry(RetryContext retryContext) throws IOException { + Request feignRequest = null; + //on retries the policy will choose the server and set it in the context + //extract the server and update the request being made + if(retryContext instanceof LoadBalancedRetryContext) { + ServiceInstance service = ((LoadBalancedRetryContext)retryContext).getServiceInstance(); + if(service != null) { + feignRequest = ((RibbonRequest)request.replaceUri(reconstructURIWithServer(new Server(service.getHost(), service.getPort()), request.getUri()))).toRequest(); + } + } + if(feignRequest == null) { + feignRequest = request.toRequest(); + } + Response response = request.client().execute(feignRequest, options); + return new RibbonResponse(request.getUri(), response); + } + }); } @Override public RequestSpecificRetryHandler getRequestSpecificRetryHandler( RibbonRequest request, IClientConfig requestConfig) { - if (this.clientConfig.get(CommonClientConfigKey.OkToRetryOnAllOperations, - false)) { - return new RequestSpecificRetryHandler(true, true, this.getRetryHandler(), - requestConfig); - } - if (!request.toRequest().method().equals("GET")) { - return new RequestSpecificRetryHandler(true, false, this.getRetryHandler(), - requestConfig); - } - else { - return new RequestSpecificRetryHandler(true, true, this.getRetryHandler(), - requestConfig); - } + return new RequestSpecificRetryHandler(false, false, this.getRetryHandler(), requestConfig); } @Override @@ -102,6 +128,12 @@ public URI reconstructURIWithServer(Server server, URI original) { return super.reconstructURIWithServer(server, uri); } + @Override + public ServiceInstance choose(String serviceId) { + return new RibbonLoadBalancerClient.RibbonServer(serviceId, + this.getLoadBalancer().chooseServer(serviceId)); + } + static class RibbonRequest extends ClientRequest implements Cloneable { private final Request request; @@ -129,6 +161,33 @@ Client client() { return this.client; } + HttpRequest toHttpRequest() { + return new HttpRequest() { + @Override + public HttpMethod getMethod() { + return HttpMethod.resolve(RibbonRequest.this.toRequest().method()); + } + + @Override + public URI getURI() { + return RibbonRequest.this.getUri(); + } + + @Override + public HttpHeaders getHeaders() { + Map> headers = new HashMap>(); + Map> feignHeaders = RibbonRequest.this.toRequest().headers(); + for(String key : feignHeaders.keySet()) { + headers.put(key, new ArrayList(feignHeaders.get(key))); + } + HttpHeaders httpHeaders = new HttpHeaders(); + httpHeaders.putAll(headers); + return httpHeaders; + + } + }; + } + @Override public Object clone() { return new RibbonRequest(this.client, this.request, getUri()); diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRetryPolicy.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRetryPolicy.java new file mode 100644 index 0000000000..349f94fbc0 --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRetryPolicy.java @@ -0,0 +1,119 @@ +/* + * + * * Copyright 2013-2016 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.ribbon; + +import java.net.URI; +import java.util.HashMap; +import java.util.Map; + +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.InterceptorRetryPolicy; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; +import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser; +import org.springframework.http.HttpRequest; +import org.springframework.retry.RetryContext; + +/** + * @author Ryan Baxter + */ +public class FeignRetryPolicy extends InterceptorRetryPolicy { + private HttpRequest request; + private String serviceId; + public FeignRetryPolicy(HttpRequest request, LoadBalancedRetryPolicy policy, ServiceInstanceChooser serviceInstanceChooser, String serviceName) { + super(request, policy, serviceInstanceChooser, serviceName); + this.request = request; + this.serviceId = serviceName; + } + + @Override + public boolean canRetry(RetryContext context) { + /* + * In InterceptorRetryPolicy.canRetry we ask the LoadBalancer to choose a server if one is not + * set in the retry context and then return true. RetryTemplat calls the canRetry method of + * the policy even on its first execution. So the fact that we didnt have a service instance set + * in the RetryContext signaled that it was the first execution and we should return true. + * + * In the Feign scenario, Feign as actually already queried the load balancer for a service instance + * and we set that service instance in the context when we call the open method of the policy. So in + * the Feign case we just return true if the retry count is 0 indicating we haven't yet made a failed + * request. + */ + if(context.getRetryCount() == 0) { + return true; + } + return super.canRetry(context); + } + + @Override + public RetryContext open(RetryContext parent) { + /* + * With Feign (unlike Ribbon) the request already has the URI for the service instance + * we are going to make the request to, so extract that information and set the service + * instance in the context. In the Ribbon scenario the URI in the request object still has + * the service id so we choose and set the service instance later on. + */ + LoadBalancedRetryContext context = new LoadBalancedRetryContext(parent, this.request); + context.setServiceInstance(new FeignRetryPolicyServiceInstance(serviceId, request)); + return context; + } + + class FeignRetryPolicyServiceInstance implements ServiceInstance { + + private String serviceId; + private HttpRequest request; + private Map metadata; + + FeignRetryPolicyServiceInstance(String serviceId, HttpRequest request) { + this.serviceId = serviceId; + this.request = request; + this.metadata = new HashMap(); + } + + @Override + public String getServiceId() { + return serviceId; + } + + @Override + public String getHost() { + return request.getURI().getHost(); + } + + @Override + public int getPort() { + return request.getURI().getPort(); + } + + @Override + public boolean isSecure() { + return "https".equals(request.getURI().getScheme()); + } + + @Override + public URI getUri() { + return request.getURI(); + } + + @Override + public Map getMetadata() { + return metadata; + } + } +} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java index d1d8c821ba..f6cfcaea27 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java @@ -22,11 +22,13 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.feign.FeignAutoConfiguration; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Primary; +import org.springframework.retry.support.RetryTemplate; import com.netflix.loadbalancer.ILoadBalancer; @@ -50,8 +52,15 @@ public class FeignRibbonClientAutoConfiguration { @Bean @Primary public CachingSpringLoadBalancerFactory cachingLBClientFactory( - SpringClientFactory factory) { - return new CachingSpringLoadBalancerFactory(factory); + SpringClientFactory factory, LoadBalancedRetryPolicyFactory retryPolicyFactory) { + return new CachingSpringLoadBalancerFactory(factory, retryPolicyFactory); + } + + @Bean + public RetryTemplate retryTemplate() { + RetryTemplate template = new RetryTemplate(); + template.setThrowLastExceptionOnExhausted(true); + return template; } @Bean diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java index 414e867454..000e755741 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java @@ -141,17 +141,17 @@ protected ILoadBalancer getLoadBalancer(String serviceId) { return this.clientFactory.getLoadBalancer(serviceId); } - protected static class RibbonServer implements ServiceInstance { + public static class RibbonServer implements ServiceInstance { private final String serviceId; private final Server server; private final boolean secure; private Map metadata; - protected RibbonServer(String serviceId, Server server) { + public RibbonServer(String serviceId, Server server) { this(serviceId, server, false, Collections. emptyMap()); } - protected RibbonServer(String serviceId, Server server, boolean secure, + public RibbonServer(String serviceId, Server server, boolean secure, Map metadata) { this.serviceId = serviceId; this.server = server; diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java index b623652a53..86ce576852 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java @@ -27,6 +27,8 @@ import org.springframework.cloud.netflix.ribbon.support.AbstractLoadBalancingClient; import org.springframework.web.util.UriComponentsBuilder; +import com.netflix.client.RequestSpecificRetryHandler; +import com.netflix.client.RetryHandler; import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.IClientConfig; import com.netflix.loadbalancer.ILoadBalancer; @@ -108,4 +110,9 @@ public URI reconstructURIWithServer(Server server, URI original) { return super.reconstructURIWithServer(server, uri); } + @Override + public RequestSpecificRetryHandler getRequestSpecificRetryHandler(RibbonApacheHttpRequest request, IClientConfig requestConfig) { + return new RequestSpecificRetryHandler(false, false, RetryHandler.DEFAULT, null); + } + } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java index 59c37dab02..1824794630 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java @@ -16,6 +16,10 @@ package org.springframework.cloud.netflix.zuul.filters; +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; + import java.util.Arrays; import java.util.Collections; import java.util.LinkedHashMap; @@ -24,19 +28,12 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; - import javax.annotation.PostConstruct; - import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; - import com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy; -import lombok.AllArgsConstructor; -import lombok.Data; -import lombok.NoArgsConstructor; - import static com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy.SEMAPHORE; /** diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignClientOverrideDefaultsTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignClientOverrideDefaultsTests.java index bbd84a5e66..27211ed849 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignClientOverrideDefaultsTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignClientOverrideDefaultsTests.java @@ -108,7 +108,7 @@ public void overrideLoggerLevel() { @Test public void overrideRetryer() { - assertNull(this.context.getInstance("foo", Retryer.class)); + assertEquals(Retryer.NEVER_RETRY, this.context.getInstance("foo", Retryer.class)); Retryer.Default.class.cast(this.context.getInstance("bar", Retryer.class)); } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactoryTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactoryTests.java index 5594f0b4ce..31ab020f31 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactoryTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactoryTests.java @@ -16,21 +16,23 @@ package org.springframework.cloud.netflix.feign.ribbon; -import static org.junit.Assert.assertNotNull; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - import org.junit.Before; import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; +import org.springframework.retry.support.RetryTemplate; import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.DefaultClientConfigImpl; import com.netflix.client.config.IClientConfig; +import static org.junit.Assert.assertNotNull; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + /** * @author Spencer Gibb */ @@ -39,6 +41,9 @@ public class CachingSpringLoadBalancerFactoryTests { @Mock private SpringClientFactory delegate; + @Mock + private RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; + private CachingSpringLoadBalancerFactory factory; @Before @@ -52,7 +57,7 @@ public void init() { when(this.delegate.getClientConfig("client1")).thenReturn(config); when(this.delegate.getClientConfig("client2")).thenReturn(config); - this.factory = new CachingSpringLoadBalancerFactory(this.delegate); + this.factory = new CachingSpringLoadBalancerFactory(this.delegate, loadBalancedRetryPolicyFactory); } @Test diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancerTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancerTests.java index e8d52c12b6..4be7dce97a 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancerTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancerTests.java @@ -1,26 +1,17 @@ package org.springframework.cloud.netflix.feign.ribbon; -import static com.netflix.client.config.CommonClientConfigKey.ConnectTimeout; -import static com.netflix.client.config.CommonClientConfigKey.IsSecure; -import static com.netflix.client.config.CommonClientConfigKey.MaxAutoRetries; -import static com.netflix.client.config.CommonClientConfigKey.MaxAutoRetriesNextServer; -import static com.netflix.client.config.CommonClientConfigKey.OkToRetryOnAllOperations; -import static com.netflix.client.config.CommonClientConfigKey.ReadTimeout; -import static com.netflix.client.config.DefaultClientConfigImpl.DEFAULT_MAX_AUTO_RETRIES; -import static com.netflix.client.config.DefaultClientConfigImpl.DEFAULT_MAX_AUTO_RETRIES_NEXT_SERVER; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertThat; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyBoolean; -import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.when; +import feign.Client; +import feign.Request; +import feign.Request.Options; +import feign.RequestTemplate; +import feign.Response; +import lombok.SneakyThrows; import java.net.URI; import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.Map; - import org.junit.Before; import org.junit.Test; import org.mockito.Mock; @@ -29,18 +20,27 @@ import org.springframework.cloud.netflix.feign.ribbon.FeignLoadBalancer.RibbonRequest; import org.springframework.cloud.netflix.feign.ribbon.FeignLoadBalancer.RibbonResponse; import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; +import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; - +import org.springframework.retry.support.RetryTemplate; import com.netflix.client.config.IClientConfig; import com.netflix.loadbalancer.ILoadBalancer; import com.netflix.loadbalancer.Server; -import feign.Client; -import feign.Request; -import feign.Request.Options; -import feign.RequestTemplate; -import feign.Response; -import lombok.SneakyThrows; +import static com.netflix.client.config.CommonClientConfigKey.ConnectTimeout; +import static com.netflix.client.config.CommonClientConfigKey.IsSecure; +import static com.netflix.client.config.CommonClientConfigKey.MaxAutoRetries; +import static com.netflix.client.config.CommonClientConfigKey.MaxAutoRetriesNextServer; +import static com.netflix.client.config.CommonClientConfigKey.OkToRetryOnAllOperations; +import static com.netflix.client.config.CommonClientConfigKey.ReadTimeout; +import static com.netflix.client.config.DefaultClientConfigImpl.DEFAULT_MAX_AUTO_RETRIES; +import static com.netflix.client.config.DefaultClientConfigImpl.DEFAULT_MAX_AUTO_RETRIES_NEXT_SERVER; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.when; public class FeignLoadBalancerTests { @@ -50,6 +50,8 @@ public class FeignLoadBalancerTests { private ILoadBalancer lb; @Mock private IClientConfig config; + @Mock + private RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; private FeignLoadBalancer feignLoadBalancer; @@ -75,7 +77,7 @@ public void setup() { public void testUriInsecure() { when(this.config.get(IsSecure)).thenReturn(false); this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config, - this.inspector); + this.inspector, loadBalancedRetryPolicyFactory); Request request = new RequestTemplate().method("GET").append("http://foo/") .request(); RibbonRequest ribbonRequest = new RibbonRequest(this.delegate, request, @@ -96,7 +98,7 @@ public void testUriInsecure() { public void testSecureUriFromClientConfig() { when(this.config.get(IsSecure)).thenReturn(true); this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config, - this.inspector); + this.inspector, loadBalancedRetryPolicyFactory); Server server = new Server("foo", 7777); URI uri = this.feignLoadBalancer.reconstructURIWithServer(server, new URI("http://foo/")); @@ -118,7 +120,7 @@ public boolean isSecure(Server server) { public Map getMetadata(Server server) { return null; } - }); + }, loadBalancedRetryPolicyFactory); Server server = new Server("foo", 7777); URI uri = this.feignLoadBalancer.reconstructURIWithServer(server, new URI("http://foo/")); @@ -129,7 +131,7 @@ public Map getMetadata(Server server) { @SneakyThrows public void testSecureUriFromClientConfigOverride() { this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config, - this.inspector); + this.inspector, loadBalancedRetryPolicyFactory); Server server = Mockito.mock(Server.class); when(server.getPort()).thenReturn(443); when(server.getHost()).thenReturn("foo"); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientRetryTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientRetryTests.java index 5b066bc9ac..ad5fa1d8bd 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientRetryTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientRetryTests.java @@ -16,14 +16,13 @@ package org.springframework.cloud.netflix.feign.ribbon; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; import java.lang.reflect.InvocationHandler; import java.lang.reflect.Proxy; import java.util.concurrent.atomic.AtomicInteger; - import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; @@ -43,13 +42,12 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RestController; - import com.netflix.loadbalancer.Server; import com.netflix.loadbalancer.ServerList; -import lombok.AllArgsConstructor; -import lombok.Data; -import lombok.NoArgsConstructor; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; /** * Tests the Feign Retryer, not ribbon retry. @@ -58,7 +56,8 @@ @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = FeignRibbonClientRetryTests.Application.class, webEnvironment = WebEnvironment.RANDOM_PORT, value = { "spring.application.name=feignclientretrytest", "feign.okhttp.enabled=false", - "feign.httpclient.enabled=false", "feign.hystrix.enabled=false", }) + "feign.httpclient.enabled=false", "feign.hystrix.enabled=false", "localapp.ribbon.MaxAutoRetries=2", + "localapp.ribbon.MaxAutoRetriesNextServer=3"}) @DirtiesContext public class FeignRibbonClientRetryTests { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java index 63e8753a8b..beb1677e46 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java @@ -16,19 +16,19 @@ package org.springframework.cloud.netflix.feign.ribbon; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.argThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import feign.Client; +import feign.Request; +import feign.Request.Options; +import feign.RequestTemplate; import org.hamcrest.CustomMatcher; import org.junit.Before; import org.junit.Test; import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; +import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; - +import org.springframework.retry.support.RetryTemplate; import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.DefaultClientConfigImpl; import com.netflix.client.config.IClientConfig; @@ -38,10 +38,11 @@ import com.netflix.loadbalancer.Server; import com.netflix.loadbalancer.ServerStats; -import feign.Client; -import feign.Request; -import feign.Request.Options; -import feign.RequestTemplate; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.argThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; /** * @author Dave Syer @@ -51,6 +52,7 @@ public class FeignRibbonClientTests { private AbstractLoadBalancer loadBalancer = mock(AbstractLoadBalancer.class); private Client delegate = mock(Client.class); + private RibbonLoadBalancedRetryPolicyFactory retryPolicyFactory = mock(RibbonLoadBalancedRetryPolicyFactory.class); private SpringClientFactory factory = new SpringClientFactory() { @Override @@ -79,7 +81,8 @@ public ILoadBalancer getLoadBalancer(String name) { // Even though we don't maintain FeignRibbonClient, keep these tests // around to make sure the expected behaviour doesn't break - private Client client = new LoadBalancerFeignClient(this.delegate, new CachingSpringLoadBalancerFactory(this.factory), this.factory); + private Client client = new LoadBalancerFeignClient(this.delegate, new CachingSpringLoadBalancerFactory(this.factory, + retryPolicyFactory), this.factory); @Before public void init() { 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 f255c3a8b1..26be6f78ef 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 @@ -17,6 +17,7 @@ package org.springframework.cloud.netflix.feign.valid; import org.junit.Test; +import org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration; import org.springframework.cloud.netflix.feign.EnableFeignClients; import org.springframework.cloud.netflix.feign.FeignAutoConfiguration; import org.springframework.cloud.netflix.feign.FeignClient; @@ -82,6 +83,7 @@ interface Client { @Test public void validLoadBalanced() { AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( + LoadBalancerAutoConfiguration.class, RibbonAutoConfiguration.class, FeignRibbonClientAutoConfiguration.class, GoodServiceIdConfiguration.class);