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 2008/05/29 21:41:01 UTC

svn commit: r661444 - in /httpcomponents/httpclient/trunk: ./ module-client/src/examples/org/apache/http/examples/client/ module-client/src/main/java/org/apache/http/client/ module-client/src/main/java/org/apache/http/impl/client/ module-client/src/tes...

Author: olegk
Date: Thu May 29 12:41:00 2008
New Revision: 661444

URL: http://svn.apache.org/viewvc?rev=661444&view=rev
Log:
Fixed request re-generation logic when retrying a failed request. Auto-generated headers will no accumulate.

Modified:
    httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
    httpcomponents/httpclient/trunk/module-client/src/examples/org/apache/http/examples/client/ClientFormLogin.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/client/ClientRequestDirector.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/client/HttpClient.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/DefaultHttpClient.java
    httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java
    httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/ClientConnAdapterMockup.java

Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?rev=661444&r1=661443&r2=661444&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Thu May 29 12:41:00 2008
@@ -1,6 +1,10 @@
 Changes since 4.0 Alpha 4
 -------------------
 
+* Fixed request re-generation logic when retrying a failed request.
+  Auto-generated headers will no accumulate.
+  Contributed by Oleg Kalnichevski <olegk at apache.org>
+
 * [HTTPCLIENT-424] Preemptive authentication no longer limited to BASIC
   scheme only. HttpClient can be customized to authenticate preemptively
   with DIGEST scheme.

Modified: httpcomponents/httpclient/trunk/module-client/src/examples/org/apache/http/examples/client/ClientFormLogin.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/examples/org/apache/http/examples/client/ClientFormLogin.java?rev=661444&r1=661443&r2=661444&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/examples/org/apache/http/examples/client/ClientFormLogin.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/examples/org/apache/http/examples/client/ClientFormLogin.java Thu May 29 12:41:00 2008
@@ -37,8 +37,6 @@
 import org.apache.http.client.entity.UrlEncodedFormEntity;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpPost;
-import org.apache.http.client.params.CookiePolicy;
-import org.apache.http.client.params.ClientPNames;
 import org.apache.http.cookie.Cookie;
 import org.apache.http.impl.client.DefaultHttpClient;
 import org.apache.http.message.BasicNameValuePair;
