Skip to content

Commit

Permalink
HHH-18337 - SequenceStyleGenerator not respecting physical naming str…
Browse files Browse the repository at this point in the history 
…ategy
  • Loading branch information
@sebersole
sebersole committed Sep 16, 2024
1 parent 88bcc71 commit 723e18d
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 20 deletions.
8 changes: 0 additions & 8 deletions hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyBinder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1443,14 +1443,6 @@ static void processId(
? serviceRegistry.requireService( ManagedBeanRegistry.class ).getBeanContainer()
: null;
idValue.setCustomIdGeneratorCreator( identifierGeneratorCreator( idProperty, annotation, beanContainer ) );
final Map<String,Object> parameters = new HashMap<>();
parameters.put( PersistentIdentifierGenerator.TABLE, idValue.getTable().getName() );
if ( idValue.getColumnSpan() == 1 ) {
parameters.put( PersistentIdentifierGenerator.PK, idValue.getColumns().get(0).getName() );
}
// YUCK! but cannot think of a clean way to do this given the string-config based scheme
parameters.put( PersistentIdentifierGenerator.IDENTIFIER_NORMALIZER, context.getObjectNameNormalizer() );
idValue.setIdentifierGeneratorParameters( parameters );
}
else if ( !generatorAnnotations.isEmpty() ) {
// idValue.setCustomGeneratorCreator( generatorCreator( idProperty, generatorAnnotation ) );
Expand Down
26 changes: 26 additions & 0 deletions hibernate-core/src/main/java/org/hibernate/generator/GeneratorCreationContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,41 @@
import org.hibernate.mapping.Property;
import org.hibernate.service.ServiceRegistry;

/**
* Access to information useful during {@linkplain Generator} creation and initialization.
*
* @see AnnotationBasedGenerator
* @see org.hibernate.id.Configurable#create(GeneratorCreationContext)
*/
@Incubating
public interface GeneratorCreationContext {
/**
* View of the relational database objects (tables, sequences, ...) and namespaces (catalogs and schemas).
*/
Database getDatabase();

/**
* Access to available services.
*/
ServiceRegistry getServiceRegistry();

/**
* The default catalog name, if one.
*/
String getDefaultCatalog();

/**
* The default schema name, if one.
*/
String getDefaultSchema();

/**
* Mapping details for the entity; may be null in the case of and id-bag id generator.
*/
PersistentClass getPersistentClass();

/**
* The entity identifier or id-bag property details.
*/
Property getProperty();
}
10 changes: 10 additions & 0 deletions hibernate-core/src/main/java/org/hibernate/id/Configurable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import java.util.Properties;

import org.hibernate.Incubating;
import org.hibernate.MappingException;
import org.hibernate.boot.model.relational.Database;
import org.hibernate.boot.model.relational.SqlStringGenerationContext;
Expand All @@ -18,6 +19,11 @@
/**
* A {@link org.hibernate.generator.Generator} that supports "configuration".
*
* @apiNote Prefer instead using either<ul>
* <li>{@linkplain org.hibernate.annotations.IdGeneratorType}</li>, or
* <li>{@linkplain org.hibernate.generator.AnnotationBasedGenerator} (in either implementation or constructor-injection form)</li>
* </ul>
*
* @author Gavin King
* @author Steve Ebersole
*/
Expand All @@ -27,7 +33,11 @@ public interface Configurable {
* with an instance of {@link GeneratorCreationContext}.
*
* @since 6.6
* @deprecated Added in 6.6 as a short-term work-around, but should really use on
* of the alternatives discussed at {@linkplain Configurable}.
* See <a href="https://hibernate.atlassian.net/browse/HHH-18615">HHH-18615</a>.
*/
@Deprecated(forRemoval = true)
default void create(GeneratorCreationContext creationContext) throws MappingException {}

/**
Expand Down
25 changes: 19 additions & 6 deletions hibernate-core/src/main/java/org/hibernate/id/enhanced/SequenceStyleGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.hibernate.MappingException;
import org.hibernate.boot.model.naming.Identifier;
import org.hibernate.boot.model.naming.ObjectNameNormalizer;
import org.hibernate.boot.model.naming.PhysicalNamingStrategy;
import org.hibernate.boot.model.relational.Database;
import org.hibernate.boot.model.relational.QualifiedName;
import org.hibernate.boot.model.relational.QualifiedNameParser;
Expand All @@ -27,6 +28,7 @@
import org.hibernate.engine.config.spi.ConfigurationService;
import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment;
import org.hibernate.engine.spi.SharedSessionContractImplementor;
import org.hibernate.generator.GeneratorCreationContext;
import org.hibernate.id.BulkInsertionCapableIdentifierGenerator;
import org.hibernate.id.IdentifierGenerator;
import org.hibernate.id.PersistentIdentifierGenerator;
Expand Down Expand Up @@ -164,6 +166,8 @@ public class SequenceStyleGenerator
private Optimizer optimizer;
private Type identifierType;

private PhysicalNamingStrategy physicalNamingStrategy;

/**
* Getter for property 'databaseStructure'.
*
Expand Down Expand Up @@ -195,8 +199,17 @@ public Type getIdentifierType() {

// Configurable implementation ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@Override
public void create(GeneratorCreationContext creationContext) throws MappingException {
physicalNamingStrategy = creationContext.getDatabase().getPhysicalNamingStrategy();
}

@Override
public void configure(Type type, Properties parameters, ServiceRegistry serviceRegistry) throws MappingException {
if ( physicalNamingStrategy == null ) {
throw new IllegalStateException( "Expecting prior call to #create" );
}

final JdbcEnvironment jdbcEnvironment = serviceRegistry.requireService( JdbcEnvironment.class );
final Dialect dialect = jdbcEnvironment.getDialect();

Expand All @@ -217,8 +230,7 @@ public void configure(Type type, Properties parameters, ServiceRegistry serviceR
physicalSequence,
optimizationStrategy,
serviceRegistry,
determineContributor( parameters ),
(ObjectNameNormalizer) parameters.get( IDENTIFIER_NORMALIZER )
determineContributor( parameters )
);

if ( physicalSequence
Expand All @@ -244,6 +256,9 @@ public void configure(Type type, Properties parameters, ServiceRegistry serviceR
getInt( INITIAL_PARAM, parameters, -1 )
);
this.databaseStructure.configure( optimizer );

// we don't want or need this after initialization is complete
physicalNamingStrategy = null;
}

private int adjustIncrementSize(
Expand All @@ -253,8 +268,7 @@ private int adjustIncrementSize(
boolean physicalSequence,
OptimizerDescriptor optimizationStrategy,
ServiceRegistry serviceRegistry,
String contributor,
ObjectNameNormalizer normalizer) {
String contributor) {
final ConfigurationService configurationService = serviceRegistry.requireService( ConfigurationService.class );
final SequenceMismatchStrategy sequenceMismatchStrategy = configurationService.getSetting(
AvailableSettings.SEQUENCE_INCREMENT_SIZE_MISMATCH_STRATEGY,
Expand All @@ -265,8 +279,7 @@ private int adjustIncrementSize(
if ( sequenceMismatchStrategy != SequenceMismatchStrategy.NONE
&& optimizationStrategy.isPooled()
&& physicalSequence ) {
final String databaseSequenceName = normalizer.database()
.getPhysicalNamingStrategy()
final String databaseSequenceName = physicalNamingStrategy
.toPhysicalSequenceName( sequenceName.getObjectName(), jdbcEnvironment )
.getText();
final Number databaseIncrementValue = isSchemaToBeRecreated( contributor, configurationService ) ? null : getSequenceIncrementValue( jdbcEnvironment, databaseSequenceName );
Expand Down
77 changes: 71 additions & 6 deletions ...te-core/src/test/java/org/hibernate/orm/test/id/enhanced/SequenceStyleConfigUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.hibernate.dialect.sequence.ANSISequenceSupport;
import org.hibernate.dialect.sequence.SequenceSupport;
import org.hibernate.engine.jdbc.spi.JdbcServices;
import org.hibernate.generator.GeneratorCreationContext;
import org.hibernate.id.OptimizableGenerator;
import org.hibernate.id.PersistentIdentifierGenerator;
import org.hibernate.id.enhanced.DatabaseStructure;
Expand All @@ -33,6 +34,9 @@
import org.hibernate.id.enhanced.SequenceStyleGenerator;
import org.hibernate.id.enhanced.StandardOptimizerDescriptor;
import org.hibernate.id.enhanced.TableStructure;
import org.hibernate.mapping.PersistentClass;
import org.hibernate.mapping.Property;
import org.hibernate.service.ServiceRegistry;
import org.hibernate.type.StandardBasicTypes;
import org.hibernate.type.spi.TypeConfiguration;

Expand Down Expand Up @@ -70,16 +74,18 @@ public void testDefaultedSequenceBackedConfiguration() {
() -> buildingContext.getBootstrapContext().getTypeConfiguration(),
serviceRegistry
);
Database database = new Database( buildingContext.getBuildingOptions() );
GeneratorCreationContextImpl generatorCreationContext = new GeneratorCreationContextImpl( database );
Properties props = buildGeneratorPropertiesBase( buildingContext );
SequenceStyleGenerator generator = new SequenceStyleGenerator();
generator.create( generatorCreationContext );
generator.configure(
new TypeConfiguration().getBasicTypeRegistry()
.resolve( StandardBasicTypes.LONG ),
props,
serviceRegistry
);

Database database = new Database( buildingContext.getBuildingOptions() );
generator.registerExportables( database );
generator.initialize( SqlStringGenerationContextImpl.forTests( database.getJdbcEnvironment() ) );

Expand Down Expand Up @@ -160,18 +166,22 @@ public void testDefaultOptimizerBasedOnIncrementBackedBySequence() {
() -> buildingContext.getBootstrapContext().getTypeConfiguration(),
serviceRegistry
);

Database database = new Database( buildingContext.getBuildingOptions() );
GeneratorCreationContextImpl generatorCreationContext = new GeneratorCreationContextImpl( database );

Properties props = buildGeneratorPropertiesBase( buildingContext );
props.setProperty( SequenceStyleGenerator.INCREMENT_PARAM, "10" );

SequenceStyleGenerator generator = new SequenceStyleGenerator();
generator.create( generatorCreationContext );
generator.configure(
new TypeConfiguration().getBasicTypeRegistry()
.resolve( StandardBasicTypes.LONG ),
props,
serviceRegistry
);

Database database = new Database( buildingContext.getBuildingOptions() );
generator.registerExportables( database );
generator.initialize( SqlStringGenerationContextImpl.forTests( database.getJdbcEnvironment() ) );

Expand All @@ -190,17 +200,20 @@ public void testDefaultOptimizerBasedOnIncrementBackedBySequence() {
() -> buildingContext.getBootstrapContext().getTypeConfiguration(),
serviceRegistry
);
Database database = new Database( buildingContext.getBuildingOptions() );
GeneratorCreationContextImpl generatorCreationContext = new GeneratorCreationContextImpl( database );

Properties props = buildGeneratorPropertiesBase( buildingContext );
props.setProperty( SequenceStyleGenerator.INCREMENT_PARAM, "10" );

SequenceStyleGenerator generator = new SequenceStyleGenerator();
generator.create( generatorCreationContext );
generator.configure(
new TypeConfiguration().getBasicTypeRegistry()
.resolve( StandardBasicTypes.LONG ),
props,
serviceRegistry
);
Database database = new Database( buildingContext.getBuildingOptions() );
generator.registerExportables( database );
generator.initialize( SqlStringGenerationContextImpl.forTests( database.getJdbcEnvironment() ) );

Expand All @@ -226,17 +239,20 @@ public void testDefaultOptimizerBasedOnIncrementBackedByTable() {
() -> buildingContext.getBootstrapContext().getTypeConfiguration(),
serviceRegistry
);
Database database = new Database( buildingContext.getBuildingOptions() );
GeneratorCreationContextImpl generatorCreationContext = new GeneratorCreationContextImpl( database );

Properties props = buildGeneratorPropertiesBase( buildingContext );
props.setProperty( SequenceStyleGenerator.INCREMENT_PARAM, "10" );

SequenceStyleGenerator generator = new SequenceStyleGenerator();
generator.create( generatorCreationContext );
generator.configure(
new TypeConfiguration().getBasicTypeRegistry()
.resolve( StandardBasicTypes.LONG ),
props,
serviceRegistry
);
Database database = new Database( buildingContext.getBuildingOptions() );
generator.registerExportables( database );
generator.initialize( SqlStringGenerationContextImpl.forTests( database.getJdbcEnvironment() ) );

Expand Down Expand Up @@ -285,6 +301,44 @@ public void testForceTableUse() {
}
}

private static class GeneratorCreationContextImpl implements GeneratorCreationContext {
private final Database database;

public GeneratorCreationContextImpl(Database database) {
this.database = database;
}

@Override
public Database getDatabase() {
return database;
}

@Override
public ServiceRegistry getServiceRegistry() {
return database.getServiceRegistry();
}

@Override
public String getDefaultCatalog() {
throw new UnsupportedOperationException();
}

@Override
public String getDefaultSchema() {
throw new UnsupportedOperationException();
}

@Override
public PersistentClass getPersistentClass() {
throw new UnsupportedOperationException();
}

@Override
public Property getProperty() {
throw new UnsupportedOperationException();
}
}

/**
* Test explicitly specifying both optimizer and increment
*/
Expand All @@ -302,14 +356,18 @@ public void testExplicitOptimizerWithExplicitIncrementSize() {
Properties props = buildGeneratorPropertiesBase( buildingContext );
props.setProperty( SequenceStyleGenerator.OPT_PARAM, StandardOptimizerDescriptor.NONE.getExternalName() );
props.setProperty( SequenceStyleGenerator.INCREMENT_PARAM, "20" );

Database database = new Database( buildingContext.getBuildingOptions() );
GeneratorCreationContextImpl generatorCreationContext = new GeneratorCreationContextImpl( database );

SequenceStyleGenerator generator = new SequenceStyleGenerator();
generator.create( generatorCreationContext );
generator.configure(
new TypeConfiguration().getBasicTypeRegistry()
.resolve( StandardBasicTypes.LONG ),
props,
serviceRegistry
);
Database database = new Database( buildingContext.getBuildingOptions() );
generator.registerExportables( database );
generator.initialize( SqlStringGenerationContextImpl.forTests( database.getJdbcEnvironment() ) );

Expand All @@ -323,6 +381,7 @@ public void testExplicitOptimizerWithExplicitIncrementSize() {
props.setProperty( SequenceStyleGenerator.OPT_PARAM, StandardOptimizerDescriptor.HILO.getExternalName() );
props.setProperty( SequenceStyleGenerator.INCREMENT_PARAM, "20" );
generator = new SequenceStyleGenerator();
generator.create( generatorCreationContext );
generator.configure(
new TypeConfiguration().getBasicTypeRegistry()
.resolve( StandardBasicTypes.LONG ),
Expand All @@ -341,6 +400,7 @@ public void testExplicitOptimizerWithExplicitIncrementSize() {
props.setProperty( SequenceStyleGenerator.OPT_PARAM, StandardOptimizerDescriptor.POOLED.getExternalName() );
props.setProperty( SequenceStyleGenerator.INCREMENT_PARAM, "20" );
generator = new SequenceStyleGenerator();
generator.create( generatorCreationContext );
generator.configure(
new TypeConfiguration().getBasicTypeRegistry()
.resolve( StandardBasicTypes.LONG ),
Expand Down Expand Up @@ -368,23 +428,27 @@ public void testPreferredPooledOptimizerSetting() {
() -> buildingContext.getBootstrapContext().getTypeConfiguration(),
serviceRegistry
);
Database database = new Database( buildingContext.getBuildingOptions() );
GeneratorCreationContextImpl generatorCreationContext = new GeneratorCreationContextImpl( database );

Properties props = buildGeneratorPropertiesBase( buildingContext );
props.setProperty( SequenceStyleGenerator.INCREMENT_PARAM, "20" );
SequenceStyleGenerator generator = new SequenceStyleGenerator();
generator.create( generatorCreationContext );
generator.configure(
new TypeConfiguration().getBasicTypeRegistry()
.resolve( StandardBasicTypes.LONG ),
props,
serviceRegistry
);
Database database = new Database( buildingContext.getBuildingOptions() );
generator.registerExportables( database );
generator.initialize( SqlStringGenerationContextImpl.forTests( database.getJdbcEnvironment() ) );
assertClassAssignability( SequenceStructure.class, generator.getDatabaseStructure().getClass() );
assertClassAssignability( PooledOptimizer.class, generator.getOptimizer().getClass() );

props.setProperty( Environment.PREFERRED_POOLED_OPTIMIZER, StandardOptimizerDescriptor.POOLED_LO.getExternalName() );
generator = new SequenceStyleGenerator();
generator.create( generatorCreationContext );
generator.configure(
new TypeConfiguration().getBasicTypeRegistry()
.resolve( StandardBasicTypes.LONG ),
Expand All @@ -398,6 +462,7 @@ public void testPreferredPooledOptimizerSetting() {

props.setProperty( Environment.PREFERRED_POOLED_OPTIMIZER, StandardOptimizerDescriptor.POOLED_LOTL.getExternalName() );
generator = new SequenceStyleGenerator();
generator.create( generatorCreationContext );
generator.configure(
new TypeConfiguration().getBasicTypeRegistry()
.resolve( StandardBasicTypes.LONG ),
Expand Down

0 comments on commit 723e18d

Please sign in to comment.