From c7c8c2ff1452420e03cd611499faf72228b61eab Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Tue, 8 Nov 2011 17:02:00 -0500 Subject: [PATCH] Implement substitution in parser and config objects --- .../config/impl/AbstractConfigObject.java | 99 +++++- .../config/impl/AbstractConfigValue.java | 18 + .../com/typesafe/config/impl/ConfigImpl.java | 31 +- .../com/typesafe/config/impl/ConfigList.java | 38 +++ .../config/impl/ConfigSubstitution.java | 157 ++++++++- .../java/com/typesafe/config/impl/Parser.java | 56 +++- .../config/impl/SimpleConfigObject.java | 4 + .../typesafe/config/impl/Substitution.java | 44 +++ .../config/impl/SubstitutionResolver.java | 53 +++ .../config/impl/SubstitutionStyle.java | 5 + .../com/typesafe/config/impl/Tokenizer.java | 5 +- .../java/com/typesafe/config/impl/Tokens.java | 15 +- src/test/resources/equiv01/substitutions.conf | 41 +++ .../config/impl/ConfigSubstitutionTest.scala | 307 ++++++++++++++++++ .../config/impl/ConfigValueTest.scala | 13 + .../config/impl/EquivalentsTest.scala | 28 +- .../com/typesafe/config/impl/TestUtils.scala | 35 +- .../typesafe/config/impl/TokenizerTest.scala | 4 +- 18 files changed, 908 insertions(+), 45 deletions(-) create mode 100644 src/main/java/com/typesafe/config/impl/Substitution.java create mode 100644 src/main/java/com/typesafe/config/impl/SubstitutionResolver.java create mode 100644 src/main/java/com/typesafe/config/impl/SubstitutionStyle.java create mode 100644 src/test/resources/equiv01/substitutions.conf create mode 100644 src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala diff --git a/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java b/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java index 8c57a1cb..76146ab3 100644 --- a/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java +++ b/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java @@ -34,6 +34,66 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements */ protected abstract ConfigValue peek(String key); + protected ConfigValue peek(String key, SubstitutionResolver resolver, + int depth, boolean withFallbacks) { + ConfigValue v = peek(key); + + if (v != null && resolver != null) { + v = resolver.resolve((AbstractConfigValue) v, depth, withFallbacks); + } + + return v; + } + + /** + * Looks up the path with no transformation, type conversion, or exceptions + * (just returns null if path not found). + */ + protected ConfigValue peekPath(String path) { + return peekPath(this, path); + } + + protected ConfigValue peekPath(String path, SubstitutionResolver resolver, + int depth, + boolean withFallbacks) { + return peekPath(this, path, resolver, depth, withFallbacks); + } + + private static ConfigValue peekPath(AbstractConfigObject self, String path) { + return peekPath(self, path, null, 0, false); + } + + private static ConfigValue peekPath(AbstractConfigObject self, String path, + SubstitutionResolver resolver, int depth, boolean withFallbacks) { + String key = ConfigUtil.firstElement(path); + String next = ConfigUtil.otherElements(path); + + if (next == null) { + ConfigValue v = self.peek(key, resolver, depth, withFallbacks); + return v; + } else { + // it's important to ONLY resolve substitutions here, not + // all values, because if you resolve arrays or objects + // it creates unnecessary cycles as a side effect (any sibling + // of the object we want to follow could cause a cycle, not just + // the object we want to follow). + + ConfigValue v = self.peek(key); + + if (v instanceof ConfigSubstitution && resolver != null) { + v = resolver.resolve((AbstractConfigValue) v, depth, + withFallbacks); + } + + if (v instanceof AbstractConfigObject) { + return peekPath((AbstractConfigObject) v, next, resolver, + depth, withFallbacks); + } else { + return null; + } + } + } + @Override public ConfigValueType valueType() { return ConfigValueType.OBJECT; @@ -51,7 +111,8 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements return transformed(obj, transformer); } - static private ConfigValue resolve(AbstractConfigObject self, String path, + static private ConfigValue resolve(AbstractConfigObject self, + String path, ConfigValueType expected, ConfigTransformer transformer, String originalPath) { String key = ConfigUtil.firstElement(path); @@ -61,6 +122,9 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements 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); @@ -79,7 +143,7 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements } } - private ConfigValue resolve(String path, ConfigValueType expected, + ConfigValue resolve(String path, ConfigValueType expected, String originalPath) { return resolve(this, path, expected, transformer, originalPath); } @@ -136,6 +200,34 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements } } + @Override + AbstractConfigObject resolveSubstitutions(SubstitutionResolver resolver, + int depth, + boolean withFallbacks) { + Map changes = new HashMap(); + for (String k : keySet()) { + AbstractConfigValue v = (AbstractConfigValue) peek(k); + AbstractConfigValue resolved = resolver.resolve(v, depth, + withFallbacks); + if (resolved != v) { + changes.put(k, resolved); + } + } + if (changes.isEmpty()) { + return this; + } else { + Map resolved = new HashMap(); + for (String k : keySet()) { + if (changes.containsKey(k)) { + resolved.put(k, changes.get(k)); + } else { + resolved.put(k, peek(k)); + } + } + return new SimpleConfigObject(origin(), transformer, resolved); + } + } + @Override public ConfigValue get(String path) { return resolve(path, null, path); @@ -234,7 +326,8 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements } if (v.valueType() != expected) throw new ConfigException.WrongType(v.origin(), path, - expected.name(), v.valueType().name()); + "list of " + expected.name(), "list of " + + v.valueType().name()); l.add((T) v.unwrapped()); } return l; diff --git a/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java b/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java index 8b026567..17aa16ba 100644 --- a/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java +++ b/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java @@ -16,6 +16,24 @@ abstract class AbstractConfigValue implements ConfigValue { return this.origin; } + /** + * Called only by SubstitutionResolver object. + * + * @param resolver + * the resolver doing the resolving + * @param depth + * the number of substitutions followed in resolving the current + * one + * @param withFallbacks + * whether to look at system props and env vars + * @return a new value if there were changes, or this if no changes + */ + AbstractConfigValue resolveSubstitutions(SubstitutionResolver resolver, + int depth, + boolean withFallbacks) { + return this; + } + protected boolean canEqual(Object other) { return other instanceof ConfigValue; } diff --git a/src/main/java/com/typesafe/config/impl/ConfigImpl.java b/src/main/java/com/typesafe/config/impl/ConfigImpl.java index 26dcc01b..54658407 100644 --- a/src/main/java/com/typesafe/config/impl/ConfigImpl.java +++ b/src/main/java/com/typesafe/config/impl/ConfigImpl.java @@ -1,6 +1,7 @@ package com.typesafe.config.impl; import java.util.ArrayList; +import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; import java.util.List; @@ -47,6 +48,14 @@ public class ConfigImpl { withExtraTransformer(extraTransformer)); } + public static ConfigObject getSystemPropertiesAsConfig( + ConfigTransformer extraTransformer) { + // This should not need to create a new config object + // as long as the transformer is just the default transformer. + return AbstractConfigObject.transformed(systemPropertiesConfig(), + withExtraTransformer(extraTransformer)); + } + private static ConfigTransformer withExtraTransformer( ConfigTransformer extraTransformer) { // idea is to avoid creating a new, unique transformer if there's no @@ -72,7 +81,7 @@ public class ConfigImpl { private static AbstractConfigObject systemProperties = null; - private synchronized static AbstractConfigObject systemPropertiesConfig() { + synchronized static AbstractConfigObject systemPropertiesConfig() { if (systemProperties == null) { systemProperties = loadSystemProperties(); } @@ -84,6 +93,11 @@ public class ConfigImpl { } + // this is a hack to let us set system props in the test suite + synchronized static void dropSystemPropertiesConfig() { + systemProperties = null; + } + private static AbstractConfigObject fromProperties(String originPrefix, Properties props) { Map> scopes = new HashMap>(); @@ -121,6 +135,9 @@ public class ConfigImpl { // put everything in its parent, ensuring all parents exist for (String path : childPaths) { String parentPath = ConfigUtil.exceptLastElement(path); + if (parentPath == null) + parentPath = ""; + Map parent = scopes.get(parentPath); if (parent == null) { parent = new HashMap(); @@ -136,15 +153,21 @@ public class ConfigImpl { parent.put(basename, o); } + Map root = scopes.get(""); + if (root == null) { + // this would happen only if you had no properties at all + // in "props" + root = Collections. emptyMap(); + } + // return root config object return new SimpleConfigObject(new SimpleConfigOrigin(originPrefix), - null, - scopes.get("")); + null, root); } private static AbstractConfigObject envVariables = null; - private synchronized static AbstractConfigObject envVariablesConfig() { + synchronized static AbstractConfigObject envVariablesConfig() { if (envVariables == null) { envVariables = loadEnvVariables(); } diff --git a/src/main/java/com/typesafe/config/impl/ConfigList.java b/src/main/java/com/typesafe/config/impl/ConfigList.java index c9aa0ade..7f568d6a 100644 --- a/src/main/java/com/typesafe/config/impl/ConfigList.java +++ b/src/main/java/com/typesafe/config/impl/ConfigList.java @@ -3,6 +3,7 @@ package com.typesafe.config.impl; import java.util.ArrayList; import java.util.List; +import com.typesafe.config.ConfigException; import com.typesafe.config.ConfigOrigin; import com.typesafe.config.ConfigValue; import com.typesafe.config.ConfigValueType; @@ -34,6 +35,43 @@ final class ConfigList extends AbstractConfigValue { return list; } + @Override + ConfigList resolveSubstitutions(SubstitutionResolver resolver, + int depth, + boolean withFallbacks) { + List changed = null; // lazy-create for optimization + int i = 0; + for (ConfigValue v : value) { + AbstractConfigValue resolved = resolver.resolve( + (AbstractConfigValue) v, depth, withFallbacks); + + // lazy-create the new list if required + if (changed == null && resolved != v) { + changed = new ArrayList(); + for (int j = 0; j < i; ++j) { + changed.add(value.get(j)); + } + } + + // once the new list is created, all elements + // have to go in it. + if (changed != null) { + changed.add(resolved); + } + + i += 1; + } + + if (changed != null) { + if (changed.size() != value.size()) + throw new ConfigException.BugOrBroken( + "substituted list's size doesn't match"); + return new ConfigList(origin(), changed); + } else { + return this; + } + } + protected boolean canEqual(Object other) { return other instanceof ConfigList; } diff --git a/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java b/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java index 621b3aee..6beef824 100644 --- a/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java +++ b/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java @@ -2,30 +2,169 @@ package com.typesafe.config.impl; import java.util.List; +import com.typesafe.config.ConfigException; import com.typesafe.config.ConfigOrigin; +import com.typesafe.config.ConfigValue; import com.typesafe.config.ConfigValueType; final class ConfigSubstitution extends AbstractConfigValue { - private AbstractConfigObject root; - private List tokens; + // this is a list of String and Substitution + private List pieces; - ConfigSubstitution(ConfigOrigin origin, AbstractConfigObject root, - List tokens) { + ConfigSubstitution(ConfigOrigin origin, List pieces) { super(origin); - this.root = root; - this.tokens = tokens; + this.pieces = pieces; } @Override public ConfigValueType valueType() { - return null; // FIXME + throw new ConfigException.BugOrBroken( + "tried to get value type on a ConfigSubstitution; need to resolve substitution first"); } @Override public Object unwrapped() { - // TODO Auto-generated method stub - return null; + throw new ConfigException.BugOrBroken( + "tried to unwrap a ConfigSubstitution; need to resolve substitution first"); } + // larger than anyone would ever want + private static final int MAX_DEPTH = 100; + + private ConfigValue findInObject(AbstractConfigObject root, + SubstitutionResolver resolver, /* null if we should not have refs */ + Substitution subst, int depth, + boolean withFallbacks) { + if (depth > MAX_DEPTH) { + throw new ConfigException.BadValue(origin(), subst.reference(), + "Substitution ${" + subst.reference() + + "} is part of a cycle of substitutions"); + } + + ConfigValue result = null; + if (subst.isPath()) { + result = root.peekPath(subst.reference(), resolver, depth, + withFallbacks); + } else { + result = root.peek(subst.reference(), resolver, depth, + withFallbacks); + } + + if (result instanceof ConfigSubstitution) { + throw new ConfigException.BugOrBroken( + "peek or peekPath returned an unresolved substitution"); + } + + if (result != null && result.valueType() == ConfigValueType.NULL) { + result = null; + } + + return result; + } + + private ConfigValue resolve(SubstitutionResolver resolver, + Substitution subst, + int depth, boolean withFallbacks) { + ConfigValue result = findInObject(resolver.root(), resolver, subst, + depth, withFallbacks); + if (withFallbacks) { + if (result == null) { + result = findInObject(ConfigImpl.systemPropertiesConfig(), + null, + subst, depth, withFallbacks); + } + if (result == null) { + result = findInObject(ConfigImpl.envVariablesConfig(), null, + subst, + depth, withFallbacks); + } + } + if (result == null) { + result = new ConfigNull(origin()); + } + return result; + } + + private ConfigValue resolve(SubstitutionResolver resolver, int depth, + boolean withFallbacks) { + if (pieces.size() > 1) { + // need to concat everything into a string + StringBuilder sb = new StringBuilder(); + for (Object p : pieces) { + if (p instanceof String) { + sb.append((String) p); + } else { + ConfigValue v = resolve(resolver, (Substitution) p, + depth, withFallbacks); + switch (v.valueType()) { + case NULL: + // nothing; becomes empty string + break; + case LIST: + case OBJECT: + // cannot substitute lists and objects into strings + throw new ConfigException.WrongType(v.origin(), + ((Substitution) p).reference(), + "not a list or object", v.valueType().name()); + default: + sb.append(((AbstractConfigValue) v).transformToString()); + } + } + } + return new ConfigString(origin(), sb.toString()); + } else { + if (!(pieces.get(0) instanceof Substitution)) + throw new ConfigException.BugOrBroken( + "ConfigSubstitution should never contain a single String piece"); + return resolve(resolver, (Substitution) pieces.get(0), depth, + withFallbacks); + } + } + + @Override + AbstractConfigValue resolveSubstitutions(SubstitutionResolver resolver, + int depth, + boolean withFallbacks) { + // only ConfigSubstitution adds to depth here, because the depth + // is the substitution depth not the recursion depth + AbstractConfigValue resolved = (AbstractConfigValue) resolve(resolver, + depth + 1, withFallbacks); + return resolved; + } + + protected boolean canEqual(Object other) { + return other instanceof ConfigSubstitution; + } + + @Override + public boolean equals(Object other) { + // note that "origin" is deliberately NOT part of equality + if (other instanceof ConfigSubstitution) { + return canEqual(other) + && this.pieces.equals(((ConfigSubstitution) other).pieces); + } else { + return false; + } + } + + @Override + public int hashCode() { + // note that "origin" is deliberately NOT part of equality + return pieces.hashCode(); + } + + @Override + public String toString() { + StringBuilder sb = new StringBuilder(); + sb.append("SUBST"); + sb.append("("); + for (Object p : pieces) { + sb.append(p.toString()); + sb.append(","); + } + sb.setLength(sb.length() - 1); // chop comma + sb.append(")"); + return sb.toString(); + } } diff --git a/src/main/java/com/typesafe/config/impl/Parser.java b/src/main/java/com/typesafe/config/impl/Parser.java index 9fe15efb..46513698 100644 --- a/src/main/java/com/typesafe/config/impl/Parser.java +++ b/src/main/java/com/typesafe/config/impl/Parser.java @@ -99,12 +99,16 @@ final class Parser { t = buffer.pop(); } - if (Tokens.isUnquotedText(t) && flavor == SyntaxFlavor.JSON) { - throw parseError("Token not allowed in valid JSON: '" + if (flavor == SyntaxFlavor.JSON) { + if (Tokens.isUnquotedText(t)) { + throw parseError("Token not allowed in valid JSON: '" + Tokens.getUnquotedText(t) + "'"); - } else { - return t; + } else if (Tokens.isSubstitution(t)) { + throw parseError("Substitutions (${} syntax) not allowed in JSON"); + } } + + return t; } private void putBack(Token token) { @@ -130,8 +134,8 @@ final class Parser { List values = null; // create only if we have value tokens Token t = nextTokenIgnoringNewline(); // ignore a newline up front - while (Tokens.isValue(t) - || Tokens.isUnquotedText(t)) { + while (Tokens.isValue(t) || Tokens.isUnquotedText(t) + || Tokens.isSubstitution(t)) { if (values == null) values = new ArrayList(); values.add(t); @@ -143,12 +147,15 @@ final class Parser { if (values == null) return; - if (values.size() == 1 && !Tokens.isUnquotedText(values.get(0))) { + if (values.size() == 1 && Tokens.isValue(values.get(0))) { // a single value token requires no consolidation putBack(values.get(0)); return; } + // this will be a list of String and Substitution + List minimized = new ArrayList(); + // we have multiple value tokens or one unquoted text token; // collapse into a string token. StringBuilder sb = new StringBuilder(); @@ -164,6 +171,20 @@ final class Parser { if (firstOrigin == null) firstOrigin = Tokens.getUnquotedTextOrigin(valueToken); sb.append(text); + } else if (Tokens.isSubstitution(valueToken)) { + if (firstOrigin == null) + firstOrigin = Tokens.getSubstitutionOrigin(valueToken); + + if (sb.length() > 0) { + // save string so far + minimized.add(sb.toString()); + sb.setLength(0); + } + // now save substitution + String reference = Tokens.getSubstitution(valueToken); + SubstitutionStyle style = Tokens + .getSubstitutionStyle(valueToken); + minimized.add(new Substitution(reference, style)); } else { throw new ConfigException.BugOrBroken( "should not be trying to consolidate token: " @@ -171,7 +192,26 @@ final class Parser { } } - Token consolidated = Tokens.newString(firstOrigin, sb.toString()); + if (sb.length() > 0) { + // save string so far + minimized.add(sb.toString()); + } + + if (minimized.isEmpty()) + throw new ConfigException.BugOrBroken( + "trying to consolidate values to nothing"); + + Token consolidated = null; + + if (minimized.size() == 1 && minimized.get(0) instanceof String) { + consolidated = Tokens.newString(firstOrigin, + (String) minimized.get(0)); + } else { + // there's some substitution to do later (post-parse step) + consolidated = Tokens.newValue(new ConfigSubstitution( + firstOrigin, minimized)); + } + putBack(consolidated); } diff --git a/src/main/java/com/typesafe/config/impl/SimpleConfigObject.java b/src/main/java/com/typesafe/config/impl/SimpleConfigObject.java index 2d17fd0d..4b13862b 100644 --- a/src/main/java/com/typesafe/config/impl/SimpleConfigObject.java +++ b/src/main/java/com/typesafe/config/impl/SimpleConfigObject.java @@ -4,6 +4,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; +import com.typesafe.config.ConfigException; import com.typesafe.config.ConfigObject; import com.typesafe.config.ConfigOrigin; import com.typesafe.config.ConfigTransformer; @@ -17,6 +18,9 @@ class SimpleConfigObject extends AbstractConfigObject { SimpleConfigObject(ConfigOrigin origin, ConfigTransformer transformer, Map value) { super(origin, transformer); + if (value == null) + throw new ConfigException.BugOrBroken( + "creating config object with null map"); this.value = value; } diff --git a/src/main/java/com/typesafe/config/impl/Substitution.java b/src/main/java/com/typesafe/config/impl/Substitution.java new file mode 100644 index 00000000..4b147ba0 --- /dev/null +++ b/src/main/java/com/typesafe/config/impl/Substitution.java @@ -0,0 +1,44 @@ +package com.typesafe.config.impl; + + +final class Substitution { + private SubstitutionStyle style; + private String reference; + + Substitution(String reference, SubstitutionStyle style) { + this.style = style; + this.reference = reference; + } + + SubstitutionStyle style() { + return style; + } + + String reference() { + return reference; + } + + boolean isPath() { + return style == SubstitutionStyle.PATH; + } + + @Override + public boolean equals(Object other) { + if (other instanceof Substitution) { + Substitution that = (Substitution) other; + return this.reference == that.reference && this.style == that.style; + } else { + return false; + } + } + + @Override + public int hashCode() { + return 41 * (41 + reference.hashCode()) + style.hashCode(); + } + + @Override + public String toString() { + return "Substitution(" + reference + "," + style.name() + ")"; + } +} diff --git a/src/main/java/com/typesafe/config/impl/SubstitutionResolver.java b/src/main/java/com/typesafe/config/impl/SubstitutionResolver.java new file mode 100644 index 00000000..3834d98a --- /dev/null +++ b/src/main/java/com/typesafe/config/impl/SubstitutionResolver.java @@ -0,0 +1,53 @@ +package com.typesafe.config.impl; + +import java.util.IdentityHashMap; +import java.util.Map; + +/** + * This exists because we have to memoize resolved substitutions as we go + * through the config tree; otherwise we could end up creating multiple copies + * of values or whole trees of values as we follow chains of substitutions. + */ +final class SubstitutionResolver { + private AbstractConfigObject root; + private Map memos; + + SubstitutionResolver(AbstractConfigObject root) { + this.root = root; + // note: the memoization is by object identity, not object value + this.memos = new IdentityHashMap(); + } + + AbstractConfigValue resolve(AbstractConfigValue original, int depth, + boolean withFallbacks) { + if (memos.containsKey(original)) { + return memos.get(original); + } else { + AbstractConfigValue resolved = original.resolveSubstitutions(this, + depth, + withFallbacks); + memos.put(original, resolved); + return resolved; + } + } + + AbstractConfigObject root() { + return this.root; + } + + private static AbstractConfigValue resolve(AbstractConfigValue value, + AbstractConfigObject root, boolean withFallbacks) { + SubstitutionResolver resolver = new SubstitutionResolver(root); + return resolver.resolve(value, 0, withFallbacks); + } + + static AbstractConfigValue resolve(AbstractConfigValue value, + AbstractConfigObject root) { + return resolve(value, root, true /* withFallbacks */); + } + + static AbstractConfigValue resolveWithoutFallbacks( + AbstractConfigValue value, AbstractConfigObject root) { + return resolve(value, root, false /* withFallbacks */); + } +} diff --git a/src/main/java/com/typesafe/config/impl/SubstitutionStyle.java b/src/main/java/com/typesafe/config/impl/SubstitutionStyle.java new file mode 100644 index 00000000..4729b3cd --- /dev/null +++ b/src/main/java/com/typesafe/config/impl/SubstitutionStyle.java @@ -0,0 +1,5 @@ +package com.typesafe.config.impl; + +enum SubstitutionStyle { + PATH, KEY +} diff --git a/src/main/java/com/typesafe/config/impl/Tokenizer.java b/src/main/java/com/typesafe/config/impl/Tokenizer.java index 03eb67a7..1d07d88d 100644 --- a/src/main/java/com/typesafe/config/impl/Tokenizer.java +++ b/src/main/java/com/typesafe/config/impl/Tokenizer.java @@ -301,7 +301,10 @@ final class Tokenizer { reference = Tokens.getUnquotedText(t); } } while (c != '}'); - return Tokens.newSubstitution(origin, reference, wasQuoted); + + SubstitutionStyle style = ((!wasQuoted) && reference.indexOf('.') >= 0) ? SubstitutionStyle.PATH + : SubstitutionStyle.KEY; + return Tokens.newSubstitution(origin, reference, style); } // called if the next token is not a simple value; diff --git a/src/main/java/com/typesafe/config/impl/Tokens.java b/src/main/java/com/typesafe/config/impl/Tokens.java index 8d379df9..6dd66d4a 100644 --- a/src/main/java/com/typesafe/config/impl/Tokens.java +++ b/src/main/java/com/typesafe/config/impl/Tokens.java @@ -127,14 +127,14 @@ final class Tokens { private String value; private boolean isPath; - Substitution(ConfigOrigin origin, String s, boolean wasQuoted) { + Substitution(ConfigOrigin origin, String s, SubstitutionStyle style) { super(TokenType.SUBSTITUTION); this.origin = origin; this.value = s; // if the string is not quoted and contains '.' then // it's a path rather than just a key name. - this.isPath = (!wasQuoted) && s.indexOf('.') >= 0; + this.isPath = style == SubstitutionStyle.PATH; } ConfigOrigin origin() { @@ -248,12 +248,13 @@ final class Tokens { } } - static boolean getSubstitutionIsPath(Token token) { + static SubstitutionStyle getSubstitutionStyle(Token token) { if (token instanceof Substitution) { - return ((Substitution) token).isPath(); + return ((Substitution) token).isPath() ? SubstitutionStyle.PATH + : SubstitutionStyle.KEY; } else { throw new ConfigException.BugOrBroken( - "tried to get substitution is path from " + token); + "tried to get substitution style from " + token); } } @@ -275,8 +276,8 @@ final class Tokens { } static Token newSubstitution(ConfigOrigin origin, String s, - boolean wasQuoted) { - return new Substitution(origin, s, wasQuoted); + SubstitutionStyle style) { + return new Substitution(origin, s, style); } static Token newValue(AbstractConfigValue value) { diff --git a/src/test/resources/equiv01/substitutions.conf b/src/test/resources/equiv01/substitutions.conf new file mode 100644 index 00000000..dcadb2fa --- /dev/null +++ b/src/test/resources/equiv01/substitutions.conf @@ -0,0 +1,41 @@ +{ + "ints" : { + "fortyTwo" : 42, + "fortyTwoAgain" : ${ints.fortyTwo} + }, + + "floats" : { + "fortyTwoPointOne" : 42.1, + "fortyTwoPointOneAgain" : ${floats.fortyTwoPointOne} + }, + + "strings" : { + "abcd" : "abcd", + "abcdAgain" : ${strings.a}${strings.b}${strings.c}${strings.d}, + "a" : "a", + "b" : "b", + "c" : "c", + "d" : "d", + "concatenated" : "null bar 42 baz true 3.14 hi" + }, + + "arrays" : { + "empty" : [], + "1" : [ 1 ], + "12" : [1, 2], + "123" : [1, 2, 3], + "ofString" : [ ${strings.a}, ${strings.b}, ${strings.c} ] + }, + + "booleans" : { + "true" : true, + "trueAgain" : ${booleans.true}, + "false" : false, + "falseAgain" : ${booleans.false} + }, + + "nulls" : { + "null" : null, + "nullAgain" : null + } +} diff --git a/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala b/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala new file mode 100644 index 00000000..e119565f --- /dev/null +++ b/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala @@ -0,0 +1,307 @@ +package com.typesafe.config.impl + +import org.junit.Assert._ +import org.junit._ +import com.typesafe.config.ConfigValue +import com.typesafe.config.ConfigException + +class ConfigSubstitutionTest extends TestUtils { + + private def parseObject(s: String) = { + Parser.parse(SyntaxFlavor.CONF, new SimpleConfigOrigin("test string"), s).asInstanceOf[AbstractConfigObject] + } + + private def subst(ref: String, style: SubstitutionStyle = SubstitutionStyle.PATH) = { + val pieces = java.util.Collections.singletonList[Object](new Substitution(ref, style)) + new ConfigSubstitution(fakeOrigin(), pieces) + } + + private def substInString(ref: String, style: SubstitutionStyle = SubstitutionStyle.PATH) = { + import scala.collection.JavaConverters._ + val pieces = List("start<", new Substitution(ref, style), ">end") + new ConfigSubstitution(fakeOrigin(), pieces.asJava) + } + + private def intValue(i: Int) = new ConfigInt(fakeOrigin(), i) + private def boolValue(b: Boolean) = new ConfigBoolean(fakeOrigin(), b) + private def nullValue() = new ConfigNull(fakeOrigin()) + private def stringValue(s: String) = new ConfigString(fakeOrigin(), s) + private def doubleValue(d: Double) = new ConfigDouble(fakeOrigin(), d) + + private def resolveWithoutFallbacks(v: AbstractConfigObject) = { + SubstitutionResolver.resolveWithoutFallbacks(v, v).asInstanceOf[AbstractConfigObject] + } + private def resolveWithoutFallbacks(s: ConfigSubstitution, root: AbstractConfigObject) = { + SubstitutionResolver.resolveWithoutFallbacks(s, root) + } + + private def resolve(v: AbstractConfigObject) = { + SubstitutionResolver.resolve(v, v).asInstanceOf[AbstractConfigObject] + } + private def resolve(s: ConfigSubstitution, root: AbstractConfigObject) = { + SubstitutionResolver.resolve(s, root) + } + + private val simpleObject = { + parseObject(""" +{ + "foo" : 42, + "bar" : { + "int" : 43, + "bool" : true, + "null" : null, + "string" : "hello", + "double" : 3.14 + } +} +""") + } + + @Test + def resolveTrivialKey() { + val s = subst("foo", SubstitutionStyle.KEY) + val v = resolveWithoutFallbacks(s, simpleObject) + assertEquals(intValue(42), v) + } + + @Test + def resolveTrivialPath() { + val s = subst("bar.int") + val v = resolveWithoutFallbacks(s, simpleObject) + assertEquals(intValue(43), v) + } + + @Test + def resolveInt() { + val s = subst("bar.int") + val v = resolveWithoutFallbacks(s, simpleObject) + assertEquals(intValue(43), v) + } + + @Test + def resolveBool() { + val s = subst("bar.bool") + val v = resolveWithoutFallbacks(s, simpleObject) + assertEquals(boolValue(true), v) + } + + @Test + def resolveNull() { + val s = subst("bar.null") + val v = resolveWithoutFallbacks(s, simpleObject) + assertEquals(nullValue(), v) + } + + @Test + def resolveString() { + val s = subst("bar.string") + val v = resolveWithoutFallbacks(s, simpleObject) + assertEquals(stringValue("hello"), v) + } + + @Test + def resolveDouble() { + val s = subst("bar.double") + val v = resolveWithoutFallbacks(s, simpleObject) + assertEquals(doubleValue(3.14), v) + } + + @Test + def resolveIntInString() { + val s = substInString("bar.int") + val v = resolveWithoutFallbacks(s, simpleObject) + assertEquals(stringValue("start<43>end"), v) + } + + @Test + def resolveNullInString() { + val s = substInString("bar.null") + val v = resolveWithoutFallbacks(s, simpleObject) + // null is supposed to become empty string + assertEquals(stringValue("start<>end"), v) + } + + @Test + def resolveMissingInString() { + val s = substInString("bar.missing") + val v = resolveWithoutFallbacks(s, simpleObject) + // absent object becomes empty string + assertEquals(stringValue("start<>end"), v) + } + + @Test + def resolveBoolInString() { + val s = substInString("bar.bool") + val v = resolveWithoutFallbacks(s, simpleObject) + assertEquals(stringValue("startend"), v) + } + + @Test + def resolveStringInString() { + val s = substInString("bar.string") + val v = resolveWithoutFallbacks(s, simpleObject) + assertEquals(stringValue("startend"), v) + } + + @Test + def resolveDoubleInString() { + val s = substInString("bar.double") + val v = resolveWithoutFallbacks(s, simpleObject) + assertEquals(stringValue("start<3.14>end"), v) + } + + private val substChainObject = { + parseObject(""" +{ + "foo" : ${bar}, + "bar" : ${a.b.c}, + "a" : { "b" : { "c" : 57 } } +} +""") + } + + @Test + def chainSubstitutions() { + val s = subst("foo") + val v = resolveWithoutFallbacks(s, substChainObject) + assertEquals(intValue(57), v) + } + + private val substCycleObject = { + parseObject(""" +{ + "foo" : ${bar}, + "bar" : ${a.b.c}, + "a" : { "b" : { "c" : ${foo} } } +} +""") + } + + @Test + def throwOnCycles() { + val s = subst("foo") + val e = intercept[ConfigException.BadValue] { + val v = resolveWithoutFallbacks(s, substCycleObject) + } + assertTrue(e.getMessage().contains("cycle")) + } + + @Test + def resolveObject() { + val resolved = resolveWithoutFallbacks(substChainObject) + assertEquals(57, resolved.getInt("foo")) + assertEquals(57, resolved.getInt("bar")) + assertEquals(57, resolved.getInt("a.b.c")) + } + + private val substSideEffectCycle = { + parseObject(""" +{ + "foo" : ${a.b.c}, + "a" : { "b" : { "c" : 42, "cycle" : ${foo} }, "cycle" : ${foo} } +} +""") + } + + @Test + def avoidSideEffectCycles() { + // The point of this test is that in traversing objects + // to resolve a path, we need to avoid resolving + // substitutions that are in the traversed objects but + // are not directly required to resolve the path. + // i.e. there should not be a cycle in this test. + + val resolved = resolveWithoutFallbacks(substSideEffectCycle) + + assertEquals(42, resolved.getInt("foo")) + assertEquals(42, resolved.getInt("a.b.cycle")) + assertEquals(42, resolved.getInt("a.cycle")) + } + + private val substComplexObject = { + parseObject(""" +{ + "foo" : ${bar}, + "bar" : ${a.b.c}, + "a" : { "b" : { "c" : 57, "d" : ${foo}, "e" : { "f" : ${foo} } } }, + "objA" : ${a}, + "objB" : ${a.b}, + "objE" : ${a.b.e}, + "foo.bar" : 37, + "arr" : [ ${foo}, ${a.b.c}, ${"foo.bar"}, ${objB.d}, ${objA.b.e.f}, ${objE.f} ], + "ptrToArr" : ${arr}, + "x" : { "y" : { "ptrToPtrToArr" : ${ptrToArr} } } +} +""") + } + + @Test + def complexResolve() { + import scala.collection.JavaConverters._ + + val resolved = resolveWithoutFallbacks(substComplexObject) + + assertEquals(57, resolved.getInt("foo")) + assertEquals(57, resolved.getInt("bar")) + assertEquals(57, resolved.getInt("a.b.c")) + assertEquals(57, resolved.getInt("a.b.d")) + assertEquals(57, resolved.getInt("objB.d")) + assertEquals(Seq(57, 57, 37, 57, 57, 57), resolved.getIntList("arr").asScala) + assertEquals(Seq(57, 57, 37, 57, 57, 57), resolved.getIntList("ptrToArr").asScala) + assertEquals(Seq(57, 57, 37, 57, 57, 57), resolved.getIntList("x.y.ptrToPtrToArr").asScala) + } + + private val substSystemPropsObject = { + parseObject(""" +{ + "a" : ${configtest.a}, + "b" : ${configtest.b} +} +""") + } + + @Test + def fallbackToSystemProps() { + System.setProperty("configtest.a", "1234") + System.setProperty("configtest.b", "5678") + ConfigImpl.dropSystemPropertiesConfig() + + val resolved = resolve(substSystemPropsObject) + + assertEquals("1234", resolved.getString("a")) + assertEquals("5678", resolved.getString("b")) + } + + private val substEnvVarObject = { + parseObject(""" +{ + "home" : ${HOME}, + "pwd" : ${PWD}, + "shell" : ${SHELL}, + "lang" : ${LANG}, + "path" : ${PATH} +} +""") + } + + @Test + def fallbackToEnv() { + import scala.collection.JavaConverters._ + + val resolved = resolve(substEnvVarObject) + + var existed = 0 + for (k <- resolved.keySet().asScala) { + val e = System.getenv(k.toUpperCase()); + if (e != null) { + existed += 1 + assertEquals(e, resolved.getString(k)) + } else { + assertEquals(null, resolved.getAny(k)) + } + } + if (existed == 0) { + throw new Exception("None of the env vars we tried to use for testing were set") + } + } +} diff --git a/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala b/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala index ebccf6c7..3a06c79d 100644 --- a/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala +++ b/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala @@ -66,4 +66,17 @@ class ConfigValueTest extends TestUtils { checkEqualObjects(a, sameAsA) checkNotEqualObjects(a, b) } + + @Test + def substitutionEquality() { + val a = new Substitution("foo", SubstitutionStyle.KEY); + val sameAsA = new Substitution("foo", SubstitutionStyle.KEY); + val differentRef = new Substitution("bar", SubstitutionStyle.KEY); + val differentStyle = new Substitution("foo", SubstitutionStyle.PATH); + + checkEqualObjects(a, a) + checkEqualObjects(a, sameAsA) + checkNotEqualObjects(a, differentRef) + checkNotEqualObjects(a, differentStyle) + } } diff --git a/src/test/scala/com/typesafe/config/impl/EquivalentsTest.scala b/src/test/scala/com/typesafe/config/impl/EquivalentsTest.scala index a90324b1..8a639a86 100644 --- a/src/test/scala/com/typesafe/config/impl/EquivalentsTest.scala +++ b/src/test/scala/com/typesafe/config/impl/EquivalentsTest.scala @@ -28,6 +28,26 @@ class EquivalentsTest extends TestUtils { files } + private def postParse(value: ConfigValue) = { + value match { + case v: AbstractConfigObject => + // for purposes of these tests, substitutions are only + // against the same file's root, and without looking at + // system prop or env variable fallbacks. + SubstitutionResolver.resolveWithoutFallbacks(v, v) + case v => + v + } + } + + private def parse(flavor: SyntaxFlavor, f: File) = { + postParse(Parser.parse(flavor, f)) + } + + private def parse(f: File) = { + postParse(Parser.parse(f)) + } + // would like each "equivNN" directory to be a suite and each file in the dir // to be a test, but not sure how to convince junit to do that. @Test @@ -43,11 +63,11 @@ class EquivalentsTest extends TestUtils { throw new RuntimeException("Need a file named 'original' in " + equiv.getPath()) if (originals.size > 1) throw new RuntimeException("Multiple 'original' files in " + equiv.getPath() + ": " + originals) - val original = Parser.parse(originals(0)) + val original = parse(originals(0)) for (testFile <- others) { fileCount += 1 - val value = Parser.parse(testFile) + val value = parse(testFile) describeFailure(testFile.getPath()) { assertEquals(original, value) } @@ -55,7 +75,7 @@ class EquivalentsTest extends TestUtils { // check that all .json files can be parsed as .conf, // i.e. .conf must be a superset of JSON if (testFile.getName().endsWith(".json")) { - val parsedAsConf = Parser.parse(SyntaxFlavor.CONF, testFile) + val parsedAsConf = parse(SyntaxFlavor.CONF, testFile) describeFailure(testFile.getPath() + " parsed as .conf") { assertEquals(original, parsedAsConf) } @@ -66,6 +86,6 @@ class EquivalentsTest extends TestUtils { // This is a little "checksum" to be sure we really tested what we were expecting. // it breaks every time you add a file, so you have to update it. assertEquals(1, dirCount) - assertEquals(2, fileCount) + assertEquals(3, fileCount) } } diff --git a/src/test/scala/com/typesafe/config/impl/TestUtils.scala b/src/test/scala/com/typesafe/config/impl/TestUtils.scala index 0e7340da..09aab253 100644 --- a/src/test/scala/com/typesafe/config/impl/TestUtils.scala +++ b/src/test/scala/com/typesafe/config/impl/TestUtils.scala @@ -48,9 +48,16 @@ abstract trait TestUtils { new SimpleConfigOrigin("fake origin") } - case class ParseTest(liftBehaviorUnexpected: Boolean, test: String) + case class ParseTest(liftBehaviorUnexpected: Boolean, whitespaceMatters: Boolean, test: String) + object ParseTest { + def apply(liftBehaviorUnexpected: Boolean, test: String): ParseTest = { + ParseTest(liftBehaviorUnexpected, false, test); + } + } implicit def string2jsontest(test: String): ParseTest = ParseTest(false, test) + // note: it's important to put {} or [] at the root if you + // want to test "invalidity reasons" other than "wrong root" private val invalidJsonInvalidConf = List[ParseTest]("", // empty document "{", "}", @@ -78,6 +85,13 @@ abstract trait TestUtils { "{}true", // trailing token after the root object ParseTest(true, "[]{}"), // trailing valid token after the root array "[]true", // trailing valid token after the root array + "[${]", // unclosed substitution + "[$]", // '$' by itself + ParseTest(false, true, "[${ foo.bar}]"), // substitution with leading spaces + ParseTest(false, true, "[${foo.bar }]"), // substitution with trailing spaces + ParseTest(false, true, "[${ \"foo.bar\"}]"), // substitution with leading spaces and quoted + ParseTest(false, true, "[${\"foo.bar\" }]"), // substitution with trailing spaces and quoted + "[${true}]", // substitution with unquoted true token "") // empty document again, just for clean formatting of this list ;-) // We'll automatically try each of these with whitespace modifications @@ -112,7 +126,11 @@ abstract trait TestUtils { "[ tru ]", "[ trux ]", "[ truex ]", - "[ 10x ]") // number token with trailing junk + "[ 10x ]", // number token with trailing junk + "[ ${foo} ]", + "[ ${\"foo\"} ]", + "[ ${foo.bar} ]", + "[ ${\"foo.bar\"} ]") protected val invalidJson = validConfInvalidJson ++ invalidJsonInvalidConf; @@ -130,10 +148,13 @@ abstract trait TestUtils { { s: String => s.replace(":", " : ") }, // could break with : in a key or value { s: String => s.replace(",", " , ") } // could break with , in a key or value ) - for { - t <- tests - v <- variations - } yield ParseTest(t.liftBehaviorUnexpected, v(t.test)) + tests flatMap { t => + if (t.whitespaceMatters) { + return Seq(t) + } else { + for (v <- variations) + yield ParseTest(t.liftBehaviorUnexpected, v(t.test)) + } + } } - } diff --git a/src/test/scala/com/typesafe/config/impl/TokenizerTest.scala b/src/test/scala/com/typesafe/config/impl/TokenizerTest.scala index edce9dae..64909ebe 100644 --- a/src/test/scala/com/typesafe/config/impl/TokenizerTest.scala +++ b/src/test/scala/com/typesafe/config/impl/TokenizerTest.scala @@ -18,8 +18,8 @@ class TokenizerTest extends TestUtils { def tokenFalse = Tokens.newBoolean(fakeOrigin(), false) def tokenNull = Tokens.newNull(fakeOrigin()) def tokenUnquoted(s: String) = Tokens.newUnquotedText(fakeOrigin(), s) - def tokenKeySubstitution(s: String) = Tokens.newSubstitution(fakeOrigin(), s, true /* wasQuoted */ ) - def tokenPathSubstitution(s: String) = Tokens.newSubstitution(fakeOrigin(), s, false /* wasQuoted */ ) + def tokenKeySubstitution(s: String) = Tokens.newSubstitution(fakeOrigin(), s, SubstitutionStyle.KEY) + def tokenPathSubstitution(s: String) = Tokens.newSubstitution(fakeOrigin(), s, SubstitutionStyle.PATH) def tokenString(s: String) = Tokens.newString(fakeOrigin(), s) def tokenDouble(d: Double) = Tokens.newDouble(fakeOrigin(), d) def tokenInt(i: Int) = Tokens.newInt(fakeOrigin(), i)