improve javadoc for Generator hierarchy

and make SelectGenerator a subclass of IdentityGenerator

hibernate-core/src/main/java/org/hibernate/id/IdentifierGenerator.java

@@ -23,8 +23,9 @@ import static org.hibernate.tuple.GenerationTiming.INSERT;
 
 /**
  * A classic extension point from the very earliest days of Hibernate,
- * this interface is now no longer the only way to generate identifiers.
+ * this interface is no longer the only way to generate identifiers. Any
- * Any {@link InMemoryGenerator} may now be used.
+ * {@link InMemoryGenerator} with timing {@link GenerationTiming#INSERT}
+ * may now be used.
  * <p>
  * This interface extends {@code InMemoryGenerator} with some additional
  * machinery for {@linkplain #configure configuration}, and for caching
@@ -54,6 +55,7 @@ import static org.hibernate.tuple.GenerationTiming.INSERT;
  *
  * @author Gavin King
  *
+ * @see PostInsertIdentifierGenerator
  * @see PersistentIdentifierGenerator
  */
 public interface IdentifierGenerator extends InMemoryGenerator, ExportableProducer, Configurable {
@@ -94,8 +96,7 @@ public interface IdentifierGenerator extends InMemoryGenerator, ExportableProduc
 	 * @throws MappingException If configuration fails.
 	 */
 	@Override
-	default void configure(Type type, Properties parameters, ServiceRegistry serviceRegistry) {
+	default void configure(Type type, Properties parameters, ServiceRegistry serviceRegistry) {}
-	}
 
 	/**
 	 * Register database objects used by this identifier generator,
@@ -107,8 +108,7 @@ public interface IdentifierGenerator extends InMemoryGenerator, ExportableProduc
 	 * @param database The database instance
 	 */
 	@Override
-	default void registerExportables(Database database) {
+	default void registerExportables(Database database) {}
-	}
 
 	/**
 	 * Initializes this instance, in particular pre-generates
@@ -120,8 +120,7 @@ public interface IdentifierGenerator extends InMemoryGenerator, ExportableProduc
 	 *
 	 * @param context A context to help generate SQL strings
 	 */
-	default void initialize(SqlStringGenerationContext context) {
+	default void initialize(SqlStringGenerationContext context) {}
-	}
 
 	/**
 	 * Generate a new identifier.
@@ -135,11 +134,19 @@ public interface IdentifierGenerator extends InMemoryGenerator, ExportableProduc
 	 */
 	Object generate(SharedSessionContractImplementor session, Object object);
 
+	/**
+	 * Generate a value.
+	 * <p>
+	 * The {@code currentValue} is usually null for id generation.
+	 */
 	@Override
 	default Object generate(SharedSessionContractImplementor session, Object owner, Object currentValue) {
 		return generate( session, owner );
 	}
 
+	/**
+	 * @return {@link GenerationTiming#INSERT}
+	 */
 	@Override
 	default GenerationTiming getGenerationTiming() {
 		return INSERT;

hibernate-core/src/main/java/org/hibernate/id/IdentityGenerator.java

@@ -6,22 +6,29 @@
  */
 package org.hibernate.id;
 
+import org.hibernate.dialect.Dialect;
 import org.hibernate.id.factory.spi.StandardGenerator;
-import org.hibernate.id.insert.InsertGeneratedIdentifierDelegate;
+import org.hibernate.tuple.InDatabaseGenerator;
-import org.hibernate.persister.entity.EntityPersister;
 
 /**
- * A generator for use with ANSI-SQL IDENTITY columns used as the primary key.
+ * An {@link InDatabaseGenerator} that handles {@code IDENTITY}/"autoincrement" columns
- * The IdentityGenerator for autoincrement/identity key generation.
+ * on those databases which support them.
  * <p>
- * Indicates to the {@code Session} that identity (i.e. identity/autoincrement
+ * Delegates to the {@link org.hibernate.dialect.identity.IdentityColumnSupport} provided
- * column) key generation should be used.
+ * by the {@linkplain Dialect#getIdentityColumnSupport() dialect}.
- *
- * @implNote Most of the functionality of this generator is delegated to
- * 		     {@link InsertGeneratedIdentifierDelegate}.
  *
  * @author Christoph Sturm
  */
 public class IdentityGenerator
 		implements PostInsertIdentifierGenerator, BulkInsertionCapableIdentifierGenerator, StandardGenerator {
+	@Override
+	public boolean referenceColumnsInSql(Dialect dialect) {
+		return dialect.getIdentityColumnSupport().hasIdentityInsertKeyword();
+	}
+
+	@Override
+	public String[] getReferencedColumnValues(Dialect dialect) {
+		return new String[] { dialect.getIdentityColumnSupport().getIdentityInsertString() };
+	}
+
 }

hibernate-core/src/main/java/org/hibernate/id/PostInsertIdentifierGenerator.java

@@ -6,7 +6,6 @@
  */
 package org.hibernate.id;
 
-import org.hibernate.dialect.Dialect;
 import org.hibernate.service.ServiceRegistry;
 import org.hibernate.tuple.GenerationTiming;
 import org.hibernate.tuple.InDatabaseGenerator;
@@ -17,31 +16,37 @@ import java.util.Properties;
 import static org.hibernate.tuple.GenerationTiming.INSERT;
 
 /**
+ * The counterpart of {@link IdentifierGenerator} for values generated by the database.
+ * This interface is no longer the only way to handle database-generate identifiers.
+ * Any {@link InDatabaseGenerator} with timing {@link GenerationTiming#INSERT} may now
+ * be used.
+ *
+ * @see IdentifierGenerator
+ *
  * @author Gavin King
  */
 public interface PostInsertIdentifierGenerator extends InDatabaseGenerator, Configurable {
 
+	/**
+	 * @return {@link GenerationTiming#INSERT}
+	 */
 	@Override
 	default GenerationTiming getGenerationTiming() {
 		return INSERT;
 	}
 
+	/**
+	 * @return {@code false}, since we don't usually have a meaningful property value
+	 *         for generated identifiers
+	 */
 	@Override
 	default boolean writePropertyValue() {
 		return false;
 	}
 
-	@Override
+	/**
-	default boolean referenceColumnsInSql(Dialect dialect) {
+	 * Noop default implementation. May be overridden by subtypes.
-		return dialect.getIdentityColumnSupport().hasIdentityInsertKeyword();
+	 */
-	}
-
-	@Override
-	default String[] getReferencedColumnValues(Dialect dialect) {
-		return new String[] { dialect.getIdentityColumnSupport().getIdentityInsertString() };
-	}
-
 	@Override
 	default void configure(Type type, Properties params, ServiceRegistry serviceRegistry) {}
-
 }

hibernate-core/src/main/java/org/hibernate/id/SelectGenerator.java

@@ -8,28 +8,30 @@ package org.hibernate.id;
 
 import java.util.Properties;
 
-import org.hibernate.id.factory.spi.StandardGenerator;
 import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.service.ServiceRegistry;
 import org.hibernate.type.Type;
 
 /**
- * A generator that {@code select}s the just-inserted row to determine the identifier
+ * A generator that {@code select}s the just-{@code insert}ed row to determine the
- * value assigned by the database. The correct row is located using a unique key of
+ * {@code IDENTITY} column value assigned by the database. The correct row is located
- * the entity, either:
+ * using a unique key of the entity, either:
  * <ul>
  * <li>the mapped {@linkplain org.hibernate.annotations.NaturalId} of the entity, or
  * <li>a property specified using the parameter named {@code "key"}.
  * </ul>
  * The second approach is provided for backward compatibility with older versions of
  * Hibernate.
+ * <p>
+ * Arguably, this class breaks the natural separation of responsibility between the
+ * {@linkplain org.hibernate.tuple.InDatabaseGenerator generator} and the coordinating
+ * code, since it's role is to specify how the generated value is <em>retrieved</em>.
  *
  * @see org.hibernate.annotations.NaturalId
  *
  * @author Gavin King
  */
-public class SelectGenerator
+public class SelectGenerator extends IdentityGenerator {
-		implements PostInsertIdentifierGenerator, BulkInsertionCapableIdentifierGenerator, StandardGenerator {
 	private String uniqueKeyPropertyName;
 
 	@Override

hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/GeneratedValuesProcessor.java

@@ -37,7 +37,18 @@ import org.hibernate.sql.results.spi.ListResultsConsumer;
 import org.hibernate.tuple.GenerationTiming;
 import org.hibernate.tuple.Generator;
 
+import static org.hibernate.sql.results.spi.ListResultsConsumer.UniqueSemantic.FILTER;
+
 /**
+ * Responsible for retrieving {@linkplain org.hibernate.tuple.InDatabaseGenerator
+ * database-generated} attribute values after an {@code insert} statement is executed.
+ * <p>
+ * Note that this class has responsibility for regular attributes of the entity. The
+ * primary key / id attribute is handled separately, being the responsibility of an
+ * instance of {@link org.hibernate.id.insert.InsertGeneratedIdentifierDelegate}.
+ *
+ * @see org.hibernate.tuple.InDatabaseGenerator
+ *
  * @author Steve Ebersole
  */
 @Incubating
@@ -56,12 +67,7 @@ public class GeneratedValuesProcessor {
 		this.entityDescriptor = entityDescriptor;
 		this.sessionFactory = sessionFactory;
 
-		// NOTE: we only care about db-generated values here. in-memory generation
+		final List<AttributeMapping> generatedValuesToSelect = getGeneratedAttributes( entityDescriptor, timing );
-		// is applied before the insert/update happens.
-
-		// todo (6.0): for now, we rely on the entity metamodel as composite attributes report
-		//             GenerationTiming.NEVER even if they have attributes that would need generation
-		final List<AttributeMapping> generatedValuesToSelect = getGeneratedValues( entityDescriptor, timing );
 		if ( generatedValuesToSelect.isEmpty() ) {
 			selectStatement = null;
 		}
@@ -80,7 +86,14 @@ public class GeneratedValuesProcessor {
 		}
 	}
 
-	private List<AttributeMapping> getGeneratedValues(EntityMappingType entityDescriptor, GenerationTiming timing) {
+	/**
+	 * Find attributes generated by a {@link org.hibernate.tuple.InDatabaseGenerator},
+	 * populate the list of {@link GeneratedValueDescriptor}s by side effect, and
+	 * return a list of {@link AttributeMapping}s.
+	 */
+	private List<AttributeMapping> getGeneratedAttributes(EntityMappingType entityDescriptor, GenerationTiming timing) {
+		// todo (6.0): For now, we rely on the entity metamodel as composite attributes report
+		//             GenerationTiming.NEVER even if they have attributes that would need generation
 		final Generator[] generators = entityDescriptor.getEntityPersister().getEntityMetamodel().getGenerators();
 		final List<AttributeMapping> generatedValuesToSelect = new ArrayList<>();
 		entityDescriptor.visitAttributeMappings( mapping -> {
@@ -99,15 +112,28 @@ public class GeneratedValuesProcessor {
 		return generatedValuesToSelect;
 	}
 
+	/**
+	 * Obtain the generated values, and populate the snapshot and the fields of the entity instance.
+	 */
 	public void processGeneratedValues(Object entity, Object id, Object[] state, SharedSessionContractImplementor session) {
-		if ( selectStatement == null ) {
+		if ( selectStatement != null ) {
-			return;
+			final List<Object[]> results = executeSelect( id, session );
+			assert results.size() == 1;
+			setEntityAttributes( entity, state, session, results.get(0) );
 		}
+	}
 
-		final JdbcServices jdbcServices = sessionFactory.getJdbcServices();
+	private List<Object[]> executeSelect(Object id, SharedSessionContractImplementor session) {
-		final JdbcEnvironment jdbcEnvironment = jdbcServices.getJdbcEnvironment();
+		final JdbcParameterBindings jdbcParamBindings = getJdbcParameterBindings( id, session );
-		final SqlAstTranslatorFactory sqlAstTranslatorFactory = jdbcEnvironment.getSqlAstTranslatorFactory();
+		final JdbcOperationQuerySelect jdbcSelect =
+				sessionFactory.getJdbcServices().getJdbcEnvironment().getSqlAstTranslatorFactory()
+						.buildSelectTranslator( sessionFactory, selectStatement )
+						.translate( jdbcParamBindings, QueryOptions.NONE );
+		return session.getFactory().getJdbcServices().getJdbcSelectExecutor()
+				.list( jdbcSelect, jdbcParamBindings, createExecutionContext( session ), (row) -> row, FILTER );
+	}
 
+	private JdbcParameterBindings getJdbcParameterBindings(Object id, SharedSessionContractImplementor session) {
 		final JdbcParameterBindings jdbcParamBindings = new JdbcParameterBindingsImpl( jdbcParameters.size() );
 		int offset = jdbcParamBindings.registerParametersForEachJdbcValue(
 				id,
@@ -117,50 +143,18 @@ public class GeneratedValuesProcessor {
 				session
 		);
 		assert offset == jdbcParameters.size();
-		final JdbcOperationQuerySelect jdbcSelect = sqlAstTranslatorFactory
+		return jdbcParamBindings;
-				.buildSelectTranslator( sessionFactory, selectStatement )
+	}
-				.translate( jdbcParamBindings, QueryOptions.NONE );
-
-		final List<Object[]> results = session.getFactory().getJdbcServices().getJdbcSelectExecutor().list(
-				jdbcSelect,
-				jdbcParamBindings,
-				new ExecutionContext() {
-					@Override
-					public SharedSessionContractImplementor getSession() {
-						return session;
-					}
-
-					@Override
-					public QueryOptions getQueryOptions() {
-						return QueryOptions.NONE;
-					}
-
-					@Override
-					public String getQueryIdentifier(String sql) {
-						return sql;
-					}
-
-					@Override
-					public QueryParameterBindings getQueryParameterBindings() {
-						return QueryParameterBindings.NO_PARAM_BINDINGS;
-					}
-
-					@Override
-					public Callback getCallback() {
-						throw new UnsupportedMappingException( "Follow-on locking not supported yet" );
-					}
-
-				},
-				(row) -> row,
-				ListResultsConsumer.UniqueSemantic.FILTER
-		);
-
-		assert results.size() == 1;
-		final Object[] dbSelectionResults = results.get( 0 );
 
+	private void setEntityAttributes(
+			Object entity,
+			Object[] state,
+			SharedSessionContractImplementor session,
+			Object[] selectionResults) {
 		for ( int i = 0; i < valueDescriptors.size(); i++ ) {
 			final GeneratedValueDescriptor descriptor = valueDescriptors.get( i );
-			final Object generatedValue = descriptor.resolver.resolveGeneratedValue( dbSelectionResults, entity, session, state[i] );
+			final Object generatedValue =
+					descriptor.resolver.resolveGeneratedValue( selectionResults, entity, session, state[i] );
 			state[ descriptor.attribute.getStateArrayPosition() ] = generatedValue;
 			descriptor.attribute.getAttributeMetadataAccess()
 					.resolveAttributeMetadata( entityDescriptor )
@@ -170,6 +164,35 @@ public class GeneratedValuesProcessor {
 		}
 	}
 
+	private static ExecutionContext createExecutionContext(SharedSessionContractImplementor session) {
+		return new ExecutionContext() {
+			@Override
+			public SharedSessionContractImplementor getSession() {
+				return session;
+			}
+
+			@Override
+			public QueryOptions getQueryOptions() {
+				return QueryOptions.NONE;
+			}
+
+			@Override
+			public String getQueryIdentifier(String sql) {
+				return sql;
+			}
+
+			@Override
+			public QueryParameterBindings getQueryParameterBindings() {
+				return QueryParameterBindings.NO_PARAM_BINDINGS;
+			}
+
+			@Override
+			public Callback getCallback() {
+				throw new UnsupportedMappingException("Follow-on locking not supported yet");
+			}
+		};
+	}
+
 	private static class GeneratedValueDescriptor {
 		public final GeneratedValueResolver resolver;
 		public final AttributeMapping attribute;

hibernate-core/src/main/java/org/hibernate/tuple/InDatabaseGenerator.java

@@ -19,6 +19,11 @@ import org.hibernate.persister.entity.EntityPersister;
  * Implementations should override {@link #referenceColumnsInSql(Dialect)},
  * {@link #writePropertyValue()}, and {@link #getReferencedColumnValues(Dialect)} as needed
  * in order to achieve the desired behavior.
+ * <p>
+ * In implementation of this interface does not specify how the generated value is retrieved
+ * from the database after it is generated, this being the responsibility of the coordinating
+ * code in {@link org.hibernate.metamodel.mapping.internal.GeneratedValuesProcessor} or in an
+ * implementation of {@link org.hibernate.id.insert.InsertGeneratedIdentifierDelegate}.
  *
  * @author Steve Ebersole
  *
@@ -63,6 +68,17 @@ public interface InDatabaseGenerator extends Generator {
 	 */
 	String[] getReferencedColumnValues(Dialect dialect);
 
+	/**
+	 * The name of a property of the entity which may be used to locate the just-{@code insert}ed
+	 * row containing the generated value. Of course, the columns mapped by this property should
+	 * form a unique key of the entity.
+	 * <p>
+	 * This is ignored by {@link org.hibernate.metamodel.mapping.internal.GeneratedValuesProcessor},
+	 * which handles multiple generators at once. This method arguably breaks the separation of
+	 * concerns between the generator and the coordinating code.
+	 *
+	 * @see org.hibernate.id.SelectGenerator
+	 */
 	default String getUniqueKeyPropertyName(EntityPersister persister) {
 		return null;
 	}