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.
quarkusio · Jan 9, 2023 · 6c419ac · 6c419ac
1 parent 7dee541
commit 6c419ac
Show file tree

Hide file tree

Showing 11 changed files with 169 additions and 40 deletions.
diff --git a/.../src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmConfigPersistenceUnit.java b/.../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.
+         * <p>
+         * 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.
+         * <p>
+         * 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/...m/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java b/...m/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/...loyment/src/test/java/io/quarkus/hibernate/orm/quoted_indentifiers/JPAQuotedTestCase.java b/...loyment/src/test/java/io/quarkus/hibernate/orm/quoted_indentifiers/JPAQuotedTestCase.java
diff --git a/...loyment/src/test/java/io/quarkus/hibernate/orm/quoting_options/AbstractJPAQuotedTest.java b/...loyment/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/...ernate/orm/quoted_indentifiers/Group.java → .../hibernate/orm/quoting_options/Group.java b/...ernate/orm/quoted_indentifiers/Group.java → .../hibernate/orm/quoting_options/Group.java
@@ -1,15 +1,17 @@
-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;
 import javax.persistence.Id;
 import javax.persistence.Table;
 
 /**
- * Table with reserved name.
- * <p>
- * 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 <a href="http://www.h2database.com/html/advanced.html#keywords">H2 Documentation, section
+ *      "Keywords / Reserved Words"</a>
  */
 @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/...ment/src/test/java/io/quarkus/hibernate/orm/quoting_options/JPAQuotedIdentifiersTest.java b/...ment/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.
+ * <p>
+ * 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/...loyment/src/test/java/io/quarkus/hibernate/orm/quoting_options/JPAQuotedKeywordsTest.java b/...loyment/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.
+ * <p>
+ * 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/...m/quoted_indentifiers/QuotedResource.java → ...e/orm/quoting_options/QuotedResource.java b/...m/quoted_indentifiers/QuotedResource.java → ...e/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/...ons/hibernate-orm/deployment/src/test/resources/application-quoted-identifiers.properties b/...ons/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/...nsions/hibernate-orm/deployment/src/test/resources/application-quoted-keywords.properties b/...nsions/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/...nt/src/main/java/io/quarkus/hibernate/reactive/deployment/HibernateReactiveProcessor.java b/...nt/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