From 6c419acfe188483f55ade9e4c0fce98f3bc757a7 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 `all-identifiers` in quoting options group. Now will be possible skip column definitions when enabling global quoting and automatically quote deemed keywords. --- .../HibernateOrmConfigPersistenceUnit.java | 44 +++++++++++++++++++ .../orm/deployment/HibernateOrmProcessor.java | 18 +++++++- .../JPAQuotedTestCase.java | 30 ------------- .../AbstractJPAQuotedTest.java | 20 +++++++++ .../Group.java | 21 +++++++-- .../JPAQuotedIdentifiersTest.java | 21 +++++++++ .../JPAQuotedKeywordsTest.java | 20 +++++++++ .../QuotedResource.java | 8 ++-- .../application-quoted-identifiers.properties | 3 +- .../application-quoted-keywords.properties | 7 +++ .../HibernateReactiveProcessor.java | 17 ++++++- 11 files changed, 169 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_options/AbstractJPAQuotedTest.java rename extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/{quoted_indentifiers => quoting_options}/Group.java (53%) create mode 100644 extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_options/JPAQuotedIdentifiersTest.java create mode 100644 extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_options/JPAQuotedKeywordsTest.java rename extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/{quoted_indentifiers => quoting_options}/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 84384ff7b0ce01..1866e6c079e1d7 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,13 @@ public class HibernateOrmConfigPersistenceUnit { @ConfigDocSection public HibernateOrmConfigPersistenceUnitDiscriminator discriminator; + /** + * Quoting options configuration. + */ + @ConfigItem + @ConfigDocSection + public HibernateOrmConfigPersistenceUnitQuote quote; + /** * The default in Quarkus is for 2nd level caching to be enabled, * and a good implementation is already integrated for you. @@ -259,6 +266,7 @@ public boolean isAnyPropertySet() { multitenantSchemaDatasource.isPresent() || fetch.isAnyPropertySet() || discriminator.isAnyPropertySet() || + quote.isAnyPropertySet() || !unsupportedProperties.isEmpty(); } @@ -357,8 +365,11 @@ public static class HibernateOrmConfigPersistenceUnitDatabase { /** * Whether Hibernate should quote all identifiers. + * + * @deprecated {@link #quote} should be used to configure quoting properties. */ @ConfigItem + @Deprecated public boolean globallyQuotedIdentifiers; public boolean isAnyPropertySet() { @@ -474,4 +485,37 @@ public boolean isAnyPropertySet() { return ignoreExplicitForJoined; } } + + /** + * Quoting configuration. + */ + @ConfigGroup + public static class HibernateOrmConfigPersistenceUnitQuote { + + /** + * Whether Hibernate should quote all identifiers. + */ + @ConfigItem + public boolean allIdentifiers; + + /** + * Whether Hibernate should skip column definitions. + *

+ * Useful when {@code all-identifiers} is enabled, otherwise it will be ignored. + */ + @ConfigItem + public boolean skipColumnDefinitions; + + /** + * Whether Hibernate should quote any names that are deemed keywords. + *

+ * Useful when enabling {@code all-identifiers} is inappropriate and limiting to reserved words is enough. + */ + @ConfigItem + public boolean autoKeywords; + + public boolean isAnyPropertySet() { + return allIdentifiers || skipColumnDefinitions || autoKeywords; + } + } } 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 0159dc8f6fa876..13b6ded48aacf2 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 @@ -1047,10 +1047,26 @@ private static void producePersistenceUnitDescriptorFromConfig( descriptor.getProperties().setProperty(AvailableSettings.HBM2DDL_CHARSET_NAME, persistenceUnitConfig.database.charset.name()); - if (persistenceUnitConfig.database.globallyQuotedIdentifiers) { + // Quoting options + boolean globalQuoting = persistenceUnitConfig.quote.allIdentifiers + || persistenceUnitConfig.database.globallyQuotedIdentifiers; + if (globalQuoting) { descriptor.getProperties().setProperty(AvailableSettings.GLOBALLY_QUOTED_IDENTIFIERS, "true"); } + if (globalQuoting && persistenceUnitConfig.quote.skipColumnDefinitions) { + descriptor.getProperties().setProperty( + AvailableSettings.GLOBALLY_QUOTED_IDENTIFIERS_SKIP_COLUMN_DEFINITIONS, "true"); + } else if (persistenceUnitConfig.quote.skipColumnDefinitions) { + LOG.warn("Enabling the quoting option 'skip-column-definitions' is useful when 'all-identifiers' is enabled too."); + } + + if (!globalQuoting && persistenceUnitConfig.quote.autoKeywords) { + descriptor.getProperties().setProperty(AvailableSettings.KEYWORD_AUTO_QUOTING_ENABLED, "true"); + } else if (persistenceUnitConfig.quote.autoKeywords) { + LOG.warn("Enabling the quoting option 'auto-keywords' is useful when 'all-identifiers' is disabled."); + } + // Query int batchSize = firstPresent(persistenceUnitConfig.fetch.batchSize, persistenceUnitConfig.batchFetchSize) .orElse(16); 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 68d711a3877f39..00000000000000 --- 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_options/AbstractJPAQuotedTest.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_options/AbstractJPAQuotedTest.java new file mode 100644 index 00000000000000..763ad2e4260b31 --- /dev/null +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_options/AbstractJPAQuotedTest.java @@ -0,0 +1,20 @@ +package io.quarkus.hibernate.orm.quoting_options; + +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_options/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_options/Group.java index e2e6963e138cbb..dbd6fa8f7b80fd 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_options/Group.java @@ -1,5 +1,6 @@ -package io.quarkus.hibernate.orm.quoted_indentifiers; +package io.quarkus.hibernate.orm.quoting_options; +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_options/JPAQuotedIdentifiersTest.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_options/JPAQuotedIdentifiersTest.java new file mode 100644 index 00000000000000..ab553a63921d67 --- /dev/null +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_options/JPAQuotedIdentifiersTest.java @@ -0,0 +1,21 @@ +package io.quarkus.hibernate.orm.quoting_options; + +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 situations, this test activates the quoting options {@code all-identifiers} and + * {@code skip-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_options/JPAQuotedKeywordsTest.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_options/JPAQuotedKeywordsTest.java new file mode 100644 index 00000000000000..b18c4feb46fcd0 --- /dev/null +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_options/JPAQuotedKeywordsTest.java @@ -0,0 +1,20 @@ +package io.quarkus.hibernate.orm.quoting_options; + +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 situations, this test activates the quoting option {@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_options/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_options/QuotedResource.java index cd18eb45fae485..83a64f2d99f321 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_options/QuotedResource.java @@ -1,4 +1,4 @@ -package io.quarkus.hibernate.orm.quoted_indentifiers; +package io.quarkus.hibernate.orm.quoting_options; 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 0e3ce6fc6405ca..205ce9810bdcdc 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,5 @@ 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.all-identifiers=true +quarkus.hibernate-orm.quote.skip-column-definitions=true 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 00000000000000..267a60a4238720 --- /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.auto-keywords=true 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 0fdcd1e39abeab..b38ea3fd3f3e4d 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 @@ -279,10 +279,25 @@ private static ParsedPersistenceXmlDescriptor generateReactivePersistenceUnit( desc.getProperties().setProperty(AvailableSettings.HBM2DDL_CHARSET_NAME, persistenceUnitConfig.database.charset.name()); - if (persistenceUnitConfig.database.globallyQuotedIdentifiers) { + // Quoting options + boolean globalQuoting = persistenceUnitConfig.quote.allIdentifiers + || persistenceUnitConfig.database.globallyQuotedIdentifiers; + if (globalQuoting) { desc.getProperties().setProperty(AvailableSettings.GLOBALLY_QUOTED_IDENTIFIERS, "true"); } + if (globalQuoting && persistenceUnitConfig.quote.skipColumnDefinitions) { + desc.getProperties().setProperty(AvailableSettings.GLOBALLY_QUOTED_IDENTIFIERS_SKIP_COLUMN_DEFINITIONS, "true"); + } else if (persistenceUnitConfig.quote.skipColumnDefinitions) { + LOG.warn("Enabling the quoting option 'skip-column-definitions' is useful when 'all-identifiers' is enabled too."); + } + + if (!globalQuoting && persistenceUnitConfig.quote.autoKeywords) { + desc.getProperties().setProperty(AvailableSettings.KEYWORD_AUTO_QUOTING_ENABLED, "true"); + } else if (persistenceUnitConfig.quote.autoKeywords) { + LOG.warn("Enabling the quoting option 'auto-keywords' is useful when 'all-identifiers' is disabled."); + } + // Query // TODO ideally we should align on ORM and use 16 as a default, but that would break applications // because of https://github.com/hibernate/hibernate-reactive/issues/742