From cb5017af8b98e0887e5507dd88f48d0e412d0244 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Sun, 27 Nov 2011 01:08:32 -0500 Subject: [PATCH] change parseResource to parseResources parse all resources with the requested name in classpath order. also extend SimpleConfigOrigin to distinguish resources by URL --- .../com/typesafe/config/ConfigFactory.java | 93 +++++++++++++-- .../com/typesafe/config/impl/ConfigImpl.java | 2 +- .../com/typesafe/config/impl/Parseable.java | 109 +++++++++++++----- .../config/impl/SimpleConfigOrigin.java | 83 ++++++++----- .../typesafe/config/impl/ConfParserTest.scala | 2 +- .../typesafe/config/impl/PublicApiTest.scala | 26 +++-- test-lib/src/test/resources/test01.conf | 11 +- 7 files changed, 244 insertions(+), 82 deletions(-) diff --git a/config/src/main/java/com/typesafe/config/ConfigFactory.java b/config/src/main/java/com/typesafe/config/ConfigFactory.java index 544a3156..6cf3fede 100644 --- a/config/src/main/java/com/typesafe/config/ConfigFactory.java +++ b/config/src/main/java/com/typesafe/config/ConfigFactory.java @@ -139,16 +139,36 @@ public final class ConfigFactory { } /** - * Parses a file. If the fileBasename already ends in a known extension, - * just parses it according to that extension. If the fileBasename does not - * end in an extension, then parse all known extensions and merge whatever - * is found. If options force a specific syntax, only parse files with an - * extension matching that syntax. If options.getAllowMissing() is true, - * then no files have to exist; if false, then at least one file has to - * exist. + * Parses a file with a flexible extension. If the fileBasename + * already ends in a known extension, this method parses it according to + * that extension (the file's syntax must match its extension). If the + * fileBasename does not end in an extension, it parses files + * with all known extensions and merges whatever is found. + * + *

+ * In the current implementation, the extension ".conf" forces + * {@link ConfigSyntax#CONF}, ".json" forces {@link ConfigSyntax#JSON}, and + * ".properties" forces {@link ConfigSyntax#PROPERTIES}. When merging files, + * ".conf" falls back to ".json" falls back to ".properties". + * + *

+ * Future versions of the implementation may add additional syntaxes or + * additional extensions. However, the ordering (fallback priority) of the + * three current extensions will remain the same. + * + *

+ * If options forces a specific syntax, this method only parses + * files with an extension matching that syntax. + * + *

+ * If {@link ConfigParseOptions#getAllowMissing options.getAllowMissing()} + * is true, then no files have to exist; if false, then at least one file + * has to exist. * * @param fileBasename + * a filename with or without extension * @param options + * parse options * @return the parsed configuration */ public static Config parseFileAnySyntax(File fileBasename, @@ -156,22 +176,71 @@ public final class ConfigFactory { return ConfigImpl.parseFileAnySyntax(fileBasename, options).toConfig(); } - public static Config parseResource(Class klass, String resource, + /** + * Parses all resources on the classpath with the given name and merges them + * into a single Config. + * + *

+ * If the resource name does not begin with a "/", it will have the supplied + * class's package added to it, in the same way as + * {@link java.lang.Class#getResource}. + * + *

+ * Duplicate resources with the same name are merged such that ones returned + * earlier from {@link ClassLoader#getResources} fall back to (have higher + * priority than) the ones returned later. This implies that resources + * earlier in the classpath override those later in the classpath when they + * configure the same setting. However, in practice real applications may + * not be consistent about classpath ordering, so be careful. It may be best + * to avoid assuming too much. + * + * @param klass + * klass.getClassLoader() will be used to load + * resources, and non-absolute resource names will have this + * class's package added + * @param resource + * resource to look up, relative to klass's package + * or absolute starting with a "/" + * @param options + * parse options + * @return the parsed configuration + */ + public static Config parseResources(Class klass, String resource, ConfigParseOptions options) { - return Parseable.newResource(klass, resource, options).parse() + return Parseable.newResources(klass, resource, options).parse() .toConfig(); } /** - * Same behavior as {@link #parseFileAnySyntax(File,ConfigParseOptions)} but - * for classpath resources instead. + * Parses classpath resources with a flexible extension. In general, this + * method has the same behavior as + * {@link #parseFileAnySyntax(File,ConfigParseOptions)} but for classpath + * resources instead, as in {@link #parseResources}. + * + *

+ * There is a thorny problem with this method, which is that + * {@link java.lang.ClassLoader#getResources} must be called separately for + * each possible extension. The implementation ends up with separate lists + * of resources called "basename.conf" and "basename.json" for example. As a + * result, the ideal ordering between two files with different extensions is + * unknown; there is no way to figure out how to merge the two lists in + * classpath order. To keep it simple, the lists are simply concatenated, + * with the same syntax priorities as + * {@link #parseFileAnySyntax(File,ConfigParseOptions) parseFileAnySyntax()} + * - all ".conf" resources are ahead of all ".json" resources which are + * ahead of all ".properties" resources. * * @param klass + * class which determines the ClassLoader and the + * package for relative resource names * @param resourceBasename + * a resource name as in {@link java.lang.Class#getResource}, + * with or without extension * @param options + * parse options * @return the parsed configuration */ - public static Config parseResourceAnySyntax(Class klass, String resourceBasename, + public static Config parseResourcesAnySyntax(Class klass, String resourceBasename, ConfigParseOptions options) { return ConfigImpl.parseResourceAnySyntax(klass, resourceBasename, options).toConfig(); 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 b13f0e75..82599522 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigImpl.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigImpl.java @@ -119,7 +119,7 @@ public class ConfigImpl { NameSource source = new NameSource() { @Override public ConfigParseable nameToParseable(String name) { - return Parseable.newResource(klass, name, baseOptions); + return Parseable.newResources(klass, name, baseOptions); } }; return fromBasename(source, resourceBasename, baseOptions); diff --git a/config/src/main/java/com/typesafe/config/impl/Parseable.java b/config/src/main/java/com/typesafe/config/impl/Parseable.java index 8acb2a8b..64f13f56 100644 --- a/config/src/main/java/com/typesafe/config/impl/Parseable.java +++ b/config/src/main/java/com/typesafe/config/impl/Parseable.java @@ -17,6 +17,7 @@ import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; +import java.util.Enumeration; import java.util.Iterator; import java.util.Properties; @@ -105,7 +106,7 @@ public abstract class Parseable implements ConfigParseable { return forceParsedToObject(parseValue(baseOptions)); } - AbstractConfigValue parseValue(ConfigParseOptions baseOptions) { + final AbstractConfigValue parseValue(ConfigParseOptions baseOptions) { // note that we are NOT using our "initialOptions", // but using the ones from the passed-in options. The idea is that // callers can get our original options and then parse with different @@ -121,22 +122,10 @@ public abstract class Parseable implements ConfigParseable { return parseValue(origin, options); } - protected AbstractConfigValue parseValue(ConfigOrigin origin, + final private AbstractConfigValue parseValue(ConfigOrigin origin, ConfigParseOptions finalOptions) { try { - Reader reader = reader(); - try { - if (finalOptions.getSyntax() == ConfigSyntax.PROPERTIES) { - return PropertiesParser.parse(reader, origin); - } else { - Iterator tokens = Tokenizer.tokenize(origin, reader, - finalOptions.getSyntax()); - return Parser.parse(tokens, origin, finalOptions, - includeContext()); - } - } finally { - reader.close(); - } + return rawParseValue(origin, finalOptions); } catch (IOException e) { if (finalOptions.getAllowMissing()) { return SimpleConfigObject.emptyMissing(origin); @@ -146,6 +135,28 @@ public abstract class Parseable implements ConfigParseable { } } + // this is parseValue without post-processing the IOException or handling + // options.getAllowMissing() + protected AbstractConfigValue rawParseValue(ConfigOrigin origin, ConfigParseOptions finalOptions) + throws IOException { + Reader reader = reader(); + try { + return rawParseValue(reader, origin, finalOptions); + } finally { + reader.close(); + } + } + + protected AbstractConfigValue rawParseValue(Reader reader, ConfigOrigin origin, + ConfigParseOptions finalOptions) throws IOException { + if (finalOptions.getSyntax() == ConfigSyntax.PROPERTIES) { + return PropertiesParser.parse(reader, origin); + } else { + Iterator tokens = Tokenizer.tokenize(origin, reader, finalOptions.getSyntax()); + return Parser.parse(tokens, origin, finalOptions, includeContext()); + } + } + public ConfigObject parse() { return forceParsedToObject(parseValue(options())); } @@ -384,11 +395,11 @@ public abstract class Parseable implements ConfigParseable { return new ParseableFile(input, options); } - private final static class ParseableResource extends Parseable { + private final static class ParseableResources extends Parseable { final private ClassLoader loader; final private String resource; - ParseableResource(ClassLoader loader, String resource, + ParseableResources(ClassLoader loader, String resource, ConfigParseOptions options) { this.loader = loader; this.resource = resource; @@ -397,12 +408,48 @@ public abstract class Parseable implements ConfigParseable { @Override protected Reader reader() throws IOException { - InputStream stream = loader.getResourceAsStream(resource); - if (stream == null) { - throw new IOException("resource not found on classpath: " - + resource); + throw new ConfigException.BugOrBroken( + "reader() should not be called on resources"); + } + + @Override + protected AbstractConfigObject rawParseValue(ConfigOrigin origin, + ConfigParseOptions finalOptions) throws IOException { + Enumeration e = loader.getResources(resource); + if (!e.hasMoreElements()) { + throw new IOException("resource not found on classpath: " + resource); } - return readerFromStream(stream); + AbstractConfigObject merged = SimpleConfigObject.empty(origin); + while (e.hasMoreElements()) { + URL url = e.nextElement(); + + ConfigOrigin elementOrigin = ((SimpleConfigOrigin) origin).addURL(url); + + AbstractConfigValue v; + + // it's tempting to use ParseableURL here but it would be wrong + // because the wrong relativeTo() would be used for includes. + InputStream stream = url.openStream(); + try { + Reader reader = readerFromStream(stream); + stream = null; // reader now owns it + try { + // parse in "raw" mode which will throw any IOException + // from here. + v = rawParseValue(reader, elementOrigin, finalOptions); + } finally { + reader.close(); + } + } finally { + // stream is null if the reader owns it + if (stream != null) + stream.close(); + } + + merged = merged.withFallback(v); + } + + return merged; } @Override @@ -428,10 +475,10 @@ public abstract class Parseable implements ConfigParseable { // search a classpath. String parent = parent(resource); if (parent == null) - return newResource(loader, sibling, options() + return newResources(loader, sibling, options() .setOriginDescription(null)); else - return newResource(loader, parent + "/" + sibling, + return newResources(loader, parent + "/" + sibling, options().setOriginDescription(null)); } @@ -442,7 +489,9 @@ public abstract class Parseable implements ConfigParseable { @Override public URL url() { - return loader.getResource(resource); + // because we may represent multiple resources, there's nothing + // good to return here. + return null; } @Override @@ -452,9 +501,9 @@ public abstract class Parseable implements ConfigParseable { } } - public static Parseable newResource(Class klass, String resource, + public static Parseable newResources(Class klass, String resource, ConfigParseOptions options) { - return newResource(klass.getClassLoader(), convertResourceName(klass, resource), options); + return newResources(klass.getClassLoader(), convertResourceName(klass, resource), options); } // this function is supposed to emulate the difference @@ -482,9 +531,9 @@ public abstract class Parseable implements ConfigParseable { } } - public static Parseable newResource(ClassLoader loader, String resource, + public static Parseable newResources(ClassLoader loader, String resource, ConfigParseOptions options) { - return new ParseableResource(loader, resource, options); + return new ParseableResources(loader, resource, options); } private final static class ParseableProperties extends Parseable { @@ -502,7 +551,7 @@ public abstract class Parseable implements ConfigParseable { } @Override - protected AbstractConfigObject parseValue(ConfigOrigin origin, + protected AbstractConfigObject rawParseValue(ConfigOrigin origin, ConfigParseOptions finalOptions) { return PropertiesParser.fromProperties(origin, props); } diff --git a/config/src/main/java/com/typesafe/config/impl/SimpleConfigOrigin.java b/config/src/main/java/com/typesafe/config/impl/SimpleConfigOrigin.java index a8c38ae8..5d99e5ca 100644 --- a/config/src/main/java/com/typesafe/config/impl/SimpleConfigOrigin.java +++ b/config/src/main/java/com/typesafe/config/impl/SimpleConfigOrigin.java @@ -11,41 +11,65 @@ import java.util.Collection; import com.typesafe.config.ConfigException; import com.typesafe.config.ConfigOrigin; +// it would be cleaner to have a class hierarchy for various origin types, +// but was hoping this would be enough simpler to be a little messy. eh. final class SimpleConfigOrigin implements ConfigOrigin { final private String description; final private int lineNumber; final private OriginType originType; + final private String urlOrNull; - private SimpleConfigOrigin(String description, int lineNumber, OriginType originType) { + protected SimpleConfigOrigin(String description, int lineNumber, OriginType originType, + String urlOrNull) { this.lineNumber = lineNumber; this.originType = originType; this.description = description; + this.urlOrNull = urlOrNull; } static SimpleConfigOrigin newSimple(String description) { - return new SimpleConfigOrigin(description, -1, OriginType.GENERIC); + return new SimpleConfigOrigin(description, -1, OriginType.GENERIC, null); } static SimpleConfigOrigin newFile(String filename) { - return new SimpleConfigOrigin(filename, -1, OriginType.FILE); + String url; + try { + url = (new File(filename)).toURI().toURL().toExternalForm(); + } catch (MalformedURLException e) { + url = null; + } + return new SimpleConfigOrigin(filename, -1, OriginType.FILE, url); } static SimpleConfigOrigin newURL(URL url) { - return new SimpleConfigOrigin(url.toExternalForm(), -1, OriginType.URL); + String u = url.toExternalForm(); + return new SimpleConfigOrigin(u, -1, OriginType.URL, u); + } + + static SimpleConfigOrigin newResource(String resource, URL url) { + return new SimpleConfigOrigin(resource, -1, OriginType.RESOURCE, + url != null ? url.toExternalForm() : null); } static SimpleConfigOrigin newResource(String resource) { - return new SimpleConfigOrigin(resource, -1, OriginType.RESOURCE); + return newResource(resource, null); } // important, this should also be able to _change_ an existing line // number SimpleConfigOrigin addLineNumber(int lineNumber) { - return new SimpleConfigOrigin(this.description, lineNumber, this.originType); + return new SimpleConfigOrigin(this.description, lineNumber, this.originType, this.urlOrNull); + } + + SimpleConfigOrigin addURL(URL url) { + return new SimpleConfigOrigin(this.description, this.lineNumber, this.originType, + url != null ? url.toExternalForm() : null); } @Override public String description() { + // not putting the URL in here for files and resources, because people + // parsing "file: line" syntax would hit the ":" in the URL. if (lineNumber < 0) { return description; } else { @@ -56,9 +80,12 @@ final class SimpleConfigOrigin implements ConfigOrigin { @Override public boolean equals(Object other) { if (other instanceof SimpleConfigOrigin) { - // two origins are equal if they are described to the user in the - // same way, for now at least this seems fine - return this.description.equals(((SimpleConfigOrigin) other).description); + SimpleConfigOrigin otherOrigin = (SimpleConfigOrigin) other; + + return this.description.equals(otherOrigin.description) + && this.lineNumber == otherOrigin.lineNumber + && this.originType == otherOrigin.originType + && ConfigUtil.equalsHandlingNull(this.urlOrNull, otherOrigin.urlOrNull); } else { return false; } @@ -66,22 +93,32 @@ final class SimpleConfigOrigin implements ConfigOrigin { @Override public int hashCode() { - return description.hashCode(); + int h = 41 * (41 + description.hashCode()); + h = 41 * (h + lineNumber); + h = 41 * (h + originType.hashCode()); + if (urlOrNull != null) + h = 41 * (h + urlOrNull.hashCode()); + return h; } @Override public String toString() { - return "ConfigOrigin(" + description + ")"; + // the url is only really useful on top of description for resources + if (originType == OriginType.RESOURCE && urlOrNull != null) { + return "ConfigOrigin(" + description + "," + urlOrNull + ")"; + } else { + return "ConfigOrigin(" + description + ")"; + } } @Override public String filename() { if (originType == OriginType.FILE) { return description; - } else if (originType == OriginType.URL) { + } else if (urlOrNull != null) { URL url; try { - url = new URL(description); + url = new URL(urlOrNull); } catch (MalformedURLException e) { return null; } @@ -97,20 +134,14 @@ final class SimpleConfigOrigin implements ConfigOrigin { @Override public URL url() { - if (originType == OriginType.URL) { - try { - return new URL(description); - } catch (MalformedURLException e) { - return null; - } - } else if (originType == OriginType.FILE) { - try { - return (new File(description)).toURI().toURL(); - } catch (MalformedURLException e) { - return null; - } - } else { + if (urlOrNull == null) { return null; + } else { + try { + return new URL(urlOrNull); + } catch (MalformedURLException e) { + return null; + } } } diff --git a/config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala b/config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala index 4521e107..8e76eb6b 100644 --- a/config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala @@ -355,7 +355,7 @@ class ConfParserTest extends TestUtils { // just be sure the toString don't throw, to get test coverage val options = ConfigParseOptions.defaults() Parseable.newFile(new File("foo"), options).toString - Parseable.newResource(classOf[ConfParserTest], "foo", options).toString + Parseable.newResources(classOf[ConfParserTest], "foo", options).toString Parseable.newURL(new URL("file:///foo"), options).toString Parseable.newProperties(new Properties(), options).toString Parseable.newReader(new StringReader("{}"), options).toString 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 423147c3..4e04694f 100644 --- a/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala @@ -309,7 +309,7 @@ class PublicApiTest extends TestUtils { @Test def includersAreUsedWithClasspath() { - val included = whatWasIncluded(ConfigFactory.parseResource(classOf[PublicApiTest], "/test03.conf", _)) + val included = whatWasIncluded(ConfigFactory.parseResources(classOf[PublicApiTest], "/test03.conf", _)) assertEquals(List("test01", "test02.conf", "equiv01/original.json", "nothere", "nothere.conf", "nothere.json", "nothere.properties"), @@ -320,7 +320,7 @@ class PublicApiTest extends TestUtils { def includersAreUsedRecursivelyWithClasspath() { // includes.conf has recursive includes in it; here we look it up // with an "absolute" class path resource. - val included = whatWasIncluded(ConfigFactory.parseResource(classOf[PublicApiTest], "/equiv03/includes.conf", _)) + val included = whatWasIncluded(ConfigFactory.parseResources(classOf[PublicApiTest], "/equiv03/includes.conf", _)) assertEquals(List("letters/a.conf", "numbers/1.conf", "numbers/2", "letters/b.json", "letters/c"), included.map(_.name)) @@ -330,7 +330,7 @@ class PublicApiTest extends TestUtils { def includersAreUsedRecursivelyWithClasspathRelativeResource() { // includes.conf has recursive includes in it; here we look it up // with a "class-relative" class path resource - val included = whatWasIncluded(ConfigFactory.parseResource(classOf[SomethingInEquiv03], "includes.conf", _)) + val included = whatWasIncluded(ConfigFactory.parseResources(classOf[SomethingInEquiv03], "includes.conf", _)) assertEquals(List("letters/a.conf", "numbers/1.conf", "numbers/2", "letters/b.json", "letters/c"), included.map(_.name)) @@ -380,7 +380,7 @@ class PublicApiTest extends TestUtils { assertEquals("true", onlyPropsViaOptions.getString("fromProps.bool")) // make sure it works with resources too - val fromResources = ConfigFactory.parseResourceAnySyntax(classOf[PublicApiTest], "/test01", + val fromResources = ConfigFactory.parseResourcesAnySyntax(classOf[PublicApiTest], "/test01", ConfigParseOptions.defaults()) assertEquals(42, fromResources.getInt("ints.fortyTwo")) assertEquals("A", fromResources.getString("fromJsonA")) @@ -389,17 +389,25 @@ class PublicApiTest extends TestUtils { @Test def resourceFromAnotherClasspath() { - val conf = ConfigFactory.parseResource(classOf[PublicApiTest], "/test-lib.conf", ConfigParseOptions.defaults()) + val conf = ConfigFactory.parseResources(classOf[PublicApiTest], "/test-lib.conf", ConfigParseOptions.defaults()) assertEquals("This is to test classpath searches.", conf.getString("test-lib.description")) } @Test - def onlyFirstResourceUsed() { - val conf = ConfigFactory.parseResource(classOf[PublicApiTest], "/test01.conf", ConfigParseOptions.defaults()) + def multipleResourcesUsed() { + val conf = ConfigFactory.parseResources(classOf[PublicApiTest], "/test01.conf", ConfigParseOptions.defaults()) assertEquals(42, conf.getInt("ints.fortyTwo")) - assertFalse(conf.hasPath("test-lib")) - assertFalse(conf.hasPath("test-lib.fromTestLib")) + assertEquals(true, conf.getBoolean("test-lib.fromTestLib")) + + // check that each value has its own ConfigOrigin + val v1 = conf.getValue("ints.fortyTwo") + val v2 = conf.getValue("test-lib.fromTestLib") + assertEquals("test01.conf", v1.origin.resource) + assertEquals("test01.conf", v2.origin.resource) + assertEquals(v1.origin.resource, v2.origin.resource) + assertFalse("same urls in " + v1.origin + " " + v2.origin, v1.origin.url == v2.origin.url) + assertFalse(v1.origin.filename == v2.origin.filename) } } diff --git a/test-lib/src/test/resources/test01.conf b/test-lib/src/test/resources/test01.conf index 72e16c36..b2735665 100644 --- a/test-lib/src/test/resources/test01.conf +++ b/test-lib/src/test/resources/test01.conf @@ -2,10 +2,15 @@ # examples/ directory and the docs for things that should be # copied. this is weird test suite stuff. -## Here we are testing that this file test01.conf is NOT seen -## since there's another resource with that name earlier in -## classpath +## Here we are testing that this file test01.conf is merged +## properly with another test01.conf on the classpath test-lib { fromTestLib = true } +ints { + ## this would override the other test01.conf if + ## we merged this file first; the test suite + ## is supposed to check this key's value. + fortyTwo = 900 +}