From 485583dad8bde99afc5f01ef3ba51febfe783ac0 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Mon, 12 Mar 2018 22:16:44 -0400 Subject: [PATCH] Refactor spring retry code (#10) --- .../CachingSpringLoadBalancerFactory.java | 59 +---- .../FeignRibbonClientAutoConfiguration.java | 10 +- .../ribbon/RetryableFeignLoadBalancer.java | 52 +---- ...CachingSpringLoadBalancerFactoryTests.java | 42 +--- .../ribbon/FeignRibbonClientTests.java | 5 +- .../RetryableFeignLoadBalancerTests.java | 201 ++++++++++-------- 6 files changed, 136 insertions(+), 233 deletions(-) diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/CachingSpringLoadBalancerFactory.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/CachingSpringLoadBalancerFactory.java index 7b743c99a..d64a3c9e2 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/CachingSpringLoadBalancerFactory.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/CachingSpringLoadBalancerFactory.java @@ -18,10 +18,7 @@ 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.client.loadbalancer.LoadBalancedRetryFactory; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.util.ConcurrentReferenceHashMap; @@ -40,61 +37,17 @@ public class CachingSpringLoadBalancerFactory { private final SpringClientFactory factory; - private final LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; - private final LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory; - private final LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory; - private boolean enableRetry = false; + private LoadBalancedRetryFactory loadBalancedRetryFactory = null; private volatile Map cache = new ConcurrentReferenceHashMap<>(); public CachingSpringLoadBalancerFactory(SpringClientFactory factory) { this.factory = factory; - this.loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(factory); - this.loadBalancedBackOffPolicyFactory = null; - this.loadBalancedRetryListenerFactory = null; } - @Deprecated - //TODO remove in 2.0.x - public CachingSpringLoadBalancerFactory(SpringClientFactory factory, - LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { + public CachingSpringLoadBalancerFactory(SpringClientFactory factory, LoadBalancedRetryFactory loadBalancedRetryPolicyFactory) { this.factory = factory; - this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; - this.loadBalancedBackOffPolicyFactory = null; - this.loadBalancedRetryListenerFactory = null; - } - - @Deprecated - //TODO remove in 2.0.0x - public CachingSpringLoadBalancerFactory(SpringClientFactory factory, - LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, boolean enableRetry) { - this.factory = factory; - 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; + this.loadBalancedRetryFactory = loadBalancedRetryPolicyFactory; } public FeignLoadBalancer create(String clientName) { @@ -105,8 +58,8 @@ public FeignLoadBalancer create(String clientName) { IClientConfig config = this.factory.getClientConfig(clientName); ILoadBalancer lb = this.factory.getLoadBalancer(clientName); ServerIntrospector serverIntrospector = this.factory.getInstance(clientName, ServerIntrospector.class); - client = enableRetry ? new RetryableFeignLoadBalancer(lb, config, serverIntrospector, - loadBalancedRetryPolicyFactory, loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory) : new FeignLoadBalancer(lb, config, serverIntrospector); + client = loadBalancedRetryFactory != null ? new RetryableFeignLoadBalancer(lb, config, serverIntrospector, + loadBalancedRetryFactory) : new FeignLoadBalancer(lb, config, serverIntrospector); this.cache.put(clientName, client); return client; } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/FeignRibbonClientAutoConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/FeignRibbonClientAutoConfiguration.java index e1184ded9..36ba91175 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/FeignRibbonClientAutoConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/FeignRibbonClientAutoConfiguration.java @@ -21,9 +21,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.client.loadbalancer.LoadBalancedRetryFactory; import org.springframework.cloud.openfeign.FeignAutoConfiguration; import org.springframework.cloud.openfeign.support.FeignHttpClientProperties; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; @@ -67,10 +65,8 @@ public CachingSpringLoadBalancerFactory cachingLBClientFactory( @ConditionalOnClass(name = "org.springframework.retry.support.RetryTemplate") public CachingSpringLoadBalancerFactory retryabeCachingLBClientFactory( SpringClientFactory factory, - LoadBalancedRetryPolicyFactory retryPolicyFactory, - LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, - LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) { - return new CachingSpringLoadBalancerFactory(factory, retryPolicyFactory, loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory); + LoadBalancedRetryFactory retryFactory) { + return new CachingSpringLoadBalancerFactory(factory, retryFactory); } @Bean diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/RetryableFeignLoadBalancer.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/RetryableFeignLoadBalancer.java index f2fd8c3ed..25d5a85c5 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/RetryableFeignLoadBalancer.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/RetryableFeignLoadBalancer.java @@ -24,12 +24,10 @@ import java.io.IOException; import java.net.URI; import org.springframework.cloud.client.ServiceInstance; -import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRecoveryCallback; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; -import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; -import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; -import org.springframework.cloud.client.loadbalancer.RibbonRecoveryCallback; import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClient; import org.springframework.cloud.netflix.ribbon.RibbonProperties; @@ -55,45 +53,13 @@ */ 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 - public RetryableFeignLoadBalancer(ILoadBalancer lb, IClientConfig clientConfig, - ServerIntrospector serverIntrospector, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { - super(lb, clientConfig, serverIntrospector); - 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) { - super(lb, clientConfig, serverIntrospector); - this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; - this.setRetryHandler(new DefaultLoadBalancerRetryHandler(clientConfig)); - this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory == null ? - new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory() : loadBalancedBackOffPolicyFactory; - this.loadBalancedRetryListenerFactory = new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory(); - } + private final LoadBalancedRetryFactory loadBalancedRetryFactory; public RetryableFeignLoadBalancer(ILoadBalancer lb, IClientConfig clientConfig, ServerIntrospector serverIntrospector, - LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, - LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, - LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) { + LoadBalancedRetryFactory loadBalancedRetryFactory) { super(lb, clientConfig, serverIntrospector); - this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; + this.loadBalancedRetryFactory = loadBalancedRetryFactory; this.setRetryHandler(new DefaultLoadBalancerRetryHandler(clientConfig)); - this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory == null ? - new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory() : loadBalancedBackOffPolicyFactory; - this.loadBalancedRetryListenerFactory = loadBalancedRetryListenerFactory == null ? - new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory() : loadBalancedRetryListenerFactory; } @Override @@ -109,11 +75,11 @@ public RibbonResponse execute(final RibbonRequest request, IClientConfig configO else { options = new Request.Options(this.connectTimeout, this.readTimeout); } - final LoadBalancedRetryPolicy retryPolicy = loadBalancedRetryPolicyFactory.create(this.getClientName(), this); + final LoadBalancedRetryPolicy retryPolicy = loadBalancedRetryFactory.createRetryPolicy(this.getClientName(), this); RetryTemplate retryTemplate = new RetryTemplate(); - BackOffPolicy backOffPolicy = loadBalancedBackOffPolicyFactory.createBackOffPolicy(this.getClientName()); + BackOffPolicy backOffPolicy = loadBalancedRetryFactory.createBackOffPolicy(this.getClientName()); retryTemplate.setBackOffPolicy(backOffPolicy == null ? new NoBackOffPolicy() : backOffPolicy); - RetryListener[] retryListeners = this.loadBalancedRetryListenerFactory.createRetryListeners(this.getClientName()); + RetryListener[] retryListeners = this.loadBalancedRetryFactory.createRetryListeners(this.getClientName()); if (retryListeners != null && retryListeners.length != 0) { retryTemplate.setListeners(retryListeners); } @@ -143,7 +109,7 @@ public RibbonResponse doWithRetry(RetryContext retryContext) throws IOException } return new RibbonResponse(request.getUri(), response); } - }, new RibbonRecoveryCallback() { + }, new LoadBalancedRecoveryCallback() { @Override protected RibbonResponse createResponse(Response response, URI uri) { return new RibbonResponse(uri, response); diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/CachingSpringLoadBalancerFactoryTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/CachingSpringLoadBalancerFactoryTests.java index 8eb62d8ba..ad9172ce3 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/CachingSpringLoadBalancerFactoryTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/CachingSpringLoadBalancerFactoryTests.java @@ -23,9 +23,7 @@ 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.RibbonLoadBalancedRetryFactory; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import static org.junit.Assert.assertNotNull; @@ -42,13 +40,7 @@ public class CachingSpringLoadBalancerFactoryTests { private SpringClientFactory delegate; @Mock - private RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; - - @Mock - private LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory; - - @Mock - private LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory; + private RibbonLoadBalancedRetryFactory loadBalancedRetryFactory; private CachingSpringLoadBalancerFactory factory; @@ -64,7 +56,7 @@ public void init() { when(this.delegate.getClientConfig("client2")).thenReturn(config); this.factory = new CachingSpringLoadBalancerFactory(this.delegate, - loadBalancedRetryPolicyFactory); + loadBalancedRetryFactory); } @Test @@ -103,34 +95,8 @@ public void delegateCreatesWithRetry() { 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); + CachingSpringLoadBalancerFactory factory = new CachingSpringLoadBalancerFactory(this.delegate, loadBalancedRetryFactory); 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-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/FeignRibbonClientTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/FeignRibbonClientTests.java index 9f5909fd6..ae82be9a5 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/FeignRibbonClientTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/FeignRibbonClientTests.java @@ -32,7 +32,6 @@ import org.junit.Before; import org.junit.Test; import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; -import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; @@ -48,7 +47,6 @@ public class FeignRibbonClientTests { private AbstractLoadBalancer loadBalancer = mock(AbstractLoadBalancer.class); private Client delegate = mock(Client.class); - private RibbonLoadBalancedRetryPolicyFactory retryPolicyFactory = mock(RibbonLoadBalancedRetryPolicyFactory.class); private SpringClientFactory factory = new SpringClientFactory() { @Override @@ -77,8 +75,7 @@ public ILoadBalancer getLoadBalancer(String name) { // Even though we don't maintain FeignRibbonClient, keep these tests // around to make sure the expected behaviour doesn't break - private Client client = new LoadBalancerFeignClient(this.delegate, new CachingSpringLoadBalancerFactory(this.factory, - retryPolicyFactory), this.factory); + private Client client = new LoadBalancerFeignClient(this.delegate, new CachingSpringLoadBalancerFactory(this.factory), this.factory); @Before public void init() { diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/RetryableFeignLoadBalancerTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/RetryableFeignLoadBalancerTests.java index 173419459..e8bc6c3ee 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/RetryableFeignLoadBalancerTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/RetryableFeignLoadBalancerTests.java @@ -37,14 +37,12 @@ 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.LoadBalancedRetryFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; -import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser; import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; +import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryFactory; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicy; -import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerContext; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; @@ -96,8 +94,6 @@ public class RetryableFeignLoadBalancerTests { @Mock private IClientConfig config; private ServerIntrospector inspector = new DefaultServerIntrospector(); - private LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory = - new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); private Integer defaultConnectTimeout = 10000; private Integer defaultReadTimeout = 10000; @@ -128,7 +124,7 @@ public void executeNoFailure() throws Exception { doReturn(defaultReadTimeout).when(config).get(eq(CommonClientConfigKey.ReadTimeout)); doReturn("404,502,foo, ,").when(config).getPropertyAsString(eq(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES),eq("")); doReturn(config).when(clientFactory).getClientConfig(eq("default")); - RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); + RibbonLoadBalancedRetryFactory loadBalancedRetryFactory = new RibbonLoadBalancedRetryFactory(clientFactory); HttpRequest springRequest = mock(HttpRequest.class); Request feignRequest = Request.create("GET", "http://foo", new HashMap>(), new byte[]{}, StandardCharsets.UTF_8); @@ -136,8 +132,7 @@ public void executeNoFailure() throws Exception { FeignLoadBalancer.RibbonRequest request = new FeignLoadBalancer.RibbonRequest(client, feignRequest, new URI("http://foo")); Response response = Response.builder().status(200).headers(new HashMap>()).build(); doReturn(response).when(client).execute(any(Request.class), any(Request.Options.class)); - RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryPolicyFactory, - loadBalancedBackOffPolicyFactory); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryFactory); FeignLoadBalancer.RibbonResponse ribbonResponse = feignLb.execute(request, null); assertEquals(200, ribbonResponse.toResponse().status()); verify(client, times(1)).execute(any(Request.class), any(Request.Options.class)); @@ -151,12 +146,22 @@ public void executeNeverRetry() throws Exception { Client client = mock(Client.class); FeignLoadBalancer.RibbonRequest request = new FeignLoadBalancer.RibbonRequest(client, feignRequest, new URI("http://foo")); doThrow(new IOException("boom")).when(client).execute(any(Request.class), any(Request.Options.class)); - RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, new LoadBalancedRetryPolicyFactory() { + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, new LoadBalancedRetryFactory() { @Override - public LoadBalancedRetryPolicy create(String s, ServiceInstanceChooser serviceInstanceChooser) { + public LoadBalancedRetryPolicy createRetryPolicy(String s, ServiceInstanceChooser serviceInstanceChooser) { return null; } - }, loadBalancedBackOffPolicyFactory); + + @Override + public RetryListener[] createRetryListeners(String service) { + return new RetryListener[0]; + } + + @Override + public BackOffPolicy createBackOffPolicy(String service) { + return null; + } + }); try { feignLb.execute(request, null); } catch(Exception e) { @@ -179,7 +184,13 @@ public void executeRetry() throws Exception { 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); + MyBackOffPolicy backOffPolicy = new MyBackOffPolicy(); + RibbonLoadBalancedRetryFactory loadBalancedRetryFactory = new RibbonLoadBalancedRetryFactory(clientFactory){ + @Override + public BackOffPolicy createBackOffPolicy(String service) { + return backOffPolicy; + } + }; HttpRequest springRequest = mock(HttpRequest.class); Request feignRequest = Request.create("GET", "http://foo", new HashMap>(), new byte[]{}, StandardCharsets.UTF_8); @@ -187,13 +198,12 @@ public void executeRetry() throws Exception { FeignLoadBalancer.RibbonRequest request = new FeignLoadBalancer.RibbonRequest(client, feignRequest, new URI("http://foo")); Response response = Response.builder().status(200).headers(new HashMap>()).build(); doThrow(new IOException("boom")).doReturn(response).when(client).execute(any(Request.class), any(Request.Options.class)); - MyBackOffPolicyFactory backOffPolicyFactory = new MyBackOffPolicyFactory(); - RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryPolicyFactory, - backOffPolicyFactory); + + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryFactory); 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, backOffPolicy.getCount()); } @Test @@ -209,7 +219,13 @@ public void executeRetryOnStatusCode() throws Exception { doReturn("404").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); + MyBackOffPolicy backOffPolicy = new MyBackOffPolicy(); + RibbonLoadBalancedRetryFactory loadBalancedRetryFactory = new RibbonLoadBalancedRetryFactory(clientFactory){ + @Override + public BackOffPolicy createBackOffPolicy(String service) { + return backOffPolicy; + } + }; HttpRequest springRequest = mock(HttpRequest.class); Request feignRequest = Request.create("GET", "http://foo", new HashMap>(), new byte[]{}, StandardCharsets.UTF_8); @@ -218,13 +234,11 @@ public void executeRetryOnStatusCode() throws Exception { Response response = Response.builder().status(200).headers(new HashMap>()).build(); Response fourOFourResponse = Response.builder().status(404).headers(new HashMap>()).build(); doReturn(fourOFourResponse).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); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryFactory); 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, backOffPolicy.getCount()); } @Test @@ -232,7 +246,7 @@ public void getRequestSpecificRetryHandler() throws Exception { RibbonLoadBalancerContext lbContext = new RibbonLoadBalancerContext(lb, config); SpringClientFactory clientFactory = mock(SpringClientFactory.class); doReturn(lbContext).when(clientFactory).getLoadBalancerContext(any(String.class)); - RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); + RibbonLoadBalancedRetryFactory loadBalancedRetryFactory = new RibbonLoadBalancedRetryFactory(clientFactory); HttpRequest springRequest = mock(HttpRequest.class); Request feignRequest = Request.create("GET", "http://foo", new HashMap>(), new byte[]{}, StandardCharsets.UTF_8); @@ -240,8 +254,7 @@ public void getRequestSpecificRetryHandler() throws Exception { FeignLoadBalancer.RibbonRequest request = new FeignLoadBalancer.RibbonRequest(client, feignRequest, new URI("http://foo")); Response response = Response.builder().status(200).headers(new HashMap>()).build(); doReturn(response).when(client).execute(any(Request.class), any(Request.Options.class)); - RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryPolicyFactory, - loadBalancedBackOffPolicyFactory); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryFactory); RequestSpecificRetryHandler retryHandler = feignLb.getRequestSpecificRetryHandler(request, config); assertEquals(1, retryHandler.getMaxRetriesOnNextServer()); assertEquals(1, retryHandler.getMaxRetriesOnSameServer()); @@ -253,7 +266,7 @@ public void choose() throws Exception { RibbonLoadBalancerContext lbContext = new RibbonLoadBalancerContext(lb, config); SpringClientFactory clientFactory = mock(SpringClientFactory.class); doReturn(lbContext).when(clientFactory).getLoadBalancerContext(any(String.class)); - RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); + RibbonLoadBalancedRetryFactory loadBalancedRetryFactory = new RibbonLoadBalancedRetryFactory(clientFactory); HttpRequest springRequest = mock(HttpRequest.class); Request feignRequest = Request.create("GET", "http://foo", new HashMap>(), new byte[]{}, StandardCharsets.UTF_8); @@ -292,7 +305,7 @@ public List getReachableServers() { public List getAllServers() { return null; } - }, config, inspector, loadBalancedRetryPolicyFactory, loadBalancedBackOffPolicyFactory); + }, config, inspector, loadBalancedRetryFactory); ServiceInstance serviceInstance = feignLb.choose("foo"); assertEquals("foo", serviceInstance.getHost()); assertEquals(80, serviceInstance.getPort()); @@ -312,7 +325,19 @@ public void retryListenerTest() throws Exception { 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); + MyBackOffPolicy backOffPolicy = new MyBackOffPolicy(); + MyRetryListener myRetryListener = new MyRetryListener(); + RibbonLoadBalancedRetryFactory loadBalancedRetryFactory = new RibbonLoadBalancedRetryFactory(clientFactory) { + @Override + public RetryListener[] createRetryListeners(String service) { + return new RetryListener[]{myRetryListener}; + } + + @Override + public BackOffPolicy createBackOffPolicy(String service) { + return backOffPolicy; + } + }; HttpRequest springRequest = mock(HttpRequest.class); Request feignRequest = Request.create("GET", "http://listener", new HashMap>(), new byte[]{}, StandardCharsets.UTF_8); @@ -320,15 +345,13 @@ public void retryListenerTest() throws Exception { 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); + + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryFactory); 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()); + assertEquals(1, backOffPolicy.getCount()); + assertEquals(1, myRetryListener.getOnError()); } @Test(expected = TerminatedRetryException.class) @@ -344,17 +367,26 @@ public void retryListenerTestNoRetry() throws Exception { 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); + Response response = Response.builder().status(200).headers(new HashMap>()).build(); + MyBackOffPolicy backOffPolicy = new MyBackOffPolicy(); + MyRetryListenerNotRetry myRetryListenerNotRetry = new MyRetryListenerNotRetry(); + RibbonLoadBalancedRetryFactory loadBalancedRetryFactory = new RibbonLoadBalancedRetryFactory(clientFactory){ + @Override + public RetryListener[] createRetryListeners(String service) { + return new RetryListener[]{myRetryListenerNotRetry}; + } + + @Override + public BackOffPolicy createBackOffPolicy(String service) { + return backOffPolicy; + } + }; 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); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryFactory); FeignLoadBalancer.RibbonResponse ribbonResponse = feignLb.execute(request, null); } @@ -371,7 +403,13 @@ public void retryWithDefaultConstructorTest() throws Exception { 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); + MyBackOffPolicy backOffPolicy = new MyBackOffPolicy(); + RibbonLoadBalancedRetryFactory loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryFactory(clientFactory){ + @Override + public BackOffPolicy createBackOffPolicy(String service) { + return backOffPolicy; + } + }; HttpRequest springRequest = mock(HttpRequest.class); Request feignRequest = Request.create("GET", "http://listener", new HashMap>(), new byte[]{}, StandardCharsets.UTF_8); @@ -379,13 +417,11 @@ public void retryWithDefaultConstructorTest() throws Exception { 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); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryPolicyFactory); FeignLoadBalancer.RibbonResponse ribbonResponse = feignLb.execute(request, null); assertEquals(200, ribbonResponse.toResponse().status()); verify(client, times(2)).execute(any(Request.class), any(Request.Options.class)); - assertEquals(1, backOffPolicyFactory.getCount()); + assertEquals(1, backOffPolicy.getCount()); } @Test @@ -402,7 +438,13 @@ public void executeRetryFail() throws Exception { doReturn("404").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); + MyBackOffPolicy backOffPolicy = new MyBackOffPolicy(); + RibbonLoadBalancedRetryFactory loadBalancedRetryFactory = new RibbonLoadBalancedRetryFactory(clientFactory){ + @Override + public BackOffPolicy createBackOffPolicy(String service) { + return backOffPolicy; + } + }; HttpRequest springRequest = mock(HttpRequest.class); Request feignRequest = Request.create("GET", "http://foo", new HashMap>(), new byte[]{}, StandardCharsets.UTF_8); @@ -435,18 +477,17 @@ public void close() throws IOException { } }).build(); doReturn(fourOFourResponse).when(client).execute(any(Request.class), any(Request.Options.class)); - MyBackOffPolicyFactory backOffPolicyFactory = new MyBackOffPolicyFactory(); - RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryPolicyFactory, backOffPolicyFactory); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryFactory); FeignLoadBalancer.RibbonResponse ribbonResponse = feignLb.execute(request, null); verify(client, times(2)).execute(any(Request.class), any(Request.Options.class)); - assertEquals(1, backOffPolicyFactory.getCount()); + assertEquals(1, backOffPolicy.getCount()); InputStream inputStream = ribbonResponse.toResponse().body().asInputStream(); byte[] buf = new byte[100]; int read = inputStream.read(buf); Assert.assertThat(new String(buf, 0, read), is("test")); } - class MyBackOffPolicyFactory implements LoadBalancedBackOffPolicyFactory, BackOffPolicy { + class MyBackOffPolicy implements BackOffPolicy { private int count = 0; @@ -464,34 +505,25 @@ public int getCount() { return count; } - @Override - public BackOffPolicy createBackOffPolicy(String service) { - return this; - } } - class MyRetryListeners implements LoadBalancedRetryListenerFactory { + class MyRetryListener implements RetryListener { 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 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() { @@ -499,27 +531,20 @@ public int getOnError() { } } - class MyRetryListenersNotRetry implements LoadBalancedRetryListenerFactory { + class MyRetryListenerNotRetry implements RetryListener { @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) { - - } + public boolean open(RetryContext context, RetryCallback callback) { + return false; + } - @Override - public void onError(RetryContext context, RetryCallback callback, Throwable throwable) { + @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