reorganize code in AbstractMultiNaturalIdLoader and AbstractMultiIdEntityLoader

hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractMultiIdEntityLoader.java

@@ -92,27 +92,28 @@ private List<T> performUnorderedMultiLoad(
 		assert !loadOptions.isOrderReturnEnabled();
 		assert ids != null;
 		if ( MULTI_KEY_LOAD_LOGGER.isTraceEnabled() ) {
-			MULTI_KEY_LOAD_LOGGER.tracef( "#performUnorderedMultiLoad(`%s`, ..)", getLoadable().getEntityName() );
+			MULTI_KEY_LOAD_LOGGER.tracef( "Unordered MultiLoad starting: "
+					+ getLoadable().getEntityName() );
 		}
-		return unorderedMultiLoad( ids, loadOptions, lockOptions( loadOptions ), session );
+		return unorderedMultiLoad( ids, loadOptions, session );
 	}
 
-	protected List<T> performOrderedMultiLoad(
+	private List<T> performOrderedMultiLoad(
 			Object[] ids,
 			MultiIdLoadOptions loadOptions,
 			SharedSessionContractImplementor session) {
 		assert loadOptions.isOrderReturnEnabled();
 		assert ids != null;
 		if ( MULTI_KEY_LOAD_LOGGER.isTraceEnabled() ) {
-			MULTI_KEY_LOAD_LOGGER.tracef( "#performOrderedMultiLoad(`%s`, ..)", getLoadable().getEntityName() );
+			MULTI_KEY_LOAD_LOGGER.tracef( "Ordered MultiLoad starting: "
+					+ getLoadable().getEntityName() );
 		}
-		return orderedMultiLoad( ids, loadOptions, lockOptions( loadOptions ), session );
+		return orderedMultiLoad( ids, loadOptions, session );
 	}
 
 	private List<T> orderedMultiLoad(
 			Object[] ids,
 			MultiIdLoadOptions loadOptions,
-			LockOptions lockOptions,
 			SharedSessionContractImplementor session) {
 		final boolean idCoercionEnabled = isIdCoercionEnabled();
 		final JavaType<?> idType = getLoadable().getIdentifierMapping().getJavaType();
@@ -124,6 +125,8 @@ private List<T> orderedMultiLoad(
 		final List<Object> idsInBatch = new ArrayList<>();
 		final List<Integer> elementPositionsLoadedByBatch = new ArrayList<>();
 
+		final LockOptions lockOptions = lockOptions( loadOptions );
+
 		for ( int i = 0; i < ids.length; i++ ) {
 			final Object id = idCoercionEnabled ? idType.coerce( ids[i], session ) : ids[i];
 			final EntityKey entityKey = new EntityKey( id, getLoadable().getEntityPersister() );
@@ -158,15 +161,15 @@ private List<T> orderedMultiLoad(
 		return (List<T>) result;
 	}
 
-	protected static LockOptions lockOptions(MultiIdLoadOptions loadOptions) {
+	private static LockOptions lockOptions(MultiIdLoadOptions loadOptions) {
 		return loadOptions.getLockOptions() == null
 				? new LockOptions( LockMode.NONE )
 				: loadOptions.getLockOptions();
 	}
 
 	protected abstract int maxBatchSize(Object[] ids, MultiIdLoadOptions loadOptions);
 
-	protected void handleResults(
+	private void handleResults(
 			MultiIdLoadOptions loadOptions,
 			SharedSessionContractImplementor session,
 			List<Integer> elementPositionsLoadedByBatch,
@@ -199,7 +202,7 @@ protected abstract void loadEntitiesById(
 			MultiIdLoadOptions loadOptions,
 			SharedSessionContractImplementor session);
 
-	protected boolean loadFromEnabledCaches(
+	private boolean loadFromEnabledCaches(
 			MultiIdLoadOptions loadOptions,
 			SharedSessionContractImplementor session,
 			Object id,
@@ -250,14 +253,14 @@ private boolean isLoadFromCaches(
 	protected List<T> unorderedMultiLoad(
 			Object[] ids,
 			MultiIdLoadOptions loadOptions,
-			LockOptions lockOptions,
 			SharedSessionContractImplementor session) {
+		final LockOptions lockOptions = lockOptions( loadOptions );
 		final List<T> result = arrayList( ids.length );
 		final Object[] unresolvableIds =
 				resolveInCachesIfEnabled( ids, loadOptions, lockOptions, session,
 						(position, entityKey, resolvedRef) -> result.add( (T) resolvedRef ) );
 		if ( !isEmpty( unresolvableIds ) ) {
-			loadEntitiesWithUnresolvedIds( loadOptions, lockOptions, session, unresolvableIds, result );
+			loadEntitiesWithUnresolvedIds( unresolvableIds, loadOptions, lockOptions, result, session );
 			final BatchFetchQueue batchFetchQueue = session.getPersistenceContextInternal().getBatchFetchQueue();
 			final EntityPersister persister = getLoadable().getEntityPersister();
 			for ( Object id : unresolvableIds ) {
@@ -273,13 +276,13 @@ protected List<T> unorderedMultiLoad(
 	}
 
 	protected abstract void loadEntitiesWithUnresolvedIds(
+			Object[] unresolvableIds,
 			MultiIdLoadOptions loadOptions,
 			LockOptions lockOptions,
-			SharedSessionContractImplementor session,
-			Object[] unresolvableIds,
-			List<T> results);
+			List<T> results,
+			SharedSessionContractImplementor session);
 
-	protected final <R> Object[] resolveInCachesIfEnabled(
+	private <R> Object[] resolveInCachesIfEnabled(
 			Object[] ids,
 			@NonNull MultiIdLoadOptions loadOptions,
 			@NonNull LockOptions lockOptions,
@@ -296,7 +299,7 @@ protected final <R> Object[] resolveInCachesIfEnabled(
 				: ids;
 	}
 
-	protected final <R> Object[] resolveInCaches(
+	private <R> Object[] resolveInCaches(
 			Object[] ids,
 			MultiIdLoadOptions loadOptions,
 			LockOptions lockOptions,

hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractMultiNaturalIdLoader.java

@@ -13,141 +13,152 @@
 import org.hibernate.loader.ast.spi.MultiNaturalIdLoadOptions;
 import org.hibernate.loader.ast.spi.MultiNaturalIdLoader;
 import org.hibernate.metamodel.mapping.EntityMappingType;
+import org.hibernate.metamodel.mapping.NaturalIdMapping;
 
-import java.util.Collections;
 import java.util.List;
 
+import static java.util.Collections.emptyList;
 import static org.hibernate.internal.util.collections.CollectionHelper.arrayList;
 import static org.hibernate.internal.util.collections.CollectionHelper.isEmpty;
 import static org.hibernate.loader.ast.internal.LoaderHelper.upgradeLock;
+import static org.hibernate.loader.ast.internal.MultiKeyLoadLogging.MULTI_KEY_LOAD_LOGGER;
 
 /**
  * @author Jan Schatteman
  */
 public abstract class AbstractMultiNaturalIdLoader<E> implements MultiNaturalIdLoader<E> {
 	private final EntityMappingType entityDescriptor;
 
-	protected MultiNaturalIdLoadOptions options;
-
 	public AbstractMultiNaturalIdLoader(EntityMappingType entityDescriptor) {
 		this.entityDescriptor = entityDescriptor;
 	}
 
 	@Override
 	public <K> List<E> multiLoad(K[] naturalIds, MultiNaturalIdLoadOptions options, SharedSessionContractImplementor session) {
 		assert naturalIds != null;
-
-		this.options = options;
-
 		if ( naturalIds.length == 0 ) {
-			return Collections.emptyList();
+			return emptyList();
+		}
+		else {
+			return options.isOrderReturnEnabled()
+					? performOrderedMultiLoad( naturalIds, options, session )
+					: performUnorderedMultiLoad( naturalIds, options, session );
 		}
-
-		return options.isOrderReturnEnabled()
-				? performOrderedMultiLoad( naturalIds, options, session )
-				: performUnorderedMultiLoad( naturalIds, options, session );
 	}
 
-	private <K> List<E> performUnorderedMultiLoad(K[] naturalIds, MultiNaturalIdLoadOptions options, SharedSessionContractImplementor session) {
-		if ( MultiKeyLoadLogging.MULTI_KEY_LOAD_LOGGER.isTraceEnabled() ) {
-			MultiKeyLoadLogging.MULTI_KEY_LOAD_LOGGER.tracef( "Unordered MultiLoad Starting - `%s`", getEntityDescriptor().getEntityName() );
+	private <K> List<E> performUnorderedMultiLoad(
+			K[] naturalIds,
+			MultiNaturalIdLoadOptions loadOptions,
+			SharedSessionContractImplementor session) {
+		if ( MULTI_KEY_LOAD_LOGGER.isTraceEnabled() ) {
+			MULTI_KEY_LOAD_LOGGER.tracef( "Unordered MultiLoad starting: "
+					+ getEntityDescriptor().getEntityName() );
 		}
+		return unorderedMultiLoad( naturalIds, loadOptions, session );
+	}
 
-		return unorderedMultiLoad(
-				naturalIds,
-				session,
-				options.getLockOptions() == null ? LockOptions.NONE : options.getLockOptions()
-		);
+	private static LockOptions lockOptions(MultiNaturalIdLoadOptions loadOptions) {
+		final LockOptions lockOptions = loadOptions.getLockOptions();
+		return lockOptions == null ? LockOptions.NONE : lockOptions;
 	}
 
-	protected <K> List<E> unorderedMultiLoad(K[] naturalIds, SharedSessionContractImplementor session, LockOptions lockOptions) {
+	private <K> List<E> unorderedMultiLoad(
+			K[] naturalIds,
+			MultiNaturalIdLoadOptions loadOptions,
+			SharedSessionContractImplementor session) {
 		final List<E> results = arrayList( naturalIds.length );
+		final LockOptions lockOptions = lockOptions( loadOptions );
 		final Object[] unresolvedIds =
-				checkPersistenceContextForCachedResults( naturalIds, session, lockOptions, results );
+				checkPersistenceContextForCachedResults( naturalIds, loadOptions, session, lockOptions, results );
 		if ( !isEmpty( unresolvedIds ) ) {
-			results.addAll( loadEntitiesWithUnresolvedIds(unresolvedIds, session, lockOptions) );
+			results.addAll( loadEntitiesWithUnresolvedIds( unresolvedIds, loadOptions, lockOptions, session ) );
 		}
-
 		return results;
 	}
 
-	protected abstract List<E> loadEntitiesWithUnresolvedIds(Object[] unresolvedIds, SharedSessionContractImplementor session, LockOptions lockOptions);
+	protected abstract List<E> loadEntitiesWithUnresolvedIds(
+			Object[] unresolvedIds,
+			MultiNaturalIdLoadOptions loadOptions,
+			LockOptions lockOptions,
+			SharedSessionContractImplementor session);
 
 	private <K> List<E> performOrderedMultiLoad(K[] naturalIds, MultiNaturalIdLoadOptions options, SharedSessionContractImplementor session) {
-		if ( MultiKeyLoadLogging.MULTI_KEY_LOAD_LOGGER.isTraceEnabled() ) {
-			MultiKeyLoadLogging.MULTI_KEY_LOAD_LOGGER.tracef( "Ordered MultiLoad Starting - `%s`", getEntityDescriptor().getEntityName() );
+		if ( MULTI_KEY_LOAD_LOGGER.isTraceEnabled() ) {
+			MULTI_KEY_LOAD_LOGGER.trace( "Ordered MultiLoad starting: "
+					+ getEntityDescriptor().getEntityName() );
 		}
-
-		return orderedMultiLoad(
-				naturalIds,
-				session,
-				options.getLockOptions() == null ? LockOptions.NONE : options.getLockOptions()
-		);
+		return orderedMultiLoad( naturalIds, options, session );
 	}
 
-	protected <K> List<E> orderedMultiLoad( K[] naturalIds, SharedSessionContractImplementor session, LockOptions lockOptions ) {
-
-		unorderedMultiLoad( naturalIds, session, lockOptions );
-
-		return sortResults( naturalIds, session, lockOptions );
+	private <K> List<E> orderedMultiLoad(
+			K[] naturalIds,
+			MultiNaturalIdLoadOptions loadOptions,
+			SharedSessionContractImplementor session) {
+		unorderedMultiLoad( naturalIds, loadOptions, session );
+		return sortResults( naturalIds, loadOptions, session );
 	}
 
-	protected <K> List<E> sortResults( K[] naturalIds, SharedSessionContractImplementor session, LockOptions lockOptions ) {
-		List<E> results = arrayList(naturalIds.length);
-		for ( int i = 0; i < naturalIds.length; i++ ) {
-			final PersistenceContext persistenceContext = session.getPersistenceContextInternal();
-
-			Object id = persistenceContext.getNaturalIdResolutions().findCachedIdByNaturalId( naturalIds[i], getEntityDescriptor() );
-
-			// Id can be null if a non-existent natural id is requested
-			Object entity = (id == null) ? null : persistenceContext.getEntity( new EntityKey( id, getEntityDescriptor().getEntityPersister() ) );
-			if ( entity != null && !options.isReturnOfDeletedEntitiesEnabled() ) {
-				// make sure it is not DELETED
-				final EntityEntry entry = persistenceContext.getEntry( entity );
-				if ( entry.getStatus().isDeletedOrGone() ) {
-					// the entity is locally deleted, and the options ask that we not return such entities...
-					entity = null;
-				}
-				else {
-					entity = persistenceContext.proxyFor( entity );
-				}
+	private <K> List<E> sortResults(
+			K[] naturalIds,
+			MultiNaturalIdLoadOptions loadOptions,
+			SharedSessionContractImplementor session) {
+		final PersistenceContext context = session.getPersistenceContextInternal();
+		final List<E> results = arrayList( naturalIds.length );
+		for ( K naturalId : naturalIds ) {
+			final Object entity = entityForNaturalId( context, naturalId );
+			final Object result;
+			if ( entity == null
+				// the entity is locally deleted, and the options ask that we not return such entities
+				|| !loadOptions.isReturnOfDeletedEntitiesEnabled()
+					&& context.getEntry( entity ).getStatus().isDeletedOrGone() ) {
+				result = null;
 			}
-			results.add( (E) entity );
+			else {
+				result = context.proxyFor( entity );
+			}
+			results.add( (E) result );
 		}
-
 		return results;
 	}
 
-	private <K> Object[] checkPersistenceContextForCachedResults( K[] naturalIds, SharedSessionContractImplementor session, LockOptions lockOptions, List<E> results ) {
-		List<K> unresolvedIds = arrayList(naturalIds.length);
-
-		final PersistenceContext persistenceContext = session.getPersistenceContextInternal();
-		for ( int i = 0; i < naturalIds.length; i++ ) {
-
-			final Object normalizedNaturalId = getEntityDescriptor().getNaturalIdMapping().normalizeInput( naturalIds[i] );
-			Object id = persistenceContext.getNaturalIdResolutions().findCachedIdByNaturalId( normalizedNaturalId, getEntityDescriptor() );
-
-			// Id can be null if a non-existent natural id is requested, or a mutable natural id was changed and then deleted
-			Object entity = id == null ? null : persistenceContext.getEntity( new EntityKey( id, getEntityDescriptor().getEntityPersister() ) );
+	private <K> Object entityForNaturalId(PersistenceContext context, K naturalId) {
+		final EntityMappingType descriptor = getEntityDescriptor();
+		final Object id = context.getNaturalIdResolutions().findCachedIdByNaturalId( naturalId, descriptor );
+		// id can be null if a non-existent natural id is requested, or a mutable natural id was changed and then deleted
+		return id == null ? null : context.getEntity( new EntityKey( id, descriptor.getEntityPersister() ) );
+	}
 
+	private <K> Object[] checkPersistenceContextForCachedResults(
+			K[] naturalIds,
+			MultiNaturalIdLoadOptions loadOptions,
+			SharedSessionContractImplementor session,
+			LockOptions lockOptions,
+			List<E> results ) {
+		final List<K> unresolvedIds = arrayList( naturalIds.length );
+		final PersistenceContext context = session.getPersistenceContextInternal();
+		final NaturalIdMapping naturalIdMapping = getEntityDescriptor().getNaturalIdMapping();
+		for ( K naturalId : naturalIds ) {
+			final Object entity = entityForNaturalId( context, naturalIdMapping.normalizeInput( naturalId ) );
 			if ( entity != null ) {
 				// Entity is already in the persistence context
-				final EntityEntry entry = persistenceContext.getEntry( entity );
-				// either a managed entry, or a deleted one with returnDeleted enabled
-				if ( !entry.getStatus().isDeletedOrGone() || options.isReturnOfDeletedEntitiesEnabled() ) {
-					results.add( (E) persistenceContext.proxyFor(entity) );
-					upgradeLock( entity, entry, lockOptions, session.getSession().asEventSource() );
+				final EntityEntry entry = context.getEntry( entity );
+				if ( loadOptions.isReturnOfDeletedEntitiesEnabled()
+						|| !entry.getStatus().isDeletedOrGone() ) {
+					// either a managed entry, or a deleted one with returnDeleted enabled
+					upgradeLock( entity, entry, lockOptions, session );
+					final Object result = context.proxyFor( entity );
+					results.add( (E) result );
 				}
 			}
 			else {
-				// entity either doesn't exist or hasn't been loaded in the PC yet, in both cases we add the natural id
-				// to the ids that still need to be recovered, in case the id corresponds to a non-existent
-				// instance nothing will be in the results for it, which is ok in unordered multiload
-				unresolvedIds.add(naturalIds[i]);
+				// entity either doesn't exist or hasn't been loaded in the PC yet, in both cases we add
+				// the natural id to the ids that still need to be recovered, in case the id corresponds
+				// to a nonexistent instance nothing will be in the results for it, which is ok in
+				// unordered multiload
+				unresolvedIds.add( naturalId );
 			}
 		}
-
-		return unresolvedIds.toArray( new Object[0] );
+		return unresolvedIds.toArray();
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdEntityLoaderArrayParam.java

@@ -126,11 +126,11 @@ public LockOptions getLockOptions() {
 
 	@Override
 	protected void loadEntitiesWithUnresolvedIds(
+			Object[] unresolvableIds,
 			MultiIdLoadOptions loadOptions,
 			LockOptions lockOptions,
-			SharedSessionContractImplementor session,
-			Object[] unresolvableIds,
-			List<E> result) {
+			List<E> result,
+			SharedSessionContractImplementor session) {
 		final SelectStatement sqlAst = createSelectBySingleArrayParameter(
 				getLoadable(),
 				getIdentifierMapping(),

hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdEntityLoaderInPredicate.java

@@ -174,11 +174,11 @@ private List<T> performSingleMultiLoad(Object id, LockOptions lockOptions, Share
 
 	@Override
 	protected void loadEntitiesWithUnresolvedIds(
+			Object[] unresolvableIds,
 			MultiIdLoadOptions loadOptions,
 			LockOptions lockOptions,
-			SharedSessionContractImplementor session,
-			Object[] unresolvableIds,
-			List<T> result) {
+			List<T> result,
+			SharedSessionContractImplementor session) {
 		final int maxBatchSize = maxBatchSize( unresolvableIds, loadOptions );
 		int numberOfIdsLeft = unresolvableIds.length;
 		int idPosition = 0;

hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiNaturalIdLoaderArrayParam.java

@@ -9,6 +9,7 @@
 import org.hibernate.LockOptions;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
+import org.hibernate.loader.ast.spi.MultiNaturalIdLoadOptions;
 import org.hibernate.loader.ast.spi.SqlArrayMultiKeyLoader;
 import org.hibernate.metamodel.mapping.EntityMappingType;
 import org.hibernate.metamodel.mapping.JdbcMapping;
@@ -44,7 +45,11 @@ protected BasicAttributeMapping getNaturalIdAttribute()  {
 	}
 
 	@Override
-	public List<E> loadEntitiesWithUnresolvedIds(Object[] naturalIds, SharedSessionContractImplementor session, LockOptions lockOptions) {
+	public List<E> loadEntitiesWithUnresolvedIds(
+			Object[] naturalIds,
+			MultiNaturalIdLoadOptions loadOptions,
+			LockOptions lockOptions,
+			SharedSessionContractImplementor session) {
 
 		final SessionFactoryImplementor sessionFactory = session.getFactory();