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

Avoid instantiating JPAConfig just to destroy it when static init fails #32214

Merged
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 @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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;
Expand Down