@@ -53,8 +51,6 @@
     public static void main(String[] args) throws Exception {
 
         DefaultHttpClient httpclient = new DefaultHttpClient();
-        httpclient.getParams().setParameter(
-                ClientPNames.COOKIE_POLICY, CookiePolicy.BROWSER_COMPATIBILITY);
 
         HttpGet httpget = new HttpGet("https://portal.sun.com/portal/dt");
 

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/client/ClientRequestDirector.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/client/ClientRequestDirector.java?rev=661444&r1=661443&r2=661444&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/client/ClientRequestDirector.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/client/ClientRequestDirector.java Thu May 29 12:41:00 2008
@@ -52,11 +52,6 @@
  * connections for reading the response entity. Such connections
  * MUST be released, but that is out of the scope of a request director.
  *
- * <p>
- * This interface and it's implementations replace the
- * <code>HttpMethodDirector</code> in HttpClient 3.
- * </p>
- *
  * @author <a href="mailto:rolandw at apache.org">Roland Weber</a>
  *
  *

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/client/HttpClient.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/client/HttpClient.java?rev=661444&r1=661443&r2=661444&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/client/HttpClient.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/client/HttpClient.java Thu May 29 12:41:00 2008
@@ -91,7 +91,6 @@
      * @throws HttpException            in case of a problem
      * @throws IOException              in case of an IO problem
      *                                     or the connection was aborted
-     * <br/><i @@@>timeout exceptions?</i>
      */
     HttpResponse execute(HttpUriRequest request)
         throws HttpException, IOException
@@ -115,7 +114,6 @@
      * @throws HttpException    in case of a problem
      * @throws IOException      in case of an IO problem
      *                             or the connection was aborted
-     * <br/><i @@@>timeout exceptions?</i>
      */
     HttpResponse execute(HttpUriRequest request, HttpContext context)
         throws HttpException, IOException
@@ -141,7 +139,6 @@
      * @throws HttpException    in case of a problem
      * @throws IOException      in case of an IO problem
      *                             or the connection was aborted
-     * <br/><i @@@>timeout exceptions?</i>
      */
     HttpResponse execute(HttpHost target, HttpRequest request)
         throws HttpException, IOException
@@ -168,7 +165,6 @@
      * @throws HttpException    in case of a problem
      * @throws IOException      in case of an IO problem
      *                             or the connection was aborted
-     * <br/><i @@@>timeout exceptions?</i>
      */
     HttpResponse execute(HttpHost target, HttpRequest request,
                          HttpContext context)

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java?rev=661444&r1=661443&r2=661444&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java Thu May 29 12:41:00 2008
@@ -61,6 +61,7 @@
 import org.apache.http.protocol.HttpContext;
 import org.apache.http.protocol.BasicHttpContext;
 import org.apache.http.protocol.HttpProcessor;
+import org.apache.http.protocol.HttpRequestExecutor;
 
 /**
  * Convenience base class for HTTP client implementations.
@@ -78,6 +79,9 @@
     /** The parameters. */
     private HttpParams defaultParams;
 
+    /** The request executor. */
+    private HttpRequestExecutor requestExec;
+
     /** The connection manager. */
     private ClientConnectionManager connManager;
 
@@ -137,6 +141,9 @@
     protected abstract HttpContext createHttpContext();
 
     
+    protected abstract HttpRequestExecutor createRequestExecutor();
+
+
     protected abstract ClientConnectionManager createClientConnectionManager();
 
 
@@ -196,7 +203,6 @@
     }
 
 
-    // non-javadoc, see interface HttpClient
     public synchronized final ClientConnectionManager getConnectionManager() {
         if (connManager == null) {
             connManager = createClientConnectionManager();
@@ -205,8 +211,12 @@
     }
 
 
-    // no setConnectionManager(), too dangerous to replace while in use
-    // derived classes may offer that method at their own risk
+    public synchronized final HttpRequestExecutor getRequestExecutor() {
+        if (requestExec == null) {
+            requestExec = createRequestExecutor();
+        }
+        return requestExec;
+    }
 
 
     public synchronized final AuthSchemeRegistry getAuthSchemes() {
@@ -512,6 +522,7 @@
             }
             // Create a director for this request
             director = createClientRequestDirector(
+                    getRequestExecutor(),
                     getConnectionManager(),
                     getConnectionReuseStrategy(),
                     getRoutePlanner(),
@@ -524,19 +535,12 @@
                     determineParams(request));
         }
 
