From 37795564cb133a73acf5b6d4c553b29da40ef464 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Tue, 17 Jan 2023 08:58:51 +0100 Subject: [PATCH] Actually fail on startup when configuring a persistence unit in Quarkus 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. --- .../deployment/HibernateOrmCdiProcessor.java | 5 +-- .../orm/deployment/HibernateOrmConfig.java | 4 +- .../orm/deployment/HibernateOrmProcessor.java | 19 ++++----- .../PersistenceUnitDescriptorBuildItem.java | 4 ++ .../PersistenceXmlAndQuarkusConfigTest.java | 40 +++++++++++++++++++ 5 files changed, 55 insertions(+), 17 deletions(-) create mode 100644 extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/xml/persistence/PersistenceXmlAndQuarkusConfigTest.java diff --git a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmCdiProcessor.java b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmCdiProcessor.java index d46d50e8183ec4..59f927a16125ad 100644 --- a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmCdiProcessor.java +++ b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmCdiProcessor.java @@ -110,7 +110,6 @@ public void transform(TransformationContext transformationContext) { @BuildStep void generateDataSourceBeans(HibernateOrmRecorder recorder, List persistenceUnitDescriptors, - ImpliedBlockingPersistenceUnitTypeBuildItem impliedBlockingPersistenceUnitType, List jdbcDataSources, // just make sure the datasources are initialized BuildProducer additionalBeans, BuildProducer syntheticBeanBuildItemBuildProducer) { @@ -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 diff --git a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmConfig.java b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmConfig.java index ee7a11a7d03b14..e3c43e4e600e2b 100644 --- a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmConfig.java +++ b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmConfig.java @@ -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; } diff --git a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java index b7f4897ffe712f..4c59cb6647c1b5 100644 --- a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java +++ b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java @@ -350,15 +350,7 @@ public void parsePersistenceXmlDescriptors(HibernateOrmConfig config, @BuildStep public ImpliedBlockingPersistenceUnitTypeBuildItem defineTypeOfImpliedPU( List jdbcDataSourcesBuildItem, //This is from Agroal SPI: safe to use even for Hibernate Reactive - List 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, @@ -879,11 +871,14 @@ private void handleHibernateORMWithNoPersistenceXml( BuildProducer persistenceUnitDescriptors, List 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; } diff --git a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/PersistenceUnitDescriptorBuildItem.java b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/PersistenceUnitDescriptorBuildItem.java index 6044d7711c1f54..78bc033673e6b7 100644 --- a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/PersistenceUnitDescriptorBuildItem.java +++ b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/PersistenceUnitDescriptorBuildItem.java @@ -90,6 +90,10 @@ public boolean hasXmlMappings() { return !xmlMappings.isEmpty(); } + public boolean isFromPersistenceXml() { + return fromPersistenceXml; + } + public QuarkusPersistenceUnitDefinition asOutputPersistenceUnitDefinition( List integrationStaticDescriptors) { return new QuarkusPersistenceUnitDefinition(descriptor, configurationName, dataSource, multiTenancyStrategy, diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/xml/persistence/PersistenceXmlAndQuarkusConfigTest.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/xml/persistence/PersistenceXmlAndQuarkusConfigTest.java new file mode 100644 index 00000000000000..373c4cede90fc2 --- /dev/null +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/xml/persistence/PersistenceXmlAndQuarkusConfigTest.java @@ -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"); + } + +}