simplify Generator instantiation lifecycle

remove a bunch of unused parameters from methods in this package

Author: gavinking

hibernate-core/src/main/java/org/hibernate/boot/internal/InFlightMetadataCollectorImpl.java

@@ -54,7 +54,6 @@
 import org.hibernate.boot.model.naming.Identifier;
 import org.hibernate.boot.model.relational.AuxiliaryDatabaseObject;
 import org.hibernate.boot.model.relational.Database;
-import org.hibernate.boot.model.relational.ExportableProducer;
 import org.hibernate.boot.model.relational.Namespace;
 import org.hibernate.boot.model.relational.QualifiedTableName;
 import org.hibernate.boot.model.source.internal.ImplicitColumnNamingSecondPass;
@@ -81,7 +80,6 @@
 import org.hibernate.dialect.Dialect;
 import org.hibernate.engine.spi.FilterDefinition;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
-import org.hibernate.generator.Generator;
 import org.hibernate.internal.CoreLogging;
 import org.hibernate.internal.CoreMessageLogger;
 import org.hibernate.internal.util.collections.CollectionHelper;
@@ -91,6 +89,7 @@
 import org.hibernate.mapping.DenormalizedTable;
 import org.hibernate.mapping.FetchProfile;
 import org.hibernate.mapping.ForeignKey;
+import org.hibernate.mapping.GeneratorSettings;
 import org.hibernate.mapping.IdentifierCollection;
 import org.hibernate.mapping.Join;
 import org.hibernate.mapping.KeyValue;