-        HttpResponse response = director.execute(target, request, execContext);
-        // If the response depends on the connection, the director
-        // will have set up an auto-release input stream.
-
-        //@@@ "finalize" response, to allow for buffering of entities?
-        //@@@ here or in director?
-
-        return response;
-
+        return director.execute(target, request, execContext);
     } // execute
 
     
     protected ClientRequestDirector createClientRequestDirector(
+            final HttpRequestExecutor requestExec,
             final ClientConnectionManager conman,
             final ConnectionReuseStrategy reustrat,
             final HttpRoutePlanner rouplan,
@@ -548,6 +552,7 @@
             final UserTokenHandler stateHandler,
             final HttpParams params) {
         return new DefaultClientRequestDirector(
+                requestExec,
                 conman,
                 reustrat,
                 rouplan,

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java?rev=661444&r1=661443&r2=661444&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java Thu May 29 12:41:00 2008
@@ -154,6 +154,7 @@
     private final AuthState proxyAuthState;
     
     public DefaultClientRequestDirector(
+            final HttpRequestExecutor requestExec,
             final ClientConnectionManager conman,
             final ConnectionReuseStrategy reustrat,
             final HttpRoutePlanner rouplan,
@@ -165,6 +166,10 @@
             final UserTokenHandler userTokenHandler,
             final HttpParams params) {
 
+        if (requestExec == null) {
+            throw new IllegalArgumentException
+                ("Request executor may not be null.");
+        }
         if (conman == null) {
             throw new IllegalArgumentException
                 ("Client connection manager may not be null.");
@@ -205,6 +210,7 @@
             throw new IllegalArgumentException
                 ("HTTP parameters may not be null");
         }
+        this.requestExec       = requestExec;
         this.connManager       = conman;
         this.reuseStrategy     = reustrat;
         this.routePlanner      = rouplan;
@@ -215,7 +221,6 @@
         this.proxyAuthHandler  = proxyAuthHandler;
         this.userTokenHandler  = userTokenHandler; 
         this.params            = params;
-        this.requestExec       = new HttpRequestExecutor();
 
         this.managedConn       = null;
         
@@ -312,6 +317,15 @@
                         iox.initCause(interrupted);
                         throw iox;
                     }
+
+                    if (HttpConnectionParams.isStaleCheckingEnabled(params)) {
+                        // validate connection
+                        LOG.debug("Stale connection check");
+                        if (managedConn.isStale()) {
+                            LOG.debug("Stale connection detected");
+                            managedConn.close();
+                        }
+                    }
                 }
 
                 if (orig instanceof AbortableHttpRequest) {
@@ -333,16 +347,10 @@
                     break;
                 }
 
-                if (HttpConnectionParams.isStaleCheckingEnabled(params)) {
-                    // validate connection
-                    LOG.debug("Stale connection check");
-                    if (managedConn.isStale()) {
-                        LOG.debug("Stale connection detected");
-                        managedConn.close();
-                        continue;
-                    }
-                }
-
+                // Clear autogenerated headers if case the request is being
+                // retried
+                wrapper.clearHeaders();
+                
                 // Re-write request URI if needed
                 rewriteRequestURI(wrapper, route);
 
@@ -367,6 +375,8 @@
                         targetAuthState);
                 context.setAttribute(ClientContext.PROXY_AUTH_STATE,
                         proxyAuthState);
+                
+                // Run request protocol interceptors
                 requestExec.preProcess(wrapper, httpProcessor, context);
                 
                 context.setAttribute(ExecutionContext.HTTP_REQUEST,
@@ -397,6 +407,7 @@
                     throw ex;
                 }
 
+                // Run response protocol interceptors
                 response.setParams(params);
                 requestExec.postProcess(response, httpProcessor, context);
                 
@@ -424,9 +435,6 @@
                         connManager.releaseConnection(managedConn);
                         managedConn = null;
                     }
-                    // In case we are going to retry the same request,
-                    // clear auto-generated headers
-                    followup.getRequest().clearHeaders();
                     roureq = followup;
                 }
                 

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/DefaultHttpClient.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/DefaultHttpClient.java?rev=661444&r1=661443&r2=661444&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/DefaultHttpClient.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/DefaultHttpClient.java Thu May 29 12:41:00 2008
@@ -72,6 +72,7 @@
 import org.apache.http.protocol.BasicHttpProcessor;
 import org.apache.http.protocol.HTTP;
 import org.apache.http.protocol.HttpContext;
+import org.apache.http.protocol.HttpRequestExecutor;
 import org.apache.http.protocol.RequestConnControl;
 import org.apache.http.protocol.RequestContent;
 import org.apache.http.protocol.RequestExpectContinue;
@@ -144,6 +145,12 @@
 
     
     @Override
