HHH-7912 - Define edge-case behavior for Session.evict

(cherry picked from commit c8b20660ed)

hibernate-core/src/main/java/org/hibernate/Session.java

@@ -266,7 +266,10 @@ public interface Session extends SharedSessionContract {
 	 * not be synchronized with the database. This operation cascades to associated
 	 * instances if the association is mapped with <tt>cascade="evict"</tt>.
 	 *
-	 * @param object a persistent instance
+	 * @param object The entity to evict
+	 *
+	 * @throws NullPointerException if the passed object is {@code null}
+	 * @throws IllegalArgumentException if the passed object is not defined as an entity
 	 */
 	public void evict(Object object);
 

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

@@ -62,18 +62,22 @@ public class DefaultEvictEventListener implements EvictEventListener {
 	 * @throws HibernateException
 	 */
 	public void onEvict(EvictEvent event) throws HibernateException {
-		EventSource source = event.getSession();
 		final Object object = event.getObject();
+		if ( object == null ) {
+			throw new NullPointerException( "null passed to Session.evict()" );
+		}
+
+		final EventSource source = event.getSession();
 		final PersistenceContext persistenceContext = source.getPersistenceContext();
 
 		if ( object instanceof HibernateProxy ) {
-			LazyInitializer li = ( (HibernateProxy) object ).getHibernateLazyInitializer();
+			final LazyInitializer li = ( (HibernateProxy) object ).getHibernateLazyInitializer();
-			Serializable id = li.getIdentifier();
+			final Serializable id = li.getIdentifier();
-			EntityPersister persister = source.getFactory().getEntityPersister( li.getEntityName() );
 			if ( id == null ) {
-				throw new IllegalArgumentException("null identifier");
+				throw new IllegalArgumentException( "Could not determine identifier of proxy passed to evict()" );
 			}
 
+			final EntityPersister persister = source.getFactory().getEntityPersister( li.getEntityName() );
 			final EntityKey key = source.generateEntityKey( id, persister );
 			persistenceContext.removeProxy( key );
 
@@ -92,6 +96,23 @@ public class DefaultEvictEventListener implements EvictEventListener {
 				persistenceContext.removeEntity( e.getEntityKey() );
 				doEvict( object, e.getEntityKey(), e.getPersister(), source );
 			}
+			else {
+				// see if the passed object is even an entity, and if not throw an exception
+				// 		this is different than legacy Hibernate behavior, but what JPA 2.1 is calling for
+				//		with EntityManager.detach
+				EntityPersister persister = null;
+				final String entityName = persistenceContext.getSession().guessEntityName( object );
+				if ( entityName != null ) {
+					try {
+						persister = persistenceContext.getSession().getFactory().getEntityPersister( entityName );
+					}
+					catch (Exception ignore) {
+					}
+				}
+				if ( persister == null ) {
+					throw new IllegalArgumentException( "Non-entity object instance passed to evict : " + object );
+				}
+			}
 		}
 	}
 

hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java

@@ -1156,7 +1156,7 @@ public final class SessionImpl extends AbstractSessionImpl implements EventSourc
 
 	/**
 	 * remove any hard references to the entity that are held by the infrastructure
-	 * (references held by application or other persistant instances are okay)
+	 * (references held by application or other persistent instances are okay)
 	 */
 	public void evict(Object object) throws HibernateException {
 		fireEvict( new EvictEvent( object, this ) );

hibernate-core/src/test/java/org/hibernate/test/eviction/EvictionTest.java

@@ -0,0 +1,140 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2013, Red Hat Inc. or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Inc.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ */
+package org.hibernate.test.eviction;
+
+import org.hibernate.Session;
+
+import org.junit.Test;
+
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
+ * @author Steve Ebersole
+ */
+public class EvictionTest extends BaseCoreFunctionalTestCase {
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class[] { IsolatedEvictableEntity.class };
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-7912" )
+	public void testNormalUsage() {
+		Session session = openSession();
+		session.beginTransaction();
+		session.save( new IsolatedEvictableEntity( 1 ) );
+		session.getTransaction().commit();
+		session.close();
+
+		session = openSession();
+		session.beginTransaction();
+		IsolatedEvictableEntity entity = (IsolatedEvictableEntity) session.get( IsolatedEvictableEntity.class, 1 );
+		assertTrue( session.contains( entity ) );
+		session.evict( entity );
+		assertFalse( session.contains( entity ) );
+		session.getTransaction().commit();
+		session.close();
+
+		session = openSession();
+		session.beginTransaction();
+		session.delete( entity );
+		session.getTransaction().commit();
+		session.close();
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-7912" )
+	public void testEvictingNull() {
+		Session session = openSession();
+		session.beginTransaction();
+		try {
+			session.evict( null );
+			fail( "Expecting evict(null) to throw NPE" );
+		}
+		catch (NullPointerException expected) {
+		}
+		session.getTransaction().commit();
+		session.close();
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-7912" )
+	public void testEvictingTransientEntity() {
+		Session session = openSession();
+		session.beginTransaction();
+		session.evict( new IsolatedEvictableEntity( 1 ) );
+		session.getTransaction().commit();
+		session.close();
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-7912" )
+	public void testEvictingDetachedEntity() {
+		Session session = openSession();
+		session.beginTransaction();
+		session.save( new IsolatedEvictableEntity( 1 ) );
+		session.getTransaction().commit();
+		session.close();
+
+		session = openSession();
+		session.beginTransaction();
+		IsolatedEvictableEntity entity = (IsolatedEvictableEntity) session.get( IsolatedEvictableEntity.class, 1 );
+		assertTrue( session.contains( entity ) );
+		// detach the entity
+		session.evict( entity );
+		assertFalse( session.contains( entity ) );
+		// evict it again the entity
+		session.evict( entity );
+		assertFalse( session.contains( entity ) );
+		session.getTransaction().commit();
+		session.close();
+
+		session = openSession();
+		session.beginTransaction();
+		session.delete( entity );
+		session.getTransaction().commit();
+		session.close();
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-7912" )
+	public void testEvictingNonEntity() {
+		Session session = openSession();
+		session.beginTransaction();
+		try {
+			session.evict( new EvictionTest() );
+			fail( "Expecting evict(non-entity) to throw IAE" );
+		}
+		catch (IllegalArgumentException expected) {
+		}
+		session.getTransaction().commit();
+		session.close();
+	}
+
+}

hibernate-core/src/test/java/org/hibernate/test/eviction/IsolatedEvictableEntity.java

@@ -0,0 +1,44 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2013, Red Hat Inc. or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Inc.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ */
+package org.hibernate.test.eviction;
+
+import javax.persistence.Entity;
+import javax.persistence.Id;
+
+/**
+ * @author Steve Ebersole
+ */
+@Entity
+public class IsolatedEvictableEntity {
+	@Id
+	private Integer id;
+	private String name;
+
+	public IsolatedEvictableEntity() {
+	}
+
+	public IsolatedEvictableEntity(Integer id) {
+		this.id = id;
+	}
+}

hibernate-entitymanager/src/test/java/org/hibernate/ejb/test/cascade/FetchTest.java

@@ -163,74 +163,6 @@ public class FetchTest extends BaseEntityManagerFunctionalTestCase {
 		em.close();
 	}
 
-	@Test
-	public void testPerfCascadeAndFetchEntity() throws Exception {
-		EntityManager em = getOrCreateEntityManager();
-		//init data
-		em.getTransaction().begin();
-		int loop = 50;
-		for ( int i = 0; i < loop ; i++ ) {
-			Troop disney = new Troop();
-			disney.setName( "Disney" );
-			Soldier mickey = new Soldier();
-			mickey.setName( "Mickey" );
-			disney.addSoldier( mickey );
-			em.persist( disney );
-		}
-		em.getTransaction().commit();
-		em.close();
-
-		//Warm up loop
-		em = getOrCreateEntityManager();
-		em.getTransaction().begin();
-		for ( int i = 0; i < loop ; i++ ) {
-			//Soldier soldier = em.find( Soldier.class, new Integer(i) );
-			Troop troop = em.find( Troop.class, new Integer( i ) );
-			//( ( HibernateEntityManager ) em ).getSession().evict(soldier);
-		}
-		long l11 = System.currentTimeMillis();
-		Query query = em.createQuery( "SELECT count(t) FROM Soldier t" );
-		query.getSingleResult();
-		long l2 = System.currentTimeMillis();
-		System.out.println( "Query1 " + ( l2 - l11 ) );
-		em.getTransaction().commit();
-		em.close();
-
-		//do not evict
-		for ( int j = 0; j < 10 ; j++ ) {
-			em = getOrCreateEntityManager();
-			em.getTransaction().begin();
-			for ( int i = 0; i < loop ; i++ ) {
-				Troop troop = em.find( Troop.class, new Integer( i ) );
-				( (HibernateEntityManager) em ).getSession().evict( troop );
-			}
-			l11 = System.currentTimeMillis();
-			query = em.createQuery( "SELECT count(t) FROM Soldier t" );
-			query.getSingleResult();
-			l2 = System.currentTimeMillis();
-			System.out.println( "Query " + ( l2 - l11 ) );
-			em.getTransaction().commit();
-			em.close();
-
-			//evict
-			em = getOrCreateEntityManager();
-			em.getTransaction().begin();
-			for ( int i = 0; i < loop ; i++ ) {
-				//Soldier soldier = em.find( Soldier.class, new Integer(i) );
-				Troop troop = em.find( Troop.class, new Integer( i ) );
-
-				//( ( HibernateEntityManager ) em ).getSession().evict(troop);
-			}
-			l11 = System.currentTimeMillis();
-			query = em.createQuery( "SELECT count(t) FROM Soldier t" );
-			query.getSingleResult();
-			l2 = System.currentTimeMillis();
-			System.out.println( "Query " + ( l2 - l11 ) );
-			em.getTransaction().commit();
-		}
-		em.close();
-	}
-
 	@Override
 	public Class[] getAnnotatedClasses() {
 		return new Class[]{