Make ${missing} throw an exception and add ${?missing} for the old behavior.

Also updated the spec; in this patch, the spec says ${?missing} will be
undefined, but the code still evaluates it to null as in the old
behavior. A follow-up patch will introduce the undefined behavior
that is now specified.

This change is needed because missing substitutions are probably
a problem in most cases, and evaluating missing to null is probably
not as useful as evaluating it to undefined. If it turns out that
we need the null behavior, a possible syntax is ${foo.bar,null}
or something like that.
This commit is contained in:
Havoc Pennington 2011-11-27 11:29:12 -05:00
parent 5a1bd5aa58
commit 4dddfdaf19
18 changed files with 268 additions and 94 deletions

View File

@ -277,8 +277,7 @@ converted to strings as follows (strings shown as quoted strings):
For purposes of value concatenation, it should be rendered
as it was written in the file.
- a substitution is replaced with its value which is then
converted to a string as above, except that a substitution
which evaluates to `null` becomes the empty string `""`.
converted to a string as above.
- it is invalid for arrays or objects to appear in a value
concatenation.
@ -398,9 +397,14 @@ implementations may try to resolve them by looking at system
environment variables, Java system properties, or other external
sources of configuration.
The syntax is `${pathexpression}` where the `pathexpression` is a
path expression as described above. This path expression has the
same syntax that you could use for an object key.
The syntax is `${pathexpression}` or `${?pathexpression}` where
the `pathexpression` is a path expression as described above. This
path expression has the same syntax that you could use for an
object key.
The `?` in `${?pathexpression}` must not have whitespace before
it; the three characters `${?` must be exactly like that, grouped
together.
Substitutions are not parsed inside quoted strings. To get a
string containing a substitution, you must use value concatenation
@ -437,8 +441,21 @@ environment variable. There is no equivalent to JavaScript's
`delete` operation in other words.
If a substitution does not match any value present in the
configuration and is not resolved by an external source, it is
evaluated to `null`.
configuration and is not resolved by an external source, then it
is undefined. An undefined substitution with the `${foo}` syntax
is invalid and should generate an error.
If a substitution with the `${?foo}` syntax is undefined:
- if it is the value of an object field then the field should not
be created.
- if it is an array element then the element should not be added.
- if it is part of a value concatenation then it should become an
empty string.
- `foo : ${?bar}` would avoid creating field `foo` if `bar` is
undefined, but `foo : ${?bar} ${?baz}` would be a value
concatenation so if `bar` or `baz` are not defined, the result
is an empty string.
Substitutions are only allowed in object field values and array
elements (value concatenations), they are not allowed in keys or
@ -447,13 +464,7 @@ nested inside other substitutions (path expressions).
A substitution is replaced with any value type (number, object,
string, array, true, false, null). If the substitution is the only
part of a value, then the type is preserved. Otherwise, it is
value-concatenated to form a string. There is one special rule:
- `null` is converted to an empty string, not the string `null`.
Because missing substitutions are evaluated to `null`, either
missing or explicitly-set-to-null substitutions become an empty
string when concatenated.
value-concatenated to form a string.
Circular substitutions are invalid and should generate an error.

View File

@ -102,6 +102,8 @@ detail.
environment variables if they don't resolve in the
config itself, so `${HOME}` or `${user.home}` would
work as you expect.
- substitutions normally cause an error if unresolved, but
there is a syntax `${?a.b}` to permit them to be missing.
### Examples of HOCON
@ -233,6 +235,10 @@ Here are some features that might be nice to add.
in system properties and the environment, for example).
This could be done using the same syntax as `include`,
potentially. It is not a backward-compatible change though.
- substitutions with fallbacks; this could be something like
`${foo.bar,baz,null}` where it would look up `foo.bar`, then
`baz`, then finally fall back to null. One question is whether
entire nested objects would be allowed as fallbacks.
## Rationale

View File

