More document JSON validation to improve error messages

6 files changed, 70 insertions, 24 deletions

diff --git a/document/src/main/java/com/yahoo/document/DocumentPut.java b/document/src/main/java/com/yahoo/document/DocumentPut.java
index e5ddc2c67a3..5906a9ca0ba 100644
--- a/document/src/main/java/com/yahoo/document/DocumentPut.java
+++ b/document/src/main/java/com/yahoo/document/DocumentPut.java
@@ -45,4 +45,9 @@ public class DocumentPut extends DocumentOperation {
         this.document = newDocument;
     }
 
+    @Override
+    public String toString() {
+        return "put of document " + getId();
+    }
+
 }
diff --git a/document/src/main/java/com/yahoo/document/json/JsonReader.java b/document/src/main/java/com/yahoo/document/json/JsonReader.java
index 6b4605623b3..b7818b06b03 100644
--- a/document/src/main/java/com/yahoo/document/json/JsonReader.java
+++ b/document/src/main/java/com/yahoo/document/json/JsonReader.java
@@ -28,7 +28,6 @@ import static com.yahoo.document.json.readers.JsonParserHelpers.expectArrayStart
  * @author Steinar Knutsen
  * @author dybis
  */
-@Beta
 public class JsonReader {
 
     public Optional<DocumentParseInfo> parseDocument() throws IOException {
diff --git a/document/src/main/java/com/yahoo/document/json/TokenBuffer.java b/document/src/main/java/com/yahoo/document/json/TokenBuffer.java
index e6fc8171a1a..e20845bfa54 100644
--- a/document/src/main/java/com/yahoo/document/json/TokenBuffer.java
+++ b/document/src/main/java/com/yahoo/document/json/TokenBuffer.java
@@ -13,9 +13,10 @@ import com.google.common.base.Preconditions;
 /**
  * Helper class to enable lookahead in the token stream.
  *
- * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a>
+ * @author Steinar Knutsen
  */
 public class TokenBuffer {
+
     public static final class Token {
         public final JsonToken token;
         public final String name;
@@ -42,6 +43,9 @@ public class TokenBuffer {
         }
     }
 
+    /** Returns whether any tokens are available in this */
+    public boolean isEmpty() { return size() == 0; }
+
     public JsonToken next() {
         buffer.removeFirst();
         Token t = buffer.peekFirst();
@@ -52,16 +56,25 @@ public class TokenBuffer {
         return t.token;
     }
 
+    /** Returns the current token without changing position, or null if none */
     public JsonToken currentToken() {
-        return buffer.peekFirst().token;
+        Token token = buffer.peekFirst();
+        if (token == null) return null;
+        return token.token;
     }
 
+    /** Returns the current token name without changing position, or null if none */
     public String currentName() {
-        return buffer.peekFirst().name;
+        Token token = buffer.peekFirst();
+        if (token == null) return null;
+        return token.name;
     }
 
+    /** Returns the current token text without changing position, or null if none */
     public String currentText() {
-        return buffer.peekFirst().text;
+        Token token = buffer.peekFirst();
+        if (token == null) return null;
+        return token.text;
     }
 
     public int size() {
diff --git a/document/src/main/java/com/yahoo/document/json/document/DocumentParser.java b/document/src/main/java/com/yahoo/document/json/document/DocumentParser.java
index 744ec12bb23..3fc2c941b99 100644
--- a/document/src/main/java/com/yahoo/document/json/document/DocumentParser.java
+++ b/document/src/main/java/com/yahoo/document/json/document/DocumentParser.java
@@ -33,31 +33,43 @@ public class DocumentParser {
         this.parser = parser;
     }
 
+    /**
+     * Parses a single document and returns it.
+     * Returns empty is we have reached the end of the stream.
+     */
     public Optional<DocumentParseInfo> parse(Optional<DocumentId> documentIdArg) throws IOException {
         indentLevel = 0;
         DocumentParseInfo documentParseInfo = new DocumentParseInfo();
         documentIdArg.ifPresent(documentId -> documentParseInfo.documentId = documentId);
+        boolean foundItems = false;
         do {
-            parseOneItem(documentParseInfo, documentIdArg.isPresent() /* doc id set externally */);
+            foundItems |= parseOneItem(documentParseInfo, documentIdArg.isPresent() /* doc id set externally */);
         } while (indentLevel > 0L);
 
-        if (documentParseInfo.documentId != null) {
-            return Optional.of(documentParseInfo);
+        if (documentParseInfo.documentId == null) {
+            if (foundItems)
+                throw new IllegalArgumentException("Missing a document operation ('put', 'update' or 'remove')");
+            else
+                return Optional.empty();
         }
-        return Optional.empty();
+        return Optional.of(documentParseInfo);
     }
 
-    private void parseOneItem(DocumentParseInfo documentParseInfo, boolean docIdAndOperationIsSetExternally) throws IOException {
+    /**
+     * Parses one item from the stream.
+     *
+     * @return whether an item was found
+     */
+    private boolean parseOneItem(DocumentParseInfo documentParseInfo, boolean docIdAndOperationIsSetExternally) throws IOException {
         parser.nextValue();
         processIndent();
-        if (parser.getCurrentName() == null) {
-            return;
-        }
+        if (parser.getCurrentName() == null) return false;
         if (indentLevel == 1L) {
             handleIdentLevelOne(documentParseInfo, docIdAndOperationIsSetExternally);
         } else if (indentLevel == 2L) {
             handleIdentLevelTwo(documentParseInfo);
         }
+        return true;
     }
 
     private void processIndent() {
diff --git a/document/src/main/java/com/yahoo/document/json/readers/VespaJsonDocumentReader.java b/document/src/main/java/com/yahoo/document/json/readers/VespaJsonDocumentReader.java
index 8e381c8e2fe..6189c12c8c9 100644
--- a/document/src/main/java/com/yahoo/document/json/readers/VespaJsonDocumentReader.java
+++ b/document/src/main/java/com/yahoo/document/json/readers/VespaJsonDocumentReader.java
@@ -74,6 +74,8 @@ public class VespaJsonDocumentReader {
     // Exposed for unit testing...
     public void readPut(TokenBuffer buffer, DocumentPut put) {
         try {
+            if (buffer.isEmpty()) // no "fields" map
+                throw new IllegalArgumentException(put + " is missing a 'fields' map");
             populateComposite(buffer, put.getDocument());
         } catch (JsonReaderException e) {
             throw JsonReaderException.addDocId(e, put.getId());
@@ -82,6 +84,8 @@ public class VespaJsonDocumentReader {
 
     // Exposed for unit testing...
     public void readUpdate(TokenBuffer buffer, DocumentUpdate update) {
+        if (buffer.isEmpty())
+            throw new IllegalArgumentException("update of document " + update.getId() + " is missing a 'fields' map");
         expectObjectStart(buffer.currentToken());
         int localNesting = buffer.nesting();
 
diff --git a/document/src/test/java/com/yahoo/document/json/JsonReaderTestCase.java b/document/src/test/java/com/yahoo/document/json/JsonReaderTestCase.java
index 443ff27cd20..1fd45cb07c4 100644
--- a/document/src/test/java/com/yahoo/document/json/JsonReaderTestCase.java
+++ b/document/src/test/java/com/yahoo/document/json/JsonReaderTestCase.java
@@ -1112,33 +1112,46 @@ public class JsonReaderTestCase {
                     "]");
 
             new JsonReader(types, jsonToInputStream(jsonData), parserFactory).next();
+            fail("Expected exception");
         }
         catch (IllegalArgumentException e) {
-            assertEquals("", e.getMessage());
+            assertEquals("Missing a document operation ('put', 'update' or 'remove')", e.getMessage());
         }
     }
 
     @Test
-    public void testMissingFieldsMap() { todo ...
+    public void testMissingFieldsMapInPut() {
         try {
             String jsonData = inputJson(
                     "[",
                     "      {",
-                    "          'fields': {",
-                    "              'actualarray': {",
-                    "                  'add': [",
-                    "                      'person',",
-                    "                      'another person'",
-                    "                   ]",
-                    "              }",
-                    "          }",
+                    "          'put': 'id:unittest:smoke::whee'",
+                    "      }",
+                    "]");
+
+            new JsonReader(types, jsonToInputStream(jsonData), parserFactory).next();
+            fail("Expected exception");
+        }
+        catch (IllegalArgumentException e) {
+            assertEquals("put of document id:unittest:smoke::whee is missing a 'fields' map", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testMissingFieldsMapInUpdate() {
+        try {
+            String jsonData = inputJson(
+                    "[",
+                    "      {",
+                    "          'update': 'id:unittest:smoke::whee'",
                     "      }",
                     "]");
 
             new JsonReader(types, jsonToInputStream(jsonData), parserFactory).next();
+            fail("Expected exception");
         }
         catch (IllegalArgumentException e) {
-            assertEquals("", e.getMessage());
+            assertEquals("update of document id:unittest:smoke::whee is missing a 'fields' map", e.getMessage());
         }
     }