From a4f30ebf1dedf501c457b1b0ff54586e098f4d9c Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Fri, 17 Feb 2017 11:12:12 -0500 Subject: [PATCH] Make the use of Spring Retry optional when using Feign. Fixes #1708 --- .../main/asciidoc/spring-cloud-netflix.adoc | 39 +-- .../CachingSpringLoadBalancerFactory.java | 12 +- .../feign/ribbon/FeignLoadBalancer.java | 79 ++---- .../FeignRibbonClientAutoConfiguration.java | 12 +- .../ribbon/RetryableFeignLoadBalancer.java | 108 +++++++++ .../feign/ribbon/FeignLoadBalancerTests.java | 11 +- .../RetryableFeignLoadBalancerTest.java | 228 ++++++++++++++++++ .../ribbon/SpringRetryDisabledTests.java | 14 +- .../ribbon/SpringRetryEnabledTests.java | 12 +- 9 files changed, 436 insertions(+), 79 deletions(-) create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancer.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancerTest.java diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 3283faef..0de67700 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -1492,20 +1492,6 @@ The default HTTP client used by zuul is now backed by the Apache HTTP Client ins deprecated Ribbon `RestClient`. To use `RestClient` or to use the `okhttp3.OkHttpClient` set `ribbon.restclient.enabled=true` or `ribbon.okhttp.enabled=true` respectively. -==== Retrying Failed Requests - -When using the Apache Http Client or the OK HTTP Client you can enable them to automatically -retry failed requests by adding https://github.com/spring-projects/spring-retry[Spring Retry] -to your application's classpath. When Zuul uses Ribbon, Zuul will honor some of the Ribbon -configuration values related to retrying failed requests. The properties you can use are -`client.ribbon.MaxAutoRetries`, `client.ribbon.MaxAutoRetriesNextServer`, and -`client.ribbon.OkToRetryOnAllOperations`. See the https://github.com/Netflix/ribbon/wiki/Getting-Started#the-properties-file-sample-clientproperties[Ribbon documentation] -for a description of what there properties do. - -You can turn off Zuul's retry functionality by setting `zuul.retryable` to `false`. You -can also disable retry functionality on route by route basis by setting -`zuul.routes.routename.retryable` to `false`. - === Cookies and Sensitive Headers It's OK to share headers between services in the same system, but you @@ -2412,3 +2398,28 @@ TIP: After executing several requests against your service, you can gather some The Atlas wiki contains a link:https://github.com/Netflix/atlas/wiki/Single-Line[compilation of sample queries] for various scenarios. Make sure to check out the link:https://github.com/Netflix/atlas/wiki/Alerting-Philosophy[alerting philosophy] and docs on using link:https://github.com/Netflix/atlas/wiki/DES[double exponential smoothing] to generate dynamic alert thresholds. + +[[retrying-failed-requests]] +=== Retrying Failed Requests + +Spring Cloud Netflix offers a variety of ways to make HTTP requests. You can use a load balanced +`RestTemplate`, Ribbon, or Feign. No matter how you choose to your HTTP requests, there is always +a chance the request may fail. When a request fails you may want to have the request retried +automatically. To accomplish this when using Sping Cloud Netflix you need to include +https://github.com/spring-projects/spring-retry[Spring Retry] on your application's classpath. +When Spring Retry is present load balanced `RestTemplates`, Feign, and Zuul will automatically +retry any failed requests (assuming you configuration allows it to). + +==== Configuration + +Anytime Ribbon is used with Spring Retry you can control the retry functionality by configuring +certain Ribbon properties. The properties you can use are +`client.ribbon.MaxAutoRetries`, `client.ribbon.MaxAutoRetriesNextServer`, and +`client.ribbon.OkToRetryOnAllOperations`. See the https://github.com/Netflix/ribbon/wiki/Getting-Started#the-properties-file-sample-clientproperties[Ribbon documentation] +for a description of what there properties do. + +==== Zuul + +You can turn off Zuul's retry functionality by setting `zuul.retryable` to `false`. You +can also disable retry functionality on route by route basis by setting +`zuul.routes.routename.retryable` to `false`. 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 a6a89db6..c2e0f940 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 @@ -37,6 +37,7 @@ public class CachingSpringLoadBalancerFactory { private final SpringClientFactory factory; private final LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; + private boolean enableRetry = false; private volatile Map cache = new ConcurrentReferenceHashMap<>(); @@ -51,6 +52,13 @@ public class CachingSpringLoadBalancerFactory { this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; } + public CachingSpringLoadBalancerFactory(SpringClientFactory factory, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, boolean enableRetry) { + this.factory = factory; + this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; + this.enableRetry = enableRetry; + } + public FeignLoadBalancer create(String clientName) { if (this.cache.containsKey(clientName)) { return this.cache.get(clientName); @@ -58,8 +66,8 @@ public class CachingSpringLoadBalancerFactory { 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, - loadBalancedRetryPolicyFactory); + FeignLoadBalancer client = enableRetry ? new RetryableFeignLoadBalancer(lb, config, serverIntrospector, + loadBalancedRetryPolicyFactory) : new FeignLoadBalancer(lb, config, serverIntrospector); 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 56fab2c8..f4ba1f08 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 @@ -29,26 +29,16 @@ 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; @@ -57,20 +47,17 @@ import com.netflix.loadbalancer.Server; import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttpsIfNeeded; public class FeignLoadBalancer extends - AbstractLoadBalancerAwareClient implements - ServiceInstanceChooser { + AbstractLoadBalancerAwareClient { - private final int connectTimeout; - private final int readTimeout; - private final IClientConfig clientConfig; - private final ServerIntrospector serverIntrospector; - private final LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; + protected int connectTimeout; + protected int readTimeout; + protected IClientConfig clientConfig; + protected ServerIntrospector serverIntrospector; public FeignLoadBalancer(ILoadBalancer lb, IClientConfig clientConfig, - ServerIntrospector serverIntrospector, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { + ServerIntrospector serverIntrospector) { super(lb, clientConfig); - this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; - this.setRetryHandler(new DefaultLoadBalancerRetryHandler(clientConfig)); + this.setRetryHandler(RetryHandler.DEFAULT); this.clientConfig = clientConfig; this.connectTimeout = clientConfig.get(CommonClientConfigKey.ConnectTimeout); this.readTimeout = clientConfig.get(CommonClientConfigKey.ReadTimeout); @@ -78,9 +65,9 @@ public class FeignLoadBalancer extends } @Override - public RibbonResponse execute(final RibbonRequest request, IClientConfig configOverride) + public RibbonResponse execute(RibbonRequest request, IClientConfig configOverride) throws IOException { - final Request.Options options; + Request.Options options; if (configOverride != null) { options = new Request.Options( configOverride.get(CommonClientConfigKey.ConnectTimeout, @@ -91,35 +78,26 @@ public class FeignLoadBalancer extends else { options = new Request.Options(this.connectTimeout, this.readTimeout); } - 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); - } - }); + Response response = request.client().execute(request.toRequest(), options); + return new RibbonResponse(request.getUri(), response); } @Override public RequestSpecificRetryHandler getRequestSpecificRetryHandler( RibbonRequest request, IClientConfig requestConfig) { - return new RequestSpecificRetryHandler(false, false, this.getRetryHandler(), 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); + } } @Override @@ -128,12 +106,6 @@ public class FeignLoadBalancer extends 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; @@ -188,6 +160,7 @@ public class FeignLoadBalancer extends }; } + @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/FeignRibbonClientAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java index c8fad1dd..4210fa6c 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 @@ -21,6 +21,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.AutoConfigureBefore; 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.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.feign.FeignAutoConfiguration; @@ -50,9 +51,18 @@ public class FeignRibbonClientAutoConfiguration { @Bean @Primary + @ConditionalOnMissingClass("org.springframework.retry.support.RetryTemplate") public CachingSpringLoadBalancerFactory cachingLBClientFactory( + SpringClientFactory factory) { + return new CachingSpringLoadBalancerFactory(factory); + } + + @Bean + @Primary + @ConditionalOnClass(name = "org.springframework.retry.support.RetryTemplate") + public CachingSpringLoadBalancerFactory retryabeCachingLBClientFactory( SpringClientFactory factory, LoadBalancedRetryPolicyFactory retryPolicyFactory) { - return new CachingSpringLoadBalancerFactory(factory, retryPolicyFactory); + return new CachingSpringLoadBalancerFactory(factory, retryPolicyFactory, true); } @Bean diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancer.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancer.java new file mode 100644 index 00000000..97071e6a --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancer.java @@ -0,0 +1,108 @@ +/* + * + * * 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 feign.Request; +import feign.Response; + +import java.io.IOException; +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.retry.RetryCallback; +import org.springframework.retry.RetryContext; +import org.springframework.retry.policy.NeverRetryPolicy; +import org.springframework.retry.support.RetryTemplate; +import com.netflix.client.DefaultLoadBalancerRetryHandler; +import com.netflix.client.RequestSpecificRetryHandler; +import com.netflix.client.config.CommonClientConfigKey; +import com.netflix.client.config.IClientConfig; +import com.netflix.loadbalancer.ILoadBalancer; +import com.netflix.loadbalancer.Server; + +/** + * A {@link FeignLoadBalancer} that leverages Spring Retry to retry failed requests. + * @author Ryan Baxter + */ +public class RetryableFeignLoadBalancer extends FeignLoadBalancer implements ServiceInstanceChooser { + + private final LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; + + public RetryableFeignLoadBalancer(ILoadBalancer lb, IClientConfig clientConfig, + ServerIntrospector serverIntrospector, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { + super(lb, clientConfig, serverIntrospector); + this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; + this.setRetryHandler(new DefaultLoadBalancerRetryHandler(clientConfig)); + } + + @Override + public RibbonResponse execute(final RibbonRequest request, IClientConfig configOverride) + throws IOException { + final Request.Options options; + if (configOverride != null) { + options = new Request.Options( + configOverride.get(CommonClientConfigKey.ConnectTimeout, + this.connectTimeout), + (configOverride.get(CommonClientConfigKey.ReadTimeout, + this.readTimeout))); + } + else { + options = new Request.Options(this.connectTimeout, this.readTimeout); + } + 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( + FeignLoadBalancer.RibbonRequest request, IClientConfig requestConfig) { + return new RequestSpecificRetryHandler(false, false, this.getRetryHandler(), requestConfig); + } + + @Override + public ServiceInstance choose(String serviceId) { + return new RibbonLoadBalancerClient.RibbonServer(serviceId, + this.getLoadBalancer().chooseServer(serviceId)); + } +} 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 6e0b8eaa..314c6824 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 @@ -17,7 +17,6 @@ import org.mockito.MockitoAnnotations; 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 java.net.URI; @@ -49,8 +48,6 @@ public class FeignLoadBalancerTests { private ILoadBalancer lb; @Mock private IClientConfig config; - @Mock - private RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; private FeignLoadBalancer feignLoadBalancer; @@ -76,7 +73,7 @@ public class FeignLoadBalancerTests { public void testUriInsecure() { when(this.config.get(IsSecure)).thenReturn(false); this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config, - this.inspector, loadBalancedRetryPolicyFactory); + this.inspector); Request request = new RequestTemplate().method("GET").append("http://foo/") .request(); RibbonRequest ribbonRequest = new RibbonRequest(this.delegate, request, @@ -97,7 +94,7 @@ public class FeignLoadBalancerTests { public void testSecureUriFromClientConfig() { when(this.config.get(IsSecure)).thenReturn(true); this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config, - this.inspector, loadBalancedRetryPolicyFactory); + this.inspector); Server server = new Server("foo", 7777); URI uri = this.feignLoadBalancer.reconstructURIWithServer(server, new URI("http://foo/")); @@ -119,7 +116,7 @@ public class FeignLoadBalancerTests { public Map getMetadata(Server server) { return null; } - }, loadBalancedRetryPolicyFactory); + }); Server server = new Server("foo", 7777); URI uri = this.feignLoadBalancer.reconstructURIWithServer(server, new URI("http://foo/")); @@ -130,7 +127,7 @@ public class FeignLoadBalancerTests { @SneakyThrows public void testSecureUriFromClientConfigOverride() { this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config, - this.inspector, loadBalancedRetryPolicyFactory); + this.inspector); 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/RetryableFeignLoadBalancerTest.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancerTest.java new file mode 100644 index 00000000..eda21e44 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancerTest.java @@ -0,0 +1,228 @@ +/* + * + * * 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 feign.Client; +import feign.Request; +import feign.Response; + +import java.io.IOException; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.springframework.cloud.client.ServiceInstance; +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.DefaultServerIntrospector; +import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; +import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerContext; +import org.springframework.cloud.netflix.ribbon.ServerIntrospector; +import org.springframework.cloud.netflix.ribbon.SpringClientFactory; +import org.springframework.http.HttpRequest; + +import com.netflix.client.RequestSpecificRetryHandler; +import com.netflix.client.config.IClientConfig; +import com.netflix.loadbalancer.ILoadBalancer; +import com.netflix.loadbalancer.Server; + +import static com.netflix.client.config.CommonClientConfigKey.ConnectTimeout; +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.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.instanceOf; +import static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * @author Ryan Baxter + */ +public class RetryableFeignLoadBalancerTest { + @Mock + private ILoadBalancer lb; + @Mock + private IClientConfig config; + private ServerIntrospector inspector = new DefaultServerIntrospector(); + + private Integer defaultConnectTimeout = 10000; + private Integer defaultReadTimeout = 10000; + + @Before + public void setup() { + MockitoAnnotations.initMocks(this); + when(this.config.get(MaxAutoRetries, DEFAULT_MAX_AUTO_RETRIES)).thenReturn(1); + when(this.config.get(MaxAutoRetriesNextServer, + DEFAULT_MAX_AUTO_RETRIES_NEXT_SERVER)).thenReturn(1); + when(this.config.get(OkToRetryOnAllOperations, eq(anyBoolean()))) + .thenReturn(true); + when(this.config.get(ConnectTimeout)).thenReturn(this.defaultConnectTimeout); + when(this.config.get(ReadTimeout)).thenReturn(this.defaultReadTimeout); + } + + @Test + public void executeNoFailure() throws Exception { + RibbonLoadBalancerContext lbContext = new RibbonLoadBalancerContext(lb, config); + SpringClientFactory clientFactory = mock(SpringClientFactory.class); + doReturn(lbContext).when(clientFactory).getLoadBalancerContext(any(String.class)); + RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); + HttpRequest springRequest = mock(HttpRequest.class); + Request feignRequest = Request.create("GET", "http://foo", new HashMap>(), + new byte[]{}, StandardCharsets.UTF_8); + Client client = mock(Client.class); + FeignLoadBalancer.RibbonRequest request = new FeignLoadBalancer.RibbonRequest(client, feignRequest, new URI("http://foo")); + Response response = Response.builder().status(200).headers(new HashMap>()).build(); + doReturn(response).when(client).execute(any(Request.class), any(Request.Options.class)); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryPolicyFactory); + FeignLoadBalancer.RibbonResponse ribbonResponse = feignLb.execute(request, null); + assertEquals(200, ribbonResponse.toResponse().status()); + verify(client, times(1)).execute(any(Request.class), any(Request.Options.class)); + } + + @Test + public void executeNeverRetry() throws Exception { + HttpRequest springRequest = mock(HttpRequest.class); + Request feignRequest = Request.create("GET", "http://foo", new HashMap>(), + new byte[]{}, StandardCharsets.UTF_8); + Client client = mock(Client.class); + FeignLoadBalancer.RibbonRequest request = new FeignLoadBalancer.RibbonRequest(client, feignRequest, new URI("http://foo")); + doThrow(new IOException("boom")).when(client).execute(any(Request.class), any(Request.Options.class)); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, new LoadBalancedRetryPolicyFactory() { + @Override + public LoadBalancedRetryPolicy create(String s, ServiceInstanceChooser serviceInstanceChooser) { + return null; + } + }); + try { + feignLb.execute(request, null); + } catch(Exception e) { + assertThat(e, instanceOf(IOException.class)); + } finally { + verify(client, times(1)).execute(any(Request.class), any(Request.Options.class)); + } + } + + @Test + public void executeRetry() throws Exception { + RibbonLoadBalancerContext lbContext = new RibbonLoadBalancerContext(lb, config); + SpringClientFactory clientFactory = mock(SpringClientFactory.class); + doReturn(lbContext).when(clientFactory).getLoadBalancerContext(any(String.class)); + RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); + HttpRequest springRequest = mock(HttpRequest.class); + Request feignRequest = Request.create("GET", "http://foo", new HashMap>(), + new byte[]{}, StandardCharsets.UTF_8); + Client client = mock(Client.class); + FeignLoadBalancer.RibbonRequest request = new FeignLoadBalancer.RibbonRequest(client, feignRequest, new URI("http://foo")); + Response response = Response.builder().status(200).headers(new HashMap>()).build(); + doThrow(new IOException("boom")).doReturn(response).when(client).execute(any(Request.class), any(Request.Options.class)); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryPolicyFactory); + FeignLoadBalancer.RibbonResponse ribbonResponse = feignLb.execute(request, null); + assertEquals(200, ribbonResponse.toResponse().status()); + verify(client, times(2)).execute(any(Request.class), any(Request.Options.class)); + } + + @Test + public void getRequestSpecificRetryHandler() throws Exception { + RibbonLoadBalancerContext lbContext = new RibbonLoadBalancerContext(lb, config); + SpringClientFactory clientFactory = mock(SpringClientFactory.class); + doReturn(lbContext).when(clientFactory).getLoadBalancerContext(any(String.class)); + RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); + HttpRequest springRequest = mock(HttpRequest.class); + Request feignRequest = Request.create("GET", "http://foo", new HashMap>(), + new byte[]{}, StandardCharsets.UTF_8); + Client client = mock(Client.class); + FeignLoadBalancer.RibbonRequest request = new FeignLoadBalancer.RibbonRequest(client, feignRequest, new URI("http://foo")); + Response response = Response.builder().status(200).headers(new HashMap>()).build(); + doReturn(response).when(client).execute(any(Request.class), any(Request.Options.class)); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryPolicyFactory); + RequestSpecificRetryHandler retryHandler = feignLb.getRequestSpecificRetryHandler(request, config); + assertEquals(1, retryHandler.getMaxRetriesOnNextServer()); + assertEquals(1, retryHandler.getMaxRetriesOnSameServer()); + + } + + @Test + public void choose() throws Exception { + RibbonLoadBalancerContext lbContext = new RibbonLoadBalancerContext(lb, config); + SpringClientFactory clientFactory = mock(SpringClientFactory.class); + doReturn(lbContext).when(clientFactory).getLoadBalancerContext(any(String.class)); + RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); + HttpRequest springRequest = mock(HttpRequest.class); + Request feignRequest = Request.create("GET", "http://foo", new HashMap>(), + new byte[]{}, StandardCharsets.UTF_8); + Client client = mock(Client.class); + FeignLoadBalancer.RibbonRequest request = new FeignLoadBalancer.RibbonRequest(client, feignRequest, new URI("http://foo")); + Response response = Response.builder().status(200).headers(new HashMap>()).build(); + doReturn(response).when(client).execute(any(Request.class), any(Request.Options.class)); + final Server server = new Server("foo", 80); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(new ILoadBalancer() { + @Override + public void addServers(List list) { + + } + + @Override + public Server chooseServer(Object o) { + return server; + } + + @Override + public void markServerDown(Server server) { + + } + + @Override + public List getServerList(boolean b) { + return null; + } + + @Override + public List getReachableServers() { + return null; + } + + @Override + public List getAllServers() { + return null; + } + }, config, inspector, loadBalancedRetryPolicyFactory); + ServiceInstance serviceInstance = feignLb.choose("foo"); + assertEquals("foo", serviceInstance.getHost()); + assertEquals(80, serviceInstance.getPort()); + + } + +} \ No newline at end of file diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/SpringRetryDisabledTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/SpringRetryDisabledTests.java index b7f738c1..53f0e91b 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/SpringRetryDisabledTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/SpringRetryDisabledTests.java @@ -28,12 +28,18 @@ import org.springframework.cloud.ClassPathExclusions; import org.springframework.cloud.FilteredClassPathRunner; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration; +import org.springframework.cloud.netflix.feign.ribbon.CachingSpringLoadBalancerFactory; +import org.springframework.cloud.netflix.feign.ribbon.FeignLoadBalancer; +import org.springframework.cloud.netflix.feign.ribbon.FeignRibbonClientAutoConfiguration; +import org.springframework.cloud.netflix.feign.ribbon.RetryableFeignLoadBalancer; import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; import org.springframework.context.ConfigurableApplicationContext; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.core.Is.is; /** * @author Ryan Baxter @@ -47,7 +53,8 @@ public class SpringRetryDisabledTests { @Before public void setUp() { context = new SpringApplicationBuilder().web(false) - .sources(RibbonAutoConfiguration.class,LoadBalancerAutoConfiguration.class, RibbonClientConfiguration.class).run(); + .sources(RibbonAutoConfiguration.class,LoadBalancerAutoConfiguration.class, RibbonClientConfiguration.class, + FeignRibbonClientAutoConfiguration.class).run(); } @After @@ -65,5 +72,10 @@ public class SpringRetryDisabledTests { Map clients = context.getBeansOfType(RibbonLoadBalancingHttpClient.class); assertThat(clients.values(), hasSize(1)); assertThat(clients.values().toArray()[0], instanceOf(RibbonLoadBalancingHttpClient.class)); + Map lbFactorys = context.getBeansOfType(CachingSpringLoadBalancerFactory.class); + assertThat(lbFactorys.values(), hasSize(1)); + FeignLoadBalancer lb =lbFactorys.values().iterator().next().create("foo"); + assertThat(lb, instanceOf(FeignLoadBalancer.class)); + assertThat(lb, is(not(instanceOf(RetryableFeignLoadBalancer.class)))); } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/SpringRetryEnabledTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/SpringRetryEnabledTests.java index 66a0e7a3..ea93d0e3 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/SpringRetryEnabledTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/SpringRetryEnabledTests.java @@ -19,11 +19,16 @@ package org.springframework.cloud.netflix.ribbon; import java.util.Map; +import org.hamcrest.Matchers; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.BeansException; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration; +import org.springframework.cloud.netflix.feign.ribbon.CachingSpringLoadBalancerFactory; +import org.springframework.cloud.netflix.feign.ribbon.FeignLoadBalancer; +import org.springframework.cloud.netflix.feign.ribbon.FeignRibbonClientAutoConfiguration; +import org.springframework.cloud.netflix.feign.ribbon.RetryableFeignLoadBalancer; import org.springframework.cloud.netflix.ribbon.apache.RetryableRibbonLoadBalancingHttpClient; import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; import org.springframework.context.ApplicationContext; @@ -39,7 +44,8 @@ import static org.hamcrest.collection.IsCollectionWithSize.hasSize; * @author Ryan Baxter */ @RunWith(SpringJUnit4ClassRunner.class) -@ContextConfiguration(classes = {RibbonAutoConfiguration.class, RibbonClientConfiguration.class, LoadBalancerAutoConfiguration.class}) +@ContextConfiguration(classes = {RibbonAutoConfiguration.class, RibbonClientConfiguration.class, LoadBalancerAutoConfiguration.class, + FeignRibbonClientAutoConfiguration.class}) public class SpringRetryEnabledTests implements ApplicationContextAware { private ApplicationContext context; @@ -52,6 +58,10 @@ public class SpringRetryEnabledTests implements ApplicationContextAware { Map clients = context.getBeansOfType(RibbonLoadBalancingHttpClient.class); assertThat(clients.values(), hasSize(1)); assertThat(clients.values().toArray()[0], instanceOf(RetryableRibbonLoadBalancingHttpClient.class)); + Map lbFactorys = context.getBeansOfType(CachingSpringLoadBalancerFactory.class); + assertThat(lbFactorys.values(), Matchers.hasSize(1)); + FeignLoadBalancer lb =lbFactorys.values().iterator().next().create("foo"); + assertThat(lb, instanceOf(RetryableFeignLoadBalancer.class)); } @Override