Skip to content

Commit

Permalink
HHH-13788 Fix default IdentifierHelper case strategy
Browse files Browse the repository at this point in the history
  • Loading branch information
dreab8 committed Jun 22, 2021
1 parent 3427249 commit 455fb0c
Show file tree
Hide file tree
Showing 8 changed files with 230 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@
*/
package org.hibernate.dialect;

import java.sql.DatabaseMetaData;
import java.sql.SQLException;

import org.hibernate.engine.jdbc.env.spi.IdentifierCaseStrategy;
import org.hibernate.engine.jdbc.env.spi.IdentifierHelper;
import org.hibernate.engine.jdbc.env.spi.IdentifierHelperBuilder;

/**
* @author Vlad Mihalcea
*/
Expand All @@ -22,4 +29,24 @@ public boolean supportsRowValueConstructorSyntaxInInList() {
protected MySQLStorageEngine getDefaultMySQLStorageEngine() {
return InnoDBStorageEngine.INSTANCE;
}

@Override
public IdentifierHelper buildIdentifierHelper(IdentifierHelperBuilder builder, DatabaseMetaData dbMetaData)
throws SQLException {

if ( dbMetaData == null ) {
builder.setUnquotedCaseStrategy( IdentifierCaseStrategy.MIXED );
builder.setQuotedCaseStrategy( IdentifierCaseStrategy.MIXED );
}

// some MariaDB drivers does not return case strategy info
if ( builder.getQuotedCaseStrategy() == null ) {
builder.setQuotedCaseStrategy( IdentifierCaseStrategy.MIXED );
}
if ( builder.getUnquotedCaseStrategy() == null ) {
builder.setUnquotedCaseStrategy( IdentifierCaseStrategy.MIXED );
}

return super.buildIdentifierHelper( builder, dbMetaData );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package org.hibernate.dialect;

import java.sql.CallableStatement;
import java.sql.DatabaseMetaData;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Types;
Expand All @@ -27,6 +28,9 @@
import org.hibernate.dialect.pagination.LimitHelper;
import org.hibernate.dialect.unique.MySQLUniqueDelegate;
import org.hibernate.dialect.unique.UniqueDelegate;
import org.hibernate.engine.jdbc.env.spi.IdentifierCaseStrategy;
import org.hibernate.engine.jdbc.env.spi.IdentifierHelper;
import org.hibernate.engine.jdbc.env.spi.IdentifierHelperBuilder;
import org.hibernate.engine.spi.RowSelection;
import org.hibernate.exception.LockAcquisitionException;
import org.hibernate.exception.LockTimeoutException;
Expand Down Expand Up @@ -562,6 +566,18 @@ public JDBCException convert(SQLException sqlException, String message, String s
};
}

@Override
public IdentifierHelper buildIdentifierHelper(IdentifierHelperBuilder builder, DatabaseMetaData dbMetaData)
throws SQLException {

if ( dbMetaData == null ) {
builder.setUnquotedCaseStrategy( IdentifierCaseStrategy.MIXED );
builder.setQuotedCaseStrategy( IdentifierCaseStrategy.MIXED );
}

return super.buildIdentifierHelper( builder, dbMetaData );
}

@Override
public String getNotExpression(String expression) {
return "not (" + expression + ")";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
import org.hibernate.dialect.pagination.AbstractLimitHandler;
import org.hibernate.dialect.pagination.LimitHandler;
import org.hibernate.dialect.pagination.LimitHelper;
import org.hibernate.engine.jdbc.env.spi.IdentifierCaseStrategy;
import org.hibernate.engine.jdbc.env.spi.IdentifierHelper;
import org.hibernate.engine.jdbc.env.spi.IdentifierHelperBuilder;
import org.hibernate.engine.spi.RowSelection;
import org.hibernate.exception.LockAcquisitionException;
import org.hibernate.exception.spi.SQLExceptionConversionDelegate;
Expand Down Expand Up @@ -76,6 +79,18 @@ public boolean bindLimitParametersInReverseOrder() {
}
};

@Override
public IdentifierHelper buildIdentifierHelper(IdentifierHelperBuilder builder, DatabaseMetaData dbMetaData)
throws SQLException {

if ( dbMetaData == null ) {
builder.setUnquotedCaseStrategy( IdentifierCaseStrategy.LOWER );
builder.setQuotedCaseStrategy( IdentifierCaseStrategy.MIXED );
}

return super.buildIdentifierHelper( builder, dbMetaData );
}

/**
* Constructs a PostgreSQL81Dialect
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
*/
package org.hibernate.dialect;

import java.sql.DatabaseMetaData;
import java.sql.SQLException;
import java.sql.Types;
import java.util.Locale;

Expand All @@ -19,6 +21,9 @@
import org.hibernate.dialect.pagination.LegacyLimitHandler;
import org.hibernate.dialect.pagination.LimitHandler;
import org.hibernate.dialect.pagination.TopLimitHandler;
import org.hibernate.engine.jdbc.env.spi.IdentifierCaseStrategy;
import org.hibernate.engine.jdbc.env.spi.IdentifierHelper;
import org.hibernate.engine.jdbc.env.spi.IdentifierHelperBuilder;
import org.hibernate.type.StandardBasicTypes;
import org.hibernate.type.StringType;
import org.hibernate.type.Type;
Expand Down Expand Up @@ -107,6 +112,18 @@ public boolean useMaxForLimit() {
return true;
}

@Override
public IdentifierHelper buildIdentifierHelper(
IdentifierHelperBuilder builder, DatabaseMetaData dbMetaData) throws SQLException {

if ( dbMetaData == null ) {
builder.setUnquotedCaseStrategy( IdentifierCaseStrategy.MIXED );
builder.setQuotedCaseStrategy( IdentifierCaseStrategy.MIXED );
}

return super.buildIdentifierHelper( builder, dbMetaData );
}

@Override
public boolean supportsLimitOffset() {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ public NormalizingIdentifierHelperImpl(
this.reservedWords = reservedWords;
this.unquotedCaseStrategy = unquotedCaseStrategy == null ? IdentifierCaseStrategy.UPPER : unquotedCaseStrategy;
this.quotedCaseStrategy = quotedCaseStrategy == null ? IdentifierCaseStrategy.MIXED : quotedCaseStrategy;
if ( this.unquotedCaseStrategy == this.quotedCaseStrategy ) {
log.debugf(
"IdentifierCaseStrategy for both quoted and unquoted identifiers was set " +
"to the same strategy [%s]; that will likely lead to problems in schema update " +
"and validation if using quoted identifiers",
this.unquotedCaseStrategy.name()
);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ public class IdentifierHelperBuilder {
private boolean globallyQuoteIdentifiers = false;
private boolean skipGlobalQuotingForColumnDefinitions = false;
private boolean autoQuoteKeywords = true;
private IdentifierCaseStrategy unquotedCaseStrategy = IdentifierCaseStrategy.MIXED;
private IdentifierCaseStrategy quotedCaseStrategy = IdentifierCaseStrategy.MIXED;
private IdentifierCaseStrategy unquotedCaseStrategy;
private IdentifierCaseStrategy quotedCaseStrategy;

public static IdentifierHelperBuilder from(JdbcEnvironment jdbcEnvironment) {
return new IdentifierHelperBuilder( jdbcEnvironment );
Expand Down Expand Up @@ -193,15 +193,6 @@ public void setReservedWords(Set<String> words) {
}

public IdentifierHelper build() {
if ( unquotedCaseStrategy == quotedCaseStrategy ) {
log.debugf(
"IdentifierCaseStrategy for both quoted and unquoted identifiers was set " +
"to the same strategy [%s]; that will likely lead to problems in schema update " +
"and validation if using quoted identifiers",
unquotedCaseStrategy.name()
);
}

return new NormalizingIdentifierHelperImpl(
jdbcEnvironment,
nameQualifierSupport,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
package org.hibernate.test.schemaupdate;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.util.EnumSet;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.Id;

import org.hibernate.boot.MetadataSources;
import org.hibernate.boot.registry.StandardServiceRegistry;
import org.hibernate.boot.registry.StandardServiceRegistryBuilder;
import org.hibernate.boot.spi.MetadataImplementor;
import org.hibernate.cfg.AvailableSettings;
import org.hibernate.dialect.Dialect;
import org.hibernate.tool.hbm2ddl.SchemaExport;
import org.hibernate.tool.hbm2ddl.SchemaUpdate;
import org.hibernate.tool.hbm2ddl.SchemaValidator;
import org.hibernate.tool.schema.JdbcMetadaAccessStrategy;
import org.hibernate.tool.schema.TargetType;

import org.hibernate.testing.TestForIssue;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertThat;

@TestForIssue(jiraKey = "HHH-13788")
@RunWith(Parameterized.class)
public class SchemaUpdateWithUseJdbcMetadataDefaultsSettingToFalseAndQuotedNameTest {
@Parameterized.Parameters
public static String[] parameters() {
return new String[] {
JdbcMetadaAccessStrategy.GROUPED.toString(),
JdbcMetadaAccessStrategy.INDIVIDUALLY.toString()
};
}

@Parameterized.Parameter
public String jdbcMetadataExtractorStrategy;

private File updateOutputFile;
private File createOutputFile;
private StandardServiceRegistry ssr;
private MetadataImplementor metadata;

@Before
public void setUp() throws IOException {
createOutputFile = File.createTempFile( "create_script", ".sql" );
createOutputFile.deleteOnExit();
updateOutputFile = File.createTempFile( "update_script", ".sql" );
updateOutputFile.deleteOnExit();
ssr = new StandardServiceRegistryBuilder()
.applySetting( "hibernate.temp.use_jdbc_metadata_defaults", "false" )
.applySetting( AvailableSettings.SHOW_SQL, "true" )
.applySetting(
AvailableSettings.HBM2DDL_JDBC_METADATA_EXTRACTOR_STRATEGY,
jdbcMetadataExtractorStrategy
)
.build();

final MetadataSources metadataSources = new MetadataSources( ssr );
metadataSources.addAnnotatedClass( AnotherTestEntity.class );

metadata = (MetadataImplementor) metadataSources.buildMetadata();
metadata.validate();
}

@After
public void tearDown() {
new SchemaExport().setHaltOnError( true )
.setFormat( false )
.drop( EnumSet.of( TargetType.DATABASE ), metadata );
StandardServiceRegistryBuilder.destroy( ssr );
}

@Test
public void testSchemaUpdateDoesNotTryToRecreateExistingTables()
throws Exception {
createSchema();

new SchemaUpdate().setHaltOnError( true )
.setOutputFile( updateOutputFile.getAbsolutePath() )
.setFormat( false )
.execute( EnumSet.of( TargetType.DATABASE, TargetType.SCRIPT ), metadata );

checkNoUpdateStatementHasBeenGenerated();
}

private void checkNoUpdateStatementHasBeenGenerated() throws IOException {
final String fileContent = new String( Files.readAllBytes( updateOutputFile.toPath() ) );
assertThat(
"The update output file should be empty because the db schema had already been generated and the domain model was not modified",
fileContent,
is( "" )
);
}

private void createSchema() throws Exception {
new SchemaUpdate().setHaltOnError( true )
.setOutputFile( createOutputFile.getAbsolutePath() )
.execute( EnumSet.of( TargetType.DATABASE, TargetType.SCRIPT ), metadata );
new SchemaValidator().validate( metadata );
checkSchemaHasBeenGenerated();
}

private void checkSchemaHasBeenGenerated() throws Exception {
String fileContent = new String( Files.readAllBytes( createOutputFile.toPath() ) );
final Dialect dialect = metadata.getDatabase().getDialect();
Pattern fileContentPattern;
if ( dialect.openQuote() == '[' ) {
fileContentPattern = Pattern.compile( "create( (column|row))? table " + "\\[" + "another_test_entity" + "\\]" );
}
else {
fileContentPattern = Pattern.compile( "create( (column|row))? table " + dialect.openQuote() + "another_test_entity" + dialect
.closeQuote() );
}
Matcher fileContentMatcher = fileContentPattern.matcher( fileContent.toLowerCase() );
assertThat(
"The schema has not been correctly generated, Script file : " + fileContent.toLowerCase(),
fileContentMatcher.find(),
is( true )
);
}

@Entity(name = "`Another_Test_Entity`")
public static class AnotherTestEntity {
@Id
private Long id;

@Column(name = "`another_NAME`")
private String name;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.EnumSet;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.Id;

Expand Down Expand Up @@ -67,6 +68,7 @@ public void setUp() throws IOException {
updateOutputFile.deleteOnExit();
ssr = new StandardServiceRegistryBuilder()
.applySetting( "hibernate.temp.use_jdbc_metadata_defaults", "false" )
.applySetting( AvailableSettings.SHOW_SQL, "true" )
.applySetting(
AvailableSettings.HBM2DDL_JDBC_METADATA_EXTRACTOR_STRATEGY,
jdbcMetadataExtractorStrategy
Expand Down Expand Up @@ -120,7 +122,7 @@ private void createSchema() throws Exception {

private void checkSchemaHasBeenGenerated() throws Exception {
String fileContent = new String( Files.readAllBytes( createOutputFile.toPath() ) );
Pattern fileContentPattern = Pattern.compile( "create( (column|row))? table test_entity" );
Pattern fileContentPattern = Pattern.compile( "create( (column|row))? table my_test_entity" );
Matcher fileContentMatcher = fileContentPattern.matcher( fileContent.toLowerCase() );
assertThat(
"The schema has not been correctly generated, Script file : " + fileContent.toLowerCase(),
Expand All @@ -129,12 +131,11 @@ private void checkSchemaHasBeenGenerated() throws Exception {
);
}

@Entity(name = "test_entity")
@Entity(name = "My_Test_Entity")
public static class TestEntity {
@Id
private Long id;

private String name;
}

}

0 comments on commit 455fb0c

Please sign in to comment.