From 4b61790fb4e6114ebb0147c67292b0c50bfbd957 Mon Sep 17 00:00:00 2001 From: Preben Ingvaldsen Date: Thu, 19 Mar 2015 14:21:49 -0700 Subject: [PATCH] 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/impl/ConfigNodeObject.java | 99 +++++++++++++------ .../typesafe/config/impl/ConfigNodePath.java | 27 +++++ .../com/typesafe/config/impl/PathParser.java | 57 ++++++++--- .../config/impl/SimpleConfigDocument.java | 2 +- .../config/impl/ConfigDocumentTest.scala | 21 +++- .../typesafe/config/impl/ConfigNodeTest.scala | 11 ++- 6 files changed, 171 insertions(+), 46 deletions(-) diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigNodeObject.java b/config/src/main/java/com/typesafe/config/impl/ConfigNodeObject.java index ccd1f8ca..03bb72c7 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigNodeObject.java +++ b/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(path, value); + return setValueOnPath(desiredPath, value, ConfigSyntax.CONF); } - 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 && - ((ConfigNodeSingleToken) super.children.get(0)).token() == Tokens.OPEN_CURLY; - ArrayList childrenCopy = new ArrayList(super.children); - ArrayList 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 addValueOnPath(desiredPath, value, flavor); } return node; } + protected ConfigNodeObject addValueOnPath(ConfigNodePath desiredPath, AbstractConfigNodeValue value, ConfigSyntax flavor) { + Path path = desiredPath.value(); + ArrayList 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 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 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); diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigNodePath.java b/config/src/main/java/com/typesafe/config/impl/ConfigNodePath.java index 9aaab0b3..00b866df 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigNodePath.java +++ b/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 tokensCopy = new ArrayList(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 tokensCopy = new ArrayList(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; + } } diff --git a/config/src/main/java/com/typesafe/config/impl/PathParser.java b/config/src/main/java/com/typesafe/config/impl/PathParser.java index 312e2ac2..8961cb71 100644 --- a/config/src/main/java/com/typesafe/config/impl/PathParser.java +++ b/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.Iterator; -import java.util.List; +import java.util.*; 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 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 expression, ConfigOrigin origin) { - return parsePathExpression(expression, origin, null, null); + return parsePathExpression(expression, origin, null, null, ConfigSyntax.CONF); } protected static Path parsePathExpression(Iterator expression, ConfigOrigin origin, String originalText) { - return parsePathExpression(expression, origin, originalText, null); + return parsePathExpression(expression, origin, originalText, null, ConfigSyntax.CONF); } protected static ConfigNodePath parsePathNodeExpression(Iterator expression, ConfigOrigin origin) { - return parsePathNodeExpression(expression, origin, null); + return parsePathNodeExpression(expression, origin, null, ConfigSyntax.CONF); } protected static ConfigNodePath parsePathNodeExpression(Iterator expression, - ConfigOrigin origin, String originalText) { + ConfigOrigin origin, String originalText, ConfigSyntax flavor) { ArrayList pathTokens = new ArrayList(); - 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 expression, ConfigOrigin origin, String originalText, - ArrayList pathTokens) { + ArrayList pathTokens, + ConfigSyntax flavor) { // each builder in "buf" is an element in the path. List buf = new ArrayList(); 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 splitTokenOnPeriod(Token t, ConfigSyntax flavor) { + String tokenText = t.tokenText(); + if (tokenText.equals(".")) { + return Collections.singletonList(t); + } + String[] splitToken = tokenText.split("\\."); + ArrayList splitTokens = new ArrayList(); + 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 buf, boolean wasQuoted, String newText) { int i = wasQuoted ? -1 : newText.indexOf('.'); diff --git a/config/src/main/java/com/typesafe/config/impl/SimpleConfigDocument.java b/config/src/main/java/com/typesafe/config/impl/SimpleConfigDocument.java index 73b8901a..28ef5926 100644 --- a/config/src/main/java/com/typesafe/config/impl/SimpleConfigDocument.java +++ b/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() { diff --git a/config/src/test/scala/com/typesafe/config/impl/ConfigDocumentTest.scala b/config/src/test/scala/com/typesafe/config/impl/ConfigDocumentTest.scala index c79b8bef..f3e66670 100644 --- a/config/src/test/scala/com/typesafe/config/impl/ConfigDocumentTest.scala +++ b/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}" - configDocumentReplaceConfTest(origText, finalText, "\"f\"", "\"e\"") - configDocumentReplaceJsonTest(origText, finalText, "\"f\"", "\"e\"") + val finalTextConf = "{\n\t\"a\":\"b\",\n\t\"c\":\"d\"\n\n\"e\" : \"f\"\n}" + val finalTextJson = "{\n\t\"a\":\"b\",\n\t\"c\":\"d\",\n\n\"e\" : \"f\"\n}" + 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 diff --git a/config/src/test/scala/com/typesafe/config/impl/ConfigNodeTest.scala b/config/src/test/scala/com/typesafe/config/impl/ConfigNodeTest.scala index 99466273..fa292cf8 100644 --- a/config/src/test/scala/com/typesafe/config/impl/ConfigNodeTest.scala +++ b/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