Merge branch 'master' into freva/fieldpath-parsing

# Conflicts:
#	document/src/main/java/com/yahoo/document/json/document/DocumentParser.java

5 files changed, 144 insertions, 100 deletions

diff --git a/document/src/main/java/com/yahoo/document/PositionDataType.java b/document/src/main/java/com/yahoo/document/PositionDataType.java
index a2a1c6012a1..816fcd7bdf7 100644
--- a/document/src/main/java/com/yahoo/document/PositionDataType.java
+++ b/document/src/main/java/com/yahoo/document/PositionDataType.java
@@ -7,6 +7,10 @@ import com.yahoo.document.datatypes.Struct;
 import com.yahoo.geo.DegreesParser;
 import com.yahoo.document.serialization.XmlStream;
 
+import java.text.DecimalFormat;
+import java.text.DecimalFormatSymbols;
+import java.util.Locale;
+
 /**
  * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a>
  */
@@ -20,6 +24,19 @@ public final class PositionDataType {
     private static final Field FFIELD_X = INSTANCE.getField(FIELD_X);
     private static final Field FFIELD_Y = INSTANCE.getField(FIELD_Y);
 
+    private static final DecimalFormat degreeFmt;
+
+    static {
+        degreeFmt = new DecimalFormat("0.0#####", DecimalFormatSymbols.getInstance(Locale.ENGLISH));
+        degreeFmt.setMinimumIntegerDigits(1);
+        degreeFmt.setMinimumFractionDigits(1);
+        degreeFmt.setMaximumFractionDigits(6);
+    }
+
+    static String fmtD(double degrees) {
+        return degreeFmt.format(degrees);
+    }
+
     private PositionDataType() {
         // unreachable
     }
@@ -29,10 +46,10 @@ public final class PositionDataType {
         double ns = getYValue(pos).getInteger() / 1.0e6;
         double ew = getXValue(pos).getInteger() / 1.0e6;
         buf.append(ns < 0 ? "S" : "N");
-        buf.append(ns < 0 ? (-ns) : ns);
+        buf.append(fmtD(ns < 0 ? (-ns) : ns));
         buf.append(";");
         buf.append(ew < 0 ? "W" : "E");
-        buf.append(ew < 0 ? (-ew) : ew);
+        buf.append(fmtD(ew < 0 ? (-ew) : ew));
         return buf.toString();
     }
 
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 51c623cdbda..e8cade15254 100644
--- a/document/src/main/java/com/yahoo/document/json/JsonReader.java
+++ b/document/src/main/java/com/yahoo/document/json/JsonReader.java
@@ -25,7 +25,6 @@ import java.io.InputStream;
 import java.util.Optional;
 
 import static com.yahoo.document.json.JsonReader.ReaderState.END_OF_FEED;
-import static com.yahoo.document.json.document.DocumentParser.parseDocumentsFields;
 import static com.yahoo.document.json.readers.AddRemoveCreator.createAdds;
 import static com.yahoo.document.json.readers.AddRemoveCreator.createRemoves;
 import static com.yahoo.document.json.readers.CompositeReader.populateComposite;
@@ -41,14 +40,14 @@ import static com.yahoo.document.json.readers.SingleValueReader.readSingleUpdate
  * valid JSON representation of a feed.
  *
  * @author Steinar Knutsen
- * @since 5.1.25
+ * @author dybis
  */
 @Beta
 public class JsonReader {
 
-    // Only used for testing.
     public Optional<DocumentParseInfo> parseDocument() throws IOException {
-        return DocumentParser.parseDocument(parser);
+        DocumentParser documentParser = new DocumentParser(parser);
+        return documentParser.parse(Optional.empty());
     }
 
     private static final String UPDATE_REMOVE = "remove";
@@ -81,9 +80,10 @@ public class JsonReader {
      */
     public DocumentOperation readSingleDocument(DocumentParser.SupportedOperation operationType, String docIdString) {
         DocumentId docId = new DocumentId(docIdString);
-        DocumentParseInfo documentParseInfo = null;
+        final DocumentParseInfo documentParseInfo;
         try {
-            documentParseInfo = parseDocumentsFields(parser, docId);
+            DocumentParser documentParser = new DocumentParser(parser);
+            documentParseInfo = documentParser.parse(Optional.of(docId)).get();
         } catch (IOException e) {
             state = END_OF_FEED;
             throw new RuntimeException(e);
@@ -108,7 +108,7 @@ public class JsonReader {
         }
         Optional<DocumentParseInfo> documentParseInfo;
         try {
-            documentParseInfo = DocumentParser.parseDocument(parser);
+            documentParseInfo = parseDocument();
         } catch (IOException r) {
             // Jackson is not able to recover from structural parse errors
             state = END_OF_FEED;
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 edb2ba0be00..4902dc65f3d 100644
--- a/document/src/main/java/com/yahoo/document/json/TokenBuffer.java
+++ b/document/src/main/java/com/yahoo/document/json/TokenBuffer.java
@@ -64,7 +64,7 @@ public class TokenBuffer {
         return buffer.peekFirst().text;
     }
 
-    int size() {
+    public int size() {
         return buffer.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 1c3f3af58b0..a9fad40912b 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
@@ -9,8 +9,11 @@ import com.yahoo.document.json.readers.DocumentParseInfo;
 import java.io.IOException;
 import java.util.Optional;
 
-import static com.yahoo.document.json.readers.JsonParserHelpers.expectObjectStart;
-
+/**
+ * Parses a document operation.
+ *
+ * @author dybis
+ */
 public class DocumentParser {
     public enum SupportedOperation {
         PUT, UPDATE, REMOVE
@@ -23,56 +26,101 @@ public class DocumentParser {
     public static final String FIELDS = "fields";
     public static final String FIELDPATHS = "fieldpaths";
     public static final String REMOVE = "remove";
+    private final JsonParser parser;
+    private  long indentLevel;
 
-    public static Optional<DocumentParseInfo> parseDocument(JsonParser parser) throws IOException {
-        // we should now be at the start of a feed operation or at the end of the feed
-        JsonToken token = parser.nextValue();
-        if (token == JsonToken.END_ARRAY) {
-            return Optional.empty(); // end of feed
-        }
-        expectObjectStart(token);
+    public DocumentParser(JsonParser parser) {
+        this.parser = parser;
+    }
 
+    public Optional<DocumentParseInfo> parse(Optional<DocumentId> documentIdArg) throws IOException {
+        indentLevel = 0;
         DocumentParseInfo documentParseInfo = new DocumentParseInfo();
+        if (documentIdArg.isPresent()) {
+            documentParseInfo.documentId = documentIdArg.get();
+        }
+        do {
+            parseOneItem(documentParseInfo, documentIdArg.isPresent() /* doc id set externally */);
+        } while (indentLevel > 0L);
+
+        if (documentParseInfo.documentId != null) {
+            return Optional.of(documentParseInfo);
+        }
+        return Optional.empty();
+    }
 
-        while (true) {
+    private void parseOneItem(DocumentParseInfo documentParseInfo, boolean docIdAndOperationIsSetExternally) throws IOException {
+        parser.nextValue();
+        processIndent();
+        if (parser.getCurrentName() == null) {
+            return;
+        }
+        if (indentLevel == 1L) {
+            handleIdentLevelOne(documentParseInfo, docIdAndOperationIsSetExternally);
+        } else if (indentLevel == 2L) {
+            handleIdentLevelTwo(documentParseInfo);
+        }
+    }
+
+    private void processIndent() throws IOException {
+        JsonToken currentToken = parser.currentToken();
+        if (currentToken == null) {
+            throw new IllegalArgumentException("Could not read document, no document?");
+        }
+        switch (currentToken) {
+            case START_OBJECT:
+                indentLevel++;
+                break;
+            case END_OBJECT:
+                indentLevel--;
+                return;
+            case START_ARRAY:
+                indentLevel+=10000L;
+                break;
+            case END_ARRAY:
+                indentLevel-=10000L;
+                break;
+        }
+    }
+
+    private void handleIdentLevelOne(DocumentParseInfo documentParseInfo, boolean docIdAndOperationIsSetExternally)
+            throws IOException {
+        JsonToken currentToken = parser.getCurrentToken();
+        if (currentToken == JsonToken.VALUE_TRUE || currentToken == JsonToken.VALUE_FALSE) {
             try {
-                token = parser.nextValue();
-                if ((token == JsonToken.VALUE_TRUE || token == JsonToken.VALUE_FALSE) &&
-                        CREATE_IF_NON_EXISTENT.equals(parser.getCurrentName())) {
-                    documentParseInfo.create = Optional.of(token == JsonToken.VALUE_TRUE);
-                    continue;
-                }
-                if (token == JsonToken.VALUE_STRING && CONDITION.equals(parser.getCurrentName())) {
-                    documentParseInfo.condition = Optional.of(parser.getText());
-                    continue;
-                }
-                if (token == JsonToken.START_OBJECT) {
-                    try {
-                        if (!FIELDS.equals(parser.getCurrentName())) {
-                            throw new IllegalArgumentException("Unexpected object key: " + parser.getCurrentName());
-                        }
-                    } catch (IOException e) {
-                        // TODO more specific wrapping
-                        throw new RuntimeException(e);
-                    }
-                    documentParseInfo.fieldsBuffer.bufferObject(token, parser);
-                    continue;
-                }
-                if (token == JsonToken.END_OBJECT) {
-                    if (documentParseInfo.documentId == null) {
-                        throw new RuntimeException("Did not find document operation");
-                    }
-                    return Optional.of(documentParseInfo);
-                }
-                if (token == JsonToken.VALUE_STRING) {
-                    documentParseInfo.operationType = operationNameToOperationType(parser.getCurrentName());
-                    documentParseInfo.documentId = new DocumentId(parser.getText());
-                    continue;
+                if (CREATE_IF_NON_EXISTENT.equals(parser.getCurrentName())) {
+                    documentParseInfo.create = Optional.ofNullable(parser.getBooleanValue());
+                    return;
                 }
-                throw new RuntimeException("Expected document start or document operation.");
             } catch (IOException e) {
-                throw new IllegalStateException(e);
+                throw new RuntimeException("Got IO exception while parsing document", e);
+            }
+        }
+        if ((currentToken == JsonToken.VALUE_TRUE || currentToken == JsonToken.VALUE_FALSE) &&
+                CREATE_IF_NON_EXISTENT.equals(parser.getCurrentName())) {
+            documentParseInfo.create = Optional.of(currentToken == JsonToken.VALUE_TRUE);
+        } else if (currentToken == JsonToken.VALUE_STRING && CONDITION.equals(parser.getCurrentName())) {
+            documentParseInfo.condition = Optional.of(parser.getText());
+        } else if (currentToken == JsonToken.VALUE_STRING) {
+            // Value is expected to be set in the header not in the document. Ignore any unknown field
+            // as well.
+            if (! docIdAndOperationIsSetExternally) {
+                documentParseInfo.operationType = operationNameToOperationType(parser.getCurrentName());
+                documentParseInfo.documentId = new DocumentId(parser.getText());
+            }
+        }
+    }
+
+    private  void handleIdentLevelTwo(DocumentParseInfo documentParseInfo) {
+        try {
+            JsonToken currentToken = parser.getCurrentToken();
+            // "Fields" opens a dictionary and is therefore on level two which might be surprising.
+            if (currentToken == JsonToken.START_OBJECT && FIELDS.equals(parser.getCurrentName())) {
+                documentParseInfo.fieldsBuffer.bufferObject(currentToken, parser);
+                processIndent();
             }
+        } catch (IOException e) {
+            throw new RuntimeException("Got IO exception while parsing document", e);
         }
     }
 
@@ -91,50 +139,4 @@ public class DocumentParser {
                                 "\"remove\" and \"update\" are supported.");
         }
     }
-
-    public static DocumentParseInfo parseDocumentsFields(JsonParser parser, DocumentId documentId) throws IOException {
-        long indentLevel = 0;
-        DocumentParseInfo documentParseInfo = new DocumentParseInfo();
-        documentParseInfo.documentId = documentId;
-        while (true) {
-            // we should now be at the start of a feed operation or at the end of the feed
-            JsonToken t = parser.nextValue();
-            if (t == null) {
-                throw new IllegalArgumentException("Could not read document, no document?");
-            }
-            switch (t) {
-                case START_OBJECT:
-                    indentLevel++;
-                    break;
-                case END_OBJECT:
-                    indentLevel--;
-                    break;
-                case START_ARRAY:
-                    indentLevel += 10000L;
-                    break;
-                case END_ARRAY:
-                    indentLevel -= 10000L;
-                    break;
-            }
-            try {
-                if (indentLevel == 0L && t == JsonToken.END_OBJECT) {
-                    break;
-                } else if (indentLevel == 1L && (t == JsonToken.VALUE_TRUE || t == JsonToken.VALUE_FALSE)) {
-                    if (CREATE_IF_NON_EXISTENT.equals(parser.getCurrentName())) {
-                        documentParseInfo.create = Optional.ofNullable(parser.getBooleanValue());
-                    }
-                } else if (indentLevel == 2L && t == JsonToken.START_OBJECT && FIELDS.equals(parser.getCurrentName())) {
-                    documentParseInfo.fieldsBuffer.bufferObject(t, parser);
-                    indentLevel--;
-                } else if (indentLevel == 10001L && t == JsonToken.START_ARRAY && FIELDPATHS.equals(parser.getCurrentName())) {
-                    documentParseInfo.fieldpathsBuffer.bufferArray(t, parser);
-                    indentLevel -= 10000L;
-                }
-            } catch (IOException e) {
-                throw new RuntimeException("Got IO exception while parsing document", e);
-            }
-        }
-
-        return documentParseInfo;
-    }
 }
diff --git a/document/src/test/java/com/yahoo/document/PositionTypeTestCase.java b/document/src/test/java/com/yahoo/document/PositionTypeTestCase.java
index 3fb9c8d3f9e..1e05c46f898 100644
--- a/document/src/test/java/com/yahoo/document/PositionTypeTestCase.java
+++ b/document/src/test/java/com/yahoo/document/PositionTypeTestCase.java
@@ -41,4 +41,29 @@ public class PositionTypeTestCase {
         assertEquals("foo.position", PositionDataType.getPositionSummaryFieldName("foo"));
         assertEquals("foo.distance", PositionDataType.getDistanceSummaryFieldName("foo"));
     }
+
+    @Test
+    public void requireFixedPointFormat() {
+        assertEquals("0.0", PositionDataType.fmtD(0));
+        assertEquals("123.0", PositionDataType.fmtD(123));
+        assertEquals("0.000123", PositionDataType.fmtD(0.000123));
+        assertEquals("0.000001", PositionDataType.fmtD(0.000001));
+        assertEquals("0.0", PositionDataType.fmtD(0.0000004));
+        assertEquals("999.123456", PositionDataType.fmtD(999.1234564));
+
+        Struct val1 = PositionDataType.valueOf(0, 0);
+        assertEquals("N0.0;E0.0", PositionDataType.renderAsString(val1));
+
+        Struct val2 = PositionDataType.valueOf(-1, -1);
+        assertEquals("S0.000001;W0.000001", PositionDataType.renderAsString(val2));
+
+        Struct val3 = PositionDataType.valueOf(123456789, 87654321);
+        assertEquals("N87.654321;E123.456789", PositionDataType.renderAsString(val3));
+
+        Struct val4 = PositionDataType.valueOf(Integer.MIN_VALUE, Integer.MIN_VALUE);
+        assertEquals("S2147.483648;W2147.483648", PositionDataType.renderAsString(val4));
+
+        Struct val5 = PositionDataType.valueOf(Integer.MAX_VALUE, Integer.MAX_VALUE);
+        assertEquals("N2147.483647;E2147.483647", PositionDataType.renderAsString(val5));
+    }
 }