Merge pull request #32214 from yrodiere/jpaconfig-instantiation-on-st…

…atic-init-failure

Avoid instantiating JPAConfig just to destroy it when static init fails

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

@@ -7,18 +7,24 @@
 import java.util.stream.Collectors;
 
 import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
 import jakarta.enterprise.inject.Default;
+import jakarta.enterprise.inject.Instance;
+import jakarta.inject.Singleton;
 
 import org.hibernate.Session;
 import org.hibernate.SessionFactory;
 import org.hibernate.StatelessSession;
+import org.jboss.jandex.AnnotationInstance;
 import org.jboss.jandex.AnnotationTarget.Kind;
 import org.jboss.jandex.AnnotationValue;
 import org.jboss.jandex.ClassType;
 import org.jboss.jandex.DotName;
 import org.jboss.jandex.FieldInfo;
+import org.jboss.jandex.ParameterizedType;
 import org.jboss.jandex.Type;
 
+import io.quarkus.agroal.runtime.DataSources;
 import io.quarkus.agroal.spi.JdbcDataSourceBuildItem;
 import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
 import io.quarkus.arc.deployment.AnnotationsTransformerBuildItem;
@@ -38,6 +44,7 @@
 import io.quarkus.deployment.annotations.Record;
 import io.quarkus.hibernate.orm.PersistenceUnit;
 import io.quarkus.hibernate.orm.runtime.HibernateOrmRecorder;
+import io.quarkus.hibernate.orm.runtime.HibernateOrmRuntimeConfig;
 import io.quarkus.hibernate.orm.runtime.JPAConfig;
 import io.quarkus.hibernate.orm.runtime.PersistenceUnitUtil;
 import io.quarkus.hibernate.orm.runtime.TransactionSessions;
@@ -112,6 +119,40 @@ public void transform(TransformationContext transformationContext) {
         return new AnnotationsTransformerBuildItem(transformer);
     }
 
+    @BuildStep
+    @Record(ExecutionTime.RUNTIME_INIT)
+    void generateJpaConfigBean(HibernateOrmRecorder recorder,
+            Capabilities capabilities,
+            HibernateOrmRuntimeConfig hibernateOrmRuntimeConfig,
+            BuildProducer<SyntheticBeanBuildItem> syntheticBeanBuildItemBuildProducer) {
+        ExtendedBeanConfigurator configurator = SyntheticBeanBuildItem
+                .configure(JPAConfig.class)
+                .addType(JPAConfig.class)
+                .scope(Singleton.class)
+                .unremovable()
+                .setRuntimeInit()
+                .supplier(recorder.jpaConfigSupplier(hibernateOrmRuntimeConfig))
+                .destroyer(JPAConfig.Destroyer.class);
+
+        // Add a synthetic dependency from JPAConfig to any datasource/pool,
+        // so that JPAConfig is destroyed before the datasource/pool.
+        // The alternative would be adding an application destruction observer
+        // (@Observes @BeforeDestroyed(ApplicationScoped.class)) to JPAConfig,
+        // but that would force initialization of JPAConfig upon application shutdown,
+        // which may cause cascading failures if the shutdown happened before JPAConfig was initialized.
+        if (capabilities.isPresent(Capability.HIBERNATE_REACTIVE)) {
+            configurator.addInjectionPoint(ParameterizedType.create(DotName.createSimple(Instance.class),
+                    new Type[] { ClassType.create(DotName.createSimple("io.vertx.sqlclient.Pool")) }, null),
+                    AnnotationInstance.builder(Any.class).build());
+        } else {
+            configurator.addInjectionPoint(ParameterizedType.create(DotName.createSimple(Instance.class),
+                    new Type[] { ClassType.create(DotName.createSimple(DataSources.class)) }, null),
+                    AnnotationInstance.builder(Any.class).build());
+        }
+
+        syntheticBeanBuildItemBuildProducer.produce(configurator.done());
+    }
+
     // These beans must be initialized at runtime because their initialization
     // depends on runtime configuration (to activate/deactivate a persistence unit)
     @Record(ExecutionTime.RUNTIME_INIT)

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

@@ -117,7 +117,6 @@
 import io.quarkus.hibernate.orm.deployment.spi.DatabaseKindDialectBuildItem;
 import io.quarkus.hibernate.orm.runtime.HibernateOrmRecorder;
 import io.quarkus.hibernate.orm.runtime.HibernateOrmRuntimeConfig;
-import io.quarkus.hibernate.orm.runtime.JPAConfig;
 import io.quarkus.hibernate.orm.runtime.PersistenceUnitUtil;
 import io.quarkus.hibernate.orm.runtime.RequestScopedSessionHolder;
 import io.quarkus.hibernate.orm.runtime.RequestScopedStatelessSessionHolder;
@@ -589,7 +588,6 @@ void registerBeans(HibernateOrmConfig hibernateOrmConfig,
         }
 
         List<Class<?>> unremovableClasses = new ArrayList<>();
