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 3930a663..3bb051ce 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 @@ -19,6 +19,7 @@ package org.springframework.cloud.netflix.feign.ribbon; import java.util.Map; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; @@ -34,12 +35,14 @@ import com.netflix.loadbalancer.ILoadBalancer; * @author Spencer Gibb * @author Dave Syer * @author Ryan Baxter + * @author Gang Li */ public class CachingSpringLoadBalancerFactory { private final SpringClientFactory factory; private final LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; private final LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory; + private final LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory; private boolean enableRetry = false; private volatile Map cache = new ConcurrentReferenceHashMap<>(); @@ -48,6 +51,7 @@ public class CachingSpringLoadBalancerFactory { this.factory = factory; this.loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(factory); this.loadBalancedBackOffPolicyFactory = null; + this.loadBalancedRetryListenerFactory = null; } @Deprecated @@ -57,6 +61,7 @@ public class CachingSpringLoadBalancerFactory { this.factory = factory; this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; this.loadBalancedBackOffPolicyFactory = null; + this.loadBalancedRetryListenerFactory = null; } @Deprecated @@ -67,14 +72,28 @@ public class CachingSpringLoadBalancerFactory { this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; this.enableRetry = enableRetry; this.loadBalancedBackOffPolicyFactory = null; + this.loadBalancedRetryListenerFactory = null; } + @Deprecated + //TODO remove in 2.0.0x public CachingSpringLoadBalancerFactory(SpringClientFactory factory, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory) { this.factory = factory; this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory; + this.loadBalancedRetryListenerFactory = null; + this.enableRetry = true; + } + + public CachingSpringLoadBalancerFactory(SpringClientFactory factory, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, + LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) { + this.factory = factory; + this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; + this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory; + this.loadBalancedRetryListenerFactory = loadBalancedRetryListenerFactory; this.enableRetry = true; } @@ -86,7 +105,7 @@ public class CachingSpringLoadBalancerFactory { ILoadBalancer lb = this.factory.getLoadBalancer(clientName); ServerIntrospector serverIntrospector = this.factory.getInstance(clientName, ServerIntrospector.class); FeignLoadBalancer client = enableRetry ? new RetryableFeignLoadBalancer(lb, config, serverIntrospector, - loadBalancedRetryPolicyFactory, loadBalancedBackOffPolicyFactory) : new FeignLoadBalancer(lb, config, serverIntrospector); + loadBalancedRetryPolicyFactory, loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory) : 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/FeignRibbonClientAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java index c9ca5d33..d8981442 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,6 +22,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.feign.FeignAutoConfiguration; import org.springframework.cloud.netflix.feign.support.FeignHttpClientProperties; @@ -65,10 +66,11 @@ public class FeignRibbonClientAutoConfiguration { @Primary @ConditionalOnClass(name = "org.springframework.retry.support.RetryTemplate") public CachingSpringLoadBalancerFactory retryabeCachingLBClientFactory( - SpringClientFactory factory, - LoadBalancedRetryPolicyFactory retryPolicyFactory, - LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory) { - return new CachingSpringLoadBalancerFactory(factory, retryPolicyFactory, loadBalancedBackOffPolicyFactory); + SpringClientFactory factory, + LoadBalancedRetryPolicyFactory retryPolicyFactory, + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, + LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) { + return new CachingSpringLoadBalancerFactory(factory, retryPolicyFactory, loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory); } @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 index 12546e2a..adaa8680 100644 --- 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 @@ -25,6 +25,7 @@ import java.io.IOException; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.RetryableStatusCodeException; @@ -33,6 +34,7 @@ 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.RetryListener; import org.springframework.retry.backoff.BackOffPolicy; import org.springframework.retry.backoff.NoBackOffPolicy; import org.springframework.retry.policy.NeverRetryPolicy; @@ -47,11 +49,13 @@ import com.netflix.loadbalancer.Server; /** * A {@link FeignLoadBalancer} that leverages Spring Retry to retry failed requests. * @author Ryan Baxter + * @author Gang Li */ public class RetryableFeignLoadBalancer extends FeignLoadBalancer implements ServiceInstanceChooser { private final LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; private final LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory; + private final LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory; @Deprecated //TODO remove in 2.0.x @@ -61,8 +65,11 @@ public class RetryableFeignLoadBalancer extends FeignLoadBalancer implements Ser this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; this.setRetryHandler(new DefaultLoadBalancerRetryHandler(clientConfig)); this.loadBalancedBackOffPolicyFactory = new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); + this.loadBalancedRetryListenerFactory = new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory(); } + @Deprecated + //TODO remove in 2.0.x public RetryableFeignLoadBalancer(ILoadBalancer lb, IClientConfig clientConfig, ServerIntrospector serverIntrospector, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory) { @@ -70,7 +77,21 @@ public class RetryableFeignLoadBalancer extends FeignLoadBalancer implements Ser this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; this.setRetryHandler(new DefaultLoadBalancerRetryHandler(clientConfig)); this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory == null ? - new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory() : loadBalancedBackOffPolicyFactory; + new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory() : loadBalancedBackOffPolicyFactory; + this.loadBalancedRetryListenerFactory = new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory(); + } + + public RetryableFeignLoadBalancer(ILoadBalancer lb, IClientConfig clientConfig, ServerIntrospector serverIntrospector, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, + LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) { + super(lb, clientConfig, serverIntrospector); + this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; + this.setRetryHandler(new DefaultLoadBalancerRetryHandler(clientConfig)); + this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory == null ? + new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory() : loadBalancedBackOffPolicyFactory; + this.loadBalancedRetryListenerFactory = loadBalancedRetryListenerFactory == null ? + new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory() : loadBalancedRetryListenerFactory; } @Override @@ -91,6 +112,10 @@ public class RetryableFeignLoadBalancer extends FeignLoadBalancer implements Ser RetryTemplate retryTemplate = new RetryTemplate(); BackOffPolicy backOffPolicy = loadBalancedBackOffPolicyFactory.createBackOffPolicy(this.getClientName()); retryTemplate.setBackOffPolicy(backOffPolicy == null ? new NoBackOffPolicy() : backOffPolicy); + RetryListener[] retryListeners = this.loadBalancedRetryListenerFactory.createRetryListeners(this.getClientName()); + if (retryListeners != null && retryListeners.length != 0) { + retryTemplate.setListeners(retryListeners); + } retryTemplate.setRetryPolicy(retryPolicy == null ? new NeverRetryPolicy() : new FeignRetryPolicy(request.toHttpRequest(), retryPolicy, this, this.getClientName())); return retryTemplate.execute(new RetryCallback() { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java index 9f9127f6..9a64923b 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java @@ -35,8 +35,8 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.actuator.HasFeatures; import org.springframework.cloud.client.loadbalancer.AsyncLoadBalancerAutoConfiguration; -import org.springframework.cloud.client.loadbalancer.LoadBalanced; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration; import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; @@ -111,6 +111,13 @@ public class RibbonAutoConfiguration { return new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); } + @Bean + @ConditionalOnClass(name = "org.springframework.retry.support.RetryTemplate") + @ConditionalOnMissingBean + public LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory() { + return new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory(); + } + @Bean @ConditionalOnMissingBean public PropertiesFactory propertiesFactory() { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/HttpClientRibbonConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/HttpClientRibbonConfiguration.java index 2eb8e41c..3c7bcae0 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/HttpClientRibbonConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/HttpClientRibbonConfiguration.java @@ -32,6 +32,7 @@ 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.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.commons.httpclient.ApacheHttpClientConnectionManagerFactory; import org.springframework.cloud.commons.httpclient.ApacheHttpClientFactory; @@ -136,10 +137,9 @@ public class HttpClientRibbonConfiguration { @ConditionalOnMissingBean(AbstractLoadBalancerAwareClient.class) @ConditionalOnMissingClass(value = "org.springframework.retry.support.RetryTemplate") public RibbonLoadBalancingHttpClient ribbonLoadBalancingHttpClient( - IClientConfig config, ServerIntrospector serverIntrospector, - ILoadBalancer loadBalancer, RetryHandler retryHandler, CloseableHttpClient httpClient) { - RibbonLoadBalancingHttpClient client = new RibbonLoadBalancingHttpClient( - httpClient, config, serverIntrospector); + IClientConfig config, ServerIntrospector serverIntrospector, + ILoadBalancer loadBalancer, RetryHandler retryHandler, CloseableHttpClient httpClient) { + RibbonLoadBalancingHttpClient client = new RibbonLoadBalancingHttpClient(httpClient, config, serverIntrospector); client.setLoadBalancer(loadBalancer); client.setRetryHandler(retryHandler); Monitors.registerObject("Client_" + this.name, client); @@ -150,13 +150,14 @@ public class HttpClientRibbonConfiguration { @ConditionalOnMissingBean(AbstractLoadBalancerAwareClient.class) @ConditionalOnClass(name = "org.springframework.retry.support.RetryTemplate") public RetryableRibbonLoadBalancingHttpClient retryableRibbonLoadBalancingHttpClient( - IClientConfig config, ServerIntrospector serverIntrospector, - ILoadBalancer loadBalancer, RetryHandler retryHandler, - LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, CloseableHttpClient httpClient, - LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory) { + IClientConfig config, ServerIntrospector serverIntrospector, + ILoadBalancer loadBalancer, RetryHandler retryHandler, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, CloseableHttpClient httpClient, + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, + LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) { RetryableRibbonLoadBalancingHttpClient client = new RetryableRibbonLoadBalancingHttpClient( - httpClient, config, serverIntrospector, loadBalancedRetryPolicyFactory, - loadBalancedBackOffPolicyFactory); + httpClient, config, serverIntrospector, loadBalancedRetryPolicyFactory, + loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory); client.setLoadBalancer(loadBalancer); client.setRetryHandler(retryHandler); Monitors.registerObject("Client_" + this.name, client); diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java index 3bff78b8..5ff08a21 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java @@ -26,6 +26,7 @@ import org.apache.http.impl.client.CloseableHttpClient; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.RetryableStatusCodeException; @@ -36,6 +37,7 @@ import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.http.HttpRequest; import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; +import org.springframework.retry.RetryListener; import org.springframework.retry.backoff.BackOffPolicy; import org.springframework.retry.backoff.NoBackOffPolicy; import org.springframework.retry.policy.NeverRetryPolicy; @@ -49,12 +51,15 @@ import com.netflix.loadbalancer.Server; /** * An Apache HTTP client which leverages Spring Retry to retry failed requests. * @author Ryan Baxter + * @author Gang Li */ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingHttpClient implements ServiceInstanceChooser { private LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new LoadBalancedRetryPolicyFactory.NeverRetryFactory(); private LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory = - new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); + new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); + private LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory = + new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory(); @Deprecated //TODO remove in 2.0.x @@ -74,6 +79,8 @@ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingH this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; } + @Deprecated + //TODO remove in 2.0.x public RetryableRibbonLoadBalancingHttpClient(CloseableHttpClient delegate, IClientConfig config, ServerIntrospector serverIntrospector, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, @@ -83,6 +90,17 @@ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingH this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory; } + public RetryableRibbonLoadBalancingHttpClient(CloseableHttpClient delegate, + IClientConfig config, ServerIntrospector serverIntrospector, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, + LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) { + super(delegate, config, serverIntrospector); + this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; + this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory; + this.loadBalancedRetryListenerFactory = loadBalancedRetryListenerFactory; + } + @Override public RibbonApacheHttpResponse execute(final RibbonApacheHttpRequest request, final IClientConfig configOverride) throws Exception { final RequestConfig.Builder builder = RequestConfig.custom(); @@ -136,6 +154,10 @@ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingH : new RetryPolicy(request, retryPolicy, this, this.getClientName())); BackOffPolicy backOffPolicy = loadBalancedBackOffPolicyFactory.createBackOffPolicy(this.getClientName()); retryTemplate.setBackOffPolicy(backOffPolicy == null ? new NoBackOffPolicy() : backOffPolicy); + RetryListener[] retryListeners = this.loadBalancedRetryListenerFactory.createRetryListeners(this.getClientName()); + if (retryListeners != null && retryListeners.length != 0) { + retryTemplate.setListeners(retryListeners); + } return retryTemplate.execute(callback); } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonConfiguration.java index 1e1f2fbf..9a796bf8 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonConfiguration.java @@ -35,6 +35,7 @@ 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.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.commons.httpclient.OkHttpClientConnectionPoolFactory; import org.springframework.cloud.commons.httpclient.OkHttpClientFactory; @@ -58,8 +59,7 @@ public class OkHttpRibbonConfiguration { @Bean @ConditionalOnMissingBean(ConnectionPool.class) - public ConnectionPool httpClientConnectionPool(IClientConfig config, - OkHttpClientConnectionPoolFactory connectionPoolFactory) { + public ConnectionPool httpClientConnectionPool(IClientConfig config, OkHttpClientConnectionPoolFactory connectionPoolFactory) { Integer maxTotalConnections = config.getPropertyAsInteger( CommonClientConfigKey.MaxTotalConnections, DefaultClientConfigImpl.DEFAULT_MAX_TOTAL_CONNECTIONS); @@ -80,8 +80,7 @@ public class OkHttpRibbonConfiguration { @Bean @ConditionalOnMissingBean(OkHttpClient.class) - public OkHttpClient client(OkHttpClientFactory httpClientFactory, - ConnectionPool connectionPool, IClientConfig config) { + public OkHttpClient client(OkHttpClientFactory httpClientFactory, ConnectionPool connectionPool, IClientConfig config) { Boolean followRedirects = config.getPropertyAsBoolean( CommonClientConfigKey.FollowRedirects, DefaultClientConfigImpl.DEFAULT_FOLLOW_REDIRECTS); @@ -107,19 +106,20 @@ public class OkHttpRibbonConfiguration { } } - @Bean @ConditionalOnMissingBean(AbstractLoadBalancerAwareClient.class) @ConditionalOnClass(name = "org.springframework.retry.support.RetryTemplate") - public RetryableOkHttpLoadBalancingClient okHttpLoadBalancingClient(IClientConfig config, - ServerIntrospector serverIntrospector, - ILoadBalancer loadBalancer, - RetryHandler retryHandler, - LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, - OkHttpClient delegate, - LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory) { + public RetryableOkHttpLoadBalancingClient okHttpLoadBalancingClient( + IClientConfig config, + ServerIntrospector serverIntrospector, + ILoadBalancer loadBalancer, + RetryHandler retryHandler, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, + OkHttpClient delegate, + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, + LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) { RetryableOkHttpLoadBalancingClient client = new RetryableOkHttpLoadBalancingClient(delegate, config, - serverIntrospector, loadBalancedRetryPolicyFactory, loadBalancedBackOffPolicyFactory); + serverIntrospector, loadBalancedRetryPolicyFactory, loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory); client.setLoadBalancer(loadBalancer); client.setRetryHandler(retryHandler); Monitors.registerObject("Client_" + this.name, client); @@ -129,9 +129,10 @@ public class OkHttpRibbonConfiguration { @Bean @ConditionalOnMissingBean(AbstractLoadBalancerAwareClient.class) @ConditionalOnMissingClass(value = "org.springframework.retry.support.RetryTemplate") - public OkHttpLoadBalancingClient retryableOkHttpLoadBalancingClient(IClientConfig config, - ServerIntrospector serverIntrospector, ILoadBalancer loadBalancer, - RetryHandler retryHandler, OkHttpClient delegate) { + public OkHttpLoadBalancingClient retryableOkHttpLoadBalancingClient( + IClientConfig config, + ServerIntrospector serverIntrospector, ILoadBalancer loadBalancer, + RetryHandler retryHandler, OkHttpClient delegate) { OkHttpLoadBalancingClient client = new OkHttpLoadBalancingClient(delegate, config, serverIntrospector); client.setLoadBalancer(loadBalancer); diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/RetryableOkHttpLoadBalancingClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/RetryableOkHttpLoadBalancingClient.java index 67876d47..594c99ea 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/RetryableOkHttpLoadBalancingClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/RetryableOkHttpLoadBalancingClient.java @@ -24,6 +24,7 @@ import org.apache.commons.lang.BooleanUtils; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.RetryableStatusCodeException; @@ -34,6 +35,7 @@ import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.http.HttpRequest; import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; +import org.springframework.retry.RetryListener; import org.springframework.retry.backoff.BackOffPolicy; import org.springframework.retry.backoff.NoBackOffPolicy; import org.springframework.retry.policy.NeverRetryPolicy; @@ -47,12 +49,15 @@ import com.netflix.loadbalancer.Server; /** * An OK HTTP client which leverages Spring Retry to retry failed request. * @author Ryan Baxter + * @author Gang Li */ public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClient implements ServiceInstanceChooser { private LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; private LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory = - new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); + new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); + private LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory = + new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory(); @Deprecated //TODO remove in 2.0.x @@ -62,6 +67,8 @@ public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClien this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; } + @Deprecated + //TODO remove in 2.0.x public RetryableOkHttpLoadBalancingClient(OkHttpClient delegate, IClientConfig config, ServerIntrospector serverIntrospector, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory) { @@ -70,12 +77,25 @@ public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClien this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory; } + public RetryableOkHttpLoadBalancingClient(OkHttpClient delegate, IClientConfig config, ServerIntrospector serverIntrospector, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, + LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) { + super(delegate, config, serverIntrospector); + this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; + this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory; + this.loadBalancedRetryListenerFactory = loadBalancedRetryListenerFactory; + } + private OkHttpRibbonResponse executeWithRetry(OkHttpRibbonRequest request, LoadBalancedRetryPolicy retryPolicy, - RetryCallback callback) - throws Exception { + RetryCallback callback) throws Exception { RetryTemplate retryTemplate = new RetryTemplate(); BackOffPolicy backOffPolicy = loadBalancedBackOffPolicyFactory.createBackOffPolicy(this.getClientName()); retryTemplate.setBackOffPolicy(backOffPolicy == null ? new NoBackOffPolicy() : backOffPolicy); + RetryListener[] retryListeners = this.loadBalancedRetryListenerFactory.createRetryListeners(this.getClientName()); + if (retryListeners != null && retryListeners.length != 0) { + retryTemplate.setListeners(retryListeners); + } boolean retryable = request.getContext() == null ? true : BooleanUtils.toBooleanDefaultIfNull(request.getContext().getRetryable(), true); retryTemplate.setRetryPolicy(retryPolicy == null || !retryable ? new NeverRetryPolicy() 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 358c4004..d86ed087 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 @@ -23,6 +23,8 @@ import org.junit.Before; import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; @@ -42,6 +44,12 @@ public class CachingSpringLoadBalancerFactoryTests { @Mock private RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; + @Mock + private LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory; + + @Mock + private LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory; + private CachingSpringLoadBalancerFactory factory; @Before @@ -78,4 +86,51 @@ public class CachingSpringLoadBalancerFactoryTests { verify(this.delegate, times(1)).getClientConfig("client2"); } + @Test + public void delegateCreatesWithNoRetry() { + IClientConfig config = new DefaultClientConfigImpl(); + config.set(CommonClientConfigKey.ConnectTimeout, 1000); + config.set(CommonClientConfigKey.ReadTimeout, 500); + when(this.delegate.getClientConfig("retry")).thenReturn(config); + CachingSpringLoadBalancerFactory factory = new CachingSpringLoadBalancerFactory(this.delegate); + FeignLoadBalancer client = this.factory.create("retry"); + assertNotNull("client was null", client); + } + + @Test + public void delegateCreatesWithRetry() { + IClientConfig config = new DefaultClientConfigImpl(); + config.set(CommonClientConfigKey.ConnectTimeout, 1000); + config.set(CommonClientConfigKey.ReadTimeout, 500); + when(this.delegate.getClientConfig("retry")).thenReturn(config); + CachingSpringLoadBalancerFactory factory = new CachingSpringLoadBalancerFactory( + this.delegate, loadBalancedRetryPolicyFactory, false); + FeignLoadBalancer client = this.factory.create("retry"); + assertNotNull("client was null", client); + } + + @Test + public void delegateCreatesWithBackOff() { + IClientConfig config = new DefaultClientConfigImpl(); + config.set(CommonClientConfigKey.ConnectTimeout, 1000); + config.set(CommonClientConfigKey.ReadTimeout, 500); + when(this.delegate.getClientConfig("retry")).thenReturn(config); + CachingSpringLoadBalancerFactory factory = new CachingSpringLoadBalancerFactory( + this.delegate, loadBalancedRetryPolicyFactory, loadBalancedBackOffPolicyFactory); + FeignLoadBalancer client = this.factory.create("retry"); + assertNotNull("client was null", client); + } + + @Test + public void delegateCreatesWithRetryListener() { + IClientConfig config = new DefaultClientConfigImpl(); + config.set(CommonClientConfigKey.ConnectTimeout, 1000); + config.set(CommonClientConfigKey.ReadTimeout, 500); + when(this.delegate.getClientConfig("retry")).thenReturn(config); + CachingSpringLoadBalancerFactory factory = new CachingSpringLoadBalancerFactory( + this.delegate, loadBalancedRetryPolicyFactory, loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory); + FeignLoadBalancer client = this.factory.create("retry"); + assertNotNull("client was null", client); + } + } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancerTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancerTests.java index 4a86ceef..b115750c 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancerTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancerTests.java @@ -34,6 +34,7 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser; @@ -44,7 +45,10 @@ 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 org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; +import org.springframework.retry.RetryListener; +import org.springframework.retry.TerminatedRetryException; import org.springframework.retry.backoff.BackOffContext; import org.springframework.retry.backoff.BackOffInterruptedException; import org.springframework.retry.backoff.BackOffPolicy; @@ -78,6 +82,7 @@ import static org.mockito.Mockito.when; /** * @author Ryan Baxter + * @author Gang Li */ public class RetryableFeignLoadBalancerTests { @Mock @@ -287,6 +292,95 @@ public class RetryableFeignLoadBalancerTests { } + @Test + public void retryListenerTest() throws Exception { + RibbonLoadBalancerContext lbContext = new RibbonLoadBalancerContext(lb, config); + SpringClientFactory clientFactory = mock(SpringClientFactory.class); + IClientConfig config = mock(IClientConfig.class); + doReturn(1).when(config).get(eq(CommonClientConfigKey.MaxAutoRetries), anyInt()); + doReturn(1).when(config).get(eq(CommonClientConfigKey.MaxAutoRetriesNextServer), anyInt()); + doReturn(true).when(config).get(eq(CommonClientConfigKey.OkToRetryOnAllOperations), eq(false)); + doReturn(defaultConnectTimeout).when(config).get(eq(CommonClientConfigKey.ConnectTimeout)); + doReturn(defaultReadTimeout).when(config).get(eq(CommonClientConfigKey.ReadTimeout)); + doReturn("").when(config).getPropertyAsString(eq(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES),eq("")); + doReturn(config).when(clientFactory).getClientConfig(eq("default")); + doReturn(lbContext).when(clientFactory).getLoadBalancerContext(any(String.class)); + RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); + HttpRequest springRequest = mock(HttpRequest.class); + Request feignRequest = Request.create("GET", "http://listener", new HashMap>(), + new byte[]{}, StandardCharsets.UTF_8); + Client client = mock(Client.class); + FeignLoadBalancer.RibbonRequest request = new FeignLoadBalancer.RibbonRequest(client, feignRequest, new URI("http://listener")); + 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)); + MyBackOffPolicyFactory backOffPolicyFactory = new MyBackOffPolicyFactory(); + MyRetryListeners myRetryListeners = new MyRetryListeners(); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryPolicyFactory, + backOffPolicyFactory, myRetryListeners); + FeignLoadBalancer.RibbonResponse ribbonResponse = feignLb.execute(request, null); + assertEquals(200, ribbonResponse.toResponse().status()); + verify(client, times(2)).execute(any(Request.class), any(Request.Options.class)); + assertEquals(1, backOffPolicyFactory.getCount()); + assertEquals(1, myRetryListeners.getOnError()); + } + + @Test(expected = TerminatedRetryException.class) + public void retryListenerTestNoRetry() throws Exception { + RibbonLoadBalancerContext lbContext = new RibbonLoadBalancerContext(lb, config); + SpringClientFactory clientFactory = mock(SpringClientFactory.class); + IClientConfig config = mock(IClientConfig.class); + doReturn(1).when(config).get(eq(CommonClientConfigKey.MaxAutoRetries), anyInt()); + doReturn(1).when(config).get(eq(CommonClientConfigKey.MaxAutoRetriesNextServer), anyInt()); + doReturn(true).when(config).get(eq(CommonClientConfigKey.OkToRetryOnAllOperations), eq(false)); + doReturn(defaultConnectTimeout).when(config).get(eq(CommonClientConfigKey.ConnectTimeout)); + doReturn(defaultReadTimeout).when(config).get(eq(CommonClientConfigKey.ReadTimeout)); + doReturn("").when(config).getPropertyAsString(eq(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES),eq("")); + doReturn(config).when(clientFactory).getClientConfig(eq("default")); + doReturn(lbContext).when(clientFactory).getLoadBalancerContext(any(String.class)); + RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); + HttpRequest springRequest = mock(HttpRequest.class); + Request feignRequest = Request.create("GET", "http://listener", new HashMap>(), + new byte[]{}, StandardCharsets.UTF_8); + Client client = mock(Client.class); + FeignLoadBalancer.RibbonRequest request = new FeignLoadBalancer.RibbonRequest(client, feignRequest, new URI("http://listener")); + Response response = Response.builder().status(200).headers(new HashMap>()).build(); + MyBackOffPolicyFactory backOffPolicyFactory = new MyBackOffPolicyFactory(); + MyRetryListenersNotRetry myRetryListenersNotRetry = new MyRetryListenersNotRetry(); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryPolicyFactory, + backOffPolicyFactory, myRetryListenersNotRetry); + FeignLoadBalancer.RibbonResponse ribbonResponse = feignLb.execute(request, null); + } + + @Test + public void retryWithDefaultConstructorTest() throws Exception { + RibbonLoadBalancerContext lbContext = new RibbonLoadBalancerContext(lb, config); + SpringClientFactory clientFactory = mock(SpringClientFactory.class); + IClientConfig config = mock(IClientConfig.class); + doReturn(1).when(config).get(eq(CommonClientConfigKey.MaxAutoRetries), anyInt()); + doReturn(1).when(config).get(eq(CommonClientConfigKey.MaxAutoRetriesNextServer), anyInt()); + doReturn(true).when(config).get(eq(CommonClientConfigKey.OkToRetryOnAllOperations), eq(false)); + doReturn(defaultConnectTimeout).when(config).get(eq(CommonClientConfigKey.ConnectTimeout)); + doReturn(defaultReadTimeout).when(config).get(eq(CommonClientConfigKey.ReadTimeout)); + doReturn("").when(config).getPropertyAsString(eq(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES),eq("")); + doReturn(config).when(clientFactory).getClientConfig(eq("default")); + doReturn(lbContext).when(clientFactory).getLoadBalancerContext(any(String.class)); + RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); + HttpRequest springRequest = mock(HttpRequest.class); + Request feignRequest = Request.create("GET", "http://listener", new HashMap>(), + new byte[]{}, StandardCharsets.UTF_8); + Client client = mock(Client.class); + FeignLoadBalancer.RibbonRequest request = new FeignLoadBalancer.RibbonRequest(client, feignRequest, new URI("http://listener")); + 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)); + MyBackOffPolicyFactory backOffPolicyFactory = new MyBackOffPolicyFactory(); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryPolicyFactory, + backOffPolicyFactory); + FeignLoadBalancer.RibbonResponse ribbonResponse = feignLb.execute(request, null); + assertEquals(200, ribbonResponse.toResponse().status()); + verify(client, times(2)).execute(any(Request.class), any(Request.Options.class)); + assertEquals(1, backOffPolicyFactory.getCount()); + } + class MyBackOffPolicyFactory implements LoadBalancedBackOffPolicyFactory, BackOffPolicy { private int count = 0; @@ -311,4 +405,56 @@ public class RetryableFeignLoadBalancerTests { } } + class MyRetryListeners implements LoadBalancedRetryListenerFactory { + + private int onError = 0; + + @Override + public RetryListener[] createRetryListeners(String service) { + return new RetryListener[] {new RetryListener() { + @Override + public boolean open(RetryContext context, RetryCallback callback) { + return true; + } + + @Override + public void close(RetryContext context, RetryCallback callback, Throwable throwable) { + + } + + @Override + public void onError(RetryContext context, RetryCallback callback, Throwable throwable) { + onError++; + } + }}; + } + + public int getOnError() { + return onError; + } + } + + class MyRetryListenersNotRetry implements LoadBalancedRetryListenerFactory { + + @Override + public RetryListener[] createRetryListeners(String service) { + return new RetryListener[] {new RetryListener() { + @Override + public boolean open(RetryContext context, RetryCallback callback) { + return false; + } + + @Override + public void close(RetryContext context, RetryCallback callback, Throwable throwable) { + + } + + @Override + public void onError(RetryContext context, RetryCallback callback, Throwable throwable) { + + } + }}; + } + } + } \ No newline at end of file diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java index d45708bd..40b793b1 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java @@ -32,9 +32,9 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; -import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; - import org.mockito.ArgumentMatcher; +import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; import org.springframework.cloud.netflix.ribbon.RibbonAutoConfiguration; @@ -49,7 +49,10 @@ import org.springframework.context.annotation.AnnotationConfigApplicationContext import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpMethod; +import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; +import org.springframework.retry.RetryListener; +import org.springframework.retry.TerminatedRetryException; import org.springframework.retry.backoff.BackOffContext; import org.springframework.retry.backoff.BackOffInterruptedException; import org.springframework.retry.backoff.BackOffPolicy; @@ -82,11 +85,13 @@ import static org.mockito.Mockito.verify; /** * @author Sébastien Nussbaumer * @author Ryan Baxter + * @author Gang Li */ public class RibbonLoadBalancingHttpClientTests { private ILoadBalancer loadBalancer; private LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory = new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); + private LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory = new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory(); @Before public void setup() { @@ -246,6 +251,34 @@ public class RibbonLoadBalancingHttpClientTests { return client; } + private RetryableRibbonLoadBalancingHttpClient setupClientForRetry(int retriesNextServer, int retriesSameServer, + boolean retryable, boolean retryOnAllOps, + String serviceName, String host, int port, + CloseableHttpClient delegate, ILoadBalancer lb, String statusCodes, + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, + boolean isSecure, LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) throws Exception { + ServerIntrospector introspector = mock(ServerIntrospector.class); + RetryHandler retryHandler = new DefaultLoadBalancerRetryHandler(retriesSameServer, retriesNextServer, retryable); + doReturn(new Server(host, port)).when(lb).chooseServer(eq(serviceName)); + DefaultClientConfigImpl clientConfig = new DefaultClientConfigImpl(); + clientConfig.set(CommonClientConfigKey.OkToRetryOnAllOperations, retryOnAllOps); + clientConfig.set(CommonClientConfigKey.MaxAutoRetriesNextServer, retriesNextServer); + clientConfig.set(CommonClientConfigKey.MaxAutoRetries, retriesSameServer); + clientConfig.set(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES, statusCodes); + clientConfig.set(CommonClientConfigKey.IsSecure, isSecure); + clientConfig.setClientName(serviceName); + RibbonLoadBalancerContext context = new RibbonLoadBalancerContext(lb, clientConfig, retryHandler); + SpringClientFactory clientFactory = mock(SpringClientFactory.class); + doReturn(context).when(clientFactory).getLoadBalancerContext(eq(serviceName)); + doReturn(clientConfig).when(clientFactory).getClientConfig(eq(serviceName)); + LoadBalancedRetryPolicyFactory factory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); + RetryableRibbonLoadBalancingHttpClient client = new RetryableRibbonLoadBalancingHttpClient(delegate, clientConfig, + introspector, factory, loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory); + client.setLoadBalancer(lb); + ReflectionTestUtils.setField(client, "delegate", delegate); + return client; + } + @Test public void testRetrySameServerOnly() throws Exception { int retriesNextServer = 0; @@ -499,6 +532,148 @@ public class RibbonLoadBalancingHttpClientTests { assertEquals(1, myBackOffPolicyFactory.getCount()); } + @Test + public void retryListenerTest() throws Exception { + int retriesNextServer = 1; + int retriesSameServer = 1; + boolean retryable = true; + boolean retryOnAllOps = true; + String serviceName = "listener"; + String host = serviceName; + int port = 80; + HttpMethod method = HttpMethod.POST; + URI uri = new URI("http://" + host + ":" + port); + CloseableHttpClient delegate = mock(CloseableHttpClient.class); + final CloseableHttpResponse response = mock(CloseableHttpResponse.class); + StatusLine statusLine = mock(StatusLine.class); + doReturn(200).when(statusLine).getStatusCode(); + doReturn(statusLine).when(response).getStatusLine(); + doThrow(new IOException("boom")).doThrow(new IOException("boom again")).doReturn(response). + when(delegate).execute(any(HttpUriRequest.class)); + ILoadBalancer lb = mock(ILoadBalancer.class); + MyBackOffPolicyFactory myBackOffPolicyFactory = new MyBackOffPolicyFactory(); + MyRetryListeners myRetryListeners = new MyRetryListeners(); + RetryableRibbonLoadBalancingHttpClient client = setupClientForRetry(retriesNextServer, retriesSameServer, retryable, retryOnAllOps, + serviceName, host, port, delegate, lb, "", myBackOffPolicyFactory, false, myRetryListeners); + RibbonApacheHttpRequest request = mock(RibbonApacheHttpRequest.class); + doReturn(method).when(request).getMethod(); + doReturn(uri).when(request).getURI(); + doReturn(request).when(request).withNewUri(any(URI.class)); + HttpUriRequest uriRequest = mock(HttpUriRequest.class); + doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class)); + RibbonApacheHttpResponse returnedResponse = client.execute(request, null); + verify(response, times(0)).close(); + verify(delegate, times(3)).execute(any(HttpUriRequest.class)); + verify(lb, times(1)).chooseServer(eq(serviceName)); + assertEquals(2, myBackOffPolicyFactory.getCount()); + assertEquals(2, myRetryListeners.getOnError()); + } + + @Test + public void retryDefaultListenerTest() throws Exception { + int retriesNextServer = 1; + int retriesSameServer = 1; + boolean retryable = true; + boolean retryOnAllOps = true; + String serviceName = "listener"; + String host = serviceName; + int port = 80; + HttpMethod method = HttpMethod.POST; + URI uri = new URI("http://" + host + ":" + port); + CloseableHttpClient delegate = mock(CloseableHttpClient.class); + final CloseableHttpResponse response = mock(CloseableHttpResponse.class); + StatusLine statusLine = mock(StatusLine.class); + doReturn(200).when(statusLine).getStatusCode(); + doReturn(statusLine).when(response).getStatusLine(); + doThrow(new IOException("boom")).doThrow(new IOException("boom again")).doReturn(response). + when(delegate).execute(any(HttpUriRequest.class)); + ILoadBalancer lb = mock(ILoadBalancer.class); + MyBackOffPolicyFactory myBackOffPolicyFactory = new MyBackOffPolicyFactory(); + MyRetryListeners myRetryListeners = new MyRetryListeners(); + RetryableRibbonLoadBalancingHttpClient client = setupClientForRetry(retriesNextServer, retriesSameServer, retryable, retryOnAllOps, + serviceName, host, port, delegate, lb, "", myBackOffPolicyFactory, false, loadBalancedRetryListenerFactory); + RibbonApacheHttpRequest request = mock(RibbonApacheHttpRequest.class); + doReturn(method).when(request).getMethod(); + doReturn(uri).when(request).getURI(); + doReturn(request).when(request).withNewUri(any(URI.class)); + HttpUriRequest uriRequest = mock(HttpUriRequest.class); + doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class)); + RibbonApacheHttpResponse returnedResponse = client.execute(request, null); + verify(response, times(0)).close(); + verify(delegate, times(3)).execute(any(HttpUriRequest.class)); + verify(lb, times(1)).chooseServer(eq(serviceName)); + assertEquals(2, myBackOffPolicyFactory.getCount()); + assertEquals(0, myRetryListeners.getOnError()); + } + + @Test(expected = TerminatedRetryException.class) + public void retryListenerTestNoRetry() throws Exception { + int retriesNextServer = 1; + int retriesSameServer = 1; + boolean retryable = true; + boolean retryOnAllOps = true; + String serviceName = "listener"; + String host = serviceName; + int port = 80; + HttpMethod method = HttpMethod.POST; + URI uri = new URI("http://" + host + ":" + port); + CloseableHttpClient delegate = mock(CloseableHttpClient.class); + final CloseableHttpResponse response = mock(CloseableHttpResponse.class); + StatusLine statusLine = mock(StatusLine.class); + doReturn(200).when(statusLine).getStatusCode(); + doReturn(statusLine).when(response).getStatusLine(); + doThrow(new IOException("boom")).doThrow(new IOException("boom again")).doReturn(response). + when(delegate).execute(any(HttpUriRequest.class)); + ILoadBalancer lb = mock(ILoadBalancer.class); + MyBackOffPolicyFactory myBackOffPolicyFactory = new MyBackOffPolicyFactory(); + MyRetryListenersNotRetry myRetryListenersNotRetry = new MyRetryListenersNotRetry(); + RetryableRibbonLoadBalancingHttpClient client = setupClientForRetry(retriesNextServer, retriesSameServer, retryable, retryOnAllOps, + serviceName, host, port, delegate, lb, "", myBackOffPolicyFactory, false, myRetryListenersNotRetry); + RibbonApacheHttpRequest request = mock(RibbonApacheHttpRequest.class); + doReturn(method).when(request).getMethod(); + doReturn(uri).when(request).getURI(); + doReturn(request).when(request).withNewUri(any(URI.class)); + HttpUriRequest uriRequest = mock(HttpUriRequest.class); + doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class)); + RibbonApacheHttpResponse returnedResponse = client.execute(request, null); + + } + + @Test + public void retryWithOriginalConstructorTest() throws Exception { + int retriesNextServer = 1; + int retriesSameServer = 1; + boolean retryable = true; + boolean retryOnAllOps = true; + String serviceName = "listener"; + String host = serviceName; + int port = 80; + HttpMethod method = HttpMethod.POST; + URI uri = new URI("http://" + host + ":" + port); + CloseableHttpClient delegate = mock(CloseableHttpClient.class); + final CloseableHttpResponse response = mock(CloseableHttpResponse.class); + StatusLine statusLine = mock(StatusLine.class); + doReturn(200).when(statusLine).getStatusCode(); + doReturn(statusLine).when(response).getStatusLine(); + doThrow(new IOException("boom")).doThrow(new IOException("boom again")).doReturn(response). + when(delegate).execute(any(HttpUriRequest.class)); + ILoadBalancer lb = mock(ILoadBalancer.class); + MyBackOffPolicyFactory myBackOffPolicyFactory = new MyBackOffPolicyFactory(); + RetryableRibbonLoadBalancingHttpClient client = setupClientForRetry(retriesNextServer, retriesSameServer, retryable, retryOnAllOps, + serviceName, host, port, delegate, lb, "", myBackOffPolicyFactory, false); + RibbonApacheHttpRequest request = mock(RibbonApacheHttpRequest.class); + doReturn(method).when(request).getMethod(); + doReturn(uri).when(request).getURI(); + doReturn(request).when(request).withNewUri(any(URI.class)); + HttpUriRequest uriRequest = mock(HttpUriRequest.class); + doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class)); + RibbonApacheHttpResponse returnedResponse = client.execute(request, null); + verify(response, times(0)).close(); + verify(delegate, times(3)).execute(any(HttpUriRequest.class)); + verify(lb, times(1)).chooseServer(eq(serviceName)); + assertEquals(2, myBackOffPolicyFactory.getCount()); + } + @Configuration protected static class UseDefaults { @@ -612,4 +787,56 @@ public class RibbonLoadBalancingHttpClientTests { } } + class MyRetryListeners implements LoadBalancedRetryListenerFactory { + + private int onError = 0; + + @Override + public RetryListener[] createRetryListeners(String service) { + return new RetryListener[] {new RetryListener() { + @Override + public boolean open(RetryContext context, RetryCallback callback) { + return true; + } + + @Override + public void close(RetryContext context, RetryCallback callback, Throwable throwable) { + + } + + @Override + public void onError(RetryContext context, RetryCallback callback, Throwable throwable) { + onError++; + } + }}; + } + + public int getOnError() { + return onError; + } + } + + class MyRetryListenersNotRetry implements LoadBalancedRetryListenerFactory { + + @Override + public RetryListener[] createRetryListeners(String service) { + return new RetryListener[] {new RetryListener() { + @Override + public boolean open(RetryContext context, RetryCallback callback) { + return false; + } + + @Override + public void close(RetryContext context, RetryCallback callback, Throwable throwable) { + + } + + @Override + public void onError(RetryContext context, RetryCallback callback, Throwable throwable) { + + } + }}; + } + } + }