Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Feb 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down