Fix parsing of array query properties in JSON payload

These were translated to request properties by calling
asString on the payload, which returns an empty value for arrays.
toString returns the correct value.

This also improves error messages for Slime JSON parsing
of queries, and in general.

4 files changed, 69 insertions, 15 deletions

diff --git a/vespajlib/src/main/java/com/yahoo/slime/JsonDecoder.java b/vespajlib/src/main/java/com/yahoo/slime/JsonDecoder.java
index a761c91f64f..f199fefd185 100644
--- a/vespajlib/src/main/java/com/yahoo/slime/JsonDecoder.java
+++ b/vespajlib/src/main/java/com/yahoo/slime/JsonDecoder.java
@@ -1,9 +1,12 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.slime;
 
+import com.yahoo.text.Text;
 import com.yahoo.text.Utf8;
+import org.w3c.dom.CharacterData;
 
 import java.io.ByteArrayOutputStream;
+import java.nio.charset.StandardCharsets;
 
 /**
  * A port of the C++ json decoder intended to be fast.
@@ -56,7 +59,7 @@ public class JsonDecoder {
             case '-': case '0': case '1': case '2': case '3': case '4': case '5':
             case '6': case '7': case '8': case '9': decodeNumber(inserter); return;
         }
-        in.fail("invalid initial character for value");
+        in.fail("Expected start of value but got " + characterToReadableString(c));
     }
 
     @SuppressWarnings("fallthrough")
@@ -86,14 +89,13 @@ public class JsonDecoder {
         }
     }
 
-    private void expect(byte [] expected) {
-        int i;
-        for (i = 0; i < expected.length && skip(expected[i]); i++)
-            ;
-        if (i != expected.length) {
-            in.fail("unexpected character");
+    private void expect(byte[] expected) {
+        for (int i = 0; i < expected.length; i++) {
+            if ( ! skip(expected[i])) {
+                in.fail("Unexpected " + characterToReadableString(c));
+                return;
+            }
         }
-
     }
 
     private void decodeArray(Inserter inserter) {
@@ -170,7 +172,7 @@ public class JsonDecoder {
                 case 't': buf.write((byte) '\t'); break;
                 case 'u': writeUtf8(dequoteUtf16(), buf, 0xffffff80); continue;
                 default:
-                    in.fail("invalid quoted char(" + c + ")");
+                    in.fail("Invalid quoted char(" + c + ")");
                     break;
                 }
                 next();
@@ -185,7 +187,7 @@ public class JsonDecoder {
                 }
                 break;
             case '\0':
-                in.fail("unterminated string");
+                in.fail("Unterminated string");
                 return Utf8.toString(buf.toByteArray());
             default:
                 buf.write(c);
@@ -216,10 +218,10 @@ public class JsonDecoder {
                 if (low >= 0xdc00 && low < 0xe000) {
                     codepoint = 0x10000 + ((codepoint - 0xd800) << 10) + (low - 0xdc00);
                 } else {
-                    in.fail("missing low surrogate");
+                    in.fail("Missing low surrogate");
                 }
             } else if (codepoint < 0xe000) { // low
-                in.fail("unexpected low surrogate");
+                in.fail("Unexpected low surrogate");
             }
         }
         return codepoint;
@@ -246,7 +248,7 @@ public class JsonDecoder {
             case 'e': case 'E': ret = (ret << 4) | 0xe; break;
             case 'f': case 'F': ret = (ret << 4) | 0xf; break;
             default:
-                in.fail("invalid hex character");
+                in.fail("Invalid hex character");
                 return 0;
             }
             next();
@@ -255,7 +257,7 @@ public class JsonDecoder {
     }
 
     private void next() {
-        if (!in.eof()) {
+        if ( ! in.eof()) {
             c = in.getByte();
         } else {
             c = 0;
@@ -281,4 +283,13 @@ public class JsonDecoder {
         }
     }
 
+    private String characterToReadableString(int codePoint) {
+        if (codePoint == 0)
+            return "end of data";
+        else if (Text.isDisplayable(codePoint))
+            return "character '" + String.valueOf(Character.toChars(c)) + "'";
+        else
+            return "character code " + c;
+    }
+
 }
diff --git a/vespajlib/src/main/java/com/yahoo/slime/SlimeInserter.java b/vespajlib/src/main/java/com/yahoo/slime/SlimeInserter.java
index 2b048c42f73..30ebb5d2a3d 100644
--- a/vespajlib/src/main/java/com/yahoo/slime/SlimeInserter.java
+++ b/vespajlib/src/main/java/com/yahoo/slime/SlimeInserter.java
@@ -4,8 +4,9 @@ package com.yahoo.slime;
 /**
  * Helper class for inserting values into a Slime object.
  * For justification read Inserter documentation.
- **/
+ */
 public final class SlimeInserter implements Inserter {
+
     private Slime target;
 
     public SlimeInserter(Slime target) {
@@ -26,4 +27,5 @@ public final class SlimeInserter implements Inserter {
     public final Cursor insertDATA(byte[] value)   { return target.setData(value); }
     public final Cursor insertARRAY()              { return target.setArray(); }
     public final Cursor insertOBJECT()             { return target.setObject(); }
+
 }
diff --git a/vespajlib/src/main/java/com/yahoo/text/Text.java b/vespajlib/src/main/java/com/yahoo/text/Text.java
index 027521ec1ad..0acb2407e68 100644
--- a/vespajlib/src/main/java/com/yahoo/text/Text.java
+++ b/vespajlib/src/main/java/com/yahoo/text/Text.java
@@ -109,6 +109,33 @@ public final class Text {
         return OptionalInt.empty();
     }
 
+    /** Returns whether the given code point is displayable. */
+    public static boolean isDisplayable(int codePoint) {
+        switch (Character.getType(codePoint)) {
+            case Character.CONNECTOR_PUNCTUATION :
+            case Character.DASH_PUNCTUATION :
+            case Character.START_PUNCTUATION :
+            case Character.END_PUNCTUATION :
+            case Character.INITIAL_QUOTE_PUNCTUATION :
+            case Character.FINAL_QUOTE_PUNCTUATION:
+            case Character.OTHER_PUNCTUATION :
+            case Character.LETTER_NUMBER :
+            case Character.OTHER_LETTER :
+            case Character.LOWERCASE_LETTER :
+            case Character.TITLECASE_LETTER :
+            case Character.MODIFIER_LETTER :
+            case Character.UPPERCASE_LETTER :
+            case Character.DECIMAL_DIGIT_NUMBER :
+            case Character.OTHER_NUMBER :
+            case Character.CURRENCY_SYMBOL :
+            case Character.OTHER_SYMBOL :
+            case Character.MATH_SYMBOL :
+                return true;
+            default :
+                return false;
+        }
+    }
+
     private static StringBuilder lazy(StringBuilder sb, String s, int i) {
         if (sb == null) {
             sb = new StringBuilder(s.substring(0, i));
diff --git a/vespajlib/src/test/java/com/yahoo/text/TextTestCase.java b/vespajlib/src/test/java/com/yahoo/text/TextTestCase.java
index f6831e3d221..e733b838c39 100644
--- a/vespajlib/src/test/java/com/yahoo/text/TextTestCase.java
+++ b/vespajlib/src/test/java/com/yahoo/text/TextTestCase.java
@@ -7,6 +7,7 @@ import java.util.OptionalInt;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 public class TextTestCase {
 
@@ -47,4 +48,17 @@ public class TextTestCase {
         assertEquals(OptionalInt.of(0xD800), Text.validateTextString(new StringBuilder().appendCodePoint(0xD800).append(0x0000).toString()));
     }
 
+    @Test
+    public void testIsDisplayable() {
+        assertTrue(Text.isDisplayable('A'));
+        assertTrue(Text.isDisplayable('a'));
+        assertTrue(Text.isDisplayable('5'));
+        assertTrue(Text.isDisplayable(','));
+        assertTrue(Text.isDisplayable('\"'));
+        assertTrue(Text.isDisplayable('}'));
+        assertTrue(Text.isDisplayable('-'));
+        assertFalse(Text.isDisplayable(' '));
+        assertFalse(Text.isDisplayable(0));
+    }
+
 }