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

spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/ClientHttpResponseStatusCodeException.java

@@ -0,0 +1,86 @@
+/*
+ * Copyright 2016-2018 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 java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.client.AbstractClientHttpResponse;
+import org.springframework.http.client.ClientHttpResponse;
+import org.springframework.util.StreamUtils;
+
+/**
+ * {@link RetryableStatusCodeException} that captures a {@link ClientHttpResponse}
+ * @author Ryan Baxter
+ */
+public class ClientHttpResponseStatusCodeException extends RetryableStatusCodeException {
+
+	private ClientHttpResponseWrapper response;
+
+	/**
+	 * Constructor
+	 * @param serviceId The service id
+	 * @param response The response object
+	 * @throws IOException Thrown if the {@link ClientHttpResponse} body cannot be retrieved
+	 */
+	public ClientHttpResponseStatusCodeException(String serviceId, ClientHttpResponse response) throws IOException {
+		super(serviceId, response.getRawStatusCode(), response, null);
+		this.response = new ClientHttpResponseWrapper(response);
+		response.close();
+	}
+
+	@Override
+	public ClientHttpResponse getResponse() {
+		return response;
+	}
+
+	static class ClientHttpResponseWrapper extends AbstractClientHttpResponse {
+
+		private ClientHttpResponse response;
+		private byte[] body;
+
+		public ClientHttpResponseWrapper(ClientHttpResponse response) throws IOException {
+			this.response = response;
+			this.body = StreamUtils.copyToByteArray(response.getBody());
+		}
+
+		@Override
+		public int getRawStatusCode() throws IOException {
+			return response.getRawStatusCode();
+		}
+
+		@Override
+		public String getStatusText() throws IOException {
+			return response.getStatusText();
+		}
+
+		@Override
+		public void close() {
+			response.close();
+		}
+
+		@Override
+		public InputStream getBody() throws IOException {
+			return new ByteArrayInputStream(body);
+		}
+
+		@Override
+		public HttpHeaders getHeaders() {
+			return response.getHeaders();
+		}
+	}
+}

spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java

@@ -16,23 +16,16 @@
 
 package org.springframework.cloud.client.loadbalancer;
 
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.io.InputStream;
 import java.net.URI;
 
 import org.springframework.cloud.client.ServiceInstance;
-import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpRequest;
-import org.springframework.http.HttpStatus;
 import org.springframework.http.client.ClientHttpRequestExecution;
 import org.springframework.http.client.ClientHttpRequestInterceptor;
 import org.springframework.http.client.ClientHttpResponse;
-import org.springframework.retry.RecoveryCallback;
 import org.springframework.retry.RetryCallback;
 import org.springframework.retry.RetryContext;
-import org.springframework.retry.RetryException;
 import org.springframework.retry.RetryListener;
 import org.springframework.retry.backoff.BackOffPolicy;
 import org.springframework.retry.backoff.NoBackOffPolicy;
@@ -122,90 +115,31 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto
 						serviceName));
 		return template
 				.execute(new RetryCallback<ClientHttpResponse, IOException>() {
-                    @Override
-                    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);
-                        }
-                        ClientHttpResponse response = RetryLoadBalancerInterceptor.this.loadBalancer.execute(
-                                serviceName, serviceInstance,
-                                requestFactory.createRequest(request, body, execution));
-                        int statusCode = response.getRawStatusCode();
-                        if (retryPolicy != null && retryPolicy.retryableStatusCode(statusCode)) {
-                            ClientHttpResponseWrapper wrapper = new ClientHttpResponseWrapper(response);
-                            wrapper.init();
-                            throw new RetryableStatusCodeException(serviceName, statusCode, wrapper, null);
-                        }
-                        return response;
-                    }
-                }, new RecoveryCallback<ClientHttpResponse>() {
-                    @Override
-                    public ClientHttpResponse recover(RetryContext retryContext) throws Exception {
-                        Throwable lastThrowable = retryContext.getLastThrowable();
-                        if (lastThrowable != null && lastThrowable instanceof RetryableStatusCodeException) {
-                            RetryableStatusCodeException ex = (RetryableStatusCodeException) lastThrowable;
-                            return (ClientHttpResponse) ex.getResponse();
-                        }
-                        throw new RetryException("Could not recover", lastThrowable);
-                    }
-                });
+					@Override
+					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);
+						}
+						ClientHttpResponse response = RetryLoadBalancerInterceptor.this.loadBalancer.execute(
+								serviceName, serviceInstance,
+								requestFactory.createRequest(request, body, execution));
+						int statusCode = response.getRawStatusCode();
+						if (retryPolicy != null && retryPolicy.retryableStatusCode(statusCode)) {
+							throw new ClientHttpResponseStatusCodeException(serviceName, response);
+						}
+						return response;
+					}
+				}, new RibbonRecoveryCallback<ClientHttpResponse, ClientHttpResponse>() {
+					@Override
+					protected ClientHttpResponse createResponse(ClientHttpResponse response, URI uri) {
+						return response;
+					}
+				});
 	}
-
-    public static class ClientHttpResponseWrapper implements ClientHttpResponse {
-
-	    private ClientHttpResponse response;
-	    private InputStream content;
-
-        public ClientHttpResponseWrapper(ClientHttpResponse response) {
-            this.response = response;
-        }
-
-        public void init() throws IOException {
-            InputStream body = response.getBody();
-            ByteArrayOutputStream temp = new ByteArrayOutputStream();
-            byte[] buffer = new byte[4096];
-            int length = 0;
-            while ((length = body.read(buffer)) != -1) {
-                temp.write(buffer, 0, length);
-            }
-            content = new ByteArrayInputStream(temp.toByteArray());
-            response.close();
-        }
-
-        @Override
-        public HttpStatus getStatusCode() throws IOException {
-            return response.getStatusCode();
-        }
-
-        @Override
-        public int getRawStatusCode() throws IOException {
-            return response.getRawStatusCode();
-        }
-
-        @Override
-        public String getStatusText() throws IOException {
-            return response.getStatusText();
-        }
-
-        @Override
-        public void close() {
-            response.close();
-        }
-
-        @Override
-        public InputStream getBody() throws IOException {
-            return content;
-        }
-
-        @Override
-        public HttpHeaders getHeaders() {
-            return response.getHeaders();
-        }
-    }
 }

spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryableStatusCodeException.java

@@ -1,3 +1,18 @@
+/*
+ * Copyright 2016-2018 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 java.io.IOException;
@@ -15,6 +30,8 @@ public class RetryableStatusCodeException extends IOException {
 
 	private URI uri;
 
+	@Deprecated
+	//TODO Remove in 2.0.x
 	public RetryableStatusCodeException(String serviceId, int statusCode) {
 		super(String.format(MESSAGE, serviceId, statusCode));
 	}

spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RibbonRecoveryCallback.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright 2013-2017 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.retry.RecoveryCallback;
+import org.springframework.retry.RetryContext;
+import org.springframework.retry.RetryException;
+
+import java.net.URI;
+
+/**
+ * An implementation of {@link RecoveryCallback} which relies on an implemtation
+ * of {@link RetryableStatusCodeException} to contain the last response object from
+ * the request
+ * @author LiYuan Lee
+ */
+public abstract class RibbonRecoveryCallback<T, R> implements RecoveryCallback<T> {
+
+	/**
+	 * Create the response returned in the {@link RecoveryCallback}
+	 * @param response The response from the HTTP client
+	 * @param uri The URI the response is from
+	 * @return The response to be returned
+	 */
+	protected abstract T createResponse(R response, URI uri);
+
+	@Override
+	public T recover(RetryContext context) throws Exception {
+		Throwable lastThrowable = context.getLastThrowable();
+		if (lastThrowable != null) {
+			if (lastThrowable instanceof RetryableStatusCodeException) {
+				RetryableStatusCodeException ex = (RetryableStatusCodeException) lastThrowable;
+				return createResponse((R) ex.getResponse(), ex.getUri());
+			} else if (lastThrowable instanceof Exception){
+				throw (Exception)lastThrowable;
+			}
+		}
+		throw new RetryException("Could not recover", lastThrowable);
+	}
+}

spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/ClientHttpResponseStatusCodeExceptionTest.java

