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..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 @@ -43,24 +43,15 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto private LoadBalancerRequestFactory requestFactory; - public RetryLoadBalancerInterceptor(LoadBalancerClient loadBalancer, RetryTemplate retryTemplate, + public RetryLoadBalancerInterceptor(LoadBalancerClient loadBalancer, 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, 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, @@ -70,11 +61,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 3601834e..025d8654 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 @@ -34,14 +34,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); @@ -50,7 +48,6 @@ public class RetryLoadBalancerInterceptorTest { @After public void tearDown() throws Exception { client = null; - retryTemplate = null; lbProperties = null; } @@ -64,14 +61,13 @@ 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); when(this.lbRequestFactory.createRequest(any(), any(), any())).thenReturn(mock(LoadBalancerRequest.class)); interceptor.intercept(request, body, execution); - verify(retryTemplate, times(1)).setRetryPolicy(any(NeverRetryPolicy.class)); verify(lbRequestFactory).createRequest(request, body, execution); } @@ -81,7 +77,7 @@ public class RetryLoadBalancerInterceptorTest { when(request.getURI()).thenReturn(new URI("http://foo_underscore")); LoadBalancedRetryPolicyFactory lbRetryPolicyFactory = mock(LoadBalancedRetryPolicyFactory.class); 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); @@ -96,11 +92,10 @@ public class RetryLoadBalancerInterceptorTest { ServiceInstance serviceInstance = mock(ServiceInstance.class); when(client.choose(eq("foo"))).thenReturn(serviceInstance); 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); } @@ -116,16 +111,13 @@ public class RetryLoadBalancerInterceptorTest { ServiceInstance serviceInstance = mock(ServiceInstance.class); when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenReturn(clientHttpResponse); - when(this.lbRequestFactory.createRequest(any(), any(), any())).thenReturn(mock(LoadBalancerRequest.class)); - - lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + lbProperties.setEnabled(true); + 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); } @@ -146,13 +138,12 @@ public class RetryLoadBalancerInterceptorTest { when(client.execute(eq("foo"), eq(serviceInstance), nullable(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), nullable(LoadBalancerRequest.class)); assertThat(rsp, is(clientHttpResponseOk)); - verify(retryTemplate, times(1)).setRetryPolicy(eq(interceptorRetryPolicy)); verify(lbRequestFactory, times(2)).createRequest(request, body, execution); } @@ -168,17 +159,14 @@ public class RetryLoadBalancerInterceptorTest { ServiceInstance serviceInstance = mock(ServiceInstance.class); when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenThrow(new IOException()).thenReturn(clientHttpResponse); - when(this.lbRequestFactory.createRequest(any(), any(), any())).thenReturn(mock(LoadBalancerRequest.class)); - - lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + lbProperties.setEnabled(true); + 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); } @@ -194,11 +182,9 @@ public class RetryLoadBalancerInterceptorTest { ServiceInstance serviceInstance = mock(ServiceInstance.class); when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenThrow(new IOException()).thenReturn(clientHttpResponse); - when(this.lbRequestFactory.createRequest(any(), any(), any())).thenReturn(mock(LoadBalancerRequest.class)); - - lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + lbProperties.setEnabled(true); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution);