Merge remote-tracking branch 'origin/1.4.x'

spring-cloud-netflix-dependencies/pom.xml

@@ -17,7 +17,7 @@
 		<archaius.version>0.7.5</archaius.version>
 		<eureka.version>1.8.6</eureka.version>
 		<hystrix.version>1.5.12</hystrix.version>
-		<ribbon.version>2.2.4</ribbon.version>
+		<ribbon.version>2.2.5</ribbon.version>
 		<servo.version>0.10.1</servo.version>
 		<zuul.version>1.3.1</zuul.version>
 		<rxjava.version>1.2.0</rxjava.version>

spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicy.java

@@ -138,7 +138,7 @@ public class RibbonLoadBalancedRetryPolicy implements LoadBalancedRetryPolicy {
 			serverStats.addToFailureCount();    				
 			LOGGER.debug(lbServer.getHostPort() + " RetryCount: " + context.getRetryCount() 
 				+ " Successive Failures: " + serverStats.getSuccessiveConnectionFailureCount() 
-				+ " CirtuitBreakerTripped:" + serverStats.isCircuitBreakerTripped());
+				+ " CircuitBreakerTripped:" + serverStats.isCircuitBreakerTripped());
 		}
 	}
 

spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java

@@ -44,6 +44,8 @@ import org.springframework.retry.backoff.NoBackOffPolicy;
 import org.springframework.retry.policy.NeverRetryPolicy;
 import org.springframework.retry.support.RetryTemplate;
 import org.springframework.web.util.UriComponentsBuilder;
+
+import com.netflix.client.ClientException;
 import com.netflix.client.RequestSpecificRetryHandler;
 import com.netflix.client.RetryHandler;
 import com.netflix.client.config.IClientConfig;
@@ -54,8 +56,7 @@ import com.netflix.loadbalancer.Server;
  * @author Ryan Baxter
  * @author Gang Li
  */
