hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ol...@apache.org
Subject 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...
Date Sat, 20 Jun 2015 12:12:03 GMT
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);



Mime
View raw message