-        unremovableClasses.add(JPAConfig.class);
         if (capabilities.isPresent(Capability.TRANSACTIONS)) {
             unremovableClasses.add(TransactionManager.class);
             unremovableClasses.add(TransactionSessions.class);

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

@@ -0,0 +1,66 @@
+package io.quarkus.hibernate.orm.boot;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.GeneratedValue;
+import jakarta.persistence.Id;
+
+import org.hibernate.annotations.SQLDeleteAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.test.QuarkusUnitTest;
+
+/**
+ * Checks that a failure during static init is correctly propagated and doesn't trigger other, cascading failures.
+ */
+public class StaticInitFailureTest {
+
+    @RegisterExtension
+    static QuarkusUnitTest runner = new QuarkusUnitTest()
+            .withApplicationRoot(jar -> jar
+                    .addClass(EntityWithIncorrectMapping.class))
+            .withConfigurationResource("application.properties")
+            // Expect only one error: the one we triggered
+            .setLogRecordPredicate(record -> record.getLevel().intValue() >= Level.WARNING.intValue())
+            // In particular we don't want a log telling us JPAConfig could not be created
+            // because HibernateOrmRuntimeConfig is not initialized yet.
+            // JPAConfig should not be created in the first place!
+            // See https://github.com/quarkusio/quarkus/issues/32188#issuecomment-1488037517
+            .assertLogRecords(records -> assertThat(records).extracting(LogRecord::getMessage).isEmpty())
+            .assertException(throwable -> assertThat(throwable)
+                    .hasNoSuppressedExceptions()
+                    .rootCause()
+                    .hasMessageContaining("@SQLDeleteAll")
+                    .hasNoSuppressedExceptions());
+
+    @Test
+    public void test() {
+        Assertions.fail("Startup should have failed");
+    }
+
+    @Entity(name = "myentity")
+    @SQLDeleteAll(sql = "foo") // Triggers a failure because this annotation shouldn't be applied to entities
+    public static class EntityWithIncorrectMapping {
+        private long id;
+
+        public EntityWithIncorrectMapping() {
+        }
+
+        @Id
+        @GeneratedValue
+        public long getId() {
+            return id;
+        }
+
+        public void setId(long id) {
+            this.id = id;
+        }
+    }
+
+}

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

@@ -93,6 +93,10 @@ public DataSourceTenantConnectionResolver get() {
         };
     }
 
+    public Supplier<JPAConfig> jpaConfigSupplier(HibernateOrmRuntimeConfig config) {
+        return () -> new JPAConfig(config);
+    }
+
     public void startAllPersistenceUnits(BeanContainer beanContainer) {
         beanContainer.beanInstance(JPAConfig.class).startAll();
     }

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

@@ -10,20 +10,16 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 
-import jakarta.annotation.PreDestroy;
-import jakarta.enterprise.context.ApplicationScoped;
-import jakarta.enterprise.context.BeforeDestroyed;
-import jakarta.enterprise.event.Observes;
+import jakarta.enterprise.context.spi.CreationalContext;
 import jakarta.inject.Inject;
-import jakarta.inject.Singleton;
 import jakarta.persistence.EntityManagerFactory;
 import jakarta.persistence.Persistence;
 
 import org.jboss.logging.Logger;
 
+import io.quarkus.arc.BeanDestroyer;
 import io.quarkus.hibernate.orm.runtime.boot.RuntimePersistenceUnitDescriptor;
 
-@Singleton
 public class JPAConfig {
 
     private static final Logger LOGGER = Logger.getLogger(JPAConfig.class.getName());
@@ -126,27 +122,20 @@ public Set<String> getDeactivatedPersistenceUnitNames() {
         return deactivatedPersistenceUnitNames;
     }
 
-    /**
-     * Need to shut down all instances of Hibernate ORM before the actual destroy event,
-     * as it might need to use the datasources during shutdown.
-     *
-     * @param event ignored
-     */
-    void destroy(@Observes @BeforeDestroyed(ApplicationScoped.class) Object event) {
-        for (LazyPersistenceUnit factory : persistenceUnits.values()) {
-            try {
-                factory.close();
-            } catch (Exception e) {
-                LOGGER.warn("Unable to close the EntityManagerFactory: " + factory, e);
+    public static class Destroyer implements BeanDestroyer<JPAConfig> {
+        @Override
+        public void destroy(JPAConfig instance, CreationalContext<JPAConfig> creationalContext, Map<String, Object> params) {
+            for (LazyPersistenceUnit factory : instance.persistenceUnits.values()) {
+                try {
+                    factory.close();
+                } catch (Exception e) {
+                    LOGGER.warn("Unable to close the EntityManagerFactory: " + factory, e);
+                }
             }
+            instance.persistenceUnits.clear();
         }
     }
 
-    @PreDestroy
-    void destroy() {
-        persistenceUnits.clear();
-    }
-
     static final class LazyPersistenceUnit {
 
         private final String name;