From 5d5a5edc3c2a10b0890c5617802cad94e5cd6836 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Wed, 5 Jul 2017 11:32:52 -0400 Subject: [PATCH 1/2] Fixes #224. Create a new RetryTemplate for each request. --- .../LoadBalancerAutoConfiguration.java | 4 ++-- .../RetryLoadBalancerInterceptor.java | 21 ++++++++++++++---- .../RetryLoadBalancerInterceptorTest.java | 22 ++++++------------- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java index 02257ac0..9f4cb42d 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java @@ -126,8 +126,8 @@ public class LoadBalancerAutoConfiguration { public RetryLoadBalancerInterceptor ribbonInterceptor( LoadBalancerClient loadBalancerClient, LoadBalancerRetryProperties properties, LoadBalancedRetryPolicyFactory lbRetryPolicyFactory, - LoadBalancerRequestFactory requestFactory, RetryTemplate retryTemplate) { - return new RetryLoadBalancerInterceptor(loadBalancerClient, retryTemplate, properties, + LoadBalancerRequestFactory requestFactory) { + return new RetryLoadBalancerInterceptor(loadBalancerClient, properties, lbRetryPolicyFactory, requestFactory); } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java index d632b471..cabf8d45 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java @@ -42,7 +42,7 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto private LoadBalancerRetryProperties lbProperties; private LoadBalancerRequestFactory requestFactory; - + @Deprecated public RetryLoadBalancerInterceptor(LoadBalancerClient loadBalancer, RetryTemplate retryTemplate, LoadBalancerRetryProperties lbProperties, LoadBalancedRetryPolicyFactory lbRetryPolicyFactory, @@ -53,7 +53,18 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto this.lbProperties = lbProperties; this.requestFactory = requestFactory; } - + + public RetryLoadBalancerInterceptor(LoadBalancerClient loadBalancer, + LoadBalancerRetryProperties lbProperties, + LoadBalancedRetryPolicyFactory lbRetryPolicyFactory, + LoadBalancerRequestFactory requestFactory) { + this.loadBalancer = loadBalancer; + this.lbRetryPolicyFactory = lbRetryPolicyFactory; + this.lbProperties = lbProperties; + this.requestFactory = requestFactory; + } + + @Deprecated public RetryLoadBalancerInterceptor(LoadBalancerClient loadBalancer, RetryTemplate retryTemplate, LoadBalancerRetryProperties lbProperties, LoadBalancedRetryPolicyFactory lbRetryPolicyFactory) { @@ -70,11 +81,13 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto Assert.state(serviceName != null, "Request URI does not contain a valid hostname: " + originalUri); final LoadBalancedRetryPolicy retryPolicy = lbRetryPolicyFactory.create(serviceName, loadBalancer); - retryTemplate.setRetryPolicy( + RetryTemplate template = this.retryTemplate == null ? new RetryTemplate() : this.retryTemplate; + template.setThrowLastExceptionOnExhausted(true); + template.setRetryPolicy( !lbProperties.isEnabled() || retryPolicy == null ? new NeverRetryPolicy() : new InterceptorRetryPolicy(request, retryPolicy, loadBalancer, serviceName)); - return retryTemplate + return template .execute(new RetryCallback() { @Override public ClientHttpResponse doWithRetry(RetryContext context) diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java index aaeaa768..307ee8d6 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java @@ -33,14 +33,12 @@ import static org.mockito.Mockito.when; public class RetryLoadBalancerInterceptorTest { private LoadBalancerClient client; - private RetryTemplate retryTemplate; private LoadBalancerRetryProperties lbProperties; private LoadBalancerRequestFactory lbRequestFactory; @Before public void setUp() throws Exception { client = mock(LoadBalancerClient.class); - retryTemplate = spy(new RetryTemplate()); lbProperties = new LoadBalancerRetryProperties(); lbRequestFactory = mock(LoadBalancerRequestFactory.class); @@ -49,7 +47,6 @@ public class RetryLoadBalancerInterceptorTest { @After public void tearDown() throws Exception { client = null; - retryTemplate = null; lbProperties = null; } @@ -64,11 +61,10 @@ public class RetryLoadBalancerInterceptorTest { when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenThrow(new IOException()); lbProperties.setEnabled(false); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); interceptor.intercept(request, body, execution); - verify(retryTemplate, times(1)).setRetryPolicy(any(NeverRetryPolicy.class)); verify(lbRequestFactory).createRequest(request, body, execution); } @@ -85,7 +81,7 @@ public class RetryLoadBalancerInterceptorTest { when(client.choose(eq("foo_underscore"))).thenReturn(serviceInstance); when(client.execute(eq("foo_underscore"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenReturn(clientHttpResponse); lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); @@ -102,11 +98,10 @@ public class RetryLoadBalancerInterceptorTest { when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenReturn(clientHttpResponse); lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); interceptor.intercept(request, body, execution); - verify(retryTemplate, times(1)).setRetryPolicy(any(NeverRetryPolicy.class)); verify(lbRequestFactory).createRequest(request, body, execution); } @@ -123,12 +118,11 @@ public class RetryLoadBalancerInterceptorTest { when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenReturn(clientHttpResponse); lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); assertThat(rsp, is(clientHttpResponse)); - verify(retryTemplate, times(1)).setRetryPolicy(eq(interceptorRetryPolicy)); verify(lbRequestFactory).createRequest(request, body, execution); } @@ -149,13 +143,12 @@ public class RetryLoadBalancerInterceptorTest { when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))). thenReturn(clientHttpResponseNotFound).thenReturn(clientHttpResponseOk); lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); verify(client, times(2)).execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class)); assertThat(rsp, is(clientHttpResponseOk)); - verify(retryTemplate, times(1)).setRetryPolicy(eq(interceptorRetryPolicy)); verify(lbRequestFactory, times(2)).createRequest(request, body, execution); } @@ -172,13 +165,12 @@ public class RetryLoadBalancerInterceptorTest { when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenThrow(new IOException()).thenReturn(clientHttpResponse); lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); verify(client, times(2)).execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class)); assertThat(rsp, is(clientHttpResponse)); - verify(retryTemplate, times(1)).setRetryPolicy(any(InterceptorRetryPolicy.class)); verify(lbRequestFactory, times(2)).createRequest(request, body, execution); } @@ -196,7 +188,7 @@ public class RetryLoadBalancerInterceptorTest { when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenThrow(new IOException()).thenReturn(clientHttpResponse); lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); From 2661b343de4c147207d02a8bbad0574068d2e442 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Sun, 9 Jul 2017 13:47:17 -0400 Subject: [PATCH 2/2] Eliminated deprecated constructors --- .../RetryLoadBalancerInterceptor.java | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java index cabf8d45..0bab8127 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java @@ -42,17 +42,6 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto private LoadBalancerRetryProperties lbProperties; private LoadBalancerRequestFactory requestFactory; - @Deprecated - public RetryLoadBalancerInterceptor(LoadBalancerClient loadBalancer, RetryTemplate retryTemplate, - LoadBalancerRetryProperties lbProperties, - LoadBalancedRetryPolicyFactory lbRetryPolicyFactory, - LoadBalancerRequestFactory requestFactory) { - this.loadBalancer = loadBalancer; - this.lbRetryPolicyFactory = lbRetryPolicyFactory; - this.retryTemplate = retryTemplate; - this.lbProperties = lbProperties; - this.requestFactory = requestFactory; - } public RetryLoadBalancerInterceptor(LoadBalancerClient loadBalancer, LoadBalancerRetryProperties lbProperties, @@ -64,15 +53,6 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto this.requestFactory = requestFactory; } - @Deprecated - public RetryLoadBalancerInterceptor(LoadBalancerClient loadBalancer, RetryTemplate retryTemplate, - LoadBalancerRetryProperties lbProperties, - LoadBalancedRetryPolicyFactory lbRetryPolicyFactory) { - // for backwards compatibility - this(loadBalancer, retryTemplate, lbProperties, lbRetryPolicyFactory, - new LoadBalancerRequestFactory(loadBalancer)); - } - @Override public ClientHttpResponse intercept(final HttpRequest request, final byte[] body, final ClientHttpRequestExecution execution) throws IOException {