From 050d5f898f453313768de23d139eb426a644c236 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Wed, 15 Feb 2017 20:09:15 -0500 Subject: [PATCH] Porting changes that remove RetryTemplate from FeignLoadBalancer constructor from 1.2.x branch --- .../CachingSpringLoadBalancerFactory.java | 22 ++++---- .../feign/ribbon/FeignLoadBalancer.java | 51 +++++++++---------- .../FeignRibbonClientAutoConfiguration.java | 13 +---- ...CachingSpringLoadBalancerFactoryTests.java | 3 +- .../feign/ribbon/FeignLoadBalancerTests.java | 11 ++-- .../feign/ribbon/FeignRibbonClientTests.java | 3 +- 6 files changed, 45 insertions(+), 58 deletions(-) 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 c88a551c..a6a89db6 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 @@ -16,15 +16,16 @@ package org.springframework.cloud.netflix.feign.ribbon; -import com.netflix.client.config.IClientConfig; -import com.netflix.loadbalancer.ILoadBalancer; +import java.util.Map; + import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; +import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; -import org.springframework.retry.support.RetryTemplate; import org.springframework.util.ConcurrentReferenceHashMap; -import java.util.Map; +import com.netflix.client.config.IClientConfig; +import com.netflix.loadbalancer.ILoadBalancer; /** * Factory for SpringLoadBalancer instances that caches the entries created. @@ -35,15 +36,18 @@ import java.util.Map; public class CachingSpringLoadBalancerFactory { private final SpringClientFactory factory; - private final RetryTemplate retryTemplate; private final LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; private volatile Map cache = new ConcurrentReferenceHashMap<>(); - public CachingSpringLoadBalancerFactory(SpringClientFactory factory, RetryTemplate retryTemplate, + public CachingSpringLoadBalancerFactory(SpringClientFactory factory) { + this.factory = factory; + this.loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(factory); + } + + public CachingSpringLoadBalancerFactory(SpringClientFactory factory, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { this.factory = factory; - this.retryTemplate = retryTemplate; this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; } @@ -54,10 +58,10 @@ 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, retryTemplate, + FeignLoadBalancer client = new FeignLoadBalancer(lb, config, serverIntrospector, loadBalancedRetryPolicyFactory); this.cache.put(clientName, client); return client; } -} +} \ No newline at end of file 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 a8fad5f7..56fab2c8 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancer.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancer.java @@ -16,22 +16,20 @@ package org.springframework.cloud.netflix.feign.ribbon; -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.config.CommonClientConfigKey; -import com.netflix.client.config.IClientConfig; -import com.netflix.loadbalancer.ILoadBalancer; -import com.netflix.loadbalancer.Server; import feign.Client; import feign.Request; import feign.Response; import feign.Util; + +import java.io.IOException; +import java.net.URI; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; import org.springframework.cloud.client.ServiceInstance; -import org.springframework.cloud.client.loadbalancer.InterceptorRetryPolicy; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; @@ -45,15 +43,16 @@ import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; import org.springframework.retry.policy.NeverRetryPolicy; import org.springframework.retry.support.RetryTemplate; - -import java.io.IOException; -import java.net.URI; -import java.util.ArrayList; -import java.util.Collection; -import java.util.HashMap; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; +import 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.config.CommonClientConfigKey; +import com.netflix.client.config.IClientConfig; +import com.netflix.loadbalancer.ILoadBalancer; +import com.netflix.loadbalancer.Server; import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttpsIfNeeded; @@ -65,14 +64,11 @@ public class FeignLoadBalancer extends private final int readTimeout; private final IClientConfig clientConfig; private final ServerIntrospector serverIntrospector; - private final RetryTemplate retryTemplate; private final LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; public FeignLoadBalancer(ILoadBalancer lb, IClientConfig clientConfig, - ServerIntrospector serverIntrospector, RetryTemplate retryTemplate, - LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { + ServerIntrospector serverIntrospector, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { super(lb, clientConfig); - this.retryTemplate = retryTemplate; this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; this.setRetryHandler(new DefaultLoadBalancerRetryHandler(clientConfig)); this.clientConfig = clientConfig; @@ -96,14 +92,15 @@ public class FeignLoadBalancer extends 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())); + : 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 + //extract the server and update the request being made if(retryContext instanceof LoadBalancedRetryContext) { ServiceInstance service = ((LoadBalancedRetryContext)retryContext).getServiceInstance(); if(service != null) { @@ -245,4 +242,4 @@ public class FeignLoadBalancer extends } -} +} \ No newline at end of file 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 2a29a252..c8fad1dd 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 @@ -28,7 +28,6 @@ import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Primary; -import org.springframework.retry.support.RetryTemplate; import com.netflix.loadbalancer.ILoadBalancer; @@ -52,16 +51,8 @@ public class FeignRibbonClientAutoConfiguration { @Bean @Primary public CachingSpringLoadBalancerFactory cachingLBClientFactory( - SpringClientFactory factory, LoadBalancedRetryPolicyFactory retryPolicyFactory, - RetryTemplate retryTemplate) { - return new CachingSpringLoadBalancerFactory(factory, retryTemplate, retryPolicyFactory); - } - - @Bean - public RetryTemplate retryTemplate() { - RetryTemplate template = new RetryTemplate(); - template.setThrowLastExceptionOnExhausted(true); - return template; + SpringClientFactory factory, LoadBalancedRetryPolicyFactory retryPolicyFactory) { + return new CachingSpringLoadBalancerFactory(factory, retryPolicyFactory); } @Bean 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 fa714a16..358c4004 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 @@ -25,7 +25,6 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; -import org.springframework.retry.support.RetryTemplate; import static org.junit.Assert.assertNotNull; import static org.mockito.Mockito.times; @@ -56,7 +55,7 @@ public class CachingSpringLoadBalancerFactoryTests { when(this.delegate.getClientConfig("client1")).thenReturn(config); when(this.delegate.getClientConfig("client2")).thenReturn(config); - this.factory = new CachingSpringLoadBalancerFactory(this.delegate, new RetryTemplate(), + this.factory = new CachingSpringLoadBalancerFactory(this.delegate, loadBalancedRetryPolicyFactory); } 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 5643b6a2..6e0b8eaa 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 @@ -19,7 +19,6 @@ import org.springframework.cloud.netflix.feign.ribbon.FeignLoadBalancer.RibbonRe import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; -import org.springframework.retry.support.RetryTemplate; import java.net.URI; import java.util.Collection; @@ -52,7 +51,6 @@ public class FeignLoadBalancerTests { private IClientConfig config; @Mock private RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; - private RetryTemplate retryTemplate; private FeignLoadBalancer feignLoadBalancer; @@ -71,7 +69,6 @@ public class FeignLoadBalancerTests { .thenReturn(true); when(this.config.get(ConnectTimeout)).thenReturn(this.defaultConnectTimeout); when(this.config.get(ReadTimeout)).thenReturn(this.defaultReadTimeout); - this.retryTemplate = new RetryTemplate(); } @Test @@ -79,7 +76,7 @@ public class FeignLoadBalancerTests { public void testUriInsecure() { when(this.config.get(IsSecure)).thenReturn(false); this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config, - this.inspector, retryTemplate, loadBalancedRetryPolicyFactory); + this.inspector, loadBalancedRetryPolicyFactory); Request request = new RequestTemplate().method("GET").append("http://foo/") .request(); RibbonRequest ribbonRequest = new RibbonRequest(this.delegate, request, @@ -100,7 +97,7 @@ public class FeignLoadBalancerTests { public void testSecureUriFromClientConfig() { when(this.config.get(IsSecure)).thenReturn(true); this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config, - this.inspector, retryTemplate, loadBalancedRetryPolicyFactory); + this.inspector, loadBalancedRetryPolicyFactory); Server server = new Server("foo", 7777); URI uri = this.feignLoadBalancer.reconstructURIWithServer(server, new URI("http://foo/")); @@ -122,7 +119,7 @@ public class FeignLoadBalancerTests { public Map getMetadata(Server server) { return null; } - }, retryTemplate, loadBalancedRetryPolicyFactory); + }, loadBalancedRetryPolicyFactory); Server server = new Server("foo", 7777); URI uri = this.feignLoadBalancer.reconstructURIWithServer(server, new URI("http://foo/")); @@ -133,7 +130,7 @@ public class FeignLoadBalancerTests { @SneakyThrows public void testSecureUriFromClientConfigOverride() { this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config, - this.inspector, retryTemplate, loadBalancedRetryPolicyFactory); + this.inspector, loadBalancedRetryPolicyFactory); Server server = Mockito.mock(Server.class); when(server.getPort()).thenReturn(443); when(server.getHost()).thenReturn("foo"); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java index dbf3159f..676ca307 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java @@ -35,7 +35,6 @@ import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; -import org.springframework.retry.support.RetryTemplate; import static org.mockito.Matchers.any; import static org.mockito.Matchers.argThat; @@ -81,7 +80,7 @@ public class FeignRibbonClientTests { // Even though we don't maintain FeignRibbonClient, keep these tests // around to make sure the expected behaviour doesn't break private Client client = new LoadBalancerFeignClient(this.delegate, new CachingSpringLoadBalancerFactory(this.factory, - new RetryTemplate(), retryPolicyFactory), this.factory); + retryPolicyFactory), this.factory); @Before public void init() {