From b838344eeb930bb64bb3d232d609fe9815ba92e9 Mon Sep 17 00:00:00 2001 From: Lukasz Antoniak Date: Mon, 8 Apr 2013 15:53:58 +0200 Subject: [PATCH] HHH-7605 - Event cache descriptive error messages --- .../internal/DefaultMergeEventListener.java | 2 +- .../hibernate/event/internal/EventCache.java | 68 ++++++++++-- .../event/internal/EventCacheTest.java | 104 ++++++++++++------ 3 files changed, 129 insertions(+), 45 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultMergeEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultMergeEventListener.java index e9ee3a13fe..c0ee70f16c 100755 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultMergeEventListener.java +++ b/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; diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/EventCache.java b/hibernate-core/src/main/java/org/hibernate/event/internal/EventCache.java index 9bdf9f47f5..7c4bc9a26a 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/EventCache.java +++ b/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 entityToCopyMap = new IdentityHashMap(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 + "]"; + } } \ No newline at end of file diff --git a/hibernate-core/src/test/java/org/hibernate/event/internal/EventCacheTest.java b/hibernate-core/src/test/java/org/hibernate/event/internal/EventCacheTest.java index 780b6fd072..ff3576e59e 100644 --- a/hibernate-core/src/test/java/org/hibernate/event/internal/EventCacheTest.java +++ b/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. - * @author Wim Ockerman @ CISCO - * - * + * 2011/10/20 Unit test for code added in EventCache for performance improvement. * + * @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 input = new HashMap(); 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() { - - EventCache cache = new EventCache(); + session.getTransaction().begin(); 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() { - - EventCache cache = new EventCache(); + session.getTransaction().begin(); 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) {