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.
quarkusio · Jan 10, 2023 · ab409bf · ab409bf
1 parent 3f919e6
commit ab409bf
Show file tree

Hide file tree

Showing 11 changed files with 132 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,16 @@ public class HibernateOrmConfigPersistenceUnit {
     @ConfigDocSection
     public HibernateOrmConfigPersistenceUnitDiscriminator discriminator;
 
+    /**
+     * Identifiers can be quoted using one of the available strategies.
+     * <p>
+     * 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/...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
@@ -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/...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/...ment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/AbstractJPAQuotedTest.java b/...ment/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/...ernate/orm/quoted_indentifiers/Group.java → ...bernate/orm/quoting_strategies/Group.java b/...ernate/orm/quoted_indentifiers/Group.java → ...bernate/orm/quoting_strategies/Group.java
@@ -1,15 +1,17 @@
-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;
 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/...t/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/JPAQuotedIdentifiersTest.java b/...t/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.
+ * <p>
+ * 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/...ment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/JPAQuotedKeywordsTest.java b/...ment/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.
+ * <p>
+ * 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/...m/quoted_indentifiers/QuotedResource.java → ...rm/quoting_strategies/QuotedResource.java b/...m/quoted_indentifiers/QuotedResource.java → ...rm/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/...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,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/...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-identifiers.strategy=only-keywords
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
@@ -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