Allow application.conf to override variables in reference.conf (#619)

* Allow application.conf to override variables in reference.conf

Fixes #167

This only affects the output of `ConfigFactory.load`. It does not change
`ConfigFactory.defaultReference`. This uses the unresolved
`reference.conf` in the building of configuration in
`ConfigFactory.load`, effectively allowing `application.conf` properties
to override variable substitutions in `reference.conf`.

However, it still requires `reference.conf` to be fully resolvable, if
it isn't, an exception will be thrown. So two resolves are still done
during load, it's just that the output of the resolve of
`reference.conf` isn't used in building the final configuration. The
documentation has been updated to reflect this behavior.

The reasoning behind this change can be read about in #167, but
essentially, it is not uncommon for configuration properties to depend
on each other by default, a good example of this is directory
hierarchies, where you might have a configuration option for a base
directory, and then a configuration for the log directory that by
default is under the base directory, and within that a configuration for
individual log files which by default are under the log directory.
Without allowing variable substitutions in `reference.conf` from
`application.conf`, there is no point in defining a configuration option
for the base directory since changing it won't have any impact, and each
path defined that depends on it will have to be manually overridden.
This limitation is contrary to convention over configuration best
practices, and hence not desirable in a configuration library.

* Renamed public method to defaultReferenceUnresolved

Also added the methods to ConfigFactory, as requested in code review.

README.md

@@ -71,7 +71,6 @@ to merge it in.
     - [Inheritance](#inheritance)
     - [Optional system or env variable overrides](#optional-system-or-env-variable-overrides)
   - [Concatenation](#concatenation)
-  - [`reference.conf` can't refer to `application.conf`](#referenceconf-cant-refer-to-applicationconf)
 - [Miscellaneous Notes](#miscellaneous-notes)
   - [Debugging Your Configuration](#debugging-your-configuration)
   - [Supports Java 8 and Later](#supports-java-8-and-later)
@@ -277,23 +276,14 @@ system properties.
 The substitution syntax `${foo.bar}` will be resolved
 twice. First, all the `reference.conf` files are merged and then
 the result gets resolved. Second, all the `application.conf` are
-layered over the `reference.conf` and the result of that gets
-resolved again.
+layered over the unresolved `reference.conf` and the result of that
+gets resolved again.
 
 The implication of this is that the `reference.conf` stack has to
 be self-contained; you can't leave an undefined value `${foo.bar}`
-to be provided by `application.conf`, or refer to `${foo.bar}` in
-a way that you want to allow `application.conf` to
-override. However, `application.conf` can refer to a `${foo.bar}`
-in `reference.conf`.
-
-This can be frustrating at times, but possible workarounds
-include:
-
-  * putting an `application.conf` in a library jar, alongside the
-`reference.conf`, with values intended for later resolution.
-  * putting some logic in code instead of building up values in the
-    config itself.
+to be provided by `application.conf`. It is however possible to
+override a variable that `reference.conf` refers to, as long as
+`reference.conf` also defines that variable itself.
 
 ### Merging config trees
 
@@ -758,14 +748,6 @@ concatenation, but not object/array concatenation. `+=` does not
 work in Play/Akka 2.0 either. Post-2.0 versions support these
 features.
 
-### `reference.conf` can't refer to `application.conf`
-
-Please see <a
-href="#note-about-resolving-substitutions-in-referenceconf-and-applicationconf">this
-earlier section</a>; all `reference.conf` have substitutions
-resolved first, without `application.conf` in the stack, so the
-reference stack has to be self-contained.
-
 ## Miscellaneous Notes
 
 ### Debugging Your Configuration

config/src/main/java/com/typesafe/config/ConfigException.java

@@ -284,13 +284,25 @@ public Parse(ConfigOrigin origin, String message) {
     public static class UnresolvedSubstitution extends Parse {
         private static final long serialVersionUID = 1L;
 
+        private final String detail;
+
         public UnresolvedSubstitution(ConfigOrigin origin, String detail, Throwable cause) {
             super(origin, "Could not resolve substitution to a value: " + detail, cause);
+            this.detail = detail;
         }
 
         public UnresolvedSubstitution(ConfigOrigin origin, String detail) {
             this(origin, detail, null);
         }
+
+        private UnresolvedSubstitution(UnresolvedSubstitution wrapped, ConfigOrigin origin, String message) {
+            super(origin, message, wrapped);
+            this.detail = wrapped.detail;
+        }
+
+        public UnresolvedSubstitution addExtraDetail(String extra) {
+            return new UnresolvedSubstitution(this, origin(), String.format(extra, detail));
+        }
     }
 
     /**

config/src/main/java/com/typesafe/config/ConfigFactory.java

@@ -211,7 +211,8 @@ public static Config load(Config config, ConfigResolveOptions resolveOptions) {
      * @return resolved configuration with overrides and fallbacks added
      */
     public static Config load(ClassLoader loader, Config config, ConfigResolveOptions resolveOptions) {
-        return defaultOverrides(loader).withFallback(config).withFallback(defaultReference(loader))
+        return defaultOverrides(loader).withFallback(config)
+                .withFallback(ConfigImpl.defaultReferenceUnresolved(loader))
                 .resolve(resolveOptions);
     }
 
@@ -368,6 +369,56 @@ public static Config defaultReference(ClassLoader loader) {
         return ConfigImpl.defaultReference(loader);
     }
 
+    /**
+     * Obtains the default reference configuration, which is currently created
+     * by merging all resources "reference.conf" found on the classpath and
+     * overriding the result with system properties.
+     *
+     * <p>
+     * While the returned reference configuration is guaranteed to be
+     * resolvable (that is, there will be no substitutions that cannot be
+     * resolved), it is returned in an unresolved state for the purpose of
+     * allowing substitutions to be overridden by a config layer that falls
+     * back to this one.
+     *
+     * <p>
+     * Libraries and frameworks should ship with a "reference.conf" in their
+     * jar.
+     *
+     * <p>
+     * The reference config must be looked up in the class loader that contains
+     * the libraries that you want to use with this config, so the
+     * "reference.conf" for each library can be found. Use
+     * {@link #defaultReference(ClassLoader)} if the context class loader is not
+     * suitable.
+     *
+     * <p>
+     * The {@link #load()} methods merge this configuration for you
+     * automatically.
+     *
+     * <p>
+     * Future versions may look for reference configuration in more places. It
+     * is not guaranteed that this method <em>only</em> looks at
+     * "reference.conf".
+     *
+     * @return the unresolved default reference config for the context class
+     *         loader
+     */
+    public static Config defaultReferenceUnresolved() {
+        return defaultReferenceUnresolved(checkedContextClassLoader("defaultReferenceUnresolved"));
+    }
+
+    /**
+     * Like {@link #defaultReferenceUnresolved()} but allows you to specify a
+     * class loader to use rather than the current context class loader.
+     *
+     * @param loader class loader to look for resources in
+     * @return the unresolved default reference config for this class loader
+     */
+    public static Config defaultReferenceUnresolved(ClassLoader loader) {
+        return ConfigImpl.defaultReferenceUnresolved(loader);
+    }
+
     /**
      * Obtains the default override configuration, which currently consists of
      * system properties. The returned override configuration will already have

config/src/main/java/com/typesafe/config/impl/ConfigImpl.java

@@ -408,15 +408,41 @@ public static Config defaultReference(final ClassLoader loader) {
         return computeCachedConfig(loader, "defaultReference", new Callable<Config>() {
             @Override
             public Config call() {
-                Config unresolvedResources = Parseable
-                        .newResources("reference.conf",
-                                ConfigParseOptions.defaults().setClassLoader(loader))
-                        .parse().toConfig();
+                Config unresolvedResources = unresolvedReference(loader);
                 return systemPropertiesAsConfig().withFallback(unresolvedResources).resolve();
             }
         });
     }
 
+    private static Config unresolvedReference(final ClassLoader loader) {
+        return computeCachedConfig(loader, "unresolvedReference", new Callable<Config>() {
+            @Override
+            public Config call() {
+                return Parseable.newResources("reference.conf",
+                        ConfigParseOptions.defaults().setClassLoader(loader))
+                    .parse().toConfig();
+            }
+        });
+    }
+
+    /**
+     * This returns the unresolved reference configuration, but before doing so,
+     * it verifies that the reference configuration resolves, to ensure that it
+     * is self contained and doesn't depend on any higher level configuration
+     * files.
+     */
+    public static Config defaultReferenceUnresolved(final ClassLoader loader) {
+        // First, verify that `reference.conf` resolves by itself.
+        try {
+            defaultReference(loader);
+        } catch (ConfigException.UnresolvedSubstitution e) {
+            throw e.addExtraDetail("Could not resolve substitution in reference.conf to a value: %s. All reference.conf files are required to be fully, independently resolvable, and should not require the presence of values for substitutions from further up the hierarchy.");
+        }
+        // Now load the unresolved version
+        return unresolvedReference(loader);
+    }
+
+
     private static class DebugHolder {
         private static String LOADS = "loads";
         private static String SUBSTITUTIONS = "substitutions";

config/src/test/resources/test13-application-override-substitutions.conf

@@ -0,0 +1 @@
+b = "overridden"

config/src/test/resources/test13-reference-bad-substitutions.conf

@@ -0,0 +1 @@
+a = ${b}

config/src/test/resources/test13-reference-with-substitutions.conf

@@ -0,0 +1,2 @@
+a = ${b}
+b = "b"

config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala

@@ -1152,6 +1152,28 @@ include "onclasspath"
         // missing underneath missing
         intercept[ConfigException.Missing] { conf.getIsNull("x.c.y") }
     }
+
+    @Test
+    def applicationConfCanOverrideReferenceConf(): Unit = {
+        val loader = new TestClassLoader(this.getClass.getClassLoader,
+            Map(
+                "reference.conf" -> resourceFile("test13-reference-with-substitutions.conf").toURI.toURL,
+                "application.conf" -> resourceFile("test13-application-override-substitutions.conf").toURI.toURL))
+
+        assertEquals("b", ConfigFactory.defaultReference(loader).getString("a"))
+        assertEquals("overridden", ConfigFactory.load(loader).getString("a"))
+    }
+
+    @Test(expected = classOf[ConfigException.UnresolvedSubstitution])
+    def referenceConfMustResolveIndependently(): Unit = {
+        val loader = new TestClassLoader(this.getClass.getClassLoader,
+            Map(
+                "reference.conf" -> resourceFile("test13-reference-bad-substitutions.conf").toURI.toURL,
+                "application.conf" -> resourceFile("test13-application-override-substitutions.conf").toURI.toURL))
+
+        ConfigFactory.load(loader)
+    }
+
 }
 
 class TestStrategy extends DefaultConfigLoadingStrategy {