Add retry support for blocking LoadBalancer (#832)

* Add LoadBalancerProperties and BlockingLoadBalancedRetryPolicy. Add spring-retry dependency in LoadBalancer.

* Add BlockingLoadBalancedRetryFactory

* Move retry properties to LoadBalancerRetryProperties.

* Refactor and remove deprecations, fix checkstyle.

* Add BlockingLoadBalancedRetryPolicy to autoconfiguration. Set default retryableStatusCode prop. Fix javadoc.

* Allow using @Order on LoadBalancedRetryFactory beans.

* Add tests. Reformat tests. Add explanatory comments.

* Add documentation.

* Fix javadoc.

* Fix docs after review.

* Change field name.

spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryFactory.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2012-2020 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
+ *
+ *      https://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.loadbalancer.blocking.retry;
+
+import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory;
+import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy;
+import org.springframework.cloud.client.loadbalancer.LoadBalancerRetryProperties;
+import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser;
+import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient;
+
+/**
+ * An implementation of {@link LoadBalancedRetryFactory} for
+ * {@link BlockingLoadBalancerClient}.
+ *
+ * @author Olga Maciaszek-Sharma
+ * @since 2.2.6
+ */
+public class BlockingLoadBalancedRetryFactory implements LoadBalancedRetryFactory {
+
+	private final LoadBalancerRetryProperties retryProperties;
+
+	public BlockingLoadBalancedRetryFactory(LoadBalancerRetryProperties retryProperties) {
+		this.retryProperties = retryProperties;
+	}
+
+	@Override
+	public LoadBalancedRetryPolicy createRetryPolicy(String serviceId,
+			ServiceInstanceChooser serviceInstanceChooser) {
+		return new BlockingLoadBalancedRetryPolicy(serviceId, serviceInstanceChooser,
+				retryProperties);
+	}
+
+}

spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryPolicy.java

@@ -0,0 +1,100 @@
+/*
+ * Copyright 2012-2020 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
+ *
+ *      https://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.loadbalancer.blocking.retry;
+
+import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext;
+import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy;
+import org.springframework.cloud.client.loadbalancer.LoadBalancerRetryProperties;
+import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser;
+import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient;
+import org.springframework.http.HttpMethod;
+
+/**
+ * A {@link LoadBalancedRetryPolicy} implementation for
+ * {@link BlockingLoadBalancerClient}. Based on <code>RibbonLoadBalancedRetryPolicy</code>
+ * to achieve feature-parity.
+ *
+ * @author Olga Maciaszek-Sharma
+ * @since 2.2.6
+ */
+public class BlockingLoadBalancedRetryPolicy implements LoadBalancedRetryPolicy {
+
+	private final LoadBalancerRetryProperties retryProperties;
+
+	private final ServiceInstanceChooser serviceInstanceChooser;
+
+	private final String serviceId;
+
+	private int sameServerCount = 0;
+
+	private int nextServerCount = 0;
+
+	public BlockingLoadBalancedRetryPolicy(String serviceId,
+			ServiceInstanceChooser serviceInstanceChooser,
+			LoadBalancerRetryProperties retryProperties) {
+		this.serviceId = serviceId;
+		this.serviceInstanceChooser = serviceInstanceChooser;
+		this.retryProperties = retryProperties;
+	}
+
+	public boolean canRetry(LoadBalancedRetryContext context) {
+		HttpMethod method = context.getRequest().getMethod();
+		return HttpMethod.GET.equals(method) || retryProperties.isRetryOnAllOperations();
+	}
+
+	@Override
+	public boolean canRetrySameServer(LoadBalancedRetryContext context) {
+		return sameServerCount < retryProperties.getMaxRetriesOnSameServiceInstance()
+				&& canRetry(context);
+	}
+
+	@Override
+	public boolean canRetryNextServer(LoadBalancedRetryContext context) {
+		// After the failure, we increment first and then check, hence the equality check
+		return nextServerCount <= retryProperties.getMaxRetriesOnNextServiceInstance()
+				&& canRetry(context);
+	}
+
+	@Override
+	public void close(LoadBalancedRetryContext context) {
+
+	}
+
+	@Override
+	public void registerThrowable(LoadBalancedRetryContext context, Throwable throwable) {
+		if (!canRetrySameServer(context) && canRetry(context)) {
+			// Reset same server since we are moving to a new ServiceInstance
+			sameServerCount = 0;
+			nextServerCount++;
+			if (!canRetryNextServer(context)) {
+				context.setExhaustedOnly();
+			}
+			else {
+				context.setServiceInstance(serviceInstanceChooser.choose(serviceId));
+			}
+		}
+		else {
+			sameServerCount++;
+		}
+	}
+
+	@Override
+	public boolean retryableStatusCode(int statusCode) {
+		return retryProperties.getRetryableStatusCodes().contains(statusCode);
+	}
+
+}

spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java

@@ -26,15 +26,21 @@ import org.springframework.boot.autoconfigure.AutoConfigureBefore;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
+import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.cloud.client.loadbalancer.AsyncLoadBalancerAutoConfiguration;
+import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory;
+import org.springframework.cloud.client.loadbalancer.LoadBalancerRetryProperties;
 import org.springframework.cloud.client.loadbalancer.reactive.OnNoRibbonDefaultCondition;
 import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClients;
 import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient;
+import org.springframework.cloud.loadbalancer.blocking.retry.BlockingLoadBalancedRetryFactory;
 import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Conditional;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Primary;
+import org.springframework.core.annotation.Order;
+import org.springframework.retry.support.RetryTemplate;
 import org.springframework.web.client.RestTemplate;
 
 /**
@@ -73,6 +79,22 @@ public class BlockingLoadBalancerClientAutoConfiguration {
 			return new BlockingLoadBalancerClient(loadBalancerClientFactory);
 		}
 
+		@Configuration
+		@ConditionalOnClass(RetryTemplate.class)
+		@EnableConfigurationProperties(LoadBalancerRetryProperties.class)
+		protected static class BlockingLoadBalancerRetryConfig {
+
+			@Bean
+			// Allow users to override the factory while avoiding loading
+			// RibbonLoadBalancedRetryFactory.
+			@Order(1000)
+			LoadBalancedRetryFactory loadBalancedRetryFactory(
+					LoadBalancerRetryProperties retryProperties) {
+				return new BlockingLoadBalancedRetryFactory(retryProperties);
+			}
+
+		}
+
 	}
 
 	static class BlockingLoadBalancerClientRibbonWarnLogger {

spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryPolicyTests.java

@@ -0,0 +1,127 @@
+/*
+ * Copyright 2013-2020 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
+ *
+ *      https://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.loadbalancer.blocking.retry;
+
+import java.util.Arrays;
+import java.util.HashSet;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext;
+import org.springframework.cloud.client.loadbalancer.LoadBalancerRetryProperties;
+import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient;
+import org.springframework.http.HttpMethod;
+import org.springframework.http.HttpRequest;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * Tests for {@link BlockingLoadBalancedRetryPolicy}.
+ *
+ * @author Olga Maciaszek-Sharma
+ */
+class BlockingLoadBalancedRetryPolicyTests {
+
+	private final BlockingLoadBalancerClient loadBalancerClient = mock(
+			BlockingLoadBalancerClient.class);
+
+	private final HttpRequest httpRequest = mock(HttpRequest.class);
+
+	private final LoadBalancedRetryContext context = mock(LoadBalancedRetryContext.class);
+
+	private final LoadBalancerRetryProperties retryProperties = new LoadBalancerRetryProperties();
+
+	private final UnsupportedOperationException exception = new UnsupportedOperationException();
+
+	@BeforeEach
+	void setUp() {
+		when(httpRequest.getMethod()).thenReturn(HttpMethod.GET);
+		when(context.getRequest()).thenReturn(httpRequest);
+	}
+
+	@Test
+	void shouldExecuteIndicatedNumberOfSameAndNextInstanceRetriesAndCloseRetryContext() {
+		retryProperties.setMaxRetriesOnSameServiceInstance(1);
+		BlockingLoadBalancedRetryPolicy retryPolicy = getRetryPolicy(retryProperties);
+
+		assertThat(retryPolicy.canRetrySameServer(context)).isTrue();
+		assertThat(retryPolicy.canRetryNextServer(context)).isTrue();
+
+		retryPolicy.registerThrowable(context, exception);
+		assertThat(retryPolicy.canRetrySameServer(context)).isFalse();
+		assertThat(retryPolicy.canRetryNextServer(context)).isTrue();
+
+		retryPolicy.registerThrowable(context, exception);
+		assertThat(retryPolicy.canRetrySameServer(context)).isTrue();
+		assertThat(retryPolicy.canRetryNextServer(context)).isTrue();
+
+		retryPolicy.registerThrowable(context, exception);
+		assertThat(retryPolicy.canRetrySameServer(context)).isFalse();
+		assertThat(retryPolicy.canRetryNextServer(context)).isTrue();
+
+		retryPolicy.registerThrowable(context, exception);
+		verify(context).setExhaustedOnly();
+		verify(context).setServiceInstance(any());
+		assertThat(retryPolicy.canRetrySameServer(context)).isTrue();
+		assertThat(retryPolicy.canRetryNextServer(context)).isFalse();
+	}
+
+	@Test
+	void shouldNotRetryWhenMethodNotGet() {
+		when(httpRequest.getMethod()).thenReturn(HttpMethod.POST);
+		when(context.getRequest()).thenReturn(httpRequest);
+		BlockingLoadBalancedRetryPolicy retryPolicy = getRetryPolicy(retryProperties);
+
+		boolean canRetry = retryPolicy.canRetry(context);
+
+		assertThat(canRetry).isFalse();
+	}
+
+	@Test
+	void shouldRetryOnPostWhenEnabled() {
+		when(httpRequest.getMethod()).thenReturn(HttpMethod.POST);
+		when(context.getRequest()).thenReturn(httpRequest);
+		retryProperties.setRetryOnAllOperations(true);
+		BlockingLoadBalancedRetryPolicy retryPolicy = getRetryPolicy(retryProperties);
+
+		boolean canRetry = retryPolicy.canRetry(context);
+
+		assertThat(canRetry).isTrue();
+	}
+
+	@Test
+	void shouldResolveRetryableStatusCode() {
+		retryProperties.setRetryableStatusCodes(new HashSet<>(Arrays.asList(404, 502)));
+		BlockingLoadBalancedRetryPolicy retryPolicy = getRetryPolicy(retryProperties);
+
+		boolean retryableStatusCode = retryPolicy.retryableStatusCode(404);
+
+		assertThat(retryableStatusCode).isTrue();
+	}
+
+	private BlockingLoadBalancedRetryPolicy getRetryPolicy(
+			LoadBalancerRetryProperties retryProperties) {
+		return new BlockingLoadBalancedRetryPolicy("test", loadBalancerClient,
+				retryProperties);
+	}
+
+}

spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfigurationTests.java

@@ -16,11 +16,12 @@
 
 package org.springframework.cloud.loadbalancer.config;
 
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
 import org.springframework.boot.autoconfigure.AutoConfigurations;
 import org.springframework.boot.test.context.FilteredClassLoader;
 import org.springframework.boot.test.context.runner.ApplicationContextRunner;
+import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory;
 import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient;
 import org.springframework.cloud.loadbalancer.config.BlockingLoadBalancerClientAutoConfiguration.BlockingLoadBalancerClientRibbonWarnLogger;
 import org.springframework.web.client.RestTemplate;
@@ -32,7 +33,7 @@ import static org.assertj.core.api.Assertions.assertThat;
  * @author Olga Maciaszek-Sharma
  * @author Tim Ysewyn
  */
-public class BlockingLoadBalancerClientAutoConfigurationTests {
+class BlockingLoadBalancerClientAutoConfigurationTests {
 
 	private ApplicationContextRunner applicationContextRunner = new ApplicationContextRunner()
 			.withPropertyValues("spring.cloud.loadbalancer.ribbon.enabled=false")
@@ -40,16 +41,17 @@ public class BlockingLoadBalancerClientAutoConfigurationTests {
 					BlockingLoadBalancerClientAutoConfiguration.class));
 
 	@Test
-	public void beansCreatedNormally() {
+	void beansCreatedNormally() {
 		applicationContextRunner.run(ctxt -> {
 			assertThat(ctxt).hasSingleBean(BlockingLoadBalancerClient.class);
+			assertThat(ctxt).hasSingleBean(LoadBalancedRetryFactory.class);
 			assertThat(ctxt)
 					.doesNotHaveBean(BlockingLoadBalancerClientRibbonWarnLogger.class);
 		});
 	}
 
 	@Test
-	public void worksWithoutSpringWeb() {
+	void worksWithoutSpringWeb() {
 		applicationContextRunner
 				.withClassLoader(new FilteredClassLoader(RestTemplate.class))
 				.run(context -> {