Skip to content

Commit

Permalink
Actually fail on startup when configuring a persistence unit in Quark…
Browse files Browse the repository at this point in the history
…us config with a persistence.xml in the classpath

Some code dealing with implied PUs was preventing that failure from even
happening...

Also note we cannot catch *every* situation where we should fail, but we can try our best.
  • Loading branch information
yrodiere committed Jan 17, 2023
1 parent 832c06f commit 3779556
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ public void transform(TransformationContext transformationContext) {
@BuildStep
void generateDataSourceBeans(HibernateOrmRecorder recorder,
List<PersistenceUnitDescriptorBuildItem> persistenceUnitDescriptors,
ImpliedBlockingPersistenceUnitTypeBuildItem impliedBlockingPersistenceUnitType,
List<JdbcDataSourceBuildItem> jdbcDataSources, // just make sure the datasources are initialized
BuildProducer<AdditionalBeanBuildItem> additionalBeans,
BuildProducer<SyntheticBeanBuildItem> syntheticBeanBuildItemBuildProducer) {
Expand All @@ -120,9 +119,7 @@ void generateDataSourceBeans(HibernateOrmRecorder recorder,
}

// we have only one persistence unit defined in a persistence.xml: we make it the default even if it has a name
if (persistenceUnitDescriptors.size() == 1
&& !impliedBlockingPersistenceUnitType.shouldGenerateImpliedBlockingPersistenceUnit()) {

if (persistenceUnitDescriptors.size() == 1 && persistenceUnitDescriptors.get(0).isFromPersistenceXml()) {
String persistenceUnitName = persistenceUnitDescriptors.get(0).getPersistenceUnitName();

syntheticBeanBuildItemBuildProducer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,13 @@ public class HibernateOrmConfig {
@ConfigItem(name = "metrics.enabled")
public boolean metricsEnabled;

public boolean isAnyPropertySet() {
public boolean isAnyNonPersistenceXmlPropertySet() {
// Do NOT include persistenceXml in here.
return defaultPersistenceUnit.isAnyPropertySet() ||
!persistenceUnits.isEmpty() ||
log.isAnyPropertySet() ||
statistics.isPresent() ||
logSessionMetrics.isPresent() ||
metricsEnabled;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -350,15 +350,7 @@ public void parsePersistenceXmlDescriptors(HibernateOrmConfig config,
@BuildStep
public ImpliedBlockingPersistenceUnitTypeBuildItem defineTypeOfImpliedPU(
List<JdbcDataSourceBuildItem> jdbcDataSourcesBuildItem, //This is from Agroal SPI: safe to use even for Hibernate Reactive
List<PersistenceXmlDescriptorBuildItem> actualXmlDescriptors,
Capabilities capabilities) {

//We won't generate an implied PU if there are explicitly configured PUs
if (actualXmlDescriptors.isEmpty() == false) {
//when we have any explicitly provided Persistence Unit, disable the automatically generated ones.
return ImpliedBlockingPersistenceUnitTypeBuildItem.none();
}

// If we have some blocking datasources defined, we can have an implied PU
if (jdbcDataSourcesBuildItem.size() == 0 && capabilities.isPresent(Capability.HIBERNATE_REACTIVE)) {
// if we don't have any blocking datasources and Hibernate Reactive is present,
Expand Down Expand Up @@ -879,11 +871,14 @@ private void handleHibernateORMWithNoPersistenceXml(
BuildProducer<PersistenceUnitDescriptorBuildItem> persistenceUnitDescriptors,
List<DatabaseKindDialectBuildItem> dbKindMetadataBuildItems) {
if (!descriptors.isEmpty()) {
if (hibernateOrmConfig.isAnyPropertySet() || !hibernateOrmConfig.persistenceUnits.isEmpty()) {
if (hibernateOrmConfig.isAnyNonPersistenceXmlPropertySet() || !hibernateOrmConfig.persistenceUnits.isEmpty()) {
throw new ConfigurationException(
"Hibernate ORM configuration present in persistence.xml and Quarkus config file at the same time\n"
+ "If you use persistence.xml remove all " + HIBERNATE_ORM_CONFIG_PREFIX
+ "* properties from the Quarkus config file.");
"A legacy persistence.xml file is present in the classpath, but Hibernate ORM is also configured through the Quarkus config file.\n"
+ "Legacy persistence.xml files and Quarkus configuration cannot be used at the same time.\n"
+ "To ignore persistence.xml files, set the configuration property"
+ " 'quarkus.hibernate-orm.persistence-xml.ignore' to 'true'.\n"
+ "To use persistence.xml files, remove all '" + HIBERNATE_ORM_CONFIG_PREFIX
+ "*' properties from the Quarkus config file.");
} else {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ public boolean hasXmlMappings() {
return !xmlMappings.isEmpty();
}

public boolean isFromPersistenceXml() {
return fromPersistenceXml;
}

public QuarkusPersistenceUnitDefinition asOutputPersistenceUnitDefinition(
List<HibernateOrmIntegrationStaticDescriptor> integrationStaticDescriptors) {
return new QuarkusPersistenceUnitDefinition(descriptor, configurationName, dataSource, multiTenancyStrategy,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package io.quarkus.hibernate.orm.xml.persistence;

import org.assertj.core.api.Assertions;
import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.runtime.configuration.ConfigurationException;
import io.quarkus.test.QuarkusUnitTest;

public class PersistenceXmlAndQuarkusConfigTest {

@RegisterExtension
static QuarkusUnitTest runner = new QuarkusUnitTest()
.assertException(e -> Assertions.assertThat(e)
.isInstanceOf(ConfigurationException.class)
.hasMessageContainingAll(
"A legacy persistence.xml file is present in the classpath, but Hibernate ORM is also configured through the Quarkus config file",
"Legacy persistence.xml files and Quarkus configuration cannot be used at the same time",
"To ignore persistence.xml files, set the configuration property 'quarkus.hibernate-orm.persistence-xml.ignore' to 'true'",
"To use persistence.xml files, remove all 'quarkus.hibernate-orm.*' properties from the Quarkus config file."))
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClass(MyEntity.class)
.addAsManifestResource("META-INF/some-persistence.xml", "persistence.xml"))
.withConfigurationResource("application.properties")
// Unfortunately the minimal config is not enough to detect that a PU is configured in Quarkus's application.properties
// -- we're paying the price of our "zero config" approach.
// We can only detect something is wrong if some optional properties are set.
.overrideConfigKey("quarkus.hibernate-orm.fetch.max-depth", "2");

@Test
public void test() {
// should not be called, deployment exception should happen first:
// it's illegal to have Hibernate configuration properties in both the
// application.properties and in the persistence.xml
Assertions.fail("Application should not start");
}

}

0 comments on commit 3779556

Please sign in to comment.