From 9733578ebbce6fc00b9e611540c3f24b76289911 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Wed, 29 Feb 2012 10:44:24 -0500 Subject: [PATCH] Do not cache default config, and use context class loader to load it So we get any reference.conf from the context class loader. This does NOT fix loading non-default configs, we need new API to allow passing in a class loader for that. It also makes things a bit less efficient since it no longer caches; in the future we could do a per-class-loader cache. --- .../com/typesafe/config/ConfigFactory.java | 79 ++++++++----------- .../com/typesafe/config/impl/ConfigImpl.java | 17 +--- config/src/test/resources/a_1.conf | 1 + config/src/test/resources/b_2.conf | 1 + .../typesafe/config/impl/PublicApiTest.scala | 24 ++++++ .../com/typesafe/config/impl/TestUtils.scala | 29 +++++++ 6 files changed, 93 insertions(+), 58 deletions(-) create mode 100644 config/src/test/resources/a_1.conf create mode 100644 config/src/test/resources/b_2.conf diff --git a/config/src/main/java/com/typesafe/config/ConfigFactory.java b/config/src/main/java/com/typesafe/config/ConfigFactory.java index 1b819ab4..8506552f 100644 --- a/config/src/main/java/com/typesafe/config/ConfigFactory.java +++ b/config/src/main/java/com/typesafe/config/ConfigFactory.java @@ -11,7 +11,6 @@ import java.util.Map; import java.util.Properties; import com.typesafe.config.impl.ConfigImpl; -import com.typesafe.config.impl.ConfigImplUtil; import com.typesafe.config.impl.Parseable; /** @@ -103,49 +102,43 @@ public final class ConfigFactory { .resolve(resolveOptions); } - private static class DefaultConfigHolder { + private static Config loadDefaultConfig() { + int specified = 0; - private static Config loadDefaultConfig() { - int specified = 0; + // override application.conf with config.file, config.resource, + // config.url if requested. + String resource = System.getProperty("config.resource"); + if (resource != null) + specified += 1; + String file = System.getProperty("config.file"); + if (file != null) + specified += 1; + String url = System.getProperty("config.url"); + if (url != null) + specified += 1; - // override application.conf with config.file, config.resource, - // config.url if requested. - String resource = System.getProperty("config.resource"); - if (resource != null) - specified += 1; - String file = System.getProperty("config.file"); - if (file != null) - specified += 1; - String url = System.getProperty("config.url"); - if (url != null) - specified += 1; - - if (specified == 0) { - return load("application"); - } else if (specified > 1) { - throw new ConfigException.Generic("You set more than one of config.file='" + file - + "', config.url='" + url + "', config.resource='" + resource - + "'; don't know which one to use!"); + if (specified == 0) { + return load("application"); + } else if (specified > 1) { + throw new ConfigException.Generic("You set more than one of config.file='" + file + + "', config.url='" + url + "', config.resource='" + resource + + "'; don't know which one to use!"); + } else { + if (resource != null) { + // this deliberately does not parseResourcesAnySyntax; if + // people want that they can use an include statement. + return load(parseResources(ConfigFactory.class, resource)); + } else if (file != null) { + return load(parseFile(new File(file))); } else { - if (resource != null) { - // this deliberately does not parseResourcesAnySyntax; if - // people want that they can use an include statement. - return load(parseResources(ConfigFactory.class, resource)); - } else if (file != null) { - return load(parseFile(new File(file))); - } else { - try { - return load(parseURL(new URL(url))); - } catch (MalformedURLException e) { - throw new ConfigException.Generic( - "Bad URL in config.url system property: '" + url + "': " - + e.getMessage(), e); - } + try { + return load(parseURL(new URL(url))); + } catch (MalformedURLException e) { + throw new ConfigException.Generic("Bad URL in config.url system property: '" + + url + "': " + e.getMessage(), e); } } } - - static final Config defaultConfig = loadDefaultConfig(); } /** @@ -176,11 +169,7 @@ public final class ConfigFactory { * @return configuration for an application */ public static Config load() { - try { - return DefaultConfigHolder.defaultConfig; - } catch (ExceptionInInitializerError e) { - throw ConfigImplUtil.extractInitializerError(e); - } + return loadDefaultConfig(); } /** @@ -301,12 +290,12 @@ public final class ConfigFactory { * string values. If you have both "a=foo" and "a.b=bar" in your properties * file, so "a" is both the object containing "b" and the string "foo", then * the string value is dropped. - * + * *

* If you want to have System.getProperties() as a * ConfigObject, it's better to use the {@link #systemProperties()} method * which returns a cached global singleton. - * + * * @param properties * a Java Properties object * @param options diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigImpl.java b/config/src/main/java/com/typesafe/config/impl/ConfigImpl.java index be5dd019..583d2507 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigImpl.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigImpl.java @@ -397,21 +397,12 @@ public class ConfigImpl { return envVariablesAsConfigObject().toConfig(); } - private static class ReferenceHolder { - private static final Config unresolvedResources = Parseable - .newResources(ConfigImpl.class, "/reference.conf", ConfigParseOptions.defaults()) - .parse().toConfig(); - static final Config referenceConfig = systemPropertiesAsConfig().withFallback( - unresolvedResources).resolve(); - } - /** For use ONLY by library internals, DO NOT TOUCH not guaranteed ABI */ public static Config defaultReference() { - try { - return ReferenceHolder.referenceConfig; - } catch (ExceptionInInitializerError e) { - throw ConfigImplUtil.extractInitializerError(e); - } + Config unresolvedResources = Parseable + .newResources(Thread.currentThread().getContextClassLoader(), "reference.conf", + ConfigParseOptions.defaults()).parse().toConfig(); + return systemPropertiesAsConfig().withFallback(unresolvedResources).resolve(); } private static class DebugHolder { diff --git a/config/src/test/resources/a_1.conf b/config/src/test/resources/a_1.conf new file mode 100644 index 00000000..73cdb8bc --- /dev/null +++ b/config/src/test/resources/a_1.conf @@ -0,0 +1 @@ +a=1 diff --git a/config/src/test/resources/b_2.conf b/config/src/test/resources/b_2.conf new file mode 100644 index 00000000..e82df74c --- /dev/null +++ b/config/src/test/resources/b_2.conf @@ -0,0 +1 @@ +b=2 diff --git a/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala b/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala index 274fb264..2948d69f 100644 --- a/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala @@ -464,4 +464,28 @@ class PublicApiTest extends TestUtils { assertEquals("\"a\"", ConfigUtil.quoteString("a")) assertEquals("\"\\n\"", ConfigUtil.quoteString("\n")) } + + @Test + def usesContextClassLoader() { + val loaderA1 = new TestClassLoader(this.getClass().getClassLoader(), + Map("reference.conf" -> resourceFile("a_1.conf").toURI.toURL())) + val loaderB2 = new TestClassLoader(this.getClass().getClassLoader(), + Map("reference.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")) + } } diff --git a/config/src/test/scala/com/typesafe/config/impl/TestUtils.scala b/config/src/test/scala/com/typesafe/config/impl/TestUtils.scala index ea7562b0..f0c585f5 100644 --- a/config/src/test/scala/com/typesafe/config/impl/TestUtils.scala +++ b/config/src/test/scala/com/typesafe/config/impl/TestUtils.scala @@ -19,6 +19,9 @@ import java.io.ByteArrayInputStream import java.io.ObjectInputStream import org.apache.commons.codec.binary.Hex import scala.annotation.tailrec +import java.net.URL +import java.util.concurrent.Executors +import java.util.concurrent.Callable abstract trait TestUtils { protected def intercept[E <: Throwable: Manifest](block: => Unit): E = { @@ -547,4 +550,30 @@ abstract trait TestUtils { protected def resourceFile(filename: String) = { new File(resourceDir, filename) } + + protected class TestClassLoader(parent: ClassLoader, val additions: Map[String, URL]) extends ClassLoader(parent) { + override def findResources(name: String) = { + import scala.collection.JavaConverters._ + val other = super.findResources(name).asScala + additions.get(name).map({ url => Iterator(url) ++ other }).getOrElse(other).asJavaEnumeration + } + override def findResource(name: String) = { + additions.get(name).getOrElse(null) + } + } + + protected def withContextClassLoader[T](loader: ClassLoader)(body: => T): T = { + val executor = Executors.newSingleThreadExecutor() + val f = executor.submit(new Callable[T] { + override def call(): T = { + val t = Thread.currentThread() + val old = t.getContextClassLoader() + t.setContextClassLoader(loader) + val result = body + t.setContextClassLoader(old) + result + } + }) + f.get + } }