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

@@ -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:
+On the Java Virtual Machine, if an include statement does not
-
+identify anything "adjacent to" the including resource,
- - if the included file is an absolute path then it should be kept
+implementations may wish to fall back to a classpath resource.
-   absolute and loaded as such.
+This allows configurations found in files or URLs to access
- - if the included file is a relative path, then it should be
+classpath resources.
-   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.
 
 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
+   the same class loader used to look up the including resource.
-   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

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);
+            File sibling;
-            if (f.isAbsolute()) {
+            if ((new File(filename)).isAbsolute()) {
-                return newFile(f, options().setOriginDescription(null));
+                sibling = new File(filename);
             } else {
-                try {
+                // this may return null
-                    URL url = relativeTo(input.toURI().toURL(), filename);
+                sibling = relativeTo(input, filename);
-                    if (url == null)
+            }
-                        return null;
+            if (sibling == null)
-                    return newURL(url, options().setOriginDescription(null));
+                return null;
-                } catch (MalformedURLException e) {
+            if (sibling.exists()) {
-                    return null;
+                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
+            if (sibling.startsWith("/")) {
-            // the class loader have it, rather than getting the
+                // if it starts with "/" then don't make it relative to
-            // url with getResource() and relativizing to that url.
+                // the including resource
-            // This is needed in case the class loader is going to
+                return newResources(loader, sibling.substring(1),
-            // 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));
+            } 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

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"

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"

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) {