don't cache the Generator in the SimpleValue

gavinking · gavinking · commit 18aa8a7c702e · 2024-09-19T22:16:55.000+02:00
that way we don't need to re-call initialize()
on the cached instance

also handle canonicalization of generators via
RuntimeModelCreationContext instead of via
deprecated method or SessionFactoryImplementor
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/internal/InFlightMetadataCollectorImpl.java b/hibernate-core/src/main/java/org/hibernate/boot/internal/InFlightMetadataCollectorImpl.java
@@ -2204,7 +2204,6 @@ private void processExportableProducers() {
 	private void handleIdentifierValueBinding(
 			KeyValue identifierValueBinding, Dialect dialect, RootClass entityBinding, Property identifierProperty) {
 		try {
-			// this call typically caches the new Generator in the instance of KeyValue
 			identifierValueBinding.createGenerator( dialect, entityBinding, identifierProperty,
 					new GeneratorSettings() {
 						@Override
diff --git a/hibernate-core/src/main/java/org/hibernate/internal/SessionFactoryImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/SessionFactoryImpl.java
@@ -67,13 +67,11 @@
 import org.hibernate.event.spi.EventEngine;
 import org.hibernate.generator.Generator;
 import org.hibernate.graph.spi.RootGraphImplementor;
-import org.hibernate.id.Configurable;
 import org.hibernate.integrator.spi.Integrator;
 import org.hibernate.integrator.spi.IntegratorService;
 import org.hibernate.jpa.internal.ExceptionMapperLegacyJpaImpl;
 import org.hibernate.jpa.internal.PersistenceUnitUtilImpl;
 import org.hibernate.mapping.Collection;
-import org.hibernate.mapping.GeneratorSettings;
 import org.hibernate.mapping.PersistentClass;
 import org.hibernate.mapping.RootClass;
 import org.hibernate.metamodel.internal.RuntimeMetamodelsImpl;
@@ -185,8 +183,6 @@ public class SessionFactoryImpl extends QueryParameterBindingTypeResolverImpl im
 
 	private final transient CurrentSessionContext currentSessionContext;
 
-	// todo : move to MetamodelImpl
-	private final transient Map<String, Generator> identifierGenerators;
 	private final transient Map<String, FilterDefinition> filters;
 	private final transient java.util.Collection<FilterDefinition> autoEnabledFilters = new HashSet<>();
 	private final transient Map<String, FetchProfile> fetchProfiles;
@@ -263,7 +259,6 @@ public SessionFactoryImpl(
 		try {
 			integrate( bootMetamodel, bootstrapContext, integratorObserver );
 
-			identifierGenerators = createGenerators( jdbcServices, bootMetamodel, options );
 			bootMetamodel.orderColumns( false );
 			bootMetamodel.validate();
 
@@ -346,6 +341,8 @@ private RuntimeModelCreationContext runtimeModelCreationContext(
 			TypeConfiguration typeConfiguration,
 			SessionFactoryOptions options) {
 		return new RuntimeModelCreationContext() {
+			final Map<String,Generator> generators = new HashMap<>();
+
 			@Override
 			public BootstrapContext getBootstrapContext() {
 				return bootstrapContext;
@@ -411,41 +408,12 @@ public SqlStringGenerationContext getSqlStringGenerationContext() {
 			public ServiceRegistry getServiceRegistry() {
 				return serviceRegistry;
 			}
-		};
-	}
 
-	private Map<String, Generator> createGenerators(
-			JdbcServices jdbcServices, MetadataImplementor metamodel, SessionFactoryOptions options) {
-		final Dialect dialect = jdbcServices.getJdbcEnvironment().getDialect();
-		final Map<String, Generator> generators = new HashMap<>();
-		for ( PersistentClass model : metamodel.getEntityBindings() ) {
-			if ( model instanceof RootClass rootClass ) {
-				final Generator generator =
-						model.getIdentifier()
-								// returns the cached Generator if it was already created
-								.createGenerator( dialect, rootClass, model.getIdentifierProperty(),
-										new GeneratorSettings() {
-											@Override
-											public String getDefaultCatalog() {
-												return options.getDefaultCatalog();
-											}
-
-											@Override
-											public String getDefaultSchema() {
-												return options.getDefaultSchema();
-											}
-										} );
-				// the cached generator might have been created from
-				// InFlightMetadataCollectorImpl.handleIdentifierValueBinding,
-				// in which case it did not have access to the property-configured
-				// default catalog and schema, so re-render SQL here
-				if ( generator instanceof Configurable configurable ) {
-					configurable.initialize( sqlStringGenerationContext );
-				}
-				generators.put( model.getEntityName(), generator );
+			@Override
+			public Map<String, Generator> getGenerators() {
+				return generators;
 			}
-		}
-		return generators;
+		};
 	}
 
 	private static SqlStringGenerationContext createSqlStringGenerationContext(
@@ -1086,7 +1054,7 @@ public Set<String> getDefinedFetchProfileNames() {
 
 	@Override @Deprecated
 	public Generator getGenerator(String rootEntityName) {
-		return identifierGenerators.get( rootEntityName );
+		return null;
 	}
 
 	private boolean canAccessTransactionManager() {
diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/Component.java b/hibernate-core/src/main/java/org/hibernate/mapping/Component.java
@@ -106,8 +106,6 @@ public class Component extends SimpleValue implements MetaAttributable, Sortable
 	private transient Class<?> componentClass;
 	private transient Boolean simpleRecord;
 
-	private transient Generator builtIdentifierGenerator;
-
 	public Component(MetadataBuildingContext metadata, PersistentClass owner) throws MappingException {
 		this( metadata, owner.getTable(), owner );
 	}
@@ -652,13 +650,9 @@ public String toString() {
 
 	@Override
 	public Generator createGenerator(Dialect dialect, RootClass rootClass, Property property, GeneratorSettings defaults) {
-		if ( builtIdentifierGenerator == null ) {
-			builtIdentifierGenerator =
-					getCustomIdGeneratorCreator().isAssigned()
-							? buildIdentifierGenerator( dialect, rootClass, defaults )
-							: super.createGenerator( dialect, rootClass, property, defaults );
-		}
-		return builtIdentifierGenerator;
+		return getCustomIdGeneratorCreator().isAssigned()
+				? buildIdentifierGenerator( dialect, rootClass, defaults )
+				: super.createGenerator( dialect, rootClass, property, defaults );
 	}
 
 	private Generator buildIdentifierGenerator(Dialect dialect, RootClass rootClass, GeneratorSettings defaults) {
diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/KeyValue.java b/hibernate-core/src/main/java/org/hibernate/mapping/KeyValue.java
@@ -27,7 +27,17 @@ public interface KeyValue extends Value {
 
 	@Deprecated(since = "7.0")
 	default Generator createGenerator(Dialect dialect, RootClass rootClass) {
-		return createGenerator( dialect, rootClass, null, null );
+		return createGenerator( dialect, rootClass, null, new GeneratorSettings() {
+			@Override
+			public String getDefaultCatalog() {
+				return null;
+			}
+
+			@Override
+			public String getDefaultSchema() {
+				return null;
+			}
+		} );
 	}
 
 	Generator createGenerator(
diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java b/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java
@@ -103,7 +103,6 @@ public abstract class SimpleValue implements KeyValue {
 	private Type type;
 
 	private GeneratorCreator customIdGeneratorCreator = ASSIGNED_IDENTIFIER_GENERATOR_CREATOR;
-	private Generator generator;
 
 	public SimpleValue(MetadataBuildingContext buildingContext) {
 		this.buildingContext = buildingContext;
@@ -137,7 +136,6 @@ protected SimpleValue(SimpleValue original) {
 		this.attributeConverterDescriptor = original.attributeConverterDescriptor;
 		this.type = original.type;
 		this.customIdGeneratorCreator = original.customIdGeneratorCreator;
-		this.generator = original.generator;
 	}
 
 	@Override
@@ -381,17 +379,18 @@ public Generator createGenerator(
 			RootClass rootClass,
 			Property property,
 			GeneratorSettings defaults) {
-		if ( generator == null ) {
-			if ( customIdGeneratorCreator != null ) {
-				final IdGeneratorCreationContext context =
-						new IdGeneratorCreationContext( rootClass, property, defaults );
-				generator = customIdGeneratorCreator.createGenerator( context );
-				if ( generator.allowAssignedIdentifiers() && getNullValue() == null ) {
-					setNullValue( "undefined" );
-				}
+		if ( customIdGeneratorCreator != null ) {
+			final IdGeneratorCreationContext context =
+					new IdGeneratorCreationContext( rootClass, property, defaults );
+			final Generator generator = customIdGeneratorCreator.createGenerator( context );
+			if ( generator.allowAssignedIdentifiers() && getNullValue() == null ) {
+				setNullValue( "undefined" );
 			}
+			return generator;
+		}
+		else {
+			return null;
 		}
-		return generator;
 	}
 
 	@Internal
diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/spi/RuntimeModelCreationContext.java b/hibernate-core/src/main/java/org/hibernate/metamodel/spi/RuntimeModelCreationContext.java
@@ -12,6 +12,7 @@
 import org.hibernate.dialect.Dialect;
 import org.hibernate.engine.jdbc.spi.JdbcServices;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
+import org.hibernate.generator.Generator;
 import org.hibernate.query.sqm.function.SqmFunctionRegistry;
 import org.hibernate.service.ServiceRegistry;
 import org.hibernate.type.descriptor.java.spi.JavaTypeRegistry;
@@ -58,4 +59,6 @@ default MetadataImplementor getMetadata() {
 	SqlStringGenerationContext getSqlStringGenerationContext();
 
 	ServiceRegistry getServiceRegistry();
+
+	Map<String, Generator> getGenerators();
 }
diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java
@@ -909,15 +909,11 @@ private static CacheLayout queryCacheLayout(CacheLayout entityQueryCacheLayout,
 	}
 
 	private boolean shouldUseShallowCacheLayout(CacheLayout entityQueryCacheLayout, SessionFactoryOptions options) {
-		switch ( queryCacheLayout( entityQueryCacheLayout, options ) ) {
-			case FULL:
-				return false;
-			case AUTO:
-				return canUseReferenceCacheEntries()
-					|| canReadFromCache();
-			default:
-				return true;
-		}
+		return switch ( queryCacheLayout( entityQueryCacheLayout, options ) ) {
+			case FULL -> false;
+			case AUTO -> canUseReferenceCacheEntries() || canReadFromCache();
+			default -> true;
+		};
 	}
 
 	private static boolean shouldStoreDiscriminatorInShallowQueryCacheLayout(
diff --git a/hibernate-core/src/main/java/org/hibernate/tuple/entity/EntityMetamodel.java b/hibernate-core/src/main/java/org/hibernate/tuple/entity/EntityMetamodel.java
@@ -18,6 +18,7 @@
 import org.hibernate.MappingException;
 import org.hibernate.annotations.NotFoundAction;
 import org.hibernate.boot.spi.MetadataImplementor;
+import org.hibernate.boot.spi.SessionFactoryOptions;
 import org.hibernate.bytecode.enhance.spi.interceptor.EnhancementHelper;
 import org.hibernate.bytecode.internal.BytecodeEnhancementMetadataNonPojoImpl;
 import org.hibernate.bytecode.internal.BytecodeEnhancementMetadataPojoImpl;
@@ -34,6 +35,7 @@
 import org.hibernate.internal.util.collections.ArrayHelper;
 import org.hibernate.mapping.Component;
 import org.hibernate.mapping.GeneratorCreator;
+import org.hibernate.mapping.GeneratorSettings;
 import org.hibernate.mapping.ManyToOne;
 import org.hibernate.mapping.PersistentClass;
 import org.hibernate.mapping.Property;
@@ -160,10 +162,8 @@ public EntityMetamodel(
 
 		subclassId = persistentClass.getSubclassId();
 
-		identifierAttribute = PropertyFactory.buildIdentifierAttribute(
-				persistentClass,
-				sessionFactory.getGenerator( rootName )
-		);
+		final Generator idgenerator = buildIdGenerator( persistentClass, creationContext );
+		identifierAttribute = PropertyFactory.buildIdentifierAttribute( persistentClass, idgenerator );
 
 		versioned = persistentClass.isVersioned();
 
@@ -472,6 +472,37 @@ && isAbstractClass( persistentClass.getMappedClass() ) ) {
 //		entityNameByInheritanceClassMap = toSmallMap( entityNameByInheritanceClassMapLocal );
 	}
 
+	private Generator buildIdGenerator(PersistentClass persistentClass, RuntimeModelCreationContext creationContext) {
+		final Generator existing = creationContext.getGenerators().get( rootName );
+		if ( existing != null ) {
+			return existing;
+		}
+		else {
+			final Generator idgenerator =
+					persistentClass.getIdentifier()
+							// returns the cached Generator if it was already created
+							.createGenerator(
+									creationContext.getDialect(),
+									persistentClass.getRootClass(),
+									persistentClass.getIdentifierProperty(),
+									new GeneratorSettings() {
+										SessionFactoryOptions options() {
+											return creationContext.getSessionFactory().getSessionFactoryOptions();
+										}
+										@Override
+										public String getDefaultCatalog() {
+											return options().getDefaultCatalog();
+										}
+										@Override
+										public String getDefaultSchema() {
+											return options().getDefaultSchema();
+										}
+									} );
+			creationContext.getGenerators().put( rootName, idgenerator );
+			return idgenerator;
+		}
+	}
+
 	private void verifyNaturalIdProperty(Property property) {
 		final Value value = property.getValue();
 		if ( value instanceof ManyToOne toOne ) {
diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/LocalTemporaryTableMutationStrategyNoDropTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/LocalTemporaryTableMutationStrategyNoDropTest.java
@@ -19,6 +19,7 @@
 import org.hibernate.engine.jdbc.spi.JdbcServices;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.engine.spi.SessionImplementor;
+import org.hibernate.generator.Generator;
 import org.hibernate.metamodel.spi.MappingMetamodelImplementor;
 import org.hibernate.metamodel.spi.RuntimeModelCreationContext;
 import org.hibernate.query.sqm.function.SqmFunctionRegistry;
@@ -40,6 +41,7 @@
 import jakarta.persistence.Inheritance;
 import jakarta.persistence.InheritanceType;
 
+import static java.util.Collections.emptyMap;
 import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
 
 @DomainModel(
@@ -165,6 +167,11 @@ public SqlStringGenerationContext getSqlStringGenerationContext() {
 					public org.hibernate.service.ServiceRegistry getServiceRegistry() {
 						return sessionFactory.getServiceRegistry();
 					}
+
+					@Override
+					public Map<String, Generator> getGenerators() {
+						return emptyMap();
+					}
 				}
 		);
 	}
diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/annotations/id/generators/UnnamedGeneratorTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/annotations/id/generators/UnnamedGeneratorTests.java
@@ -13,6 +13,7 @@
 import org.hibernate.boot.registry.StandardServiceRegistryBuilder;
 import org.hibernate.cfg.AvailableSettings;
 import org.hibernate.generator.Generator;
+import org.hibernate.mapping.GeneratorSettings;
 import org.hibernate.mapping.RootClass;
 
 import org.junit.jupiter.api.Test;
@@ -92,7 +93,17 @@ private void withGenerator(Class<?> entityClass, boolean strictlyGlobal, Consume
 					metadata.getDatabase().getDialect(),
 					entityBinding,
 					entityBinding.getIdentifierProperty(),
-					null
+					new GeneratorSettings() {
+						@Override
+						public String getDefaultCatalog() {
+							return null;
+						}
+
+						@Override
+						public String getDefaultSchema() {
+							return null;
+						}
+					}
 			);
 
 			checks.accept( generator );
diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/id/PooledHiLoSequenceIdentifierTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/id/PooledHiLoSequenceIdentifierTest.java
@@ -108,7 +108,7 @@ private void insertNewRow(Session session) {
 						statement = connection.prepareStatement( "INSERT INTO sequenceIdentifier VALUES (?,?)" );
 						statement.setObject(
 								1,
-								((BeforeExecutionGenerator) sfi.getGenerator( SequenceIdentifier.class.getName() ))
+								((BeforeExecutionGenerator) sfi.getMappingMetamodel().getEntityDescriptor( SequenceIdentifier.class).getGenerator())
 										.generate( si, null, null, EventType.INSERT )
 						);
 						statement.setString( 2,"name" );
diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/id/custom/CustomSequenceGenerator.java b/hibernate-core/src/test/java/org/hibernate/orm/test/id/custom/CustomSequenceGenerator.java
