From 9578a7953860e59809955c664ea30ca5f00e4cb4 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Wed, 30 Nov 2011 10:35:21 -0500 Subject: [PATCH] Better handle includes for files and classpaths - support "/" to get an absolute file or resource - fall back to a resource if there's no file --- HOCON.md | 36 ++++++--- .../com/typesafe/config/impl/Parseable.java | 77 +++++++++++++------ config/src/test/resources/test07.conf | 4 + config/src/test/resources/test08.conf | 6 ++ .../com/typesafe/config/impl/ConfigTest.scala | 36 +++++++++ 5 files changed, 126 insertions(+), 33 deletions(-) create mode 100644 config/src/test/resources/test07.conf create mode 100644 config/src/test/resources/test08.conf diff --git a/HOCON.md b/HOCON.md index ec9526f5..ac92dcfe 100644 --- a/HOCON.md +++ b/HOCON.md @@ -640,22 +640,19 @@ separately for each kind of resource. Implementations may vary in the kinds of resources they support including. -For plain files on the filesystem: - - - if the included file is an absolute path then it should be kept - absolute and loaded as such. - - if the included file is a relative path, then it should be - located relative to the directory containing the including - file. The current working directory of the process parsing a - file must NOT be used when interpreting included paths. +On the Java Virtual Machine, if an include statement does not +identify anything "adjacent to" the including resource, +implementations may wish to fall back to a classpath resource. +This allows configurations found in files or URLs to access +classpath resources. For resources located on the Java classpath: - included resources are looked up by calling `getResource()` on - the same class or class loader used to look up the including - resource. + the same class loader used to look up the including resource. - if the included resource name is absolute (starts with '/') - then it should be passed to `getResource()` as-is. + then it should be passed to `getResource()` with the '/' + removed. - if the included resource name does not start with '/' then it should have the "directory" of the including resource. prepended to it, before passing it to `getResource()`. If the @@ -669,6 +666,23 @@ For resources located on the Java classpath: In other words, the "adjacent to" computation should be done on the resource name not on the resource's URL. +For plain files on the filesystem: + + - if the included file is an absolute path then it should be kept + absolute and loaded as such. + - if the included file is a relative path, then it should be + located relative to the directory containing the including + file. The current working directory of the process parsing a + file must NOT be used when interpreting included paths. + - if the file is not found, fall back to the classpath resource. + The classpath resource should not have any package name added + in front, it should be relative to the "root"; which means any + leading "/" should just be removed (absolute is the same as + relative since it's root-relative). The "/" is handled for + consistency with including resources from inside other + classpath resources, where the resource name may not be + root-relative and "/" allows specifying relative to root. + URLs: - for both filesystem files and Java resources, if the 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 20661f72..e5b67540 100644 --- a/config/src/main/java/com/typesafe/config/impl/Parseable.java +++ b/config/src/main/java/com/typesafe/config/impl/Parseable.java @@ -247,6 +247,20 @@ public abstract class Parseable implements ConfigParseable { } } + static File relativeTo(File file, String filename) { + File child = new File(filename); + + if (child.isAbsolute()) + return null; + + File parent = file.getParentFile(); + + if (parent == null) + return null; + else + return new File(parent, filename); + } + private final static class ParseableReader extends Parseable { final private Reader reader; @@ -368,18 +382,27 @@ public abstract class Parseable implements ConfigParseable { @Override ConfigParseable relativeTo(String filename) { - File f = new File(filename); - if (f.isAbsolute()) { - return newFile(f, options().setOriginDescription(null)); + File sibling; + if ((new File(filename)).isAbsolute()) { + sibling = new File(filename); } else { - try { - URL url = relativeTo(input.toURI().toURL(), filename); - if (url == null) - return null; - return newURL(url, options().setOriginDescription(null)); - } catch (MalformedURLException e) { - return null; - } + // this may return null + sibling = relativeTo(input, filename); + } + if (sibling == null) + return null; + if (sibling.exists()) { + return newFile(sibling, options().setOriginDescription(null)); + } else { + // fall back to classpath; we treat the "filename" as absolute + // (don't add a package name in front), + // if it starts with "/" then remove the "/", for consistency + // with ParseableResources.relativeTo + String resource = filename; + if (filename.startsWith("/")) + resource = filename.substring(1); + return newResources(this.getClass().getClassLoader(), resource, options() + .setOriginDescription(null)); } } @@ -461,6 +484,10 @@ public abstract class Parseable implements ConfigParseable { } static String parent(String resource) { + // the "resource" is not supposed to begin with a "/" + // because it's supposed to be the raw resource + // (ClassLoader#getResource), not the + // resource "syntax" (Class#getResource) int i = resource.lastIndexOf('/'); if (i < 0) { return null; @@ -471,18 +498,24 @@ public abstract class Parseable implements ConfigParseable { @Override ConfigParseable relativeTo(String sibling) { - // here we want to build a new resource name and let - // the class loader have it, rather than getting the - // url with getResource() and relativizing to that url. - // This is needed in case the class loader is going to - // search a classpath. - String parent = parent(resource); - if (parent == null) - return newResources(loader, sibling, options() - .setOriginDescription(null)); - else - return newResources(loader, parent + "/" + sibling, + 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)); + } else { + // here we want to build a new resource name and let + // the class loader have it, rather than getting the + // url with getResource() and relativizing to that url. + // This is needed in case the class loader is going to + // search a classpath. + String parent = parent(resource); + if (parent == null) + return newResources(loader, sibling, options().setOriginDescription(null)); + else + return newResources(loader, parent + "/" + sibling, options() + .setOriginDescription(null)); + } } @Override diff --git a/config/src/test/resources/test07.conf b/config/src/test/resources/test07.conf new file mode 100644 index 00000000..fe47a518 --- /dev/null +++ b/config/src/test/resources/test07.conf @@ -0,0 +1,4 @@ +# This file tests including a classpath resource when +# loading something as a file or as a resource + +include "test-lib.conf" diff --git a/config/src/test/resources/test08.conf b/config/src/test/resources/test08.conf new file mode 100644 index 00000000..a7d9c47f --- /dev/null +++ b/config/src/test/resources/test08.conf @@ -0,0 +1,6 @@ +# This file tests including a classpath resource when +# loading something as a file or as a resource. +# It specifies the classpath resource with a "/" in front, +# while test07 specifies it "relative" + +include "/test-lib.conf" diff --git a/config/src/test/scala/com/typesafe/config/impl/ConfigTest.scala b/config/src/test/scala/com/typesafe/config/impl/ConfigTest.scala index cf2711fd..f1b378b7 100644 --- a/config/src/test/scala/com/typesafe/config/impl/ConfigTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/ConfigTest.scala @@ -884,6 +884,42 @@ class ConfigTest extends TestUtils { assertEquals("world", conf.getString("y.hello")) } + @Test + def test07IncludingResourcesFromFiles() { + // first, check that when loading from classpath we include another classpath resource + val fromClasspath = ConfigFactory.parseResources(classOf[ConfigTest], "/test07.conf") + + assertEquals("This is to test classpath searches.", fromClasspath.getString("test-lib.description")) + + // second, check that when loading from a file it falls back to classpath + val fromFile = ConfigFactory.parseFile(resourceFile("test07.conf")) + + assertEquals("This is to test classpath searches.", fromFile.getString("test-lib.description")) + + // third, check that a file: URL is the same + val fromURL = ConfigFactory.parseURL(resourceFile("test07.conf").toURI.toURL) + + assertEquals("This is to test classpath searches.", fromURL.getString("test-lib.description")) + } + + @Test + def test08IncludingSlashPrefixedResources() { + // first, check that when loading from classpath we include another classpath resource + val fromClasspath = ConfigFactory.parseResources(classOf[ConfigTest], "/test08.conf") + + assertEquals("This is to test classpath searches.", fromClasspath.getString("test-lib.description")) + + // second, check that when loading from a file it falls back to classpath + val fromFile = ConfigFactory.parseFile(resourceFile("test08.conf")) + + assertEquals("This is to test classpath searches.", fromFile.getString("test-lib.description")) + + // third, check that a file: URL is the same + val fromURL = ConfigFactory.parseURL(resourceFile("test08.conf").toURI.toURL) + + assertEquals("This is to test classpath searches.", fromURL.getString("test-lib.description")) + } + @Test def renderRoundTrip() { for (i <- 1 to 6) {