+    protected HttpRequestExecutor createRequestExecutor() {
+        return new HttpRequestExecutor();
+    }
+
+
+    @Override
     protected ClientConnectionManager createClientConnectionManager() {
         SchemeRegistry registry = new SchemeRegistry();
         registry.register(

Modified: httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java?rev=661444&r1=661443&r2=661444&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java Thu May 29 12:41:00 2008
@@ -38,13 +38,17 @@
 import junit.framework.Test;
 import junit.framework.TestSuite;
 
+import org.apache.http.Header;
+import org.apache.http.HttpClientConnection;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpException;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpRequest;
+import org.apache.http.HttpRequestInterceptor;
 import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
 import org.apache.http.ProtocolVersion;
+import org.apache.http.client.HttpRequestRetryHandler;
 import org.apache.http.client.methods.AbortableHttpRequest;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.params.ClientPNames;
@@ -67,7 +71,9 @@
 import org.apache.http.params.BasicHttpParams;
 import org.apache.http.params.HttpParams;
 import org.apache.http.protocol.BasicHttpContext;
+import org.apache.http.protocol.ExecutionContext;
 import org.apache.http.protocol.HttpContext;
+import org.apache.http.protocol.HttpRequestExecutor;
 import org.apache.http.protocol.HttpRequestHandler;
 
 /**
@@ -621,4 +627,83 @@
         assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
     }
 
+    private static class FaultyHttpRequestExecutor extends HttpRequestExecutor {
+
+        private static final String MARKER = "marker";
+        
+        @Override
+        public HttpResponse execute(
+                final HttpRequest request, 
+                final HttpClientConnection conn, 
+                final HttpContext context) throws IOException, HttpException {
+            
+            Object marker = context.getAttribute(MARKER);
+            if (marker == null) {
+                context.setAttribute(MARKER, Boolean.TRUE);
+                throw new IOException("Oppsie");
+            }
+            return super.execute(request, conn, context);
+        }
+        
+    }
+    
+    private static class FaultyHttpClient extends DefaultHttpClient {
+
+        @Override
+        protected HttpRequestExecutor createRequestExecutor() {
+            return new FaultyHttpRequestExecutor();
+        }
+        
+    }
+    
+    
+    public void testAutoGeneratedHeaders() throws Exception {
+        int port = this.localServer.getServicePort();
+        this.localServer.register("*", new SimpleService());
+        
+        FaultyHttpClient client = new FaultyHttpClient(); 
+        
+        client.addRequestInterceptor(new HttpRequestInterceptor() {
+
+            public void process(
+                    final HttpRequest request, 
+                    final HttpContext context) throws HttpException, IOException {
+                request.addHeader("my-header", "stuff");
+            }
+            
+        }) ;      
+        
+        client.setHttpRequestRetryHandler(new HttpRequestRetryHandler() {
+
+            public boolean retryRequest(
+                    final IOException exception, 
+                    int executionCount, 
+                    final HttpContext context) {
+                return true;
+            }
+            
+        });
+        
+        HttpContext context = new BasicHttpContext();
+
+        String s = "http://localhost:" + port;
+        HttpGet httpget = new HttpGet(s);
+
+        HttpResponse response = client.execute(getServerHttp(), httpget, context);
+        HttpEntity e = response.getEntity();
+        if (e != null) {
+            e.consumeContent();
+        }
+
+        HttpRequest reqWrapper = (HttpRequest) context.getAttribute(
+                ExecutionContext.HTTP_REQUEST);
+        
+        assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
+
+        assertTrue(reqWrapper instanceof RequestWrapper);
+        Header[] myheaders = reqWrapper.getHeaders("my-header");
+        assertNotNull(myheaders);
+        assertEquals(1, myheaders.length);
+    }
+    
 }

Modified: httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/ClientConnAdapterMockup.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/ClientConnAdapterMockup.java?rev=661444&r1=661443&r2=661444&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/ClientConnAdapterMockup.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/ClientConnAdapterMockup.java Thu May 29 12:41:00 2008
@@ -48,7 +48,6 @@
     }
 
     public void close() {
-        throw new UnsupportedOperationException("just a mockup");
     }
 
     public HttpRoute getRoute() {