make http a bit faster by improving parsing of qstring

modules/elasticsearch/src/main/java/org/elasticsearch/http/HttpRequest.java

@@ -22,7 +22,7 @@ package org.elasticsearch.http;
 import org.elasticsearch.rest.RestRequest;
 
 /**
- * @author kimchy (Shay Banon)
+ * @author kimchy (shay.banon)
  */
 public interface HttpRequest extends RestRequest {
 

modules/elasticsearch/src/main/java/org/elasticsearch/http/HttpServer.java

@@ -98,7 +98,7 @@ public class HttpServer extends AbstractLifecycleComponent<HttpServer> {
         transport.close();
     }
 
-    private void internalDispatchRequest(final HttpRequest request, final HttpChannel channel) {
+    void internalDispatchRequest(final HttpRequest request, final HttpChannel channel) {
         final HttpServerHandler httpHandler = getHandler(request);
         if (httpHandler == null) {
             restController.dispatchRequest(request, channel);
@@ -148,13 +148,6 @@ public class HttpServer extends AbstractLifecycleComponent<HttpServer> {
     }
 
     private String getPath(HttpRequest request) {
-        String uri = request.uri();
+        return request.path();
-        int questionMarkIndex = uri.indexOf('?');
-        if (questionMarkIndex == -1) {
-            return uri;
-        }
-        return uri.substring(0, questionMarkIndex);
     }
-
-
 }

modules/elasticsearch/src/main/java/org/elasticsearch/http/netty/NettyHttpRequest.java

@@ -19,39 +19,42 @@
 
 package org.elasticsearch.http.netty;
 
-import org.elasticsearch.ElasticSearchIllegalArgumentException;
 import org.elasticsearch.http.HttpRequest;
-import org.elasticsearch.util.Booleans;
+import org.elasticsearch.rest.support.AbstractRestRequest;
-import org.elasticsearch.util.SizeValue;
+import org.elasticsearch.rest.support.RestUtils;
-import org.elasticsearch.util.TimeValue;
 import org.jboss.netty.buffer.ChannelBufferInputStream;
 import org.jboss.netty.handler.codec.http.HttpHeaders;
 import org.jboss.netty.handler.codec.http.HttpMethod;
-import org.jboss.netty.handler.codec.http.QueryStringDecoder;
 
 import java.io.InputStream;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.regex.Pattern;
-
-import static org.elasticsearch.util.SizeValue.*;
-import static org.elasticsearch.util.TimeValue.*;
 
 /**
- * @author kimchy (Shay Banon)
+ * @author kimchy (shay.banon)
  */
-public class NettyHttpRequest implements HttpRequest {
+public class NettyHttpRequest extends AbstractRestRequest implements HttpRequest {
-
-    private final Pattern commaPattern = Pattern.compile(",");
 
     private final org.jboss.netty.handler.codec.http.HttpRequest request;
 
-    private QueryStringDecoder queryStringDecoder;
+    private Map<String, String> params;
+
+    private String path;
 
     public NettyHttpRequest(org.jboss.netty.handler.codec.http.HttpRequest request) {
         this.request = request;
-        this.queryStringDecoder = new QueryStringDecoder(request.getUri());
+        this.params = new HashMap<String, String>();
+
+        String uri = request.getUri();
+        int pathEndPos = uri.indexOf('?');
+        if (pathEndPos < 0) {
+            this.path = uri;
+        } else {
+            this.path = uri.substring(0, pathEndPos);
+            RestUtils.decodeQueryString(uri, pathEndPos + 1, params);
+        }
     }
 
     @Override public Method method() {
@@ -75,6 +78,14 @@ public class NettyHttpRequest implements HttpRequest {
         return request.getUri();
     }
 
+    @Override public String path() {
+        return path;
+    }
+
+    @Override public Map<String, String> params() {
+        return params;
+    }
+
     @Override public boolean hasContent() {
         return request.getContent().readableBytes() > 0;
     }
@@ -109,75 +120,11 @@ public class NettyHttpRequest implements HttpRequest {
         return request.getHeader(HttpHeaders.Names.COOKIE);
     }
 
-    @Override public float paramAsFloat(String key, float defaultValue) {
-        String sValue = param(key);
-        if (sValue == null) {
-            return defaultValue;
-        }
-        try {
-            return Float.parseFloat(sValue);
-        } catch (NumberFormatException e) {
-            throw new ElasticSearchIllegalArgumentException("Failed to parse float parameter [" + key + "] with value [" + sValue + "]", e);
-        }
-    }
-
-    @Override public int paramAsInt(String key, int defaultValue) {
-        String sValue = param(key);
-        if (sValue == null) {
-            return defaultValue;
-        }
-        try {
-            return Integer.parseInt(sValue);
-        } catch (NumberFormatException e) {
-            throw new ElasticSearchIllegalArgumentException("Failed to parse int parameter [" + key + "] with value [" + sValue + "]", e);
-        }
-    }
-
-    @Override public boolean paramAsBoolean(String key, boolean defaultValue) {
-        return Booleans.parseBoolean(param(key), defaultValue);
-    }
-
-    @Override public Boolean paramAsBoolean(String key, Boolean defaultValue) {
-        String sValue = param(key);
-        if (sValue == null) {
-            return defaultValue;
-        }
-        return !(sValue.equals("false") || sValue.equals("0") || sValue.equals("off"));
-    }
-
-    @Override public TimeValue paramAsTime(String key, TimeValue defaultValue) {
-        return parseTimeValue(param(key), defaultValue);
-    }
-
-    @Override public SizeValue paramAsSize(String key, SizeValue defaultValue) {
-        return parseSizeValue(param(key), defaultValue);
-    }
-
-    @Override public String[] paramAsStringArray(String key, String[] defaultValue) {
-        String value = param(key);
-        if (value == null) {
-            return defaultValue;
-        }
-        return commaPattern.split(value);
-    }
-
     @Override public boolean hasParam(String key) {
-        return queryStringDecoder.getParameters().containsKey(key);
+        return params.containsKey(key);
     }
 
     @Override public String param(String key) {
-        List<String> keyParams = params(key);
+        return params.get(key);
-        if (keyParams == null || keyParams.isEmpty()) {
-            return null;
-        }
-        return keyParams.get(0);
-    }
-
-    @Override public List<String> params(String key) {
-        return queryStringDecoder.getParameters().get(key);
-    }
-
-    @Override public Map<String, List<String>> params() {
-        return queryStringDecoder.getParameters();
     }
 }

modules/elasticsearch/src/main/java/org/elasticsearch/rest/RestController.java

@@ -106,11 +106,6 @@ public class RestController extends AbstractLifecycleComponent<RestController> {
     }
 
     private String getPath(RestRequest request) {
-        String uri = request.uri();
+        return request.path();
-        int questionMarkIndex = uri.indexOf('?');
-        if (questionMarkIndex == -1) {
-            return uri;
-        }
-        return uri.substring(0, questionMarkIndex);
     }
 }

modules/elasticsearch/src/main/java/org/elasticsearch/rest/RestRequest.java

@@ -29,7 +29,7 @@ import java.util.Map;
 import java.util.Set;
 
 /**
- * @author kimchy (Shay Banon)
+ * @author kimchy (shay.banon)
  */
 public interface RestRequest extends ToJson.Params {
 
@@ -39,8 +39,16 @@ public interface RestRequest extends ToJson.Params {
 
     Method method();
 
+    /**
+     * The uri of the rest request, with the query string.
+     */
     String uri();
 
+    /**
+     * The path part of the URI (without the query string).
+     */
+    String path();
+
     boolean hasContent();
 
     InputStream contentAsStream();
@@ -75,7 +83,5 @@ public interface RestRequest extends ToJson.Params {
 
     SizeValue paramAsSize(String key, SizeValue defaultValue);
 
-    List<String> params(String key);
+    Map<String, String> params();
-
-    Map<String, List<String>> params();
 }

modules/elasticsearch/src/main/java/org/elasticsearch/rest/action/get/RestGetAction.java

@@ -30,10 +30,8 @@ import org.elasticsearch.util.json.JsonBuilder;
 import org.elasticsearch.util.settings.Settings;
 
 import java.io.IOException;
-import java.util.List;
 import java.util.regex.Pattern;
 
-import static com.google.common.collect.Lists.*;
 import static org.elasticsearch.rest.RestRequest.Method.*;
 import static org.elasticsearch.rest.RestResponse.Status.*;
 import static org.elasticsearch.rest.action.support.RestJsonBuilder.*;
@@ -62,22 +60,13 @@ public class RestGetAction extends BaseRestHandler {
         getRequest.operationThreaded(true);
 
 
-        List<String> fields = request.params("field");
         String sField = request.param("fields");
         if (sField != null) {
             String[] sFields = fieldsPattern.split(sField);
             if (sFields != null) {
-                if (fields == null) {
+                getRequest.fields(sFields);
-                    fields = newArrayListWithExpectedSize(sField.length());
-                }
-                for (String field : sFields) {
-                    fields.add(field);
-                }
             }
         }
-        if (fields != null) {
-            getRequest.fields(fields.toArray(new String[fields.size()]));
-        }
 
 
         client.get(getRequest, new ActionListener<GetResponse>() {

modules/elasticsearch/src/main/java/org/elasticsearch/rest/action/index/RestIndexAction.java

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.rest.action.index;
 
-import com.google.common.collect.ImmutableList;
 import com.google.inject.Inject;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.index.IndexRequest;
@@ -49,7 +48,7 @@ public class RestIndexAction extends BaseRestHandler {
 
     final class CreateHandler implements RestHandler {
         @Override public void handleRequest(RestRequest request, RestChannel channel) {
-            request.params().put("op_type", ImmutableList.of("create"));
+            request.params().put("op_type", "create");
             RestIndexAction.this.handleRequest(request, channel);
         }
     }

modules/elasticsearch/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java

@@ -36,7 +36,6 @@ import org.elasticsearch.util.json.JsonBuilder;
 import org.elasticsearch.util.settings.Settings;
 
 import java.io.IOException;
-import java.util.List;
 import java.util.regex.Pattern;
 
 import static org.elasticsearch.rest.RestRequest.Method.*;
@@ -173,10 +172,6 @@ public class RestSearchAction extends BaseRestHandler {
         searchSourceBuilder.queryParserName(request.param("query_parser_name"));
         searchSourceBuilder.explain(request.paramAsBoolean("explain", null));
 
-        List<String> fields = request.params("field");
-        if (fields != null && !fields.isEmpty()) {
-            searchSourceBuilder.fields(fields);
-        }
         String sField = request.param("fields");
         if (sField != null) {
             String[] sFields = fieldsPattern.split(sField);
@@ -187,8 +182,9 @@ public class RestSearchAction extends BaseRestHandler {
             }
         }
 
-        List<String> sorts = request.params("sort");
+        String sSorts = request.param("sort");
-        if (sorts != null && !sorts.isEmpty()) {
+        if (sSorts != null) {
+            String[] sorts = fieldsPattern.split(sSorts);
             for (String sort : sorts) {
                 int delimiter = sort.lastIndexOf(":");
                 if (delimiter != -1) {

modules/elasticsearch/src/main/java/org/elasticsearch/rest/action/terms/RestTermsAction.java

@@ -33,8 +33,6 @@ import org.elasticsearch.util.json.JsonBuilder;
 import org.elasticsearch.util.settings.Settings;
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.regex.Pattern;
 
 import static org.elasticsearch.rest.RestResponse.Status.*;
@@ -71,20 +69,11 @@ public class RestTermsAction extends BaseRestHandler {
             }
             termsRequest.operationThreading(operationThreading);
 
-            List<String> fields = request.params("field");
             String sField = request.param("fields");
             if (sField != null) {
                 String[] sFields = fieldsPattern.split(sField);
-                if (sFields != null) {
+                termsRequest.fields(sFields);
-                    if (fields == null) {
-                        fields = new ArrayList<String>();
-                    }
-                    for (String field : sFields) {
-                        fields.add(field);
-                    }
-                }
             }
-            termsRequest.fields(fields.toArray(new String[fields.size()]));
 
             termsRequest.from(request.param("from"));
             termsRequest.to(request.param("to"));

modules/elasticsearch/src/main/java/org/elasticsearch/rest/support/AbstractRestRequest.java

@@ -0,0 +1,91 @@
+/*
+ * Licensed to Elastic Search and Shay Banon under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. Elastic Search 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.
+ */
+
+package org.elasticsearch.rest.support;
+
+import org.elasticsearch.ElasticSearchIllegalArgumentException;
+import org.elasticsearch.rest.RestRequest;
+import org.elasticsearch.util.Booleans;
+import org.elasticsearch.util.SizeValue;
+import org.elasticsearch.util.TimeValue;
+
+import java.util.regex.Pattern;
+
+import static org.elasticsearch.util.SizeValue.*;
+import static org.elasticsearch.util.TimeValue.*;
+
+/**
+ * @author kimchy (shay.banon)
+ */
+public abstract class AbstractRestRequest implements RestRequest {
+
+    private static final Pattern commaPattern = Pattern.compile(",");
+
+    @Override public float paramAsFloat(String key, float defaultValue) {
+        String sValue = param(key);
+        if (sValue == null) {
+            return defaultValue;
+        }
+        try {
+            return Float.parseFloat(sValue);
+        } catch (NumberFormatException e) {
+            throw new ElasticSearchIllegalArgumentException("Failed to parse float parameter [" + key + "] with value [" + sValue + "]", e);
+        }
+    }
+
+    @Override public int paramAsInt(String key, int defaultValue) {
+        String sValue = param(key);
+        if (sValue == null) {
+            return defaultValue;
+        }
+        try {
+            return Integer.parseInt(sValue);
+        } catch (NumberFormatException e) {
+            throw new ElasticSearchIllegalArgumentException("Failed to parse int parameter [" + key + "] with value [" + sValue + "]", e);
+        }
+    }
+
+    @Override public boolean paramAsBoolean(String key, boolean defaultValue) {
+        return Booleans.parseBoolean(param(key), defaultValue);
+    }
+
+    @Override public Boolean paramAsBoolean(String key, Boolean defaultValue) {
+        String sValue = param(key);
+        if (sValue == null) {
+            return defaultValue;
+        }
+        return !(sValue.equals("false") || sValue.equals("0") || sValue.equals("off"));
+    }
+
+    @Override public TimeValue paramAsTime(String key, TimeValue defaultValue) {
+        return parseTimeValue(param(key), defaultValue);
+    }
+
+    @Override public SizeValue paramAsSize(String key, SizeValue defaultValue) {
+        return parseSizeValue(param(key), defaultValue);
+    }
+
+    @Override public String[] paramAsStringArray(String key, String[] defaultValue) {
+        String value = param(key);
+        if (value == null) {
+            return defaultValue;
+        }
+        return commaPattern.split(value);
+    }
+}

modules/elasticsearch/src/main/java/org/elasticsearch/rest/support/RestUtils.java

@@ -0,0 +1,65 @@
+/*
+ * Licensed to Elastic Search and Shay Banon under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. Elastic Search 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.
+ */
+
+package org.elasticsearch.rest.support;
+
+import java.io.UnsupportedEncodingException;
+import java.net.URLDecoder;
+import java.nio.charset.UnsupportedCharsetException;
+import java.util.Map;
+
+/**
+ * @author kimchy (shay.banon)
+ */
+public class RestUtils {
+
+    public static void decodeQueryString(String queryString, int fromIndex, Map<String, String> params) {
+        if (fromIndex < 0) {
+            return;
+        }
+        if (fromIndex >= queryString.length()) {
+            return;
+        }
+        int toIndex;
+        while ((toIndex = queryString.indexOf('&', fromIndex)) >= 0) {
+            int idx = queryString.indexOf('=', fromIndex);
+            if (idx < 0) {
+                continue;
+            }
+            params.put(decodeComponent(queryString.substring(fromIndex, idx)), decodeComponent(queryString.substring(idx + 1, toIndex)));
+            fromIndex = toIndex + 1;
+        }
+        int idx = queryString.indexOf('=', fromIndex);
+        if (idx < 0) {
+            return;
+        }
+        params.put(decodeComponent(queryString.substring(fromIndex, idx)), decodeComponent(queryString.substring(idx + 1)));
+    }
+
+    private static String decodeComponent(String s) {
+        if (s == null) {
+            return "";
+        }
+        try {
+            return URLDecoder.decode(s, "UTF8");
+        } catch (UnsupportedEncodingException e) {
+            throw new UnsupportedCharsetException("UTF8");
+        }
+    }
+}

modules/elasticsearch/src/main/java/org/elasticsearch/transport/TransportService.java

@@ -45,9 +45,9 @@ public class TransportService extends AbstractLifecycleComponent<TransportServic
 
     private final ThreadPool threadPool;
 
-    private final ConcurrentMap<String, TransportRequestHandler> serverHandlers = newConcurrentMap();
+    final ConcurrentMap<String, TransportRequestHandler> serverHandlers = newConcurrentMap();
 
-    private final NonBlockingHashMapLong<TransportResponseHandler> clientHandlers = new NonBlockingHashMapLong<TransportResponseHandler>();
+    final NonBlockingHashMapLong<TransportResponseHandler> clientHandlers = new NonBlockingHashMapLong<TransportResponseHandler>();
 
     final AtomicLong requestIds = new AtomicLong();
 

modules/elasticsearch/src/main/java/org/elasticsearch/util/path/PathTrie.java

@@ -21,8 +21,6 @@ package org.elasticsearch.util.path;
 
 import com.google.common.collect.ImmutableMap;
 
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Map;
 import java.util.regex.Pattern;
 
@@ -135,7 +133,7 @@ public class PathTrie<T> {
             return namedWildcard != null;
         }
 
-        public T retrieve(String[] path, int index, Map<String, List<String>> params) {
+        public T retrieve(String[] path, int index, Map<String, String> params) {
             if (index >= path.length)
                 return null;
 
@@ -191,7 +189,7 @@ public class PathTrie<T> {
         return retrieve(path, null);
     }
 
-    public T retrieve(String path, Map<String, List<String>> params) {
+    public T retrieve(String path, Map<String, String> params) {
         if (path.length() == 0) {
             return rootValue;
         }
@@ -207,12 +205,7 @@ public class PathTrie<T> {
         return root.retrieve(strings, index, params);
     }
 
-    private static void put(Map<String, List<String>> params, String key, String value) {
+    private static void put(Map<String, String> params, String key, String value) {
-        List<String> list = params.get(key);
+        params.put(key, value);
-        if (list == null) {
-            list = new ArrayList<String>(1);
-            params.put(key, list);
-        }
-        list.add(value);
     }
 }

modules/elasticsearch/src/test/java/org/elasticsearch/rest/util/RestUtilsTests.java

@@ -0,0 +1,58 @@
+/*
+ * Licensed to Elastic Search and Shay Banon under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. Elastic Search 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.
+ */
+
+package org.elasticsearch.rest.util;
+
+import org.elasticsearch.rest.support.RestUtils;
+import org.testng.annotations.Test;
+
+import java.util.Map;
+
+import static com.google.common.collect.Maps.*;
+import static org.hamcrest.MatcherAssert.*;
+import static org.hamcrest.Matchers.*;
+
+/**
+ * @author kimchy (shay.banon)
+ */
+@Test
+public class RestUtilsTests {
+
+    @Test
+    public void testDecodeQueryString() {
+        Map<String, String> params = newHashMap();
+
+        String uri = "something?test=value";
+        RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params);
+        assertThat(params.size(), equalTo(1));
+        assertThat(params.get("test"), equalTo("value"));
+
+        params.clear();
+        uri = "something?test=value&test1=value1";
+        RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params);
+        assertThat(params.size(), equalTo(2));
+        assertThat(params.get("test"), equalTo("value"));
+        assertThat(params.get("test1"), equalTo("value1"));
+
+        params.clear();
+        uri = "something";
+        RestUtils.decodeQueryString(uri, uri.indexOf('?') + 1, params);
+        assertThat(params.size(), equalTo(0));
+    }
+}

modules/elasticsearch/src/test/java/org/elasticsearch/util/path/PathTrieTests.java

@@ -21,10 +21,9 @@ package org.elasticsearch.util.path;
 
 import org.testng.annotations.Test;
 
-import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 
+import static com.google.common.collect.Maps.*;
 import static org.hamcrest.MatcherAssert.*;
 import static org.hamcrest.Matchers.*;
 
@@ -53,11 +52,11 @@ public class PathTrieTests {
         assertThat(trie.retrieve("a/b/c/d"), nullValue());
         assertThat(trie.retrieve("g/t/x"), equalTo("three"));
 
-        Map<String, List<String>> params = new HashMap<String, List<String>>();
+        Map<String, String> params = newHashMap();
         assertThat(trie.retrieve("index1/insert/12", params), equalTo("bingo"));
         assertThat(params.size(), equalTo(2));
-        assertThat(params.get("index").get(0), equalTo("index1"));
+        assertThat(params.get("index"), equalTo("index1"));
-        assertThat(params.get("docId").get(0), equalTo("12"));
+        assertThat(params.get("docId"), equalTo("12"));
     }
 
     @Test public void testEmptyPath() {