Avoid building full Slime AST when decoding json just to throw it all away when making it into a string.

Instead try cheap stream parsing with jackson and keep strings for as long as possible.

4 files changed, 125 insertions, 51 deletions

diff --git a/container-search/src/main/java/com/yahoo/search/handler/Json2SingleLevelMap.java b/container-search/src/main/java/com/yahoo/search/handler/Json2SingleLevelMap.java
new file mode 100644
index 00000000000..5b8c99506c5
--- /dev/null
+++ b/container-search/src/main/java/com/yahoo/search/handler/Json2SingleLevelMap.java
@@ -0,0 +1,81 @@
+package com.yahoo.search.handler;
+
+import com.fasterxml.jackson.core.JsonLocation;
+import com.fasterxml.jackson.core.JsonParseException;
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.core.JsonToken;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.yahoo.processing.IllegalInputException;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Parser that consumes json and creates a single level key value map by dotting nested objects.
+ * This is specially tailored for the json query input coming as post.
+ * It does the cheapest possible json parsing delaying number parsing to where it is needed and avoids dreaded toString()
+ * of complicated json object trees.
+ *
+ * @author baldersheim
+ */
+class Json2SingleLevelMap {
+    private static final ObjectMapper jsonMapper = new ObjectMapper();
+    private final byte [] buf;
+    private final JsonParser parser;
+    Json2SingleLevelMap(InputStream data) {
+        try {
+            buf = data.readAllBytes();
+            parser = jsonMapper.createParser(buf);
+        } catch (IOException e) {
+            throw new RuntimeException("Problem reading POSTed data", e);
+        }
+    }
+    Map<String, String> parse() {
+        try {
+            Map<String, String> map = new HashMap<>();
+            if (parser.nextToken() != JsonToken.START_OBJECT) {
+                throw new IllegalInputException("Expected start of object, got '" + parser.currentToken() + "'");
+            }
+            parse(map, "");
+            return map;
+        } catch (JsonParseException e) {
+            throw new IllegalInputException("Json parse error.", e);
+        } catch (IOException e) {
+            throw new RuntimeException("Problem reading POSTed data", e);
+        }
+    }
+    void parse(Map<String, String> map, String parent) throws IOException {
+        for (parser.nextToken(); parser.getCurrentToken() != JsonToken.END_OBJECT; parser.nextToken()) {
+            String fieldName = parent + parser.getCurrentName();
+            JsonToken token = parser.nextToken();
+            if ((token == JsonToken.VALUE_STRING) ||
+                    (token == JsonToken.VALUE_NUMBER_FLOAT) ||
+                    (token == JsonToken.VALUE_NUMBER_INT) ||
+                    (token == JsonToken.VALUE_TRUE) ||
+                    (token == JsonToken.VALUE_FALSE) ||
+                    (token == JsonToken.VALUE_NULL)) {
+                map.put(fieldName, parser.getText());
+            } else if (token == JsonToken.START_ARRAY) {
+                map.put(fieldName, skipChildren(parser, buf));
+            } else if (token == JsonToken.START_OBJECT) {
+                if (fieldName.equals("select.where") || fieldName.equals("select.grouping")) {
+                    map.put(fieldName, skipChildren(parser, buf));
+                } else {
+                    parse(map, fieldName + ".");
+                }
+            } else {
+                throw new IllegalInputException("In field '" + fieldName + "', got unknown json token '" + token.asString() + "'");
+            }
+        }
+    }
+    private String skipChildren(JsonParser parser, byte [] input) throws IOException {
+        JsonLocation start = parser.getCurrentLocation();
+        parser.skipChildren();
+        JsonLocation end = parser.getCurrentLocation();
+        int offset = (int)start.getByteOffset() - 1;
+        return new String(input, offset, (int)(end.getByteOffset() - offset), StandardCharsets.UTF_8);
+    }
+}
diff --git a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java
index 2c26a302ece..76702a1d4e0 100644
--- a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java
+++ b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java
@@ -17,7 +17,6 @@ import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.container.jdisc.LoggingRequestHandler;
 import com.yahoo.container.jdisc.RequestHandlerSpec;
 import com.yahoo.container.jdisc.VespaHeaders;
