allow setting a ClassLoader on ConfigParseOptions

This should have been the API, rather than adding a loader
parameter to ConfigFactory methods.

By adding to the options, the class loader is inherited by
any included files or urls. Previously, it was only inherited
by included classpath resources, but including classpath resources
from non-classpath resources would lose track of the class loader.

The methods that take a ClassLoader are now convenience API that
just adds that passed-in loader to your ConfigParseOptions.

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

@@ -58,7 +58,8 @@ public final class ConfigFactory {
      * @return configuration for an application relative to context class loader
      */
     public static Config load(String resourceBasename) {
-        return load(Thread.currentThread().getContextClassLoader(), resourceBasename);
+        return load(resourceBasename, ConfigParseOptions.defaults(),
+                ConfigResolveOptions.defaults());
     }
 
     /**
@@ -70,7 +71,7 @@ public final class ConfigFactory {
      * @return configuration for an application relative to given class loader
      */
     public static Config load(ClassLoader loader, String resourceBasename) {
-        return load(loader, resourceBasename, ConfigParseOptions.defaults(),
+        return load(resourceBasename, ConfigParseOptions.defaults().setClassLoader(loader),
                 ConfigResolveOptions.defaults());
     }
 
@@ -88,29 +89,30 @@ public final class ConfigFactory {
      */
     public static Config load(String resourceBasename, ConfigParseOptions parseOptions,
             ConfigResolveOptions resolveOptions) {
-        return load(Thread.currentThread().getContextClassLoader(), resourceBasename, parseOptions,
-                resolveOptions);
+        Config appConfig = ConfigFactory.parseResourcesAnySyntax(resourceBasename, parseOptions);
+        return load(parseOptions.getClassLoader(), appConfig, resolveOptions);
     }
 
     /**
      * Like {@link #load(String,ConfigParseOptions,ConfigResolveOptions)} but
-     * allows you to specify a class loader
+     * has a class loader parameter that overrides any from the
+     * {@code ConfigParseOptions}.
      *
      * @param loader
-     *            class loader in which to find resources
+     *            class loader in which to find resources (overrides loader in
+     *            parse options)
      * @param resourceBasename
      *            the classpath resource name with optional extension
      * @param parseOptions
-     *            options to use when parsing the resource
+     *            options to use when parsing the resource (class loader
+     *            overridden)
      * @param resolveOptions
      *            options to use when resolving the stack
      * @return configuration for an application
      */
     public static Config load(ClassLoader loader, String resourceBasename,
             ConfigParseOptions parseOptions, ConfigResolveOptions resolveOptions) {
-        Config appConfig = ConfigFactory.parseResourcesAnySyntax(loader, resourceBasename,
-                parseOptions);
-        return load(loader, appConfig, resolveOptions);
+        return load(resourceBasename, parseOptions.setClassLoader(loader), resolveOptions);
     }
 
     /**
@@ -192,10 +194,16 @@ public final class ConfigFactory {
                 // people want that they can use an include statement.
                 return load(loader, parseResources(loader, resource));
             } else if (file != null) {
-                return load(loader, parseFile(new File(file)));
+                return load(
+                        loader,
+                        parseFile(new File(file),
+                                ConfigParseOptions.defaults().setClassLoader(loader)));
             } else {
                 try {
-                    return load(loader, parseURL(new URL(url)));
+                    return load(
+                            loader,
+                            parseURL(new URL(url),
+                                    ConfigParseOptions.defaults().setClassLoader(loader)));
                 } catch (MalformedURLException e) {
                     throw new ConfigException.Generic("Bad URL in config.url system property: '"
                             + url + "': " + e.getMessage(), e);
@@ -550,7 +558,8 @@ public final class ConfigFactory {
      *            a resource name as in {@link java.lang.Class#getResource},
      *            with or without extension
      * @param options
-     *            parse options
+     *            parse options (class loader is ignored in favor of the one
+     *            from klass)
      * @return the parsed configuration
      */
     public static Config parseResourcesAnySyntax(Class<?> klass, String resourceBasename,
@@ -577,16 +586,17 @@ public final class ConfigFactory {
      * details.
      * 
      * @param loader
-     *            will be used to load resources
+     *            will be used to load resources by setting this loader on the
+     *            provided options
      * @param resource
      *            resource to look up
      * @param options
-     *            parse options
+     *            parse options (class loader is ignored)
      * @return the parsed configuration
      */
     public static Config parseResources(ClassLoader loader, String resource,
             ConfigParseOptions options) {
-        return Parseable.newResources(loader, resource, options).parse().toConfig();
+        return Parseable.newResources(resource, options.setClassLoader(loader)).parse().toConfig();
     }
 
     public static Config parseResources(ClassLoader loader, String resource) {
@@ -607,18 +617,19 @@ public final class ConfigFactory {
      * some details and caveats on this method.
      *
      * @param loader
-     *            class loader to look up resources in
+     *            class loader to look up resources in, will be set on options
      * @param resourceBasename
      *            a resource name as in
      *            {@link java.lang.ClassLoader#getResource}, with or without
      *            extension
      * @param options
-     *            parse options
+     *            parse options (class loader ignored)
      * @return the parsed configuration
      */
     public static Config parseResourcesAnySyntax(ClassLoader loader, String resourceBasename,
             ConfigParseOptions options) {
-        return ConfigImpl.parseResourcesAnySyntax(loader, resourceBasename, options).toConfig();
+        return ConfigImpl.parseResourcesAnySyntax(resourceBasename, options.setClassLoader(loader))
+                .toConfig();
     }
 
     public static Config parseResourcesAnySyntax(ClassLoader loader, String resourceBasename) {
@@ -630,8 +641,7 @@ public final class ConfigFactory {
      * uses thread's current context class loader.
      */
     public static Config parseResources(String resource, ConfigParseOptions options) {
-        return Parseable
-                .newResources(Thread.currentThread().getContextClassLoader(), resource, options)
+        return Parseable.newResources(resource, options)
                 .parse().toConfig();
     }
 
@@ -640,8 +650,7 @@ public final class ConfigFactory {
      * current context class loader.
      */
     public static Config parseResources(String resource) {
-        return parseResources(Thread.currentThread().getContextClassLoader(), resource,
-                ConfigParseOptions.defaults());
+        return parseResources(resource, ConfigParseOptions.defaults());
     }
 
     /**
@@ -650,8 +659,7 @@ public final class ConfigFactory {
      * but uses thread's current context class loader.
      */
     public static Config parseResourcesAnySyntax(String resourceBasename, ConfigParseOptions options) {
-        return ConfigImpl.parseResourcesAnySyntax(Thread.currentThread().getContextClassLoader(),
-                resourceBasename, options).toConfig();
+        return ConfigImpl.parseResourcesAnySyntax(resourceBasename, options).toConfig();
     }
 
     /**
@@ -659,8 +667,7 @@ public final class ConfigFactory {
      * thread's current context class loader.
      */
     public static Config parseResourcesAnySyntax(String resourceBasename) {
-        return parseResourcesAnySyntax(Thread.currentThread().getContextClassLoader(),
-                resourceBasename, ConfigParseOptions.defaults());
+        return parseResourcesAnySyntax(resourceBasename, ConfigParseOptions.defaults());
     }
 
     public static Config parseString(String s, ConfigParseOptions options) {

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

@@ -25,17 +25,19 @@ public final class ConfigParseOptions {
     final String originDescription;
     final boolean allowMissing;
     final ConfigIncluder includer;
+    final ClassLoader classLoader;
 
-    protected ConfigParseOptions(ConfigSyntax syntax, String originDescription,
-            boolean allowMissing, ConfigIncluder includer) {
+    private ConfigParseOptions(ConfigSyntax syntax, String originDescription, boolean allowMissing,
+            ConfigIncluder includer, ClassLoader classLoader) {
         this.syntax = syntax;
         this.originDescription = originDescription;
         this.allowMissing = allowMissing;
         this.includer = includer;
+        this.classLoader = classLoader;
     }
 
     public static ConfigParseOptions defaults() {
-        return new ConfigParseOptions(null, null, true, null);
+        return new ConfigParseOptions(null, null, true, null, null);
     }
 
     /**
@@ -50,8 +52,8 @@ public final class ConfigParseOptions {
         if (this.syntax == syntax)
             return this;
         else
-            return new ConfigParseOptions(syntax, this.originDescription,
-                    this.allowMissing, this.includer);
+            return new ConfigParseOptions(syntax, this.originDescription, this.allowMissing,
+                    this.includer, this.classLoader);
     }
 
     public ConfigSyntax getSyntax() {
@@ -75,8 +77,8 @@ public final class ConfigParseOptions {
                 && this.originDescription.equals(originDescription))
             return this;
         else
-            return new ConfigParseOptions(this.syntax, originDescription,
-                    this.allowMissing, this.includer);
+            return new ConfigParseOptions(this.syntax, originDescription, this.allowMissing,
+                    this.includer, this.classLoader);
     }
 
     public String getOriginDescription() {
@@ -103,8 +105,8 @@ public final class ConfigParseOptions {
         if (this.allowMissing == allowMissing)
             return this;
         else
-            return new ConfigParseOptions(this.syntax, this.originDescription,
-                    allowMissing, this.includer);
+            return new ConfigParseOptions(this.syntax, this.originDescription, allowMissing,
+                    this.includer, this.classLoader);
     }
 
     public boolean getAllowMissing() {
@@ -122,7 +124,8 @@ public final class ConfigParseOptions {
             return this;
         else
             return new ConfigParseOptions(this.syntax, this.originDescription,
-                    this.allowMissing, includer);
+ this.allowMissing,
+                    includer, this.classLoader);
     }
 
     public ConfigParseOptions prependIncluder(ConfigIncluder includer) {
@@ -147,4 +150,34 @@ public final class ConfigParseOptions {
         return includer;
     }
 
+    /**
+     * Set the class loader. If set to null,
+     * <code>Thread.currentThread().getContextClassLoader()</code> will be used.
+     * 
+     * @param loader
+     *            a class loader or {@code null} to use thread context class
+     *            loader
+     * @return options with the class loader set
+     */
+    public ConfigParseOptions setClassLoader(ClassLoader loader) {
+        if (this.classLoader == loader)
+            return this;
+        else
+            return new ConfigParseOptions(this.syntax, this.originDescription, this.allowMissing,
+                    this.includer, loader);
+    }
+
+    /**
+     * Get the class loader; never returns {@code null}, if the class loader was
+     * unset, returns
+     * <code>Thread.currentThread().getContextClassLoader()</code>.
+     *
+     * @return class loader to use
+     */
+    public ClassLoader getClassLoader() {
+        if (this.classLoader == null)
+            return Thread.currentThread().getContextClassLoader();
+        else
+            return this.classLoader;
+    }
 }

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

@@ -90,12 +90,12 @@ public class ConfigImpl {
     }
 
     /** For use ONLY by library internals, DO NOT TOUCH not guaranteed ABI */
-    public static ConfigObject parseResourcesAnySyntax(final ClassLoader loader,
-            String resourceBasename, final ConfigParseOptions baseOptions) {
+    public static ConfigObject parseResourcesAnySyntax(String resourceBasename,
+            final ConfigParseOptions baseOptions) {
         NameSource source = new NameSource() {
             @Override
             public ConfigParseable nameToParseable(String name) {
-                return Parseable.newResources(loader, name, baseOptions);
+                return Parseable.newResources(name, baseOptions);
             }
         };
         return SimpleIncluder.fromBasename(source, resourceBasename, baseOptions);
@@ -334,7 +334,8 @@ public class ConfigImpl {
             @Override
             public Config call() {
                 Config unresolvedResources = Parseable
-                        .newResources(loader, "reference.conf", ConfigParseOptions.defaults())
+                        .newResources("reference.conf",
+                                ConfigParseOptions.defaults().setClassLoader(loader))
                         .parse().toConfig();
                 return systemPropertiesAsConfig().withFallback(unresolvedResources).resolve();
             }

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

@@ -438,8 +438,7 @@ public abstract class Parseable implements ConfigParseable {
                 String resource = filename;
                 if (filename.startsWith("/"))
                     resource = filename.substring(1);
-                return newResources(this.getClass().getClassLoader(), resource, options()
-                        .setOriginDescription(null));
+                return newResources(resource, options().setOriginDescription(null));
             }
         }
 
@@ -459,11 +458,9 @@ public abstract class Parseable implements ConfigParseable {
     }
 
     private final static class ParseableResources extends Parseable {
-        final private ClassLoader loader;
         final private String resource;
 
-        ParseableResources(ClassLoader loader, String resource, ConfigParseOptions options) {
-            this.loader = loader;
+        ParseableResources(String resource, ConfigParseOptions options) {
             this.resource = resource;
             postConstruct(options);
         }
@@ -476,6 +473,7 @@ public abstract class Parseable implements ConfigParseable {
         @Override
         protected AbstractConfigObject rawParseValue(ConfigOrigin origin,
                 ConfigParseOptions finalOptions) throws IOException {
+            ClassLoader loader = finalOptions.getClassLoader();
             Enumeration<URL> e = loader.getResources(resource);
             if (!e.hasMoreElements()) {
                 if (ConfigImpl.traceLoadsEnabled())
@@ -543,8 +541,7 @@ public abstract class Parseable implements ConfigParseable {
             if (sibling.startsWith("/")) {
                 // if it starts with "/" then don't make it relative to
                 // the including resource
-                return newResources(loader, sibling.substring(1),
-                        options().setOriginDescription(null));
+                return newResources(sibling.substring(1), options().setOriginDescription(null));
             } else {
                 // here we want to build a new resource name and let
                 // the class loader have it, rather than getting the
@@ -553,9 +550,9 @@ public abstract class Parseable implements ConfigParseable {
                 // search a classpath.
                 String parent = parent(resource);
                 if (parent == null)
-                    return newResources(loader, sibling, options().setOriginDescription(null));
+                    return newResources(sibling, options().setOriginDescription(null));
                 else
-                    return newResources(loader, parent + "/" + sibling, options()
+                    return newResources(parent + "/" + sibling, options()
                             .setOriginDescription(null));
             }
         }
@@ -567,13 +564,13 @@ public abstract class Parseable implements ConfigParseable {
 
         @Override
         public String toString() {
-            return getClass().getSimpleName() + "(" + resource + ","
-                    + loader.getClass().getSimpleName() + ")";
+            return getClass().getSimpleName() + "(" + resource + ")";
         }
     }
 
     public static Parseable newResources(Class<?> klass, String resource, ConfigParseOptions options) {
-        return newResources(klass.getClassLoader(), convertResourceName(klass, resource), options);
+        return newResources(convertResourceName(klass, resource),
+                options.setClassLoader(klass.getClassLoader()));
     }
 
     // this function is supposed to emulate the difference
@@ -601,9 +598,8 @@ public abstract class Parseable implements ConfigParseable {
         }
     }
 
-    public static Parseable newResources(ClassLoader loader, String resource,
-            ConfigParseOptions options) {
-        return new ParseableResources(loader, resource, options);
+    public static Parseable newResources(String resource, ConfigParseOptions options) {
+        return new ParseableResources(resource, options);
     }
 
     private final static class ParseableProperties extends Parseable {

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

@@ -466,7 +466,7 @@ class PublicApiTest extends TestUtils {
     }
 
     @Test
-    def usesContextClassLoader() {
+    def usesContextClassLoaderForReferenceConf() {
         val loaderA1 = new TestClassLoader(this.getClass().getClassLoader(),
             Map("reference.conf" -> resourceFile("a_1.conf").toURI.toURL()))
         val loaderB2 = new TestClassLoader(this.getClass().getClassLoader(),
@@ -490,7 +490,31 @@ class PublicApiTest extends TestUtils {
     }
 
     @Test
-    def usesSuppliedClassLoader() {
+    def usesContextClassLoaderForApplicationConf() {
+        val loaderA1 = new TestClassLoader(this.getClass().getClassLoader(),
+            Map("application.conf" -> resourceFile("a_1.conf").toURI.toURL()))
+        val loaderB2 = new TestClassLoader(this.getClass().getClassLoader(),
+            Map("application.conf" -> resourceFile("b_2.conf").toURI.toURL()))
+
+        val configA1 = withContextClassLoader(loaderA1) {
+            ConfigFactory.load()
+        }
+        assertEquals(1, configA1.getInt("a"))
+        assertFalse("no b", configA1.hasPath("b"))
+
+        val configB2 = withContextClassLoader(loaderB2) {
+            ConfigFactory.load()
+        }
+        assertEquals(2, configB2.getInt("b"))
+        assertFalse("no a", configB2.hasPath("a"))
+
+        val configPlain = ConfigFactory.load()
+        assertFalse("no a", configPlain.hasPath("a"))
+        assertFalse("no b", configPlain.hasPath("b"))
+    }
+
+    @Test
+    def usesSuppliedClassLoaderForReferenceConf() {
         val loaderA1 = new TestClassLoader(this.getClass().getClassLoader(),
             Map("reference.conf" -> resourceFile("a_1.conf").toURI.toURL()))
         val loaderB2 = new TestClassLoader(this.getClass().getClassLoader(),
@@ -526,6 +550,18 @@ class PublicApiTest extends TestUtils {
             assertFalse("no b", c.hasPath("b"))
         }
 
+        // check providing the loader via ConfigParseOptions
+        val withLoader = ConfigParseOptions.defaults().setClassLoader(loaderA1);
+        for (
+            c <- Seq(ConfigFactory.parseResources("reference.conf", withLoader),
+                ConfigFactory.parseResourcesAnySyntax("reference", withLoader),
+                ConfigFactory.load("application", withLoader, ConfigResolveOptions.defaults()))
+        ) {
+            assertEquals(1, c.getInt("a"))
+            assertFalse("no b", c.hasPath("b"))
+        }
+
+        // check not providing the loader
         for (
             c <- Seq(ConfigFactory.parseResources("reference.conf"),
                 ConfigFactory.parseResourcesAnySyntax("reference"),
@@ -540,6 +576,98 @@ class PublicApiTest extends TestUtils {
             assertFalse("no a", c.hasPath("a"))
             assertFalse("no b", c.hasPath("b"))
         }
+
+        // check providing the loader via current context
+        withContextClassLoader(loaderA1) {
+            for (
+                c <- Seq(ConfigFactory.parseResources("reference.conf"),
+                    ConfigFactory.parseResourcesAnySyntax("reference"),
+                    ConfigFactory.parseResources("reference.conf", ConfigParseOptions.defaults()),
+                    ConfigFactory.parseResourcesAnySyntax("reference", ConfigParseOptions.defaults()),
+                    ConfigFactory.load("application"),
+                    ConfigFactory.load("application", ConfigParseOptions.defaults(), ConfigResolveOptions.defaults()),
+                    ConfigFactory.load(ConfigFactory.parseString("")),
+                    ConfigFactory.load(ConfigFactory.parseString(""), ConfigResolveOptions.defaults()),
+                    ConfigFactory.defaultReference())
+            ) {
+                assertEquals(1, c.getInt("a"))
+                assertFalse("no b", c.hasPath("b"))
+            }
+        }
+    }
+
+    @Test
+    def usesSuppliedClassLoaderForApplicationConf() {
+        val loaderA1 = new TestClassLoader(this.getClass().getClassLoader(),
+            Map("application.conf" -> resourceFile("a_1.conf").toURI.toURL()))
+        val loaderB2 = new TestClassLoader(this.getClass().getClassLoader(),
+            Map("application.conf" -> resourceFile("b_2.conf").toURI.toURL()))
+
+        val configA1 = ConfigFactory.load(loaderA1)
+
+        assertEquals(1, configA1.getInt("a"))
+        assertFalse("no b", configA1.hasPath("b"))
+
+        val configB2 = ConfigFactory.load(loaderB2)
+
+        assertEquals(2, configB2.getInt("b"))
+        assertFalse("no a", configB2.hasPath("a"))
+
+        val configPlain = ConfigFactory.load()
+        assertFalse("no a", configPlain.hasPath("a"))
+        assertFalse("no b", configPlain.hasPath("b"))
+
+        // check the various overloads that take a loader parameter
+        for (
+            c <- Seq(ConfigFactory.parseResources(loaderA1, "application.conf"),
+                ConfigFactory.parseResourcesAnySyntax(loaderA1, "application"),
+                ConfigFactory.parseResources(loaderA1, "application.conf", ConfigParseOptions.defaults()),
+                ConfigFactory.parseResourcesAnySyntax(loaderA1, "application", ConfigParseOptions.defaults()),
+                ConfigFactory.load(loaderA1, "application"),
+                ConfigFactory.load(loaderA1, "application", ConfigParseOptions.defaults(), ConfigResolveOptions.defaults()))
+        ) {
+            assertEquals(1, c.getInt("a"))
+            assertFalse("no b", c.hasPath("b"))
+        }
+
+        // check providing the loader via ConfigParseOptions
+        val withLoader = ConfigParseOptions.defaults().setClassLoader(loaderA1);
+        for (
+            c <- Seq(ConfigFactory.parseResources("application.conf", withLoader),
+                ConfigFactory.parseResourcesAnySyntax("application", withLoader),
+                ConfigFactory.load("application", withLoader, ConfigResolveOptions.defaults()))
+        ) {
+            assertEquals(1, c.getInt("a"))
+            assertFalse("no b", c.hasPath("b"))
+        }
+
+        // check not providing the loader
+        for (
+            c <- Seq(ConfigFactory.parseResources("application.conf"),
+                ConfigFactory.parseResourcesAnySyntax("application"),
+                ConfigFactory.parseResources("application.conf", ConfigParseOptions.defaults()),
+                ConfigFactory.parseResourcesAnySyntax("application", ConfigParseOptions.defaults()),
+                ConfigFactory.load("application"),
+                ConfigFactory.load("application", ConfigParseOptions.defaults(), ConfigResolveOptions.defaults()))
+        ) {
+            assertFalse("no a", c.hasPath("a"))
+            assertFalse("no b", c.hasPath("b"))
+        }
+
+        // check providing the loader via current context
+        withContextClassLoader(loaderA1) {
+            for (
+                c <- Seq(ConfigFactory.parseResources("application.conf"),
+                    ConfigFactory.parseResourcesAnySyntax("application"),
+                    ConfigFactory.parseResources("application.conf", ConfigParseOptions.defaults()),
+                    ConfigFactory.parseResourcesAnySyntax("application", ConfigParseOptions.defaults()),
+                    ConfigFactory.load("application"),
+                    ConfigFactory.load("application", ConfigParseOptions.defaults(), ConfigResolveOptions.defaults()))
+            ) {
+                assertEquals(1, c.getInt("a"))
+                assertFalse("no b", c.hasPath("b"))
+            }
+        }
     }
 
     @Test

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

@@ -584,8 +584,11 @@ abstract trait TestUtils {
                 val t = Thread.currentThread()
                 val old = t.getContextClassLoader()
                 t.setContextClassLoader(loader)
-                val result = body
+                val result = try {
+                    body
+                } finally {
                     t.setContextClassLoader(old)
+                }
                 result
             }
         })