Author: steve.ebersole(a)jboss.com Date: 2009-11-20 21:57:20 -0500 (Fri, 20 Nov 2009) New Revision: 18020 Modified: core/trunk/core/src/main/java/org/hibernate/collection/PersistentMap.java core/trunk/testsuite/src/test/java/org/hibernate/test/collection/map/Mappings.hbm.xml core/trunk/testsuite/src/test/java/org/hibernate/test/collection/map/PersistentMapTest.java Log: HHH-2584 - PersistentMap.remove() incorrect on uninitialized, non-extra-lazy map Modified: core/trunk/core/src/main/java/org/hibernate/collection/PersistentMap.java =================================================================== --- core/trunk/core/src/main/java/org/hibernate/collection/PersistentMap.java 2009-11-21 02:55:36 UTC (rev 18019) +++ core/trunk/core/src/main/java/org/hibernate/collection/PersistentMap.java 2009-11-21 02:57:20 UTC (rev 18020) @@ -199,17 +199,17 @@ 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 ); } /** Modified: core/trunk/testsuite/src/test/java/org/hibernate/test/collection/map/Mappings.hbm.xml =================================================================== --- core/trunk/testsuite/src/test/java/org/hibernate/test/collection/map/Mappings.hbm.xml 2009-11-21 02:55:36 UTC (rev 18019) +++ core/trunk/testsuite/src/test/java/org/hibernate/test/collection/map/Mappings.hbm.xml 2009-11-21 02:57:20 UTC (rev 18020) @@ -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> Modified: core/trunk/testsuite/src/test/java/org/hibernate/test/collection/map/PersistentMapTest.java =================================================================== --- core/trunk/testsuite/src/test/java/org/hibernate/test/collection/map/PersistentMapTest.java 2009-11-21 02:55:36 UTC (rev 18019) +++ core/trunk/testsuite/src/test/java/org/hibernate/test/collection/map/PersistentMapTest.java 2009-11-21 02:57:20 UTC (rev 18020) @@ -10,7 +10,7 @@ import org.hibernate.junit.functional.FunctionalTestClassTestSuite; /** - * todo: describe PersistentMapTest + * Test various situations using a {@link PersistentMap}. * * @author Steve Ebersole */ @@ -27,6 +27,7 @@ return new FunctionalTestClassTestSuite( PersistentMapTest.class ); } + @SuppressWarnings({ "unchecked" }) public void testWriteMethodDirtying() { Parent parent = new Parent( "p1" ); Child child = new Child( "c1" ); @@ -38,7 +39,7 @@ 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 @@ 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(); + } }