-import com.yahoo.io.IOUtils;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.jdisc.Request;
 import com.yahoo.language.process.Embedder;
@@ -43,14 +42,13 @@ import com.yahoo.search.searchchain.SearchChainRegistry;
 import com.yahoo.search.statistics.ElapsedTime;
 import com.yahoo.slime.Inspector;
 import com.yahoo.slime.ObjectTraverser;
-import com.yahoo.slime.SlimeUtils;
 import com.yahoo.yolean.Exceptions;
 import com.yahoo.yolean.trace.TraceNode;
 
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.Executor;
@@ -490,23 +488,9 @@ public class SearchHandler extends LoggingRequestHandler {
             ||  ! JSON_CONTENT_TYPE.equals(getMediaType(request)))
             return request.propertyMap();
 
-        Inspector inspector;
-        try {
-            // Use an 4k buffer, that should be plenty for most json requests to pass in a single chunk
-            byte[] byteArray = IOUtils.readBytes(request.getData(), 4096);
-            inspector = SlimeUtils.jsonToSlime(byteArray).get();
-            if (inspector.field("error_message").valid()) {
-                throw new IllegalInputException("Illegal query: " + inspector.field("error_message").asString() + " at: '" +
-                                                new String(inspector.field("offending_input").asData(), StandardCharsets.UTF_8) + "'");
-            }
-
-        } catch (IOException e) {
-            throw new RuntimeException("Problem reading POSTed data", e);
-        }
+        Map<String, String> requestMap = new Json2SingleLevelMap(request.getData()).parse();
 
         // Add fields from JSON to the request map
-        Map<String, String> requestMap = new HashMap<>();
-        createRequestMapping(inspector, requestMap, "");
         requestMap.putAll(request.propertyMap());
 
         if (requestMap.containsKey("yql") && (requestMap.containsKey("select.where") || requestMap.containsKey("select.grouping")) )
@@ -517,35 +501,13 @@ public class SearchHandler extends LoggingRequestHandler {
         return requestMap;
     }
 
+    @Deprecated // TODO: Remove on Vespa 9
     public void createRequestMapping(Inspector inspector, Map<String, String> map, String parent) {
-        inspector.traverse((ObjectTraverser) (key, value) -> {
-            String qualifiedKey = parent + key;
-            switch (value.type()) {
-                case BOOL:
-                    map.put(qualifiedKey, Boolean.toString(value.asBool()));
-                    break;
-                case DOUBLE:
-                    map.put(qualifiedKey, Double.toString(value.asDouble()));
-                    break;
-                case LONG:
-                    map.put(qualifiedKey, Long.toString(value.asLong()));
-                    break;
-                case STRING:
-                    map.put(qualifiedKey , value.asString());
-                    break;
-                case ARRAY:
-                    map.put(qualifiedKey, value.toString()); // XXX: Causes parsing the JSON twice (Query.setPropertiesFromRequestMap)
-                    break;
-                case OBJECT:
-                    if (qualifiedKey.equals("select.where") || qualifiedKey.equals("select.grouping")) {
-                        map.put(qualifiedKey, value.toString());  // XXX: Causes parsing the JSON twice (Query.setPropertiesFromRequestMap)
-                        break;
-                    }
-                    createRequestMapping(value, map, qualifiedKey + ".");
-                    break;
-            }
-
-        });
+        try {
+            new Json2SingleLevelMap(new ByteArrayInputStream(inspector.toString().getBytes(StandardCharsets.UTF_8))).parse(map, parent);
+        } catch (IOException e) {
+            throw new RuntimeException("Failed creating request mapping for parent '" + parent + "'", e);
+        }
     }
 
     @Override
