diff --git a/hibernate-core/src/main/java/org/hibernate/collection/internal/AbstractPersistentCollection.java b/hibernate-core/src/main/java/org/hibernate/collection/internal/AbstractPersistentCollection.java index 4573572b7b..fba620402f 100644 --- a/hibernate-core/src/main/java/org/hibernate/collection/internal/AbstractPersistentCollection.java +++ b/hibernate-core/src/main/java/org/hibernate/collection/internal/AbstractPersistentCollection.java @@ -371,12 +371,16 @@ public abstract class AbstractPersistentCollection implements Serializable, Pers return cachedSize; } - private boolean isConnectedToSession() { + protected boolean isConnectedToSession() { return session != null && session.isOpen() && session.getPersistenceContext().containsCollection( this ); } + protected boolean isInitialized() { + return initialized; + } + /** * Called by any writer method of the collection interface */ @@ -424,7 +428,7 @@ public abstract class AbstractPersistentCollection implements Serializable, Pers * Is this the "inverse" end of a bidirectional association? */ @SuppressWarnings({"JavaDoc"}) - private boolean isInverseCollection() { + protected boolean isInverseCollection() { final CollectionEntry ce = session.getPersistenceContext().getCollectionEntry( this ); return ce != null && ce.getLoadedPersister().isInverse(); } @@ -434,7 +438,7 @@ public abstract class AbstractPersistentCollection implements Serializable, Pers * no orphan delete enabled? */ @SuppressWarnings({"JavaDoc"}) - private boolean isInverseCollectionNoOrphanDelete() { + protected boolean isInverseCollectionNoOrphanDelete() { final CollectionEntry ce = session.getPersistenceContext().getCollectionEntry( this ); return ce != null && @@ -447,7 +451,7 @@ public abstract class AbstractPersistentCollection implements Serializable, Pers * of a collection with no orphan delete? */ @SuppressWarnings({"JavaDoc"}) - private boolean isInverseOneToManyOrNoOrphanDelete() { + protected boolean isInverseOneToManyOrNoOrphanDelete() { final CollectionEntry ce = session.getPersistenceContext().getCollectionEntry( this ); return ce != null && ce.getLoadedPersister().isInverse() diff --git a/hibernate-core/src/main/java/org/hibernate/collection/internal/PersistentList.java b/hibernate-core/src/main/java/org/hibernate/collection/internal/PersistentList.java index b1d96fb0b5..0ad78cf0c3 100644 --- a/hibernate-core/src/main/java/org/hibernate/collection/internal/PersistentList.java +++ b/hibernate-core/src/main/java/org/hibernate/collection/internal/PersistentList.java @@ -14,6 +14,7 @@ import java.util.Collection; import java.util.Iterator; import java.util.List; import java.util.ListIterator; +import java.util.Map; import org.hibernate.HibernateException; import org.hibernate.engine.spi.SessionImplementor; @@ -314,7 +315,12 @@ public class PersistentList extends AbstractPersistentCollection implements List if ( index < 0 ) { throw new ArrayIndexOutOfBoundsException( "negative index" ); } - if ( !isOperationQueueEnabled() ) { + if ( !isInitialized() && isConnectedToSession() ) { + // NOTE : we don't care about the inverse part here because + // even if the collection is inverse, this side is driving the + // writing of the indexes. And because this is a positioned-add + // we need to load the underlying elements to know how that + // affects overall re-ordering write(); list.add( index, value ); } diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractFlushingEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractFlushingEventListener.java index d266aa291b..ea6d2bea76 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractFlushingEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractFlushingEventListener.java @@ -289,6 +289,8 @@ public abstract class AbstractFlushingEventListener implements Serializable { ) ); } + + // todo : I'm not sure the !wasInitialized part should really be part of this check if ( !coll.wasInitialized() && coll.hasQueuedOperations() ) { actionQueue.addAction( new QueuedOperationCollectionAction( diff --git a/hibernate-core/src/test/java/org/hibernate/test/collection/delayedOperation/ListAddTest.java b/hibernate-core/src/test/java/org/hibernate/test/collection/delayedOperation/ListAddTest.java index 8c010b5f37..e99ca3c6e6 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/collection/delayedOperation/ListAddTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/collection/delayedOperation/ListAddTest.java @@ -18,16 +18,19 @@ import javax.persistence.OrderColumn; import javax.persistence.Table; import org.hibernate.Hibernate; +import org.hibernate.LazyInitializationException; import org.hibernate.Session; import org.hibernate.Transaction; +import org.hibernate.collection.spi.PersistentCollection; -import org.hibernate.testing.FailureExpected; import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase; import org.junit.After; import org.junit.Before; import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.fail; /** * @author Steve Ebersole @@ -70,31 +73,24 @@ public class ListAddTest extends BaseNonConfigCoreFunctionalTestCase { * This test fails, but shouldn't */ @Test - @FailureExpected( jiraKey = "HHH-9195" ) public void addQuestionWithIndexShouldAddQuestionAtSpecifiedIndex() { Session session = openSession(); Transaction transaction = session.beginTransaction(); - Quizz quizz = session.get( Quizz.class, 1 ); quizz.addQuestion( 1, new Question( 4, "question that should be at index 1" ) ); - transaction.commit(); session.close(); session = openSession(); transaction = session.beginTransaction(); - quizz = session.get( Quizz.class, 1); - assertEquals( 4, quizz.getQuestions().size() ); assertEquals( 4, quizz.getQuestions().get( 1 ).getId().longValue() ); - transaction.commit(); session.close(); } @Test - @FailureExpected( jiraKey = "HHH-9195" ) public void addQuestionToDetachedQuizz() { Session session = openSession(); session.beginTransaction(); @@ -102,21 +98,34 @@ public class ListAddTest extends BaseNonConfigCoreFunctionalTestCase { session.getTransaction().commit(); session.close(); - quizz.addQuestion( 1, new Question( 4, "question that should be at index 1" ) ); + assertFalse( ( (PersistentCollection) quizz.getQuestions() ).wasInitialized() ); - session = openSession(); - session.beginTransaction(); - session.merge( quizz ); - session.getTransaction().commit(); - session.close(); + try { + // this is the crux of the comment on HHH-9195 in regard to uninitialized, detached collections and + // not allowing additions + quizz.addQuestion( new Question( 4, "question 4" ) ); - session = openSession(); - session.getTransaction().begin(); - quizz = session.get( Quizz.class, 1); - assertEquals( 4, quizz.getQuestions().size() ); - assertEquals( 4, quizz.getQuestions().get( 1 ).getId().longValue() ); - session.getTransaction().commit(); - session.close(); + // indexed-addition should fail + quizz.addQuestion( 1, new Question( 5, "question that should be at index 1" ) ); + fail( "Expecting a failure" ); + } + catch (LazyInitializationException ignore) { + // expected + } + +// session = openSession(); +// session.beginTransaction(); +// session.merge( quizz ); +// session.getTransaction().commit(); +// session.close(); +// +// session = openSession(); +// session.getTransaction().begin(); +// quizz = session.get( Quizz.class, 1); +// assertEquals( 5, quizz.getQuestions().size() ); +// assertEquals( 5, quizz.getQuestions().get( 1 ).getId().longValue() ); +// session.getTransaction().commit(); +// session.close(); } /**