rework collection loading state management - remove the "temp" load collections in various PersistentCollection impls

hibernate-core/src/main/java/org/hibernate/collection/internal/AbstractPersistentCollection.java

@@ -557,6 +557,10 @@ public abstract class AbstractPersistentCollection implements Serializable, Pers
 		return afterInitialize();
 	}
 
+	@Override
+	public void beforeInitialize(CollectionPersister persister, int anticipatedSize) {
+	}
+
 	@Override
 	public boolean afterInitialize() {
 		setInitialized();

hibernate-core/src/main/java/org/hibernate/collection/internal/PersistentArrayHolder.java

@@ -189,22 +189,12 @@ public class PersistentArrayHolder extends AbstractPersistentCollection {
 		return elements();
 	}
 
-	@Override
-	public void beginRead() {
-		super.beginRead();
-	}
-
 	@Override
 	public boolean endRead() {
 		setInitialized();
 		return true;
 	}
 
-	@Override
-	public void beforeInitialize(CollectionPersister persister, int anticipatedSize) {
-		//if (tempList==null) throw new UnsupportedOperationException("Can't lazily initialize arrays");
-	}
-
 	@Override
 	public boolean isDirectlyAccessible() {
 		return true;

hibernate-core/src/main/java/org/hibernate/collection/internal/PersistentBag.java

@@ -124,10 +124,11 @@ public class PersistentBag extends AbstractPersistentCollection implements List
 		return bag.iterator();
 	}
 
-	public void injectLoadedState(PluralAttributeMapping collectionAttributeMapping, List loadingState) {
+	public void injectLoadedState(PluralAttributeMapping attributeMapping, List loadingState) {
-		assert isInitializing();
+		assert bag == null;
-		assert bag != null;
+
-		assert bag.isEmpty();
+		final CollectionPersister collectionDescriptor = attributeMapping.getCollectionDescriptor();
+		this.bag = (List) collectionDescriptor.getCollectionSemantics().instantiateRaw( loadingState.size(), collectionDescriptor );
 
 		for ( int i = 0; i < loadingState.size(); i++ ) {
 			//noinspection unchecked,UseBulkOperation
@@ -135,11 +136,6 @@ public class PersistentBag extends AbstractPersistentCollection implements List
 		}
 	}
 
-	@Override
-	public void beforeInitialize(CollectionPersister persister, int anticipatedSize) {
-		this.bag = (List) persister.getCollectionType().instantiate( anticipatedSize );
-	}
-
 	@Override
 	@SuppressWarnings("unchecked")
 	public boolean equalsSnapshot(CollectionPersister persister) throws HibernateException {
@@ -276,13 +272,17 @@ public class PersistentBag extends AbstractPersistentCollection implements List
 
 	@Override
 	@SuppressWarnings("unchecked")
-	public void initializeFromCache(CollectionPersister persister, Object disassembled, Object owner)
+	public void initializeFromCache(CollectionPersister collectionDescriptor, Object disassembled, Object owner)
 			throws HibernateException {
+		assert bag == null;
+
 		final Serializable[] array = (Serializable[]) disassembled;
 		final int size = array.length;
-		beforeInitialize( persister, size );
+
+		this.bag = (List) collectionDescriptor.getCollectionSemantics().instantiateRaw( size, collectionDescriptor );
+
 		for ( Serializable item : array ) {
-			final Object element = persister.getElementType().assemble( item, getSession(), owner );
+			final Object element = collectionDescriptor.getElementType().assemble( item, getSession(), owner );
 			if ( element != null ) {
 				bag.add( element );
 			}

hibernate-core/src/main/java/org/hibernate/collection/internal/PersistentIdentifierBag.java

@@ -20,8 +20,6 @@ import org.hibernate.engine.spi.SessionImplementor;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.metamodel.mapping.PluralAttributeMapping;
 import org.hibernate.persister.collection.CollectionPersister;
-import org.hibernate.sql.results.graph.DomainResultAssembler;
-import org.hibernate.sql.results.jdbc.spi.RowProcessingState;
 import org.hibernate.type.Type;
 
 /**
@@ -110,7 +108,15 @@ public class PersistentIdentifierBag extends AbstractPersistentCollection implem
 			throws HibernateException {
 		final Serializable[] array = (Serializable[]) disassembled;
 		final int size = array.length;
-		beforeInitialize( persister, size );
+
+		assert identifiers == null;
+		assert values == null;
+
+		identifiers = new HashMap<>();
+		values = size <= 0
+				? new ArrayList<>()
+				: new ArrayList<>( size );
+
 		for ( int i = 0; i < size; i+=2 ) {
 			identifiers.put(
 				(i/2),
@@ -236,16 +242,6 @@ public class PersistentIdentifierBag extends AbstractPersistentCollection implem
 		return values.toArray( a );
 	}
 
-	@Override
-	public void beforeInitialize(CollectionPersister persister, int anticipatedSize) {
-		identifiers = anticipatedSize <= 0
-				? new HashMap<>()
-				: new HashMap<>( anticipatedSize + 1 + (int) ( anticipatedSize * .75f ), .75f );
-		values = anticipatedSize <= 0
-				? new ArrayList<>()
-				: new ArrayList<>( anticipatedSize );
-	}
-
 	@Override
 	public Object disassemble(CollectionPersister persister) {
 		final Object[] result = new Object[ values.size() * 2 ];
@@ -354,30 +350,6 @@ public class PersistentIdentifierBag extends AbstractPersistentCollection implem
 		return old != null && elemType.isDirty( old, entry, getSession() );
 	}
 
-	@Override
-	public Object readFrom(
-			RowProcessingState rowProcessingState,
-			DomainResultAssembler elementAssembler,
-			DomainResultAssembler indexAssembler,
-			DomainResultAssembler identifierAssembler,
-			Object owner) throws HibernateException {
-		assert indexAssembler == null;
-		final Object element = elementAssembler.assemble( rowProcessingState );
-		final Object identifier = identifierAssembler.assemble( rowProcessingState );
-
-		final Object old = identifiers.put(
-				values.size(),
-				identifier
-		);
-
-		if ( old == null ) {
-			//maintain correct duplication if loaded in a cartesian product
-			values.add( element );
-		}
-
-		return element;
-	}
-
 	@Override
 	@SuppressWarnings("unchecked")
 	public Serializable getSnapshot(CollectionPersister persister) throws HibernateException {
@@ -525,21 +497,12 @@ public class PersistentIdentifierBag extends AbstractPersistentCollection implem
 		//TODO: if we are using identity columns, fetch the identifier
 	}
 
-	public void load(Object identifier, Object element) {
-		assert isInitializing();
-		Object old = identifiers.put( values.size(), identifier );
-		if ( old == null ) {
-			//maintain correct duplication if loaded in a cartesian product
-			values.add( element );
-		}
-	}
-
 	public void injectLoadedState(PluralAttributeMapping attributeMapping, List loadingState) {
-		assert isInitializing();
+		assert identifiers == null;
-		assert identifiers != null;
+		assert values == null;
-		assert identifiers.isEmpty();
+
-		assert values != null;
+		identifiers = new HashMap<>();
-		assert values.isEmpty();
+		values = new ArrayList<>( loadingState.size() );
 
 		for ( int i = 0; i < loadingState.size(); i++ ) {
 			final Object[] row = (Object[]) loadingState.get( i );

hibernate-core/src/main/java/org/hibernate/collection/internal/PersistentList.java

@@ -121,17 +121,34 @@ public class PersistentList extends AbstractPersistentCollection implements List
 	}
 
 	@Override
-	public void beforeInitialize(CollectionPersister persister, int anticipatedSize) {
+	@SuppressWarnings("unchecked")
-		this.list = (List) persister.getCollectionType().instantiate( anticipatedSize );
+	public void initializeFromCache(CollectionPersister persister, Object disassembled, Object owner)
+			throws HibernateException {
+		final Serializable[] array = (Serializable[]) disassembled;
+		final int size = array.length;
+
+		assert list == null;
+		this.list = (List) persister.getCollectionType().instantiate( size );
+
+		for ( Serializable arrayElement : array ) {
+			list.add( persister.getElementType().assemble( arrayElement, getSession(), owner ) );
+		}
 	}
 
-	public void load(int index, Object element) {
+	@Override
+	public void injectLoadedState(PluralAttributeMapping attributeMapping, List loadingStateList) {
 		assert isInitializing();
-		// todo (6.0) : we need to account for base - but it is not exposed from collection descriptor nor attribute
+		assert list == null;
-		for ( int i = list.size(); i <= index; ++i ) {
+
-			list.add( i, null );
+		final CollectionPersister collectionDescriptor = attributeMapping.getCollectionDescriptor();
-		}
+
-		list.set( index, element );
+		this.list = (List) collectionDescriptor.getCollectionSemantics().instantiateRaw(
+				loadingStateList.size(),
+				collectionDescriptor
+		);
+
+		//noinspection unchecked
+		list.addAll( loadingStateList );
 	}
 
 	@Override
@@ -397,18 +414,6 @@ public class PersistentList extends AbstractPersistentCollection implements List
 		return list.iterator();
 	}
 
-	@Override
-	@SuppressWarnings("unchecked")
-	public void initializeFromCache(CollectionPersister persister, Object disassembled, Object owner)
-			throws HibernateException {
-		final Serializable[] array = (Serializable[]) disassembled;
-		final int size = array.length;
-		beforeInitialize( persister, size );
-		for ( Serializable arrayElement : array ) {
-			list.add( persister.getElementType().assemble( arrayElement, getSession(), owner ) );
-		}
-	}
-
 	@Override
 	public Object disassemble(CollectionPersister persister) throws HibernateException {
 		final int length = list.size();
@@ -493,14 +498,6 @@ public class PersistentList extends AbstractPersistentCollection implements List
 		return entry!=null;
 	}
 
-	public void injectLoadedState(PluralAttributeMapping collectionAttributeMapping, List loadingStateList) {
-		assert isInitializing();
-		assert list != null;
-
-		//noinspection unchecked
-		list.addAll( loadingStateList );
-	}
-
 	final class Clear implements DelayedOperation {
 		@Override
 		public void operate() {

hibernate-core/src/main/java/org/hibernate/collection/internal/PersistentMap.java

@@ -136,16 +136,6 @@ public class PersistentMap extends AbstractPersistentCollection implements Map {
 		return map==collection;
 	}
 
-	@Override
-	public void beforeInitialize(CollectionPersister persister, int anticipatedSize) {
-		this.map = (Map) persister.getCollectionType().instantiate( anticipatedSize );
-	}
-
-	public void load(Object key, Object value) {
-		assert isInitializing();
-		map.put( key, value );
-	}
-
 	@Override
 	public int size() {
 		return readSize() ? getCachedSize() : map.size();
@@ -273,19 +263,17 @@ public class PersistentMap extends AbstractPersistentCollection implements Map {
 		return map.toString();
 	}
 
-	@Override
-	public boolean endRead() {
-		return super.endRead();
-	}
-
 	@Override
 	public Iterator entries(CollectionPersister persister) {
 		return map.entrySet().iterator();
 	}
 
-	public void injectLoadedState(PluralAttributeMapping collectionAttributeMapping, List loadingState) {
+	public void injectLoadedState(PluralAttributeMapping attributeMapping, List loadingState) {
 		assert isInitializing();
-		assert map != null;
+		assert map == null;
+
+		final CollectionPersister collectionDescriptor = attributeMapping.getCollectionDescriptor();
+		this.map = (Map) collectionDescriptor.getCollectionSemantics().instantiateRaw( loadingState.size(), collectionDescriptor );
 
 		for ( int i = 0; i < loadingState.size(); i++ ) {
 			final Object[] keyVal = (Object[]) loadingState.get( i );
@@ -294,6 +282,37 @@ public class PersistentMap extends AbstractPersistentCollection implements Map {
 		}
 	}
 
+	@Override
+	@SuppressWarnings("unchecked")
+	public void initializeFromCache(CollectionPersister persister, Object disassembled, Object owner)
+			throws HibernateException {
+		final Serializable[] array = (Serializable[]) disassembled;
+		final int size = array.length;
+
+		this.map = (Map) persister.getCollectionSemantics().instantiateRaw( size, persister );
+
+		for ( int i = 0; i < size; i+=2 ) {
+			map.put(
+					persister.getIndexType().assemble( array[i], getSession(), owner ),
+					persister.getElementType().assemble( array[i+1], getSession(), owner )
+			);
+		}
+	}
+
+	@Override
+	public Object disassemble(CollectionPersister persister) throws HibernateException {
+		final Object[] result = new Object[ map.size() * 2 ];
+		final Iterator itr = map.entrySet().iterator();
+		int i=0;
+		while ( itr.hasNext() ) {
+			final Map.Entry e = (Map.Entry) itr.next();
+			result[i++] = persister.getIndexType().disassemble( e.getKey(), getSession(), null );
+			result[i++] = persister.getElementType().disassemble( e.getValue(), getSession(), null );
+		}
+		return result;
+
+	}
+
 	/**
 	 * a wrapper for Map.Entry sets
 	 */
@@ -456,35 +475,6 @@ public class PersistentMap extends AbstractPersistentCollection implements Map {
 		}
 	}
 
-	@Override
-	@SuppressWarnings("unchecked")
-	public void initializeFromCache(CollectionPersister persister, Object disassembled, Object owner)
-			throws HibernateException {
-		final Serializable[] array = (Serializable[]) disassembled;
-		final int size = array.length;
-		beforeInitialize( persister, size );
-		for ( int i = 0; i < size; i+=2 ) {
-			map.put(
-					persister.getIndexType().assemble( array[i], getSession(), owner ),
-					persister.getElementType().assemble( array[i+1], getSession(), owner )
-			);
-		}
-	}
-
-	@Override
-	public Object disassemble(CollectionPersister persister) throws HibernateException {
-		final Object[] result = new Object[ map.size() * 2 ];
-		final Iterator itr = map.entrySet().iterator();
-		int i=0;
-		while ( itr.hasNext() ) {
-			final Map.Entry e = (Map.Entry) itr.next();
-			result[i++] = persister.getIndexType().disassemble( e.getKey(), getSession(), null );
-			result[i++] = persister.getElementType().disassemble( e.getValue(), getSession(), null );
-		}
-		return result;
-
-	}
-
 	@Override
 	@SuppressWarnings("unchecked")
 	public Iterator getDeletes(CollectionPersister persister, boolean indexIsFormula) throws HibernateException {

hibernate-core/src/main/java/org/hibernate/collection/internal/PersistentSet.java

@@ -136,18 +136,15 @@ public class PersistentSet extends AbstractPersistentCollection implements java.
 		return ( (java.util.Map) snapshot ).isEmpty();
 	}
 
-	@Override
-	public void beforeInitialize(CollectionPersister persister, int anticipatedSize) {
-		this.set = (Set) persister.getCollectionType().instantiate( anticipatedSize );
-	}
-
 	@Override
 	@SuppressWarnings("unchecked")
 	public void initializeFromCache(CollectionPersister persister, Object disassembled, Object owner)
 			throws HibernateException {
 		final Serializable[] array = (Serializable[]) disassembled;
 		final int size = array.length;
-		beforeInitialize( persister, size );
+
+		this.set = (Set) persister.getCollectionSemantics().instantiateRaw( size, persister );
+
 		for ( Serializable arrayElement : array ) {
 			final Object assembledArrayElement = persister.getElementType().assemble( arrayElement, getSession(), owner );
 			if ( assembledArrayElement != null ) {
@@ -329,6 +326,7 @@ public class PersistentSet extends AbstractPersistentCollection implements java.
 			PluralAttributeMapping attributeMapping,
 			List loadingStateList) {
 		final CollectionPersister collectionDescriptor = attributeMapping.getCollectionDescriptor();
+
 		this.set = (Set) attributeMapping.getCollectionDescriptor().getCollectionSemantics().instantiateRaw(
 				loadingStateList.size(),
 				collectionDescriptor

hibernate-core/src/main/java/org/hibernate/collection/spi/PersistentCollection.java

@@ -15,8 +15,6 @@ import org.hibernate.HibernateException;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.metamodel.mapping.PluralAttributeMapping;
 import org.hibernate.persister.collection.CollectionPersister;
-import org.hibernate.sql.results.graph.DomainResultAssembler;
-import org.hibernate.sql.results.jdbc.spi.RowProcessingState;
 import org.hibernate.type.Type;
 
 /**
@@ -91,31 +89,6 @@ public interface PersistentCollection {
 	 */
 	Object getValue();
 
-	/**
-	 * Called just before reading any rows from the JDBC result set
-	 *
-	 * todo (6.0) remove these.  should no longer be a need with change in how collections are initialized
-	 */
-	void beginRead();
-
-	/**
-	 * Called after reading all rows from the JDBC result set
-	 *
-	 * @return Whether to end the read.
-	 *
-	 * todo (6.0) remove these.  should no longer be a need with change in how collections are initialized
-	 */
-	boolean endRead();
-
-	/**
-	 * Called after initializing from cache
-	 *
-	 * @return ??
-	 *
-	 * todo (6.0) remove these.  should no longer be a need with change in how collections are initialized
-	 */
-	boolean afterInitialize();
-
 	/**
 	 * Could the application possibly have a direct reference to
 	 * the underlying collection implementation?
@@ -145,14 +118,6 @@ public interface PersistentCollection {
 	 */
 	boolean setCurrentSession(SharedSessionContractImplementor session) throws HibernateException;
 
-	/**
-	 * Read the state of the collection from a disassembled cached value
-	 *  @param persister The collection persister
-	 * @param disassembled The disassembled cached state
-	 * @param owner The collection owner
-	 */
-	void initializeFromCache(CollectionPersister persister, Object disassembled, Object owner);
-
 	/**
 	 * Iterate all collection entries, during update of the database
 	 *
@@ -162,23 +127,6 @@ public interface PersistentCollection {
 	 */
 	Iterator entries(CollectionPersister persister);
 
-	/**
-	 * Read a row from the JDBC values
-	 *
-	 * @throws HibernateException Generally indicates a problem resolving data read from the ResultSet
-	 *
-	 *
-	  todo (6.0) remove these.  should no longer be a need with change in how collections are initialized
-	 */
-	default Object readFrom(
-			RowProcessingState rowProcessingState,
-			DomainResultAssembler elementAssembler,
-			DomainResultAssembler indexAssembler,
-			DomainResultAssembler identifierAssembler,
-			Object owner) throws HibernateException {
-		return null;
-	}
-
 	/**
 	 * Get the identifier of the given collection entry.  This refers to the collection identifier, not the
 	 * identifier of the (possibly) entity elements.  This is only valid for invocation on the
@@ -222,15 +170,6 @@ public interface PersistentCollection {
 	 */
 	Object getSnapshotElement(Object entry, int i);
 
-	/**
-	 * Called before any elements are read into the collection,
-	 * allowing appropriate initializations to occur.
-	 *
-	 * @param persister The underlying collection persister.
-	 * @param anticipatedSize The anticipated size of the collection after initialization is complete.
-	 */
-	void beforeInitialize(CollectionPersister persister, int anticipatedSize);
-
 	/**
 	 * Does the current state exactly match the snapshot?
 	 *
@@ -250,15 +189,6 @@ public interface PersistentCollection {
 	 */
 	boolean isSnapshotEmpty(Serializable snapshot);
 
-	/**
-	 * Disassemble the collection to get it ready for the cache
-	 *
-	 * @param persister The collection persister
-	 *
-	 * @return The disassembled state
-	 */
-	Object disassemble(CollectionPersister persister) ;
-
 	/**
 	 * Do we need to completely recreate this collection when it changes?
 	 *
@@ -353,6 +283,53 @@ public interface PersistentCollection {
 	 */
 	boolean wasInitialized();
 
+	/**
+	 * Called prior to the initialization of this yet-uninitialized collection.  Pairs
+	 * with {@link #afterInitialize}
+	 */
+	void beforeInitialize(CollectionPersister persister, int anticipatedSize);
+
+	/**
+	 * Read the state of the collection from a disassembled cached value
+	 *
+	 * @param persister The collection persister
+	 * @param disassembled The disassembled cached state
+	 * @param owner The collection owner
+	 */
+	void initializeFromCache(CollectionPersister persister, Object disassembled, Object owner);
+
+	/**
+	 * Called just before reading any rows from the JDBC result set.  Pairs with {@link #endRead}
+	 */
+	void beginRead();
+
+	/**
+	 * Inject the state loaded for a collection instance.
+	 */
+	void injectLoadedState(PluralAttributeMapping attributeMapping, List loadingState);
+
+	/**
+	 * Called after reading all rows from the JDBC result set.  Pairs with {@link #beginRead}
+	 *
+	 * @see #injectLoadedState
+	 */
+	@SuppressWarnings("UnusedReturnValue")
+	boolean endRead();
+
+	/**
+	 * Called after initialization is complete.  Pairs with {@link #beforeInitialize}
+	 */
+	boolean afterInitialize();
+
+	/**
+	 * Disassemble the collection to get it ready for the cache
+	 *
+	 * @param persister The collection persister
+	 *
+	 * @return The disassembled state
+	 */
+	Object disassemble(CollectionPersister persister) ;
+
 	/**
 	 * Does this instance have any "queued" operations?
 	 *
@@ -470,9 +447,4 @@ public interface PersistentCollection {
 	 * @return The orphans
 	 */
 	Collection getOrphans(Serializable snapshot, String entityName);
-
-	/**
-	 * Inject the state loaded for a collection instance.
-	 */
-	void injectLoadedState(PluralAttributeMapping attributeMapping, List loadingState);
 }