HHH-12454 - Offer flag to consider id generator with local scope (legacy non JPA behavior)

documentation/src/main/asciidoc/userguide/appendices/Configurations.adoc

@@ -186,6 +186,13 @@ If true, generated identifier properties are reset to default values when object
 `*hibernate.id.optimizer.pooled.preferred*` (e.g. `none`, `hilo`, `legacy-hilo`, `pooled` (default value), `pooled-lo`, `pooled-lotl` or a fully-qualified name of the https://docs.jboss.org/hibernate/orm/{majorMinorVersion}/javadocs/org/hibernate/id/enhanced/Optimizer.html[`Optimizer`] implementation)::
 When a generator specified an increment-size and an optimizer was not explicitly specified, which of the _pooled_ optimizers should be preferred?
 
+`*hibernate.jpa.compliance.global_id_generators*` (e.g. `true` or `false` (default value) )::
+The JPA spec says that the scope of TableGenerator and SequenceGenerator names is global to the persistence unit (across all generator types).
++
+Traditionally, Hibernate has considered the names locally scoped.
++
+If enabled, the names used by `@TableGenerator` and `@SequenceGenerator` will be considered global so configuring two different generators
+with the same name will cause a `java.lang.IllegalArgumentException' to be thrown at boot time.
 ==== Quoting options
 
 `*hibernate.globally_quoted_identifiers*` (e.g. `true` or `false` (default value))::

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

@@ -71,6 +71,7 @@ import org.hibernate.cfg.annotations.NamedProcedureCallDefinition;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.dialect.function.SQLFunction;
 import org.hibernate.engine.ResultSetMappingDefinition;
+import org.hibernate.engine.config.spi.ConfigurationService;
 import org.hibernate.engine.spi.FilterDefinition;
 import org.hibernate.engine.spi.NamedQueryDefinition;
 import org.hibernate.engine.spi.NamedSQLQueryDefinition;
@@ -100,6 +101,8 @@ import org.hibernate.mapping.UniqueKey;
 import org.hibernate.query.spi.NamedQueryRepository;
 import org.hibernate.type.TypeResolver;
 
+import static org.hibernate.cfg.AvailableSettings.JPA_ID_GENERATOR_GLOBAL_SCOPE_COMPLIANCE;
+
 /**
  * The implementation of the in-flight Metadata collector contract.
  *
@@ -443,11 +446,13 @@ public class InFlightMetadataCollectorImpl implements InFlightMetadataCollector
 			return;
 		}
 		final IdentifierGeneratorDefinition old = idGeneratorDefinitionMap.put( generator.getName(), generator );
-		if ( old != null ) {
+		if ( old != null && !old.equals( generator ) ) {
-			if ( !old.equals( generator ) ) {
+			if ( options.isJpaGeneratorGlobalScopeComplianceEnabled() ) {
-				throw new IllegalArgumentException( "Duplicate generator name " + old.getName() );
+				throw new IllegalArgumentException( "Duplicate generator name " + old.getName() + " you will likely want to set the property " + JPA_ID_GENERATOR_GLOBAL_SCOPE_COMPLIANCE + " to false " );
+			}
+			else {
+				log.duplicateGeneratorName( old.getName() );
 			}
-//			log.duplicateGeneratorName( old.getName() );
 		}
 	}
 

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

@@ -41,7 +41,6 @@ import org.hibernate.boot.model.TypeContributions;
 import org.hibernate.boot.model.TypeContributor;
 import org.hibernate.boot.model.naming.ImplicitNamingStrategy;
 import org.hibernate.boot.model.naming.ImplicitNamingStrategyJpaCompliantImpl;
-import org.hibernate.boot.model.naming.ImplicitNamingStrategyLegacyJpaImpl;
 import org.hibernate.boot.model.naming.PhysicalNamingStrategy;
 import org.hibernate.boot.model.naming.PhysicalNamingStrategyStandardImpl;
 import org.hibernate.boot.model.process.spi.MetadataBuildingProcess;

hibernate-core/src/main/java/org/hibernate/boot/spi/MetadataBuildingOptions.java

@@ -25,9 +25,13 @@ import org.hibernate.cache.spi.access.AccessType;
 import org.hibernate.cfg.AttributeConverterDefinition;
 import org.hibernate.cfg.MetadataSourceType;
 import org.hibernate.dialect.function.SQLFunction;
+import org.hibernate.engine.config.spi.ConfigurationService;
 
 import org.jboss.jandex.IndexView;
 
+import static org.hibernate.cfg.AvailableSettings.JPA_ID_GENERATOR_GLOBAL_SCOPE_COMPLIANCE;
+import static org.hibernate.engine.config.spi.StandardConverters.BOOLEAN;
+
 /**
  * Describes the options used while building the Metadata object (during
  * {@link org.hibernate.boot.MetadataBuilder#build()} processing).
@@ -227,4 +231,13 @@ public interface MetadataBuildingOptions {
 //	 * @return The select resolver strategy
 //	 */
 //	PersistentAttributeMemberResolver getPersistentAttributeMemberResolver();
+
+	default boolean isJpaGeneratorGlobalScopeComplianceEnabled() {
+		ConfigurationService cfgService = getServiceRegistry().getService( ConfigurationService.class );
+		return cfgService.getSetting(
+				JPA_ID_GENERATOR_GLOBAL_SCOPE_COMPLIANCE,
+				BOOLEAN,
+				false
+		);
+	}
 }

hibernate-core/src/main/java/org/hibernate/cfg/AnnotationBinder.java

@@ -10,6 +10,7 @@ import java.lang.annotation.Annotation;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -122,7 +123,6 @@ import org.hibernate.annotations.TypeDef;
 import org.hibernate.annotations.TypeDefs;
 import org.hibernate.annotations.Where;
 import org.hibernate.annotations.common.reflection.ClassLoadingException;
-import org.hibernate.annotations.common.reflection.ReflectionManager;
 import org.hibernate.annotations.common.reflection.XAnnotatedElement;
 import org.hibernate.annotations.common.reflection.XClass;
 import org.hibernate.annotations.common.reflection.XMethod;
@@ -131,6 +131,7 @@ import org.hibernate.annotations.common.reflection.XProperty;
 import org.hibernate.boot.model.IdGeneratorStrategyInterpreter;
 import org.hibernate.boot.model.IdentifierGeneratorDefinition;
 import org.hibernate.boot.model.TypeDefinition;
+import org.hibernate.boot.spi.InFlightMetadataCollector;
 import org.hibernate.boot.spi.InFlightMetadataCollector.EntityTableXref;
 import org.hibernate.boot.spi.MetadataBuildingContext;
 import org.hibernate.cfg.annotations.CollectionBinder;
@@ -2270,17 +2271,30 @@ public final class AnnotationBinder {
 					foreignGeneratorBuilder.addParam( "property", mapsIdProperty.getPropertyName() );
 
 					final IdentifierGeneratorDefinition foreignGenerator = foreignGeneratorBuilder.build();
-//					Map<String, IdentifierGeneratorDefinition> localGenerators = ( HashMap<String, IdentifierGeneratorDefinition> ) classGenerators.clone();
-//					localGenerators.put( foreignGenerator.getName(), foreignGenerator );
 
-					SecondPass secondPass = new IdGeneratorResolverSecondPass(
+					if ( isGlobalGeneratorNameGlobal( context ) ) {
-							( SimpleValue ) propertyBinder.getValue(),
+						SecondPass secondPass = new IdGeneratorResolverSecondPass(
-							foreignGenerator.getStrategy(),
+								(SimpleValue) propertyBinder.getValue(),
-							foreignGenerator.getName(),
+								foreignGenerator.getStrategy(),
-							context,
+								foreignGenerator.getName(),
-							foreignGenerator
+								context,
-					);
+								foreignGenerator
-					context.getMetadataCollector().addSecondPass( secondPass );
+						);
+						context.getMetadataCollector().addSecondPass( secondPass );
+					}
+					else {
+						Map<String, IdentifierGeneratorDefinition> localGenerators = (HashMap<String, IdentifierGeneratorDefinition>) classGenerators
+								.clone();
+						localGenerators.put( foreignGenerator.getName(), foreignGenerator );
+
+						BinderHelper.makeIdGenerator(
+								(SimpleValue) propertyBinder.getValue(),
+								foreignGenerator.getStrategy(),
+								foreignGenerator.getName(),
+								context,
+								localGenerators
+						);
+					}
 				}
 				if ( isId ) {
 					//components and regular basic types create SimpleValue objects
@@ -2338,6 +2352,10 @@ public final class AnnotationBinder {
 		}
 	}
 
+	private static boolean isGlobalGeneratorNameGlobal(MetadataBuildingContext context) {
+		return context.getBuildingOptions().isJpaGeneratorGlobalScopeComplianceEnabled();
+	}
+
 	private static boolean isToManyAssociationWithinEmbeddableCollection(PropertyHolder propertyHolder) {
 		if(propertyHolder instanceof ComponentPropertyHolder) {
 			ComponentPropertyHolder componentPropertyHolder = (ComponentPropertyHolder) propertyHolder;
@@ -2369,10 +2387,6 @@ public final class AnnotationBinder {
 		}
 		XClass entityXClass = inferredData.getClassOrElement();
 		XProperty idXProperty = inferredData.getProperty();
-		//clone classGenerator and override with local values
-//		HashMap<String, IdentifierGeneratorDefinition> localGenerators = ( HashMap<String, IdentifierGeneratorDefinition> ) classGenerators.clone();
-//		localGenerators.putAll( buildGenerators( idXProperty, buildingContext ) );
-		buildGenerators( idXProperty, buildingContext );
 
 		//manage composite related metadata
 		//guess if its a component and find id data access (property, field etc)
@@ -2391,13 +2405,29 @@ public final class AnnotationBinder {
 			generatorType = "assigned";
 		}
 
-		SecondPass secondPass = new IdGeneratorResolverSecondPass(
+		if ( isGlobalGeneratorNameGlobal( buildingContext ) ) {
-				idValue,
+			buildGenerators( idXProperty, buildingContext );
-				generatorType,
+			SecondPass secondPass = new IdGeneratorResolverSecondPass(
-				generatorName,
+					idValue,
-				buildingContext
+					generatorType,
-		);
+					generatorName,
-		buildingContext.getMetadataCollector().addSecondPass( secondPass );
+					buildingContext
+			);
+			buildingContext.getMetadataCollector().addSecondPass( secondPass );
+		}
+		else {
+			//clone classGenerator and override with local values
+			HashMap<String, IdentifierGeneratorDefinition> localGenerators = (HashMap<String, IdentifierGeneratorDefinition>) classGenerators
+					.clone();
+			localGenerators.putAll( buildGenerators( idXProperty, buildingContext ) );
+			BinderHelper.makeIdGenerator(
+					idValue,
+					generatorType,
+					generatorName,
+					buildingContext,
+					localGenerators
+			);
+		}
 
 		if ( LOG.isTraceEnabled() ) {
 			LOG.tracev( "Bind {0} on {1}", ( isComponent ? "@EmbeddedId" : "@Id" ), inferredData.getPropertyName() );
@@ -2728,26 +2758,37 @@ public final class AnnotationBinder {
 			XProperty property = propertyAnnotatedElement.getProperty();
 			if ( property.isAnnotationPresent( GeneratedValue.class ) &&
 					property.isAnnotationPresent( Id.class ) ) {
-				//clone classGenerator and override with local values
-//				Map<String, IdentifierGeneratorDefinition> localGenerators = new HashMap<>();
-//				localGenerators.putAll( buildGenerators( property, buildingContext ) );
-
-				buildGenerators( property, buildingContext );
 				GeneratedValue generatedValue = property.getAnnotation( GeneratedValue.class );
 				String generatorType = generatedValue != null
 						? generatorType( generatedValue, buildingContext, property.getType() )
 						: "assigned";
-				String generator = generatedValue != null ? generatedValue.generator() : BinderHelper.ANNOTATION_STRING_DEFAULT;
+				String generator = generatedValue != null ?
+						generatedValue.generator() :
+						BinderHelper.ANNOTATION_STRING_DEFAULT;
 
-				SecondPass secondPass = new IdGeneratorResolverSecondPass(
+				if ( isGlobalGeneratorNameGlobal( buildingContext ) ) {
-						( SimpleValue ) comp.getProperty( property.getName() ).getValue(),
+					buildGenerators( property, buildingContext );
-						generatorType,
+					SecondPass secondPass = new IdGeneratorResolverSecondPass(
-						generator,
+							(SimpleValue) comp.getProperty( property.getName() ).getValue(),
-						buildingContext
+							generatorType,
-				);
+							generator,
-				buildingContext.getMetadataCollector().addSecondPass( secondPass );
+							buildingContext
+					);
+					buildingContext.getMetadataCollector().addSecondPass( secondPass );
+				}
+				else {
+					//clone classGenerator and override with local values
+					Map<String, IdentifierGeneratorDefinition> localGenerators = new HashMap<>();
+					localGenerators.putAll( buildGenerators( property, buildingContext ) );
+					BinderHelper.makeIdGenerator(
+							(SimpleValue) comp.getProperty( property.getName() ).getValue(),
+							generatorType,
+							generator,
+							buildingContext,
+							localGenerators
+					);
+				}
 			}
-
 		}
 		return comp;
 	}
@@ -2845,13 +2886,24 @@ public final class AnnotationBinder {
 			id = value.make();
 		}
 		rootClass.setIdentifier( id );
-		SecondPass secondPass = new IdGeneratorResolverSecondPass(
+		if ( isGlobalGeneratorNameGlobal( buildingContext ) ) {
-				id,
+			SecondPass secondPass = new IdGeneratorResolverSecondPass(
-				generatorType,
+					id,
-				generatorName,
+					generatorType,
-				buildingContext
+					generatorName,
-		);
+					buildingContext
-		buildingContext.getMetadataCollector().addSecondPass( secondPass );
+			);
+			buildingContext.getMetadataCollector().addSecondPass( secondPass );
+		}
+		else {
+			BinderHelper.makeIdGenerator(
+					id,
+					generatorType,
+					generatorName,
+					buildingContext,
+					Collections.emptyMap()
+			);
+		}
 
 		if ( isEmbedded ) {
 			rootClass.setEmbeddedIdentifier( inferredData.getPropertyClass() == null );
@@ -3348,6 +3400,7 @@ public final class AnnotationBinder {
 	}
 
 	private static HashMap<String, IdentifierGeneratorDefinition> buildGenerators(XAnnotatedElement annElt, MetadataBuildingContext context) {
+		InFlightMetadataCollector metadataCollector = context.getMetadataCollector();
 		HashMap<String, IdentifierGeneratorDefinition> generators = new HashMap<>();
 		TableGenerator tabGen = annElt.getAnnotation( TableGenerator.class );
 		SequenceGenerator seqGen = annElt.getAnnotation( SequenceGenerator.class );
@@ -3355,20 +3408,19 @@ public final class AnnotationBinder {
 		if ( tabGen != null ) {
 			IdentifierGeneratorDefinition idGen = buildIdGenerator( tabGen, context );
 			generators.put( idGen.getName(), idGen );
+			metadataCollector.addIdentifierGenerator( idGen );
+
 		}
 		if ( seqGen != null ) {
 			IdentifierGeneratorDefinition idGen = buildIdGenerator( seqGen, context );
 			generators.put( idGen.getName(), idGen );
+			metadataCollector.addIdentifierGenerator( idGen );
 		}
 		if ( genGen != null ) {
 			IdentifierGeneratorDefinition idGen = buildIdGenerator( genGen, context );
 			generators.put( idGen.getName(), idGen );
+			metadataCollector.addIdentifierGenerator( idGen );
 		}
-
-		generators.forEach( (name, idGenerator) -> {
-			context.getMetadataCollector().addIdentifierGenerator( idGenerator );
-		} );
-
 		return generators;
 	}
 
@@ -3387,7 +3439,6 @@ public final class AnnotationBinder {
 	public static Map<XClass, InheritanceState> buildInheritanceStates(
 			List<XClass> orderedClasses,
 			MetadataBuildingContext buildingContext) {
-		ReflectionManager reflectionManager = buildingContext.getBuildingOptions().getReflectionManager();
 		Map<XClass, InheritanceState> inheritanceStatePerClass = new HashMap<XClass, InheritanceState>(
 				orderedClasses.size()
 		);

hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java

@@ -1741,6 +1741,17 @@ public interface AvailableSettings {
 	 */
 	String JPA_PROXY_COMPLIANCE = "hibernate.jpa.compliance.proxy";
 
+	/**
+	 * Determine if the scope of {@link javax.persistence.TableGenerator#name()} and {@link javax.persistence.SequenceGenerator#name()} should be
+	 * considered globally or locally defined.
+	 *
+	 * If enabled, the names will considered globally scoped so defining two different generators with the same name
+	 * will cause a name collision and an exception will be thrown during the bootstrap phase.
+	 *
+	 * @since 5.2.17
+	 */
+	String JPA_ID_GENERATOR_GLOBAL_SCOPE_COMPLIANCE = "hibernate.jpa.compliance.global_id_generators";
+
 	/**
 	 * Raises an exception when in-memory pagination over collection fetch is about to be performed.
 	 * Disabled by default. Set to true to enable.

hibernate-core/src/main/java/org/hibernate/cfg/annotations/IdBagBinder.java

@@ -107,13 +107,24 @@ public class IdBagBinder extends BagBinder {
 				generatorType = null;
 			}
 
-			SecondPass secondPass = new IdGeneratorResolverSecondPass(
+			if ( buildingContext.getBuildingOptions().isJpaGeneratorGlobalScopeComplianceEnabled() ) {
-					id,
+				SecondPass secondPass = new IdGeneratorResolverSecondPass(
-					generatorType,
+						id,
-					generator,
+						generatorType,
-					getBuildingContext()
+						generator,
-			);
+						getBuildingContext()
-			buildingContext.getMetadataCollector().addSecondPass( secondPass );
+				);
+				buildingContext.getMetadataCollector().addSecondPass( secondPass );
+			}
+			else {
+				BinderHelper.makeIdGenerator(
+						id,
+						generatorType,
+						generator,
+						getBuildingContext(),
+						localGenerators
+				);
+			}
 		}
 		return result;
 	}

migration-guide.adoc

@@ -173,44 +173,10 @@ set clobfield = lo_from_bytea( 0, cast( clobfield as bytea ) ),
 
 === Change in the `@TableGenerator` and `@SequenceGenerator` name scope
 
-In order to be compliant with the JPA specification, from 5.2.13 generators names are considered global (e.g. https://hibernate.atlassian.net/browse/HHH-12157[HHH-12157]) .
+[IMPORTANT]
-Configuring two generators, even if with different types but with the same name will now cause a `java.lang.IllegalArgumentException' to be thrown at boot time.
+From 5.2.13 the id generator name scope was considered global but realizing this change may cause troubles for few existing projects (https://hibernate.atlassian.net/browse/HHH-12454[HHH-12454]), starting *from 5.2.17* the scope of the id generators names
-
+will be considered local by default (which is the pre-5.2.13 behavior) and a new configuration setting `hibernate.jpa.compliance.global_id_generators`
-For example, the following mappings are no longer valid:
+can be used to enable the JPA compliant global scoping.
-
-[source,java]
-----
-@Entity
-@TableGenerator(name = "ID_GENERATOR", ... )
-public class FirstEntity {
-    ....
-}
-
-@Entity
-@TableGenerator(name = "ID_GENERATOR", ... )
-public class SecondEntity {
-    ....
-}
-----
-
-or
-
-[source,java]
-----
-@Entity
-@TableGenerator(name = "ID_GENERATOR", ... )
-public class FirstEntity {
-    ....
-}
-
-@Entity
-@SequenceGenerator(name="ID_GENERATOR", ... )
-public class SecondEntity {
-    ....
-}
-----
-
-The solution is to make all generators unique so that there are no two generators with the same name.
 
 == Misc