You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2015/06/20 14:12:03 UTC
svn commit: r1686589 - in /httpcomponents/httpclient/trunk/httpclient/src:
main/java/org/apache/http/client/ main/java/org/apache/http/impl/client/
main/java/org/apache/http/impl/execchain/
test/java/org/apache/http/impl/client/ test/java/org/apache/ht...
Author: olegk
Date: Sat Jun 20 12:12:03 2015
New Revision: 1686589
URL: http://svn.apache.org/r1686589
Log:
RFC 7231: automatic retrial of idempotent methods
Removed:
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/StandardHttpRequestRetryHandler.java
Modified:
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/client/HttpRequestRetryHandler.java
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultHttpRequestRetryHandler.java
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/execchain/RetryExec.java
httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultHttpRequestRetryHandler.java
httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientRequestExecution.java
httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/execchain/TestRetryExec.java
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/client/HttpRequestRetryHandler.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/client/HttpRequestRetryHandler.java?rev=1686589&r1=1686588&r2=1686589&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/client/HttpRequestRetryHandler.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/client/HttpRequestRetryHandler.java Sat Jun 20 12:12:03 2015
@@ -29,6 +29,7 @@ package org.apache.http.client;
import java.io.IOException;
+import org.apache.http.HttpRequest;
import org.apache.http.protocol.HttpContext;
/**
@@ -47,6 +48,7 @@ public interface HttpRequestRetryHandler
* Determines if a method should be retried after an IOException
* occurs during execution.
*
+ * @param request request failed die to an I/O exception.
* @param exception the exception that occurred
* @param executionCount the number of times this method has been
* unsuccessfully executed
@@ -55,6 +57,6 @@ public interface HttpRequestRetryHandler
* @return {@code true} if the method should be retried, {@code false}
* otherwise
*/
- boolean retryRequest(IOException exception, int executionCount, HttpContext context);
+ boolean retryRequest(HttpRequest request, IOException exception, int executionCount, HttpContext context);
}
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultHttpRequestRetryHandler.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultHttpRequestRetryHandler.java?rev=1686589&r1=1686588&r2=1686589&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultHttpRequestRetryHandler.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultHttpRequestRetryHandler.java Sat Jun 20 12:12:03 2015
@@ -34,16 +34,17 @@ import java.net.UnknownHostException;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
+import java.util.Locale;
+import java.util.Map;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import javax.net.ssl.SSLException;
-import org.apache.http.HttpEntityEnclosingRequest;
import org.apache.http.HttpRequest;
import org.apache.http.annotation.Immutable;
import org.apache.http.client.HttpRequestRetryHandler;
import org.apache.http.client.methods.HttpUriRequest;
-import org.apache.http.client.protocol.HttpClientContext;
import org.apache.http.protocol.HttpContext;
import org.apache.http.util.Args;
@@ -60,9 +61,7 @@ public class DefaultHttpRequestRetryHand
/** the number of times a method will be retried */
private final int retryCount;
- /** Whether or not methods that have successfully sent their request will be retried */
- private final boolean requestSentRetryEnabled;
-
+ private final Map<String, Boolean> idempotentMethods;
private final Set<Class<? extends IOException>> nonRetriableClasses;
/**
@@ -79,7 +78,13 @@ public class DefaultHttpRequestRetryHand
final Collection<Class<? extends IOException>> clazzes) {
super();
this.retryCount = retryCount;
- this.requestSentRetryEnabled = requestSentRetryEnabled;
+ this.idempotentMethods = new ConcurrentHashMap<>();
+ this.idempotentMethods.put("GET", Boolean.TRUE);
+ this.idempotentMethods.put("HEAD", Boolean.TRUE);
+ this.idempotentMethods.put("PUT", Boolean.TRUE);
+ this.idempotentMethods.put("DELETE", Boolean.TRUE);
+ this.idempotentMethods.put("OPTIONS", Boolean.TRUE);
+ this.idempotentMethods.put("TRACE", Boolean.TRUE);
this.nonRetriableClasses = new HashSet<>();
for (final Class<? extends IOException> clazz: clazzes) {
this.nonRetriableClasses.add(clazz);
@@ -120,17 +125,20 @@ public class DefaultHttpRequestRetryHand
public DefaultHttpRequestRetryHandler() {
this(3, false);
}
+
/**
* Used {@code retryCount} and {@code requestSentRetryEnabled} to determine
* if the given method should be retried.
*/
@Override
public boolean retryRequest(
+ final HttpRequest request,
final IOException exception,
final int executionCount,
final HttpContext context) {
- Args.notNull(exception, "Exception parameter");
- Args.notNull(context, "HTTP context");
+ Args.notNull(request, "HTTP request");
+ Args.notNull(exception, "I/O exception");
+
if (executionCount > this.retryCount) {
// Do not retry if over max retry count
return false;
@@ -144,36 +152,18 @@ public class DefaultHttpRequestRetryHand
}
}
}
- final HttpClientContext clientContext = HttpClientContext.adapt(context);
- final HttpRequest request = clientContext.getRequest();
-
if (request instanceof HttpUriRequest && ((HttpUriRequest)request).isAborted()) {
return false;
}
-
if (handleAsIdempotent(request)) {
// Retry if the request is considered idempotent
return true;
}
-
- if (!clientContext.isRequestSent() || this.requestSentRetryEnabled) {
- // Retry if the request has not been sent fully or
- // if it's OK to retry methods that have been sent
- return true;
- }
// otherwise do not retry
return false;
}
/**
- * @return {@code true} if this handler will retry methods that have
- * successfully sent their request, {@code false} otherwise
- */
- public boolean isRequestSentRetryEnabled() {
- return requestSentRetryEnabled;
- }
-
- /**
* @return the maximum number of times a method will be retried
*/
public int getRetryCount() {
@@ -184,7 +174,9 @@ public class DefaultHttpRequestRetryHand
* @since 4.2
*/
protected boolean handleAsIdempotent(final HttpRequest request) {
- return !(request instanceof HttpEntityEnclosingRequest);
+ final String method = request.getRequestLine().getMethod().toUpperCase(Locale.ROOT);
+ final Boolean b = this.idempotentMethods.get(method);
+ return b != null && b.booleanValue();
}
}
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/execchain/RetryExec.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/execchain/RetryExec.java?rev=1686589&r1=1686588&r2=1686589&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/execchain/RetryExec.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/execchain/RetryExec.java Sat Jun 20 12:12:03 2015
@@ -91,7 +91,7 @@ public class RetryExec implements Client
this.log.debug("Request has been aborted");
throw ex;
}
- if (retryHandler.retryRequest(ex, execCount, context)) {
+ if (retryHandler.retryRequest(request, ex, execCount, context)) {
if (this.log.isInfoEnabled()) {
this.log.info("I/O exception ("+ ex.getClass().getName() +
") caught when processing request to "
Modified: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultHttpRequestRetryHandler.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultHttpRequestRetryHandler.java?rev=1686589&r1=1686588&r2=1686589&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultHttpRequestRetryHandler.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultHttpRequestRetryHandler.java Sat Jun 20 12:12:03 2015
@@ -26,16 +26,12 @@
*/
package org.apache.http.impl.client;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
import java.io.IOException;
import java.net.UnknownHostException;
+import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.conn.ConnectTimeoutException;
-import org.apache.http.protocol.HttpContext;
-import org.apache.http.protocol.HttpCoreContext;
import org.junit.Assert;
import org.junit.Test;
@@ -45,70 +41,49 @@ public class TestDefaultHttpRequestRetry
@Test
public void noRetryOnConnectTimeout() throws Exception {
- final HttpContext context = mock(HttpContext.class);
- final HttpUriRequest request = mock(HttpUriRequest.class);
+ final HttpUriRequest request = new HttpGet("/");
final DefaultHttpRequestRetryHandler retryHandler = new DefaultHttpRequestRetryHandler();
Assert.assertEquals(3, retryHandler.getRetryCount());
- when(request.isAborted()).thenReturn(Boolean.FALSE);
- when(context.getAttribute(HttpCoreContext.HTTP_REQUEST)).thenReturn(request);
-
- Assert.assertFalse(retryHandler.retryRequest(new ConnectTimeoutException(), 1, context));
+ Assert.assertFalse(retryHandler.retryRequest(request, new ConnectTimeoutException(), 1, null));
}
@Test
public void noRetryOnUnknownHost() throws Exception {
- final HttpContext context = mock(HttpContext.class);
- final HttpUriRequest request = mock(HttpUriRequest.class);
+ final HttpUriRequest request = new HttpGet("/");
final DefaultHttpRequestRetryHandler retryHandler = new DefaultHttpRequestRetryHandler();
- when(request.isAborted()).thenReturn(Boolean.FALSE);
- when(context.getAttribute(HttpCoreContext.HTTP_REQUEST)).thenReturn(request);
-
- Assert.assertFalse(retryHandler.retryRequest(new UnknownHostException(), 1, context));
+ Assert.assertFalse(retryHandler.retryRequest(request, new UnknownHostException(), 1, null));
}
@Test
public void noRetryOnAbortedRequests() throws Exception{
- final HttpContext context = mock(HttpContext.class);
- final HttpUriRequest request = mock(HttpUriRequest.class);
+ final HttpUriRequest request = new HttpGet("/");
+ request.abort();
final DefaultHttpRequestRetryHandler retryHandler = new DefaultHttpRequestRetryHandler();
- when(request.isAborted()).thenReturn(Boolean.TRUE);
- when(context.getAttribute(HttpCoreContext.HTTP_REQUEST)).thenReturn(request);
-
- Assert.assertFalse(retryHandler.retryRequest(new IOException(),3,context));
+ Assert.assertFalse(retryHandler.retryRequest(request, new IOException(), 3, null));
}
@Test
public void retryOnNonAbortedRequests() throws Exception{
-
- final HttpContext context = mock(HttpContext.class);
- final HttpUriRequest request = mock(HttpUriRequest.class);
+ final HttpUriRequest request = new HttpGet("/");
final DefaultHttpRequestRetryHandler retryHandler = new DefaultHttpRequestRetryHandler();
- when(request.isAborted()).thenReturn(Boolean.FALSE);
- when(context.getAttribute(HttpCoreContext.HTTP_REQUEST)).thenReturn(request);
-
- Assert.assertTrue(retryHandler.retryRequest(new IOException(),3,context));
+ Assert.assertTrue(retryHandler.retryRequest(request, new IOException(), 3, null));
}
@Test
public void noRetryOnConnectionTimeout() throws Exception{
-
- final HttpContext context = mock(HttpContext.class);
- final HttpUriRequest request = mock(HttpUriRequest.class);
+ final HttpUriRequest request = new HttpGet("/");
final DefaultHttpRequestRetryHandler retryHandler = new DefaultHttpRequestRetryHandler();
- when(request.isAborted()).thenReturn(false);
- when(context.getAttribute(HttpCoreContext.HTTP_REQUEST)).thenReturn(request);
-
- Assert.assertFalse(retryHandler.retryRequest(new ConnectTimeoutException(),3,context));
+ Assert.assertFalse(retryHandler.retryRequest(request, new ConnectTimeoutException(), 3, null));
}
}
Modified: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientRequestExecution.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientRequestExecution.java?rev=1686589&r1=1686588&r2=1686589&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientRequestExecution.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientRequestExecution.java Sat Jun 20 12:12:03 2015
@@ -126,6 +126,7 @@ public class TestClientRequestExecution
@Override
public boolean retryRequest(
+ final HttpRequest request,
final IOException exception,
final int executionCount,
final HttpContext context) {
@@ -166,6 +167,7 @@ public class TestClientRequestExecution
@Override
public boolean retryRequest(
+ final HttpRequest request,
final IOException exception,
final int executionCount,
final HttpContext context) {
Modified: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/execchain/TestRetryExec.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/execchain/TestRetryExec.java?rev=1686589&r1=1686588&r2=1686589&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/execchain/TestRetryExec.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/execchain/TestRetryExec.java Sat Jun 20 12:12:03 2015
@@ -29,6 +29,7 @@ package org.apache.http.impl.execchain;
import org.apache.http.Header;
import org.apache.http.HttpEntityEnclosingRequest;
import org.apache.http.HttpHost;
+import org.apache.http.HttpRequest;
import org.apache.http.client.HttpRequestRetryHandler;
import org.apache.http.client.NonRepeatableRequestException;
import org.apache.http.client.entity.EntityBuilder;
@@ -101,6 +102,7 @@ public class TestRetryExec {
});
Mockito.when(retryHandler.retryRequest(
+ Mockito.<HttpRequest>any(),
Mockito.<IOException>any(),
Mockito.eq(1),
Mockito.<HttpContext>any())).thenReturn(Boolean.TRUE);
@@ -139,6 +141,7 @@ public class TestRetryExec {
Mockito.same(context),
Mockito.same(execAware));
Mockito.verify(retryHandler, Mockito.never()).retryRequest(
+ Mockito.<HttpRequest>any(),
Mockito.<IOException>any(),
Mockito.anyInt(),
Mockito.<HttpContext>any());
@@ -173,6 +176,7 @@ public class TestRetryExec {
});
Mockito.when(retryHandler.retryRequest(
+ Mockito.<HttpRequest>any(),
Mockito.<IOException>any(),
Mockito.eq(1),
Mockito.<HttpContext>any())).thenReturn(Boolean.TRUE);