HHH-16516 rework CamelCaseToUnderscoresNamingStrategy and handle quoted identifiers

Author: gavinking

documentation/src/main/asciidoc/introduction/Advanced.adoc

@@ -582,9 +582,10 @@ We happen to not much like the naming rules defined by JPA, which specify that m
 We bet you could easily come up with a much better `ImplicitNamingStrategy` than that!
 (Hint: it should always produce legit mixed case identifiers.)
 ====
+
 [TIP]
 ====
-A popular `PhysicalNamingStrategy` produces snake case identifiers.
+The popular link:{doc-javadoc-url}org/hibernate/boot/model/naming/PhysicalNamingStrategySnakeCaseImpl.html[`PhysicalNamingStrategySnakeCaseImpl`] produces snake case identifiers.
 ====
 
 Custom naming strategies may be enabled using the configuration properties we already mentioned without much explanation back in <<minimizing>>.

hibernate-core/src/main/java/org/hibernate/boot/model/naming/CamelCaseToUnderscoresNamingStrategy.java

@@ -4,88 +4,9 @@
  */
 package org.hibernate.boot.model.naming;
 
-import java.util.Locale;
-
-import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment;
-
 /**
- * Originally copied from Spring Boot as this strategy is popular there
- * (original name is SpringPhysicalNamingStrategy).
- *
- * @author Phillip Webb
- * @author Madhura Bhave
+ * @deprecated Use {@link PhysicalNamingStrategySnakeCaseImpl}.
  */
