Improve documentation, testing and error messages around persistence.xml and DB versions

docs/src/main/asciidoc/datasource.adoc

@@ -126,8 +126,9 @@
 == Configure datasources
 
 The following section describes the configuration for single or multiple datasources.
 For simplicity, we will reference a single datasource as the default (unnamed) datasource.
 
+[[configure-a-single-datasource]]
 === Configure a single datasource
 
 A datasource can be either a JDBC datasource, reactive, or both.

docs/src/main/asciidoc/hibernate-orm.adoc

@@ -584,17 +584,29 @@
 [[persistence-xml]]
 == Setting up and configuring Hibernate ORM with a `persistence.xml`
 
-Alternatively, you can use a `META-INF/persistence.xml` to set up Hibernate ORM.
-This is useful for:
+To set up and configure Hibernate ORM, <<hibernate-configuration-properties,using `application.properties`>> is recommended,
+but you can alternatively use a `META-INF/persistence.xml` file.
+This is mainly useful for migrating existing code to Quarkus.
 
-* migrating existing code
-* when you have relatively complex settings requiring the full flexibility of the configuration
-* or if you like it the good old way
-
-[NOTE]
+[WARNING]
 ====
-If you use a `persistence.xml`, then you cannot use the `quarkus.hibernate-orm.*` properties
-and only persistence units defined in `persistence.xml` will be taken into account.
+Using a `persistence.xml` file implies a few constraints:
+
+* Persistence units defined in `persistence.xml` always use the xref:datasource.adoc#configure-a-single-datasource[default datasource].
+* Persistence units defined in `persistence.xml` must be configured explicitly:
+Quarkus will keep injection of environment-related configuration to a minimum.
++
+In particular, Quarkus will not configure the dialect or database version automatically based on the datasource,
+so if the default configuration of Hibernate ORM doesn't suit your needs,
+you will need to include in `persistence.xml` configuration such as
+link:{hibernate-orm-docs-url}#settings-hibernate.dialect[`hibernate.dialect`]/link:{hibernate-orm-docs-url}#settings-jakarta.persistence.database-product-name[`jakarta.persistence.database-product-name`]
+and possibly link:{hibernate-orm-docs-url}#settings-jakarta.persistence.database-product-version[`jakarta.persistence.database-product-version`].
+* Using `persistence.xml` is incompatible with using `quarkus.hibernate-orm.*` properties  in `{config-file}`:
+if you mix them, Quarkus will raise an exception.
+* Developer experience may be impacted negatively when using `persistence.xml`
+compared to when <<hibernate-configuration-properties,using `application.properties`>>,
+due to unavailable features, limited guidance in the Quarkus documentation,
+and error messages providing resolution hints that cannot be applied (e.g. using `quarkus.hibernate-orm.*` properties).
 
 If your classpath contains a `persistence.xml` that you want to ignore,
 set the following configuration property:

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

