HTTPCLIENT-781: made DefaultConnectionKeepAliveStrategy a little cleaner / more efficient

git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@671953 13f79535-47bb-0310-9956-ffa450edef68

module-client/src/examples/org/apache/http/examples/conn/ManagerConnectDirect.java

@@ -145,7 +145,7 @@ public final static void main(String[] args)
             }
 
             System.out.println("releasing connection");
-            clcm.releaseConnection(conn);
+            clcm.releaseConnection(conn, -1, null);
         }
 
     } // main

module-client/src/examples/org/apache/http/examples/conn/ManagerConnectProxy.java

@@ -173,7 +173,7 @@ public final static void main(String[] args)
             }
 
             System.out.println("releasing connection");
-            clcm.releaseConnection(conn);
+            clcm.releaseConnection(conn, -1, null);
         }
 
     } // main

module-client/src/main/java/org/apache/http/conn/ConnectionKeepAliveStrategy.java

@@ -50,7 +50,7 @@
 public interface ConnectionKeepAliveStrategy {
 
     /** Returns the TimeUnit that this uses for specifying duration. */
-    public TimeUnit getTimeUnit();
+    TimeUnit getTimeUnit();
     
     /**
      * Returns the duration of time which this connection can be safely kept
@@ -71,6 +71,6 @@ public interface ConnectionKeepAliveStrategy {
      * @return the duration which it is safe to keep the connection idle,
      *         or <=0 if no suggested duration.
      */
-    public long getKeepAliveDuration(HttpResponse response, HttpContext context);
+    long getKeepAliveDuration(HttpResponse response, HttpContext context);
     
 }

module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java

@@ -158,8 +158,10 @@ protected AbstractHttpClient(
 
     
     protected abstract ConnectionReuseStrategy createConnectionReuseStrategy();
+
     
     protected abstract ConnectionKeepAliveStrategy createConnectionKeepAliveStrategy();
+
     
     protected abstract BasicHttpProcessor createHttpProcessor();
 
@@ -269,6 +271,7 @@ public synchronized final ConnectionKeepAliveStrategy getConnectionKeepAliveStra
         }
         return keepAliveStrategy;
     }
