From f3e702b32ec06373f53bb7ee606aa71a225e89f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Leonardo=20Gon=C3=A7alves?= Date: Wed, 4 Jan 2023 18:40:58 -0300 Subject: [PATCH] Allow setting additional Hibernate quoting options Deprecate the property `globally-quoted-identifiers` indicating to use `quote-identifiers.strategy` property. It can be possible to skip column definitions when quoting identifiers or quote only the deemed keywords. --- .../HibernateOrmConfigPersistenceUnit.java | 21 +++++++++++++ .../orm/deployment/HibernateOrmProcessor.java | 12 +++++++- .../JPAQuotedTestCase.java | 30 ------------------- .../AbstractJPAQuotedTest.java | 20 +++++++++++++ .../Group.java | 21 ++++++++++--- .../JPAQuotedIdentifiersTest.java | 20 +++++++++++++ .../JPAQuotedKeywordsTest.java | 20 +++++++++++++ .../QuotedResource.java | 8 +++-- .../application-quoted-identifiers.properties | 2 +- .../application-quoted-keywords.properties | 7 +++++ .../HibernateReactiveProcessor.java | 11 ++++++- 11 files changed, 132 insertions(+), 40 deletions(-) delete mode 100644 extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoted_indentifiers/JPAQuotedTestCase.java create mode 100644 extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/AbstractJPAQuotedTest.java rename extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/{quoted_indentifiers => quoting_strategies}/Group.java (53%) create mode 100644 extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/JPAQuotedIdentifiersTest.java create mode 100644 extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/JPAQuotedKeywordsTest.java rename extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/{quoted_indentifiers => quoting_strategies}/QuotedResource.java (73%) create mode 100644 extensions/hibernate-orm/deployment/src/test/resources/application-quoted-keywords.properties diff --git a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmConfigPersistenceUnit.java b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmConfigPersistenceUnit.java index 84384ff7b0ce0..0cb783066e288 100644 --- a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmConfigPersistenceUnit.java +++ b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmConfigPersistenceUnit.java @@ -197,6 +197,16 @@ public class HibernateOrmConfigPersistenceUnit { @ConfigDocSection public HibernateOrmConfigPersistenceUnitDiscriminator discriminator; + /** + * Identifiers can be quoted using one of the available strategies. + *

+ * By default, {@code none} identifiers will be quoted. If set to {@code all}, all identifiers and column definitions will + * be quoted. Additionally, setting it to {@code all-except-column-definitions} will skip the column definitions, which can + * usually be required when they exist, or else use the option {@code only-keywords} to quote only the deemed keywords. + */ + @ConfigItem(defaultValue = "none", name = "quote-identifiers.strategy") + public IdentifierQuotingStrategy identifierQuotingStrategy; + /** * The default in Quarkus is for 2nd level caching to be enabled, * and a good implementation is already integrated for you. @@ -259,6 +269,7 @@ public boolean isAnyPropertySet() { multitenantSchemaDatasource.isPresent() || fetch.isAnyPropertySet() || discriminator.isAnyPropertySet() || + identifierQuotingStrategy != IdentifierQuotingStrategy.NONE || !unsupportedProperties.isEmpty(); } @@ -357,8 +368,11 @@ public static class HibernateOrmConfigPersistenceUnitDatabase { /** * Whether Hibernate should quote all identifiers. + * + * @deprecated {@link #identifierQuotingStrategy} should be used to configure quoting strategy. */ @ConfigItem + @Deprecated public boolean globallyQuotedIdentifiers; public boolean isAnyPropertySet() { @@ -474,4 +488,11 @@ public boolean isAnyPropertySet() { return ignoreExplicitForJoined; } } + + public enum IdentifierQuotingStrategy { + NONE, + ALL, + ALL_EXCEPT_COLUMN_DEFINITIONS, + ONLY_KEYWORDS + } } 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 0159dc8f6fa87..ae1453055c9b2 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 @@ -116,6 +116,7 @@ import io.quarkus.dev.console.DevConsoleManager; import io.quarkus.devconsole.spi.DevConsoleRuntimeTemplateInfoBuildItem; import io.quarkus.hibernate.orm.PersistenceUnit; +import io.quarkus.hibernate.orm.deployment.HibernateOrmConfigPersistenceUnit.IdentifierQuotingStrategy; import io.quarkus.hibernate.orm.deployment.integration.HibernateOrmIntegrationRuntimeConfiguredBuildItem; import io.quarkus.hibernate.orm.deployment.integration.HibernateOrmIntegrationStaticConfiguredBuildItem; import io.quarkus.hibernate.orm.deployment.spi.DatabaseKindDialectBuildItem; @@ -1047,9 +1048,18 @@ private static void producePersistenceUnitDescriptorFromConfig( descriptor.getProperties().setProperty(AvailableSettings.HBM2DDL_CHARSET_NAME, persistenceUnitConfig.database.charset.name()); - if (persistenceUnitConfig.database.globallyQuotedIdentifiers) { + // Quoting strategy + if (persistenceUnitConfig.identifierQuotingStrategy == IdentifierQuotingStrategy.ALL + || persistenceUnitConfig.identifierQuotingStrategy == IdentifierQuotingStrategy.ALL_EXCEPT_COLUMN_DEFINITIONS + || persistenceUnitConfig.database.globallyQuotedIdentifiers) { descriptor.getProperties().setProperty(AvailableSettings.GLOBALLY_QUOTED_IDENTIFIERS, "true"); } + if (persistenceUnitConfig.identifierQuotingStrategy == IdentifierQuotingStrategy.ALL_EXCEPT_COLUMN_DEFINITIONS) { + descriptor.getProperties().setProperty( + AvailableSettings.GLOBALLY_QUOTED_IDENTIFIERS_SKIP_COLUMN_DEFINITIONS, "true"); + } else if (persistenceUnitConfig.identifierQuotingStrategy == IdentifierQuotingStrategy.ONLY_KEYWORDS) { + descriptor.getProperties().setProperty(AvailableSettings.KEYWORD_AUTO_QUOTING_ENABLED, "true"); + } // Query int batchSize = firstPresent(persistenceUnitConfig.fetch.batchSize, persistenceUnitConfig.batchFetchSize) diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoted_indentifiers/JPAQuotedTestCase.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoted_indentifiers/JPAQuotedTestCase.java deleted file mode 100644 index 68d711a3877f3..0000000000000 --- a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoted_indentifiers/JPAQuotedTestCase.java +++ /dev/null @@ -1,30 +0,0 @@ -package io.quarkus.hibernate.orm.quoted_indentifiers; - -import static org.hamcrest.core.StringContains.containsString; - -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; - -import io.quarkus.test.QuarkusDevModeTest; -import io.restassured.RestAssured; - -/** - * Failed to fetch entity with reserved name. - */ -public class JPAQuotedTestCase { - - @RegisterExtension - final static QuarkusDevModeTest TEST = new QuarkusDevModeTest() - .withApplicationRoot((jar) -> jar - .addClasses(Group.class, QuotedResource.class) - .addAsResource("application-quoted-identifiers.properties", "application.properties")); - - @Test - public void testQuotedIdentifiers() { - RestAssured.when().post("/jpa-test-quoted").then() - .body(containsString("ok")); - - RestAssured.when().get("/jpa-test-quoted").then() - .body(containsString("group_name")); - } -} diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/AbstractJPAQuotedTest.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/AbstractJPAQuotedTest.java new file mode 100644 index 0000000000000..72c8795484464 --- /dev/null +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/AbstractJPAQuotedTest.java @@ -0,0 +1,20 @@ +package io.quarkus.hibernate.orm.quoting_strategies; + +import static org.hamcrest.core.StringContains.containsString; + +import org.junit.jupiter.api.Test; + +import io.restassured.RestAssured; + +public abstract class AbstractJPAQuotedTest { + + @Test + public void testQuotedIdentifiers() { + RestAssured.when().post("/jpa-test-quoted").then() + .body(containsString("ok")); + + RestAssured.when().get("/jpa-test-quoted").then() + .body(containsString("group_name"), containsString("group_value")); + } + +} diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoted_indentifiers/Group.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/Group.java similarity index 53% rename from extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoted_indentifiers/Group.java rename to extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/Group.java index e2e6963e138cb..6cabd8351cf84 100644 --- a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoted_indentifiers/Group.java +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/Group.java @@ -1,5 +1,6 @@ -package io.quarkus.hibernate.orm.quoted_indentifiers; +package io.quarkus.hibernate.orm.quoting_strategies; +import javax.persistence.Column; import javax.persistence.Entity; import javax.persistence.GeneratedValue; import javax.persistence.GenerationType; @@ -7,9 +8,10 @@ import javax.persistence.Table; /** - * Table with reserved name. - *

- * http://www.h2database.com/html/advanced.html section "Keywords / Reserved Words". + * Table with reserved name and containing one column with reserved name and column definition. + * + * @see H2 Documentation, section + * "Keywords / Reserved Words" */ @Entity @Table(name = "group") @@ -19,6 +21,8 @@ public class Group { private String name; + private String value; + @Id @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "groupSeq") public Long getId() { @@ -36,4 +40,13 @@ public String getName() { public void setName(String name) { this.name = name; } + + @Column(columnDefinition = "varchar(255)") + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } } diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/JPAQuotedIdentifiersTest.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/JPAQuotedIdentifiersTest.java new file mode 100644 index 0000000000000..607189716a5c5 --- /dev/null +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/JPAQuotedIdentifiersTest.java @@ -0,0 +1,20 @@ +package io.quarkus.hibernate.orm.quoting_strategies; + +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.test.QuarkusDevModeTest; + +/** + * Failed to fetch entity with reserved name and containing one column with reserved name and column definition. + *

+ * To resolve the simulated situation, this test uses the quoting strategy {@code all-except-column-definitions}. + */ +public class JPAQuotedIdentifiersTest extends AbstractJPAQuotedTest { + + @RegisterExtension + final static QuarkusDevModeTest TEST = new QuarkusDevModeTest() + .withApplicationRoot((jar) -> jar + .addClasses(Group.class, QuotedResource.class) + .addAsResource("application-quoted-identifiers.properties", "application.properties")); + +} diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/JPAQuotedKeywordsTest.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/JPAQuotedKeywordsTest.java new file mode 100644 index 0000000000000..e9f3e285fd3c4 --- /dev/null +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/JPAQuotedKeywordsTest.java @@ -0,0 +1,20 @@ +package io.quarkus.hibernate.orm.quoting_strategies; + +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.test.QuarkusDevModeTest; + +/** + * Failed to fetch entity with reserved name and containing one column with reserved name and column definition. + *

+ * To resolve the simulated situation, this test uses the quoting strategy {@code auto-keywords}. + */ +public class JPAQuotedKeywordsTest extends AbstractJPAQuotedTest { + + @RegisterExtension + final static QuarkusDevModeTest TEST = new QuarkusDevModeTest() + .withApplicationRoot((jar) -> jar + .addClasses(Group.class, QuotedResource.class) + .addAsResource("application-quoted-keywords.properties", "application.properties")); + +} diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoted_indentifiers/QuotedResource.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/QuotedResource.java similarity index 73% rename from extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoted_indentifiers/QuotedResource.java rename to extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/QuotedResource.java index cd18eb45fae48..35a0be4dc2cfe 100644 --- a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoted_indentifiers/QuotedResource.java +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/QuotedResource.java @@ -1,4 +1,4 @@ -package io.quarkus.hibernate.orm.quoted_indentifiers; +package io.quarkus.hibernate.orm.quoting_strategies; import javax.enterprise.context.ApplicationScoped; import javax.inject.Inject; @@ -11,7 +11,7 @@ import javax.ws.rs.core.MediaType; /** - * Try to fetch entity with reserved name. + * Try to fetch entity with reserved name and containing one column with reserved name and column definition. */ @Path("/jpa-test-quoted") @ApplicationScoped @@ -27,6 +27,7 @@ public String create() { Group group = new Group(); group.setId(1L); group.setName("group_name"); + group.setValue("group_value"); em.merge(group); return "ok"; } @@ -36,7 +37,8 @@ public String create() { @Transactional public String selectWithQuotedEntity() { try { - return em.find(Group.class, 1L).getName(); + var group = em.find(Group.class, 1L); + return group.getName() + " " + group.getValue(); } catch (Exception e) { return "Unable to fetch group."; } diff --git a/extensions/hibernate-orm/deployment/src/test/resources/application-quoted-identifiers.properties b/extensions/hibernate-orm/deployment/src/test/resources/application-quoted-identifiers.properties index 0e3ce6fc6405c..8fda8087e710e 100644 --- a/extensions/hibernate-orm/deployment/src/test/resources/application-quoted-identifiers.properties +++ b/extensions/hibernate-orm/deployment/src/test/resources/application-quoted-identifiers.properties @@ -4,4 +4,4 @@ quarkus.datasource.jdbc.url=jdbc:h2:mem:test #quarkus.hibernate-orm.log.sql=true quarkus.hibernate-orm.database.generation=drop-and-create -quarkus.hibernate-orm.database.globally-quoted-identifiers=true +quarkus.hibernate-orm.quote-identifiers.strategy=all-except-column-definitions diff --git a/extensions/hibernate-orm/deployment/src/test/resources/application-quoted-keywords.properties b/extensions/hibernate-orm/deployment/src/test/resources/application-quoted-keywords.properties new file mode 100644 index 0000000000000..b32a241ac00e9 --- /dev/null +++ b/extensions/hibernate-orm/deployment/src/test/resources/application-quoted-keywords.properties @@ -0,0 +1,7 @@ +quarkus.datasource.db-kind=h2 +quarkus.datasource.jdbc.url=jdbc:h2:mem:test + +#quarkus.hibernate-orm.log.sql=true +quarkus.hibernate-orm.database.generation=drop-and-create + +quarkus.hibernate-orm.quote-identifiers.strategy=only-keywords diff --git a/extensions/hibernate-reactive/deployment/src/main/java/io/quarkus/hibernate/reactive/deployment/HibernateReactiveProcessor.java b/extensions/hibernate-reactive/deployment/src/main/java/io/quarkus/hibernate/reactive/deployment/HibernateReactiveProcessor.java index 0fdcd1e39abea..950c40ff537a2 100644 --- a/extensions/hibernate-reactive/deployment/src/main/java/io/quarkus/hibernate/reactive/deployment/HibernateReactiveProcessor.java +++ b/extensions/hibernate-reactive/deployment/src/main/java/io/quarkus/hibernate/reactive/deployment/HibernateReactiveProcessor.java @@ -55,6 +55,7 @@ import io.quarkus.hibernate.orm.deployment.PersistenceProviderSetUpBuildItem; import io.quarkus.hibernate.orm.deployment.PersistenceUnitDescriptorBuildItem; import io.quarkus.hibernate.orm.deployment.PersistenceXmlDescriptorBuildItem; +import io.quarkus.hibernate.orm.deployment.HibernateOrmConfigPersistenceUnit.IdentifierQuotingStrategy; import io.quarkus.hibernate.orm.deployment.integration.HibernateOrmIntegrationRuntimeConfiguredBuildItem; import io.quarkus.hibernate.orm.deployment.spi.DatabaseKindDialectBuildItem; import io.quarkus.hibernate.orm.runtime.HibernateOrmRuntimeConfig; @@ -279,9 +280,17 @@ private static ParsedPersistenceXmlDescriptor generateReactivePersistenceUnit( desc.getProperties().setProperty(AvailableSettings.HBM2DDL_CHARSET_NAME, persistenceUnitConfig.database.charset.name()); - if (persistenceUnitConfig.database.globallyQuotedIdentifiers) { + // Quoting strategy + if (persistenceUnitConfig.identifierQuotingStrategy == IdentifierQuotingStrategy.ALL + || persistenceUnitConfig.identifierQuotingStrategy == IdentifierQuotingStrategy.ALL_EXCEPT_COLUMN_DEFINITIONS + || persistenceUnitConfig.database.globallyQuotedIdentifiers) { desc.getProperties().setProperty(AvailableSettings.GLOBALLY_QUOTED_IDENTIFIERS, "true"); } + if (persistenceUnitConfig.identifierQuotingStrategy == IdentifierQuotingStrategy.ALL_EXCEPT_COLUMN_DEFINITIONS) { + desc.getProperties().setProperty(AvailableSettings.GLOBALLY_QUOTED_IDENTIFIERS_SKIP_COLUMN_DEFINITIONS, "true"); + } else if (persistenceUnitConfig.identifierQuotingStrategy == IdentifierQuotingStrategy.ONLY_KEYWORDS) { + desc.getProperties().setProperty(AvailableSettings.KEYWORD_AUTO_QUOTING_ENABLED, "true"); + } // Query // TODO ideally we should align on ORM and use 16 as a default, but that would break applications