@ -98,19 +98,19 @@ public interface Config extends ConfigMergeable {
* <code>Config</code> as the root object, that is, a substitution
* <code>${foo.bar}</code> will be replaced with the result of
* <code>getValue("foo.bar")</code>.
*
*
* <p>
* This method uses {@link ConfigResolveOptions#defaults()}, there is
* another variant {@link Config#resolve(ConfigResolveOptions)} which lets
* you specify non-default options.
*
*
* <p>
* A given {@link Config} must be resolved before using it to retrieve
* config values, but ideally should be resolved one time for your entire
* stack of fallbacks (see {@link Config#withFallback}). Otherwise, some
* substitutions that could have resolved with all fallbacks available may
* not resolve, which will be a user-visible oddity.
*
*
* <p>
* <code>resolve()</code> should be invoked on root config objects, rather
* than on a subtree (a subtree is the result of something like
@ -120,15 +120,19 @@ public interface Config extends ConfigMergeable {
* from the root. For example, if you did
* <code>config.getConfig("foo").resolve()</code> on the below config file,
* it would not work:
*
*
* <pre>
* common-value = 10
* foo {
* whatever = ${common-value}
* }
* </pre>
*
*
* @return an immutable object with substitutions resolved
* @throws ConfigException.UnresolvedSubstitution
* if any substitutions refer to nonexistent paths
* @throws ConfigException
* some other config exception if there are other problems
*/
Config resolve();

View File

@ -36,7 +36,7 @@ public class ConfigException extends RuntimeException {
* for a given exception, or the kind of exception doesn't meaningfully
* relate to a particular origin file, this returns null. Never assume this
* will return non-null, it can always return null.
*
*
* @return origin of the problem, or null if unknown/inapplicable
*/
public ConfigOrigin origin() {
@ -229,12 +229,29 @@ public class ConfigException extends RuntimeException {
}
}
/**
* Exception indicating that a substitution did not resolve to anything.
* Thrown by {@link Config#resolve}.
*/
public static class UnresolvedSubstitution extends Parse {
private static final long serialVersionUID = 1L;
public UnresolvedSubstitution(ConfigOrigin origin, String expression, Throwable cause) {
super(origin, "Could not resolve substitution to a value: " + expression, cause);
}
public UnresolvedSubstitution(ConfigOrigin origin, String expression) {
this(origin, expression, null);
}
}
/**
* Exception indicating that you tried to use a function that requires
* substitutions to be resolved, but substitutions have not been resolved.
* This is always a bug in either application code or the library; it's
* wrong to write a handler for this exception because you should be able to
* fix the code to avoid it.
* substitutions to be resolved, but substitutions have not been resolved
* (that is, {@link Config#resolve} was not called). This is always a bug in
* either application code or the library; it's wrong to write a handler for
* this exception because you should be able to fix the code to avoid it by
* adding calls to {@link Config#resolve}.
*/
public static class NotResolved extends BugOrBroken {
private static final long serialVersionUID = 1L;

View File

@ -22,8 +22,8 @@ import com.typesafe.config.ConfigValueType;
final class ConfigSubstitution extends AbstractConfigValue implements
Unmergeable {
// this is a list of String and Path where the Path
// have to be resolved to values, then if there's more
// this is a list of String and SubstitutionExpression where the
// SubstitutionExpression has to be resolved to values, then if there's more
// than one piece everything is stringified and concatenated
final private List<Object> pieces;
// the length of any prefixes added with relativized()
@ -40,6 +40,10 @@ final class ConfigSubstitution extends AbstractConfigValue implements
this.pieces = pieces;
this.prefixLength = prefixLength;
this.ignoresFallbacks = ignoresFallbacks;
for (Object p : pieces) {
if (p instanceof Path)
throw new RuntimeException("broken here");
}
}
@Override
@ -125,15 +129,15 @@ final class ConfigSubstitution extends AbstractConfigValue implements
return result;
}
private ConfigValue resolve(SubstitutionResolver resolver, Path subst,
private ConfigValue resolve(SubstitutionResolver resolver, SubstitutionExpression subst,
int depth, ConfigResolveOptions options) {
ConfigValue result = findInObject(resolver.root(), resolver, subst,
ConfigValue result = findInObject(resolver.root(), resolver, subst.path(),
depth, options);
// when looking up system props and env variables,
// we don't want the prefix that was added when
// we were included in another file.
Path unprefixed = subst.subPath(prefixLength);
Path unprefixed = subst.path().subPath(prefixLength);
if (result == null && options.getUseSystemProperties()) {
result = findInObject(ConfigImpl.systemPropertiesAsConfigObject(), null,
@ -145,10 +149,6 @@ final class ConfigSubstitution extends AbstractConfigValue implements
unprefixed, depth, options);
}
if (result == null) {
result = new ConfigNull(origin());
}
return result;
}
@ -161,28 +161,46 @@ final class ConfigSubstitution extends AbstractConfigValue implements
if (p instanceof String) {
sb.append((String) p);
} else {
ConfigValue v = resolve(resolver, (Path) p, depth, options);
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(),
((Path) p).render(),
"not a list or object", v.valueType().name());
default:
sb.append(((AbstractConfigValue) v).transformToString());
SubstitutionExpression exp = (SubstitutionExpression) p;
ConfigValue v = resolve(resolver, exp, depth, options);
if (v == null) {
if (exp.optional()) {
// append nothing to StringBuilder
} else {
throw new ConfigException.UnresolvedSubstitution(origin(),
exp.toString());
}
} else {
switch (v.valueType()) {
case LIST:
case OBJECT:
// cannot substitute lists and objects into strings
throw new ConfigException.WrongType(v.origin(), exp.path().render(),
"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 Path))
if (!(pieces.get(0) instanceof SubstitutionExpression))
throw new ConfigException.BugOrBroken(
"ConfigSubstitution should never contain a single String piece");
return resolve(resolver, (Path) pieces.get(0), depth, options);
SubstitutionExpression exp = (SubstitutionExpression) pieces.get(0);
ConfigValue v = resolve(resolver, exp, depth, options);
if (v == null) {
if (exp.optional()) {
// FIXME want to delete the field or array element here
// instead of this
v = new ConfigNull(origin());
} else {
throw new ConfigException.UnresolvedSubstitution(origin(), exp.toString());
}
}
return v;
}
}
@ -211,8 +229,10 @@ final class ConfigSubstitution extends AbstractConfigValue implements
ConfigSubstitution relativized(Path prefix) {
List<Object> newPieces = new ArrayList<Object>();
for (Object p : pieces) {
if (p instanceof Path) {
newPieces.add(((Path) p).prepend(prefix));
if (p instanceof SubstitutionExpression) {
SubstitutionExpression exp = (SubstitutionExpression) p;
newPieces.add(exp.changePath(exp.path().prepend(prefix)));
} else {
newPieces.add(p);
}
@ -246,10 +266,8 @@ final class ConfigSubstitution extends AbstractConfigValue implements
@Override
protected void render(StringBuilder sb, int indent, boolean formatted) {
for (Object p : pieces) {
if (p instanceof Path) {
sb.append("${");
sb.append(((Path) p).render());
sb.append("}");
if (p instanceof SubstitutionExpression) {
sb.append(p.toString());
} else {
sb.append(ConfigUtil.renderJsonString((String) p));
}

View File

@ -155,7 +155,7 @@ final class Parser {
return;
}
// this will be a list of String and Path
// this will be a list of String and SubstitutionExpression
List<Object> minimized = new ArrayList<Object>();
// we have multiple value tokens or one unquoted text token;
@ -187,7 +187,9 @@ final class Parser {
.getSubstitutionPathExpression(valueToken);
Path path = parsePathExpression(expression.iterator(),
Tokens.getSubstitutionOrigin(valueToken));
minimized.add(path);
boolean optional = Tokens.getSubstitutionOptional(valueToken);
minimized.add(new SubstitutionExpression(path, optional));
} else {
throw new ConfigException.BugOrBroken(
"should not be trying to consolidate token: "

View File

@ -0,0 +1,46 @@
package com.typesafe.config.impl;
final class SubstitutionExpression {
final private Path path;
final private boolean optional;
SubstitutionExpression(Path path, boolean optional) {
this.path = path;
this.optional = optional;
}
Path path() {
return path;
}
boolean optional() {
return optional;
}
SubstitutionExpression changePath(Path newPath) {
return new SubstitutionExpression(newPath, optional);
}
@Override
public String toString() {
return "${" + (optional ? "?" : "") + path.render() + "}";
}
@Override
public boolean equals(Object other) {
if (other instanceof SubstitutionExpression) {
SubstitutionExpression otherExp = (SubstitutionExpression) other;
return otherExp.path.equals(this.path) && otherExp.optional == this.optional;
} else {
return false;
}
}
@Override
public int hashCode() {
int h = 41 * (41 + path.hashCode());
h = 41 * (h + (optional ? 1 : 0));
return h;
}
}

View File

@ -403,6 +403,14 @@ final class Tokenizer {
throw parseError("'$' not followed by {");
}
boolean optional = false;
c = nextCharSkippingComments();
if (c == '?') {
optional = true;
} else {
putBack(c);
}
WhitespaceSaver saver = new WhitespaceSaver();
List<Token> expression = new ArrayList<Token>();
@ -427,7 +435,7 @@ final class Tokenizer {
}
} while (true);
return Tokens.newSubstitution(origin, expression);
return Tokens.newSubstitution(origin, optional, expression);
}
private Token pullNextToken(WhitespaceSaver saver) {

View File

@ -125,11 +125,13 @@ final class Tokens {
// This is not a Value, because it requires special processing
static private class Substitution extends Token {
final private ConfigOrigin origin;
final private boolean optional;
final private List<Token> value;
Substitution(ConfigOrigin origin, List<Token> expression) {
Substitution(ConfigOrigin origin, boolean optional, List<Token> expression) {
super(TokenType.SUBSTITUTION);
this.origin = origin;
this.optional = optional;
this.value = expression;
}
@ -137,6 +139,10 @@ final class Tokens {
return origin;
}
boolean optional() {
return optional;
}
List<Token> value() {
return value;
}
@ -237,6 +243,15 @@ final class Tokens {
}
}
static boolean getSubstitutionOptional(Token token) {
if (token instanceof Substitution) {
return ((Substitution) token).optional();
} else {
throw new ConfigException.BugOrBroken("tried to get substitution optionality from "
+ token);
}
}
final static Token START = new Token(TokenType.START);
final static Token END = new Token(TokenType.END);
final static Token COMMA = new Token(TokenType.COMMA);
@ -255,8 +270,8 @@ final class Tokens {
return new UnquotedText(origin, s);
}
static Token newSubstitution(ConfigOrigin origin, List<Token> expression) {
return new Substitution(origin, expression);
static Token newSubstitution(ConfigOrigin origin, boolean optional, List<Token> expression) {
return new Substitution(origin, optional, expression);
}
static Token newValue(AbstractConfigValue value) {

View File

@ -0,0 +1,4 @@
a=${?NOT_DEFINED_ANYWHERE}
b=${?also.not.defined.anywhere}
c=${?a}

View File

@ -0,0 +1,6 @@
{
"a" : null,
"b" : null,
"c" : null
}

View File

@ -65,14 +65,14 @@
},
"system" : {
"javaversion" : ${java.version},
"userhome" : ${user.home},
"home" : ${HOME},
"pwd" : ${PWD},
"shell" : ${SHELL},
"lang" : ${LANG},
"path" : ${PATH},
"not_here" : ${NOT_HERE},
"concatenated" : Your Java version is ${system.javaversion} and your user.home is ${system.userhome}
"javaversion" : ${?java.version},
"userhome" : ${?user.home},
"home" : ${?HOME},
"pwd" : ${?PWD},
"shell" : ${?SHELL},
"lang" : ${?LANG},
"path" : ${?PATH},
"not_here" : ${?NOT_HERE},
"concatenated" : Your Java version is ${?system.javaversion} and your user.home is ${?system.userhome}
}
}

View File

@ -27,10 +27,10 @@ application.secret=s1kwayg211q9v4387pvarbmyqnht7hrl54d34lsz0yh9btb117br293a25trz
# the application root.
# Import the crud module
module.crud=${play.path}/modules/crud
module.crud=${?play.path}/modules/crud
# Import the secure module
module.secure=${play.path}/modules/secure
module.secure=${?play.path}/modules/secure
# Import the cobertura module in test mode
#%test.module.cobertura=${play.path}/modules/cobertura

View File

@ -79,7 +79,7 @@ class ConfParserTest extends TestUtils {
list.get(0) match {
case subst: ConfigSubstitution =>
subst.pieces().get(0) match {
case p: Path => p
case exp: SubstitutionExpression => exp.path()
}
}
}

View File

@ -95,6 +95,14 @@ class ConfigSubstitutionTest extends TestUtils {
assertEquals(doubleValue(3.14), v)
}
@Test
def resolveMissingThrows() {
intercept[ConfigException.UnresolvedSubstitution] {
val s = subst("bar.missing")
val v = resolveWithoutFallbacks(s, simpleObject)
}
}
@Test
def resolveIntInString() {
val s = substInString("bar.int")
@ -106,17 +114,16 @@ class ConfigSubstitutionTest extends TestUtils {
def resolveNullInString() {
val s = substInString("bar.null")
val v = resolveWithoutFallbacks(s, simpleObject)
// null is supposed to become empty string
assertEquals(stringValue("start<>end"), v)
assertEquals(stringValue("start<null>end"), v)
// but when null is NOT a subst, it should not become empty, incidentally
// when null is NOT a subst, it should also not become empty
val o = parseConfig("""{ "a" : null foo bar }""")
assertEquals("null foo bar", o.getString("a"))
}
@Test
def resolveMissingInString() {
val s = substInString("bar.missing")
val s = substInString("bar.missing", true /* optional */ )
val v = resolveWithoutFallbacks(s, simpleObject)
// absent object becomes empty string
assertEquals(stringValue("start<>end"), v)
@ -268,12 +275,12 @@ class ConfigSubstitutionTest extends TestUtils {
private val substEnvVarObject = {
parseObject("""
{
"home" : ${HOME},
"pwd" : ${PWD},
"shell" : ${SHELL},
"lang" : ${LANG},
"path" : ${PATH},
"not_here" : ${NOT_HERE}
"home" : ${?HOME},
"pwd" : ${?PWD},
"shell" : ${?SHELL},
"lang" : ${?LANG},
"path" : ${?PATH},
"not_here" : ${?NOT_HERE}
}
""")
}
@ -345,4 +352,12 @@ class ConfigSubstitutionTest extends TestUtils {
throw new Exception("None of the env vars we tried to use for testing were set")
}
}
@Test
def throwWhenEnvNotFound() {
val obj = parseObject("""{ a : ${NOT_HERE} }""")
intercept[ConfigException.UnresolvedSubstitution] {
resolve(obj)
}
}
}

View File

@ -90,8 +90,8 @@ 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(3, dirCount)
assertEquals(4, dirCount)
// this is the number of files not named original.*
assertEquals(13, fileCount)
assertEquals(14, fileCount)
}
}

View File

@ -355,17 +355,27 @@ abstract trait TestUtils {
ConfigFactory.parseString(s, options).asInstanceOf[SimpleConfig]
}
protected def subst(ref: String) = {
val pieces = java.util.Collections.singletonList[Object](Path.newPath(ref))
protected def subst(ref: String, optional: Boolean): ConfigSubstitution = {
val path = Path.newPath(ref)
val pieces = java.util.Collections.singletonList[Object](new SubstitutionExpression(path, optional))
new ConfigSubstitution(fakeOrigin(), pieces)
}
protected def substInString(ref: String) = {
protected def subst(ref: String): ConfigSubstitution = {
subst(ref, false)
}
protected def substInString(ref: String, optional: Boolean): ConfigSubstitution = {
import scala.collection.JavaConverters._
val pieces = List("start<", Path.newPath(ref), ">end")
val path = Path.newPath(ref)
val pieces = List("start<", new SubstitutionExpression(path, optional), ">end")
new ConfigSubstitution(fakeOrigin(), pieces.asJava)
}
protected def substInString(ref: String): ConfigSubstitution = {
substInString(ref, false)
}
def tokenTrue = Tokens.newBoolean(fakeOrigin(), true)
def tokenFalse = Tokens.newBoolean(fakeOrigin(), false)
def tokenNull = Tokens.newNull(fakeOrigin())
@ -375,12 +385,20 @@ abstract trait TestUtils {
def tokenInt(i: Int) = Tokens.newInt(fakeOrigin(), i, null)
def tokenLong(l: Long) = Tokens.newLong(fakeOrigin(), l, null)
def tokenSubstitution(expression: Token*) = {
private def tokenMaybeOptionalSubstitution(optional: Boolean, expression: Token*) = {
val l = new java.util.ArrayList[Token]
for (t <- expression) {
l.add(t);
}
Tokens.newSubstitution(fakeOrigin(), l);
Tokens.newSubstitution(fakeOrigin(), optional, l);
}
def tokenSubstitution(expression: Token*) = {
tokenMaybeOptionalSubstitution(false, expression: _*)
}
def tokenOptionalSubstitution(expression: Token*) = {
tokenMaybeOptionalSubstitution(true, expression: _*)
}
// quoted string substitution (no interpretation of periods)

View File

@ -36,8 +36,9 @@ class TokenizerTest extends TestUtils {
Tokens.OPEN_CURLY, Tokens.CLOSE_SQUARE, Tokens.OPEN_SQUARE, tokenString("foo"),
tokenTrue, tokenDouble(3.14), tokenFalse,
tokenLong(42), tokenNull, tokenSubstitution(tokenUnquoted("a.b")),
tokenOptionalSubstitution(tokenUnquoted("x.y")),
tokenKeySubstitution("c.d"), Tokens.newLine(1), Tokens.END)
assertEquals(expected, tokenizeAsList(""",:=}{]["foo"true3.14false42null${a.b}${"c.d"}""" + "\n"))
assertEquals(expected, tokenizeAsList(""",:=}{]["foo"true3.14false42null${a.b}${?x.y}${"c.d"}""" + "\n"))
}
@Test
@ -46,9 +47,11 @@ class TokenizerTest extends TestUtils {
Tokens.OPEN_CURLY, Tokens.CLOSE_SQUARE, Tokens.OPEN_SQUARE, tokenString("foo"),
tokenUnquoted(" "), tokenLong(42), tokenUnquoted(" "), tokenTrue, tokenUnquoted(" "),
tokenDouble(3.14), tokenUnquoted(" "), tokenFalse, tokenUnquoted(" "), tokenNull,
tokenUnquoted(" "), tokenSubstitution(tokenUnquoted("a.b")), tokenUnquoted(" "), tokenKeySubstitution("c.d"),
tokenUnquoted(" "), tokenSubstitution(tokenUnquoted("a.b")), tokenUnquoted(" "),
tokenOptionalSubstitution(tokenUnquoted("x.y")), tokenUnquoted(" "),
tokenKeySubstitution("c.d"),
Tokens.newLine(1), Tokens.END)
assertEquals(expected, tokenizeAsList(""" , : = } { ] [ "foo" 42 true 3.14 false null ${a.b} ${"c.d"} """ + "\n "))
assertEquals(expected, tokenizeAsList(""" , : = } { ] [ "foo" 42 true 3.14 false null ${a.b} ${?x.y} ${"c.d"} """ + "\n "))
}
@Test
@ -58,9 +61,10 @@ class TokenizerTest extends TestUtils {
tokenUnquoted(" "), tokenLong(42), tokenUnquoted(" "), tokenTrue, tokenUnquoted(" "),
tokenDouble(3.14), tokenUnquoted(" "), tokenFalse, tokenUnquoted(" "), tokenNull,
tokenUnquoted(" "), tokenSubstitution(tokenUnquoted("a.b")), tokenUnquoted(" "),
tokenOptionalSubstitution(tokenUnquoted("x.y")), tokenUnquoted(" "),
tokenKeySubstitution("c.d"),
Tokens.newLine(1), Tokens.END)
assertEquals(expected, tokenizeAsList(""" , : = } { ] [ "foo" 42 true 3.14 false null ${a.b} ${"c.d"} """ + "\n "))
assertEquals(expected, tokenizeAsList(""" , : = } { ] [ "foo" 42 true 3.14 false null ${a.b} ${?x.y} ${"c.d"} """ + "\n "))
}
@Test