-public class CamelCaseToUnderscoresNamingStrategy implements PhysicalNamingStrategy {
-
-	@Override
-	public Identifier toPhysicalCatalogName(Identifier logicalName, JdbcEnvironment jdbcEnvironment) {
-		return apply( logicalName, jdbcEnvironment );
-	}
-
-	@Override
-	public Identifier toPhysicalSchemaName(Identifier logicalName, JdbcEnvironment jdbcEnvironment) {
-		return apply( logicalName, jdbcEnvironment );
-	}
-
-	@Override
-	public Identifier toPhysicalTableName(Identifier logicalName, JdbcEnvironment jdbcEnvironment) {
-		return apply( logicalName, jdbcEnvironment );
-	}
-
-	@Override
-	public Identifier toPhysicalSequenceName(Identifier logicalName, JdbcEnvironment jdbcEnvironment) {
-		return apply( logicalName, jdbcEnvironment );
-	}
-
-	@Override
-	public Identifier toPhysicalColumnName(Identifier logicalName, JdbcEnvironment jdbcEnvironment) {
-		return apply( logicalName, jdbcEnvironment );
-	}
-
-	private Identifier apply(final Identifier name, final JdbcEnvironment jdbcEnvironment) {
-		if ( name == null ) {
-			return null;
-		}
-		StringBuilder builder = new StringBuilder( name.getText().replace( '.', '_' ) );
-		for ( int i = 1; i < builder.length() - 1; i++ ) {
-			if ( isUnderscoreRequired( builder.charAt( i - 1 ), builder.charAt( i ), builder.charAt( i + 1 ) ) ) {
-				builder.insert( i++, '_' );
-			}
-		}
-		return getIdentifier( builder.toString(), name.isQuoted(), jdbcEnvironment );
-	}
-
-	/**
-	 * Get an identifier for the specified details. By default this method will return an identifier
-	 * with the name adapted based on the result of {@link #isCaseInsensitive(JdbcEnvironment)}
-	 *
-	 * @param name the name of the identifier
-	 * @param quoted if the identifier is quoted
-	 * @param jdbcEnvironment the JDBC environment
-	 *
-	 * @return an identifier instance
-	 */
-	protected Identifier getIdentifier(String name, final boolean quoted, final JdbcEnvironment jdbcEnvironment) {
-		if ( isCaseInsensitive( jdbcEnvironment ) ) {
-			name = name.toLowerCase( Locale.ROOT );
-		}
-		return new Identifier( name, quoted );
-	}
-
-	/**
-	 * Specify whether the database is case sensitive.
-	 *
-	 * @param jdbcEnvironment the JDBC environment which can be used to determine case
-	 *
-	 * @return true if the database is case insensitive sensitivity
-	 */
-	protected boolean isCaseInsensitive(JdbcEnvironment jdbcEnvironment) {
-		return true;
-	}
-
-	private boolean isUnderscoreRequired(final char before, final char current, final char after) {
-		return ( Character.isLowerCase( before ) || Character.isDigit( before ) ) && Character.isUpperCase( current ) && ( Character.isLowerCase(
-				after ) || Character.isDigit( after ) );
-	}
-
+@Deprecated(since = "7", forRemoval = true)
+public class CamelCaseToUnderscoresNamingStrategy extends PhysicalNamingStrategySnakeCaseImpl {
 }

hibernate-core/src/main/java/org/hibernate/boot/model/naming/ImplicitNamingStrategyJpaCompliantImpl.java

@@ -36,7 +36,7 @@ public Identifier determinePrimaryTableName(ImplicitEntityNameSource source) {
 			throw new HibernateException( "Entity naming information was not provided." );
 		}
 
-		String tableName = transformEntityName( source.getEntityNaming() );
+		final String tableName = transformEntityName( source.getEntityNaming() );
 
 		if ( tableName == null ) {
 			// todo : add info to error message - but how to know what to write since we failed to interpret the naming source
@@ -136,9 +136,9 @@ public Identifier determineJoinColumnName(ImplicitJoinColumnNameSource source) {
 
 		// todo : we need to better account for "referencing relationship property"
 
-		final String name;
+		final String referencedColumnName = source.getReferencedColumnName().getText();
 
-		String referencedColumnName = source.getReferencedColumnName().getText();
+		final String name;
 		if ( source.getNature() == ELEMENT_COLLECTION
 				|| source.getAttributePath() == null ) {
 			name = transformEntityName( source.getEntityNaming() )

hibernate-core/src/main/java/org/hibernate/boot/model/naming/PhysicalNamingStrategySnakeCaseImpl.java

@@ -0,0 +1,84 @@
+/*
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ * Copyright Red Hat Inc. and Hibernate Authors
+ */
+package org.hibernate.boot.model.naming;
+
+import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment;
+
+import java.util.Locale;
+
+import static java.lang.Character.isDigit;
+import static java.lang.Character.isLowerCase;
+import static java.lang.Character.isUpperCase;
+
+/**
+ * Converts {@code camelCase} or {@code MixedCase} logical names to {@code snake_case}.
+ *
+ * @author Phillip Webb
+ * @author Madhura Bhave
+ */
+// Originally copied from Spring's SpringPhysicalNamingStrategy as this strategy is popular there.
+public class PhysicalNamingStrategySnakeCaseImpl implements PhysicalNamingStrategy {
+
+	@Override
+	public Identifier toPhysicalCatalogName(Identifier logicalName, JdbcEnvironment jdbcEnvironment) {
+		return apply( logicalName );
+	}
+
+	@Override
+	public Identifier toPhysicalSchemaName(Identifier logicalName, JdbcEnvironment jdbcEnvironment) {
+		return apply( logicalName );
+	}
+
+	@Override
+	public Identifier toPhysicalTableName(Identifier logicalName, JdbcEnvironment jdbcEnvironment) {
+		return apply( logicalName );
+	}
+
+	@Override
+	public Identifier toPhysicalSequenceName(Identifier logicalName, JdbcEnvironment jdbcEnvironment) {
+		return apply( logicalName );
+	}
+
+	@Override
+	public Identifier toPhysicalColumnName(Identifier logicalName, JdbcEnvironment jdbcEnvironment) {
+		return apply( logicalName );
+	}
+
+	private Identifier apply(final Identifier name) {
+		if ( name == null ) {
+			return null;
+		}
+		else if ( name.isQuoted() ) {
+			return quotedIdentifier( name );
+		}
+		else {
+			return unquotedIdentifier( name );
+		}
+	}
+
+	private String camelCaseToSnakeCase(String name) {
+		final StringBuilder builder = new StringBuilder( name.replace( '.', '_' ) );
+		for ( int i = 1; i < builder.length() - 1; i++ ) {
+			if ( isUnderscoreRequired( builder.charAt( i - 1 ), builder.charAt( i ), builder.charAt( i + 1 ) ) ) {
+				builder.insert( i++, '_' );
+			}
+		}
+		return builder.toString();
+	}
+
+	protected Identifier unquotedIdentifier(Identifier name) {
+		return new Identifier( camelCaseToSnakeCase( name.getText() ).toLowerCase( Locale.ROOT ) );
+	}
+
+	protected Identifier quotedIdentifier(Identifier quotedName) {
+		return quotedName;
+	}
+
+	private boolean isUnderscoreRequired(final char before, final char current, final char after) {
+		return ( isLowerCase( before ) || isDigit( before ) )
+			&& isUpperCase( current )
+			&& ( isLowerCase( after ) || isDigit( after ) );
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/annotations/namingstrategy/CamelCaseToUnderscoresNamingStrategyTest.java

@@ -4,12 +4,12 @@
  */
 package org.hibernate.orm.test.annotations.namingstrategy;
 
+import jakarta.persistence.Column;
 import org.hibernate.boot.Metadata;
 import org.hibernate.boot.MetadataSources;
-import org.hibernate.boot.model.naming.CamelCaseToUnderscoresNamingStrategy;
+import org.hibernate.boot.model.naming.PhysicalNamingStrategySnakeCaseImpl;
 import org.hibernate.cfg.Environment;
 import org.hibernate.mapping.PersistentClass;
-import org.hibernate.mapping.Selectable;
 import org.hibernate.service.ServiceRegistry;
 
 import org.hibernate.testing.ServiceRegistryBuilder;
@@ -50,29 +50,33 @@ public void testWithWordWithDigitNamingStrategy() throws Exception {
 		Metadata metadata = new MetadataSources( serviceRegistry )
 				.addAnnotatedClass( B.class )
 				.getMetadataBuilder()
-				.applyPhysicalNamingStrategy( new CamelCaseToUnderscoresNamingStrategy() )
+				.applyPhysicalNamingStrategy( new PhysicalNamingStrategySnakeCaseImpl() )
 				.build();
 
 		PersistentClass entityBinding = metadata.getEntityBinding( B.class.getName() );
 		assertEquals(
 				"word_with_digit_d1",
-				( (Selectable) entityBinding.getProperty( "wordWithDigitD1" ).getSelectables().get( 0 ) ).getText()
+				entityBinding.getProperty( "wordWithDigitD1" ).getSelectables().get( 0 ).getText()
 		);
 		assertEquals(
 				"abcd_efgh_i21",
-				( (Selectable) entityBinding.getProperty( "AbcdEfghI21" ).getSelectables().get( 0 ) ).getText()
+				entityBinding.getProperty( "AbcdEfghI21" ).getSelectables().get( 0 ).getText()
 		);
 		assertEquals(
 				"hello1",
-				( (Selectable) entityBinding.getProperty( "hello1" ).getSelectables().get( 0 ) ).getText()
+				entityBinding.getProperty( "hello1" ).getSelectables().get( 0 ).getText()
 		);
 		assertEquals(
 				"hello1_d2",
-				( (Selectable) entityBinding.getProperty( "hello1D2" ).getSelectables().get( 0 ) ).getText()
+				entityBinding.getProperty( "hello1D2" ).getSelectables().get( 0 ).getText()
 		);
 		assertEquals(
 				"hello3d4",
-				( (Selectable) entityBinding.getProperty( "hello3d4" ).getSelectables().get( 0 ) ).getText()
+				entityBinding.getProperty( "hello3d4" ).getSelectables().get( 0 ).getText()
+		);
+		assertEquals(
+				"Quoted-ColumnName",
+				entityBinding.getProperty( "quoted" ).getSelectables().get( 0 ).getText()
 		);
 	}
 
@@ -85,6 +89,8 @@ class B implements java.io.Serializable {
 		protected String hello1;
 		protected String hello1D2;
 		protected String hello3d4;
+		@Column(name = "\"Quoted-ColumnName\"")
+		protected String quoted;
 
 		public String getAbcdEfghI21() {
 			return AbcdEfghI21;

hibernate-core/src/test/java/org/hibernate/orm/test/annotations/onetomany/orderby/IdClassAndOrderByTest.java

@@ -4,10 +4,10 @@
  */
 package org.hibernate.orm.test.annotations.onetomany.orderby;
 
-import org.hibernate.boot.model.naming.CamelCaseToUnderscoresNamingStrategy;
 import org.hibernate.boot.model.naming.Identifier;
 import org.hibernate.boot.model.naming.ImplicitJoinTableNameSource;
 import org.hibernate.boot.model.naming.ImplicitNamingStrategyJpaCompliantImpl;
+import org.hibernate.boot.model.naming.PhysicalNamingStrategySnakeCaseImpl;
 import org.hibernate.cfg.AvailableSettings;
 
 import org.hibernate.testing.orm.junit.JiraKey;
@@ -42,7 +42,7 @@ public class IdClassAndOrderByTest {
 	public static class PhysicalNamingStrategyProvider implements SettingProvider.Provider<String> {
 		@Override
 		public String getSetting() {
-			return CamelCaseToUnderscoresNamingStrategy.class.getName();
+			return PhysicalNamingStrategySnakeCaseImpl.class.getName();
 		}
 	}
 

hibernate-core/src/test/java/org/hibernate/orm/test/namingstrategy/ejb3joincolumn/PhysicalNamingStrategyImpl.java

@@ -8,6 +8,8 @@
 import org.hibernate.boot.model.naming.PhysicalNamingStrategyStandardImpl;
 import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment;
 
+import static org.hibernate.boot.model.naming.Identifier.toIdentifier;
+
 /**
  * @author Anton Wimmer
  * @author Steve Ebersole
@@ -20,19 +22,17 @@ public class PhysicalNamingStrategyImpl extends PhysicalNamingStrategyStandardIm
 
 	@Override
 	public Identifier toPhysicalTableName(Identifier logicalName, JdbcEnvironment jdbcEnvironment) {
-		return Identifier.toIdentifier(makeCleanIdentifier("tbl_" + logicalName.getText()), logicalName.isQuoted());
+		return toIdentifier( makeCleanIdentifier("tbl_" + logicalName.getText()), logicalName.isQuoted() );
 	}
 
 	@Override
 	public Identifier toPhysicalColumnName(Identifier logicalName, JdbcEnvironment jdbcEnvironment) {
-		if ( logicalName.getText().equals("DTYPE") ) {
-			return logicalName;
-		}
-
-		return Identifier.toIdentifier(makeCleanIdentifier("c_" + logicalName.getText()), logicalName.isQuoted());
+		return logicalName.getText().equals( "DTYPE" )
+				? logicalName
+				: toIdentifier( makeCleanIdentifier( "c_" + logicalName.getText() ), logicalName.isQuoted() );
 	}
 
 	private String makeCleanIdentifier(String s) {
-		return s.substring(0, Math.min(s.length(), 63)).toLowerCase();
+		return s.substring( 0, Math.min(s.length(), 63) ).toLowerCase();
 	}
 }