Author: steve.ebersole(a)jboss.com Date: 2006-09-01 16:56:40 -0400 (Fri, 01 Sep 2006) New Revision: 10422 Added: branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/CollectionSuite.java branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/ branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/Child.java branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/Mappings.hbm.xml branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/Parent.java branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/PersistentMapTest.java branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/ branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/Child.java branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/Mappings.hbm.xml branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/Parent.java branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/PersistentSetTest.java Modified: branches/Branch_3_2/Hibernate3/src/org/hibernate/collection/PersistentMap.java branches/Branch_3_2/Hibernate3/src/org/hibernate/collection/PersistentSet.java branches/Branch_3_2/Hibernate3/test/org/hibernate/test/AllTests.java Log: HHH-1668 (and friends) : PersistentSet and PersistentMap unecessary dirtying Modified: branches/Branch_3_2/Hibernate3/src/org/hibernate/collection/PersistentMap.java =================================================================== --- branches/Branch_3_2/Hibernate3/src/org/hibernate/collection/PersistentMap.java 2006-09-01 20:52:35 UTC (rev 10421) +++ branches/Branch_3_2/Hibernate3/src/org/hibernate/collection/PersistentMap.java 2006-09-01 20:56:40 UTC (rev 10422) @@ -31,6 +31,39 @@ protected Map map; + /** + * Empty constructor. + * <p/> + * Note: this form is not ever ever ever used by Hibernate; it is, however, + * needed for SOAP libraries and other such marshalling code. + */ + public PersistentMap() { + // intentionally empty + } + + /** + * Instantiates a lazy map (the underlying map is un-initialized). + * + * @param session The session to which this map will belong. + */ + public PersistentMap(SessionImplementor session) { + super(session); + } + + /** + * Instantiates a non-lazy map (the underlying map is constructed + * from the incoming map reference). + * + * @param session The session to which this map will belong. + * @param map The underlying map data. + */ + public PersistentMap(SessionImplementor session, Map map) { + super(session); + this.map = map; + setInitialized(); + setDirectlyAccessible(true); + } + public Serializable getSnapshot(CollectionPersister persister) throws HibernateException { EntityMode entityMode = getSession().getEntityMode(); HashMap clonedMap = new HashMap( map.size() ); @@ -68,23 +101,11 @@ public boolean isWrapper(Object collection) { return map==collection; } - - public PersistentMap(SessionImplementor session) { - super(session); - } - public PersistentMap() {} //needed for SOAP libraries, etc - public void beforeInitialize(CollectionPersister persister, int anticipatedSize) { this.map = ( Map ) persister.getCollectionType().instantiate( anticipatedSize ); } - public PersistentMap(SessionImplementor session, Map map) { - super(session); - this.map = map; - setInitialized(); - setDirectlyAccessible(true); - } /** * @see java.util.Map#size() @@ -130,14 +151,21 @@ * @see java.util.Map#put(Object, Object) */ public Object put(Object key, Object value) { - Object old = isPutQueueEnabled() ? - readElementByIndex(key) : UNKNOWN; - if ( old==UNKNOWN ) { - write(); - return map.put(key, value); + if ( isPutQueueEnabled() ) { + Object old = readElementByIndex( key ); + queueOperation( new Put( key, value, old ) ); + return old; } else { - queueOperation( new Put(key, value, old) ); + initialize( true ); + Object old = map.put( key, value ); + // would be better to use the element-type to determine + // whether the old and the new are equal here; the problem being + // we do not necessarily have access to the element type in all + // cases + if ( value != old ) { + dirty(); + } return old; } } @@ -146,15 +174,18 @@ * @see java.util.Map#remove(Object) */ public Object remove(Object key) { - Object old = isPutQueueEnabled() ? - readElementByIndex(key) : UNKNOWN; - if ( old==UNKNOWN ) { - write(); - return map.remove(key); + if ( isPutQueueEnabled() ) { + Object old = readElementByIndex( key ); + queueOperation( new Remove( key, old ) ); + return old; } else { - queueOperation( new Remove(key, old) ); - return old; + // TODO : safe to interpret "map.remove(key) == null" as non-dirty? + initialize( true ); + if ( map.containsKey( key ) ) { + dirty(); + } + return map.remove( key ); } } @@ -163,8 +194,12 @@ */ public void putAll(Map puts) { if ( puts.size()>0 ) { - write(); - map.putAll(puts); + initialize( true ); + Iterator itr = puts.entrySet().iterator(); + while ( itr.hasNext() ) { + Map.Entry entry = ( Entry ) itr.next(); + put( entry.getKey(), entry.getValue() ); + } } } @@ -176,8 +211,11 @@ queueOperation( new Clear() ); } else { - write(); - map.clear(); + initialize( true ); + if ( ! map.isEmpty() ) { + dirty(); + map.clear(); + } } } Modified: branches/Branch_3_2/Hibernate3/src/org/hibernate/collection/PersistentSet.java =================================================================== --- branches/Branch_3_2/Hibernate3/src/org/hibernate/collection/PersistentSet.java 2006-09-01 20:52:35 UTC (rev 10421) +++ branches/Branch_3_2/Hibernate3/src/org/hibernate/collection/PersistentSet.java 2006-09-01 20:56:40 UTC (rev 10422) @@ -31,6 +31,45 @@ protected Set set; protected transient List tempList; + /** + * Empty constructor. + * <p/> + * Note: this form is not ever ever ever used by Hibernate; it is, however, + * needed for SOAP libraries and other such marshalling code. + */ + public PersistentSet() { + // intentionally empty + } + + /** + * Constructor matching super. Instantiates a lazy set (the underlying + * set is un-initialized). + * + * @param session The session to which this set will belong. + */ + public PersistentSet(SessionImplementor session) { + super( session ); + } + + /** + * Instantiates a non-lazy set (the underlying set is constructed + * from the incoming set reference). + * + * @param session The session to which this set will belong. + * @param set The underlying set data. + */ + public PersistentSet(SessionImplementor session, java.util.Set set) { + super(session); + // Sets can be just a view of a part of another collection. + // do we need to copy it to be sure it won't be changing + // underneath us? + // ie. this.set.addAll(set); + this.set = set; + setInitialized(); + setDirectlyAccessible(true); + } + + public Serializable getSnapshot(CollectionPersister persister) throws HibernateException { EntityMode entityMode = getSession().getEntityMode(); @@ -71,27 +110,10 @@ public boolean isSnapshotEmpty(Serializable snapshot) { return ( (java.util.Map) snapshot ).isEmpty(); } - - public PersistentSet(SessionImplementor session) { - super(session); - } - public PersistentSet() {} //needed for SOAP libraries, etc - public void beforeInitialize(CollectionPersister persister, int anticipatedSize) { this.set = ( Set ) persister.getCollectionType().instantiate( anticipatedSize ); } - - public PersistentSet(SessionImplementor session, java.util.Set set) { - super(session); - // Sets can be just a view of a part of another collection. - // do we need to copy it to be sure it won't be changing - // underneath us? - // ie. this.set.addAll(set); - this.set = set; - setInitialized(); - setDirectlyAccessible(true); - } public void initializeFromCache(CollectionPersister persister, Serializable disassembled, Object owner) throws HibernateException { @@ -162,11 +184,16 @@ * @see java.util.Set#add(Object) */ public boolean add(Object value) { - Boolean exists = isOperationQueueEnabled() ? - readElementExistence(value) : null; - if (exists==null) { - write(); - return set.add(value); + Boolean exists = isOperationQueueEnabled() ? readElementExistence( value ) : null; + if ( exists == null ) { + initialize( true ); + if ( set.add( value ) ) { + dirty(); + return true; + } + else { + return false; + } } else if ( exists.booleanValue() ) { return false; @@ -181,11 +208,16 @@ * @see java.util.Set#remove(Object) */ public boolean remove(Object value) { - Boolean exists = isPutQueueEnabled() ? - readElementExistence(value) : null; + Boolean exists = isPutQueueEnabled() ? readElementExistence( value ) : null; if ( exists==null ) { - write(); - return set.remove(value); + initialize( true ); + if ( set.remove( value ) ) { + dirty(); + return true; + } + else { + return false; + } } else if ( exists.booleanValue() ) { queueOperation( new SimpleRemove(value) ); @@ -208,9 +240,15 @@ * @see java.util.Set#addAll(Collection) */ public boolean addAll(Collection coll) { - if ( coll.size()> 0 ) { - write(); - return set.addAll(coll); + if ( coll.size() > 0 ) { + initialize( true ); + if ( set.addAll( coll ) ) { + dirty(); + return true; + } + else { + return false; + } } else { return false; @@ -221,17 +259,29 @@ * @see java.util.Set#retainAll(Collection) */ public boolean retainAll(Collection coll) { - write(); - return set.retainAll(coll); + initialize( true ); + if ( set.addAll( coll ) ) { + dirty(); + return true; + } + else { + return false; + } } /** * @see java.util.Set#removeAll(Collection) */ public boolean removeAll(Collection coll) { - if ( coll.size()>0 ) { - write(); - return set.removeAll(coll); + if ( coll.size() > 0 ) { + initialize( true ); + if ( set.removeAll( coll ) ) { + dirty(); + return true; + } + else { + return false; + } } else { return false; @@ -246,8 +296,11 @@ queueOperation( new Clear() ); } else { - write(); - set.clear(); + initialize( true ); + if ( !set.isEmpty() ) { + set.clear(); + dirty(); + } } } Modified: branches/Branch_3_2/Hibernate3/test/org/hibernate/test/AllTests.java =================================================================== --- branches/Branch_3_2/Hibernate3/test/org/hibernate/test/AllTests.java 2006-09-01 20:52:35 UTC (rev 10421) +++ branches/Branch_3_2/Hibernate3/test/org/hibernate/test/AllTests.java 2006-09-01 20:56:40 UTC (rev 10422) @@ -17,7 +17,7 @@ import org.hibernate.test.cache.CacheSuite; import org.hibernate.test.cascade.RefreshTest; import org.hibernate.test.cid.CompositeIdTest; -import org.hibernate.test.collection.CollectionTest; +import org.hibernate.test.collection.CollectionSuite; import org.hibernate.test.component.ComponentTest; import org.hibernate.test.compositeelement.CompositeElementTest; import org.hibernate.test.comppropertyref.ComponentPropertyRefTest; @@ -187,7 +187,7 @@ suite.addTest( IdClassTest.suite() ); suite.addTest( ArrayTest.suite() ); suite.addTest( TernaryTest.suite() ); - suite.addTest( CollectionTest.suite() ); + suite.addTest( CollectionSuite.suite() ); suite.addTest( IdBagTest.suite() ); suite.addTest( MapCompositeElementTest.suite() ); suite.addTest( MapIndexFormulaTest.suite() ); Added: branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/CollectionSuite.java =================================================================== --- branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/CollectionSuite.java 2006-09-01 20:52:35 UTC (rev 10421) +++ branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/CollectionSuite.java 2006-09-01 20:56:40 UTC (rev 10422) @@ -0,0 +1,23 @@ +package org.hibernate.test.collection; + +import junit.framework.Test; +import junit.framework.TestSuite; +import org.hibernate.test.collection.map.PersistentMapTest; +import org.hibernate.test.collection.set.PersistentSetTest; + +/** + * todo: describe CollectionSuite + * + * @author Steve Ebersole + */ +public class CollectionSuite { + + public static Test suite() { + TestSuite suite = new TestSuite( "Collection-related tests" ); + suite.addTest( CollectionTest.suite() ); + suite.addTest( PersistentMapTest.suite() ); + suite.addTest( PersistentSetTest.suite() ); + return suite; + } + +} Added: branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/Child.java =================================================================== --- branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/Child.java 2006-09-01 20:52:35 UTC (rev 10421) +++ branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/Child.java 2006-09-01 20:56:40 UTC (rev 10422) @@ -0,0 +1,34 @@ +package org.hibernate.test.collection.map; + +/** + * todo: describe Child + * + * @author Steve Ebersole + */ +public class Child { + private String name; + private Parent parent; + + public Child() { + } + + public Child(String name) { + this.name = name; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public Parent getParent() { + return parent; + } + + public void setParent(Parent parent) { + this.parent = parent; + } +} Added: branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/Mappings.hbm.xml =================================================================== --- branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/Mappings.hbm.xml 2006-09-01 20:52:35 UTC (rev 10421) +++ branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/Mappings.hbm.xml 2006-09-01 20:56:40 UTC (rev 10422) @@ -0,0 +1,24 @@ +<?xml version="1.0"?> +<!DOCTYPE hibernate-mapping PUBLIC + "-//Hibernate/Hibernate Mapping DTD 3.0//EN" + "http://hibernate.sourceforge.net/hibernate-mapping-3.0.dtd"> + + +<hibernate-mapping package="org.hibernate.test.collection.map"> + + <class name="Parent"> + <id name="name" column="NAME" type="string" /> + + <map name="children" inverse="true" cascade="all"> + <key column="PARENT" /> + <map-key type="string" /> + <one-to-many class="Child" /> + </map> + </class> + + <class name="Child"> + <id name="name" column="NAME" type="string"/> + <many-to-one name="parent" class="Parent" cascade="none" /> + </class> + +</hibernate-mapping> Added: branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/Parent.java =================================================================== --- branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/Parent.java 2006-09-01 20:52:35 UTC (rev 10421) +++ branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/Parent.java 2006-09-01 20:56:40 UTC (rev 10422) @@ -0,0 +1,37 @@ +package org.hibernate.test.collection.map; + +import java.util.Map; +import java.util.HashMap; + +/** + * todo: describe Parent + * + * @author Steve Ebersole + */ +public class Parent { + private String name; + private Map children = new HashMap(); + + public Parent() { + } + + public Parent(String name) { + this.name = name; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public Map getChildren() { + return children; + } + + public void setChildren(Map children) { + this.children = children; + } +} Added: branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/PersistentMapTest.java =================================================================== --- branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/PersistentMapTest.java 2006-09-01 20:52:35 UTC (rev 10421) +++ branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/map/PersistentMapTest.java 2006-09-01 20:56:40 UTC (rev 10422) @@ -0,0 +1,75 @@ +package org.hibernate.test.collection.map; + +import org.hibernate.test.TestCase; +import org.hibernate.Session; +import org.hibernate.collection.PersistentMap; + +import java.util.HashMap; + +import junit.framework.Test; +import junit.framework.TestSuite; + +/** + * todo: describe PersistentMapTest + * + * @author Steve Ebersole + */ +public class PersistentMapTest extends TestCase { + public PersistentMapTest(String name) { + super( name ); + } + + protected String[] getMappings() { + return new String[] { "collection/map/Mappings.hbm.xml" }; + } + + public static Test suite() { + return new TestSuite( PersistentMapTest.class ); + } + + public void testWriteMethodDirtying() { + Parent parent = new Parent( "p1" ); + Child child = new Child( "c1" ); + parent.getChildren().put( child.getName(), child ); + child.setParent( parent ); + Child otherChild = new Child( "c2" ); + + Session session = openSession(); + session.beginTransaction(); + session.save( parent ); + session.flush(); + // at this point, the set on parent has now been replaced with a PersistentSet... + PersistentMap children = ( PersistentMap ) parent.getChildren(); + + Object old = children.put( child.getName(), child ); + assertTrue( old == child ); + assertFalse( children.isDirty() ); + + old = children.remove( otherChild.getName() ); + assertNull( old ); + assertFalse( children.isDirty() ); + + HashMap otherMap = new HashMap(); + otherMap.put( child.getName(), child ); + children.putAll( otherMap ); + assertFalse( children.isDirty() ); + + otherMap = new HashMap(); + otherMap.put( otherChild.getName(), otherChild ); + children.putAll( otherMap ); + assertTrue( children.isDirty() ); + + children.clearDirty(); + session.delete( child ); + children.clear(); + assertTrue( children.isDirty() ); + session.flush(); + + children.clear(); + assertFalse( children.isDirty() ); + + session.delete( parent ); + session.getTransaction().commit(); + session.close(); + } +} Added: branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/Child.java =================================================================== --- branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/Child.java 2006-09-01 20:52:35 UTC (rev 10421) +++ branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/Child.java 2006-09-01 20:56:40 UTC (rev 10422) @@ -0,0 +1,34 @@ +package org.hibernate.test.collection.set; + +/** + * todo: describe Child + * + * @author Steve Ebersole + */ +public class Child { + private String name; + private Parent parent; + + public Child() { + } + + public Child(String name) { + this.name = name; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public Parent getParent() { + return parent; + } + + public void setParent(Parent parent) { + this.parent = parent; + } +} Added: branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/Mappings.hbm.xml =================================================================== --- branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/Mappings.hbm.xml 2006-09-01 20:52:35 UTC (rev 10421) +++ branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/Mappings.hbm.xml 2006-09-01 20:56:40 UTC (rev 10422) @@ -0,0 +1,23 @@ +<?xml version="1.0"?> +<!DOCTYPE hibernate-mapping PUBLIC + "-//Hibernate/Hibernate Mapping DTD 3.0//EN" + "http://hibernate.sourceforge.net/hibernate-mapping-3.0.dtd"> + + +<hibernate-mapping package="org.hibernate.test.collection.set"> + + <class name="Parent"> + <id name="name" column="NAME" type="string" /> + + <set name="children" inverse="true" cascade="all"> + <key column="PARENT" /> + <one-to-many class="Child" /> + </set> + </class> + + <class name="Child"> + <id name="name" column="NAME" type="string"/> + <many-to-one name="parent" class="Parent" cascade="none" /> + </class> + +</hibernate-mapping> Added: branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/Parent.java =================================================================== --- branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/Parent.java 2006-09-01 20:52:35 UTC (rev 10421) +++ branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/Parent.java 2006-09-01 20:56:40 UTC (rev 10422) @@ -0,0 +1,37 @@ +package org.hibernate.test.collection.set; + +import java.util.Set; +import java.util.HashSet; + +/** + * todo: describe Parent + * + * @author Steve Ebersole + */ +public class Parent { + private String name; + private Set children = new HashSet(); + + public Parent() { + } + + public Parent(String name) { + this.name = name; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public Set getChildren() { + return children; + } + + public void setChildren(Set children) { + this.children = children; + } +} Added: branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/PersistentSetTest.java =================================================================== --- branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/PersistentSetTest.java 2006-09-01 20:52:35 UTC (rev 10421) +++ branches/Branch_3_2/Hibernate3/test/org/hibernate/test/collection/set/PersistentSetTest.java 2006-09-01 20:56:40 UTC (rev 10422) @@ -0,0 +1,76 @@ +package org.hibernate.test.collection.set; + +import org.hibernate.test.TestCase; +import org.hibernate.Session; +import org.hibernate.collection.PersistentSet; + +import java.util.HashSet; + +import junit.framework.Test; +import junit.framework.TestSuite; + +/** + * todo: describe PersistentSetTest + * + * @author Steve Ebersole + */ +public class PersistentSetTest extends TestCase { + public PersistentSetTest(String name) { + super( name ); + } + + protected String[] getMappings() { + return new String[] { "collection/set/Mappings.hbm.xml" }; + } + + public static Test suite() { + return new TestSuite( PersistentSetTest.class ); + } + + public void testWriteMethodDirtying() { + Parent parent = new Parent( "p1" ); + Child child = new Child( "c1" ); + parent.getChildren().add( child ); + child.setParent( parent ); + Child otherChild = new Child( "c2" ); + + Session session = openSession(); + session.beginTransaction(); + session.save( parent ); + session.flush(); + // at this point, the set on parent has now been replaced with a PersistentSet... + PersistentSet children = ( PersistentSet ) parent.getChildren(); + + assertFalse( children.add( child ) ); + assertFalse( children.isDirty() ); + + assertFalse( children.remove( otherChild ) ); + assertFalse( children.isDirty() ); + + HashSet otherSet = new HashSet(); + otherSet.add( child ); + assertFalse( children.addAll( otherSet ) ); + assertFalse( children.isDirty() ); + + assertFalse( children.retainAll( otherSet ) ); + assertFalse( children.isDirty() ); + + otherSet = new HashSet(); + otherSet.add( otherChild ); + assertFalse( children.removeAll( otherSet ) ); + assertFalse( children.isDirty() ); + + children.clear(); + session.delete( child ); + assertTrue( children.isDirty() ); + + session.flush(); + + children.clear(); + assertFalse( children.isDirty() ); + + session.delete( parent ); + session.getTransaction().commit(); + session.close(); + } +}