Improve field addition in node replacement

Improve the field addition in node replacement so that it will
create any non-existent objects along the desired path to the
desired value. Modify Path Node parsing so that subpaths can
be retrieved with the necessary tokens for those subpaths.
Ensure that the addition of a new field in a JSON document
results in valid JSON.

config/src/main/java/com/typesafe/config/impl/ConfigNodeObject.java

@@ -1,6 +1,7 @@
 package com.typesafe.config.impl;
 
 import com.typesafe.config.ConfigException;
+import com.typesafe.config.ConfigSyntax;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -41,46 +42,84 @@ final class ConfigNodeObject extends ConfigNodeComplexValue {
     }
 
     public ConfigNodeObject setValueOnPath(String desiredPath, AbstractConfigNodeValue value) {
-        ConfigNodePath path = PathParser.parsePathNode(desiredPath);
+        return setValueOnPath(desiredPath, value, ConfigSyntax.CONF);
-        return setValueOnPath(path, value);
     }
 
-    private ConfigNodeObject setValueOnPath(ConfigNodePath desiredPath, AbstractConfigNodeValue value) {
+    public ConfigNodeObject setValueOnPath(String desiredPath, AbstractConfigNodeValue value, ConfigSyntax flavor) {
+        ConfigNodePath path = PathParser.parsePathNode(desiredPath, flavor);
+        return setValueOnPath(path, value, flavor);
+    }
+
+    private ConfigNodeObject setValueOnPath(ConfigNodePath desiredPath, AbstractConfigNodeValue value, ConfigSyntax flavor) {
         ConfigNodeObject node = changeValueOnPath(desiredPath.value(), value);
 
         // If the desired Path did not exist, add it
         if (node.render().equals(render())) {
-            boolean startsWithBrace = super.children.get(0) instanceof ConfigNodeSingleToken &&
+            return addValueOnPath(desiredPath, value, flavor);
-                                        ((ConfigNodeSingleToken) super.children.get(0)).token() == Tokens.OPEN_CURLY;
-            ArrayList<AbstractConfigNode> childrenCopy = new ArrayList<AbstractConfigNode>(super.children);
-            ArrayList<AbstractConfigNode> newNodes = new ArrayList();
-            newNodes.add(new ConfigNodeSingleToken(Tokens.newLine(null)));
-            if (startsWithBrace)
-                newNodes.add(new ConfigNodeSingleToken(Tokens.newIgnoredWhitespace(null, "\t")));
-            newNodes.add(desiredPath);
-            newNodes.add(new ConfigNodeSingleToken(Tokens.newIgnoredWhitespace(null, " ")));
-            newNodes.add(new ConfigNodeSingleToken(Tokens.COLON));
-            newNodes.add(new ConfigNodeSingleToken(Tokens.newIgnoredWhitespace(null, " ")));
-            newNodes.add(value);
-            newNodes.add(new ConfigNodeSingleToken(Tokens.newLine(null)));
-
-            if (startsWithBrace) {
-                for (int i = childrenCopy.size() - 1; i >= 0; i--) {
-                    if (childrenCopy.get(i) instanceof ConfigNodeSingleToken &&
-                            ((ConfigNodeSingleToken) childrenCopy.get(i)).token == Tokens.CLOSE_CURLY) {
-                        childrenCopy.add(i, new ConfigNodeField(newNodes));
-                        return new ConfigNodeObject(childrenCopy);
-                    }
-                }
-                throw new ConfigException.BugOrBroken("Object had an opening brace, but no closing brace");
-            } else {
-                childrenCopy.add(new ConfigNodeField(newNodes));
-                node = new ConfigNodeObject(childrenCopy);
-            }
         }
         return node;
     }
 
+    protected ConfigNodeObject addValueOnPath(ConfigNodePath desiredPath, AbstractConfigNodeValue value, ConfigSyntax flavor) {
+        Path path = desiredPath.value();
+        ArrayList<AbstractConfigNode> childrenCopy = new ArrayList(super.children);
+        if (path.length() > 1) {
+            for (int i = super.children.size() - 1; i >= 0; i--) {
+                if (!(super.children.get(i) instanceof ConfigNodeField)) {
+                    continue;
+                }
+                ConfigNodeField node = (ConfigNodeField) super.children.get(i);
+                Path key = node.path().value();
+                if (path.startsWith(key) && node.value() instanceof ConfigNodeObject) {
+                    ConfigNodePath remainingPath = desiredPath.subPath(key.length());
+                    ConfigNodeObject newValue = (ConfigNodeObject) node.value();
+                    childrenCopy.set(i, node.replaceValue(newValue.addValueOnPath(remainingPath, value, flavor)));
+                    return new ConfigNodeObject(childrenCopy);
+                }
+            }
+        }
+        boolean startsWithBrace = super.children.get(0) instanceof ConfigNodeSingleToken &&
+                ((ConfigNodeSingleToken) super.children.get(0)).token() == Tokens.OPEN_CURLY;
+        ArrayList<AbstractConfigNode> newNodes = new ArrayList();
+        newNodes.add(new ConfigNodeSingleToken(Tokens.newLine(null)));
+        newNodes.add(desiredPath.first());
+        newNodes.add(new ConfigNodeSingleToken(Tokens.newIgnoredWhitespace(null, " ")));
+        newNodes.add(new ConfigNodeSingleToken(Tokens.COLON));
+        newNodes.add(new ConfigNodeSingleToken(Tokens.newIgnoredWhitespace(null, " ")));
+
+        if (path.length() == 1) {
+            newNodes.add(value);
+        } else {
+            ArrayList<AbstractConfigNode> newObjectNodes = new ArrayList();
+            newObjectNodes.add(new ConfigNodeSingleToken(Tokens.OPEN_CURLY));
+            newObjectNodes.add(new ConfigNodeSingleToken(Tokens.CLOSE_CURLY));
+            ConfigNodeObject newObject = new ConfigNodeObject(newObjectNodes);
+            newNodes.add(newObject.addValueOnPath(desiredPath.subPath(1), value, flavor));
+        }
+        newNodes.add(new ConfigNodeSingleToken(Tokens.newLine(null)));
+
+        // Combine these two cases so that we only have to iterate once
+        if (flavor == ConfigSyntax.JSON || startsWithBrace) {
+            for (int i = childrenCopy.size() - 1; i >= 0; i--) {
+
+                // Valid JSON requires all key-value pairs except the last one to be succeeded by a comma,
+                // so we'll need to add a comma when adding a value
+                if (flavor == ConfigSyntax.JSON && childrenCopy.get(i) instanceof ConfigNodeField) {
+                    childrenCopy.add(i+1, new ConfigNodeSingleToken(Tokens.COMMA));
+                    break;
+                }
+                if (startsWithBrace && childrenCopy.get(i) instanceof ConfigNodeSingleToken &&
+                        ((ConfigNodeSingleToken) childrenCopy.get(i)).token == Tokens.CLOSE_CURLY) {
+                    childrenCopy.add(i, new ConfigNodeField(newNodes));
+                }
+            }
+        }
+        if (!startsWithBrace) {
+            childrenCopy.add(new ConfigNodeField(newNodes));
+        }
+        return new ConfigNodeObject(childrenCopy);
+    }
+
     public ConfigNodeComplexValue removeValueOnPath(String desiredPath) {
         Path path = PathParser.parsePath(desiredPath);
         return changeValueOnPath(path, null);

config/src/main/java/com/typesafe/config/impl/ConfigNodePath.java

@@ -3,6 +3,8 @@
  */
 package com.typesafe.config.impl;
 
+import com.typesafe.config.ConfigException;
+
 import java.util.ArrayList;
 import java.util.Collection;
 
@@ -22,4 +24,29 @@ final class ConfigNodePath extends AbstractConfigNode {
     protected Path value() {
         return path;
     }
+
+    protected ConfigNodePath subPath(int toRemove) {
+        int periodCount = 0;
+        ArrayList<Token> tokensCopy = new ArrayList<Token>(tokens);
+        for (int i = 0; i < tokensCopy.size(); i++) {
+            if (Tokens.isUnquotedText(tokensCopy.get(i)) &&
+                    tokensCopy.get(i).tokenText().equals("."))
+                periodCount++;
+
+            if (periodCount == toRemove) {
+                return new ConfigNodePath(path.subPath(toRemove), tokensCopy.subList(i + 1, tokensCopy.size()));
+            }
+        }
+        throw new ConfigException.BugOrBroken("Tried to remove too many elements from a Path node");
+    }
+
+    protected ConfigNodePath first() {
+        ArrayList<Token> tokensCopy = new ArrayList<Token>(tokens);
+        for (int i = 0; i < tokensCopy.size(); i++) {
+            if (Tokens.isUnquotedText(tokensCopy.get(i)) &&
+                    tokensCopy.get(i).tokenText().equals("."))
+                return new ConfigNodePath(path.subPath(0, 1), tokensCopy.subList(0, i));
+        }
+        return this;
+    }
 }

config/src/main/java/com/typesafe/config/impl/PathParser.java

@@ -9,9 +9,7 @@ import com.typesafe.config.ConfigSyntax;
 import com.typesafe.config.ConfigValueType;
 
 import java.io.StringReader;
-import java.util.ArrayList;
+import java.util.*;
-import java.util.Iterator;
-import java.util.List;
 
 final class PathParser {
     static class Element {
@@ -33,13 +31,17 @@ final class PathParser {
     static ConfigOrigin apiOrigin = SimpleConfigOrigin.newSimple("path parameter");
 
     static ConfigNodePath parsePathNode(String path) {
+        return parsePathNode(path, ConfigSyntax.CONF);
+    }
+
+    static ConfigNodePath parsePathNode(String path, ConfigSyntax flavor) {
         StringReader reader = new StringReader(path);
 
         try {
             Iterator<Token> tokens = Tokenizer.tokenize(apiOrigin, reader,
-                    ConfigSyntax.CONF);
+                    flavor);
             tokens.next(); // drop START
-            return parsePathNodeExpression(tokens, apiOrigin, path);
+            return parsePathNodeExpression(tokens, apiOrigin, path, flavor);
         } finally {
             reader.close();
         }
@@ -64,30 +66,31 @@ final class PathParser {
 
     protected static Path parsePathExpression(Iterator<Token> expression,
                                             ConfigOrigin origin) {
-        return parsePathExpression(expression, origin, null, null);
+        return parsePathExpression(expression, origin, null, null, ConfigSyntax.CONF);
     }
 
     protected static Path parsePathExpression(Iterator<Token> expression,
                                               ConfigOrigin origin, String originalText) {
-        return parsePathExpression(expression, origin, originalText, null);
+        return parsePathExpression(expression, origin, originalText, null, ConfigSyntax.CONF);
     }
 
     protected static ConfigNodePath parsePathNodeExpression(Iterator<Token> expression,
                                                             ConfigOrigin origin) {
-        return parsePathNodeExpression(expression, origin, null);
+        return parsePathNodeExpression(expression, origin, null, ConfigSyntax.CONF);
     }
 
     protected static ConfigNodePath parsePathNodeExpression(Iterator<Token> expression,
-                                                            ConfigOrigin origin, String originalText) {
+                                                            ConfigOrigin origin, String originalText, ConfigSyntax flavor) {
         ArrayList<Token> pathTokens = new ArrayList<Token>();
-        Path path = parsePathExpression(expression, origin, originalText, pathTokens);
+        Path path = parsePathExpression(expression, origin, originalText, pathTokens, flavor);
         return new ConfigNodePath(path, pathTokens);
     }
 
     // originalText may be null if not available
     protected static Path parsePathExpression(Iterator<Token> expression,
                                             ConfigOrigin origin, String originalText,
-                                            ArrayList<Token> pathTokens) {
+                                            ArrayList<Token> pathTokens,
+                                            ConfigSyntax flavor) {
         // each builder in "buf" is an element in the path.
         List<Element> buf = new ArrayList<Element>();
         buf.add(new Element("", false));
@@ -132,8 +135,21 @@ final class PathParser {
                     // we tokenize non-string values is largely an
                     // implementation detail.
                     AbstractConfigValue v = Tokens.getValue(t);
+
+                    // We need to split the tokens on a . so that we can get sub-paths but still preserve
+                    // the original path text when doing an insertion
+                    if (pathTokens != null) {
+                        pathTokens.remove(pathTokens.size() - 1);
+                        pathTokens.addAll(splitTokenOnPeriod(t, flavor));
+                    }
                     text = v.transformToString();
                 } else if (Tokens.isUnquotedText(t)) {
+                    // We need to split the tokens on a . so that we can get sub-paths but still preserve
+                    // the original path text when doing an insertion on ConfigNodeObjects
+                    if (pathTokens != null) {
+                        pathTokens.remove(pathTokens.size() - 1);
+                        pathTokens.addAll(splitTokenOnPeriod(t, flavor));
+                    }
                     text = Tokens.getUnquotedText(t);
                 } else {
                     throw new ConfigException.BadPath(
@@ -163,6 +179,25 @@ final class PathParser {
         return pb.result();
     }
 
+    private static Collection<Token> splitTokenOnPeriod(Token t, ConfigSyntax flavor) {
+        String tokenText = t.tokenText();
+        if (tokenText.equals(".")) {
+            return Collections.singletonList(t);
+        }
+        String[] splitToken = tokenText.split("\\.");
+        ArrayList<Token> splitTokens = new ArrayList<Token>();
+        for (String s : splitToken) {
+            if (flavor == ConfigSyntax.CONF)
+                splitTokens.add(Tokens.newUnquotedText(t.origin(), s));
+            else
+                splitTokens.add(Tokens.newString(t.origin(), s, "\"" + s + "\""));
+            splitTokens.add(Tokens.newUnquotedText(t.origin(), "."));
+        }
+        if (tokenText.charAt(tokenText.length() - 1) != '.')
+            splitTokens.remove(splitTokens.size() - 1);
+        return splitTokens;
+    }
+
     private static void addPathText(List<Element> buf, boolean wasQuoted,
                                     String newText) {
         int i = wasQuoted ? -1 : newText.indexOf('.');

config/src/main/java/com/typesafe/config/impl/SimpleConfigDocument.java

@@ -27,7 +27,7 @@ final class SimpleConfigDocument implements ConfigDocument {
         AbstractConfigNodeValue parsedValue = ConfigDocumentParser.parseValue(tokens, parseOptions);
         reader.close();
 
-        return new SimpleConfigDocument(((ConfigNodeObject)configNodeTree).setValueOnPath(path, parsedValue), parseOptions);
+        return new SimpleConfigDocument(((ConfigNodeObject)configNodeTree).setValueOnPath(path, parsedValue, parseOptions.getSyntax()), parseOptions);
     }
 
     public String render() {

config/src/test/scala/com/typesafe/config/impl/ConfigDocumentTest.scala

@@ -139,9 +139,10 @@ class ConfigDocumentTest extends TestUtils {
     @Test
     def configDocumentSetNewValueBraceRoot {
         val origText = "{\n\t\"a\":\"b\",\n\t\"c\":\"d\"\n}"
-        val finalText = "{\n\t\"a\":\"b\",\n\t\"c\":\"d\"\n\n\t\"e\" : \"f\"\n}"
+        val finalTextConf = "{\n\t\"a\":\"b\",\n\t\"c\":\"d\"\n\n\"e\" : \"f\"\n}"
-        configDocumentReplaceConfTest(origText, finalText, "\"f\"", "\"e\"")
+        val finalTextJson = "{\n\t\"a\":\"b\",\n\t\"c\":\"d\",\n\n\"e\" : \"f\"\n}"
-        configDocumentReplaceJsonTest(origText, finalText, "\"f\"", "\"e\"")
+        configDocumentReplaceConfTest(origText, finalTextConf, "\"f\"", "\"e\"")
+        configDocumentReplaceJsonTest(origText, finalTextJson, "\"f\"", "\"e\"")
     }
 
     @Test
@@ -151,6 +152,20 @@ class ConfigDocumentTest extends TestUtils {
         configDocumentReplaceConfTest(origText, finalText, "\"f\"", "\"e\"")
     }
 
+    @Test
+    def configDocumentSetNewValueMultiLevelConf {
+        val origText = "a:b\nc:d"
+        val finalText = "a:b\nc:d\ne : {\nf : {\ng : 12\n}\n}\n"
+        configDocumentReplaceConfTest(origText, finalText, "12", "e.f.g")
+    }
+
+    @Test
+    def configDocumentSetNewValueMultiLevelJson {
+        val origText = "{\"a\":\"b\",\n\"c\":\"d\"}"
+        val finalText = "{\"a\":\"b\",\n\"c\":\"d\",\n\"e\" : {\n\"f\" : {\n\"g\" : 12\n}\n}\n}"
+        configDocumentReplaceJsonTest(origText, finalText, "12", "e.f.g")
+    }
+
     @Test
     def configDocumentReplaceFailure {
         // Attempting a replace on a ConfigDocument parsed from an array throws an error

config/src/test/scala/com/typesafe/config/impl/ConfigNodeTest.scala

@@ -92,6 +92,15 @@ class ConfigNodeTest extends TestUtils {
         keyNodeTest("\"Hello I am a key how are you today\"")
     }
 
+    @Test
+    def pathNodeSubpath() {
+        val origPath = "a.b.c.\"@$%#@!@#$\".\"\".1234.5678"
+        val pathNode = configNodeKey(origPath)
+        assertEquals(origPath, pathNode.render())
+        assertEquals("c.\"@$%#@!@#$\".\"\".1234.5678", pathNode.subPath(2).render())
+        assertEquals("5678", pathNode.subPath(6).render())
+    }
+
     @Test
     def createConfigNodeSimpleValue() {
         //Ensure a ConfigNodeSimpleValue can handle the normal value types
@@ -204,7 +213,7 @@ class ConfigNodeTest extends TestUtils {
                                               nodeCloseBrace))
         assertEquals(origText, origNode.render())
         val finalText = "foo : bar\nbaz : {\n\t\"abc.def\" : true\n\t//This is a comment about the below setting\n\n\tabc : {\n\t\t" +
-          "def : false\n\t}\n}\nbaz.abc.ghi : randomunquotedString\n}\nbaz.abc.\"this.does.not.exist@@@+$#\".end : doesnotexist\n"
+          "def : false\n\t\n\"this.does.not.exist@@@+$#\" : {\nend : doesnotexist\n}\n}\n}\nbaz.abc.ghi : randomunquotedString\n}"
 
         //Can replace settings in nested maps
         // Paths with quotes in the name are treated as a single Path, rather than multiple sub-paths