From 932d21df7b24a35fea571fb0817c9875805d6138 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Thu, 26 Jun 2008 17:12:45 +0000 Subject: [PATCH] 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 --- .../examples/conn/ManagerConnectDirect.java | 2 +- .../examples/conn/ManagerConnectProxy.java | 2 +- .../conn/ConnectionKeepAliveStrategy.java | 4 +- .../http/impl/client/AbstractHttpClient.java | 3 + .../DefaultConnectionKeepAliveStrategy.java | 83 +++--------- .../impl/client/TestAllHttpClientImpl.java | 1 + .../TestDefaultConnKeepAliveStrategy.java | 121 ++++++++++++++++++ 7 files changed, 146 insertions(+), 70 deletions(-) create mode 100644 module-client/src/test/java/org/apache/http/impl/client/TestDefaultConnKeepAliveStrategy.java diff --git a/module-client/src/examples/org/apache/http/examples/conn/ManagerConnectDirect.java b/module-client/src/examples/org/apache/http/examples/conn/ManagerConnectDirect.java index cd809b643..3564083fa 100644 --- a/module-client/src/examples/org/apache/http/examples/conn/ManagerConnectDirect.java +++ b/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 diff --git a/module-client/src/examples/org/apache/http/examples/conn/ManagerConnectProxy.java b/module-client/src/examples/org/apache/http/examples/conn/ManagerConnectProxy.java index bf2482f32..e87c38071 100644 --- a/module-client/src/examples/org/apache/http/examples/conn/ManagerConnectProxy.java +++ b/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 diff --git a/module-client/src/main/java/org/apache/http/conn/ConnectionKeepAliveStrategy.java b/module-client/src/main/java/org/apache/http/conn/ConnectionKeepAliveStrategy.java index b5cd23124..2c50b0338 100644 --- a/module-client/src/main/java/org/apache/http/conn/ConnectionKeepAliveStrategy.java +++ b/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); } diff --git a/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java b/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java index 080ff23b3..461c82618 100644 --- a/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java +++ b/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; diff --git a/module-client/src/main/java/org/apache/http/impl/client/DefaultConnectionKeepAliveStrategy.java b/module-client/src/main/java/org/apache/http/impl/client/DefaultConnectionKeepAliveStrategy.java index f18ffb275..2c2cf3841 100644 --- a/module-client/src/main/java/org/apache/http/impl/client/DefaultConnectionKeepAliveStrategy.java +++ b/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.HeaderIterator; +import org.apache.http.HeaderElement; +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; - HeaderIterator hit = response.headerIterator(HTTP.CONN_KEEP_ALIVE); - - while(hit.hasNext() && duration==-1) { - Header header = hit.nextHeader(); - if(header.getValue() == null) - continue; - StringTokenizer tokenizer = new StringTokenizer(header.getValue(), ","); - while(tokenizer.hasMoreTokens()) { - String token = tokenizer.nextToken().trim(); - if(token.toLowerCase(Locale.US).startsWith("timeout=")) { - duration = parseTimeout(token); - break; + if (response == null) { + throw new IllegalArgumentException("HTTP response may not be null"); + } + HeaderElementIterator it = new BasicHeaderElementIterator( + response.headerIterator(HTTP.CONN_KEEP_ALIVE)); + while (it.hasNext()) { + HeaderElement he = it.nextElement(); + String param = he.getName(); + String value = he.getValue(); + if (value != null && param.equalsIgnoreCase("timeout")) { + try { + return Long.parseLong(value); + } catch(NumberFormatException ignore) { } } } - - // 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; + return -1; } - /** - * 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); - } - } diff --git a/module-client/src/test/java/org/apache/http/impl/client/TestAllHttpClientImpl.java b/module-client/src/test/java/org/apache/http/impl/client/TestAllHttpClientImpl.java index be2cb8c88..e4d8f5154 100644 --- a/module-client/src/test/java/org/apache/http/impl/client/TestAllHttpClientImpl.java +++ b/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; } diff --git a/module-client/src/test/java/org/apache/http/impl/client/TestDefaultConnKeepAliveStrategy.java b/module-client/src/test/java/org/apache/http/impl/client/TestDefaultConnKeepAliveStrategy.java new file mode 100644 index 000000000..49a74fbf0 --- /dev/null +++ b/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 + * . + * + */ + +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); + } + +}