From 723e18d30addcba159211f1fb4130759b08012d4 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Mon, 16 Sep 2024 13:57:53 -0500 Subject: [PATCH] HHH-18337 - SequenceStyleGenerator not respecting physical naming strategy --- .../boot/model/internal/PropertyBinder.java | 8 -- .../generator/GeneratorCreationContext.java | 26 +++++++ .../java/org/hibernate/id/Configurable.java | 10 +++ .../id/enhanced/SequenceStyleGenerator.java | 25 ++++-- .../enhanced/SequenceStyleConfigUnitTest.java | 77 +++++++++++++++++-- 5 files changed, 126 insertions(+), 20 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyBinder.java index 73a45c935e..df494300e3 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyBinder.java @@ -1443,14 +1443,6 @@ public class PropertyBinder { ? serviceRegistry.requireService( ManagedBeanRegistry.class ).getBeanContainer() : null; idValue.setCustomIdGeneratorCreator( identifierGeneratorCreator( idProperty, annotation, beanContainer ) ); - final Map 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 ) ); diff --git a/hibernate-core/src/main/java/org/hibernate/generator/GeneratorCreationContext.java b/hibernate-core/src/main/java/org/hibernate/generator/GeneratorCreationContext.java index 094f0c1e2c..e47b146ad8 100644 --- a/hibernate-core/src/main/java/org/hibernate/generator/GeneratorCreationContext.java +++ b/hibernate-core/src/main/java/org/hibernate/generator/GeneratorCreationContext.java @@ -12,15 +12,41 @@ import org.hibernate.mapping.PersistentClass; 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(); } diff --git a/hibernate-core/src/main/java/org/hibernate/id/Configurable.java b/hibernate-core/src/main/java/org/hibernate/id/Configurable.java index 34bfadad6f..b43accdcb2 100644 --- a/hibernate-core/src/main/java/org/hibernate/id/Configurable.java +++ b/hibernate-core/src/main/java/org/hibernate/id/Configurable.java @@ -8,6 +8,7 @@ package org.hibernate.id; 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; @@ -18,6 +19,11 @@ import org.hibernate.type.Type; /** * A {@link org.hibernate.generator.Generator} that supports "configuration". * + * @apiNote Prefer instead using either + * * @author Gavin King * @author Steve Ebersole */ @@ -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 HHH-18615. */ + @Deprecated(forRemoval = true) default void create(GeneratorCreationContext creationContext) throws MappingException {} /** diff --git a/hibernate-core/src/main/java/org/hibernate/id/enhanced/SequenceStyleGenerator.java b/hibernate-core/src/main/java/org/hibernate/id/enhanced/SequenceStyleGenerator.java index 5e482c70fe..dc5bcb6ab4 100644 --- a/hibernate-core/src/main/java/org/hibernate/id/enhanced/SequenceStyleGenerator.java +++ b/hibernate-core/src/main/java/org/hibernate/id/enhanced/SequenceStyleGenerator.java @@ -16,6 +16,7 @@ import org.hibernate.HibernateException; 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; @@ -27,6 +28,7 @@ import org.hibernate.dialect.Dialect; 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; @@ -164,6 +166,8 @@ public class SequenceStyleGenerator private Optimizer optimizer; private Type identifierType; + private PhysicalNamingStrategy physicalNamingStrategy; + /** * Getter for property 'databaseStructure'. * @@ -195,8 +199,17 @@ public class SequenceStyleGenerator // 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(); @@ -217,8 +230,7 @@ public class SequenceStyleGenerator physicalSequence, optimizationStrategy, serviceRegistry, - determineContributor( parameters ), - (ObjectNameNormalizer) parameters.get( IDENTIFIER_NORMALIZER ) + determineContributor( parameters ) ); if ( physicalSequence @@ -244,6 +256,9 @@ public class SequenceStyleGenerator 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( @@ -253,8 +268,7 @@ public class SequenceStyleGenerator 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, @@ -265,8 +279,7 @@ public class SequenceStyleGenerator 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 ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/id/enhanced/SequenceStyleConfigUnitTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/id/enhanced/SequenceStyleConfigUnitTest.java index 37a9109369..a0b32f685e 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/id/enhanced/SequenceStyleConfigUnitTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/id/enhanced/SequenceStyleConfigUnitTest.java @@ -20,6 +20,7 @@ import org.hibernate.dialect.Dialect; 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; @@ -33,6 +34,9 @@ import org.hibernate.id.enhanced.SequenceStructure; 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; @@ -70,8 +74,11 @@ public class SequenceStyleConfigUnitTest { () -> 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 ), @@ -79,7 +86,6 @@ public class SequenceStyleConfigUnitTest { serviceRegistry ); - Database database = new Database( buildingContext.getBuildingOptions() ); generator.registerExportables( database ); generator.initialize( SqlStringGenerationContextImpl.forTests( database.getJdbcEnvironment() ) ); @@ -160,10 +166,15 @@ public class SequenceStyleConfigUnitTest { () -> 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 ), @@ -171,7 +182,6 @@ public class SequenceStyleConfigUnitTest { serviceRegistry ); - Database database = new Database( buildingContext.getBuildingOptions() ); generator.registerExportables( database ); generator.initialize( SqlStringGenerationContextImpl.forTests( database.getJdbcEnvironment() ) ); @@ -190,17 +200,20 @@ public class SequenceStyleConfigUnitTest { () -> 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() ) ); @@ -226,17 +239,20 @@ public class SequenceStyleConfigUnitTest { () -> 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() ) ); @@ -285,6 +301,44 @@ public class SequenceStyleConfigUnitTest { } } + 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 */ @@ -302,14 +356,18 @@ public class SequenceStyleConfigUnitTest { 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() ) ); @@ -323,6 +381,7 @@ public class SequenceStyleConfigUnitTest { 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 ), @@ -341,6 +400,7 @@ public class SequenceStyleConfigUnitTest { 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 ), @@ -368,16 +428,19 @@ public class SequenceStyleConfigUnitTest { () -> 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() ); @@ -385,6 +448,7 @@ public class SequenceStyleConfigUnitTest { props.setProperty( Environment.PREFERRED_POOLED_OPTIMIZER, StandardOptimizerDescriptor.POOLED_LO.getExternalName() ); generator = new SequenceStyleGenerator(); + generator.create( generatorCreationContext ); generator.configure( new TypeConfiguration().getBasicTypeRegistry() .resolve( StandardBasicTypes.LONG ), @@ -398,6 +462,7 @@ public class SequenceStyleConfigUnitTest { props.setProperty( Environment.PREFERRED_POOLED_OPTIMIZER, StandardOptimizerDescriptor.POOLED_LOTL.getExternalName() ); generator = new SequenceStyleGenerator(); + generator.create( generatorCreationContext ); generator.configure( new TypeConfiguration().getBasicTypeRegistry() .resolve( StandardBasicTypes.LONG ),