+
     
     public synchronized void setKeepAliveStrategy(final ConnectionKeepAliveStrategy keepAliveStrategy) {
         this.keepAliveStrategy = keepAliveStrategy;

module-client/src/main/java/org/apache/http/impl/client/DefaultConnectionKeepAliveStrategy.java

@@ -30,16 +30,13 @@
  */
 package org.apache.http.impl.client;
 
-import java.util.Locale;
-import java.util.StringTokenizer;
 import java.util.concurrent.TimeUnit;
 
-import org.apache.http.Header;
+import org.apache.http.HeaderElement;
-import org.apache.http.HeaderIterator;
+import org.apache.http.HeaderElementIterator;
 import org.apache.http.HttpResponse;
-import org.apache.http.TokenIterator;
 import org.apache.http.conn.ConnectionKeepAliveStrategy;
-import org.apache.http.message.BasicTokenIterator;
+import org.apache.http.message.BasicHeaderElementIterator;
 import org.apache.http.protocol.HTTP;
 import org.apache.http.protocol.HttpContext;
 
@@ -59,73 +56,27 @@
 public class DefaultConnectionKeepAliveStrategy implements ConnectionKeepAliveStrategy {
     
     public long getKeepAliveDuration(HttpResponse response, HttpContext context) {
-        long duration = -1;
+        if (response == null) {
-        HeaderIterator hit = response.headerIterator(HTTP.CONN_KEEP_ALIVE);
+            throw new IllegalArgumentException("HTTP response may not be null");
-        
+        }
-        while(hit.hasNext() && duration==-1) {
+        HeaderElementIterator it = new BasicHeaderElementIterator(
-            Header header = hit.nextHeader();
+                response.headerIterator(HTTP.CONN_KEEP_ALIVE));
-            if(header.getValue() == null)
+        while (it.hasNext()) {
-                continue;
+            HeaderElement he = it.nextElement();
-            StringTokenizer tokenizer = new StringTokenizer(header.getValue(), ",");
+            String param = he.getName(); 
-            while(tokenizer.hasMoreTokens()) {
+            String value = he.getValue();
-                String token = tokenizer.nextToken().trim();
+            if (value != null && param.equalsIgnoreCase("timeout")) {
-                if(token.toLowerCase(Locale.US).startsWith("timeout=")) {
+                try {
-                  duration = parseTimeout(token);
+                    return Long.parseLong(value);
-                  break;
+                } catch(NumberFormatException ignore) {
                 }
             }
         }
-            
+        return -1;
-        // TODO: I'd prefer to do it this way, but BasicTokenIterator
-        // freaks out on an '=' character.
-//        if(hit.hasNext()) {
-//            try {
-//                TokenIterator it = createTokenIterator(hit);
-//                while(it.hasNext()) {
-//                    String token = it.nextToken();
-//                    if(token.toLowerCase(Locale.US).startsWith("timeout=")) {
-//                        duration = parseTimeout(token);
-//                        break;
-//                    }
-//                }
-//            } catch(ParseException pe) {
-//                // Stop trying to find it and just fall-through.
-//            }
-//        }
-        
-        return duration;
     }
     
-    /**
-     * Parses the # out of the 'timeout=#' token.
-     */
-    protected long parseTimeout(String token) {
-        // Make sure the length is valid.
-        if(token.length() == "timeout=".length())
-            return -1;
-        
-        try {
-            return Long.parseLong(token.substring("timeout=".length()));
-        } catch(NumberFormatException nfe) {
-            return -1;
-        }
-    }
-
     public TimeUnit getTimeUnit() {
         return TimeUnit.SECONDS;
     }
 
-    /**
-     * Creates a token iterator from a header iterator.
-     * This method can be overridden to replace the implementation of
-     * the token iterator.
-     *
-     * @param hit       the header iterator
-     *
-     * @return  the token iterator
-     */
-    protected TokenIterator createTokenIterator(HeaderIterator hit) {
-        return new BasicTokenIterator(hit);
-    }
-    
 }

module-client/src/test/java/org/apache/http/impl/client/TestAllHttpClientImpl.java

@@ -46,6 +46,7 @@ public static Test suite() {
         suite.addTest(TestRequestWrapper.suite());
         suite.addTest(TestDefaultClientRequestDirector.suite());
         suite.addTest(TestStatefulConnManagement.suite());
+        suite.addTest(TestDefaultConnKeepAliveStrategy.suite());
         return suite;
     }
 

module-client/src/test/java/org/apache/http/impl/client/TestDefaultConnKeepAliveStrategy.java

@@ -0,0 +1,121 @@
+/*
+ * $HeadURL: $
+ * $Revision: $
+ * $Date: $
+ * ====================================================================
+ *
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You 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.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+
+package org.apache.http.impl.client;
+
+import java.io.IOException;
+
+import junit.framework.Test;
+import junit.framework.TestCase;
+import junit.framework.TestSuite;
+
+import org.apache.http.HttpResponse;
+import org.apache.http.HttpStatus;
+import org.apache.http.HttpVersion;
+import org.apache.http.conn.ConnectionKeepAliveStrategy;
+import org.apache.http.message.BasicHttpResponse;
+import org.apache.http.message.BasicStatusLine;
+import org.apache.http.protocol.BasicHttpContext;
+import org.apache.http.protocol.HttpContext;
+
+/**
+ *  Simple tests for {@link DefaultConnectionKeepAliveStrategy}.
+ * 
+ * @version $Revision$
+ */
+public class TestDefaultConnKeepAliveStrategy extends TestCase {
+
+    // ------------------------------------------------------------ Constructor
+    public TestDefaultConnKeepAliveStrategy(final String testName) throws IOException {
+        super(testName);
+    }
+
+    // ------------------------------------------------------------------- Main
+    public static void main(String args[]) {
+        String[] testCaseName = { TestDefaultConnKeepAliveStrategy.class.getName() };
+        junit.textui.TestRunner.main(testCaseName);
+    }
+
+    // ------------------------------------------------------- TestCase Methods
+
+    public static Test suite() {
+        return new TestSuite(TestDefaultConnKeepAliveStrategy.class);
+    }
+
+    public void testIllegalResponseArg() throws Exception {
+        HttpContext context = new BasicHttpContext(null);
+        ConnectionKeepAliveStrategy keepAliveStrat = new DefaultConnectionKeepAliveStrategy();
+        try {
+            keepAliveStrat.getKeepAliveDuration(null, context);
+            fail("IllegalArgumentException should have been thrown");
+        } catch (IllegalArgumentException ex) {
+            // expected
+        }
+    }
+
+    public void testNoKeepAliveHeader() throws Exception {
+        HttpContext context = new BasicHttpContext(null);
+        HttpResponse response = new BasicHttpResponse(
+                new BasicStatusLine(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "OK"));
+        ConnectionKeepAliveStrategy keepAliveStrat = new DefaultConnectionKeepAliveStrategy();
+        long d = keepAliveStrat.getKeepAliveDuration(response, context);
+        assertEquals(-1, d);
+    }
+
+    public void testEmptyKeepAliveHeader() throws Exception {
+        HttpContext context = new BasicHttpContext(null);
+        HttpResponse response = new BasicHttpResponse(
+                new BasicStatusLine(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "OK"));
+        response.addHeader("Keep-Alive", "timeout, max=20");
+        ConnectionKeepAliveStrategy keepAliveStrat = new DefaultConnectionKeepAliveStrategy();
+        long d = keepAliveStrat.getKeepAliveDuration(response, context);
+        assertEquals(-1, d);
+    }
+
+    public void testInvalidKeepAliveHeader() throws Exception {
+        HttpContext context = new BasicHttpContext(null);
+        HttpResponse response = new BasicHttpResponse(
+                new BasicStatusLine(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "OK"));
+        response.addHeader("Keep-Alive", "timeout=whatever, max=20");
+        ConnectionKeepAliveStrategy keepAliveStrat = new DefaultConnectionKeepAliveStrategy();
+        long d = keepAliveStrat.getKeepAliveDuration(response, context);
+        assertEquals(-1, d);
+    }
+
+    public void testKeepAliveHeader() throws Exception {
+        HttpContext context = new BasicHttpContext(null);
+        HttpResponse response = new BasicHttpResponse(
+                new BasicStatusLine(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "OK"));
+        response.addHeader("Keep-Alive", "timeout=300, max=20");
+        ConnectionKeepAliveStrategy keepAliveStrat = new DefaultConnectionKeepAliveStrategy();
+        long d = keepAliveStrat.getKeepAliveDuration(response, context);
+        assertEquals(300, d);
+    }
+
+}