From 64c6c753e5e786105516e96526f9cfb240f6a6d2 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Tue, 8 Nov 2011 22:03:22 -0500 Subject: [PATCH] use internal AbstractConfigValue instead of public ConfigValue throughout implementation In practice don't want to try to support custom ConfigValue. --- .../com/typesafe/config/ConfigObject.java | 6 +-- .../config/impl/AbstractConfigObject.java | 52 +++++++++---------- .../com/typesafe/config/impl/ConfigImpl.java | 21 ++++---- .../com/typesafe/config/impl/ConfigList.java | 22 ++++---- .../config/impl/ConfigTransformer.java | 16 ++++-- .../config/impl/DefaultTransformer.java | 6 +-- .../java/com/typesafe/config/impl/Parser.java | 7 ++- .../config/impl/SimpleConfigObject.java | 13 +++-- .../config/impl/StackConfigObject.java | 5 +- .../config/impl/StackTransformer.java | 6 +-- .../config/impl/TransformedConfigObject.java | 4 +- .../config/impl/ConfigValueTest.scala | 4 +- .../com/typesafe/config/impl/JsonTest.scala | 4 +- 13 files changed, 86 insertions(+), 80 deletions(-) diff --git a/src/main/java/com/typesafe/config/ConfigObject.java b/src/main/java/com/typesafe/config/ConfigObject.java index c051d65e..1d5caec4 100644 --- a/src/main/java/com/typesafe/config/ConfigObject.java +++ b/src/main/java/com/typesafe/config/ConfigObject.java @@ -52,7 +52,7 @@ public interface ConfigObject extends ConfigValue { */ Long getNanoseconds(String path); - List getList(String path); + List getList(String path); List getBooleanList(String path); @@ -64,9 +64,9 @@ public interface ConfigObject extends ConfigValue { List getDoubleList(String path); - List getObjectList(String path); + List getObjectList(String path); - List getAnyList(String path); + List getAnyList(String path); List getMemorySizeList(String path); diff --git a/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java b/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java index 822325d2..17d6862d 100644 --- a/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java +++ b/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java @@ -31,14 +31,15 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements * @param key * @return the unmodified raw value or null */ - protected abstract ConfigValue peek(String key); + protected abstract AbstractConfigValue peek(String key); - protected ConfigValue peek(String key, SubstitutionResolver resolver, + protected AbstractConfigValue peek(String key, + SubstitutionResolver resolver, int depth, boolean withFallbacks) { - ConfigValue v = peek(key); + AbstractConfigValue v = peek(key); if (v != null && resolver != null) { - v = resolver.resolve((AbstractConfigValue) v, depth, withFallbacks); + v = resolver.resolve(v, depth, withFallbacks); } return v; @@ -110,20 +111,17 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements return transformed(obj, transformer); } - static private ConfigValue resolve(AbstractConfigObject self, + static private AbstractConfigValue resolve(AbstractConfigObject self, String path, ConfigValueType expected, ConfigTransformer transformer, String originalPath) { String key = ConfigUtil.firstElement(path); String next = ConfigUtil.otherElements(path); if (next == null) { - ConfigValue v = self.peek(key); + AbstractConfigValue v = self.peek(key); if (v == null) throw new ConfigException.Missing(originalPath); - // FIXME if ConfigTransformer remains public API then - // casting to AbstractConfigValue here is broken, - // but want to make it not public API. if (expected != null && transformer != null) v = transformer.transform(v, expected); @@ -136,13 +134,13 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements else return v; } else { - AbstractConfigObject o = (AbstractConfigObject) self.getObject(key); + AbstractConfigObject o = self.getObject(key); assert (o != null); // missing was supposed to throw return resolve(o, next, expected, transformer, originalPath); } } - ConfigValue resolve(String path, ConfigValueType expected, + AbstractConfigValue resolve(String path, ConfigValueType expected, String originalPath) { return resolve(this, path, expected, transformer, originalPath); } @@ -158,17 +156,17 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements ConfigTransformer transformer) { if (stack.isEmpty()) { return new SimpleConfigObject(origin, transformer, - Collections. emptyMap()); + Collections. emptyMap()); } else if (stack.size() == 1) { return transformed(stack.get(0), transformer); } else { // for non-objects, we just take the first value; but for objects we // have to do work to combine them. - Map merged = new HashMap(); + Map merged = new HashMap(); Map> objects = new HashMap>(); for (AbstractConfigObject obj : stack) { for (String key : obj.keySet()) { - ConfigValue v = obj.peek(key); + AbstractConfigValue v = obj.peek(key); if (!merged.containsKey(key)) { if (v.valueType() == ConfigValueType.OBJECT) { // requires recursive merge and transformer fixup @@ -203,9 +201,9 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements AbstractConfigObject resolveSubstitutions(SubstitutionResolver resolver, int depth, boolean withFallbacks) { - Map changes = new HashMap(); + Map changes = new HashMap(); for (String k : keySet()) { - AbstractConfigValue v = (AbstractConfigValue) peek(k); + AbstractConfigValue v = peek(k); AbstractConfigValue resolved = resolver.resolve(v, depth, withFallbacks); if (resolved != v) { @@ -215,7 +213,7 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements if (changes.isEmpty()) { return this; } else { - Map resolved = new HashMap(); + Map resolved = new HashMap(); for (String k : keySet()) { if (changes.containsKey(k)) { resolved.put(k, changes.get(k)); @@ -266,8 +264,8 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements } @Override - public List getList(String path) { - ConfigValue v = resolve(path, ConfigValueType.LIST, path); + public List getList(String path) { + AbstractConfigValue v = resolve(path, ConfigValueType.LIST, path); return ((ConfigList) v).asJavaList(); } @@ -318,8 +316,10 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements private List getHomogeneousUnwrappedList(String path, ConfigValueType expected) { List l = new ArrayList(); - List list = getList(path); - for (ConfigValue v : list) { + List list = getList(path); + for (ConfigValue cv : list) { + // variance would be nice, but stupid cast will do + AbstractConfigValue v = (AbstractConfigValue) cv; if (expected != null && transformer != null) { v = transformer.transform(v, expected); } @@ -375,7 +375,7 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements @Override public List getObjectList(String path) { List l = new ArrayList(); - List list = getList(path); + List list = getList(path); for (ConfigValue v : list) { if (v.valueType() != ConfigValueType.OBJECT) throw new ConfigException.WrongType(v.origin(), path, @@ -386,9 +386,9 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements } @Override - public List getAnyList(String path) { + public List getAnyList(String path) { List l = new ArrayList(); - List list = getList(path); + List list = getList(path); for (ConfigValue v : list) { l.add(v.unwrapped()); } @@ -398,7 +398,7 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements @Override public List getMemorySizeList(String path) { List l = new ArrayList(); - List list = getList(path); + List list = getList(path); for (ConfigValue v : list) { if (v.valueType() == ConfigValueType.NUMBER) { l.add(((Number) v.unwrapped()).longValue()); @@ -428,7 +428,7 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements @Override public List getNanosecondsList(String path) { List l = new ArrayList(); - List list = getList(path); + List list = getList(path); for (ConfigValue v : list) { if (v.valueType() == ConfigValueType.NUMBER) { l.add(((Number) v.unwrapped()).longValue()); diff --git a/src/main/java/com/typesafe/config/impl/ConfigImpl.java b/src/main/java/com/typesafe/config/impl/ConfigImpl.java index 92082d7f..8d5daf3f 100644 --- a/src/main/java/com/typesafe/config/impl/ConfigImpl.java +++ b/src/main/java/com/typesafe/config/impl/ConfigImpl.java @@ -11,7 +11,6 @@ import java.util.Properties; import com.typesafe.config.ConfigConfig; import com.typesafe.config.ConfigException; import com.typesafe.config.ConfigObject; -import com.typesafe.config.ConfigValue; /** This is public but is only supposed to be used by the "config" package */ public class ConfigImpl { @@ -96,7 +95,7 @@ public class ConfigImpl { private static AbstractConfigObject fromProperties(String originPrefix, Properties props) { - Map> scopes = new HashMap>(); + Map> scopes = new HashMap>(); Enumeration i = props.propertyNames(); while (i.hasMoreElements()) { Object o = i.nextElement(); @@ -107,9 +106,10 @@ public class ConfigImpl { String exceptLast = ConfigUtil.exceptLastElement(path); if (exceptLast == null) exceptLast = ""; - Map scope = scopes.get(exceptLast); + Map scope = scopes + .get(exceptLast); if (scope == null) { - scope = new HashMap(); + scope = new HashMap(); scopes.put(exceptLast, scope); } String value = props.getProperty(path); @@ -134,26 +134,27 @@ public class ConfigImpl { if (parentPath == null) parentPath = ""; - Map parent = scopes.get(parentPath); + Map parent = scopes.get(parentPath); if (parent == null) { - parent = new HashMap(); + parent = new HashMap(); scopes.put(parentPath, parent); } // NOTE: we are evil and cheating, we mutate the map we // provide to SimpleConfigObject, which is not allowed by // its contract, but since we know nobody has a ref to this // SimpleConfigObject yet we can get away with it. - ConfigObject o = new SimpleConfigObject(new SimpleConfigOrigin( + AbstractConfigObject o = new SimpleConfigObject( + new SimpleConfigOrigin( originPrefix + " " + path), null, scopes.get(path)); String basename = ConfigUtil.lastElement(path); parent.put(basename, o); } - Map root = scopes.get(""); + Map root = scopes.get(""); if (root == null) { // this would happen only if you had no properties at all // in "props" - root = Collections. emptyMap(); + root = Collections. emptyMap(); } // return root config object @@ -172,7 +173,7 @@ public class ConfigImpl { private static AbstractConfigObject loadEnvVariables() { Map env = System.getenv(); - Map m = new HashMap(); + Map m = new HashMap(); for (String key : env.keySet()) { m.put(key, new ConfigString( new SimpleConfigOrigin("env var " + key), env.get(key))); diff --git a/src/main/java/com/typesafe/config/impl/ConfigList.java b/src/main/java/com/typesafe/config/impl/ConfigList.java index 7f568d6a..e27db690 100644 --- a/src/main/java/com/typesafe/config/impl/ConfigList.java +++ b/src/main/java/com/typesafe/config/impl/ConfigList.java @@ -10,14 +10,14 @@ import com.typesafe.config.ConfigValueType; final class ConfigList extends AbstractConfigValue { - private List value; + private List value; - ConfigList(ConfigOrigin origin, List value) { + ConfigList(ConfigOrigin origin, List value) { super(origin); this.value = value; } - List asJavaList() { + List asJavaList() { return value; } @@ -29,25 +29,25 @@ final class ConfigList extends AbstractConfigValue { @Override public List unwrapped() { List list = new ArrayList(); - for (ConfigValue v : value) { + for (AbstractConfigValue v : value) { list.add(v.unwrapped()); } return list; } @Override - ConfigList resolveSubstitutions(SubstitutionResolver resolver, - int depth, + ConfigList resolveSubstitutions(SubstitutionResolver resolver, int depth, boolean withFallbacks) { - List changed = null; // lazy-create for optimization + // lazy-create for optimization + List changed = null; int i = 0; - for (ConfigValue v : value) { - AbstractConfigValue resolved = resolver.resolve( - (AbstractConfigValue) v, depth, withFallbacks); + for (AbstractConfigValue v : value) { + AbstractConfigValue resolved = resolver.resolve(v, depth, + withFallbacks); // lazy-create the new list if required if (changed == null && resolved != v) { - changed = new ArrayList(); + changed = new ArrayList(); for (int j = 0; j < i; ++j) { changed.add(value.get(j)); } diff --git a/src/main/java/com/typesafe/config/impl/ConfigTransformer.java b/src/main/java/com/typesafe/config/impl/ConfigTransformer.java index 3777d879..32b604f5 100644 --- a/src/main/java/com/typesafe/config/impl/ConfigTransformer.java +++ b/src/main/java/com/typesafe/config/impl/ConfigTransformer.java @@ -1,17 +1,27 @@ package com.typesafe.config.impl; -import com.typesafe.config.ConfigValue; import com.typesafe.config.ConfigValueType; /** * A ConfigTransformer converts values in the config to other values, most often * it's used to parse strings and treat them as some other kind of value. - * + * * This was originally in the public API but I'm now thinking it is not useful * to customize, so it's not public for now. It's still used internally, but * probably the code could be cleaned up by just hard-coding the equivalent of * the DefaultTransformer. */ interface ConfigTransformer { - ConfigValue transform(ConfigValue value, ConfigValueType requested); + /** + * Because this interface is currently private, it uses AbstractConfigValue; + * if public it would have to use plain ConfigValue. + * + * @param value + * the value to potentially transform + * @param requested + * the target type to transform to + * @return a new value or the original value + */ + AbstractConfigValue transform(AbstractConfigValue value, + ConfigValueType requested); } diff --git a/src/main/java/com/typesafe/config/impl/DefaultTransformer.java b/src/main/java/com/typesafe/config/impl/DefaultTransformer.java index e884cbe1..6c1e9a84 100644 --- a/src/main/java/com/typesafe/config/impl/DefaultTransformer.java +++ b/src/main/java/com/typesafe/config/impl/DefaultTransformer.java @@ -1,6 +1,5 @@ package com.typesafe.config.impl; -import com.typesafe.config.ConfigValue; import com.typesafe.config.ConfigValueType; /** @@ -9,7 +8,8 @@ import com.typesafe.config.ConfigValueType; class DefaultTransformer implements ConfigTransformer { @Override - public ConfigValue transform(ConfigValue value, ConfigValueType requested) { + public AbstractConfigValue transform(AbstractConfigValue value, + ConfigValueType requested) { if (value.valueType() == ConfigValueType.STRING) { String s = (String) value.unwrapped(); switch (requested) { @@ -47,7 +47,7 @@ class DefaultTransformer implements ConfigTransformer { case NUMBER: // FALL THROUGH case BOOLEAN: return new ConfigString(value.origin(), - ((AbstractConfigValue) value).transformToString()); + value.transformToString()); } } diff --git a/src/main/java/com/typesafe/config/impl/Parser.java b/src/main/java/com/typesafe/config/impl/Parser.java index 46513698..f3afdf20 100644 --- a/src/main/java/com/typesafe/config/impl/Parser.java +++ b/src/main/java/com/typesafe/config/impl/Parser.java @@ -19,7 +19,6 @@ import java.util.Stack; import com.typesafe.config.ConfigException; import com.typesafe.config.ConfigOrigin; -import com.typesafe.config.ConfigValue; import com.typesafe.config.ConfigValueType; final class Parser { @@ -243,7 +242,7 @@ final class Parser { private AbstractConfigObject parseObject() { // invoked just after the OPEN_CURLY - Map values = new HashMap(); + Map values = new HashMap(); ConfigOrigin objectOrigin = lineOrigin(); while (true) { Token t = nextTokenIgnoringNewline(); @@ -286,7 +285,7 @@ final class Parser { private ConfigList parseArray() { // invoked just after the OPEN_SQUARE ConfigOrigin arrayOrigin = lineOrigin(); - List values = new ArrayList(); + List values = new ArrayList(); consolidateValueTokens(); @@ -295,7 +294,7 @@ final class Parser { // special-case the first element if (t == Tokens.CLOSE_SQUARE) { return new ConfigList(arrayOrigin, - Collections. emptyList()); + Collections. emptyList()); } else if (Tokens.isValue(t)) { values.add(parseValue(t)); } else if (t == Tokens.OPEN_CURLY) { diff --git a/src/main/java/com/typesafe/config/impl/SimpleConfigObject.java b/src/main/java/com/typesafe/config/impl/SimpleConfigObject.java index a17202ea..c5c2aa07 100644 --- a/src/main/java/com/typesafe/config/impl/SimpleConfigObject.java +++ b/src/main/java/com/typesafe/config/impl/SimpleConfigObject.java @@ -7,15 +7,14 @@ import java.util.Set; import com.typesafe.config.ConfigException; import com.typesafe.config.ConfigObject; import com.typesafe.config.ConfigOrigin; -import com.typesafe.config.ConfigValue; class SimpleConfigObject extends AbstractConfigObject { // this map should never be modified - assume immutable - private Map value; + private Map value; SimpleConfigObject(ConfigOrigin origin, ConfigTransformer transformer, - Map value) { + Map value) { super(origin, transformer); if (value == null) throw new ConfigException.BugOrBroken( @@ -24,7 +23,7 @@ class SimpleConfigObject extends AbstractConfigObject { } @Override - protected ConfigValue peek(String key) { + protected AbstractConfigValue peek(String key) { return value.get(key); } @@ -47,8 +46,8 @@ class SimpleConfigObject extends AbstractConfigObject { return value.keySet(); } - private static boolean mapEquals(Map a, - Map b) { + private static boolean mapEquals(Map a, + Map b) { Set aKeys = a.keySet(); Set bKeys = b.keySet(); @@ -62,7 +61,7 @@ class SimpleConfigObject extends AbstractConfigObject { return true; } - private static int mapHash(Map m) { + private static int mapHash(Map m) { Set keys = m.keySet(); int valuesHash = 0; for (String k : keys) { diff --git a/src/main/java/com/typesafe/config/impl/StackConfigObject.java b/src/main/java/com/typesafe/config/impl/StackConfigObject.java index 91a13897..f9d2784e 100644 --- a/src/main/java/com/typesafe/config/impl/StackConfigObject.java +++ b/src/main/java/com/typesafe/config/impl/StackConfigObject.java @@ -8,7 +8,6 @@ import java.util.Map; import java.util.Set; import com.typesafe.config.ConfigOrigin; -import com.typesafe.config.ConfigValue; /** * This is unused for now, decided that it was too annoying to "lazy merge" and @@ -64,11 +63,11 @@ final class StackConfigObject extends AbstractConfigObject { } @Override - protected ConfigValue peek(String key) { + protected AbstractConfigValue peek(String key) { for (AbstractConfigObject o : stack) { // Important: A ConfigNull value would override // and keep us from returning a later non-null value. - ConfigValue v = o.peek(key); + AbstractConfigValue v = o.peek(key); if (v != null) return v; } diff --git a/src/main/java/com/typesafe/config/impl/StackTransformer.java b/src/main/java/com/typesafe/config/impl/StackTransformer.java index 013a9c34..70d88107 100644 --- a/src/main/java/com/typesafe/config/impl/StackTransformer.java +++ b/src/main/java/com/typesafe/config/impl/StackTransformer.java @@ -2,7 +2,6 @@ package com.typesafe.config.impl; import java.util.List; -import com.typesafe.config.ConfigValue; import com.typesafe.config.ConfigValueType; final class StackTransformer implements ConfigTransformer { @@ -14,8 +13,9 @@ final class StackTransformer implements ConfigTransformer { } @Override - public ConfigValue transform(ConfigValue value, ConfigValueType requested) { - ConfigValue current = value; + public AbstractConfigValue transform(AbstractConfigValue value, + ConfigValueType requested) { + AbstractConfigValue current = value; for (ConfigTransformer t : stack) { current = t.transform(current, requested); } diff --git a/src/main/java/com/typesafe/config/impl/TransformedConfigObject.java b/src/main/java/com/typesafe/config/impl/TransformedConfigObject.java index 9098af18..3cd52e77 100644 --- a/src/main/java/com/typesafe/config/impl/TransformedConfigObject.java +++ b/src/main/java/com/typesafe/config/impl/TransformedConfigObject.java @@ -2,8 +2,6 @@ package com.typesafe.config.impl; import java.util.Set; -import com.typesafe.config.ConfigValue; - class TransformedConfigObject extends AbstractConfigObject { private AbstractConfigObject underlying; @@ -30,7 +28,7 @@ class TransformedConfigObject extends AbstractConfigObject { } @Override - protected ConfigValue peek(String key) { + protected AbstractConfigValue peek(String key) { return underlying.peek(key); } } diff --git a/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala b/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala index 4c44ab89..5474f99e 100644 --- a/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala +++ b/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala @@ -41,8 +41,8 @@ class ConfigValueTest extends TestUtils { checkNotEqualObjects(intValueB, longValue) } - private def configMap(pairs: (String, Int)*): java.util.Map[String, ConfigValue] = { - val m = new java.util.HashMap[String, ConfigValue]() + private def configMap(pairs: (String, Int)*): java.util.Map[String, AbstractConfigValue] = { + val m = new java.util.HashMap[String, AbstractConfigValue]() for (p <- pairs) { m.put(p._1, new ConfigInt(fakeOrigin(), p._2)) } diff --git a/src/test/scala/com/typesafe/config/impl/JsonTest.scala b/src/test/scala/com/typesafe/config/impl/JsonTest.scala index c1136ff6..54bdedcd 100644 --- a/src/test/scala/com/typesafe/config/impl/JsonTest.scala +++ b/src/test/scala/com/typesafe/config/impl/JsonTest.scala @@ -41,12 +41,12 @@ class ParseTest extends TestUtils { } } - private[this] def fromLift(liftValue: lift.JValue): ConfigValue = { + private[this] def fromLift(liftValue: lift.JValue): AbstractConfigValue = { import scala.collection.JavaConverters._ liftValue match { case lift.JObject(fields) => - val m = new HashMap[String, ConfigValue]() + val m = new HashMap[String, AbstractConfigValue]() fields.foreach({ field => m.put(field.name, fromLift(field.value)) }) new SimpleConfigObject(fakeOrigin(), null, m) case lift.JArray(values) =>