@@ -0,0 +1,29 @@
+package io.quarkus.hibernate.orm;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+
+import org.apache.commons.io.IOUtils;
+
+public final class ResourceUtil {
+    private ResourceUtil() {
+    }
+
+    public static String loadResourceAndReplacePlaceholders(String resourceName, Map<String, String> placeholders) {
+        String content = null;
+        try (var stream = ResourceUtil.class.getClassLoader().getResourceAsStream(resourceName)) {
+            if (stream == null) {
+                throw new RuntimeException("Could not load '" + resourceName + "' from classpath");
+            }
+            content = IOUtils.toString(stream, StandardCharsets.UTF_8);
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
+        for (var entry : placeholders.entrySet()) {
+            content = content.replace("${" + entry.getKey() + "}", entry.getValue());
+        }
+        return content;
+    }
+}

extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/config/dialect/DbVersionInvalidPersistenceXmlTest.java

@@ -0,0 +1,71 @@
+package io.quarkus.hibernate.orm.config.dialect;
+
+import static io.quarkus.hibernate.orm.ResourceUtil.loadResourceAndReplacePlaceholders;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Map;
+
+import jakarta.inject.Inject;
+
+import org.hibernate.Session;
+import org.hibernate.SessionFactory;
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+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.hibernate.orm.runtime.config.DialectVersions;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class DbVersionInvalidPersistenceXmlTest {
+
+    private static final String ACTUAL_H2_VERSION = DialectVersions.Defaults.H2;
+    // We will set the DB version to something higher than the actual version: this is invalid.
+    private static final String CONFIGURED_DB_VERSION = "999.999";
+    static {
+        assertThat(ACTUAL_H2_VERSION)
+                .as("Test setup - we need the required version to be different from the actual one")
+                .doesNotStartWith(CONFIGURED_DB_VERSION);
+    }
+
+    private static final String ACTUAL_H2_VERSION_REPORTED;
+    private static final String CONFIGURED_DB_VERSION_REPORTED;
+    static {
+        // For some reason Hibernate ORM does not catch the actual micro version of H2 and default to 0; no big deal.
+        ACTUAL_H2_VERSION_REPORTED = ACTUAL_H2_VERSION.replaceAll("\\.[\\d]+$", ".0");
+        // For some reason Hibernate ORM infers a micro version of 0; no big deal.
+        CONFIGURED_DB_VERSION_REPORTED = CONFIGURED_DB_VERSION + ".0";
+    }
+
+    @RegisterExtension
+    static QuarkusUnitTest runner = new QuarkusUnitTest()
+            .withApplicationRoot((jar) -> jar
+                    .addClass(MyEntity.class)
+                    .addAsManifestResource(new StringAsset(loadResourceAndReplacePlaceholders(
+                            "META-INF/some-persistence-with-h2-version-placeholder.xml",
+                            Map.of("H2_VERSION", "999.999"))),
+                            "persistence.xml"))
+            .withConfigurationResource("application-datasource-only.properties")
+            .assertException(throwable -> assertThat(throwable)
+                    .rootCause()
+                    .hasMessageContainingAll(
+                            "Persistence unit 'templatePU' was configured to run with a database version"
+                                    + " of at least '" + CONFIGURED_DB_VERSION_REPORTED + "', but the actual version is '"
+                                    + ACTUAL_H2_VERSION_REPORTED + "'",
+                            "Consider upgrading your database",
+                            "Alternatively, rebuild your application with 'jakarta.persistence.database-product-version="
+                                    + ACTUAL_H2_VERSION_REPORTED + "'",
+                            "this may disable some features and/or impact performance negatively"));
+
+    @Inject
+    SessionFactory sessionFactory;
+
+    @Inject
+    Session session;
+
+    @Test
+    public void test() {
+        Assertions.fail("Bootstrap should have failed");
+    }
+}

extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/config/dialect/DbVersionValidPersistenceXmlTest.java

@@ -0,0 +1,66 @@
+package io.quarkus.hibernate.orm.config.dialect;
+
+import static io.quarkus.hibernate.orm.ResourceUtil.loadResourceAndReplacePlaceholders;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Map;
+
+import jakarta.inject.Inject;
+import jakarta.transaction.Transactional;
+
+import org.hibernate.Session;
+import org.hibernate.SessionFactory;
+import org.hibernate.engine.spi.SessionFactoryImplementor;
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.hibernate.orm.MyEntity;
+import io.quarkus.hibernate.orm.SmokeTestUtils;
+import io.quarkus.hibernate.orm.runtime.config.DialectVersions;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class DbVersionValidPersistenceXmlTest {
+
+    private static final String ACTUAL_H2_VERSION = DialectVersions.Defaults.H2;
+    private static final String CONFIGURED_DB_VERSION;
+    static {
+        // We will set the DB version to something lower than the actual version: this is valid.
+        CONFIGURED_DB_VERSION = ACTUAL_H2_VERSION.replaceAll("\\.[\\d]+\\.[\\d]+$", ".0.0");
+        assertThat(ACTUAL_H2_VERSION)
+                .as("Test setup - we need the required version to be different from the actual one")
+                .isNotEqualTo(CONFIGURED_DB_VERSION);
+    }
+
+    @RegisterExtension
+    static QuarkusUnitTest runner = new QuarkusUnitTest()
+            .withApplicationRoot((jar) -> jar
+                    .addClass(SmokeTestUtils.class)
+                    .addClass(MyEntity.class)
+                    .addAsManifestResource(new StringAsset(loadResourceAndReplacePlaceholders(
+                            "META-INF/some-persistence-with-h2-version-placeholder.xml",
+                            Map.of("H2_VERSION", CONFIGURED_DB_VERSION))),
+                            "persistence.xml"))
+            .withConfigurationResource("application-datasource-only.properties");
+
+    @Inject
+    SessionFactory sessionFactory;
+
+    @Inject
+    Session session;
+
+    @Test
+    public void dialectVersion() {
+        var dialectVersion = sessionFactory.unwrap(SessionFactoryImplementor.class).getJdbcServices().getDialect().getVersion();
+        assertThat(DialectVersions.toString(dialectVersion)).isEqualTo(CONFIGURED_DB_VERSION);
+    }
+
+    @Test
+    @Transactional
+    public void smokeTest() {
+        SmokeTestUtils.testSimplePersistRetrieveUpdateDelete(session,
+                MyEntity.class, MyEntity::new,
+                MyEntity::getId,
+                MyEntity::setName, MyEntity::getName);
+    }
+}

extensions/hibernate-orm/deployment/src/test/resources/META-INF/some-persistence-with-h2-version-placeholder.xml

@@ -0,0 +1,28 @@
+<persistence xmlns="http://xmlns.jcp.org/xml/ns/persistence"
+             xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+             xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/persistence
+             http://xmlns.jcp.org/xml/ns/persistence/persistence_2_1.xsd"
+             version="2.1">
+
+    <persistence-unit name="templatePU" transaction-type="JTA">
+
+        <description>Hibernate test case template Persistence Unit</description>
+
+        <class>io.quarkus.hibernate.orm.MyEntity</class>
+
+        <properties>
+            <property name="jakarta.persistence.database-product-name" value="H2"/>
+            <!-- This placeholder is replaced programmatically in tests -->
+            <property name="jakarta.persistence.database-product-version" value="${H2_VERSION}"/>
+
+            <!--
+                Optimistically create the tables;
+                will cause background errors being logged if they already exist,
+                but is practical to retain existing data across runs (or create as needed) -->
+            <property name="jakarta.persistence.schema-generation.database.action" value="drop-and-create"/>
+
+            <property name="jakarta.persistence.validation.mode" value="NONE"/>
+        </properties>
+
+    </persistence-unit>
+</persistence>

extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/boot/registry/PreconfiguredServiceRegistryBuilder.java

@@ -201,8 +201,8 @@ private static List<StandardServiceInitiator<?>> buildQuarkusServiceInitiatorLis
 
         // Custom one: Dialect is injected explicitly
         var recordedConfig = rs.getBuildTimeSettings().getSource();
-        serviceInitiators.add(new QuarkusRuntimeInitDialectFactoryInitiator(puName, rs.getDialect(),
-                rs.getBuildTimeSettings().getSource()));
+        serviceInitiators.add(new QuarkusRuntimeInitDialectFactoryInitiator(puName, rs.isFromPersistenceXml(),
+                rs.getDialect(), rs.getBuildTimeSettings().getSource()));
 
         // Default implementation
         serviceInitiators.add(BatchBuilderInitiator.INSTANCE);

extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/service/QuarkusRuntimeInitDialectFactory.java

@@ -8,6 +8,7 @@
 import java.util.Optional;
 
 import org.hibernate.HibernateException;
+import org.hibernate.cfg.AvailableSettings;
 import org.hibernate.dialect.DatabaseVersion;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.engine.jdbc.dialect.spi.DialectFactory;
@@ -27,16 +28,18 @@
 public class QuarkusRuntimeInitDialectFactory implements DialectFactory {
     private static final EntityManagerMessageLogger LOG = messageLogger(QuarkusRuntimeInitDialectFactory.class);
     private final String persistenceUnitName;
+    private final boolean isFromPersistenceXml;
     private final Dialect dialect;
     private final Optional<String> datasourceName;
     private final DatabaseVersion buildTimeDbVersion;
 
     private boolean triedToRetrieveDbVersion = false;
     private Optional<DatabaseVersion> actualDbVersion = Optional.empty();
 
-    public QuarkusRuntimeInitDialectFactory(String persistenceUnitName, Dialect dialect,
+    public QuarkusRuntimeInitDialectFactory(String persistenceUnitName, boolean isFromPersistenceXml, Dialect dialect,
             Optional<String> datasourceName, DatabaseVersion buildTimeDbVersion) {
         this.persistenceUnitName = persistenceUnitName;
+        this.isFromPersistenceXml = isFromPersistenceXml;
         this.dialect = dialect;
         this.datasourceName = datasourceName;
         this.buildTimeDbVersion = buildTimeDbVersion;
@@ -58,23 +61,25 @@ public void checkActualDbVersion() {
             return;
         }
         if (actualDbVersion.isPresent() && buildTimeDbVersion.isAfter(actualDbVersion.get())) {
-            throw new ConfigurationException(String.format(Locale.ROOT,
+            StringBuilder errorMessage = new StringBuilder(String.format(Locale.ROOT,
                     "Persistence unit '%1$s' was configured to run with a database version"
                             + " of at least '%2$s', but the actual version is '%3$s'."
                             + " Consider upgrading your database.",
                     persistenceUnitName,
-                    DialectVersions.toString(buildTimeDbVersion), DialectVersions.toString(actualDbVersion.get()))
-                    // It shouldn't be possible to reach this code if datasourceName is empty,
-                    // but just let's be safe...
-                    + (datasourceName.isEmpty() ? ""
-                            : String.format(Locale.ROOT,
-                                    " Alternatively, rebuild your application with"
-                                            + " '%1$s=%2$s'"
-                                            + " (but this may disable some features and/or impact performance negatively).",
-                                    DataSourceUtil.dataSourcePropertyKey(datasourceName.get(), "db-version"),
-                                    DialectVersions.toString(actualDbVersion.get()))));
+                    DialectVersions.toString(buildTimeDbVersion), DialectVersions.toString(actualDbVersion.get())));
+            // It shouldn't be possible to reach this code if datasourceName is not present,
+            // but just let's be safe...
+            if (datasourceName.isPresent()) {
+                errorMessage.append(String.format(Locale.ROOT,
+                        " Alternatively, rebuild your application with"
+                                + " '%1$s=%2$s'"
+                                + " (but this may disable some features and/or impact performance negatively).",
+                        isFromPersistenceXml ? AvailableSettings.JAKARTA_HBM2DDL_DB_VERSION
+                                : DataSourceUtil.dataSourcePropertyKey(datasourceName.get(), "db-version"),
+                        DialectVersions.toString(actualDbVersion.get())));
+            }
+            throw new ConfigurationException(errorMessage.toString());
         }
-
     }
 
     private Optional<DatabaseVersion> retrieveDbVersion(DialectResolutionInfoSource resolutionInfoSource) {

extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/service/QuarkusRuntimeInitDialectFactoryInitiator.java

@@ -14,13 +14,16 @@
 public class QuarkusRuntimeInitDialectFactoryInitiator implements StandardServiceInitiator<DialectFactory> {
 
     private final String persistenceUnitName;
+    private final boolean isFromPersistenceXml;
     private final Dialect dialect;
     private final Optional<String> datasourceName;
     private final DatabaseVersion buildTimeDbVersion;
 
-    public QuarkusRuntimeInitDialectFactoryInitiator(String persistenceUnitName, Dialect dialect,
+    public QuarkusRuntimeInitDialectFactoryInitiator(String persistenceUnitName,
+            boolean isFromPersistenceXml, Dialect dialect,
             RecordedConfig recordedConfig) {
         this.persistenceUnitName = persistenceUnitName;
+        this.isFromPersistenceXml = isFromPersistenceXml;
         this.dialect = dialect;
         this.datasourceName = recordedConfig.getDataSource();
         // We set the version from the dialect since if it wasn't provided explicitly through the `recordedConfig.getDbVersion()`
@@ -35,6 +38,7 @@ public Class<DialectFactory> getServiceInitiated() {
 
     @Override
     public DialectFactory initiateService(Map<String, Object> configurationValues, ServiceRegistryImplementor registry) {
-        return new QuarkusRuntimeInitDialectFactory(persistenceUnitName, dialect, datasourceName, buildTimeDbVersion);
+        return new QuarkusRuntimeInitDialectFactory(persistenceUnitName, isFromPersistenceXml, dialect, datasourceName,
+                buildTimeDbVersion);
     }
 }

extensions/hibernate-reactive/runtime/src/main/java/io/quarkus/hibernate/reactive/runtime/boot/registry/PreconfiguredReactiveServiceRegistryBuilder.java

@@ -202,8 +202,8 @@ private static List<StandardServiceInitiator<?>> buildQuarkusServiceInitiatorLis
         serviceInitiators.add(new QuarkusRuntimeInitDialectResolverInitiator(rs.getDialect()));
 
         // Custom one: Dialect is injected explicitly
-        serviceInitiators.add(new QuarkusRuntimeInitDialectFactoryInitiator(puName, rs.getDialect(),
-                rs.getBuildTimeSettings().getSource()));
+        serviceInitiators.add(new QuarkusRuntimeInitDialectFactoryInitiator(puName, rs.isFromPersistenceXml(),
+                rs.getDialect(), rs.getBuildTimeSettings().getSource()));
 
         // Default implementation
         serviceInitiators.add(BatchBuilderInitiator.INSTANCE);