diff --git a/docs/pom.xml b/docs/pom.xml index 06b7eeb0..1c4173d2 100644 --- a/docs/pom.xml +++ b/docs/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-commons-parent - 1.1.4.BUILD-SNAPSHOT + 1.1.5.BUILD-SNAPSHOT pom Spring Cloud Commons Docs diff --git a/docs/src/main/asciidoc/spring-cloud-commons.adoc b/docs/src/main/asciidoc/spring-cloud-commons.adoc index beb51e12..1571e09c 100644 --- a/docs/src/main/asciidoc/spring-cloud-commons.adoc +++ b/docs/src/main/asciidoc/spring-cloud-commons.adoc @@ -345,6 +345,20 @@ The Ribbon client is used to create a full physical address. See {githubroot}/spring-cloud-netflix/blob/master/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java[RibbonAutoConfiguration] for details of how the `RestTemplate` is set up. +==== Retrying Failed Requests + +A load balanced `RestTemplate` can be configured to retry failed requests. +By default this logic is disabled, you can enable it by setting +`spring.cloud.loadbalancer.retry=true`. The load balanced `RestTemplate` will +honor some of the Ribbon configuration values related to retrying failed requests. +The properties you can use are `client.ribbon.MaxAutoRetries`, +`client.ribbon.MaxAutoRetriesNextServer`, and `client.ribbon.OkToRetryOnAllOperations`. +See the https://github.com/Netflix/ribbon/wiki/Getting-Started#the-properties-file-sample-clientproperties[Ribbon documentation] +for a description of what there properties do. + +NOTE: `client` in the above examples should be replaced with your Ribbon client's +name. + === Multiple RestTemplate objects If you want a `RestTemplate` that is not load balanced, create a `RestTemplate` diff --git a/pom.xml b/pom.xml index e6009a2b..f022dc04 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ 4.0.0 org.springframework.cloud spring-cloud-commons-parent - 1.1.4.BUILD-SNAPSHOT + 1.1.5.BUILD-SNAPSHOT pom Spring Cloud Commons Parent Spring Cloud Commons Parent diff --git a/spring-cloud-commons-dependencies/pom.xml b/spring-cloud-commons-dependencies/pom.xml index c1de226b..1daf3c56 100644 --- a/spring-cloud-commons-dependencies/pom.xml +++ b/spring-cloud-commons-dependencies/pom.xml @@ -9,7 +9,7 @@ spring-cloud-commons-dependencies - 1.1.4.BUILD-SNAPSHOT + 1.1.5.BUILD-SNAPSHOT pom spring-cloud-commons-dependencies Spring Cloud Commons Dependencies diff --git a/spring-cloud-commons/pom.xml b/spring-cloud-commons/pom.xml index 18d442d7..764975ed 100644 --- a/spring-cloud-commons/pom.xml +++ b/spring-cloud-commons/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-commons-parent - 1.1.4.BUILD-SNAPSHOT + 1.1.5.BUILD-SNAPSHOT .. spring-cloud-commons @@ -53,6 +53,16 @@ spring-boot-starter-hateoas true + + org.springframework.boot + spring-boot-starter-aop + true + + + org.springframework.retry + spring-retry + true + com.jayway.jsonpath json-path diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicy.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicy.java new file mode 100644 index 00000000..eafb3d4c --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicy.java @@ -0,0 +1,102 @@ +/* + * Copyright 2013-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.cloud.client.loadbalancer; + +import org.springframework.http.HttpRequest; +import org.springframework.retry.RetryContext; +import org.springframework.retry.RetryPolicy; + +/** + * {@link RetryPolicy} used by the {@link LoadBalancerClient} when retrying failed requests. + * @author Ryan Baxter + */ +public class InterceptorRetryPolicy implements RetryPolicy { + + private HttpRequest request; + private LoadBalancedRetryPolicy policy; + private LoadBalanceChooser loadBalanceChooser; + private String serviceName; + + /** + * Creates a new retry policy. + * @param request the request that will be retried + * @param policy the retry policy from the load balancer + * @param loadBalanceChooser the load balancer client + * @param serviceName the name of the service + */ + public InterceptorRetryPolicy(HttpRequest request, LoadBalancedRetryPolicy policy, + LoadBalanceChooser loadBalanceChooser, String serviceName) { + this.request = request; + this.policy = policy; + this.loadBalanceChooser = loadBalanceChooser; + this.serviceName = serviceName; + } + + @Override + public boolean canRetry(RetryContext context) { + LoadBalancedRetryContext lbContext = (LoadBalancedRetryContext)context; + if(lbContext.getRetryCount() == 0 && lbContext.getServiceInstance() == null) { + //We haven't even tried to make the request yet so return true so we do + lbContext.setServiceInstance(loadBalanceChooser.choose(serviceName)); + return true; + } + return policy.canRetryNextServer(lbContext); + } + + @Override + public RetryContext open(RetryContext parent) { + return new LoadBalancedRetryContext(parent, request); + } + + + @Override + public void close(RetryContext context) { + policy.close((LoadBalancedRetryContext)context); + } + + + @Override + public void registerThrowable(RetryContext context, Throwable throwable) { + LoadBalancedRetryContext lbContext = (LoadBalancedRetryContext) context; + //this is important as it registers the last exception in the context and also increases the retry count + lbContext.registerThrowable(throwable); + //let the policy know about the exception as well + policy.registerThrowable(lbContext, throwable); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + InterceptorRetryPolicy that = (InterceptorRetryPolicy) o; + + if (!request.equals(that.request)) return false; + if (!policy.equals(that.policy)) return false; + if (!loadBalanceChooser.equals(that.loadBalanceChooser)) return false; + return serviceName.equals(that.serviceName); + + } + + @Override + public int hashCode() { + int result = request.hashCode(); + result = 31 * result + policy.hashCode(); + result = 31 * result + loadBalanceChooser.hashCode(); + result = 31 * result + serviceName.hashCode(); + return result; + } +} diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalanceChooser.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalanceChooser.java new file mode 100644 index 00000000..fd3ba114 --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalanceChooser.java @@ -0,0 +1,35 @@ +/* + * Copyright 2013-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.client.loadbalancer; + +import org.springframework.cloud.client.ServiceInstance; + +/** + * Implemented by classes which use a load balancer to choose a server to + * send a request to. + * + * @author Ryan Baxter + */ +public interface LoadBalanceChooser { + + /** + * Choose a ServiceInstance from the LoadBalancer for the specified service + * @param serviceId the service id to look up the LoadBalancer + * @return a ServiceInstance that matches the serviceId + */ + ServiceInstance choose(String serviceId); +} diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancedRetryContext.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancedRetryContext.java new file mode 100644 index 00000000..c0caebc7 --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancedRetryContext.java @@ -0,0 +1,73 @@ +/* + * Copyright 2013-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.cloud.client.loadbalancer; + +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.http.HttpRequest; +import org.springframework.retry.RetryContext; +import org.springframework.retry.context.RetryContextSupport; + +/** + * {@link RetryContext} for load balanced retries. + * @author Ryan Baxter + */ +public class LoadBalancedRetryContext extends RetryContextSupport { + + private HttpRequest request; + private ServiceInstance serviceInstance; + + /** + * Creates a new load balanced context. + * @param parent the parent context + * @param request the request that is being load balanced + */ + public LoadBalancedRetryContext(RetryContext parent, HttpRequest request) { + super(parent); + this.request = request; + } + + /** + * Gets the request that is being load balanced. + * @return the request that is being load balanced + */ + public HttpRequest getRequest() { + return request; + } + + /** + * Sets the request that is being load baalnced. + * @param request the request to load balanced + */ + public void setRequest(HttpRequest request) { + this.request = request; + } + + /** + * Gets the service instance used during the retry. + * @return the service instance used during the retry + */ + public ServiceInstance getServiceInstance() { + return serviceInstance; + } + + /** + * Sets the service instance to use during the retry. + * @param serviceInstance the service instance to use during the retry + */ + public void setServiceInstance(ServiceInstance serviceInstance) { + this.serviceInstance = serviceInstance; + } +} diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancedRetryPolicy.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancedRetryPolicy.java new file mode 100644 index 00000000..02ee113c --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancedRetryPolicy.java @@ -0,0 +1,52 @@ +/* + * Copyright 2013-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.cloud.client.loadbalancer; + +/** + * Retry logic to use for the {@link LoadBalancerClient}. + * @author Ryan Baxter + */ +public interface LoadBalancedRetryPolicy { + + /** + * Return true to retry the failed request on the same server. + * This method may be called more than once when executing a single operation. + * @param context the context for the retry operation + * @return true to retry the failed request on the same server, false otherwise + */ + public boolean canRetrySameServer(LoadBalancedRetryContext context); + + /** + * Return true to retry the failed request on the next server from the load balancer. + * This method may be called more than once when executing a single operation. + * @param context the context for the retry operation + * @return true to retry the failed request on the next server from the load balancer, false otherwise + */ + public boolean canRetryNextServer(LoadBalancedRetryContext context); + + /** + * Called when the retry operation has ended. + * @param context the context for the retry operation + */ + public abstract void close(LoadBalancedRetryContext context); + + /** + * Called when the execution fails. + * @param context the context for the retry operation + * @param throwable the throwable from the failed execution. + */ + public abstract void registerThrowable(LoadBalancedRetryContext context, Throwable throwable); +} diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancedRetryPolicyFactory.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancedRetryPolicyFactory.java new file mode 100644 index 00000000..a2021c7f --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancedRetryPolicyFactory.java @@ -0,0 +1,39 @@ +/* + * Copyright 2013-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.cloud.client.loadbalancer; + +/** + * Responsible for creating the {@link LoadBalancedRetryPolicy}. + * @author Ryan Baxter + */ +public interface LoadBalancedRetryPolicyFactory { + + /** + * Creates a {@link LoadBalancedRetryPolicy}. + * @param serviceId The ID of the service to create the retry policy for. + * @param loadBalanceChooser Used to get the next server from a load balancer + * @return A retry policy for the service. + */ + public LoadBalancedRetryPolicy create(String serviceId, LoadBalanceChooser loadBalanceChooser); + + static class NeverRetryFactory implements LoadBalancedRetryPolicyFactory { + + @Override + public LoadBalancedRetryPolicy create(String serviceId, LoadBalanceChooser loadBalanceChooser) { + return null; + } + } +} 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 88dc2f7d..f2d73c06 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 @@ -25,9 +25,11 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.client.ClientHttpRequestInterceptor; +import org.springframework.retry.support.RetryTemplate; import org.springframework.web.client.RestTemplate; /** @@ -39,6 +41,7 @@ import org.springframework.web.client.RestTemplate; @Configuration @ConditionalOnClass(RestTemplate.class) @ConditionalOnBean(LoadBalancerClient.class) +@EnableConfigurationProperties(LoadBalancerRetryProperties.class) public class LoadBalancerAutoConfiguration { @LoadBalanced @@ -76,9 +79,22 @@ public class LoadBalancerAutoConfiguration { } @Bean - public LoadBalancerInterceptor ribbonInterceptor( - LoadBalancerClient loadBalancerClient) { - return new LoadBalancerInterceptor(loadBalancerClient); + public RetryTemplate retryTemplate() { + RetryTemplate template = new RetryTemplate(); + template.setThrowLastExceptionOnExhausted(true); + return template; } + @Bean + @ConditionalOnMissingBean + public LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory() { + return new LoadBalancedRetryPolicyFactory.NeverRetryFactory(); + } + + @Bean + public LoadBalancerInterceptor ribbonInterceptor( + LoadBalancerClient loadBalancerClient, LoadBalancerRetryProperties properties, + LoadBalancedRetryPolicyFactory lbRetryPolicyFactory) { + return new LoadBalancerInterceptor(loadBalancerClient, retryTemplate(), properties, lbRetryPolicyFactory); + } } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerClient.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerClient.java index 2a13c14c..ed9ad317 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerClient.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerClient.java @@ -16,22 +16,16 @@ package org.springframework.cloud.client.loadbalancer; +import org.springframework.cloud.client.ServiceInstance; + import java.io.IOException; import java.net.URI; -import org.springframework.cloud.client.ServiceInstance; - /** * Represents a client side load balancer * @author Spencer Gibb */ -public interface LoadBalancerClient { - /** - * Choose a ServiceInstance from the LoadBalancer for the specified service - * @param serviceId the service id to look up the LoadBalancer - * @return a ServiceInstance that matches the serviceId - */ - ServiceInstance choose(String serviceId); +public interface LoadBalancerClient extends LoadBalanceChooser { /** * execute request using a ServiceInstance from the LoadBalancer for the specified @@ -44,6 +38,18 @@ public interface LoadBalancerClient { */ T execute(String serviceId, LoadBalancerRequest request) throws IOException; + /** + * execute request using a ServiceInstance from the LoadBalancer for the specified + * service + * @param serviceId the service id to look up the LoadBalancer + * @param serviceInstance the service to execute the request to + * @param request allows implementations to execute pre and post actions such as + * incrementing metrics + * @return the result of the LoadBalancerRequest callback on the selected + * ServiceInstance + */ + T execute(String serviceId, ServiceInstance serviceInstance, LoadBalancerRequest request) throws IOException; + /** * Create a proper URI with a real host and port for systems to utilize. * Some systems use a URI with the logical serivce name as the host, @@ -54,5 +60,4 @@ public interface LoadBalancerClient { * @return a reconstructed URI */ URI reconstructURI(ServiceInstance instance, URI original); - } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerInterceptor.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerInterceptor.java index 5f1877b1..b7846afb 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerInterceptor.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerInterceptor.java @@ -16,44 +16,79 @@ package org.springframework.cloud.client.loadbalancer; -import java.io.IOException; -import java.net.URI; - import org.springframework.cloud.client.ServiceInstance; import org.springframework.http.HttpRequest; import org.springframework.http.client.ClientHttpRequestExecution; import org.springframework.http.client.ClientHttpRequestInterceptor; import org.springframework.http.client.ClientHttpResponse; import org.springframework.http.client.support.HttpRequestWrapper; +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; /** * @author Spencer Gibb * @author Dave Syer + * @author Ryan Baxter */ public class LoadBalancerInterceptor implements ClientHttpRequestInterceptor { private LoadBalancerClient loadBalancer; + private RetryTemplate retryTemplate; + private LoadBalancerRetryProperties lbProperties; + private LoadBalancedRetryPolicyFactory lbRetryPolicyFactory; - public LoadBalancerInterceptor(LoadBalancerClient loadBalancer) { + public LoadBalancerInterceptor(LoadBalancerClient loadBalancer, RetryTemplate retryTemplate, LoadBalancerRetryProperties lbProperties, + LoadBalancedRetryPolicyFactory lbRetryPolicyFactory) { this.loadBalancer = loadBalancer; + this.retryTemplate = retryTemplate; + this.lbProperties = lbProperties; + this.lbRetryPolicyFactory = lbRetryPolicyFactory; } @Override public ClientHttpResponse intercept(final HttpRequest request, final byte[] body, final ClientHttpRequestExecution execution) throws IOException { final URI originalUri = request.getURI(); - String serviceName = originalUri.getHost(); - return this.loadBalancer.execute(serviceName, - new LoadBalancerRequest() { - + final String serviceName = originalUri.getHost(); + LoadBalancedRetryPolicy retryPolicy = lbRetryPolicyFactory.create(serviceName, + loadBalancer); + retryTemplate.setRetryPolicy( + !lbProperties.isEnabled() || retryPolicy == null ? new NeverRetryPolicy() + : new InterceptorRetryPolicy(request, retryPolicy, loadBalancer, + serviceName)); + return retryTemplate + .execute(new RetryCallback() { @Override - public ClientHttpResponse apply(final ServiceInstance instance) - throws Exception { - HttpRequest serviceRequest = new ServiceRequestWrapper(request, - instance); - return execution.execute(serviceRequest, body); - } + public ClientHttpResponse doWithRetry(RetryContext context) + throws IOException { + ServiceInstance serviceInstance = null; + if (context instanceof LoadBalancedRetryContext) { + LoadBalancedRetryContext lbContext = (LoadBalancedRetryContext) context; + serviceInstance = lbContext.getServiceInstance(); + } + if (serviceInstance == null) { + serviceInstance = loadBalancer.choose(serviceName); + } + return LoadBalancerInterceptor.this.loadBalancer.execute( + serviceName, serviceInstance, + new LoadBalancerRequest() { + @Override + public ClientHttpResponse apply( + final ServiceInstance instance) + throws Exception { + HttpRequest serviceRequest = new ServiceRequestWrapper( + request, instance); + return execution.execute(serviceRequest, body); + } + + }); + } }); } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRetryProperties.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRetryProperties.java new file mode 100644 index 00000000..634249c0 --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRetryProperties.java @@ -0,0 +1,43 @@ +/* + * Copyright 2013-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.cloud.client.loadbalancer; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +/** + * Configuration properties for the {@link LoadBalancerClient}. + * @author Ryan Baxter + */ +@ConfigurationProperties("spring.cloud.loadbalancer.retry") +public class LoadBalancerRetryProperties { + private boolean enabled = false; + + /** + * Returns true if the load balancer should retry failed requests. + * @return true if the load balancer should retry failed request, false otherwise. + */ + public boolean isEnabled() { + return enabled; + } + + /** + * Sets whether the load balancer should retry failed request. + * @param enabled whether the load balancer should retry failed requests + */ + public void setEnabled(boolean enabled) { + this.enabled = enabled; + } +} diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicyTest.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicyTest.java new file mode 100644 index 00000000..82ef50cc --- /dev/null +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicyTest.java @@ -0,0 +1,115 @@ +package org.springframework.cloud.client.loadbalancer; + +import org.hamcrest.core.IsInstanceOf; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.http.HttpRequest; +import org.springframework.retry.RetryContext; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * @author Ryan Baxter + */ +@RunWith(MockitoJUnitRunner.class) +public class InterceptorRetryPolicyTest { + + private HttpRequest request; + private LoadBalancedRetryPolicy policy; + private LoadBalanceChooser loadBalanceChooser; + private String serviceName; + + @Before + public void setup() { + request = mock(HttpRequest.class); + policy = mock(LoadBalancedRetryPolicy.class); + loadBalanceChooser = mock(LoadBalanceChooser.class); + serviceName = "foo"; + } + + @After + public void teardown() { + request = null; + policy = null; + loadBalanceChooser = null; + serviceName = null; + } + + @Test + public void canRetryBeforeExecution() throws Exception { + InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(request, policy, loadBalanceChooser, serviceName); + LoadBalancedRetryContext context = mock(LoadBalancedRetryContext.class); + when(context.getRetryCount()).thenReturn(0); + ServiceInstance serviceInstance = mock(ServiceInstance.class); + when(loadBalanceChooser.choose(eq(serviceName))).thenReturn(serviceInstance); + assertThat(interceptorRetryPolicy.canRetry(context), is(true)); + verify(context, times(1)).setServiceInstance(eq(serviceInstance)); + + } + + @Test + public void canRetryNextServer() throws Exception { + InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(request, policy, loadBalanceChooser, serviceName); + LoadBalancedRetryContext context = mock(LoadBalancedRetryContext.class); + when(context.getRetryCount()).thenReturn(1); + when(policy.canRetryNextServer(eq(context))).thenReturn(true); + assertThat(interceptorRetryPolicy.canRetry(context), is(true)); + } + + @Test + public void cannotRetry() throws Exception { + InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(request, policy, loadBalanceChooser, serviceName); + LoadBalancedRetryContext context = mock(LoadBalancedRetryContext.class); + when(context.getRetryCount()).thenReturn(1); + assertThat(interceptorRetryPolicy.canRetry(context), is(false)); + } + + @Test + public void open() throws Exception { + InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(request, policy, loadBalanceChooser, serviceName); + RetryContext context = interceptorRetryPolicy.open(null); + assertThat(context, IsInstanceOf.instanceOf(LoadBalancedRetryContext.class)); + } + + @Test + public void close() throws Exception { + InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(request, policy, loadBalanceChooser, serviceName); + LoadBalancedRetryContext context = mock(LoadBalancedRetryContext.class); + interceptorRetryPolicy.close(context); + verify(policy, times(1)).close(eq(context)); + } + + @Test + public void registerThrowable() throws Exception { + InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(request, policy, loadBalanceChooser, serviceName); + LoadBalancedRetryContext context = mock(LoadBalancedRetryContext.class); + Throwable thrown = new Exception(); + when(policy.canRetryNextServer(eq(context))).thenReturn(true); + when(policy.canRetrySameServer(eq(context))).thenReturn(false); + ServiceInstance serviceInstance = mock(ServiceInstance.class); + when(loadBalanceChooser.choose(eq(serviceName))).thenReturn(serviceInstance); + interceptorRetryPolicy.registerThrowable(context, thrown); + verify(context, times(1)).registerThrowable(eq(thrown)); + verify(policy, times(1)).registerThrowable(eq(context), eq(thrown)); + } + + @Test + public void equals() throws Exception { + InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(request, policy, loadBalanceChooser, serviceName); + assertThat(interceptorRetryPolicy.equals(null), is(false)); + assertThat(interceptorRetryPolicy.equals(new Object()), is(false)); + assertThat(interceptorRetryPolicy.equals(interceptorRetryPolicy), is(true)); + assertThat(interceptorRetryPolicy.equals(new InterceptorRetryPolicy(request, policy, loadBalanceChooser, serviceName)), is(true)); + } + +} \ No newline at end of file diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancedRetryContextTest.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancedRetryContextTest.java new file mode 100644 index 00000000..6e7f9aa6 --- /dev/null +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancedRetryContextTest.java @@ -0,0 +1,58 @@ +package org.springframework.cloud.client.loadbalancer; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.http.HttpRequest; +import org.springframework.retry.RetryContext; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.mockito.Mockito.mock; + +/** + * @author Ryan Baxter + */ +@RunWith(MockitoJUnitRunner.class) +public class LoadBalancedRetryContextTest { + + private RetryContext context; + private HttpRequest request; + + @Before + public void setUp() throws Exception { + context = mock(RetryContext.class); + request = mock(HttpRequest.class); + } + + @After + public void tearDown() throws Exception { + context = null; + request = null; + } + + @Test + public void getRequest() throws Exception { + LoadBalancedRetryContext lbContext = new LoadBalancedRetryContext(context, request); + assertThat(lbContext.getRequest(), is(request)); + } + + @Test + public void setRequest() throws Exception { + LoadBalancedRetryContext lbContext = new LoadBalancedRetryContext(context, request); + HttpRequest newRequest = mock(HttpRequest.class); + lbContext.setRequest(newRequest); + assertThat(lbContext.getRequest(), is(newRequest)); + } + + @Test + public void getServiceInstance() throws Exception { + LoadBalancedRetryContext lbContext = new LoadBalancedRetryContext(context, request); + ServiceInstance serviceInstance = mock(ServiceInstance.class); + lbContext.setServiceInstance(serviceInstance); + assertThat(lbContext.getServiceInstance(), is(serviceInstance)); + } +} \ No newline at end of file diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfigurationTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfigurationTests.java index 3606fa0d..b1ecdf07 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfigurationTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfigurationTests.java @@ -1,5 +1,6 @@ package org.springframework.cloud.client.loadbalancer; +import java.io.IOException; import java.net.URI; import java.util.Collection; import java.util.List; @@ -16,6 +17,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Primary; import org.springframework.http.client.ClientHttpRequestInterceptor; +import org.springframework.retry.RetryPolicy; import org.springframework.web.client.RestTemplate; import static org.hamcrest.Matchers.empty; @@ -92,6 +94,9 @@ public class LoadBalancerAutoConfigurationTests { return new NoopLoadBalancerClient(); } + @Bean + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory() { return new LoadBalancedRetryPolicyFactory.NeverRetryFactory();} + } @Configuration @@ -114,6 +119,9 @@ public class LoadBalancerAutoConfigurationTests { return new NoopLoadBalancerClient(); } + @Bean + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory() { return new LoadBalancedRetryPolicyFactory.NeverRetryFactory();} + @Configuration protected static class Two { @Autowired @@ -141,6 +149,12 @@ public class LoadBalancerAutoConfigurationTests { return request.apply(choose(serviceId)); } + @Override + @SneakyThrows + public T execute(String serviceId, ServiceInstance serviceInstance, LoadBalancerRequest request) throws IOException { + return request.apply(choose(serviceId)); + } + @Override public URI reconstructURI(ServiceInstance instance, URI original) { return DefaultServiceInstance.getUri(instance); diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerInterceptorTest.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerInterceptorTest.java new file mode 100644 index 00000000..3ad2a90f --- /dev/null +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerInterceptorTest.java @@ -0,0 +1,153 @@ +package org.springframework.cloud.client.loadbalancer; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.http.HttpRequest; +import org.springframework.http.HttpStatus; +import org.springframework.http.client.ClientHttpRequestExecution; +import org.springframework.http.client.ClientHttpResponse; +import org.springframework.mock.http.client.MockClientHttpResponse; +import org.springframework.retry.policy.NeverRetryPolicy; +import org.springframework.retry.support.RetryTemplate; + +import java.io.IOException; +import java.net.URI; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * @author Ryan Baxter + */ +@RunWith(MockitoJUnitRunner.class) +public class LoadBalancerInterceptorTest { + + private LoadBalancerClient client; + private RetryTemplate retryTemplate; + private LoadBalancerRetryProperties lbProperties; + + @Before + public void setUp() throws Exception { + client = mock(LoadBalancerClient.class); + retryTemplate = spy(new RetryTemplate()); + lbProperties = new LoadBalancerRetryProperties(); + + } + + @After + public void tearDown() throws Exception { + client = null; + retryTemplate = null; + lbProperties = null; + } + + @Test(expected = IOException.class) + public void interceptDisableRetry() throws Throwable { + HttpRequest request = mock(HttpRequest.class); + when(request.getURI()).thenReturn(new URI("http://foo")); + ClientHttpResponse clientHttpResponse = new MockClientHttpResponse(new byte[]{}, HttpStatus.OK); + LoadBalancedRetryPolicyFactory lbRetryPolicyFactory = mock(LoadBalancedRetryPolicyFactory.class); + when(lbRetryPolicyFactory.create(eq("foo"), any(LoadBalanceChooser.class))).thenReturn(null); + 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()); + lbProperties.setEnabled(false); + LoadBalancerInterceptor interceptor = new LoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory); + byte[] body = new byte[]{}; + ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); + interceptor.intercept(request, body, execution); + verify(retryTemplate, times(1)).setRetryPolicy(any(NeverRetryPolicy.class)); + } + + @Test + public void interceptNeverRetry() throws Throwable { + HttpRequest request = mock(HttpRequest.class); + when(request.getURI()).thenReturn(new URI("http://foo")); + ClientHttpResponse clientHttpResponse = new MockClientHttpResponse(new byte[]{}, HttpStatus.OK); + LoadBalancedRetryPolicyFactory lbRetryPolicyFactory = mock(LoadBalancedRetryPolicyFactory.class); + when(lbRetryPolicyFactory.create(eq("foo"), any(LoadBalanceChooser.class))).thenReturn(null); + ServiceInstance serviceInstance = mock(ServiceInstance.class); + when(client.choose(eq("foo"))).thenReturn(serviceInstance); + when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenReturn(clientHttpResponse); + lbProperties.setEnabled(true); + LoadBalancerInterceptor interceptor = new LoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory); + byte[] body = new byte[]{}; + ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); + interceptor.intercept(request, body, execution); + verify(retryTemplate, times(1)).setRetryPolicy(any(NeverRetryPolicy.class)); + } + + @Test + public void interceptSuccess() throws Throwable { + HttpRequest request = mock(HttpRequest.class); + when(request.getURI()).thenReturn(new URI("http://foo")); + ClientHttpResponse clientHttpResponse = new MockClientHttpResponse(new byte[]{}, HttpStatus.OK); + LoadBalancedRetryPolicy policy = mock(LoadBalancedRetryPolicy.class); + InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(request, policy, client,"foo"); + LoadBalancedRetryPolicyFactory lbRetryPolicyFactory = mock(LoadBalancedRetryPolicyFactory.class); + when(lbRetryPolicyFactory.create(eq("foo"), any(LoadBalanceChooser.class))).thenReturn(policy); + ServiceInstance serviceInstance = mock(ServiceInstance.class); + when(client.choose(eq("foo"))).thenReturn(serviceInstance); + when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenReturn(clientHttpResponse); + lbProperties.setEnabled(true); + LoadBalancerInterceptor interceptor = new LoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory); + 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)); + } + + @Test + public void interceptRetry() throws Throwable { + HttpRequest request = mock(HttpRequest.class); + when(request.getURI()).thenReturn(new URI("http://foo")); + ClientHttpResponse clientHttpResponse = new MockClientHttpResponse(new byte[]{}, HttpStatus.OK); + LoadBalancedRetryPolicy policy = mock(LoadBalancedRetryPolicy.class); + when(policy.canRetryNextServer(any(LoadBalancedRetryContext.class))).thenReturn(true); + LoadBalancedRetryPolicyFactory lbRetryPolicyFactory = mock(LoadBalancedRetryPolicyFactory.class); + when(lbRetryPolicyFactory.create(eq("foo"), any(LoadBalanceChooser.class))).thenReturn(policy); + 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); + lbProperties.setEnabled(true); + LoadBalancerInterceptor interceptor = new LoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory); + 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)); + } + + @Test(expected = IOException.class) + public void interceptFailedRetry() throws Exception { + HttpRequest request = mock(HttpRequest.class); + when(request.getURI()).thenReturn(new URI("http://foo")); + ClientHttpResponse clientHttpResponse = new MockClientHttpResponse(new byte[]{}, HttpStatus.OK); + LoadBalancedRetryPolicy policy = mock(LoadBalancedRetryPolicy.class); + when(policy.canRetrySameServer(any(LoadBalancedRetryContext.class))).thenReturn(false); + when(policy.canRetryNextServer(any(LoadBalancedRetryContext.class))).thenReturn(false); + LoadBalancedRetryPolicyFactory lbRetryPolicyFactory = mock(LoadBalancedRetryPolicyFactory.class); + when(lbRetryPolicyFactory.create(eq("foo"), any(LoadBalanceChooser.class))).thenReturn(policy); + 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); + lbProperties.setEnabled(true); + LoadBalancerInterceptor interceptor = new LoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory); + byte[] body = new byte[]{}; + ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); + ClientHttpResponse rsp = interceptor.intercept(request, body, execution); + } +} \ No newline at end of file diff --git a/spring-cloud-context/pom.xml b/spring-cloud-context/pom.xml index 102b83e2..82e4e50a 100644 --- a/spring-cloud-context/pom.xml +++ b/spring-cloud-context/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-commons-parent - 1.1.4.BUILD-SNAPSHOT + 1.1.5.BUILD-SNAPSHOT .. spring-cloud-context diff --git a/spring-cloud-starter/pom.xml b/spring-cloud-starter/pom.xml index 954c1576..7aa7b7c7 100644 --- a/spring-cloud-starter/pom.xml +++ b/spring-cloud-starter/pom.xml @@ -5,7 +5,7 @@ org.springframework.cloud spring-cloud-commons-parent - 1.1.4.BUILD-SNAPSHOT + 1.1.5.BUILD-SNAPSHOT spring-cloud-starter spring-cloud-starter