From 7d3d17de4cc96a3015b7113783bc4313fd7e5aaa Mon Sep 17 00:00:00 2001 From: Gavin King Date: Fri, 17 May 2024 16:21:13 +0200 Subject: [PATCH] HHH-18136 clean up legacy handling of identity columns Signed-off-by: Gavin King --- .../generator/OnExecutionGenerator.java | 19 +++++----- .../org/hibernate/id/IdentityGenerator.java | 25 ++++++------- .../factory/IdentifierGeneratorFactory.java | 22 ----------- .../StandardIdentifierGeneratorFactory.java | 10 ++--- .../java/org/hibernate/mapping/Column.java | 9 +++++ .../java/org/hibernate/mapping/Component.java | 6 +-- .../java/org/hibernate/mapping/KeyValue.java | 13 ++----- .../org/hibernate/mapping/SimpleValue.java | 37 ++++++++----------- .../schema/internal/ColumnDefinitions.java | 24 ++++-------- .../IdentityGeneratorExtendsTest.java | 2 +- .../idgen/userdefined/NativeGenerator.java | 6 +-- 11 files changed, 67 insertions(+), 106 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/generator/OnExecutionGenerator.java b/hibernate-core/src/main/java/org/hibernate/generator/OnExecutionGenerator.java index faa66a0587..ba8db9ebb9 100644 --- a/hibernate-core/src/main/java/org/hibernate/generator/OnExecutionGenerator.java +++ b/hibernate-core/src/main/java/org/hibernate/generator/OnExecutionGenerator.java @@ -8,6 +8,7 @@ package org.hibernate.generator; import org.hibernate.Incubating; import org.hibernate.dialect.Dialect; +import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.id.PostInsertIdentityPersister; import org.hibernate.id.insert.GetGeneratedKeysDelegate; import org.hibernate.id.insert.InsertGeneratedIdentifierDelegate; @@ -15,6 +16,7 @@ import org.hibernate.id.insert.InsertReturningDelegate; import org.hibernate.id.insert.UniqueKeySelectingDelegate; import org.hibernate.persister.entity.EntityPersister; +import static org.hibernate.generator.EventType.INSERT; import static org.hibernate.generator.internal.NaturalIdHelper.getNaturalIdPropertyNames; import static org.hibernate.generator.values.internal.GeneratedValuesHelper.noCustomSql; @@ -117,21 +119,18 @@ public interface OnExecutionGenerator extends Generator { */ @Incubating default InsertGeneratedIdentifierDelegate getGeneratedIdentifierDelegate(PostInsertIdentityPersister persister) { - final Dialect dialect = persister.getFactory().getJdbcServices().getDialect(); + final SessionFactoryImplementor factory = persister.getFactory(); + final Dialect dialect = factory.getJdbcServices().getDialect(); if ( dialect.supportsInsertReturningGeneratedKeys() - && persister.getFactory().getSessionFactoryOptions().isGetGeneratedKeysEnabled() ) { - return new GetGeneratedKeysDelegate( persister, false, EventType.INSERT ); + && factory.getSessionFactoryOptions().isGetGeneratedKeysEnabled() ) { + return new GetGeneratedKeysDelegate( persister, false, INSERT ); } - else if ( dialect.supportsInsertReturning() && noCustomSql( persister, EventType.INSERT ) ) { - return new InsertReturningDelegate( persister, EventType.INSERT ); + else if ( dialect.supportsInsertReturning() && noCustomSql( persister, INSERT ) ) { + return new InsertReturningDelegate( persister, INSERT ); } else { // let's just hope the entity has a @NaturalId! - return new UniqueKeySelectingDelegate( - persister, - getUniqueKeyPropertyNames( persister ), - EventType.INSERT - ); + return new UniqueKeySelectingDelegate( persister, getUniqueKeyPropertyNames( persister ), INSERT ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/id/IdentityGenerator.java b/hibernate-core/src/main/java/org/hibernate/id/IdentityGenerator.java index dec705ef4e..407cab115e 100644 --- a/hibernate-core/src/main/java/org/hibernate/id/IdentityGenerator.java +++ b/hibernate-core/src/main/java/org/hibernate/id/IdentityGenerator.java @@ -6,8 +6,9 @@ */ package org.hibernate.id; +import org.hibernate.boot.spi.SessionFactoryOptions; import org.hibernate.dialect.Dialect; -import org.hibernate.generator.EventType; +import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.generator.OnExecutionGenerator; import org.hibernate.id.factory.spi.StandardGenerator; import org.hibernate.id.insert.BasicSelectingDelegate; @@ -16,6 +17,7 @@ import org.hibernate.id.insert.InsertGeneratedIdentifierDelegate; import org.hibernate.id.insert.InsertReturningDelegate; import org.hibernate.id.insert.UniqueKeySelectingDelegate; +import static org.hibernate.generator.EventType.INSERT; import static org.hibernate.generator.internal.NaturalIdHelper.getNaturalIdPropertyNames; import static org.hibernate.generator.values.internal.GeneratedValuesHelper.noCustomSql; @@ -52,26 +54,23 @@ public class IdentityGenerator @Override public InsertGeneratedIdentifierDelegate getGeneratedIdentifierDelegate(PostInsertIdentityPersister persister) { - final Dialect dialect = persister.getFactory().getJdbcServices().getDialect(); + final SessionFactoryImplementor factory = persister.getFactory(); + final Dialect dialect = factory.getJdbcServices().getDialect(); // Try to use generic delegates if the dialects supports them - if ( dialect.supportsInsertReturningGeneratedKeys() - && persister.getFactory().getSessionFactoryOptions().isGetGeneratedKeysEnabled() ) { - return new GetGeneratedKeysDelegate( persister, false, EventType.INSERT ); + final SessionFactoryOptions sessionFactoryOptions = factory.getSessionFactoryOptions(); + if ( dialect.supportsInsertReturningGeneratedKeys() && sessionFactoryOptions.isGetGeneratedKeysEnabled() ) { + return new GetGeneratedKeysDelegate( persister, false, INSERT ); } - else if ( dialect.supportsInsertReturning() && noCustomSql( persister, EventType.INSERT ) ) { - return new InsertReturningDelegate( persister, EventType.INSERT ); + else if ( dialect.supportsInsertReturning() && noCustomSql( persister, INSERT ) ) { + return new InsertReturningDelegate( persister, INSERT ); } // Fall back to delegates which only handle identifiers - else if ( persister.getFactory().getSessionFactoryOptions().isGetGeneratedKeysEnabled() ) { + else if ( sessionFactoryOptions.isGetGeneratedKeysEnabled() ) { return dialect.getIdentityColumnSupport().buildGetGeneratedKeysDelegate( persister, dialect ); } else if ( persister.getNaturalIdentifierProperties() != null && !persister.getEntityMetamodel().isNaturalIdentifierInsertGenerated() ) { - return new UniqueKeySelectingDelegate( - persister, - getNaturalIdPropertyNames( persister ), - EventType.INSERT - ); + return new UniqueKeySelectingDelegate( persister, getNaturalIdPropertyNames( persister ), INSERT ); } else { return new BasicSelectingDelegate( persister ); diff --git a/hibernate-core/src/main/java/org/hibernate/id/factory/IdentifierGeneratorFactory.java b/hibernate-core/src/main/java/org/hibernate/id/factory/IdentifierGeneratorFactory.java index 29843e094c..ec21727bbf 100644 --- a/hibernate-core/src/main/java/org/hibernate/id/factory/IdentifierGeneratorFactory.java +++ b/hibernate-core/src/main/java/org/hibernate/id/factory/IdentifierGeneratorFactory.java @@ -69,29 +69,7 @@ public interface IdentifierGeneratorFactory extends Service { * @return The appropriate generator instance. * * @deprecated use {@link #createIdentifierGenerator(GenerationType, String, String, JavaType, Properties, GeneratorDefinitionResolver)} - * instead */ @Deprecated(since = "6.0") Generator createIdentifierGenerator(String strategy, Type type, Properties parameters); - - /** - * Retrieve the class that will be used as the {@link IdentifierGenerator} for the given strategy. - * - * @param strategy The strategy - * @return The generator class. - * - * @deprecated with no replacement. See - * {@link #createIdentifierGenerator(GenerationType, String, String, JavaType, Properties, GeneratorDefinitionResolver)} - */ - @Deprecated(since = "6.0") - Class getIdentifierGeneratorClass(String strategy); - - /** - * Get the dialect. - * @deprecated should be removed - * - * @return the dialect - */ - @Deprecated(since = "6.2", forRemoval = true) - Dialect getDialect(); } diff --git a/hibernate-core/src/main/java/org/hibernate/id/factory/internal/StandardIdentifierGeneratorFactory.java b/hibernate-core/src/main/java/org/hibernate/id/factory/internal/StandardIdentifierGeneratorFactory.java index 37a8bcc400..ed30caee7f 100644 --- a/hibernate-core/src/main/java/org/hibernate/id/factory/internal/StandardIdentifierGeneratorFactory.java +++ b/hibernate-core/src/main/java/org/hibernate/id/factory/internal/StandardIdentifierGeneratorFactory.java @@ -198,15 +198,14 @@ public class StandardIdentifierGeneratorFactory throw new UnsupportedOperationException( "No GenerationTypeStrategy specified" ); } - @Override @Deprecated - public Dialect getDialect() { + private Dialect getDialect() { if ( dialect == null ) { dialect = serviceRegistry.requireService( JdbcEnvironment.class ).getDialect(); } return dialect; } - @Override + @Override @Deprecated public Generator createIdentifierGenerator(String strategy, Type type, Properties parameters) { try { final Class clazz = getIdentifierGeneratorClass( strategy ); @@ -243,8 +242,7 @@ public class StandardIdentifierGeneratorFactory return true; } - @Override - public Class getIdentifierGeneratorClass(String strategy) { + private Class getIdentifierGeneratorClass(String strategy) { switch ( strategy ) { case "hilo": throw new UnsupportedOperationException( "Support for 'hilo' generator has been removed" ); @@ -257,7 +255,7 @@ public class StandardIdentifierGeneratorFactory } } - protected Class generatorClassForName(String strategy) { + private Class generatorClassForName(String strategy) { try { Class clazz = serviceRegistry.requireService( ClassLoaderService.class ) diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/Column.java b/hibernate-core/src/main/java/org/hibernate/mapping/Column.java index c49a71461e..3c1d5124f4 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/Column.java +++ b/hibernate-core/src/main/java/org/hibernate/mapping/Column.java @@ -70,6 +70,7 @@ public class Column implements Selectable, Serializable, Cloneable, ColumnTypeIn private boolean quoted; private boolean explicit; int uniqueInteger; + private boolean identity; private String comment; private String defaultValue; private String generatedAs; @@ -143,6 +144,14 @@ public class Column implements Selectable, Serializable, Cloneable, ColumnTypeIn this.explicit = explicit; } + public boolean isIdentity() { + return identity; + } + + public void setIdentity(boolean identity) { + this.identity = identity; + } + private static boolean isQuoted(String name) { //TODO: deprecated, remove eventually return name != null diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/Component.java b/hibernate-core/src/main/java/org/hibernate/mapping/Component.java index 8409b78be5..8813839186 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/Component.java +++ b/hibernate-core/src/main/java/org/hibernate/mapping/Component.java @@ -680,10 +680,8 @@ public class Component extends SimpleValue implements MetaAttributable, Sortable final CompositeNestedGeneratedValueGenerator.GenerationContextLocator locator = new StandardGenerationContextLocator( rootClass.getEntityName() ); - final CompositeNestedGeneratedValueGenerator generator = new CompositeNestedGeneratedValueGenerator( - locator, - getType() - ); + final CompositeNestedGeneratedValueGenerator generator = + new CompositeNestedGeneratedValueGenerator( locator, getType() ); final List properties = getProperties(); for ( int i = 0; i < properties.size(); i++ ) { diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/KeyValue.java b/hibernate-core/src/main/java/org/hibernate/mapping/KeyValue.java index d77d10669b..8904f6d895 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/KeyValue.java +++ b/hibernate-core/src/main/java/org/hibernate/mapping/KeyValue.java @@ -36,11 +36,12 @@ public interface KeyValue extends Value { /** * @deprecated Use {@link #createGenerator(IdentifierGeneratorFactory, Dialect, RootClass)} instead. + * No longer used except in legacy tests. * * @return {@code null} if the {@code Generator} returned by {@link #createGenerator} is not an instance * of {@link IdentifierGenerator}. */ - @Deprecated(since="6.2") + @Deprecated(since="6.2", forRemoval = true) default IdentifierGenerator createIdentifierGenerator( IdentifierGeneratorFactory identifierGeneratorFactory, Dialect dialect, @@ -53,11 +54,12 @@ public interface KeyValue extends Value { /** * @deprecated Use {@link #createGenerator(IdentifierGeneratorFactory, Dialect, RootClass)} instead. + * No longer used except in legacy tests. * * @return {@code null} if the {@code Generator} returned by {@link #createGenerator} is not an instance * of {@link IdentifierGenerator}. */ - @Deprecated(since="6.2") + @Deprecated(since="6.2", forRemoval = true) default IdentifierGenerator createIdentifierGenerator( IdentifierGeneratorFactory identifierGeneratorFactory, Dialect dialect, @@ -65,11 +67,4 @@ public interface KeyValue extends Value { final Generator generator = createGenerator( identifierGeneratorFactory, dialect, rootClass ); return generator instanceof IdentifierGenerator ? (IdentifierGenerator) generator : null; } - - /** - * @deprecated We need to add {@code Column.isIdentity()} - */ - @Deprecated(since="6.2") - boolean isIdentityColumn(IdentifierGeneratorFactory identifierGeneratorFactory, Dialect dialect); - } diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java b/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java index 81088fba0d..c11cca6133 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java +++ b/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java @@ -39,7 +39,6 @@ import org.hibernate.boot.spi.MetadataBuildingContext; import org.hibernate.boot.spi.MetadataImplementor; import org.hibernate.dialect.Dialect; import org.hibernate.engine.spi.Mapping; -import org.hibernate.id.IdentifierGenerator; import org.hibernate.id.IdentityGenerator; import org.hibernate.id.factory.IdentifierGeneratorFactory; import org.hibernate.id.factory.spi.CustomIdGeneratorCreationContext; @@ -378,18 +377,6 @@ public abstract class SimpleValue implements KeyValue { getTable().createUniqueKey( getConstraintColumns(), context ); } - /** - * Returns the cached {@link IdentifierGenerator}, or null if - * {@link #createIdentifierGenerator(IdentifierGeneratorFactory, Dialect, String, String, RootClass)} - * was never completed. - * - * @deprecated not used and no longer supported. - */ - @Deprecated(since = "6.0") - public IdentifierGenerator getIdentifierGenerator() { - return (IdentifierGenerator) generator; - } - @Internal public void setCustomIdGeneratorCreator(IdentifierGeneratorCreator customIdGeneratorCreator) { this.customIdGeneratorCreator = customIdGeneratorCreator; @@ -415,12 +402,28 @@ public abstract class SimpleValue implements KeyValue { } else { generator = createLegacyIdentifierGenerator(this, identifierGeneratorFactory, dialect, null, null, rootClass ); + if ( generator instanceof IdentityGenerator ) { + setColumnToIdentity(); + } } } return generator; } + private void setColumnToIdentity() { + if ( getColumnSpan() != 1 ) { + throw new MappingException( "Identity generation requires exactly one column" ); + } + final Selectable column = getColumn(0); + if ( column instanceof Column ) { + ( (Column) column).setIdentity( true ); + } + else { + throw new MappingException( "Identity generation requires a column" ); + } + } + public boolean isUpdateable() { //needed to satisfy KeyValue return true; @@ -450,14 +453,6 @@ public abstract class SimpleValue implements KeyValue { this.identifierGeneratorStrategy = identifierGeneratorStrategy; } - @Deprecated - @Override - public boolean isIdentityColumn(IdentifierGeneratorFactory identifierGeneratorFactory, Dialect dialect) { - return IdentityGenerator.class.isAssignableFrom( - identifierGeneratorFactory.getIdentifierGeneratorClass( identifierGeneratorStrategy ) - ); - } - public Map getIdentifierGeneratorParameters() { return identifierGeneratorParameters; } diff --git a/hibernate-core/src/main/java/org/hibernate/tool/schema/internal/ColumnDefinitions.java b/hibernate-core/src/main/java/org/hibernate/tool/schema/internal/ColumnDefinitions.java index ecf15b43b2..5d76e9fcf6 100644 --- a/hibernate-core/src/main/java/org/hibernate/tool/schema/internal/ColumnDefinitions.java +++ b/hibernate-core/src/main/java/org/hibernate/tool/schema/internal/ColumnDefinitions.java @@ -8,7 +8,6 @@ package org.hibernate.tool.schema.internal; import org.hibernate.boot.Metadata; import org.hibernate.boot.model.relational.SqlStringGenerationContext; -import org.hibernate.boot.spi.MetadataImplementor; import org.hibernate.dialect.Dialect; import org.hibernate.engine.jdbc.Size; import org.hibernate.mapping.CheckConstraint; @@ -173,7 +172,7 @@ class ColumnDefinitions { Table table, Metadata metadata, Dialect dialect) { - if ( isIdentityColumn( column, table, metadata, dialect) ) { + if ( isIdentityColumn( column, table, dialect) ) { // to support dialects that have their own identity data type if ( dialect.getIdentityColumnSupport().hasDataTypeInIdentityColumn() ) { definition.append( ' ' ).append( column.getSqlType( metadata ) ); @@ -213,9 +212,9 @@ class ColumnDefinitions { } } - private static boolean isIdentityColumn(Column column, Table table, Metadata metadata, Dialect dialect) { + private static boolean isIdentityColumn(Column column, Table table, Dialect dialect) { // Try to find out the name of the primary key in case the dialect needs it to create an identity - return isPrimaryKeyIdentity( table, metadata, dialect ) + return isPrimaryKeyIdentity( table ) && column.getQuotedName( dialect ).equals( getPrimaryKeyColumnName( table, dialect ) ); } @@ -225,20 +224,11 @@ class ColumnDefinitions { : null; } - private static boolean isPrimaryKeyIdentity(Table table, Metadata metadata, Dialect dialect) { - // TODO: this is the much better form moving forward as we move to metamodel - //return hasPrimaryKey - // && table.getPrimaryKey().getColumnSpan() == 1 - // && table.getPrimaryKey().getColumn( 0 ).isIdentity(); - MetadataImplementor metadataImplementor = (MetadataImplementor) metadata; + private static boolean isPrimaryKeyIdentity(Table table) { return table.hasPrimaryKey() - && table.getIdentifierValue() != null - && table.getIdentifierValue() - .isIdentityColumn( - metadataImplementor.getMetadataBuildingOptions() - .getIdentifierGeneratorFactory(), - dialect - ); + && table.getPrimaryKey().getColumnSpan() == 1 + && table.getPrimaryKey().getColumn( 0 ).isIdentity(); + } private static String normalize(String typeName) { diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/identity/hhh10429/IdentityGeneratorExtendsTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/identity/hhh10429/IdentityGeneratorExtendsTest.java index d0bb54698d..1f3cd8a858 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/identity/hhh10429/IdentityGeneratorExtendsTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/identity/hhh10429/IdentityGeneratorExtendsTest.java @@ -43,7 +43,7 @@ public class IdentityGeneratorExtendsTest { final PersistentClass entityBinding = domainModel.getEntityBinding( EntityBean.class.getName() ); final KeyValue identifier = entityBinding.getIdentifier(); - assertTrue( identifier.isIdentityColumn( domainModel.getMetadataBuildingOptions().getIdentifierGeneratorFactory(), dialect ) ); + assertTrue( identifier.getColumns().get(0).isIdentity() ); } @Entity(name = "EntityBean") diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/userdefined/NativeGenerator.java b/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/userdefined/NativeGenerator.java index 2c0501c9cd..6fb8a192a3 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/userdefined/NativeGenerator.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/userdefined/NativeGenerator.java @@ -14,7 +14,6 @@ import org.hibernate.id.PostInsertIdentityPersister; import org.hibernate.id.factory.IdentifierGeneratorFactory; import org.hibernate.id.factory.spi.CustomIdGeneratorCreationContext; import org.hibernate.id.insert.InsertGeneratedIdentifierDelegate; -import org.hibernate.mapping.SimpleValue; import org.hibernate.service.ServiceRegistry; import org.hibernate.type.Type; @@ -33,8 +32,9 @@ public class NativeGenerator public NativeGenerator(NativeId nativeId, Member member, CustomIdGeneratorCreationContext creationContext) { factory = creationContext.getIdentifierGeneratorFactory(); strategy = creationContext.getDatabase().getDialect().getNativeIdentifierGeneratorStrategy(); - SimpleValue value = (SimpleValue) creationContext.getProperty().getValue(); - value.setIdentifierGeneratorStrategy(strategy); + if ( "identity".equals(strategy) ) { + creationContext.getProperty().getValue().getColumns().get(0).setIdentity(true); + } } @Override