Merge pull request #23863 from yrodiere/sql-load-script-location

Clarify sql-load-script must be in the application resources

extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmConfigPersistenceUnit.java

@@ -43,8 +43,12 @@ public class HibernateOrmConfigPersistenceUnit {
 
     // @formatter:off
     /**
-     * Name of the file containing the SQL statements to execute when Hibernate ORM starts.
-     * Its default value differs depending on the Quarkus launch mode:
+     * Path to a file containing the SQL statements to execute when Hibernate ORM starts.
+     *
+     * The file is retrieved from the classpath resources,
+     * so it must be located in the resources directory (e.g. `src/main/resources`).
+     *
+     * The default value for this setting differs depending on the Quarkus launch mode:
      *
      * * In dev and test modes, it defaults to `import.sql`.
      *   Simply add an `import.sql` file in the root of your resources directory

extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java

@@ -1109,7 +1109,16 @@ private static void producePersistenceUnitDescriptorFromConfig(
 
         if (!importFiles.isEmpty()) {
             for (String importFile : importFiles) {
-                Path loadScriptPath = applicationArchivesBuildItem.getRootArchive().getChildPath(importFile);
+                Path loadScriptPath;
+                try {
+                    loadScriptPath = applicationArchivesBuildItem.getRootArchive().getChildPath(importFile);
+                } catch (RuntimeException e) {
+                    throw new ConfigurationException(
+                            "Unable to interpret path referenced in '"
+                                    + HibernateOrmConfig.puPropertyKey(persistenceUnitName, "sql-load-script") + "="
+                                    + String.join(",", persistenceUnitConfig.sqlLoadScript.get())
+                                    + "': " + e.getMessage());
+                }
 
                 if (loadScriptPath != null && !Files.isDirectory(loadScriptPath)) {
                     // enlist resource if present

extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/sql_load_script/SqlLoadScriptAbsoluteFileSystemPathTestCase.java

@@ -0,0 +1,71 @@
+package io.quarkus.hibernate.orm.sql_load_script;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.net.URISyntaxException;
+import java.net.URL;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.hibernate.orm.MyEntity;
+import io.quarkus.runtime.configuration.ConfigurationException;
+import io.quarkus.test.QuarkusUnitTest;
+
+/**
+ * Test that setting {@code quarkus.hibernate-orm.sql-load-script}
+ * to the absolute path to a resource file on the filesystem
+ * makes the build fail.
+ *
+ * The build used to run just fine because we were interpreting the "absolute" path
+ * as relative to the FS root rather than relative to the classpath root,
+ * and ended up deciding that it does exist... only to not be able to find it later and ignoring it.
+ *
+ * See https://github.com/quarkusio/quarkus/issues/23574
+ */
+public class SqlLoadScriptAbsoluteFileSystemPathTestCase {
+    private static final String sqlLoadScriptAbsolutePath;
+    private static final String escapedSqlLoadScriptAbsolutePath;
+    static {
+        // For this reproducer, we need the absolute path to a file
+        // that actually exists in src/test/resources
+        URL resource = SqlLoadScriptAbsoluteFileSystemPathTestCase.class.getResource("/import.sql");
+        Path path;
+        try {
+            path = Paths.get(resource.toURI()).toAbsolutePath();
+        } catch (URISyntaxException e) {
+            throw new IllegalStateException(e);
+        }
+        sqlLoadScriptAbsolutePath = path.toString();
+        System.out.println("Absolute filesystem path used in test: " + sqlLoadScriptAbsolutePath);
+        if (path.getFileSystem().getSeparator().equals("\\")) {
+            // "\" is a meta-character in property files, and thus it needs to be escaped for Windows paths.
+            escapedSqlLoadScriptAbsolutePath = sqlLoadScriptAbsolutePath.replace("\\", "\\\\");
+        } else {
+            escapedSqlLoadScriptAbsolutePath = sqlLoadScriptAbsolutePath;
+        }
+        System.out.println("Escaped absolute filesystem path passed to sql-load-script: " + escapedSqlLoadScriptAbsolutePath);
+    }
+
+    @RegisterExtension
+    static QuarkusUnitTest runner = new QuarkusUnitTest()
+            .withApplicationRoot((jar) -> jar
+                    .addClasses(MyEntity.class))
+            .withConfigurationResource("application.properties")
+            .overrideConfigKey("quarkus.hibernate-orm.sql-load-script", escapedSqlLoadScriptAbsolutePath)
+            .assertException(t -> assertThat(t)
+                    .isInstanceOf(ConfigurationException.class)
+                    .hasMessageContainingAll(
+                            "Unable to interpret path referenced in 'quarkus.hibernate-orm.sql-load-script="
+                                    + sqlLoadScriptAbsolutePath + "'",
+                            "Expected a path relative to the root of the path tree"));
+
+    @Test
+    public void testSqlLoadScriptAbsolutePath() {
+        // deployment exception should happen first
+        Assertions.fail();
+    }
+}

extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/sql_load_script/SqlLoadScriptAbsoluteFileSystemPathUnescapedOnWindowsTestCase.java

@@ -0,0 +1,73 @@
+package io.quarkus.hibernate.orm.sql_load_script;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.net.URISyntaxException;
+import java.net.URL;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.hibernate.orm.MyEntity;
+import io.quarkus.runtime.configuration.ConfigurationException;
+import io.quarkus.test.QuarkusUnitTest;
+
+/**
+ * Test that setting {@code quarkus.hibernate-orm.sql-load-script}
+ * to the absolute path to a resource file on the filesystem on Windows,
+ * while also forgetting about the fact that backslashes need to be escaped in properties files,
+ * makes the build fail.
+ *
+ * Added while working on https://github.com/quarkusio/quarkus/issues/23574,
+ * because we noticed such paths cannot be correctly detected as being absolute
+ * (they cannot be distinguished from a weird relative path to a file starting with "C:" and not containing any backslash).
+ */
+@EnabledOnOs(OS.WINDOWS)
+public class SqlLoadScriptAbsoluteFileSystemPathUnescapedOnWindowsTestCase {
+    private static final String sqlLoadScriptAbsolutePath;
+    static {
+        // For this reproducer, we need the absolute path to a file
+        // that actually exists in src/test/resources
+        URL resource = SqlLoadScriptAbsoluteFileSystemPathUnescapedOnWindowsTestCase.class.getResource("/import.sql");
+        Path path;
+        try {
+            path = Paths.get(resource.toURI()).toAbsolutePath();
+        } catch (URISyntaxException e) {
+            throw new IllegalStateException(e);
+        }
+        System.out.println("Absolute filesystem path used in test: " + path);
+        // This path will contain "\",
+        // which is a meta-character and will be stripped by Quarkus when parsing the properties file,
+        // resulting in the path being interpreted wrongly.
+        // That's exactly what we want: we want to check that a user forgetting to escape backslashes
+        // in a Windows path in a properties file will still get an error messsage,
+        // even though it's not that clear.
+        sqlLoadScriptAbsolutePath = path.toString();
+        System.out.println("(Unescaped) absolute filesystem path passed to sql-load-script: " + sqlLoadScriptAbsolutePath);
+    }
+
+    @RegisterExtension
+    static QuarkusUnitTest runner = new QuarkusUnitTest()
+            .withApplicationRoot((jar) -> jar
+                    .addClasses(MyEntity.class))
+            .withConfigurationResource("application.properties")
+            .overrideConfigKey("quarkus.hibernate-orm.sql-load-script", sqlLoadScriptAbsolutePath)
+            .assertException(t -> assertThat(t)
+                    .isInstanceOf(ConfigurationException.class)
+                    .hasMessageContainingAll("Unable to find file referenced in 'quarkus.hibernate-orm.sql-load-script="
+                            // The path will appear without the backslashes in the error message;
+                            // hopefully that'll be enough to hint at what went wrong.
+                            + sqlLoadScriptAbsolutePath.replace("\\", "") + "'",
+                            "Remove property or add file to your path"));
+
+    @Test
+    public void testSqlLoadScriptAbsolutePath() {
+        // deployment exception should happen first
+        Assertions.fail();
+    }
+}

independent-projects/bootstrap/app-model/src/test/java/io/quarkus/paths/DirectoryPathTreeTest.java

@@ -59,12 +59,25 @@ public void acceptNonExistentPath() throws Exception {
     }
 
     @Test
-    public void acceptAbsolutePath() throws Exception {
+    public void acceptUnixAbsolutePath() throws Exception {
         final Path root = resolveTreeRoot("root");
         final PathTree tree = PathTree.ofDirectoryOrArchive(root);
         try {
             tree.accept("/README.md", visit -> {
-                fail("Absolute paths aren't allwed");
+                fail("Absolute paths aren't allowed");
+            });
+        } catch (IllegalArgumentException e) {
+            // expected
+        }
+    }
+
+    @Test
+    public void acceptOSSpecificAbsolutePath() throws Exception {
+        final Path root = resolveTreeRoot("root");
+        final PathTree tree = PathTree.ofDirectoryOrArchive(root);
+        try {
+            tree.accept(root.resolve("README.md").toAbsolutePath().toString(), visit -> {
+                fail("Absolute paths aren't allowed");
             });
         } catch (IllegalArgumentException e) {
             // expected
@@ -79,7 +92,7 @@ public void acceptIllegalAbsolutePathOutsideTree() throws Exception {
         assertThat(absolute).exists();
         try {
             tree.accept(absolute.toString(), visit -> {
-                fail("Absolute paths aren't allwed");
+                fail("Absolute paths aren't allowed");
             });
         } catch (IllegalArgumentException e) {
             // expected

independent-projects/bootstrap/app-model/src/test/java/io/quarkus/paths/JarPathTreeTest.java

@@ -61,14 +61,26 @@ public void acceptNonExistentPath() throws Exception {
     }
 
     @Test
-    public void acceptAbsolutePath() throws Exception {
+    public void acceptUnixAbsolutePath() throws Exception {
         final PathTree tree = PathTree.ofDirectoryOrArchive(root);
         try {
             tree.accept("/README.md", visit -> {
                 fail("Absolute paths aren't allowed");
             });
         } catch (IllegalArgumentException e) {
-            //expected
+            // expected
+        }
+    }
+
+    @Test
+    public void acceptOSSpecificAbsolutePath() throws Exception {
+        final PathTree tree = PathTree.ofDirectoryOrArchive(root);
+        try {
+            tree.accept(root.resolve("README.md").toAbsolutePath().toString(), visit -> {
+                fail("Absolute paths aren't allowed");
+            });
+        } catch (IllegalArgumentException e) {
+            // expected
         }
     }