Optimize URI.create/String round trips, fix proposal for AsyncHttpClient#139

Author: Stephane Landelle

src/main/java/com/ning/http/client/Request.java

@@ -21,6 +21,7 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.InetAddress;
+import java.net.URI;
 import java.util.Collection;
 import java.util.List;
 
@@ -66,6 +67,10 @@ public static interface EntityWriter {
      */
     public String getUrl();
 
+    public URI getOriginalURI();
+    public URI getURI();
+    public URI getRawURI();
+
     /**
      * Return the InetAddress to override
      *

src/main/java/com/ning/http/client/RequestBuilderBase.java

@@ -18,6 +18,7 @@
 import static com.ning.http.util.MiscUtil.isNonEmpty;
 
 import com.ning.http.client.Request.EntityWriter;
+import com.ning.http.util.AsyncHttpProviderUtils;
 import com.ning.http.util.UTF8UrlEncoder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -38,15 +39,19 @@
 
 /**
  * Builder for {@link Request}
- *
+ * 
  * @param <T>
  */
 public abstract class RequestBuilderBase<T extends RequestBuilderBase<T>> {
     private final static Logger logger = LoggerFactory.getLogger(RequestBuilderBase.class);
 
+    private static final URI DEFAULT_REQUEST_URL = URI.create("http://localhost");
+
     private static final class RequestImpl implements Request {
         private String method;
-        private String url = null;
+        private URI originalUri = null;
+        private URI uri = null;
+        private URI rawUri = null;
         private InetAddress address = null;
         private InetAddress localAddress = null;
         private FluentCaseInsensitiveStringsMap headers = new FluentCaseInsensitiveStringsMap();
@@ -78,9 +83,7 @@ public RequestImpl(boolean useRawUrl) {
         public RequestImpl(Request prototype) {
             if (prototype != null) {
                 this.method = prototype.getMethod();
-                String prototypeUrl = prototype.getUrl();
-                int pos = prototypeUrl.indexOf("?");
-                this.url = pos > 0 ? prototypeUrl.substring(0, pos) : prototypeUrl;
+                this.originalUri = prototype.getOriginalURI();
                 this.address = prototype.getInetAddress();
                 this.localAddress = prototype.getLocalAddress();
                 this.headers = new FluentCaseInsensitiveStringsMap(prototype.getHeaders());
@@ -117,12 +120,6 @@ public String getMethod() {
             return method;
         }
 
-        /* @Override */
-
-        public String getUrl() {
-            return toUrl(true);
-        }
-
         public InetAddress getInetAddress() {
             return address;
         }
@@ -131,34 +128,66 @@ public InetAddress getLocalAddress() {
             return localAddress;
         }
 
-        private String toUrl(boolean encode) {
+        private String removeTrailingSlash(URI uri) {
+            String uriString = uri.toString();
+            if (uriString.endsWith("/")) {
+                return uriString.substring(0, uriString.length() - 1);
+            } else {
+                return uriString;
+            }
+        }
 
-            if (url == null) {
+        /* @Override */
+        public String getUrl() {
+            return removeTrailingSlash(getURI());
+        }
+
+        /* @Override */
+        public String getRawUrl() {
+            return removeTrailingSlash(getRawURI());
+        }
+
+        public URI getOriginalURI() {
+            return originalUri;
+        }
+
+        public URI getURI() {
+            if (uri == null)
+                uri = toURI(true);
+            return uri;
+        }
+
+        public URI getRawURI() {
+            if (rawUri == null)
+                rawUri = toURI(false);
+            return rawUri;
+        }
+
+        private URI toURI(boolean encode) {
+
+            if (originalUri == null) {
                 logger.debug("setUrl hasn't been invoked. Using http://localhost");
-                url = "http://localhost/";
+                originalUri = DEFAULT_REQUEST_URL;
             }
 
-            String uri = url;
-            if (!uri.startsWith("ws")) {
-                try {
-                    uri = URI.create(url).toURL().toString();
-                } catch (Throwable e) {
-                    throw new IllegalArgumentException("Illegal URL: " + url, e);
-                }
+            AsyncHttpProviderUtils.validateSupportedScheme(originalUri);
+
+            StringBuilder builder = new StringBuilder();
+            builder.append(originalUri.getScheme()).append("://").append(originalUri.getAuthority());
+            if (isNonEmpty(originalUri.getRawPath())) {
+                builder.append(originalUri.getRawPath());
+            } else {
+                builder.append("/");
             }
 
             if (isNonEmpty(queryParams)) {
 
-                StringBuilder builder = new StringBuilder();
-                if (!url.substring(8).contains("/")) { // no other "/" than http[s]:// -> http://localhost:1234
-                    builder.append("/");
-                }
                 builder.append("?");
 
-                for (Iterator<Entry<String, List<String>>> i = queryParams.iterator(); i.hasNext(); ) {
+                for (Iterator<Entry<String, List<String>>> i = queryParams.iterator(); i.hasNext();) {
                     Map.Entry<String, List<String>> param = i.next();
                     String name = param.getKey();
-                    for (Iterator<String> j = param.getValue().iterator(); j.hasNext(); ) {
+                    for (Iterator<String> j = param.getValue().iterator(); j.hasNext();) {
                         String value = j.next();
                         if (encode) {
                             UTF8UrlEncoder.appendEncoded(builder, name);
@@ -181,14 +210,9 @@ private String toUrl(boolean encode) {
                         builder.append('&');
                     }
                 }
-                uri += builder.toString();
             }
-            return uri;
-        }
 
-        /* @Override */
-        public String getRawUrl() {
-            return toUrl(false);
+            return URI.create(builder.toString());
         }
 
         /* @Override */
@@ -292,12 +316,12 @@ public String getBodyEncoding() {
         }
 
         public ConnectionPoolKeyStrategy getConnectionPoolKeyStrategy() {
-        	return connectionPoolKeyStrategy;
-       }
-        
+            return connectionPoolKeyStrategy;
+        }
+
         @Override
         public String toString() {
-            StringBuilder sb = new StringBuilder(url);
+            StringBuilder sb = new StringBuilder(getURI().toString());
 
             sb.append("\t");
             sb.append(method);
@@ -346,7 +370,7 @@ protected RequestBuilderBase(Class<T> derived, Request prototype) {
     }
 
     public T setUrl(String url) {
-        request.url = buildUrl(url);
+        request.originalUri = buildURI(url);
         return derived.cast(this);
     }
 
@@ -360,7 +384,7 @@ public T setLocalInetAddress(InetAddress address) {
         return derived.cast(this);
     }
 
-    private String buildUrl(String url) {
+    private URI buildURI(String url) {
         URI uri = URI.create(url);
         StringBuilder buildedUrl = new StringBuilder();
 
@@ -380,7 +404,7 @@ private String buildUrl(String url) {
             if (url.indexOf("://") == -1) {
                 String s = buildedUrl.toString();
                 url = s + url.substring(uri.getScheme().length() + 1);
-                return buildUrl(url);
+                return buildURI(url);
             } else {
                 throw new IllegalArgumentException("Invalid url " + uri.toString());
             }
@@ -395,7 +419,7 @@ private String buildUrl(String url) {
                     addQueryParameter(query, null);
                 } else {
                     try {
-                        if (this.useRawUrl) {
+                        if (useRawUrl) {
                             addQueryParameter(query.substring(0, pos), query.substring(pos + 1));
                         } else {
                             addQueryParameter(URLDecoder.decode(query.substring(0, pos), "UTF-8"), URLDecoder.decode(query.substring(pos + 1), "UTF-8"));
@@ -406,10 +430,9 @@ private String buildUrl(String url) {
                 }
             }
         }
-        return buildedUrl.toString();
+        return uri;
     }
 
-
     public T setVirtualHost(String virtualHost) {
         request.virtualHost = virtualHost;
         return derived.cast(this);
@@ -612,8 +635,8 @@ public T setBodyEncoding(String charset) {
     }
 
     public T setConnectionPoolKeyStrategy(ConnectionPoolKeyStrategy connectionPoolKeyStrategy) {
-    	request.connectionPoolKeyStrategy = connectionPoolKeyStrategy;
-    	return derived.cast(this);
+        request.connectionPoolKeyStrategy = connectionPoolKeyStrategy;
+        return derived.cast(this);
     }
 
     public Request build() {
@@ -633,13 +656,7 @@ public Request build() {
     }
 
     private boolean allowBody(String method) {
-        if (method.equalsIgnoreCase("GET") || method.equalsIgnoreCase("OPTIONS")
-                && method.equalsIgnoreCase("TRACE")
-                && method.equalsIgnoreCase("HEAD")) {
-            return false;
-        } else {
-            return true;
-        }
+        return !(method.equalsIgnoreCase("GET") || method.equalsIgnoreCase("OPTIONS") || method.equalsIgnoreCase("TRACE") || method.equalsIgnoreCase("HEAD"));
     }
 
     public T addOrReplaceCookie(Cookie cookie) {

src/main/java/com/ning/http/client/providers/grizzly/GrizzlyAsyncHttpProvider.java

@@ -846,7 +846,7 @@ private boolean sendAsGrizzlyRequest(final Request request,
                 httpCtx.isWSRequest = true;
                 convertToUpgradeRequest(httpCtx);
             }
-            final URI uri = AsyncHttpProviderUtils.createUri(httpCtx.requestUrl);
+            final URI uri = httpCtx.request.getURI();
             final HttpRequestPacket.Builder builder = HttpRequestPacket.builder();
             boolean secure = "https".equals(uri.getScheme());
             builder.method(request.getMethod());
@@ -1209,7 +1209,7 @@ protected void onInitialLineParsed(HttpHeader httpHeader,
             }
             final GrizzlyResponseStatus responseStatus =
                     new GrizzlyResponseStatus((HttpResponsePacket) httpHeader,
-                            getURI(context.requestUrl),
+                            context.request.getURI(),
                             provider);
             context.responseStatus = responseStatus;
             if (context.statusHandler != null) {
@@ -1457,14 +1457,6 @@ private static HttpTransactionContext cleanup(final FilterChainContext ctx,
 
         }
 
-
-        private static URI getURI(String url) {
-
-            return AsyncHttpProviderUtils.createUri(url);
-
-        }
-
-
         private static boolean redirectCountExceeded(final HttpTransactionContext context) {
 
             return (context.redirectCount.get() > context.maxRedirectCount);
@@ -1521,7 +1513,7 @@ public boolean handleStatus(final HttpResponsePacket responsePacket,
                 final Request req = httpTransactionContext.request;
                 realm = new Realm.RealmBuilder().clone(realm)
                                 .setScheme(realm.getAuthScheme())
-                                .setUri(URI.create(httpTransactionContext.requestUrl).getPath())
+                                .setUri(httpTransactionContext.request.getURI().getPath())
                                 .setMethodName(req.getMethod())
                                 .setUsePreemptiveAuth(true)
                                 .parseWWWAuthenticateHeader(auth)
@@ -1600,9 +1592,9 @@ public boolean handleStatus(final HttpResponsePacket responsePacket,
 
                 URI orig;
                 if (httpTransactionContext.lastRedirectURI == null) {
-                    orig = AsyncHttpProviderUtils.createUri(httpTransactionContext.requestUrl);
+                    orig = httpTransactionContext.request.getURI();
                 } else {
-                    orig = AsyncHttpProviderUtils.getRedirectUri(AsyncHttpProviderUtils.createUri(httpTransactionContext.requestUrl),
+                    orig = AsyncHttpProviderUtils.getRedirectUri(httpTransactionContext.request.getURI(),
                                                                  httpTransactionContext.lastRedirectURI);
                 }
                 httpTransactionContext.lastRedirectURI = redirectURL;
@@ -2320,13 +2312,12 @@ void doAsyncTrackedConnection(final Request request,
                                       final GrizzlyResponseFuture requestFuture,
                                       final CompletionHandler<Connection> connectHandler)
         throws IOException, ExecutionException, InterruptedException {
-            final String url = request.getUrl();
             Connection c = pool.poll(getPoolKey(request));
             if (c == null) {
                 if (!connectionMonitor.acquire()) {
                     throw new IOException("Max connections exceeded");
                 }
-                doAsyncConnect(url, request, requestFuture, connectHandler);
+                doAsyncConnect(request, requestFuture, connectHandler);
             } else {
                 provider.touchConnection(c, request);
                 connectHandler.completed(c);
@@ -2338,25 +2329,23 @@ Connection obtainConnection(final Request request,
                                     final GrizzlyResponseFuture requestFuture)
         throws IOException, ExecutionException, InterruptedException, TimeoutException {
 
-            final Connection c = (obtainConnection0(request.getUrl(),
-                                                    request,
+            final Connection c = (obtainConnection0(request,
                                                     requestFuture));
             DO_NOT_CACHE.set(c, Boolean.TRUE);
             return c;
 
         }
 
-        void doAsyncConnect(final String url,
-                            final Request request,
+        void doAsyncConnect(final Request request,
                             final GrizzlyResponseFuture requestFuture,
                             final CompletionHandler<Connection> connectHandler)
         throws IOException, ExecutionException, InterruptedException {
 
-            final URI uri = AsyncHttpProviderUtils.createUri(url);
             ProxyServer proxy = getProxyServer(request);
             if (ProxyUtils.avoidProxy(proxy, request)) {
                 proxy = null;
             }
+            final URI uri = request.getURI();
             String host = ((proxy != null) ? proxy.getHost() : uri.getHost());
             int port = ((proxy != null) ? proxy.getPort() : uri.getPort());
             if(request.getLocalAddress()!=null) {
@@ -2369,12 +2358,11 @@ void doAsyncConnect(final String url,
 
         }
 
-        private Connection obtainConnection0(final String url,
-                                             final Request request,
+        private Connection obtainConnection0(final Request request,
                                              final GrizzlyResponseFuture requestFuture)
         throws IOException, ExecutionException, InterruptedException, TimeoutException {
 
-            final URI uri = AsyncHttpProviderUtils.createUri(url);
+            final URI uri = request.getURI();
             ProxyServer proxy = getProxyServer(request);
             if (ProxyUtils.avoidProxy(proxy, request)) {
                 proxy = null;
@@ -2471,7 +2459,7 @@ public void updated(Connection result) {
 
         private static String getPoolKey(final Request request) {
             final ConnectionPoolKeyStrategy keyStrategy = request.getConnectionPoolKeyStrategy();
-            return keyStrategy.getKey(AsyncHttpProviderUtils.createUri(AsyncHttpProviderUtils.getBaseUrl(request.getUrl())));
+            return keyStrategy.getKey(request.getURI());
         }
 
         // ------------------------------------------------------ Nested Classes