From 1a31549ced7bce7d889f185975c65a61482f09b9 Mon Sep 17 00:00:00 2001
From: Steve Ebersole <steve@hibernate.org>
Date: Sat, 21 Nov 2009 02:57:20 +0000
Subject: [PATCH] HHH-2584 - PersistentMap.remove() incorrect on uninitialized,
 non-extra-lazy map

git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@18020 1b8cb986-b30d-0410-93ca-fae66ebed9b2
---
 .../hibernate/collection/PersistentMap.java   | 18 ++++-----
 .../test/collection/map/Mappings.hbm.xml      |  2 +-
 .../collection/map/PersistentMapTest.java     | 39 ++++++++++++++++++-
 3 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/core/src/main/java/org/hibernate/collection/PersistentMap.java b/core/src/main/java/org/hibernate/collection/PersistentMap.java
index 879c3c1561..df28b2e9e2 100644
--- a/core/src/main/java/org/hibernate/collection/PersistentMap.java
+++ b/core/src/main/java/org/hibernate/collection/PersistentMap.java
@@ -199,17 +199,17 @@ public class PersistentMap extends AbstractPersistentCollection implements Map {
 	public Object remove(Object key) {
 		if ( isPutQueueEnabled() ) {
 			Object old = readElementByIndex( key );
-			queueOperation( new Remove( key, old ) );
-			return old;
-		}
-		else {
-			// TODO : safe to interpret "map.remove(key) == null" as non-dirty?
-			initialize( true );
-			if ( map.containsKey( key ) ) {
-				dirty();
+			if ( old != UNKNOWN ) {
+				queueOperation( new Remove( key, old ) );
+				return old;
 			}
-			return map.remove( key );
 		}
+		// TODO : safe to interpret "map.remove(key) == null" as non-dirty?
+		initialize( true );
+		if ( map.containsKey( key ) ) {
+			dirty();
+		}
+		return map.remove( key );
 	}
 
 	/**
diff --git a/testsuite/src/test/java/org/hibernate/test/collection/map/Mappings.hbm.xml b/testsuite/src/test/java/org/hibernate/test/collection/map/Mappings.hbm.xml
index 97c6472bc1..6481906179 100644
--- a/testsuite/src/test/java/org/hibernate/test/collection/map/Mappings.hbm.xml
+++ b/testsuite/src/test/java/org/hibernate/test/collection/map/Mappings.hbm.xml
@@ -11,7 +11,7 @@
 
         <map name="children" inverse="true" cascade="all">
             <key column="PARENT" />
-            <map-key type="string" />
+            <map-key type="string" formula="NAME"/>
             <one-to-many class="Child" />
         </map>
 	</class>
diff --git a/testsuite/src/test/java/org/hibernate/test/collection/map/PersistentMapTest.java b/testsuite/src/test/java/org/hibernate/test/collection/map/PersistentMapTest.java
index 03a4583290..d03d107526 100644
--- a/testsuite/src/test/java/org/hibernate/test/collection/map/PersistentMapTest.java
+++ b/testsuite/src/test/java/org/hibernate/test/collection/map/PersistentMapTest.java
@@ -10,7 +10,7 @@ import org.hibernate.junit.functional.FunctionalTestCase;
 import org.hibernate.junit.functional.FunctionalTestClassTestSuite;
 
 /**
- * todo: describe PersistentMapTest
+ * Test various situations using a {@link PersistentMap}.
  *
  * @author Steve Ebersole
  */
@@ -27,6 +27,7 @@ public class PersistentMapTest extends FunctionalTestCase {
 		return new FunctionalTestClassTestSuite( PersistentMapTest.class );
 	}
 
+	@SuppressWarnings({ "unchecked" })
 	public void testWriteMethodDirtying() {
 		Parent parent = new Parent( "p1" );
 		Child child = new Child( "c1" );
@@ -38,7 +39,7 @@ public class PersistentMapTest extends FunctionalTestCase {
 		session.beginTransaction();
 		session.save( parent );
 		session.flush();
-		// at this point, the set on parent has now been replaced with a PersistentSet...
+		// at this point, the map on parent has now been replaced with a PersistentMap...
 		PersistentMap children = ( PersistentMap ) parent.getChildren();
 
 		Object old = children.put( child.getName(), child );
@@ -99,4 +100,38 @@ public class PersistentMapTest extends FunctionalTestCase {
 		session.getTransaction().commit();
 		session.close();
 	}
+
+    public void testRemoveAgainstUninitializedMap() {
+        Parent parent = new Parent( "p1" );
+        Child child = new Child( "c1" );
+        parent.addChild( child );
+
+        Session session = openSession();
+        session.beginTransaction();
+        session.save( parent );
+        session.getTransaction().commit();
+        session.close();
+
+        // Now reload the parent and test removing the child
+        session = openSession();
+        session.beginTransaction();
+        parent = ( Parent ) session.get( Parent.class, parent.getName() );
+        Child child2 = ( Child ) parent.getChildren().remove( child.getName() );
+		child2.setParent( null );
+		assertNotNull( child2 );
+		assertTrue( parent.getChildren().isEmpty() );
+        session.getTransaction().commit();
+        session.close();
+
+		// Load the parent once again and make sure child is still gone
+		//		then cleanup
+        session = openSession();
+        session.beginTransaction();
+		parent = ( Parent ) session.get( Parent.class, parent.getName() );
+		assertTrue( parent.getChildren().isEmpty() );
+		session.delete( child2 );
+		session.delete( parent );
+        session.getTransaction().commit();
+        session.close();
+    }
 }