From 7d5f72e248b1679cdd6f4925226da5295c3fa706 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Thu, 17 Nov 2011 12:09:12 -0500 Subject: [PATCH] Add Config.fromPathMap() which takes map with paths Share implementation of this with the Properties parser; it makes the Properties parser somewhat less efficient due to creating the intermediate Path objects, but should not matter. --- src/main/java/com/typesafe/config/Config.java | 35 +++++ .../com/typesafe/config/impl/ConfigImpl.java | 42 ++++-- .../com/typesafe/config/impl/FromMapMode.java | 5 + .../config/impl/PropertiesParser.java | 134 +++++++++++++----- .../typesafe/config/impl/PropertiesTest.scala | 9 ++ .../typesafe/config/impl/PublicApiTest.scala | 41 ++++++ 6 files changed, 214 insertions(+), 52 deletions(-) create mode 100644 src/main/java/com/typesafe/config/impl/FromMapMode.java diff --git a/src/main/java/com/typesafe/config/Config.java b/src/main/java/com/typesafe/config/Config.java index a54cb710..3edeb50b 100644 --- a/src/main/java/com/typesafe/config/Config.java +++ b/src/main/java/com/typesafe/config/Config.java @@ -228,6 +228,9 @@ public final class Config { * wrapped in ConfigValue. To get nested ConfigObject, some of the values in * the map would have to be more maps. * + * There is a separate fromPathMap() that interprets the keys in the map as + * path expressions. + * * @param values * @param originDescription * @return @@ -237,6 +240,27 @@ public final class Config { return (ConfigObject) fromAnyRef(values, originDescription); } + /** + * Similar to fromMap(), but the keys in the map are path expressions, + * rather than keys. This is more convenient if you are writing literal maps + * in code, and less convenient if you are getting your maps from some data + * source such as a parser. + * + * An exception will be thrown (and it is a bug in the caller of the method) + * if a path is both an object and a value, for example if you had both + * "a=foo" and "a.b=bar", then "a" is both the string "foo" and the parent + * object of "b". The caller of this method should ensure that doesn't + * happen. + * + * @param values + * @param originDescription + * @return + */ + public static ConfigObject fromPathMap( + Map values, String originDescription) { + return ConfigImpl.fromPathMap(values, originDescription); + } + /** * See the fromAnyRef() documentation for details. This is a typesafe * wrapper that only works on Iterable and returns ConfigList rather than @@ -273,6 +297,17 @@ public final class Config { return fromMap(values, null); } + /** + * See the other overload of fromPathMap() for details, this one just uses a + * default origin description. + * + * @param values + * @return + */ + public static ConfigObject fromPathMap(Map values) { + return fromPathMap(values, null); + } + /** * See the other overload of fromIterable() for details, this one just uses * a default origin description. diff --git a/src/main/java/com/typesafe/config/impl/ConfigImpl.java b/src/main/java/com/typesafe/config/impl/ConfigImpl.java index a80d2355..7aeab828 100644 --- a/src/main/java/com/typesafe/config/impl/ConfigImpl.java +++ b/src/main/java/com/typesafe/config/impl/ConfigImpl.java @@ -175,10 +175,19 @@ public class ConfigImpl { /** For use ONLY by library internals, DO NOT TOUCH not guaranteed ABI */ public static ConfigValue fromAnyRef(Object object, String originDescription) { ConfigOrigin origin = valueOrigin(originDescription); - return fromAnyRef(object, origin); + return fromAnyRef(object, origin, FromMapMode.KEYS_ARE_KEYS); } - static AbstractConfigValue fromAnyRef(Object object, ConfigOrigin origin) { + /** For use ONLY by library internals, DO NOT TOUCH not guaranteed ABI */ + public static ConfigObject fromPathMap( + Map pathMap, String originDescription) { + ConfigOrigin origin = valueOrigin(originDescription); + return (ConfigObject) fromAnyRef(pathMap, origin, + FromMapMode.KEYS_ARE_PATHS); + } + + static AbstractConfigValue fromAnyRef(Object object, ConfigOrigin origin, + FromMapMode mapMode) { if (origin == null) throw new ConfigException.BugOrBroken( "origin not supposed to be null"); @@ -218,18 +227,23 @@ public class ConfigImpl { if (((Map) object).isEmpty()) return emptyObject(origin); - Map values = new HashMap(); - for (Map.Entry entry : ((Map) object).entrySet()) { - Object key = entry.getKey(); - if (!(key instanceof String)) - throw new ConfigException.BugOrBroken( - "bug in method caller: not valid to create ConfigObject from map with non-String key: " - + key); - AbstractConfigValue value = fromAnyRef(entry.getValue(), origin); - values.put((String) key, value); - } + if (mapMode == FromMapMode.KEYS_ARE_KEYS) { + Map values = new HashMap(); + for (Map.Entry entry : ((Map) object).entrySet()) { + Object key = entry.getKey(); + if (!(key instanceof String)) + throw new ConfigException.BugOrBroken( + "bug in method caller: not valid to create ConfigObject from map with non-String key: " + + key); + AbstractConfigValue value = fromAnyRef(entry.getValue(), + origin, mapMode); + values.put((String) key, value); + } - return new SimpleConfigObject(origin, values); + return new SimpleConfigObject(origin, values); + } else { + return PropertiesParser.fromPathMap(origin, (Map) object); + } } else if (object instanceof Iterable) { Iterator i = ((Iterable) object).iterator(); if (!i.hasNext()) @@ -237,7 +251,7 @@ public class ConfigImpl { List values = new ArrayList(); while (i.hasNext()) { - AbstractConfigValue v = fromAnyRef(i.next(), origin); + AbstractConfigValue v = fromAnyRef(i.next(), origin, mapMode); values.add(v); } diff --git a/src/main/java/com/typesafe/config/impl/FromMapMode.java b/src/main/java/com/typesafe/config/impl/FromMapMode.java new file mode 100644 index 00000000..0a646b2e --- /dev/null +++ b/src/main/java/com/typesafe/config/impl/FromMapMode.java @@ -0,0 +1,5 @@ +package com.typesafe.config.impl; + +enum FromMapMode { + KEYS_ARE_PATHS, KEYS_ARE_KEYS +} diff --git a/src/main/java/com/typesafe/config/impl/PropertiesParser.java b/src/main/java/com/typesafe/config/impl/PropertiesParser.java index 30ccc46f..c73293d9 100644 --- a/src/main/java/com/typesafe/config/impl/PropertiesParser.java +++ b/src/main/java/com/typesafe/config/impl/PropertiesParser.java @@ -5,7 +5,6 @@ import java.io.Reader; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; -import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -13,6 +12,7 @@ import java.util.Map; import java.util.Properties; import java.util.Set; +import com.typesafe.config.ConfigException; import com.typesafe.config.ConfigOrigin; final class PropertiesParser { @@ -39,69 +39,127 @@ final class PropertiesParser { return path.substring(0, i); } + static Path pathFromPropertyKey(String key) { + String last = lastElement(key); + String exceptLast = exceptLastElement(key); + Path path = new Path(last, null); + while (exceptLast != null) { + last = lastElement(exceptLast); + exceptLast = exceptLastElement(exceptLast); + path = new Path(last, path); + } + return path; + } + static AbstractConfigObject fromProperties(ConfigOrigin origin, Properties props) { - /* - * First, build a list of paths that will have values, either strings or - * objects. - */ - Set scopePaths = new HashSet(); - Set valuePaths = new HashSet(); - Enumeration i = props.propertyNames(); - while (i.hasMoreElements()) { - Object o = i.nextElement(); - if (o instanceof String) { - // add value's path - String path = (String) o; - valuePaths.add(path); + Map pathMap = new HashMap(); + for (Map.Entry entry : props.entrySet()) { + Object key = entry.getKey(); + if (key instanceof String) { + Path path = pathFromPropertyKey((String) key); + pathMap.put(path, entry.getValue()); + } + } + return fromPathMap(origin, pathMap, true /* from properties */); + } - // all parent paths are objects - String next = exceptLastElement(path); - while (next != null) { - scopePaths.add(next); - next = exceptLastElement(next); + static AbstractConfigObject fromPathMap(ConfigOrigin origin, + Map pathExpressionMap) { + Map pathMap = new HashMap(); + for (Map.Entry entry : pathExpressionMap.entrySet()) { + Object keyObj = entry.getKey(); + if (!(keyObj instanceof String)) { + throw new ConfigException.BugOrBroken( + "Map has a non-string as a key, expecting a path expression as a String"); + } + Path path = Path.newPath((String) keyObj); + pathMap.put(path, entry.getValue()); + } + return fromPathMap(origin, pathMap, false /* from properties */); + } + + private static AbstractConfigObject fromPathMap(ConfigOrigin origin, + Map pathMap, boolean convertedFromProperties) { + /* + * First, build a list of paths that will have values, either string or + * object values. + */ + Set scopePaths = new HashSet(); + Set valuePaths = new HashSet(); + for (Path path : pathMap.keySet()) { + // add value's path + valuePaths.add(path); + + // all parent paths are objects + Path next = path.parent(); + while (next != null) { + scopePaths.add(next); + next = next.parent(); + } + } + + if (convertedFromProperties) { + /* + * If any string values are also objects containing other values, + * drop those string values - objects "win". + */ + valuePaths.removeAll(scopePaths); + } else { + /* If we didn't start out as properties, then this is an error. */ + for (Path path : valuePaths) { + if (scopePaths.contains(path)) { + throw new ConfigException.BugOrBroken( + "In the map, path '" + + path.render() + + "' occurs as both the parent object of a value and as a value. " + + "Because Map has no defined ordering, this is a broken situation."); } } } - /* - * If any string values are also objects containing other values, drop - * those string values - objects "win". - */ - valuePaths.removeAll(scopePaths); - /* * Create maps for the object-valued values. */ Map root = new HashMap(); - Map> scopes = new HashMap>(); + Map> scopes = new HashMap>(); - for (String path : scopePaths) { + for (Path path : scopePaths) { Map scope = new HashMap(); scopes.put(path, scope); } /* Store string values in the associated scope maps */ - for (String path : valuePaths) { - String parentPath = exceptLastElement(path); + for (Path path : valuePaths) { + Path parentPath = path.parent(); Map parent = parentPath != null ? scopes .get(parentPath) : root; - String last = lastElement(path); - String value = props.getProperty(path); - parent.put(last, new ConfigString(origin, value)); + String last = path.last(); + Object rawValue = pathMap.get(path); + AbstractConfigValue value; + if (convertedFromProperties) { + value = new ConfigString(origin, (String) rawValue); + } else { + value = ConfigImpl.fromAnyRef(pathMap.get(path), origin, + FromMapMode.KEYS_ARE_PATHS); + } + parent.put(last, value); } /* * Make a list of scope paths from longest to shortest, so children go * before parents. */ - List sortedScopePaths = new ArrayList(); + List sortedScopePaths = new ArrayList(); sortedScopePaths.addAll(scopePaths); // sort descending by length - Collections.sort(sortedScopePaths, new Comparator() { + Collections.sort(sortedScopePaths, new Comparator() { @Override - public int compare(String a, String b) { + public int compare(Path a, Path b) { + // Path.length() is O(n) so in theory this sucks + // but in practice we can make Path precompute length + // if it ever matters. return b.length() - a.length(); } }); @@ -111,16 +169,16 @@ final class PropertiesParser { * parents to avoid modifying any already-created ConfigObject. This is * where we need the sorted list. */ - for (String scopePath : sortedScopePaths) { + for (Path scopePath : sortedScopePaths) { Map scope = scopes.get(scopePath); - String parentPath = exceptLastElement(scopePath); + Path parentPath = scopePath.parent(); Map parent = parentPath != null ? scopes .get(parentPath) : root; AbstractConfigObject o = new SimpleConfigObject(origin, scope, ResolveStatus.RESOLVED); - parent.put(lastElement(scopePath), o); + parent.put(scopePath.last(), o); } // return root config object diff --git a/src/test/scala/com/typesafe/config/impl/PropertiesTest.scala b/src/test/scala/com/typesafe/config/impl/PropertiesTest.scala index 69b86e4c..85c50efb 100644 --- a/src/test/scala/com/typesafe/config/impl/PropertiesTest.scala +++ b/src/test/scala/com/typesafe/config/impl/PropertiesTest.scala @@ -34,6 +34,15 @@ class PropertiesTest extends TestUtils { assertEquals("..", exceptLast("...")) } + @Test + def pathObjectCreating() { + def p(key: String) = PropertiesParser.pathFromPropertyKey(key) + + assertEquals(path("a"), p("a")) + assertEquals(path("a", "b"), p("a.b")) + assertEquals(path(""), p("")) + } + @Test def funkyPathsInProperties() { def testPath(propsPath: String, confPath: String) { diff --git a/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala b/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala index 07f23720..232e115b 100644 --- a/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala +++ b/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala @@ -176,6 +176,47 @@ class PublicApiTest extends TestUtils { assertEquals(reunwrapped, unwrapped) } + private def testFromPathMap(expectedValue: ConfigObject, createFrom: java.util.Map[String, Object]) { + assertEquals(expectedValue, Config.fromPathMap(createFrom)) + assertEquals(defaultValueDesc, Config.fromPathMap(createFrom).origin().description()) + assertEquals(expectedValue, Config.fromPathMap(createFrom, "foo")) + assertEquals("foo", Config.fromPathMap(createFrom, "foo").origin().description()) + } + + @Test + def fromJavaPathMap() { + // first the same tests as with fromMap + val emptyMapValue = Collections.emptyMap[String, AbstractConfigValue] + val aMapValue = Map("a" -> 1, "b" -> 2, "c" -> 3).mapValues(intValue(_): AbstractConfigValue).asJava + testFromPathMap(new SimpleConfigObject(fakeOrigin(), emptyMapValue), + Collections.emptyMap[String, Object]) + testFromPathMap(new SimpleConfigObject(fakeOrigin(), aMapValue), + Map("a" -> 1, "b" -> 2, "c" -> 3).asInstanceOf[Map[String, AnyRef]].asJava) + + assertEquals("hardcoded value", Config.fromPathMap(Map("a" -> 1, "b" -> 2, "c" -> 3).asJava).origin().description()) + assertEquals("foo", Config.fromPathMap(Map("a" -> 1, "b" -> 2, "c" -> 3).asJava, "foo").origin().description()) + + // now some tests with paths; be sure to test nested path maps + val simplePathMapValue = Map("x.y" -> 4, "z" -> 5).asInstanceOf[Map[String, AnyRef]].asJava + val pathMapValue = Map("a.c" -> 1, "b" -> simplePathMapValue).asInstanceOf[Map[String, AnyRef]].asJava + + val obj = Config.fromPathMap(pathMapValue) + + assertEquals(2, obj.size) + assertEquals(4, obj.getInt("b.x.y")) + assertEquals(5, obj.getInt("b.z")) + assertEquals(1, obj.getInt("a.c")) + } + + @Test + def brokenPathMap() { + // "a" is both number 1 and an object + val pathMapValue = Map("a" -> 1, "a.b" -> 2).asInstanceOf[Map[String, AnyRef]].asJava + intercept[ConfigException.BugOrBroken] { + Config.fromPathMap(pathMapValue) + } + } + private def resource(filename: String) = { val resourceDir = new File("src/test/resources") if (!resourceDir.exists())