[hibernate-issues] [Hibernate-JIRA] Commented: (HHH-3294) Version incorrectly incremented for unchanged persistent entity that is parent of a one to many relationship

Version incorrectly incremented for unchanged persistent entity that is parent of a one to many relationship ------------------------------------------------------------------------------------------------------------ Key: HHH-3294 URL: http://opensource.atlassian.com/projects/hibernate/browse/HHH-3294 Project: Hibernate3 Issue Type: Patch Components: core Affects Versions: 3.2.4.sp1 Environment: 3.2.4.sp1, Oracle 10g Reporter: Raymond Chapman Assignee: Gail Badner Fix For: 3.2.x, 3.3.0 Attachments: versionbumpissue.zip Given a persisted parent entity and a single persisted child entity involved in a one to many relationship where children are defined in parent HBM file to include merge and persist cascade action, we find that loading the parent and child in a transaction followed by a merge of the parent (no updates performed on either child or parent), and then a transaction commit will increment the version of the parent by 1. In attached zip I've isolated the problem in a new test testNoExtraUpdatesOnPersistentMergeVersionedWithCollection(), displayed below, added to org/hibernate/test/ops/MergeTest.java. It fails when parent is checked for updates following transaction commit. While the behavior was observed originally against 3.2.4.sp1, I can also reproduce in 3.2.6. In our system this issue causes org.hibernate.StaleObjectStateException exceptions when load is applied. I believe I've tracked down the origin of the failure. The merge(persistentParent) call results in child collection of parent being cleared, and consequently marked dirty. This happens when org.hibernate.type.CollectionType method replaceElements() is invoked as part of processing cascade action for merge on parent. It seems dirty flag should be reset after call to this method by CollectionType.replace(), performed when original==target. I see a similar effort was applied to replaceElements() but unfortunately this has no impact for this particular test scenario. I've applied a change to CollectionType.replace() to reset dirty flag if 'original' object was not dirty upon method entry. This resolves the issue for both 3.2.4.sp1 and 3.2.6 and I don't see any adverse impact to execution of Hibernate tests against HSQL. The updated CollectionType class is supplied in attached zip (deltas displayed below). Definition of children for parent in HBM definition OptLockEntity of org/hibernate/test/ops: <set name="children" inverse="true" cascade="persist,merge,save-update,evict,delete"> <key column="parent"/> <one-to-many class="VersionedEntity"/> </set> Index: test/org/hibernate/test/ops/MergeTest.java =================================================================== --- test/org/hibernate/test/ops/MergeTest.java (revision 14669) +++ test/org/hibernate/test/ops/MergeTest.java (working copy) @@ -248,6 +248,56 @@ cleanup(); } + public void testNoExtraUpdatesOnPersistentMergeVersionedWithCollection() throws Exception { + Session s = openSession(); + s.beginTransaction(); + VersionedEntity parent = new VersionedEntity( "parent", "parent" ); + VersionedEntity child = new VersionedEntity( "child", "child" ); + parent.getChildren().add( child ); + child.setParent( parent ); + s.persist( parent ); + s.getTransaction().commit(); + s.close(); + + clearCounts(); + + // parent is now detached, but we have made no changes. so attempt to merge it + // into this new session; this should cause no updates... + s = openSession(); + s.beginTransaction(); + // load parent so that merge will follow entityIsPersistent path + VersionedEntity persistentParent = ( VersionedEntity ) s.get( VersionedEntity.class, parent.getId() ); + // load children + VersionedEntity persistentChild = ( VersionedEntity ) persistentParent.getChildren().iterator().next(); + VersionedEntity mergedParent = ( VersionedEntity ) s.merge( persistentParent ); // <-- This merge leads to failure + s.getTransaction().commit(); + s.close(); + + assertUpdateCount( 0 ); + assertInsertCount( 0 ); + assertEquals( "unexpected parent version increment", parent.getVersion(), mergedParent.getVersion() ); + VersionedEntity mergedChild = ( VersionedEntity ) mergedParent.getChildren().iterator().next(); + assertEquals( "unexpected child version increment", child.getVersion(), mergedChild.getVersion() ); + + /////////////////////////////////////////////////////////////////////// + // as a control measure, now update the node once it is loaded and + // make sure we get an update as a result... + s = openSession(); + s.beginTransaction(); + persistentParent = ( VersionedEntity ) s.get( VersionedEntity.class, parent.getId() ); + persistentParent.setName( "new name" ); + persistentParent.getChildren().add( new VersionedEntity( "child2", "new child" ) ); + persistentParent = ( VersionedEntity ) s.merge( persistentParent ); + s.getTransaction().commit(); + s.close(); + assertUpdateCount( 1 ); + assertInsertCount( 1 ); + /////////////////////////////////////////////////////////////////////// + +// cleanup(); + } + + public void testPersistThenMergeInSameTxnWithVersion() { Session s = openSession(); Transaction tx = s.beginTransaction(); Index: src/org/hibernate/type/CollectionType.java =================================================================== --- src/org/hibernate/type/CollectionType.java (revision 14669) +++ src/org/hibernate/type/CollectionType.java (working copy) @@ -521,7 +521,18 @@ //get the elements back into the target //TODO: this is a little inefficient, don't need to do a whole // deep replaceElements() call - replaceElements( result, target, owner, copyCache, session ); + if ( target instanceof PersistentCollection ) { + if ( ! ( ( PersistentCollection ) target ).isDirty() ) { + replaceElements( result, target, owner, copyCache, session ); + ( ( PersistentCollection ) target ).clearDirty(); + } + else { + replaceElements( result, target, owner, copyCache, session ); + } + } + else { + replaceElements( result, target, owner, copyCache, session ); + } result = target; }