HHH-7605 - Event cache descriptive error messages

hibernate-core/src/main/java/org/hibernate/event/internal/DefaultMergeEventListener.java

@@ -72,7 +72,7 @@ public class DefaultMergeEventListener extends AbstractSaveEventListener impleme
 	 * @throws HibernateException
 	 */
 	public void onMerge(MergeEvent event) throws HibernateException {
-		EventCache copyCache = new EventCache();
+		EventCache copyCache = new EventCache( event.getSession() );
 		onMerge( event, copyCache );
 		copyCache.clear();
 		copyCache = null;

hibernate-core/src/main/java/org/hibernate/event/internal/EventCache.java

@@ -30,6 +30,8 @@ import java.util.Map;
 import java.util.Set;
 
 import org.hibernate.AssertionFailure;
+import org.hibernate.event.spi.EventSource;
+import org.hibernate.pretty.MessageHelper;
 
 /**
  * EventCache is a Map implementation that can be used by an event
@@ -59,6 +61,8 @@ import org.hibernate.AssertionFailure;
  * @author Gail Badner
  */
 class EventCache implements Map {
+	private final EventSource session;
+
 	private Map<Object,Object> entityToCopyMap = new IdentityHashMap<Object,Object>(10);
 		// key is an entity involved with the operation performed by the listener;
 		// value can be either a copy of the entity or the entity itself
@@ -70,6 +74,10 @@ class EventCache implements Map {
 	    // key is an entity involved with the operation performed by the listener;
 	    // value is a flag indicating if the listener explicitly operates on the entity
 
+	EventCache(EventSource session) {
+		this.session = session;
+	}
+
 	/**
 	 * Clears the EventCache.
 	 */
@@ -181,10 +189,16 @@ class EventCache implements Map {
 
 		if ( oldCopy == null ) {
 			if  ( oldEntity != null ) {
-				throw new IllegalStateException( "An entity copy was already assigned to a different entity." );
+				throw new IllegalStateException(
+						"Error occurred while storing entity " + printEntity( entity ) + ". An entity copy " + printEntity( copy )
+								+ " was already assigned to a different entity " + printEntity( oldEntity ) + "."
+				);
 			}
 			if ( oldOperatedOn != null ) {
-				throw new IllegalStateException( "entityToOperatedOnFlagMap contains an entity, but entityToCopyMap does not." );
+				throw new IllegalStateException(
+						"EventCache#entityToOperatedOnFlagMap contains an entity " + printEntity( entity )
+								+ ", but EventCache#entityToCopyMap does not."
+				);
 			}
 		}
 		else {
@@ -193,21 +207,33 @@ class EventCache implements Map {
 				// to synch things up.
 				Object removedEntity = copyToEntityMap.remove( oldCopy );
 				if ( removedEntity != entity ) {
-					throw new IllegalStateException( "An unexpected entity was associated with the old entity copy." );
+					throw new IllegalStateException(
+							"Error occurred while storing entity " + printEntity( entity ) + ". An unexpected entity " + printEntity( removedEntity )
+									+ " was associated with the old entity copy " + printEntity( oldCopy ) + "."
+					);
 				}
 				if ( oldEntity != null ) {
-					throw new IllegalStateException( "A new entity copy is already associated with a different entity." );
+					throw new IllegalStateException(
+							"Error occurred while storing entity " + printEntity( entity ) + ". A new entity copy " + printEntity( copy )
+									+ " is already associated with a different entity " + printEntity( oldEntity ) + "."
+					);
 				}
 			}
 			else {
 				// Replaced an entity copy with the same copy in entityToCopyMap.
 				// Make sure that copy is associated with the same entity in copyToEntityMap.
 				if ( oldEntity != entity ) {
-					throw new IllegalStateException( "An entity copy was associated with a different entity than provided." );
+					throw new IllegalStateException(
+							"An entity copy " + printEntity( copy ) + " was associated with a different entity "
+									+ printEntity( oldEntity ) + " than provided " + printEntity( entity ) + "."
+					);
 				}
 			}
 			if ( oldOperatedOn == null ) {
-				throw new IllegalStateException( "entityToCopyMap contained an entity, but entityToOperatedOnFlagMap did not." );
+				throw new IllegalStateException(
+						"EventCache#entityToCopyMap contained an entity " + printEntity( entity )
+								+ ", but EventCache#entityToOperatedOnFlagMap did not."
+				);
 			}
 		}
 
@@ -242,18 +268,30 @@ class EventCache implements Map {
 
 		if ( oldCopy == null ) {
 			if ( oldOperatedOn != null ) {
-				throw new IllegalStateException( "Removed entity from entityToOperatedOnFlagMap, but entityToCopyMap did not contain the entity." );
+				throw new IllegalStateException(
+						"Removed entity " + printEntity( entity )
+								+ " from EventCache#entityToOperatedOnFlagMap, but EventCache#entityToCopyMap did not contain the entity."
+				);
 			}
 		}
 		else {
 			if ( oldEntity == null ) {
-				throw new IllegalStateException( "Removed entity from entityToCopyMap, but copyToEntityMap did not contain the entity." );
+				throw new IllegalStateException(
+						"Removed entity " + printEntity( entity )
+								+ " from EventCache#entityToCopyMap, but EventCache#copyToEntityMap did not contain the entity."
+				);
 			}
 			if ( oldOperatedOn == null ) {
-				throw new IllegalStateException( "entityToCopyMap contained an entity, but entityToOperatedOnFlagMap did not." );
+				throw new IllegalStateException(
+						"EventCache#entityToCopyMap contained an entity " + printEntity( entity )
+								+ ", but EventCache#entityToOperatedOnFlagMap did not."
+				);
 			}
 			if ( oldEntity != entity ) {
-				throw new IllegalStateException( "An entity copy was associated with a different entity than provided." );
+				throw new IllegalStateException(
+						"An entity copy " + printEntity( oldCopy ) + " was associated with a different entity "
+								+ printEntity( oldEntity ) + " than provided " + printEntity( entity ) + "."
+				);
 			}
 		}
 
@@ -303,7 +341,7 @@ class EventCache implements Map {
 		}
 		if ( ! entityToOperatedOnFlagMap.containsKey( entity ) ||
 			! entityToCopyMap.containsKey( entity ) ) {
-			throw new AssertionFailure( "called EventCache.setOperatedOn() for entity not found in EventCache" );
+			throw new AssertionFailure( "called EventCache#setOperatedOn() for entity not found in EventCache" );
 		}
 		entityToOperatedOnFlagMap.put( entity, isOperatedOn );
 	}
@@ -317,4 +355,12 @@ class EventCache implements Map {
 	public Map invertMap() {
 		return Collections.unmodifiableMap( copyToEntityMap );
 	}
+
+	private String printEntity(Object entity) {
+		if ( session.getPersistenceContext().getEntry( entity ) != null ) {
+			return MessageHelper.infoString( session.getEntityName( entity ), session.getIdentifier( entity ) );
+		}
+		// Entity was not found in current persistence context. Use Object#toString() method.
+		return "[" + entity + "]";
+	}
 }

hibernate-core/src/test/java/org/hibernate/event/internal/EventCacheTest.java

@@ -29,20 +29,53 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
+import javax.persistence.Entity;
+import javax.persistence.Id;
 
-import junit.framework.TestCase;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import org.hibernate.Session;
+import org.hibernate.event.spi.EventSource;
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
- * 2011/10/20 Unit test for code added in EventCache for performance improvement. 
+ * 2011/10/20 Unit test for code added in EventCache for performance improvement.
- * @author Wim Ockerman @ CISCO
- * 
- * 
  *
+ * @author Wim Ockerman @ CISCO
  */
-public class EventCacheTest extends TestCase {
+public class EventCacheTest extends BaseCoreFunctionalTestCase {
-    
+	private Session session = null;
+	private EventCache cache = null;
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class<?>[] { Simple.class };
+	}
+
+	@Before
+	public void setUp() {
+		session = openSession();
+		cache = new EventCache( ( EventSource) session );
+	}
+
+	@After
+	public void tearDown() {
+		cache = null;
+		session.close();
+		session = null;
+	}
+
+	@Test
     public void testEntityToCopyFillFollowedByCopyToEntityMapping() {
-        EventCache cache = new EventCache(); 
         Object entity = new Simple( 1 );
         Object copy = new Simple( 2 );
         
@@ -66,8 +99,8 @@ public class EventCacheTest extends TestCase {
         assertFalse(cache.invertMap().containsKey(copy));
 	}
 
+	@Test
     public void testEntityToCopyFillFollowedByCopyToEntityMappingOnRemove() {
-        EventCache cache = new EventCache(); 
         Object entity = new Simple( 1 );
         Object copy = new Simple( 2 );
         
@@ -88,9 +121,9 @@ public class EventCacheTest extends TestCase {
         assertFalse(cache.containsKey(entity)); 
         assertFalse(cache.invertMap().containsKey(copy));        
     }
-    
+
+	@Test
     public void testEntityToCopyFillFollowedByCopyToEntityUsingPutAll() {
-        EventCache cache = new EventCache(); 
         Map<Object,Object> input = new HashMap<Object,Object>();
         Object entity1 = new Simple( 1 );
 		//
@@ -114,9 +147,9 @@ public class EventCacheTest extends TestCase {
         assertTrue(cache.invertMap().containsKey(copy2));
         assertFalse(cache.invertMap().containsKey(entity2));
     }
-    
+
+	@Test
     public void testEntityToCopyFillFollowedByCopyToEntityMappingUsingPutWithSetOperatedOnArg() {
-        EventCache cache = new EventCache(); 
         Object entity = new Simple( 1 );
         Object copy = new Simple( 2 );
         
@@ -142,8 +175,8 @@ public class EventCacheTest extends TestCase {
         assertFalse(cache.containsKey(copy)); 
     }
 
+	@Test
 	public void testEntityToCopyFillFollowedByIterateEntrySet() {
-		EventCache cache = new EventCache();
 		Object entity = new Simple( 1 );
 		Object copy = new Simple( 2 );
 
@@ -160,8 +193,8 @@ public class EventCacheTest extends TestCase {
 
 	}
 
+	@Test
 	public void testEntityToCopyFillFollowedByModifyEntrySet() {
-		EventCache cache = new EventCache();
 		Object entity = new Simple( 1 );
 		Object copy = new Simple( 2 );
 
@@ -215,8 +248,8 @@ public class EventCacheTest extends TestCase {
 
 	}
 
+	@Test
 	public void testEntityToCopyFillFollowedByModifyKeys() {
-		EventCache cache = new EventCache();
 		Object entity = new Simple( 1 );
 		Object copy = new Simple( 2 );
 
@@ -249,8 +282,8 @@ public class EventCacheTest extends TestCase {
 		}
 	}
 
+	@Test
 	public void testEntityToCopyFillFollowedByModifyValues() {
-		EventCache cache = new EventCache();
 		Object entity = new Simple( 1 );
 		Object copy = new Simple( 2 );
 
@@ -283,9 +316,8 @@ public class EventCacheTest extends TestCase {
 		}
 	}
 
+	@Test
 	public void testEntityToCopyFillFollowedByModifyKeyOfEntrySetElement() {
-
-		EventCache cache = new EventCache();
 		Simple entity = new Simple( 1 );
 		Simple copy = new Simple( 0 );
 		cache.put(entity, copy, true);
@@ -301,9 +333,8 @@ public class EventCacheTest extends TestCase {
 		assertSame( copy, entry.getValue() );
 	}
 
+	@Test
 	public void testEntityToCopyFillFollowedByModifyValueOfEntrySetElement() {
-
-		EventCache cache = new EventCache();
 		Simple entity = new Simple( 1 );
 		Simple copy = new Simple( 0 );
 		cache.put(entity, copy, true);
@@ -319,9 +350,8 @@ public class EventCacheTest extends TestCase {
 		assertSame( copy, entry.getValue() );
 	}
 
+	@Test
 	public void testReplaceEntityCopy() {
-
-		EventCache cache = new EventCache();
 		Simple entity = new Simple( 1 );
 		Simple copy = new Simple( 0 );
 		cache.put(entity, copy);
@@ -340,12 +370,14 @@ public class EventCacheTest extends TestCase {
 		checkCacheConsistency( cache, 1 );
 	}
 
+	@Test
 	public void testCopyAssociatedWithNewAndExistingEntity() {
-
+		session.getTransaction().begin();
-		EventCache cache = new EventCache();
 		Simple entity = new Simple( 1 );
 		Simple copy = new Simple( 0 );
+		session.persist( entity );
 		cache.put(entity, copy);
+		session.flush();
 
 		try {
 			cache.put( new Simple( 1 ), copy );
@@ -353,19 +385,23 @@ public class EventCacheTest extends TestCase {
 		}
 		catch( IllegalStateException ex ) {
 			// expected
+			assertTrue( ex.getMessage().startsWith( "Error occurred while storing entity [org.hibernate.event.internal.EventCacheTest$Simple@" ) );
 		}
-
+		session.getTransaction().rollback();
 	}
 
+	@Test
 	public void testCopyAssociatedWith2ExistingEntities() {
-
+		session.getTransaction().begin();
-		EventCache cache = new EventCache();
 		Simple entity1 = new Simple( 1 );
-		Simple copy1 = new Simple( 0 );
+		session.persist( entity1 );
+		Simple copy1 = new Simple( 1 );
 		cache.put(entity1, copy1);
 		Simple entity2 = new Simple( 2 );
-		Simple copy2 = new Simple( 0 );
+		session.persist( entity2 );
+		Simple copy2 = new Simple( 2 );
 		cache.put( entity2, copy2 );
+		session.flush();
 
 		try {
 			cache.put( entity1, copy2 );
@@ -373,17 +409,17 @@ public class EventCacheTest extends TestCase {
 		}
 		catch( IllegalStateException ex ) {
 			// expected
+			assertTrue( ex.getMessage().startsWith( "Error occurred while storing entity [org.hibernate.event.internal.EventCacheTest$Simple#1]." ) );
 		}
+		session.getTransaction().rollback();
 	}
 
+	@Test
 	public void testRemoveNonExistingEntity() {
-
-		EventCache cache = new EventCache();
 		assertNull( cache.remove( new Simple( 1 ) ) );
 	}
 
 	private void checkCacheConsistency(EventCache cache, int expectedSize) {
-
 		Set entrySet = cache.entrySet();
 		Set cacheKeys = cache.keySet();
 		Collection cacheValues = cache.values();
@@ -404,7 +440,9 @@ public class EventCacheTest extends TestCase {
 		}
 	}
 
+	@Entity
 	private static class Simple {
+		@Id
 		private int value;
 
 		public Simple(int value) {