Remove repeats when setting value in ConfigNode

Remove repeats of a key when setting a value in a
ConfigNodeComplexValue. Add a new node type, ConfigNodeKeyValue,
to represent a key-value pair and its surrounding whitespace.

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

@@ -1,14 +1,26 @@
 package com.typesafe.config.impl;
 
 import com.typesafe.config.ConfigNode;
-import java.util.ArrayList;
+
-import java.util.Collection;
+import java.util.*;
 
 final class ConfigNodeComplexValue implements ConfigNode, ConfigNodeValue {
-    final private ArrayList<ConfigNode> children;
+    private ArrayList<ConfigNode> children;
+    final private LinkedHashMap<Path, Integer> map = new LinkedHashMap<>();
+    final ArrayList<Integer> keyValueIndexes;
 
     ConfigNodeComplexValue(Collection<ConfigNode> children) {
-        this.children = new ArrayList<ConfigNode>(children);
+        this.children = new ArrayList(children);
+        keyValueIndexes = new ArrayList<Integer>();
+
+        // Construct the list of indexes of Key-Value nodes. Do this
+        // in reverse order, since all but the final duplicate will be removed.
+        for (int i = this.children.size() - 1; i >= 0; i--) {
+            ConfigNode currNode = this.children.get(i);
+            if (currNode instanceof ConfigNodeKeyValue) {
+                keyValueIndexes.add(i);
+            }
+        }
     }
 
     public ArrayList<ConfigNode> children() {
@@ -23,34 +35,45 @@ final class ConfigNodeComplexValue implements ConfigNode, ConfigNodeValue {
         return renderedText.toString();
     }
 
-    public ConfigNodeComplexValue replaceValueOnPath(Path desiredPath, ConfigNodeValue value) {
+    private ConfigNodeComplexValue changeValueOnPath(Path desiredPath, ConfigNodeValue value) {
-        boolean matchedFullPath = false;
+        ArrayList<ConfigNode> childrenCopy = (ArrayList<ConfigNode>)(children.clone());
-        boolean matchedPartialPath = false;
+        boolean replaced = value == null;
-        Path remainingPath = desiredPath;
+        ConfigNodeKeyValue node;
-        ArrayList<ConfigNode> childrenCopy = (ArrayList<ConfigNode>)children.clone();
+        Path key;
-        for (int i = 0; i < childrenCopy.size(); i++) {
+        for (Integer keyValIndex : keyValueIndexes) {
-            ConfigNode child = childrenCopy.get(i);
+            node = (ConfigNodeKeyValue)children.get(keyValIndex.intValue());
-            if (child instanceof ConfigNodeKey) {
+            key = Path.newPath(node.key().render());
-                Path key = Path.newPath(child.render());
             if (key.equals(desiredPath)) {
-                    matchedFullPath = true;
+                if (!replaced) {
+                    childrenCopy.set(keyValIndex.intValue(), node.replaceValue(value));
+                    replaced = true;
+                }
+                else
+                    childrenCopy.remove(keyValIndex.intValue());
             } else if (desiredPath.startsWith(key)) {
-                    matchedPartialPath = true;
+                if (node.value() instanceof ConfigNodeComplexValue) {
-                    remainingPath = desiredPath.subPath(key.length());
+                    Path remainingPath = desiredPath.subPath(key.length());
+                    if (!replaced) {
+                        node = node.replaceValue(((ConfigNodeComplexValue) node.value()).setValueOnPath(remainingPath, value));
+                        if (node.render() != children.get(keyValIndex.intValue()).render())
+                            replaced = true;
+                        childrenCopy.set(keyValIndex.intValue(), node);
+                    } else {
+                        node = node.replaceValue(((ConfigNodeComplexValue) node.value()).removeValueOnPath(remainingPath));
+                        childrenCopy.set(keyValIndex.intValue(), node);
+                    }
+                }
+            }
         }
-            } else if (child instanceof ConfigNodeValue) {
-                if (matchedFullPath) {
-                    childrenCopy.set(i, value);
-                    return new ConfigNodeComplexValue(childrenCopy);
-                } else if (matchedPartialPath) {
-                    if (child instanceof ConfigNodeComplexValue) {
-                        childrenCopy.set(i, ((ConfigNodeComplexValue) child).replaceValueOnPath(remainingPath, value));
         return new ConfigNodeComplexValue(childrenCopy);
     }
-                    matchedPartialPath = false;
+
+    public ConfigNodeComplexValue setValueOnPath(Path desiredPath, ConfigNodeValue value) {
+        return changeValueOnPath(desiredPath, value);
     }
+
+    public ConfigNodeComplexValue removeValueOnPath(Path desiredPath) {
+        return changeValueOnPath(desiredPath, null);
     }
-        }
+
-        return this;
-    }
 }

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

@@ -0,0 +1,48 @@
+package com.typesafe.config.impl;
+
+import com.typesafe.config.ConfigNode;
+
+import java.util.ArrayList;
+import java.util.Collection;
+
+public class ConfigNodeKeyValue implements ConfigNode{
+    final private ArrayList<ConfigNode> children;
+    private int configNodeValueIndex;
+    private ConfigNodeKey key;
+    private ConfigNodeValue value;
+
+    public ConfigNodeKeyValue(Collection<ConfigNode> children) {
+        this.children = new ArrayList<ConfigNode>(children);
+        for (int i = 0; i < this.children.size(); i++) {
+            ConfigNode currNode = this.children.get(i);
+            if (currNode instanceof ConfigNodeKey) {
+                key = (ConfigNodeKey)currNode;
+            } else if (currNode instanceof ConfigNodeValue) {
+                value = (ConfigNodeValue)currNode;
+                configNodeValueIndex = i;
+            }
+        }
+    }
+
+    public String render() {
+        StringBuilder renderedText = new StringBuilder();
+        for (ConfigNode child : children) {
+            renderedText.append(child.render());
+        }
+        return renderedText.toString();
+    }
+
+    public ConfigNodeKeyValue replaceValue(ConfigNodeValue newValue) {
+        ArrayList<ConfigNode> newChildren = (ArrayList<ConfigNode>)(children.clone());
+        newChildren.set(configNodeValueIndex, newValue);
+        return new ConfigNodeKeyValue(newChildren);
+    }
+
+    public ConfigNodeValue value() {
+        return value;
+    }
+
+    public ConfigNodeKey key() {
+        return key;
+    }
+}

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

@@ -21,14 +21,25 @@ class ConfigNodeTest extends TestUtils {
         assertEquals(node.render(), token.tokenText())
     }
 
+    private def keyValueNodeTest(key: ConfigNodeKey, value: ConfigNodeValue, trailingWhitespace: BasicConfigNode, newValue: ConfigNodeValue) {
+        val keyValNode = nodeKeyValuePair(key, value, trailingWhitespace)
+        assertEquals(key.render() + " : " + value.render() + trailingWhitespace.render(), keyValNode.render())
+        assertEquals(key.render, keyValNode.key().render())
+        assertEquals(value.render, keyValNode.value().render())
+
+        val newKeyValNode = keyValNode.replaceValue(newValue)
+        assertEquals(key.render() + " : " + newValue.render() + trailingWhitespace.render(), newKeyValNode.render())
+        assertEquals(newValue.render(), newKeyValNode.value().render())
+    }
+
     private def topLevelValueReplaceTest(value: ConfigNodeValue, newValue: ConfigNodeValue, key: Token = tokenString("foo")) {
-        val complexNodeChildren = List(nodeOpenBrace, nodeWhitespace("       "),
+        val complexNodeChildren = List(nodeOpenBrace,
-                                       configNodeKey(key), nodeWhitespace("    "), nodeColon,
+                                       nodeKeyValuePair(nodeWhitespace("       "), configNodeKey(key),value, nodeWhitespace("    ")),
-                                       value, nodeWhitespace("    "), nodeCloseBrace)
+                                       nodeCloseBrace)
         val complexNode = configNodeComplexValue(complexNodeChildren)
-        val newNode = complexNode.replaceValueOnPath(Path.newPath(key.tokenText()), newValue)
+        val newNode = complexNode.setValueOnPath(Path.newPath(key.tokenText()), newValue)
-        val origText = "{       " + key.tokenText() + "    :" + value.render() + "    }"
+        val origText = "{       " + key.tokenText() + " : " + value.render() + "    }"
-        val finalText = "{       " + key.tokenText() + "    :" + newValue.render() + "    }"
+        val finalText = "{       " + key.tokenText() + " : " + newValue.render() + "    }"
 
         assertEquals(origText, complexNode.render())
         assertEquals(finalText, newNode.render())
@@ -37,7 +48,7 @@ class ConfigNodeTest extends TestUtils {
     private def replaceInComplexValueTest(nodes: List[ConfigNode], origText: String, newText: String, replaceVal: ConfigNodeValue, replacePath: String) {
         val complexNode = configNodeComplexValue(nodes)
         assertEquals(complexNode.render(), origText)
-        val newNode = complexNode.replaceValueOnPath(Path.newPath(replacePath), replaceVal)
+        val newNode = complexNode.setValueOnPath(Path.newPath(replacePath), replaceVal)
         assertEquals(newNode.render(), newText)
     }
 
@@ -84,6 +95,17 @@ class ConfigNodeTest extends TestUtils {
         simpleValueNodeTest(tokenSubstitution(tokenUnquoted("a.b")))
     }
 
+    @Test
+    def createConfigNodeKeyValue() {
+        // Supports Quoted and Unquoted keys
+        keyValueNodeTest(nodeQuotedKey("abc"), nodeInt(123), nodeLine(1), nodeInt(245))
+        keyValueNodeTest(nodeUnquotedKey("abc"), nodeInt(123), nodeLine(1), nodeInt(245))
+
+        // Can replace value with values of different types
+        keyValueNodeTest(nodeQuotedKey("abc"), nodeInt(123), nodeLine(1), nodeString("I am a string"))
+        keyValueNodeTest(nodeQuotedKey("abc"), nodeInt(123), nodeLine(1), configNodeComplexValue(List(nodeOpenBrace, nodeCloseBrace)))
+    }
+
     @Test
     def replaceNodesTopLevel() {
         //Ensure simple values can be replaced by other simple values
@@ -114,33 +136,36 @@ class ConfigNodeTest extends TestUtils {
     @Test
     def replaceInNestedMapComplexValue() {
         val origText = "{\n\tfoo : bar\n\tbaz : {\n\t\t\"abc.def\" : 123\n\t\t//This is a comment about the below setting\n\n\t\tabc : {\n\t\t\t" +
-          "def : \"this is a string\"\n\t\t\tghi: ${\"a.b\"}\n\t\t}\n\t}\n}"
+          "def : \"this is a string\"\n\t\t\tghi : ${\"a.b\"}\n\t\t}\n\t}\n\tbaz.abc.ghi : 52\n\tbaz.abc.ghi : 53\n}"
-        val lowestLevelMap = configNodeComplexValue(List(nodeOpenBrace,
+        val lowestLevelMap = configNodeComplexValue(List(nodeOpenBrace, nodeLine(7),
-                                                         nodeLine(7), nodeWhitespace("\t\t\t"), nodeUnquotedKey("def"), nodeSpace, nodeColon, nodeSpace,
+                                                         nodeKeyValuePair(nodeWhitespace("\t\t\t"), nodeUnquotedKey("def"), configNodeSimpleValue(tokenString("this is a string")), nodeLine(8)),
-                                                         configNodeSimpleValue(tokenString("this is a string")), nodeLine(8), nodeWhitespace("\t\t\t"),
+                                                         nodeKeyValuePair(nodeWhitespace("\t\t\t"), nodeUnquotedKey("ghi"), configNodeSimpleValue(tokenKeySubstitution("a.b")), nodeLine(9)),
-                                                         nodeUnquotedKey("ghi"), nodeColon, nodeSpace, configNodeSimpleValue(tokenKeySubstitution("a.b")),
+                                                         nodeWhitespace("\t\t"), nodeCloseBrace))
-                                                         nodeLine(9), nodeWhitespace("\t\t"), nodeCloseBrace))
+        val higherLevelMap = configNodeComplexValue(List(nodeOpenBrace, nodeLine(3),
-        val higherLevelMap = configNodeComplexValue(List(nodeOpenBrace, configNodeBasic(tokenLine(3)), nodeWhitespace("\t\t"),
+                                                         nodeKeyValuePair(nodeWhitespace("\t\t"), configNodeKey(tokenString("abc.def")), configNodeSimpleValue(tokenInt(123)), nodeLine(4)),
-                                                         configNodeKey(tokenString("abc.def")), nodeSpace, nodeColon,
-                                                         nodeSpace, configNodeSimpleValue(tokenInt(123)), configNodeBasic(tokenLine(4)),
                                                          nodeWhitespace("\t\t"), configNodeBasic(tokenCommentDoubleSlash("This is a comment about the below setting")),
-                                                         configNodeBasic(tokenLine(5)), configNodeBasic(tokenLine(6)), nodeWhitespace("\t\t"),
+                                                         nodeLine(5), nodeLine(6),
-                                                         nodeUnquotedKey("abc"), nodeSpace, nodeColon, nodeSpace, lowestLevelMap, nodeLine(10), nodeWhitespace("\t"),
+                                                         nodeKeyValuePair(nodeWhitespace("\t\t"), nodeUnquotedKey("abc"), lowestLevelMap, nodeLine(10)), nodeWhitespace("\t"),
+                                                         nodeCloseBrace))
+        val origNode =  configNodeComplexValue(List(nodeOpenBrace, nodeLine(1),
+                                                    nodeKeyValuePair(nodeWhitespace("\t"), nodeUnquotedKey("foo"), configNodeSimpleValue(tokenUnquoted("bar")), nodeLine(2)),
+                                                    nodeKeyValuePair(nodeWhitespace("\t"), nodeUnquotedKey("baz"), higherLevelMap, nodeLine(11)),
+                                                    nodeKeyValuePair(nodeWhitespace("\t"), nodeUnquotedKey("baz.abc.ghi"), configNodeSimpleValue(tokenInt(52)), nodeLine(12)),
+                                                    nodeKeyValuePair(nodeWhitespace("\t"), nodeUnquotedKey("baz.abc.ghi"), configNodeSimpleValue(tokenInt(53)), nodeLine(13)),
                                                     nodeCloseBrace))
-        val origNode =  configNodeComplexValue(List(nodeOpenBrace, nodeLine(1), nodeWhitespace("\t"),
-                                                    nodeUnquotedKey("foo"), nodeSpace, nodeColon,
-                                                    nodeSpace, configNodeSimpleValue(tokenUnquoted("bar")),
-                                                    nodeLine(2), nodeWhitespace("\t"), nodeUnquotedKey("baz"),
-                                                    nodeSpace, nodeColon, nodeSpace,
-                                                    higherLevelMap, nodeLine(11), nodeCloseBrace))
         assertEquals(origText, origNode.render())
         val finalText = "{\n\tfoo : bar\n\tbaz : {\n\t\t\"abc.def\" : true\n\t\t//This is a comment about the below setting\n\n\t\tabc : {\n\t\t\t" +
-          "def : false\n\t\t\tghi: randomunquotedString\n\t\t}\n\t}\n}"
+          "def : false\n\t\t}\n\t}\n\tbaz.abc.ghi : randomunquotedString\n}"
 
         //Can replace settings in nested maps
-        var newNode = origNode.replaceValueOnPath(Path.newPath("baz.\"abc.def\""), configNodeSimpleValue(tokenTrue))
+        // Paths with quotes in the name are treated as a single Path, rather than multiple sub-paths
-        newNode = newNode.replaceValueOnPath(Path.newPath("baz.abc.def"), configNodeSimpleValue(tokenFalse))
+        var newNode = origNode.setValueOnPath(Path.newPath("baz.\"abc.def\""), configNodeSimpleValue(tokenTrue))
-        newNode = newNode.replaceValueOnPath(Path.newPath("baz.abc.ghi"), configNodeSimpleValue(tokenUnquoted("randomunquotedString")))
+        newNode = newNode.setValueOnPath(Path.newPath("baz.abc.def"), configNodeSimpleValue(tokenFalse))
+
+        // Repeats are removed
+        newNode = newNode.setValueOnPath(Path.newPath("baz.abc.ghi"), configNodeSimpleValue(tokenUnquoted("randomunquotedString")))
+
+        // The above operations cause the resultant map to be rendered properly
         assertEquals(finalText, newNode.render())
     }
 

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

@@ -664,19 +664,19 @@ abstract trait TestUtils {
     }
 
     def configNodeSimpleValue(value: Token) = {
-        new ConfigNodeSimpleValue(value);
+        new ConfigNodeSimpleValue(value)
     }
 
     def configNodeKey(value: Token) = {
-        new ConfigNodeKey(value);
+        new ConfigNodeKey(value)
     }
 
     def configNodeBasic(value: Token) = {
-        new BasicConfigNode(value: Token);
+        new BasicConfigNode(value: Token)
     }
 
     def configNodeComplexValue(nodes: List[ConfigNode]) = {
-        new ConfigNodeComplexValue(nodes.asJavaCollection);
+        new ConfigNodeComplexValue(nodes.asJavaCollection)
     }
 
     def nodeColon = new BasicConfigNode(Tokens.COLON)
@@ -687,6 +687,16 @@ abstract trait TestUtils {
     def nodeWhitespace(whitespace: String) = new BasicConfigNode(tokenWhitespace(whitespace))
     def nodeQuotedKey(key: String) = configNodeKey(tokenString(key))
     def nodeUnquotedKey(key: String) = configNodeKey(tokenUnquoted(key))
+    def nodeKeyValuePair(key: ConfigNodeKey, value: ConfigNodeValue, trailingWhitespace: BasicConfigNode) = {
+        val nodes = List(key, nodeSpace, nodeColon, nodeSpace, value, trailingWhitespace)
+        new ConfigNodeKeyValue(nodes.asJavaCollection)
+    }
+    def nodeKeyValuePair(leadingWhitespace: BasicConfigNode, key: ConfigNodeKey, value: ConfigNodeValue, trailingWhitespace: BasicConfigNode) = {
+        val nodes = List(leadingWhitespace, key, nodeSpace, nodeColon, nodeSpace, value, trailingWhitespace)
+        new ConfigNodeKeyValue(nodes.asJavaCollection);
+    }
+    def nodeInt(value: Integer) = new ConfigNodeSimpleValue(tokenInt(value))
+    def nodeString(value: String) = new ConfigNodeSimpleValue(tokenString(value))
 
     // this is importantly NOT using Path.newPath, which relies on
     // the parser; in the test suite we are often testing the parser,