diff --git a/container-search/src/test/java/com/yahoo/search/handler/test/JSONSearchHandlerTestCase.java b/container-search/src/test/java/com/yahoo/search/handler/JSONSearchHandlerTestCase.java
index dd78cfefe28..b92bf68e099 100644
--- a/container-search/src/test/java/com/yahoo/search/handler/test/JSONSearchHandlerTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/handler/JSONSearchHandlerTestCase.java
@@ -1,5 +1,5 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.search.handler.test;
+package com.yahoo.search.handler;
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -24,6 +24,7 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
+import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
@@ -324,8 +325,7 @@ public class JSONSearchHandlerTestCase {
         json.set("select", select);
 
         Inspector inspector = SlimeUtils.jsonToSlime(json.toString().getBytes(StandardCharsets.UTF_8)).get();
-        Map<String, String> map = new HashMap<>();
-        searchHandler.createRequestMapping(inspector, map, "");
+        Map<String, String> map = new Json2SingleLevelMap(new ByteArrayInputStream(inspector.toString().getBytes(StandardCharsets.UTF_8))).parse();
 
         JsonNode processedWhere = jsonMapper.readTree(map.get("select.where"));
         JsonTestHelper.assertJsonEquals(where.toString(), processedWhere.toString());
@@ -510,8 +510,7 @@ public class JSONSearchHandlerTestCase {
 
         // Create mapping
         Inspector inspector = SlimeUtils.jsonToSlime(json.toString().getBytes(StandardCharsets.UTF_8)).get();
-        Map<String, String> map = new HashMap<>();
-        searchHandler.createRequestMapping(inspector, map, "");
+        Map<String, String> map = new Json2SingleLevelMap(new ByteArrayInputStream(inspector.toString().getBytes(StandardCharsets.UTF_8))).parse();
 
         // Create GET-request with same query
         String url = uri + "&model.sources=source1%2Csource2&select=_all&model.language=en&presentation.timing=false&pos.attribute=default&pos.radius=71234m&model.searchPath=node1&nocachewrite=false&ranking.matchPhase.maxHits=100&presentation.summary=none" +
diff --git a/container-search/src/test/java/com/yahoo/search/handler/Json2SinglelevelMapTestCase.java b/container-search/src/test/java/com/yahoo/search/handler/Json2SinglelevelMapTestCase.java
new file mode 100644
index 00000000000..d8db88323c7
--- /dev/null
+++ b/container-search/src/test/java/com/yahoo/search/handler/Json2SinglelevelMapTestCase.java
@@ -0,0 +1,32 @@
+package com.yahoo.search.handler;
+
+import org.junit.Test;
+
+import java.io.ByteArrayInputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class Json2SinglelevelMapTestCase {
+    @Test
+    public void testDecodeString() {
+        Map<String, String> m = new Json2SingleLevelMap(new ByteArrayInputStream("{\"yql\":\"text\", \"f1\":7.3, \"i1\":7, \"t\":true, \"f\":false,  \"n\":null, \"a\":[0.786, 0.193]}".getBytes(StandardCharsets.UTF_8))).parse();
+        assertEquals(7, m.size());
+        assertTrue(m.containsKey("yql"));
+        assertTrue(m.containsKey("f1"));
+        assertTrue(m.containsKey("i1"));
+        assertTrue(m.containsKey("t"));
+        assertTrue(m.containsKey("f"));
+        assertTrue(m.containsKey("n"));
+        assertTrue(m.containsKey("a"));
+        assertEquals("text", m.get("yql"));
+        assertEquals("7.3", m.get("f1"));
+        assertEquals("7", m.get("i1"));
+        assertEquals("true", m.get("t"));
+        assertEquals("false", m.get("f"));
+        assertEquals("null", m.get("n"));
+        assertEquals("[0.786, 0.193]", m.get("a"));
+    }
+}