Commit 050d5f89 by Ryan Baxter

Porting changes that remove RetryTemplate from FeignLoadBalancer constructor from 1.2.x branch

parent 384c9b97
...@@ -16,15 +16,16 @@ ...@@ -16,15 +16,16 @@
package org.springframework.cloud.netflix.feign.ribbon; package org.springframework.cloud.netflix.feign.ribbon;
import com.netflix.client.config.IClientConfig; import java.util.Map;
import com.netflix.loadbalancer.ILoadBalancer;
import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; 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.ServerIntrospector;
import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
import org.springframework.retry.support.RetryTemplate;
import org.springframework.util.ConcurrentReferenceHashMap; 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. * Factory for SpringLoadBalancer instances that caches the entries created.
...@@ -35,15 +36,18 @@ import java.util.Map; ...@@ -35,15 +36,18 @@ import java.util.Map;
public class CachingSpringLoadBalancerFactory { public class CachingSpringLoadBalancerFactory {
private final SpringClientFactory factory; private final SpringClientFactory factory;
private final RetryTemplate retryTemplate;
private final LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; private final LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory;
private volatile Map<String, FeignLoadBalancer> cache = new ConcurrentReferenceHashMap<>(); private volatile Map<String, FeignLoadBalancer> 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) { LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) {
this.factory = factory; this.factory = factory;
this.retryTemplate = retryTemplate;
this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory;
} }
...@@ -54,10 +58,10 @@ public class CachingSpringLoadBalancerFactory { ...@@ -54,10 +58,10 @@ public class CachingSpringLoadBalancerFactory {
IClientConfig config = this.factory.getClientConfig(clientName); IClientConfig config = this.factory.getClientConfig(clientName);
ILoadBalancer lb = this.factory.getLoadBalancer(clientName); ILoadBalancer lb = this.factory.getLoadBalancer(clientName);
ServerIntrospector serverIntrospector = this.factory.getInstance(clientName, ServerIntrospector.class); ServerIntrospector serverIntrospector = this.factory.getInstance(clientName, ServerIntrospector.class);
FeignLoadBalancer client = new FeignLoadBalancer(lb, config, serverIntrospector, retryTemplate, FeignLoadBalancer client = new FeignLoadBalancer(lb, config, serverIntrospector,
loadBalancedRetryPolicyFactory); loadBalancedRetryPolicyFactory);
this.cache.put(clientName, client); this.cache.put(clientName, client);
return client; return client;
} }
} }
\ No newline at end of file
...@@ -16,22 +16,20 @@ ...@@ -16,22 +16,20 @@
package org.springframework.cloud.netflix.feign.ribbon; 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.Client;
import feign.Request; import feign.Request;
import feign.Response; import feign.Response;
import feign.Util; 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.ServiceInstance;
import org.springframework.cloud.client.loadbalancer.InterceptorRetryPolicy;
import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext;
import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy;
import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory;
...@@ -45,15 +43,16 @@ import org.springframework.retry.RetryCallback; ...@@ -45,15 +43,16 @@ import org.springframework.retry.RetryCallback;
import org.springframework.retry.RetryContext; import org.springframework.retry.RetryContext;
import org.springframework.retry.policy.NeverRetryPolicy; import org.springframework.retry.policy.NeverRetryPolicy;
import org.springframework.retry.support.RetryTemplate; import org.springframework.retry.support.RetryTemplate;
import com.netflix.client.AbstractLoadBalancerAwareClient;
import java.io.IOException; import com.netflix.client.ClientException;
import java.net.URI; import com.netflix.client.ClientRequest;
import java.util.ArrayList; import com.netflix.client.DefaultLoadBalancerRetryHandler;
import java.util.Collection; import com.netflix.client.IResponse;
import java.util.HashMap; import com.netflix.client.RequestSpecificRetryHandler;
import java.util.LinkedHashMap; import com.netflix.client.config.CommonClientConfigKey;
import java.util.List; import com.netflix.client.config.IClientConfig;
import java.util.Map; import com.netflix.loadbalancer.ILoadBalancer;
import com.netflix.loadbalancer.Server;
import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttpsIfNeeded; import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttpsIfNeeded;
...@@ -65,14 +64,11 @@ public class FeignLoadBalancer extends ...@@ -65,14 +64,11 @@ public class FeignLoadBalancer extends
private final int readTimeout; private final int readTimeout;
private final IClientConfig clientConfig; private final IClientConfig clientConfig;
private final ServerIntrospector serverIntrospector; private final ServerIntrospector serverIntrospector;
private final RetryTemplate retryTemplate;
private final LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; private final LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory;
public FeignLoadBalancer(ILoadBalancer lb, IClientConfig clientConfig, public FeignLoadBalancer(ILoadBalancer lb, IClientConfig clientConfig,
ServerIntrospector serverIntrospector, RetryTemplate retryTemplate, ServerIntrospector serverIntrospector, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) {
LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) {
super(lb, clientConfig); super(lb, clientConfig);
this.retryTemplate = retryTemplate;
this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory;
this.setRetryHandler(new DefaultLoadBalancerRetryHandler(clientConfig)); this.setRetryHandler(new DefaultLoadBalancerRetryHandler(clientConfig));
this.clientConfig = clientConfig; this.clientConfig = clientConfig;
...@@ -96,14 +92,15 @@ public class FeignLoadBalancer extends ...@@ -96,14 +92,15 @@ public class FeignLoadBalancer extends
options = new Request.Options(this.connectTimeout, this.readTimeout); options = new Request.Options(this.connectTimeout, this.readTimeout);
} }
LoadBalancedRetryPolicy retryPolicy = loadBalancedRetryPolicyFactory.create(this.getClientName(), this); LoadBalancedRetryPolicy retryPolicy = loadBalancedRetryPolicyFactory.create(this.getClientName(), this);
RetryTemplate retryTemplate = new RetryTemplate();
retryTemplate.setRetryPolicy(retryPolicy == null ? new NeverRetryPolicy() 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<RibbonResponse, IOException>() { return retryTemplate.execute(new RetryCallback<RibbonResponse, IOException>() {
@Override @Override
public RibbonResponse doWithRetry(RetryContext retryContext) throws IOException { public RibbonResponse doWithRetry(RetryContext retryContext) throws IOException {
Request feignRequest = null; Request feignRequest = null;
//on retries the policy will choose the server and set it in the context //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) { if(retryContext instanceof LoadBalancedRetryContext) {
ServiceInstance service = ((LoadBalancedRetryContext)retryContext).getServiceInstance(); ServiceInstance service = ((LoadBalancedRetryContext)retryContext).getServiceInstance();
if(service != null) { if(service != null) {
...@@ -245,4 +242,4 @@ public class FeignLoadBalancer extends ...@@ -245,4 +242,4 @@ public class FeignLoadBalancer extends
} }
} }
\ No newline at end of file
...@@ -28,7 +28,6 @@ import org.springframework.cloud.netflix.ribbon.SpringClientFactory; ...@@ -28,7 +28,6 @@ import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Primary; import org.springframework.context.annotation.Primary;
import org.springframework.retry.support.RetryTemplate;
import com.netflix.loadbalancer.ILoadBalancer; import com.netflix.loadbalancer.ILoadBalancer;
...@@ -52,16 +51,8 @@ public class FeignRibbonClientAutoConfiguration { ...@@ -52,16 +51,8 @@ public class FeignRibbonClientAutoConfiguration {
@Bean @Bean
@Primary @Primary
public CachingSpringLoadBalancerFactory cachingLBClientFactory( public CachingSpringLoadBalancerFactory cachingLBClientFactory(
SpringClientFactory factory, LoadBalancedRetryPolicyFactory retryPolicyFactory, SpringClientFactory factory, LoadBalancedRetryPolicyFactory retryPolicyFactory) {
RetryTemplate retryTemplate) { return new CachingSpringLoadBalancerFactory(factory, retryPolicyFactory);
return new CachingSpringLoadBalancerFactory(factory, retryTemplate, retryPolicyFactory);
}
@Bean
public RetryTemplate retryTemplate() {
RetryTemplate template = new RetryTemplate();
template.setThrowLastExceptionOnExhausted(true);
return template;
} }
@Bean @Bean
......
...@@ -25,7 +25,6 @@ import org.mockito.Mock; ...@@ -25,7 +25,6 @@ import org.mockito.Mock;
import org.mockito.MockitoAnnotations; import org.mockito.MockitoAnnotations;
import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory;
import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
import org.springframework.retry.support.RetryTemplate;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
import static org.mockito.Mockito.times; import static org.mockito.Mockito.times;
...@@ -56,7 +55,7 @@ public class CachingSpringLoadBalancerFactoryTests { ...@@ -56,7 +55,7 @@ public class CachingSpringLoadBalancerFactoryTests {
when(this.delegate.getClientConfig("client1")).thenReturn(config); when(this.delegate.getClientConfig("client1")).thenReturn(config);
when(this.delegate.getClientConfig("client2")).thenReturn(config); when(this.delegate.getClientConfig("client2")).thenReturn(config);
this.factory = new CachingSpringLoadBalancerFactory(this.delegate, new RetryTemplate(), this.factory = new CachingSpringLoadBalancerFactory(this.delegate,
loadBalancedRetryPolicyFactory); loadBalancedRetryPolicyFactory);
} }
......
...@@ -19,7 +19,6 @@ import org.springframework.cloud.netflix.feign.ribbon.FeignLoadBalancer.RibbonRe ...@@ -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.DefaultServerIntrospector;
import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory;
import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.cloud.netflix.ribbon.ServerIntrospector;
import org.springframework.retry.support.RetryTemplate;
import java.net.URI; import java.net.URI;
import java.util.Collection; import java.util.Collection;
...@@ -52,7 +51,6 @@ public class FeignLoadBalancerTests { ...@@ -52,7 +51,6 @@ public class FeignLoadBalancerTests {
private IClientConfig config; private IClientConfig config;
@Mock @Mock
private RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; private RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory;
private RetryTemplate retryTemplate;
private FeignLoadBalancer feignLoadBalancer; private FeignLoadBalancer feignLoadBalancer;
...@@ -71,7 +69,6 @@ public class FeignLoadBalancerTests { ...@@ -71,7 +69,6 @@ public class FeignLoadBalancerTests {
.thenReturn(true); .thenReturn(true);
when(this.config.get(ConnectTimeout)).thenReturn(this.defaultConnectTimeout); when(this.config.get(ConnectTimeout)).thenReturn(this.defaultConnectTimeout);
when(this.config.get(ReadTimeout)).thenReturn(this.defaultReadTimeout); when(this.config.get(ReadTimeout)).thenReturn(this.defaultReadTimeout);
this.retryTemplate = new RetryTemplate();
} }
@Test @Test
...@@ -79,7 +76,7 @@ public class FeignLoadBalancerTests { ...@@ -79,7 +76,7 @@ public class FeignLoadBalancerTests {
public void testUriInsecure() { public void testUriInsecure() {
when(this.config.get(IsSecure)).thenReturn(false); when(this.config.get(IsSecure)).thenReturn(false);
this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config, 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 request = new RequestTemplate().method("GET").append("http://foo/")
.request(); .request();
RibbonRequest ribbonRequest = new RibbonRequest(this.delegate, request, RibbonRequest ribbonRequest = new RibbonRequest(this.delegate, request,
...@@ -100,7 +97,7 @@ public class FeignLoadBalancerTests { ...@@ -100,7 +97,7 @@ public class FeignLoadBalancerTests {
public void testSecureUriFromClientConfig() { public void testSecureUriFromClientConfig() {
when(this.config.get(IsSecure)).thenReturn(true); when(this.config.get(IsSecure)).thenReturn(true);
this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config, this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config,
this.inspector, retryTemplate, loadBalancedRetryPolicyFactory); this.inspector, loadBalancedRetryPolicyFactory);
Server server = new Server("foo", 7777); Server server = new Server("foo", 7777);
URI uri = this.feignLoadBalancer.reconstructURIWithServer(server, URI uri = this.feignLoadBalancer.reconstructURIWithServer(server,
new URI("http://foo/")); new URI("http://foo/"));
...@@ -122,7 +119,7 @@ public class FeignLoadBalancerTests { ...@@ -122,7 +119,7 @@ public class FeignLoadBalancerTests {
public Map<String, String> getMetadata(Server server) { public Map<String, String> getMetadata(Server server) {
return null; return null;
} }
}, retryTemplate, loadBalancedRetryPolicyFactory); }, loadBalancedRetryPolicyFactory);
Server server = new Server("foo", 7777); Server server = new Server("foo", 7777);
URI uri = this.feignLoadBalancer.reconstructURIWithServer(server, URI uri = this.feignLoadBalancer.reconstructURIWithServer(server,
new URI("http://foo/")); new URI("http://foo/"));
...@@ -133,7 +130,7 @@ public class FeignLoadBalancerTests { ...@@ -133,7 +130,7 @@ public class FeignLoadBalancerTests {
@SneakyThrows @SneakyThrows
public void testSecureUriFromClientConfigOverride() { public void testSecureUriFromClientConfigOverride() {
this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config, this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config,
this.inspector, retryTemplate, loadBalancedRetryPolicyFactory); this.inspector, loadBalancedRetryPolicyFactory);
Server server = Mockito.mock(Server.class); Server server = Mockito.mock(Server.class);
when(server.getPort()).thenReturn(443); when(server.getPort()).thenReturn(443);
when(server.getHost()).thenReturn("foo"); when(server.getHost()).thenReturn("foo");
......
...@@ -35,7 +35,6 @@ import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; ...@@ -35,7 +35,6 @@ import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector;
import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory;
import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.cloud.netflix.ribbon.ServerIntrospector;
import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
import org.springframework.retry.support.RetryTemplate;
import static org.mockito.Matchers.any; import static org.mockito.Matchers.any;
import static org.mockito.Matchers.argThat; import static org.mockito.Matchers.argThat;
...@@ -81,7 +80,7 @@ public class FeignRibbonClientTests { ...@@ -81,7 +80,7 @@ public class FeignRibbonClientTests {
// Even though we don't maintain FeignRibbonClient, keep these tests // Even though we don't maintain FeignRibbonClient, keep these tests
// around to make sure the expected behaviour doesn't break // around to make sure the expected behaviour doesn't break
private Client client = new LoadBalancerFeignClient(this.delegate, new CachingSpringLoadBalancerFactory(this.factory, private Client client = new LoadBalancerFeignClient(this.delegate, new CachingSpringLoadBalancerFactory(this.factory,
new RetryTemplate(), retryPolicyFactory), this.factory); retryPolicyFactory), this.factory);
@Before @Before
public void init() { public void init() {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment