diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/ClientHttpResponseStatusCodeException.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/ClientHttpResponseStatusCodeException.java new file mode 100644 index 00000000..0d7dd299 --- /dev/null +++ b/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(); + } + } +} diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java index ee46b919..c2616653 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java @@ -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() { - @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() { - @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() { + @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(); - } - } } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryableStatusCodeException.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryableStatusCodeException.java index ddce92d8..876df791 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryableStatusCodeException.java +++ b/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)); } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RibbonRecoveryCallback.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RibbonRecoveryCallback.java new file mode 100644 index 00000000..0382bdc9 --- /dev/null +++ b/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 implements RecoveryCallback { + + /** + * 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); + } +} diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/ClientHttpResponseStatusCodeExceptionTest.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/ClientHttpResponseStatusCodeExceptionTest.java new file mode 100644 index 00000000..9358e7ea --- /dev/null +++ b/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; + } + } +} \ No newline at end of file diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java index 9f0185b3..46fd8f6a 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java @@ -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; } }