-public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingHttpClient
-		implements ServiceInstanceChooser {
+public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingHttpClient {
 	private LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new LoadBalancedRetryPolicyFactory.NeverRetryFactory();
 	private LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory =
 		new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory();
@@ -158,7 +159,7 @@ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingH
 	}
 
 	private RibbonApacheHttpResponse executeWithRetry(RibbonApacheHttpRequest request, LoadBalancedRetryPolicy retryPolicy,
-													  RetryCallback<RibbonApacheHttpResponse, IOException> callback,
+													  RetryCallback<RibbonApacheHttpResponse, Exception> callback,
 													  RecoveryCallback<RibbonApacheHttpResponse> recoveryCallback) throws Exception {
 		RetryTemplate retryTemplate = new RetryTemplate();
 		boolean retryable = isRequestRetryable(request);
@@ -174,12 +175,6 @@ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingH
 	}
 
 	@Override
-	public ServiceInstance choose(String serviceId) {
-		Server server = this.getLoadBalancer().chooseServer(serviceId);
-		return new RibbonLoadBalancerClient.RibbonServer(serviceId, server);
-	}
-
-	@Override
 	public RequestSpecificRetryHandler getRequestSpecificRetryHandler(RibbonApacheHttpRequest request, IClientConfig requestConfig) {
 		return new RequestSpecificRetryHandler(false, false, RetryHandler.DEFAULT, null);
 	}

spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/RetryableOkHttpLoadBalancingClient.java

@@ -42,19 +42,18 @@ import org.springframework.retry.backoff.NoBackOffPolicy;
 import org.springframework.retry.policy.NeverRetryPolicy;
 import org.springframework.retry.support.RetryTemplate;
 import org.springframework.web.util.UriComponentsBuilder;
-import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClient;
 import org.springframework.cloud.netflix.ribbon.ServerIntrospector;
+
 import com.netflix.client.RequestSpecificRetryHandler;
 import com.netflix.client.RetryHandler;
 import com.netflix.client.config.IClientConfig;
-import com.netflix.loadbalancer.Server;
 
 /**
  * An OK HTTP client which leverages Spring Retry to retry failed request.
  * @author Ryan Baxter
  * @author Gang Li
  */
-public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClient implements ServiceInstanceChooser {
+public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClient {
 
 	private LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory;
 	private LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory =
@@ -128,13 +127,12 @@ public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClien
 				OkHttpRibbonRequest newRequest = ribbonRequest;
 				if(context instanceof LoadBalancedRetryContext) {
 					ServiceInstance service = ((LoadBalancedRetryContext)context).getServiceInstance();
-					if(service != null) {
-						//Reconstruct the request URI using the host and port set in the retry context
-						newRequest = newRequest.withNewUri(new URI(service.getUri().getScheme(),
-								newRequest.getURI().getUserInfo(), service.getHost(), service.getPort(),
-								newRequest.getURI().getPath(), newRequest.getURI().getQuery(),
-								newRequest.getURI().getFragment()));
-					}
+					validateServiceInstance(service);
+					//Reconstruct the request URI using the host and port set in the retry context
+					newRequest = newRequest.withNewUri(new URI(service.getUri().getScheme(),
+							newRequest.getURI().getUserInfo(), service.getHost(), service.getPort(),
+							newRequest.getURI().getPath(), newRequest.getURI().getQuery(),
+							newRequest.getURI().getFragment()));
 				}
 				if (isSecure(configOverride)) {
 					final URI secureUri = UriComponentsBuilder.fromUri(newRequest.getUri())
@@ -163,12 +161,7 @@ public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClien
 		});
 	}
 
-	@Override
-	public ServiceInstance choose(String serviceId) {
-		Server server = this.getLoadBalancer().chooseServer(serviceId);
-		return new RibbonLoadBalancerClient.RibbonServer(serviceId,
-				server);
-	}
+	
 
 	@Override
 	public RequestSpecificRetryHandler getRequestSpecificRetryHandler(OkHttpRibbonRequest request, IClientConfig requestConfig) {

spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/support/AbstractLoadBalancingClient.java

@@ -17,17 +17,21 @@
 
 package org.springframework.cloud.netflix.ribbon.support;
 
+import org.springframework.cloud.client.ServiceInstance;
+import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser;
 import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector;
+import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClient;
 import org.springframework.cloud.netflix.ribbon.RibbonProperties;
 import org.springframework.cloud.netflix.ribbon.ServerIntrospector;
-
 import com.netflix.client.AbstractLoadBalancerAwareClient;
+import com.netflix.client.ClientException;
 import com.netflix.client.IResponse;
 import com.netflix.client.RequestSpecificRetryHandler;
 import com.netflix.client.RetryHandler;
 import com.netflix.client.config.DefaultClientConfigImpl;
 import com.netflix.client.config.IClientConfig;
 import com.netflix.loadbalancer.ILoadBalancer;
+import com.netflix.loadbalancer.Server;
 import com.netflix.loadbalancer.reactive.LoadBalancerCommand;
 
 import static org.springframework.cloud.netflix.ribbon.RibbonClientConfiguration.DEFAULT_CONNECT_TIMEOUT;
@@ -37,7 +41,7 @@ import static org.springframework.cloud.netflix.ribbon.RibbonClientConfiguration
  * @author Spencer Gibb
  */
 public abstract class AbstractLoadBalancingClient<S extends ContextAwareRequest, T extends IResponse, D> extends
-		AbstractLoadBalancerAwareClient<S, T> {
+		AbstractLoadBalancerAwareClient<S, T> implements ServiceInstanceChooser {
 
 	protected int connectTimeout;
 
@@ -56,7 +60,7 @@ public abstract class AbstractLoadBalancingClient<S extends ContextAwareRequest,
 	public boolean isClientRetryable(ContextAwareRequest request) {
 		return false;
 	}
-
+	
 	@Deprecated
 	public AbstractLoadBalancingClient() {
 		super(null);
@@ -143,4 +147,21 @@ public abstract class AbstractLoadBalancingClient<S extends ContextAwareRequest,
 			builder.withServerLocator(request.getLoadBalancerKey());
 		}
 	}
+	
+	@Override
+	public ServiceInstance choose(String serviceId) {
+		Server server = this.getLoadBalancer().chooseServer(serviceId);
+		if (server != null) {
+			return new RibbonLoadBalancerClient.RibbonServer(serviceId, server);
+		}
+		return null;
+	}
+	
+	public void validateServiceInstance(ServiceInstance serviceInstance) throws ClientException {
+		if (serviceInstance == null) {
+			throw new ClientException("Load balancer does not have available server for client: " + clientName);
+		} else if (serviceInstance.getHost() == null) {
+			throw new ClientException("Invalid Server for: " + serviceInstance.getServiceId() + " null Host");
+		}
+	}
 }

spring-cloud-netflix-ribbon/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java

@@ -64,6 +64,8 @@ import org.springframework.retry.backoff.BackOffInterruptedException;
 import org.springframework.retry.backoff.BackOffPolicy;
 import org.springframework.test.util.ReflectionTestUtils;
 import org.springframework.util.LinkedMultiValueMap;
+
+import com.netflix.client.ClientException;
 import com.netflix.client.DefaultLoadBalancerRetryHandler;
 import com.netflix.client.RetryHandler;
 import com.netflix.client.config.CommonClientConfigKey;
@@ -74,6 +76,7 @@ import com.netflix.loadbalancer.ILoadBalancer;
 import com.netflix.loadbalancer.Server;
 
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.CoreMatchers.containsString;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
@@ -734,6 +737,81 @@ public class RibbonLoadBalancingHttpClientTests {
 		verify(lb, times(2)).chooseServer(eq(serviceName));
 		assertEquals(2, myBackOffPolicyFactory.getCount());
 	}
+	
+	private RetryableRibbonLoadBalancingHttpClient setupClientForServerValidation(String serviceName, String host, int port,
+			CloseableHttpClient delegate, ILoadBalancer lb) throws Exception {
+		ServerIntrospector introspector = mock(ServerIntrospector.class);
+		RetryHandler retryHandler = new DefaultLoadBalancerRetryHandler(1, 1, true);
+		DefaultClientConfigImpl clientConfig = new DefaultClientConfigImpl();
+		clientConfig.set(CommonClientConfigKey.OkToRetryOnAllOperations, true);
+		clientConfig.set(CommonClientConfigKey.MaxAutoRetriesNextServer, 0);
+		clientConfig.set(CommonClientConfigKey.MaxAutoRetries, 1);
+		clientConfig.set(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES, "");
+		clientConfig.set(CommonClientConfigKey.IsSecure, false);
+		clientConfig.setClientName(serviceName);
+		RibbonLoadBalancerContext context = new RibbonLoadBalancerContext(lb, clientConfig, retryHandler);
+		SpringClientFactory clientFactory = mock(SpringClientFactory.class);
+		doReturn(context).when(clientFactory).getLoadBalancerContext(eq(serviceName));
+		doReturn(clientConfig).when(clientFactory).getClientConfig(eq(serviceName));
+		LoadBalancedRetryPolicyFactory factory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory);
+		RetryableRibbonLoadBalancingHttpClient client = new RetryableRibbonLoadBalancingHttpClient(delegate,
+				clientConfig, introspector, factory, loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory);
+		client.setLoadBalancer(lb);
+		ReflectionTestUtils.setField(client, "delegate", delegate);
+		return client;
+	}
+	
+	@Test
+	public void noServersFoundTest() throws Exception {
+		String serviceName = "noservers";
+		String host = serviceName;
+		int port = 80;
+		HttpMethod method = HttpMethod.POST;
+		URI uri = new URI("http://" + host + ":" + port);
+		CloseableHttpClient delegate = mock(CloseableHttpClient.class);
+		ILoadBalancer lb = mock(ILoadBalancer.class);
+		
+		RetryableRibbonLoadBalancingHttpClient client = setupClientForServerValidation(serviceName, host, port, delegate, lb);
+		RibbonApacheHttpRequest request = mock(RibbonApacheHttpRequest.class);
+		doReturn(null).when(lb).chooseServer(eq(serviceName));
+		doReturn(method).when(request).getMethod();
+		doReturn(uri).when(request).getURI();
+		doReturn(request).when(request).withNewUri(any(URI.class));
+		HttpUriRequest uriRequest = mock(HttpUriRequest.class);
+		doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class));
+		try {
+			client.execute(request, null);
+			fail("Expected IOException for no servers available");
+		} catch (ClientException ex) {
+			assertThat(ex.getMessage(), containsString("Load balancer does not have available server for client"));
+		}
+	}
+	
+	@Test
+	public void invalidServerTest() throws Exception {
+		String serviceName = "noservers";
+		String host = serviceName;
+		int port = 80;
+		HttpMethod method = HttpMethod.POST;
+		URI uri = new URI("http://" + host + ":" + port);
+		CloseableHttpClient delegate = mock(CloseableHttpClient.class);
+		ILoadBalancer lb = mock(ILoadBalancer.class);
+		
+		RetryableRibbonLoadBalancingHttpClient client = setupClientForServerValidation(serviceName, host, port, delegate, lb);
+		RibbonApacheHttpRequest request = mock(RibbonApacheHttpRequest.class);
+		doReturn(new Server(null,8000)).when(lb).chooseServer(eq(serviceName));
+		doReturn(method).when(request).getMethod();
+		doReturn(uri).when(request).getURI();
+		doReturn(request).when(request).withNewUri(any(URI.class));
+		HttpUriRequest uriRequest = mock(HttpUriRequest.class);
+		doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class));
+		try {
+			client.execute(request, null);
+			fail("Expected IOException for no servers available");
+		} catch (ClientException ex) {
+			assertThat(ex.getMessage(), containsString("Invalid Server for: "));
+		}
+	}
 
 	@Configuration
 	protected static class UseDefaults {

spring-cloud-netflix-ribbon/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryEnabledOkHttpClientTests.java

@@ -15,25 +15,52 @@
  */
 package org.springframework.cloud.netflix.ribbon.okhttp;
 
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.junit.Assert.fail;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+
+import java.net.URI;
 import java.util.Map;
+
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.beans.BeansException;
 import org.springframework.boot.test.context.SpringBootTest;
+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.LoadBalancerAutoConfiguration;
 import org.springframework.cloud.commons.httpclient.HttpClientConfiguration;
 import org.springframework.cloud.netflix.ribbon.RibbonAutoConfiguration;
 import org.springframework.cloud.netflix.ribbon.RibbonClientConfiguration;
+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;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
+import org.springframework.http.HttpMethod;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.test.util.ReflectionTestUtils;
 
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.hasSize;
-import static org.hamcrest.Matchers.instanceOf;
+import com.netflix.client.ClientException;
+import com.netflix.client.DefaultLoadBalancerRetryHandler;
+import com.netflix.client.RetryHandler;
+import com.netflix.client.config.CommonClientConfigKey;
+import com.netflix.client.config.DefaultClientConfigImpl;
+import com.netflix.loadbalancer.ILoadBalancer;
+import com.netflix.loadbalancer.Server;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
 
 /**
  * @author Ryan Baxter
@@ -46,7 +73,10 @@ import static org.hamcrest.Matchers.instanceOf;
 public class SpringRetryEnabledOkHttpClientTests implements ApplicationContextAware {
 
 	private ApplicationContext context;
-
+	private ILoadBalancer loadBalancer;
+	private LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory = new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory();
+	private LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory = new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory();
+	
 	@Test
 	public void testLoadBalancedRetryFactoryBean() throws Exception {
 		Map<String, LoadBalancedRetryPolicyFactory> factories = context
@@ -65,4 +95,80 @@ public class SpringRetryEnabledOkHttpClientTests implements ApplicationContextAw
 	public void setApplicationContext(ApplicationContext context) throws BeansException {
 		this.context = context;
 	}
+	
+	private RetryableOkHttpLoadBalancingClient setupClientForServerValidation(String serviceName, String host, int port,
+			OkHttpClient delegate, ILoadBalancer lb) throws Exception {
+		ServerIntrospector introspector = mock(ServerIntrospector.class);
+		RetryHandler retryHandler = new DefaultLoadBalancerRetryHandler(1, 1, true);
+		DefaultClientConfigImpl clientConfig = new DefaultClientConfigImpl();
+		clientConfig.set(CommonClientConfigKey.OkToRetryOnAllOperations, true);
+		clientConfig.set(CommonClientConfigKey.MaxAutoRetriesNextServer, 0);
+		clientConfig.set(CommonClientConfigKey.MaxAutoRetries, 1);
+		clientConfig.set(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES, "");
+		clientConfig.set(CommonClientConfigKey.IsSecure, false);
+		clientConfig.setClientName(serviceName);
+		RibbonLoadBalancerContext context = new RibbonLoadBalancerContext(lb, clientConfig, retryHandler);
+		SpringClientFactory clientFactory = mock(SpringClientFactory.class);
+		doReturn(context).when(clientFactory).getLoadBalancerContext(eq(serviceName));
+		doReturn(clientConfig).when(clientFactory).getClientConfig(eq(serviceName));
+		LoadBalancedRetryPolicyFactory factory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory);
+		RetryableOkHttpLoadBalancingClient client = new RetryableOkHttpLoadBalancingClient(delegate, clientConfig, introspector,
+				factory, loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory);
+		client.setLoadBalancer(lb);
+		ReflectionTestUtils.setField(client, "delegate", delegate);
+		return client;
+	}
+	
+	@Test
+	public void noServersFoundTest() throws Exception {
+		String serviceName = "noservers";
+		String host = serviceName;
+		int port = 80;
+		HttpMethod method = HttpMethod.POST;
+		URI uri = new URI("http://" + host + ":" + port);
+		OkHttpClient delegate = mock(OkHttpClient.class);
+		ILoadBalancer lb = mock(ILoadBalancer.class);
+		
+		RetryableOkHttpLoadBalancingClient client = setupClientForServerValidation(serviceName, host, port, delegate, lb);
+		OkHttpRibbonRequest request = mock(OkHttpRibbonRequest.class);
+		doReturn(null).when(lb).chooseServer(eq(serviceName));
+		doReturn(method).when(request).getMethod();
+		doReturn(uri).when(request).getURI();
+		doReturn(request).when(request).withNewUri(any(URI.class));
+		Request okRequest = new Request.Builder().url("ws:testerror.sc").build();
+		doReturn(okRequest).when(request).toRequest();
+		try {
+			client.execute(request, null);
+			fail("Expected ClientException for no servers available");
+		} catch (ClientException ex) {
+			assertThat(ex.getMessage(), containsString("Load balancer does not have available server for client"));
+		}
+	}
+	
+	@Test
+	public void invalidServerTest() throws Exception {
+		String serviceName = "noservers";
+		String host = serviceName;
+		int port = 80;
+		HttpMethod method = HttpMethod.POST;
+		URI uri = new URI("http://" + host + ":" + port);
+		OkHttpClient delegate = mock(OkHttpClient.class);
+		ILoadBalancer lb = mock(ILoadBalancer.class);
+		
+		RetryableOkHttpLoadBalancingClient client = setupClientForServerValidation(serviceName, host, port, delegate, lb);
+		OkHttpRibbonRequest request = mock(OkHttpRibbonRequest.class);
+		doReturn(new Server(null,8000)).when(lb).chooseServer(eq(serviceName));
+		doReturn(method).when(request).getMethod();
+		doReturn(uri).when(request).getURI();
+		doReturn(request).when(request).withNewUri(any(URI.class));
+		Request okRequest = new Request.Builder().url("ws:testerror.sc").build();
+		doReturn(okRequest).when(request).toRequest();
+
+		try {
+			client.execute(request, null);
+			fail("Expected ClientException for no Invalid Host");
+		} catch (ClientException ex) {
+			assertThat(ex.getMessage(), containsString("Invalid Server for: "));
+		}
+	}
 }