From cde859c57d7af8c801ebcdb29be806a18677d625 Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Tue, 21 Jun 2016 00:52:22 -0700 Subject: [PATCH] HHH-8999 : NullPointerException when updating or deleting multiple entities of same type with non-comparable IDs (cherry picked from commit 6853fdae704644a22dc77cd54bbc312db2501c27) --- .../internal/CacheDataDescriptionImpl.java | 7 + .../cache/spi/CacheDataDescription.java | 3 +- .../org/hibernate/engine/spi/ActionQueue.java | 282 ++++++++++-------- .../hibernate/engine/spi/ExecutableList.java | 43 ++- .../java/ByteArrayTypeDescriptor.java | 7 + .../java/CharacterArrayTypeDescriptor.java | 7 + .../PrimitiveByteArrayTypeDescriptor.java | 7 + ...PrimitiveCharacterArrayTypeDescriptor.java | 7 + .../spi/NonSortedExecutableListTest.java | 255 ++++++++++++++++ ...est.java => SortedExecutableListTest.java} | 27 +- .../test/id/array/ByteArrayIdTest.java | 3 - .../test/id/array/CharacterArrayIdTest.java | 3 - .../id/array/PrimitiveByteArrayIdTest.java | 3 - .../array/PrimitiveCharacterArrayIdTest.java | 3 - .../usertype/UserTypeNonComparableIdTest.java | 2 - 15 files changed, 501 insertions(+), 158 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/engine/spi/NonSortedExecutableListTest.java rename hibernate-core/src/test/java/org/hibernate/engine/spi/{ExecutableListTest.java => SortedExecutableListTest.java} (88%) diff --git a/hibernate-core/src/main/java/org/hibernate/cache/internal/CacheDataDescriptionImpl.java b/hibernate-core/src/main/java/org/hibernate/cache/internal/CacheDataDescriptionImpl.java index 1cf9292f2a..3e3f123155 100644 --- a/hibernate-core/src/main/java/org/hibernate/cache/internal/CacheDataDescriptionImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/cache/internal/CacheDataDescriptionImpl.java @@ -13,6 +13,7 @@ import org.hibernate.mapping.Collection; import org.hibernate.mapping.PersistentClass; import org.hibernate.type.Type; import org.hibernate.type.VersionType; +import org.hibernate.type.descriptor.java.IncomparableComparator; /** * Standard CacheDataDescription implementation. @@ -37,6 +38,12 @@ public class CacheDataDescriptionImpl implements CacheDataDescription { this.mutable = mutable; this.versioned = versioned; this.versionComparator = versionComparator; + if ( versioned && + ( versionComparator == null || IncomparableComparator.class.isInstance( versionComparator ) ) ) { + throw new IllegalArgumentException( + "versionComparator must not be null or an instance of " + IncomparableComparator.class.getName() + ); + } this.keyType = keyType; } diff --git a/hibernate-core/src/main/java/org/hibernate/cache/spi/CacheDataDescription.java b/hibernate-core/src/main/java/org/hibernate/cache/spi/CacheDataDescription.java index eeecaaf969..850075284c 100644 --- a/hibernate-core/src/main/java/org/hibernate/cache/spi/CacheDataDescription.java +++ b/hibernate-core/src/main/java/org/hibernate/cache/spi/CacheDataDescription.java @@ -26,7 +26,8 @@ public interface CacheDataDescription { /** * Is the data to be cached considered versioned? * - * If {@code true}, it is illegal for {@link #getVersionComparator} to return {@code null}. + * If {@code true}, it is illegal for {@link #getVersionComparator} to return {@code null} + * or an instance of {@link org.hibernate.type.descriptor.java.IncomparableComparator}. * * @return {@code true} if the data is versioned; {@code false} otherwise. */ diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java index 3c4d13da17..c89721b3ed 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java @@ -12,6 +12,7 @@ import java.io.ObjectOutputStream; import java.io.Serializable; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -62,6 +63,9 @@ public class ActionQueue { private UnresolvedEntityInsertActions unresolvedInsertions; + // NOTE: ExecutableList fields must be instantiated via ListProvider#init or #getOrInit + // to ensure that they are instantiated consistently. + // Object insertions, updates, and deletions have list semantics because // they must happen in the right order so as to respect referential // integrity @@ -88,84 +92,121 @@ public class ActionQueue { private BeforeTransactionCompletionProcessQueue beforeTransactionProcesses; /** - * An array containing providers for all the ExecutableLists in execution order + * An LinkedHashMap containing providers for all the ExecutableLists, inserted in execution order */ - private static final ListProvider[] EXECUTABLE_LISTS; - + private static final LinkedHashMap,ListProvider> EXECUTABLE_LISTS_MAP; static { - EXECUTABLE_LISTS = new ListProvider[8]; - EXECUTABLE_LISTS[0] = new ListProvider() { - ExecutableList get(ActionQueue instance) { - return instance.orphanRemovals; - } + EXECUTABLE_LISTS_MAP = new LinkedHashMap,ListProvider>( 8 ); - ExecutableList init(ActionQueue instance) { - return instance.orphanRemovals = new ExecutableList(); - } - }; - EXECUTABLE_LISTS[1] = new ListProvider() { - ExecutableList get(ActionQueue instance) { - return instance.insertions; - } - - ExecutableList init(ActionQueue instance) { - return instance.insertions = new ExecutableList( new InsertActionSorter() ); - } - }; - EXECUTABLE_LISTS[2] = new ListProvider() { - ExecutableList get(ActionQueue instance) { - return instance.updates; - } - - ExecutableList init(ActionQueue instance) { - return instance.updates = new ExecutableList(); - } - }; - EXECUTABLE_LISTS[3] = new ListProvider() { - ExecutableList get(ActionQueue instance) { - return instance.collectionQueuedOps; - } - - ExecutableList init(ActionQueue instance) { - return instance.collectionQueuedOps = new ExecutableList(); - } - }; - EXECUTABLE_LISTS[4] = new ListProvider() { - ExecutableList get(ActionQueue instance) { - return instance.collectionRemovals; - } - - ExecutableList init(ActionQueue instance) { - return instance.collectionRemovals = new ExecutableList(); - } - }; - EXECUTABLE_LISTS[5] = new ListProvider() { - ExecutableList get(ActionQueue instance) { - return instance.collectionUpdates; - } - - ExecutableList init(ActionQueue instance) { - return instance.collectionUpdates = new ExecutableList(); - } - }; - EXECUTABLE_LISTS[6] = new ListProvider() { - ExecutableList get(ActionQueue instance) { - return instance.collectionCreations; - } - - ExecutableList init(ActionQueue instance) { - return instance.collectionCreations = new ExecutableList(); - } - }; - EXECUTABLE_LISTS[7] = new ListProvider() { - ExecutableList get(ActionQueue instance) { - return instance.deletions; - } - - ExecutableList init(ActionQueue instance) { - return instance.deletions = new ExecutableList(); - } - }; + EXECUTABLE_LISTS_MAP.put( + OrphanRemovalAction.class, + new ListProvider() { + ExecutableList get(ActionQueue instance) { + return instance.orphanRemovals; + } + ExecutableList init(ActionQueue instance) { + // OrphanRemovalAction executables never require sorting. + return instance.orphanRemovals = new ExecutableList( false ); + } + } + ); + EXECUTABLE_LISTS_MAP.put( + AbstractEntityInsertAction.class, + new ListProvider() { + ExecutableList get(ActionQueue instance) { + return instance.insertions; + } + ExecutableList init(ActionQueue instance) { + if ( instance.isOrderInsertsEnabled() ) { + return instance.insertions = new ExecutableList( + new InsertActionSorter() + ); + } + else { + return instance.insertions = new ExecutableList( + false + ); + } + } + } + ); + EXECUTABLE_LISTS_MAP.put( + EntityUpdateAction.class, + new ListProvider() { + ExecutableList get(ActionQueue instance) { + return instance.updates; + } + ExecutableList init(ActionQueue instance) { + return instance.updates = new ExecutableList( + instance.isOrderUpdatesEnabled() + ); + } + } + ); + EXECUTABLE_LISTS_MAP.put( + QueuedOperationCollectionAction.class, + new ListProvider() { + ExecutableList get(ActionQueue instance) { + return instance.collectionQueuedOps; + } + ExecutableList init(ActionQueue instance) { + return instance.collectionQueuedOps = new ExecutableList( + instance.isOrderUpdatesEnabled() + ); + } + } + ); + EXECUTABLE_LISTS_MAP.put( + CollectionRemoveAction.class, + new ListProvider() { + ExecutableList get(ActionQueue instance) { + return instance.collectionRemovals; + } + ExecutableList init(ActionQueue instance) { + return instance.collectionRemovals = new ExecutableList( + instance.isOrderUpdatesEnabled() + ); + } + } + ); + EXECUTABLE_LISTS_MAP.put( + CollectionUpdateAction.class, + new ListProvider() { + ExecutableList get(ActionQueue instance) { + return instance.collectionUpdates; + } + ExecutableList init(ActionQueue instance) { + return instance.collectionUpdates = new ExecutableList( + instance.isOrderUpdatesEnabled() + ); + } + } + ); + EXECUTABLE_LISTS_MAP.put( + CollectionRecreateAction.class, + new ListProvider() { + ExecutableList get(ActionQueue instance) { + return instance.collectionCreations; + } + ExecutableList init(ActionQueue instance) { + return instance.collectionCreations = new ExecutableList( + instance.isOrderUpdatesEnabled() + ); + } + } + ); + EXECUTABLE_LISTS_MAP.put( + EntityDeleteAction.class, + new ListProvider() { + ExecutableList get(ActionQueue instance) { + return instance.deletions; + } + ExecutableList init(ActionQueue instance) { + // EntityDeleteAction executables never require sorting. + return instance.deletions = new ExecutableList( false ); + } + } + ); } /** @@ -179,8 +220,8 @@ public class ActionQueue { } public void clear() { - for ( int i = 0; i < EXECUTABLE_LISTS.length; ++i ) { - ExecutableList l = EXECUTABLE_LISTS[i].get(this); + for ( ListProvider listProvider : EXECUTABLE_LISTS_MAP.values() ) { + ExecutableList l = listProvider.get( this ); if( l != null ) { l.clear(); } @@ -233,10 +274,7 @@ public class ActionQueue { } else { LOG.trace( "Adding resolved non-early insert action." ); - if( insertions == null ) { - insertions = new ExecutableList( new InsertActionSorter() ); - } - insertions.add(insert); + addAction( AbstractEntityInsertAction.class, insert ); } insert.makeEntityManaged(); if( unresolvedInsertions != null ) { @@ -246,6 +284,11 @@ public class ActionQueue { } } + @SuppressWarnings("unchecked") + private void addAction(Class executableClass, T action) { + EXECUTABLE_LISTS_MAP.get( executableClass ).getOrInit( this ).add( action ); + } + /** * Adds an entity (IDENTITY) insert action * @@ -262,10 +305,7 @@ public class ActionQueue { * @param action The action representing the entity deletion */ public void addAction(EntityDeleteAction action) { - if( deletions == null ) { - deletions = new ExecutableList(); - } - deletions.add( action ); + addAction( EntityDeleteAction.class, action ); } /** @@ -274,10 +314,7 @@ public class ActionQueue { * @param action The action representing the orphan removal */ public void addAction(OrphanRemovalAction action) { - if( orphanRemovals == null ) { - orphanRemovals = new ExecutableList(); - } - orphanRemovals.add( action ); + addAction( OrphanRemovalAction.class, action ); } /** @@ -286,10 +323,7 @@ public class ActionQueue { * @param action The action representing the entity update */ public void addAction(EntityUpdateAction action) { - if( updates == null ) { - updates = new ExecutableList(); - } - updates.add( action ); + addAction( EntityUpdateAction.class, action ); } /** @@ -298,10 +332,7 @@ public class ActionQueue { * @param action The action representing the (re)creation of a collection */ public void addAction(CollectionRecreateAction action) { - if( collectionCreations == null) { - collectionCreations = new ExecutableList(); - } - collectionCreations.add( action ); + addAction( CollectionRecreateAction.class, action ); } /** @@ -310,10 +341,7 @@ public class ActionQueue { * @param action The action representing the removal of a collection */ public void addAction(CollectionRemoveAction action) { - if( collectionRemovals == null ) { - collectionRemovals = new ExecutableList(); - } - collectionRemovals.add( action ); + addAction( CollectionRemoveAction.class, action ); } /** @@ -322,10 +350,7 @@ public class ActionQueue { * @param action The action representing the update of a collection */ public void addAction(CollectionUpdateAction action) { - if( collectionUpdates == null ) { - collectionUpdates = new ExecutableList(); - } - collectionUpdates.add( action ); + addAction( CollectionUpdateAction.class, action ); } /** @@ -334,10 +359,7 @@ public class ActionQueue { * @param action The action representing the queued operation */ public void addAction(QueuedOperationCollectionAction action) { - if( collectionQueuedOps == null) { - collectionQueuedOps = new ExecutableList(); - } - collectionQueuedOps.add( action ); + addAction( QueuedOperationCollectionAction.class, action ); } /** @@ -428,8 +450,8 @@ public class ActionQueue { throw new IllegalStateException( "About to execute actions, but there are unresolved entity insert actions." ); } - for ( int i = 0; i < EXECUTABLE_LISTS.length; ++i ) { - ExecutableList l = EXECUTABLE_LISTS[i].get(this); + for ( ListProvider listProvider : EXECUTABLE_LISTS_MAP.values() ) { + ExecutableList l = listProvider.get( this ); if ( l != null && !l.isEmpty() ) { executeActions( l ); } @@ -503,9 +525,9 @@ public class ActionQueue { if ( tables.isEmpty() ) { return false; } - for ( int i = 0; i < EXECUTABLE_LISTS.length; ++i ) { - ExecutableList l = EXECUTABLE_LISTS[i].get(this); - if ( areTablesToBeUpdated(l, tables) ) { + for ( ListProvider listProvider : EXECUTABLE_LISTS_MAP.values() ) { + ExecutableList l = listProvider.get( this ); + if ( areTablesToBeUpdated( l, tables ) ) { return true; } } @@ -708,7 +730,7 @@ public class ActionQueue { } public void sortCollectionActions() { - if ( session.getFactory().getSessionFactoryOptions().isOrderUpdatesEnabled() ) { + if ( isOrderUpdatesEnabled() ) { // sort the updates by fk if( collectionCreations != null ) { collectionCreations.sort(); @@ -726,15 +748,23 @@ public class ActionQueue { } public void sortActions() { - if ( session.getFactory().getSessionFactoryOptions().isOrderUpdatesEnabled() && updates != null ) { + if ( isOrderUpdatesEnabled() && updates != null ) { // sort the updates by pk updates.sort(); } - if ( session.getFactory().getSessionFactoryOptions().isOrderInsertsEnabled() && insertions != null ) { + if ( isOrderInsertsEnabled() && insertions != null ) { insertions.sort(); } } + private boolean isOrderUpdatesEnabled() { + return session.getFactory().getSessionFactoryOptions().isOrderUpdatesEnabled(); + } + + private boolean isOrderInsertsEnabled() { + return session.getFactory().getSessionFactoryOptions().isOrderInsertsEnabled(); + } + public void clearFromFlushNeededCheck(int previousCollectionRemovalSize) { if( collectionCreations != null ) { collectionCreations.clear(); @@ -813,7 +843,7 @@ public class ActionQueue { } unresolvedInsertions.serialize( oos ); - for ( ListProvider p : EXECUTABLE_LISTS ) { + for ( ListProvider p : EXECUTABLE_LISTS_MAP.values() ) { ExecutableList l = p.get(this); if( l == null ) { oos.writeBoolean(false); @@ -843,8 +873,7 @@ public class ActionQueue { rtn.unresolvedInsertions = UnresolvedEntityInsertActions.deserialize( ois, session ); - for ( int i = 0; i < EXECUTABLE_LISTS.length; ++i ) { - ListProvider provider = EXECUTABLE_LISTS[i]; + for ( ListProvider provider : EXECUTABLE_LISTS_MAP.values() ) { ExecutableList l = provider.get(rtn); boolean notNull = ois.readBoolean(); if( notNull ) { @@ -1088,8 +1117,15 @@ public class ActionQueue { } - private static abstract class ListProvider { - abstract ExecutableList get(ActionQueue instance); - abstract ExecutableList init(ActionQueue instance); + private static abstract class ListProvider { + abstract ExecutableList get(ActionQueue instance); + abstract ExecutableList init(ActionQueue instance); + ExecutableList getOrInit( ActionQueue instance ) { + ExecutableList list = get( instance ); + if ( list == null ) { + list = init( instance ); + } + return list; + } } } diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/ExecutableList.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/ExecutableList.java index 4f0aa92628..af6e2db340 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/ExecutableList.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/ExecutableList.java @@ -54,6 +54,7 @@ public class ExecutableList im private final ArrayList executables; private final Sorter sorter; + private final boolean requiresSorting; private boolean sorted; /** @@ -77,7 +78,20 @@ public class ExecutableList im * @param initialCapacity The initial capacity for instantiating the internal List */ public ExecutableList(int initialCapacity) { - this( initialCapacity, null ); + // pass true for requiresSorting argument to maintain original behavior + this( initialCapacity, true ); + } + + public ExecutableList(boolean requiresSorting) { + this( INIT_QUEUE_LIST_SIZE, requiresSorting ); + } + + public ExecutableList(int initialCapacity, boolean requiresSorting) { + this.sorter = null; + this.executables = new ArrayList( initialCapacity ); + this.querySpaces = null; + this.requiresSorting = requiresSorting; + this.sorted = requiresSorting; } /** @@ -99,6 +113,8 @@ public class ExecutableList im this.sorter = sorter; this.executables = new ArrayList( initialCapacity ); this.querySpaces = null; + // require sorting by default, even if sorter is null to maintain original behavior + this.requiresSorting = true; this.sorted = true; } @@ -162,7 +178,7 @@ public class ExecutableList im public void clear() { executables.clear(); querySpaces = null; - sorted = true; + sorted = requiresSorting; } /** @@ -199,17 +215,19 @@ public class ExecutableList im return false; } - // see if the addition invalidated the sorting - if ( sorter != null ) { - // we don't have intrinsic insight into the sorter's algorithm, so invalidate sorting - sorted = false; - } - else { - // otherwise, we added to the end of the list. So check the comparison between the incoming - // executable and the one previously at the end of the list using the Comparable contract - if ( previousLast != null && previousLast.compareTo( executable ) > 0 ) { + // if it was sorted before the addition, then check if the addition invalidated the sorting + if ( sorted ) { + if ( sorter != null ) { + // we don't have intrinsic insight into the sorter's algorithm, so invalidate sorting sorted = false; } + else { + // otherwise, we added to the end of the list. So check the comparison between the incoming + // executable and the one previously at the end of the list using the Comparable contract + if ( previousLast != null && previousLast.compareTo( executable ) > 0 ) { + sorted = false; + } + } } Serializable[] querySpaces = executable.getPropertySpaces(); @@ -225,7 +243,8 @@ public class ExecutableList im */ @SuppressWarnings("unchecked") public void sort() { - if ( sorted ) { + if ( sorted || !requiresSorting ) { + // nothing to do return; } diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/ByteArrayTypeDescriptor.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/ByteArrayTypeDescriptor.java index 8dfe9e8317..465837be3e 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/ByteArrayTypeDescriptor.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/ByteArrayTypeDescriptor.java @@ -11,6 +11,7 @@ import java.io.InputStream; import java.sql.Blob; import java.sql.SQLException; import java.util.Arrays; +import java.util.Comparator; import org.hibernate.HibernateException; import org.hibernate.engine.jdbc.BinaryStream; @@ -71,6 +72,12 @@ public class ByteArrayTypeDescriptor extends AbstractTypeDescriptor { return bytes; } + @Override + @SuppressWarnings({ "unchecked" }) + public Comparator getComparator() { + return IncomparableComparator.INSTANCE; + } + @SuppressWarnings({ "unchecked" }) @Override public X unwrap(Byte[] value, Class type, WrapperOptions options) { diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/CharacterArrayTypeDescriptor.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/CharacterArrayTypeDescriptor.java index 814532ad17..ed797c7e91 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/CharacterArrayTypeDescriptor.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/CharacterArrayTypeDescriptor.java @@ -10,6 +10,7 @@ import java.io.Reader; import java.io.StringReader; import java.sql.Clob; import java.util.Arrays; +import java.util.Comparator; import org.hibernate.engine.jdbc.CharacterStream; import org.hibernate.engine.jdbc.internal.CharacterStreamImpl; @@ -51,6 +52,12 @@ public class CharacterArrayTypeDescriptor extends AbstractTypeDescriptor getComparator() { + return IncomparableComparator.INSTANCE; + } + @SuppressWarnings({ "unchecked" }) @Override public X unwrap(Character[] value, Class type, WrapperOptions options) { diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/PrimitiveByteArrayTypeDescriptor.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/PrimitiveByteArrayTypeDescriptor.java index f8515a3606..32c26e3389 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/PrimitiveByteArrayTypeDescriptor.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/PrimitiveByteArrayTypeDescriptor.java @@ -11,6 +11,7 @@ import java.io.InputStream; import java.sql.Blob; import java.sql.SQLException; import java.util.Arrays; +import java.util.Comparator; import org.hibernate.HibernateException; import org.hibernate.engine.jdbc.BinaryStream; @@ -72,6 +73,12 @@ public class PrimitiveByteArrayTypeDescriptor extends AbstractTypeDescriptor getComparator() { + return IncomparableComparator.INSTANCE; + } + @SuppressWarnings({ "unchecked" }) public X unwrap(byte[] value, Class type, WrapperOptions options) { if ( value == null ) { diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/PrimitiveCharacterArrayTypeDescriptor.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/PrimitiveCharacterArrayTypeDescriptor.java index 764969d543..86dd9032bc 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/PrimitiveCharacterArrayTypeDescriptor.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/PrimitiveCharacterArrayTypeDescriptor.java @@ -10,6 +10,7 @@ import java.io.Reader; import java.io.StringReader; import java.sql.Clob; import java.util.Arrays; +import java.util.Comparator; import org.hibernate.engine.jdbc.CharacterStream; import org.hibernate.engine.jdbc.internal.CharacterStreamImpl; @@ -51,6 +52,12 @@ public class PrimitiveCharacterArrayTypeDescriptor extends AbstractTypeDescripto return hashCode; } + @Override + @SuppressWarnings({ "unchecked" }) + public Comparator getComparator() { + return IncomparableComparator.INSTANCE; + } + @SuppressWarnings({ "unchecked" }) public X unwrap(char[] value, Class type, WrapperOptions options) { if ( value == null ) { diff --git a/hibernate-core/src/test/java/org/hibernate/engine/spi/NonSortedExecutableListTest.java b/hibernate-core/src/test/java/org/hibernate/engine/spi/NonSortedExecutableListTest.java new file mode 100644 index 0000000000..a20ff803eb --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/engine/spi/NonSortedExecutableListTest.java @@ -0,0 +1,255 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.engine.spi; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.io.Serializable; +import java.util.Iterator; +import java.util.Set; + +import org.hibernate.HibernateException; +import org.hibernate.action.spi.AfterTransactionCompletionProcess; +import org.hibernate.action.spi.BeforeTransactionCompletionProcess; +import org.hibernate.action.spi.Executable; + +import org.hibernate.testing.junit4.BaseUnitTestCase; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +/** + * @author Anton Marsden + * @author Gail Badner + */ +public class NonSortedExecutableListTest extends BaseUnitTestCase { + + // For testing, we need an Executable that is also Comparable and Serializable + private static class AnExecutable implements Executable, Comparable, Serializable { + + private final int n; + private Serializable[] spaces; + private transient boolean afterDeserializeCalled; + + public AnExecutable(int n, String... spaces) { + this.n = n; + this.spaces = spaces; + } + + @Override + public int compareTo(Object o) { + return new Integer(n).compareTo( new Integer(( (AnExecutable) o ).n )); + } + + @Override + public int hashCode() { + return n; + } + + @Override + public boolean equals(Object obj) { + if ( this == obj ) + return true; + if ( obj == null ) + return false; + AnExecutable other = (AnExecutable) obj; + return n == other.n; + } + + @Override + public Serializable[] getPropertySpaces() { + return spaces; + } + + @Override + public void beforeExecutions() throws HibernateException { + } + + @Override + public void execute() throws HibernateException { + } + + @Override + public AfterTransactionCompletionProcess getAfterTransactionCompletionProcess() { + return null; + } + + @Override + public BeforeTransactionCompletionProcess getBeforeTransactionCompletionProcess() { + return null; + } + + @Override + public void afterDeserialize(SharedSessionContractImplementor session) { + this.afterDeserializeCalled = true; + } + + public String toString() { + return String.valueOf(n); + } + + } + + private ExecutableList l; + + private AnExecutable action1 = new AnExecutable( 0, "a" ); + private AnExecutable action2 = new AnExecutable( 1, "b", "c" ); + private AnExecutable action3 = new AnExecutable( 2, "b", "d" ); + private AnExecutable action4 = new AnExecutable( 3 ); + + @Before + public void setUp() { + // false indicates sorting is not required. + l = new ExecutableList( false ); + } + + @After + public void tearDown() { + l = null; + } + + @Test + public void testAdd() { + Assert.assertEquals( 0, l.size() ); + l.add( action1 ); + Assert.assertEquals( action1, l.get( 0 ) ); + Assert.assertEquals( 1, l.size() ); + l.add( action2 ); + Assert.assertEquals( action2, l.get( 1 ) ); + l.add( action3 ); + Assert.assertEquals( action3, l.get( 2 ) ); + Assert.assertEquals( 3, l.size() ); + } + + @Test + public void testClear() { + Assert.assertTrue( l.isEmpty() ); + l.add( action1 ); + Assert.assertFalse( l.isEmpty() ); + l.add( action2 ); + l.clear(); + Assert.assertTrue( l.isEmpty() ); + Assert.assertEquals( 0, l.size() ); + } + + @Test + public void testIterator() { + l.add( action1 ); + l.add( action2 ); + l.add( action3 ); + Iterator iterator = l.iterator(); + Assert.assertEquals(action1, iterator.next()); + Assert.assertEquals(action2, iterator.next()); + Assert.assertEquals(action3, iterator.next()); + Assert.assertFalse(iterator.hasNext()); + } + + @Test + public void testRemoveLastN() { + l.add( action1 ); + l.add( action2 ); + l.add( action3 ); + l.removeLastN( 0 ); + Assert.assertEquals( 3, l.size() ); + l.removeLastN( 2 ); + Assert.assertEquals( 1, l.size() ); + Assert.assertEquals( action1, l.get( 0 ) ); + } + + @Test + public void testGetSpaces() { + l.add( action1 ); + Set ss = l.getQuerySpaces(); + Assert.assertEquals( 1, ss.size() ); + Assert.assertTrue( ss.contains( "a" ) ); + l.add( action2 ); + l.add( action3 ); + l.add( action4 ); + Set ss2 = l.getQuerySpaces(); + Assert.assertEquals( 4, ss2.size() ); + Assert.assertTrue( ss2.contains( "a" ) ); + Assert.assertTrue( ss2.contains( "b" ) ); + Assert.assertTrue( ss2.contains( "c" ) ); + Assert.assertTrue( ss2.contains( "d" ) ); + Assert.assertTrue( ss == ss2 ); // same Set (cached) + // now remove action4 + l.remove( 3 ); + ss2 = l.getQuerySpaces(); + Assert.assertTrue( ss == ss2 ); // same Set (action4 has no spaces) + Assert.assertEquals( 4, ss2.size() ); + l.remove( 2 ); + ss2 = l.getQuerySpaces(); + Assert.assertTrue( ss != ss2 ); // Different Set because it has been rebuilt. This would be incorrect if + // Set.clear() was used + } + + @Test + public void testSort() { + l.add( action4 ); + l.add( action3 ); + l.add( action2 ); + l.add( action1 ); + // sort should have no affect + l.sort(); + Assert.assertEquals( action4, l.get( 0 ) ); + Assert.assertEquals( action3, l.get( 1 ) ); + Assert.assertEquals( action2, l.get( 2 ) ); + Assert.assertEquals( action1, l.get( 3 ) ); + } + + @Test + public void testSerializeDeserialize() throws IOException, ClassNotFoundException { + l.add( action4 ); + l.add( action3 ); + l.add( action2 ); + l.add( action1 ); + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + ObjectOutputStream oos = new ObjectOutputStream( baos ); + l.writeExternal( oos ); + // this OOS stream needs to be flushed... + oos.flush(); + ByteArrayInputStream bin = new ByteArrayInputStream( baos.toByteArray() ); + ObjectInputStream ois = new ObjectInputStream( bin ); + l = new ExecutableList( false ); + l.readExternal( ois ); + + Assert.assertEquals( 4, l.size() ); + Assert.assertEquals( action4, l.get( 0 ) ); + Assert.assertEquals( action3, l.get( 1 ) ); + Assert.assertEquals( action2, l.get( 2 ) ); + Assert.assertEquals( action1, l.get( 3 ) ); + + Assert.assertFalse( l.get( 0 ).afterDeserializeCalled ); + Assert.assertFalse( l.get( 1 ).afterDeserializeCalled ); + Assert.assertFalse( l.get( 2 ).afterDeserializeCalled ); + Assert.assertFalse( l.get( 3 ).afterDeserializeCalled ); + + l.afterDeserialize( null ); + + Assert.assertTrue( l.get( 0 ).afterDeserializeCalled ); + Assert.assertTrue( l.get( 1 ).afterDeserializeCalled ); + Assert.assertTrue( l.get( 2 ).afterDeserializeCalled ); + Assert.assertTrue( l.get( 3 ).afterDeserializeCalled ); + + Assert.assertEquals( action4, l.get( 0 ) ); + Assert.assertEquals( action3, l.get( 1 ) ); + Assert.assertEquals( action2, l.get( 2 ) ); + Assert.assertEquals( action1, l.get( 3 ) ); + + // sort after deserializing; it should still have no affect + l.sort(); + Assert.assertEquals( action4, l.get( 0 ) ); + Assert.assertEquals( action3, l.get( 1 ) ); + Assert.assertEquals( action2, l.get( 2 ) ); + Assert.assertEquals( action1, l.get( 3 ) ); + } +} + diff --git a/hibernate-core/src/test/java/org/hibernate/engine/spi/ExecutableListTest.java b/hibernate-core/src/test/java/org/hibernate/engine/spi/SortedExecutableListTest.java similarity index 88% rename from hibernate-core/src/test/java/org/hibernate/engine/spi/ExecutableListTest.java rename to hibernate-core/src/test/java/org/hibernate/engine/spi/SortedExecutableListTest.java index 95ea9ef9e1..f6d6d41208 100644 --- a/hibernate-core/src/test/java/org/hibernate/engine/spi/ExecutableListTest.java +++ b/hibernate-core/src/test/java/org/hibernate/engine/spi/SortedExecutableListTest.java @@ -28,7 +28,7 @@ import org.junit.Test; /** * @author Anton Marsden */ -public class ExecutableListTest extends BaseUnitTestCase { +public class SortedExecutableListTest extends BaseUnitTestCase { // For testing, we need an Executable that is also Comparable and Serializable private static class AnExecutable implements Executable, Comparable, Serializable { @@ -214,7 +214,7 @@ public class ExecutableListTest extends BaseUnitTestCase { oos.flush(); ByteArrayInputStream bin = new ByteArrayInputStream( baos.toByteArray() ); ObjectInputStream ois = new ObjectInputStream( bin ); - l = new ExecutableList(); + l = new ExecutableList(); l.readExternal( ois ); Assert.assertEquals( 4, l.size() ); @@ -226,15 +226,26 @@ public class ExecutableListTest extends BaseUnitTestCase { Assert.assertFalse(l.get(0).afterDeserializeCalled); Assert.assertFalse(l.get(1).afterDeserializeCalled); Assert.assertFalse(l.get(2).afterDeserializeCalled); - Assert.assertFalse(l.get(3).afterDeserializeCalled); + Assert.assertFalse( l.get( 3 ).afterDeserializeCalled ); l.afterDeserialize( null ); - Assert.assertTrue(l.get(0).afterDeserializeCalled); - Assert.assertTrue(l.get(1).afterDeserializeCalled); - Assert.assertTrue(l.get(2).afterDeserializeCalled); - Assert.assertTrue(l.get(3).afterDeserializeCalled); + Assert.assertTrue( l.get( 0 ).afterDeserializeCalled ); + Assert.assertTrue( l.get( 1 ).afterDeserializeCalled ); + Assert.assertTrue( l.get( 2 ).afterDeserializeCalled ); + Assert.assertTrue( l.get( 3 ).afterDeserializeCalled ); + + Assert.assertEquals( action4, l.get( 0 ) ); + Assert.assertEquals( action3, l.get( 1 ) ); + Assert.assertEquals( action2, l.get( 2 ) ); + Assert.assertEquals( action1, l.get( 3 ) ); + + // sort after deserializing + l.sort(); + Assert.assertEquals( action1, l.get( 0 ) ); + Assert.assertEquals( action2, l.get( 1 ) ); + Assert.assertEquals( action3, l.get( 2 ) ); + Assert.assertEquals( action4, l.get( 3 ) ); } - } diff --git a/hibernate-core/src/test/java/org/hibernate/test/id/array/ByteArrayIdTest.java b/hibernate-core/src/test/java/org/hibernate/test/id/array/ByteArrayIdTest.java index d19277a3f1..742d1cf0fb 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/id/array/ByteArrayIdTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/id/array/ByteArrayIdTest.java @@ -19,7 +19,6 @@ import org.junit.Test; import org.hibernate.Query; import org.hibernate.Session; -import org.hibernate.testing.FailureExpected; import org.hibernate.testing.TestForIssue; import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; @@ -70,7 +69,6 @@ public class ByteArrayIdTest extends BaseCoreFunctionalTestCase { */ @Test @TestForIssue(jiraKey = "HHH-8999") - @FailureExpected(jiraKey = "HHH-8999") public void testMultipleDeletions() { Session s = openSession(); s.getTransaction().begin(); @@ -94,7 +92,6 @@ public class ByteArrayIdTest extends BaseCoreFunctionalTestCase { */ @Test @TestForIssue(jiraKey = "HHH-8999") - @FailureExpected(jiraKey = "HHH-8999") public void testMultipleUpdates() { Session s = openSession(); s.getTransaction().begin(); diff --git a/hibernate-core/src/test/java/org/hibernate/test/id/array/CharacterArrayIdTest.java b/hibernate-core/src/test/java/org/hibernate/test/id/array/CharacterArrayIdTest.java index f68206de7e..c1911ad19c 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/id/array/CharacterArrayIdTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/id/array/CharacterArrayIdTest.java @@ -19,7 +19,6 @@ import org.junit.Test; import org.hibernate.Query; import org.hibernate.Session; -import org.hibernate.testing.FailureExpected; import org.hibernate.testing.TestForIssue; import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; @@ -70,7 +69,6 @@ public class CharacterArrayIdTest extends BaseCoreFunctionalTestCase { */ @Test @TestForIssue(jiraKey = "HHH-8999") - @FailureExpected(jiraKey = "HHH-8999") public void testMultipleDeletions() { Session s = openSession(); s.getTransaction().begin(); @@ -94,7 +92,6 @@ public class CharacterArrayIdTest extends BaseCoreFunctionalTestCase { */ @Test @TestForIssue(jiraKey = "HHH-8999") - @FailureExpected(jiraKey = "HHH-8999") public void testMultipleUpdates() { Session s = openSession(); s.getTransaction().begin(); diff --git a/hibernate-core/src/test/java/org/hibernate/test/id/array/PrimitiveByteArrayIdTest.java b/hibernate-core/src/test/java/org/hibernate/test/id/array/PrimitiveByteArrayIdTest.java index 09b573fa52..a2998b55d4 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/id/array/PrimitiveByteArrayIdTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/id/array/PrimitiveByteArrayIdTest.java @@ -19,7 +19,6 @@ import org.junit.Test; import org.hibernate.Query; import org.hibernate.Session; -import org.hibernate.testing.FailureExpected; import org.hibernate.testing.TestForIssue; import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; @@ -70,7 +69,6 @@ public class PrimitiveByteArrayIdTest extends BaseCoreFunctionalTestCase { */ @Test @TestForIssue(jiraKey = "HHH-8999") - @FailureExpected(jiraKey = "HHH-8999") public void testMultipleDeletions() { Session s = openSession(); s.getTransaction().begin(); @@ -94,7 +92,6 @@ public class PrimitiveByteArrayIdTest extends BaseCoreFunctionalTestCase { */ @Test @TestForIssue(jiraKey = "HHH-8999") - @FailureExpected(jiraKey = "HHH-8999") public void testMultipleUpdates() { Session s = openSession(); s.getTransaction().begin(); diff --git a/hibernate-core/src/test/java/org/hibernate/test/id/array/PrimitiveCharacterArrayIdTest.java b/hibernate-core/src/test/java/org/hibernate/test/id/array/PrimitiveCharacterArrayIdTest.java index 916060ecc9..45c4671f00 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/id/array/PrimitiveCharacterArrayIdTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/id/array/PrimitiveCharacterArrayIdTest.java @@ -19,7 +19,6 @@ import org.junit.Test; import org.hibernate.Query; import org.hibernate.Session; -import org.hibernate.testing.FailureExpected; import org.hibernate.testing.TestForIssue; import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; @@ -70,7 +69,6 @@ public class PrimitiveCharacterArrayIdTest extends BaseCoreFunctionalTestCase { */ @Test @TestForIssue(jiraKey = "HHH-8999") - @FailureExpected(jiraKey = "HHH-8999") public void testMultipleDeletions() { Session s = openSession(); s.getTransaction().begin(); @@ -94,7 +92,6 @@ public class PrimitiveCharacterArrayIdTest extends BaseCoreFunctionalTestCase { */ @Test @TestForIssue(jiraKey = "HHH-8999") - @FailureExpected(jiraKey = "HHH-8999") public void testMultipleUpdates() { Session s = openSession(); s.getTransaction().begin(); diff --git a/hibernate-core/src/test/java/org/hibernate/test/id/usertype/UserTypeNonComparableIdTest.java b/hibernate-core/src/test/java/org/hibernate/test/id/usertype/UserTypeNonComparableIdTest.java index d2ecbfca76..48ea325ff3 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/id/usertype/UserTypeNonComparableIdTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/id/usertype/UserTypeNonComparableIdTest.java @@ -24,7 +24,6 @@ import org.hibernate.Session; import org.hibernate.annotations.Type; import org.hibernate.annotations.TypeDef; import org.hibernate.engine.spi.SharedSessionContractImplementor; -import org.hibernate.testing.FailureExpected; import org.hibernate.testing.TestForIssue; import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; import org.hibernate.type.LongType; @@ -34,7 +33,6 @@ public class UserTypeNonComparableIdTest extends BaseCoreFunctionalTestCase { @Test @TestForIssue(jiraKey = "HHH-8999") - @FailureExpected(jiraKey = "HHH-8999") public void testUserTypeId() { Session s = openSession(); s.beginTransaction();