@@ -2204,15 +2203,22 @@ private void processExportableProducers() {
 
 	private void handleIdentifierValueBinding(
 			KeyValue identifierValueBinding, Dialect dialect, RootClass entityBinding, Property identifierProperty) {
-		// todo : store this result (back into the entity or into the KeyValue, maybe?)
-		// 		This process of instantiating the id-generator is called multiple times.
-		//		It was done this way in the old code too, so no "regression" here; but
-		//		it could be done better
 		try {
-			final Generator generator = identifierValueBinding.createGenerator( dialect, entityBinding, identifierProperty );
-			if ( generator instanceof ExportableProducer exportableProducer ) {
-				exportableProducer.registerExportables( getDatabase() );
-			}
+			// this call typically caches the new Generator in the instance of KeyValue
+			identifierValueBinding.createGenerator( dialect, entityBinding, identifierProperty,
+					new GeneratorSettings() {
+						@Override
+						public String getDefaultCatalog() {
+							//TODO: does not have access to property-configured default
+							return persistenceUnitMetadata.getDefaultCatalog();
+						}
+
+						@Override
+						public String getDefaultSchema() {
+							//TODO: does not have access to property-configured default
+							return persistenceUnitMetadata.getDefaultSchema();
+						}
+					} );
 		}
 		catch (MappingException e) {
 			// ignore this for now.  The reasoning being "non-reflective" binding as needed

hibernate-core/src/main/java/org/hibernate/boot/model/internal/GeneratorAnnotationHelper.java

@@ -94,10 +94,11 @@ public static <A extends Annotation> A findLocalizedMatch(
 		// lastly, on the package
 		final String packageInfoFqn = StringHelper.qualifier( idMember.getDeclaringType().getClassName() ) + ".package-info";
 		try {
-			final ClassDetails packageInfo = context.getMetadataCollector()
-					.getSourceModelBuildingContext()
-					.getClassDetailsRegistry()
-					.resolveClassDetails( packageInfoFqn );
+			final ClassDetails packageInfo =
+					context.getMetadataCollector()
+							.getSourceModelBuildingContext()
+							.getClassDetailsRegistry()
+							.resolveClassDetails( packageInfoFqn );
 			for ( A generatorAnnotation : packageInfo.getRepeatedAnnotationUsages( generatorAnnotationType, sourceModelContext ) ) {
 				if ( nameExtractor != null ) {
 					final String registrationName = nameExtractor.apply( generatorAnnotation );
@@ -138,10 +139,7 @@ public static void handleUuidStrategy(
 		idValue.setCustomIdGeneratorCreator( (creationContext) -> new UuidGenerator( generatorConfig, idMember ) );
 	}
 
-	public static void handleIdentityStrategy(
-			SimpleValue idValue,
-			MemberDetails idMember,
-			MetadataBuildingContext context) {
+	public static void handleIdentityStrategy(SimpleValue idValue) {
 		idValue.setCustomIdGeneratorCreator( (creationContext) -> new IdentityGenerator() );
 		idValue.setColumnToIdentity();
 	}
@@ -189,7 +187,6 @@ public static void handleGenericGenerator(
 			GenericGenerator generatorConfig,
 			PersistentClass entityMapping,
 			SimpleValue idValue,
-			MemberDetails idMember,
 			MetadataBuildingContext context) {
 		//generator settings
 		final Map<String,String> configuration = new HashMap<>();
@@ -206,9 +203,7 @@ public static void handleGenericGenerator(
 
 		GeneratorBinder.createGeneratorFrom(
 				new IdentifierGeneratorDefinition( generatorName, determineStrategyName( generatorConfig ), configuration ),
-				idMember,
 				idValue,
-				entityMapping,
 				context
 		);
 	}
@@ -232,17 +227,14 @@ public static void handleTableGenerator(
 			TableGenerator generatorConfig,
 			PersistentClass entityMapping,
 			SimpleValue idValue,
-			MemberDetails idMember,
 			MetadataBuildingContext context) {
 		final Map<String,String> configuration = new HashMap<>();
 		applyBaselineConfiguration( generatorConfig, idValue, entityMapping.getRootClass(), context, configuration::put );
-		org.hibernate.id.enhanced.TableGenerator.applyConfiguration( generatorConfig, idValue, configuration::put );
+		org.hibernate.id.enhanced.TableGenerator.applyConfiguration( generatorConfig, configuration::put );
 
 		GeneratorBinder.createGeneratorFrom(
 				new IdentifierGeneratorDefinition( generatorName, org.hibernate.id.enhanced.TableGenerator.class.getName(), configuration ),
-				idMember,
 				idValue,
-				entityMapping,
 				context
 		);
 

hibernate-core/src/main/java/org/hibernate/boot/model/internal/GeneratorBinder.java

@@ -7,7 +7,6 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Member;
-import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
@@ -46,6 +45,7 @@
 import org.hibernate.internal.util.StringHelper;
 import org.hibernate.internal.util.collections.CollectionHelper;
 import org.hibernate.mapping.GeneratorCreator;
+import org.hibernate.mapping.KeyValue;
 import org.hibernate.mapping.PersistentClass;
 import org.hibernate.mapping.SimpleValue;
 import org.hibernate.models.spi.AnnotationTarget;
@@ -283,22 +283,6 @@ private static GenerationType interpretGenerationType(GeneratedValue generatedVa
 		return strategy == null ? AUTO : strategy;
 	}
 
-	/**
-	 * Collects definition objects for all generators defined using any of {@link TableGenerator},
-	 * {@link SequenceGenerator}, and {@link GenericGenerator} on the given annotated element.
-	 */
-	public static List<IdentifierGeneratorDefinition> collectIdGeneratorDefinitions(
-			AnnotationTarget annotatedElement,
-			MetadataBuildingContext context) {
-		final ArrayList<IdentifierGeneratorDefinition> definitions = new ArrayList<>();
-		visitIdGeneratorDefinitions(
-				annotatedElement,
-				definitions::add,
-				context
-		);
-		return definitions;
-	}
-
 	public static void visitIdGeneratorDefinitions(
 			AnnotationTarget annotatedElement,
 			Consumer<IdentifierGeneratorDefinition> consumer,
@@ -693,7 +677,7 @@ public static void callConfigure(
 			Generator generator,
 			Map<String, Object> configuration,
 			SimpleValue identifierValue) {
-		if ( generator instanceof final Configurable configurable ) {
+		if ( generator instanceof Configurable configurable ) {
 			final Properties parameters = collectParameters(
 					identifierValue,
 					creationContext.getDatabase().getDialect(),
@@ -702,6 +686,12 @@ public static void callConfigure(
 			);
 			configurable.configure( creationContext, parameters );
 		}
+		if ( generator instanceof ExportableProducer exportableProducer ) {
+			exportableProducer.registerExportables( creationContext.getDatabase() );
+		}
+		if ( generator instanceof Configurable configurable ) {
+			configurable.initialize( creationContext.getSqlStringGenerationContext() );
+		}
 	}
 
 	private static void checkIdGeneratorTiming(Class<? extends Annotation> annotationType, Generator generator) {
@@ -726,19 +716,20 @@ private static void createIdGenerator(
 		// NOTE: `generatedValue` is never null here
 		final GeneratedValue generatedValue = castNonNull( idMember.getDirectAnnotationUsage( GeneratedValue.class ) );
 
+		final InFlightMetadataCollector metadataCollector = context.getMetadataCollector();
 		if ( isGlobalGeneratorNameGlobal( context ) ) {
 			// process and register any generators defined on the member.
 			// according to JPA these are also global.
-			context.getMetadataCollector().getGlobalRegistrations().as( GlobalRegistrar.class ).collectIdGenerators( idMember );
-			context.getMetadataCollector().addSecondPass( new StrictIdGeneratorResolverSecondPass(
+			metadataCollector.getGlobalRegistrations().as( GlobalRegistrar.class ).collectIdGenerators( idMember );
+			metadataCollector.addSecondPass( new StrictIdGeneratorResolverSecondPass(
 					persistentClass,
 					idValue,
 					idMember,
 					context
 			) );
 		}
 		else {
-			context.getMetadataCollector().addSecondPass( new IdGeneratorResolverSecondPass(
+			metadataCollector.addSecondPass( new IdGeneratorResolverSecondPass(
 					persistentClass,
 					idValue,
 					idMember,
@@ -750,7 +741,6 @@ private static void createIdGenerator(
 
 	public static void createGeneratorFrom(
 			IdentifierGeneratorDefinition defaultedGenerator,
-			MemberDetails idMember,
 			SimpleValue idValue,
 			Map<String, Object> configuration,
 			MetadataBuildingContext context) {
@@ -766,43 +756,29 @@ public static void createGeneratorFrom(
 			if ( identifierGenerator instanceof IdentityGenerator) {
 				idValue.setColumnToIdentity();
 			}
-
-			if ( identifierGenerator instanceof ExportableProducer exportableProducer ) {
-				exportableProducer.registerExportables( creationContext.getDatabase() );
-			}
-
 			return identifierGenerator;
 		} );
 	}
 
 
 	public static void createGeneratorFrom(
 			IdentifierGeneratorDefinition defaultedGenerator,
-			MemberDetails idMember,
 			SimpleValue idValue,
-			PersistentClass persistentClass,
 			MetadataBuildingContext context) {
 		createGeneratorFrom(
 				defaultedGenerator,
-				idMember,
 				idValue,
-				buildConfigurationMap( defaultedGenerator, idValue, persistentClass ),
+				buildConfigurationMap( idValue ),
 				context
 		);
 	}
 
-	public static Map<String, Object> buildConfigurationMap(
-			IdentifierGeneratorDefinition defaultedGenerator,
-			SimpleValue idValue,
-			PersistentClass persistentClass) {
+	private static Map<String, Object> buildConfigurationMap(KeyValue idValue) {
 		final Map<String,Object> configuration = new HashMap<>();
-
 		configuration.put( PersistentIdentifierGenerator.TABLE, idValue.getTable().getName() );
-
 		if ( idValue.getColumnSpan() == 1 ) {
-			configuration.put( PersistentIdentifierGenerator.PK, idValue.getColumns().get( 0).getName() );
+			configuration.put( PersistentIdentifierGenerator.PK, idValue.getColumns().get(0).getName() );
 		}
-
 		return configuration;
 	}
 
@@ -941,15 +917,12 @@ static GeneratorCreator createValueGeneratorFromAnnotations(
 		final List<? extends Annotation> generatorAnnotations =
 				property.getMetaAnnotated( ValueGenerationType.class,
 						context.getMetadataCollector().getSourceModelBuildingContext() );
-		switch ( generatorAnnotations.size() ) {
-			case 0:
-				return null;
-			case 1:
-				return generatorCreator( property, generatorAnnotations.get(0), beanContainer( context ) );
-			default:
-				throw new AnnotationException( "Property '" + qualify( holder.getPath(), propertyName )
-						+ "' has too many generator annotations: " + generatorAnnotations );
-		}
+		return switch ( generatorAnnotations.size() ) {
+			case 0 -> null;
+			case 1 -> generatorCreator( property, generatorAnnotations.get(0), beanContainer( context ) );
+			default -> throw new AnnotationException( "Property '" + qualify( holder.getPath(), propertyName )
+					+ "' has too many generator annotations: " + generatorAnnotations );
+		};
 	}
 
 	public static void applyIfNotEmpty(String name, String value, BiConsumer<String,String> consumer) {