Skip to content

simplify Generator instantiation lifecycle #8985

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 19, 2024
Merged

simplify Generator instantiation lifecycle #8985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a9b262a
simplify Generator instantiation lifecycle
gavinking Sep 19, 2024
d30178f
don't cache the Generator in the SimpleValue
gavinking Sep 19, 2024
df3d7b0
more fallout from new Generator instantiation lifecycle
gavinking Sep 19, 2024
fdc9367
remove rogue ;
gavinking Sep 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 33 additions & 19 deletions hibernate-core/src/main/java/org/hibernate/boot/internal/InFlightMetadataCollectorImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@
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.relational.SqlStringGenerationContext;
import org.hibernate.boot.model.source.internal.ImplicitColumnNamingSecondPass;
import org.hibernate.boot.model.source.spi.LocalMetadataBuildingContext;
import org.hibernate.boot.models.categorize.internal.ClassLoaderServiceLoading;
Expand All @@ -79,9 +79,10 @@
import org.hibernate.boot.spi.SecondPass;
import org.hibernate.cfg.AvailableSettings;
import org.hibernate.dialect.Dialect;
import org.hibernate.engine.config.spi.ConfigurationService;
import org.hibernate.engine.config.spi.StandardConverters;
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;
Expand All @@ -91,6 +92,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;
Expand Down Expand Up @@ -121,6 +123,9 @@
import jakarta.persistence.MapsId;

import static org.hibernate.boot.model.naming.Identifier.toIdentifier;
import static org.hibernate.boot.model.relational.internal.SqlStringGenerationContextImpl.fromExplicit;
import static org.hibernate.cfg.MappingSettings.DEFAULT_CATALOG;
import static org.hibernate.cfg.MappingSettings.DEFAULT_SCHEMA;
import static org.hibernate.internal.util.collections.CollectionHelper.mapOfSize;

/**
Expand All @@ -133,7 +138,8 @@
*
* @author Steve Ebersole
*/
public class InFlightMetadataCollectorImpl implements InFlightMetadataCollector, ConverterRegistry {
public class InFlightMetadataCollectorImpl
implements InFlightMetadataCollector, ConverterRegistry, GeneratorSettings {
private static final CoreMessageLogger log = CoreLogging.messageLogger( InFlightMetadataCollectorImpl.class );

private final BootstrapContext bootstrapContext;
Expand Down Expand Up @@ -171,6 +177,8 @@

private Map<String, SqmFunctionDescriptor> sqlFunctionMap;

final ConfigurationService configurationService;

// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// All the annotation-processing-specific state :(
private final Set<String> defaultIdentifierGeneratorNames = new HashSet<>();
Expand Down Expand Up @@ -211,16 +219,12 @@
}

bootstrapContext.getAuxiliaryDatabaseObjectList().forEach( getDatabase()::addAuxiliaryDatabaseObject );

configurationService = bootstrapContext.getServiceRegistry().requireService(ConfigurationService.class);
}

public InFlightMetadataCollectorImpl(
BootstrapContext bootstrapContext,
MetadataBuildingOptions options) {
this(
bootstrapContext,
createModelBuildingContext( bootstrapContext ),
options
);
public InFlightMetadataCollectorImpl(BootstrapContext bootstrapContext, MetadataBuildingOptions options) {
this( bootstrapContext, createModelBuildingContext( bootstrapContext ), options );
}

private static SourceModelBuildingContext createModelBuildingContext(BootstrapContext bootstrapContext) {
Expand Down Expand Up @@ -1147,7 +1151,7 @@
* names for a table. Mainly this is used to ensure that the defined NamingStrategy
* is not creating duplicate column names.
*/
private class TableColumnNameBinding implements Serializable {

Check warning

Code scanning / CodeQL

Serializable inner class of non-serializable class Warning

Serializable inner class of non-serializable class
InFlightMetadataCollectorImpl
Loading
. Consider making the class static or implementing readObject() and writeObject().
private final String tableName;
private final Map<Identifier, String> logicalToPhysical = new HashMap<>();
private final Map<String, Identifier> physicalToLogical = new HashMap<>();
Expand Down Expand Up @@ -2204,15 +2208,8 @@

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() );
}
identifierValueBinding.createGenerator( dialect, entityBinding, identifierProperty, this );
}
catch (MappingException e) {
// ignore this for now. The reasoning being "non-reflective" binding as needed
Expand All @@ -2222,4 +2219,21 @@
log.debugf( "Ignoring exception thrown when trying to build IdentifierGenerator as part of Metadata building", e );
}
}

@Override
public String getDefaultCatalog() {
final String defaultCatalog = configurationService.getSetting( DEFAULT_CATALOG, StandardConverters.STRING );
return defaultCatalog == null ? persistenceUnitMetadata.getDefaultCatalog() : defaultCatalog;
}

@Override
public String getDefaultSchema() {
final String defaultSchema = configurationService.getSetting( DEFAULT_SCHEMA, StandardConverters.STRING );
return defaultSchema == null ? persistenceUnitMetadata.getDefaultSchema() : defaultSchema;
}

@Override
public SqlStringGenerationContext getSqlStringGenerationContext() {
return fromExplicit( database.getJdbcEnvironment(), database, getDefaultCatalog(), getDefaultSchema() );
}
}
22 changes: 7 additions & 15 deletions ...rnate-core/src/main/java/org/hibernate/boot/model/internal/GeneratorAnnotationHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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<>();
Expand All @@ -206,9 +203,7 @@ public static void handleGenericGenerator(

GeneratorBinder.createGeneratorFrom(
new IdentifierGeneratorDefinition( generatorName, determineStrategyName( generatorConfig ), configuration ),
idMember,
idValue,
entityMapping,
context
);
}
Expand All @@ -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
);

Expand Down
69 changes: 21 additions & 48 deletions hibernate-core/src/main/java/org/hibernate/boot/model/internal/GeneratorBinder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand All @@ -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) {
Expand All @@ -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,
Expand All @@ -750,7 +741,6 @@ private static void createIdGenerator(

public static void createGeneratorFrom(
IdentifierGeneratorDefinition defaultedGenerator,
MemberDetails idMember,
SimpleValue idValue,
Map<String, Object> configuration,
MetadataBuildingContext context) {
Expand All @@ -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;
}

Expand Down Expand Up @@ -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) {
Expand Down
Loading
Loading