diff --git a/HOCON.md b/HOCON.md index ed8beee6..551879f2 100644 --- a/HOCON.md +++ b/HOCON.md @@ -561,6 +561,10 @@ include statement in the including file. #### Include semantics: substitution +Substitutions in included files are looked up at two different +paths; first, relative to the root of the included file; second, +relative to the root of the including configuration. + Recall that substitution happens as a final step, _after_ parsing. It should be done for the entire app's configuration, not for single files in isolation. @@ -592,6 +596,12 @@ Then the `${x}` in "foo.conf", which has been fixed up to `${a.x}`, would evaluate to `42` rather than to `10`. Substitution happens _after_ parsing the whole configuration. +However, there are plenty of cases where the included file might +intend to refer to the application's root config. For example, to +get a value from a system property or from the reference +configuration. So it's not enough to only look up the "fixed up" +path, it's necessary to look up the original path as well. + #### Include semantics: missing files If an included file does not exist, the include statement should diff --git a/README.md b/README.md index d18df257..365ff423 100644 --- a/README.md +++ b/README.md @@ -267,21 +267,6 @@ Here are some features that might be nice to add. entire nested objects would be allowed as fallbacks. This feature may not really be needed because you can just list the key multiple times instead: `a=null,a=${?baz},a=${?foo.bar}` - - improve handling of substitutions in includes. Right now if you - have a parent file `a : { include "child.conf" }` and - `child.conf` contains a substitution written `${b}`, then - `${b}` ends up referring to `${a.b}` (that is, its parent key - is prepended to it). The downside of this is that there's no - way to refer to toplevel settings from inside `child.conf`, and - system properties must be special-case fallbacks for - substitutions to allow them to be used from `child.conf`. If - `${b}` in an include always referred to the root `${b}` then - substitutions would not need any special system property - awareness, but there would be no way to write a file to be - included in another file without knowing the parent key the - file would be included into. The solution probably involves - special syntax for one of "root-relative" or "file-relative", - with the regular syntax meaning one or the other. ## Rationale diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java b/config/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java index 1b274086..390ec893 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java @@ -131,22 +131,30 @@ final class ConfigSubstitution extends AbstractConfigValue implements private ConfigValue resolve(SubstitutionResolver resolver, SubstitutionExpression subst, int depth, ConfigResolveOptions options) { + // First we look up the full path, which means relative to the + // included file if we were not a root file 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.path().subPath(prefixLength); + if (result == null) { + // Then we want to check relative to the root file. We don't + // want the prefix we were included at to be used when looking up + // env variables either. + Path unprefixed = subst.path().subPath(prefixLength); - if (result == null && options.getUseSystemProperties()) { - result = findInObject(ConfigImpl.systemPropertiesAsConfigObject(), null, - unprefixed, depth, options); - } + if (result == null && prefixLength > 0) { + result = findInObject(resolver.root(), resolver, unprefixed, depth, options); + } - if (result == null && options.getUseSystemEnvironment()) { - result = findInObject(ConfigImpl.envVariablesAsConfigObject(), null, - unprefixed, depth, options); + if (result == null && options.getUseSystemProperties()) { + result = findInObject(ConfigImpl.systemPropertiesAsConfigObject(), null, + unprefixed, depth, options); + } + + if (result == null && options.getUseSystemEnvironment()) { + result = findInObject(ConfigImpl.envVariablesAsConfigObject(), null, unprefixed, + depth, options); + } } return result; diff --git a/config/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala b/config/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala index 984a3679..f65670cc 100644 --- a/config/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala @@ -249,6 +249,37 @@ class ConfigSubstitutionTest extends TestUtils { assertEquals(42, resolved.getInt("a.cycle")) } + @Test + def useRelativeToSameFileWhenRelativized() { + val child = parseObject("""foo=in child,bar=${foo}""") + + val values = new java.util.HashMap[String, AbstractConfigValue]() + + values.put("a", child.relativized(new Path("a"))) + // this "foo" should NOT be used. + values.put("foo", stringValue("in parent")); + + val resolved = resolve(new SimpleConfigObject(fakeOrigin(), values)); + + assertEquals("in child", resolved.getString("a.bar")) + } + + @Test + def useRelativeToRootWhenRelativized() { + // here, "foo" is not defined in the child + val child = parseObject("""bar=${foo}""") + + val values = new java.util.HashMap[String, AbstractConfigValue]() + + values.put("a", child.relativized(new Path("a"))) + // so this "foo" SHOULD be used + values.put("foo", stringValue("in parent")); + + val resolved = resolve(new SimpleConfigObject(fakeOrigin(), values)); + + assertEquals("in parent", resolved.getString("a.bar")) + } + private val substComplexObject = { parseObject(""" {