@@ -0,0 +1,72 @@
+package org.springframework.cloud.client.loadbalancer;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.runners.MockitoJUnitRunner;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.client.AbstractClientHttpResponse;
+import org.springframework.http.client.ClientHttpResponse;
+import org.springframework.util.StreamUtils;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
+
+/**
+ * @author Ryan Baxter
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class ClientHttpResponseStatusCodeExceptionTest {
+
+	@Test
+	public void testCreation() throws Exception {
+		MyClientHttpResponse response = new MyClientHttpResponse();
+		assertFalse(response.isClosed());
+		ClientHttpResponseStatusCodeException exp = new ClientHttpResponseStatusCodeException("service", response);
+		assertTrue(response.isClosed());
+		ClientHttpResponse expResponse = exp.getResponse();
+		assertEquals(response.getRawStatusCode(), expResponse.getRawStatusCode());
+		assertEquals(response.getStatusText(), expResponse.getStatusText());
+		assertEquals(response.getHeaders(), expResponse.getHeaders());
+		assertEquals(response.getStatusText(), new String(StreamUtils.copyToByteArray(expResponse.getBody())));
+	}
+
+	class MyClientHttpResponse extends AbstractClientHttpResponse {
+
+		private boolean closed = false;
+
+		@Override
+		public int getRawStatusCode() throws IOException {
+			return 200;
+		}
+
+		@Override
+		public String getStatusText() throws IOException {
+			return "foo";
+		}
+
+		@Override
+		public void close() {
+			this.closed = true;
+		}
+
+		public boolean isClosed() {
+			return closed;
+		}
+
+		@Override
+		public InputStream getBody() throws IOException {
+			return new ByteArrayInputStream(getStatusText().getBytes());
+		}
+
+		@Override
+		public HttpHeaders getHeaders() {
+			HttpHeaders headers = new HttpHeaders();
+			headers.add("foo", "bar");
+			return headers;
+		}
+	}
+}

spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java

@@ -19,7 +19,6 @@ import org.springframework.mock.http.client.MockClientHttpResponse;
 
 import org.springframework.retry.RetryCallback;
 import org.springframework.retry.RetryContext;
-import org.springframework.retry.RetryException;
 import org.springframework.retry.RetryListener;
 import org.springframework.retry.TerminatedRetryException;
 
@@ -64,7 +63,7 @@ public class RetryLoadBalancerInterceptorTest {
         lbProperties = null;
     }
 
-    @Test(expected = RetryException.class)
+    @Test(expected = IOException.class)
     public void interceptDisableRetry() throws Throwable {
         HttpRequest request = mock(HttpRequest.class);
         when(request.getURI()).thenReturn(new URI("http://foo"));
@@ -95,7 +94,7 @@ public class RetryLoadBalancerInterceptorTest {
                 backOffPolicyFactory, retryListenerFactory);
         byte[] body = new byte[]{};
         ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class);
-        ClientHttpResponse rsp = interceptor.intercept(request, body, execution);
+        interceptor.intercept(request, body, execution);
     }
 
     @Test
@@ -124,7 +123,6 @@ public class RetryLoadBalancerInterceptorTest {
         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(ServiceInstanceChooser.class))).thenReturn(policy);
         ServiceInstance serviceInstance = mock(ServiceInstance.class);
@@ -152,7 +150,6 @@ public class RetryLoadBalancerInterceptorTest {
         LoadBalancedRetryPolicy policy = mock(LoadBalancedRetryPolicy.class);
         when(policy.retryableStatusCode(eq(HttpStatus.NOT_FOUND.value()))).thenReturn(true);
         when(policy.canRetryNextServer(any(LoadBalancedRetryContext.class))).thenReturn(true);
-        InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(request, policy, client,"foo");
         LoadBalancedRetryPolicyFactory lbRetryPolicyFactory = mock(LoadBalancedRetryPolicyFactory.class);
         when(lbRetryPolicyFactory.create(eq("foo"), any(ServiceInstanceChooser.class))).thenReturn(policy);
         ServiceInstance serviceInstance = mock(ServiceInstance.class);
@@ -192,11 +189,11 @@ public class RetryLoadBalancerInterceptorTest {
                 .thenReturn(clientHttpResponseNotFound);
 
         lbProperties.setEnabled(true);
+        RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory,
+        new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(), new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory());
         byte[] body = new byte[]{};
         ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class);
 
-        RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties,
-                lbRetryPolicyFactory, lbRequestFactory, backOffPolicyFactory);
         ClientHttpResponse rsp = interceptor.intercept(request, body, execution);
 
         verify(client, times(1)).execute(eq("foo"), eq(serviceInstance),
@@ -239,7 +236,7 @@ public class RetryLoadBalancerInterceptorTest {
         assertThat(backOffPolicy.getBackoffAttempts(), is(1));
     }
 
-    @Test(expected = RetryException.class)
+    @Test(expected = IOException.class)
     public void interceptFailedRetry() throws Exception {
         HttpRequest request = mock(HttpRequest.class);
         when(request.getURI()).thenReturn(new URI("http://foo"));
@@ -257,7 +254,7 @@ public class RetryLoadBalancerInterceptorTest {
                 lbRequestFactory, backOffPolicyFactory, retryListenerFactory);
         byte[] body = new byte[]{};
         ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class);
-        ClientHttpResponse rsp = interceptor.intercept(request, body, execution);
+        interceptor.intercept(request, body, execution);
         verify(lbRequestFactory).createRequest(request, body, execution);
     }
 
@@ -309,7 +306,7 @@ public class RetryLoadBalancerInterceptorTest {
             backOffPolicyFactory, retryListeners);
         byte[] body = new byte[]{};
         ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class);
-        ClientHttpResponse rsp = interceptor.intercept(request, body, execution);
+        interceptor.intercept(request, body, execution);
     }
 
     @Test
@@ -330,7 +327,7 @@ public class RetryLoadBalancerInterceptorTest {
         lbProperties.setEnabled(true);
         when(this.lbRequestFactory.createRequest(any(), any(), any())).thenReturn(mock(LoadBalancerRequest.class));
         RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory,
-            backOffPolicyFactory);
+            backOffPolicyFactory, new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory());
         byte[] body = new byte[]{};
         ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class);
         ClientHttpResponse rsp = interceptor.intercept(request, body, execution);
@@ -359,7 +356,7 @@ public class RetryLoadBalancerInterceptorTest {
             backoffAttempts++;
         }
 
-        public int getBackoffAttempts() {
+        int getBackoffAttempts() {
             return backoffAttempts;
         }
     }
@@ -388,7 +385,7 @@ public class RetryLoadBalancerInterceptorTest {
             }};
         }
 
-        public int getOnError() {
+        int getOnError() {
             return onError;
         }
     }