[hibernate-commits] Hibernate SVN: r14513 - in core/branches/Branch_3_2: src/org/hibernate/type and 1 other directories.

Author: gbadner Date: 2008-04-17 19:05:11 -0400 (Thu, 17 Apr 2008) New Revision: 14513 Modified: core/branches/Branch_3_2/src/org/hibernate/event/def/DefaultMergeEventListener.java core/branches/Branch_3_2/src/org/hibernate/type/EntityType.java core/branches/Branch_3_2/test/org/hibernate/test/cascade/MultiPathCascadeTest.java Log: HHH-3229 : Cascade merge transient entities regardless of property traversal order Modified: core/branches/Branch_3_2/src/org/hibernate/event/def/DefaultMergeEventListener.java =================================================================== --- core/branches/Branch_3_2/src/org/hibernate/event/def/DefaultMergeEventListener.java 2008-04-15 01:29:20 UTC (rev 14512) +++ core/branches/Branch_3_2/src/org/hibernate/event/def/DefaultMergeEventListener.java 2008-04-17 23:05:11 UTC (rev 14513) @@ -2,23 +2,27 @@ package org.hibernate.event.def; import java.io.Serializable; +import java.util.Iterator; import java.util.Map; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.hibernate.AssertionFailure; import org.hibernate.HibernateException; import org.hibernate.ObjectDeletedException; import org.hibernate.StaleObjectStateException; +import org.hibernate.TransientObjectException; import org.hibernate.WrongClassException; import org.hibernate.engine.Cascade; import org.hibernate.engine.CascadingAction; +import org.hibernate.engine.EntityEntry; +import org.hibernate.engine.EntityKey; +import org.hibernate.engine.SessionImplementor; +import org.hibernate.engine.Status; import org.hibernate.event.EventSource; import org.hibernate.event.MergeEvent; import org.hibernate.event.MergeEventListener; -import org.hibernate.engine.SessionImplementor; -import org.hibernate.engine.EntityEntry; -import org.hibernate.engine.EntityKey; import org.hibernate.intercept.FieldInterceptionHelper; import org.hibernate.intercept.FieldInterceptor; import org.hibernate.persister.entity.EntityPersister; @@ -43,14 +47,33 @@ return IdentityMap.invert( (Map) anything ); } - /** + /** * Handle the given merge event. * * @param event The merge event to be handled. * @throws HibernateException */ public void onMerge(MergeEvent event) throws HibernateException { - onMerge( event, IdentityMap.instantiate(10) ); + Map copyCache = IdentityMap.instantiate(10); + onMerge( event, copyCache ); + for ( Iterator it=copyCache.values().iterator(); it.hasNext(); ) { + Object entity = it.next(); + if ( entity instanceof HibernateProxy ) { + entity = ( (HibernateProxy) entity ).getHibernateLazyInitializer().getImplementation(); + } + EntityEntry entry = event.getSession().getPersistenceContext().getEntry( entity ); + if ( entry == null ) { + throw new TransientObjectException( + "object references an unsaved transient instance - save the transient instance before merging: " + + event.getSession().guessEntityName( entity ) + ); + // TODO: cache the entity name somewhere so that it is available to this exception + // entity name will not be available for non-POJO entities + } + if ( entry.getStatus() != Status.MANAGED ) { + throw new AssertionFailure( "Merged entity does not have status set to MANAGED; "+entry+" status="+entry.getStatus() ); + } + } } /** @@ -82,7 +105,8 @@ entity = original; } - if ( copyCache.containsKey(entity) ) { + if ( copyCache.containsKey(entity) && + source.getContextEntityIdentifier( copyCache.get( entity ) ) != null ) { log.trace("already merged"); event.setResult(entity); } @@ -126,7 +150,7 @@ entityIsPersistent(event, copyCache); break; default: //DELETED - throw new ObjectDeletedException( + throw new ObjectDeletedException( "deleted instance passed to merge", null, getLoggableName( event.getEntityName(), entity ) @@ -137,7 +161,7 @@ } } - + protected void entityIsPersistent(MergeEvent event, Map copyCache) { log.trace("ignoring persistent instance"); @@ -168,10 +192,15 @@ final Serializable id = persister.hasIdentifierProperty() ? persister.getIdentifier( entity, source.getEntityMode() ) : null; - - final Object copy = persister.instantiate( id, source.getEntityMode() ); //TODO: should this be Session.instantiate(Persister, ...)? - copyCache.put(entity, copy); //before cascade! - + if ( copyCache.containsKey( entity ) ) { + persister.setIdentifier( copyCache.get( entity ), id, source.getEntityMode() ); + } + else { + copyCache.put(entity, persister.instantiate( id, source.getEntityMode() ) ); //before cascade! + //TODO: should this be Session.instantiate(Persister, ...)? + } + final Object copy = copyCache.get( entity ); + // cascade first, so that all unsaved objects get their // copy created before we actually copy //cascadeOnMerge(event, persister, entity, copyCache, Cascades.CASCADE_BEFORE_MERGE); Modified: core/branches/Branch_3_2/src/org/hibernate/type/EntityType.java =================================================================== --- core/branches/Branch_3_2/src/org/hibernate/type/EntityType.java 2008-04-15 01:29:20 UTC (rev 14512) +++ core/branches/Branch_3_2/src/org/hibernate/type/EntityType.java 2008-04-17 23:05:11 UTC (rev 14513) @@ -250,13 +250,23 @@ if ( original == target ) { return target; } - Object id = getIdentifier( original, session ); - if ( id == null ) { - throw new AssertionFailure("cannot copy a reference to an object with a null id"); + if ( session.getContextEntityIdentifier( original ) == null && + ForeignKeys.isTransient( associatedEntityName, original, Boolean.FALSE, session ) ) { + final Object copy = session.getFactory().getEntityPersister( associatedEntityName ) + .instantiate( null, session.getEntityMode() ); + //TODO: should this be Session.instantiate(Persister, ...)? + copyCache.put( original, copy ); + return copy; } - id = getIdentifierOrUniqueKeyType( session.getFactory() ) - .replace(id, null, session, owner, copyCache); - return resolve( id, session, owner ); + else { + Object id = getIdentifier( original, session ); + if ( id == null ) { + throw new AssertionFailure("non-transient entity has a null id"); + } + id = getIdentifierOrUniqueKeyType( session.getFactory() ) + .replace(id, null, session, owner, copyCache); + return resolve( id, session, owner ); + } } } Modified: core/branches/Branch_3_2/test/org/hibernate/test/cascade/MultiPathCascadeTest.java =================================================================== --- core/branches/Branch_3_2/test/org/hibernate/test/cascade/MultiPathCascadeTest.java 2008-04-15 01:29:20 UTC (rev 14512) +++ core/branches/Branch_3_2/test/org/hibernate/test/cascade/MultiPathCascadeTest.java 2008-04-17 23:05:11 UTC (rev 14513) @@ -2,14 +2,13 @@ package org.hibernate.test.cascade; -import java.util.Collections; - import junit.framework.Test; import org.hibernate.Session; -import org.hibernate.Transaction; +import org.hibernate.TransientObjectException; import org.hibernate.junit.functional.FunctionalTestCase; import org.hibernate.junit.functional.FunctionalTestClassTestSuite; +import org.hibernate.proxy.HibernateProxy; /** * @author <a href="mailto:ovidiu@feodorov.com">Ovidiu Feodorov</a> @@ -33,15 +32,23 @@ return new FunctionalTestClassTestSuite( MultiPathCascadeTest.class ); } - public void testMultiPathMergeDetachedFailureExpected() throws Exception + protected void cleanupTest() { + Session s = openSession(); + s.beginTransaction(); + s.createQuery( "delete from A" ); + s.createQuery( "delete from G" ); + s.createQuery( "delete from H" ); + } + + public void testMultiPathMergeModifiedDetached() throws Exception { // persist a simple A in the database Session s = openSession(); s.beginTransaction(); A a = new A(); - a.setData("Anna"); - s.save(a); + a.setData( "Anna" ); + s.save( a ); s.getTransaction().commit(); s.close(); @@ -50,22 +57,22 @@ s = openSession(); s.beginTransaction(); - s.merge(a); + a = ( A ) s.merge( a ); s.getTransaction().commit(); s.close(); verifyModifications( a.getId() ); } - public void testMultiPathUpdateDetached() throws Exception + public void testMultiPathMergeModifiedDetachedIntoProxy() throws Exception { // persist a simple A in the database Session s = openSession(); s.beginTransaction(); A a = new A(); - a.setData("Anna"); - s.save(a); + a.setData( "Anna" ); + s.save( a ); s.getTransaction().commit(); s.close(); @@ -74,13 +81,39 @@ s = openSession(); s.beginTransaction(); - s.update(a); + A aLoaded = ( A ) s.load( A.class, new Long( a.getId() ) ); + assertTrue( aLoaded instanceof HibernateProxy ); + assertSame( aLoaded, s.merge( a ) ); s.getTransaction().commit(); s.close(); verifyModifications( a.getId() ); } + public void testMultiPathUpdateModifiedDetached() throws Exception + { + // persist a simple A in the database + + Session s = openSession(); + s.beginTransaction(); + A a = new A(); + a.setData( "Anna" ); + s.save( a ); + s.getTransaction().commit(); + s.close(); + + // modify detached entity + modifyEntity( a ); + + s = openSession(); + s.beginTransaction(); + s.update( a ); + s.getTransaction().commit(); + s.close(); + + verifyModifications( a.getId() ); + } + public void testMultiPathGetAndModify() throws Exception { // persist a simple A in the database @@ -88,8 +121,8 @@ Session s = openSession(); s.beginTransaction(); A a = new A(); - a.setData("Anna"); - s.save(a); + a.setData( "Anna" ); + s.save( a ); s.getTransaction().commit(); s.close(); @@ -104,24 +137,168 @@ verifyModifications( a.getId() ); } + public void testMultiPathMergeNonCascadedTransientEntityInCollection() throws Exception + { + // persist a simple A in the database + + Session s = openSession(); + s.beginTransaction(); + A a = new A(); + a.setData( "Anna" ); + s.save( a ); + s.getTransaction().commit(); + s.close(); + + // modify detached entity + modifyEntity( a ); + + s = openSession(); + s.beginTransaction(); + a = ( A ) s.merge( a ); + s.getTransaction().commit(); + s.close(); + + verifyModifications( a.getId() ); + + // add a new (transient) G to collection in h + // there is no cascade from H to the collection, so this should fail when merged + assertEquals( 1, a.getHs().size() ); + H h = ( H ) a.getHs().iterator().next(); + G gNew = new G(); + gNew.setData( "Gail" ); + gNew.getHs().add( h ); + h.getGs().add( gNew ); + + s = openSession(); + s.beginTransaction(); + try { + s.merge( a ); + s.merge( h ); + fail( "should have thrown TransientObjectException" ); + } + catch ( TransientObjectException ex ) { + // expected + } + finally { + s.getTransaction().rollback(); + } + s.close(); + } + + public void testMultiPathMergeNonCascadedTransientEntityInOneToOne() throws Exception + { + // persist a simple A in the database + + Session s = openSession(); + s.beginTransaction(); + A a = new A(); + a.setData( "Anna" ); + s.save( a ); + s.getTransaction().commit(); + s.close(); + + // modify detached entity + modifyEntity( a ); + + s = openSession(); + s.beginTransaction(); + a = ( A ) s.merge( a ); + s.getTransaction().commit(); + s.close(); + + verifyModifications( a.getId() ); + + // change the one-to-one association from g to be a new (transient) A + // there is no cascade from G to A, so this should fail when merged + G g = a.getG(); + a.setG( null ); + A aNew = new A(); + aNew.setData( "Alice" ); + g.setA( aNew ); + aNew.setG( g ); + + s = openSession(); + s.beginTransaction(); + try { + s.merge( a ); + s.merge( g ); + fail( "should have thrown TransientObjectException" ); + } + catch ( TransientObjectException ex ) { + // expected + } + finally { + s.getTransaction().rollback(); + } + s.close(); + } + + public void testMultiPathMergeNonCascadedTransientEntityInManyToOne() throws Exception + { + // persist a simple A in the database + + Session s = openSession(); + s.beginTransaction(); + A a = new A(); + a.setData( "Anna" ); + s.save( a ); + s.getTransaction().commit(); + s.close(); + + // modify detached entity + modifyEntity( a ); + + s = openSession(); + s.beginTransaction(); + a = ( A ) s.merge( a ); + s.getTransaction().commit(); + s.close(); + + verifyModifications( a.getId() ); + + // change the many-to-one association from h to be a new (transient) A + // there is no cascade from H to A, so this should fail when merged + assertEquals( 1, a.getHs().size() ); + H h = ( H ) a.getHs().iterator().next(); + a.getHs().remove( h ); + A aNew = new A(); + aNew.setData( "Alice" ); + aNew.addH( h ); + + s = openSession(); + s.beginTransaction(); + try { + s.merge( a ); + s.merge( h ); + fail( "should have thrown TransientObjectException" ); + } + catch ( TransientObjectException ex ) { + // expected + } + finally { + s.getTransaction().rollback(); + } + s.close(); + } + private void modifyEntity(A a) { // create a *circular* graph in detached entity a.setData("Anthony"); G g = new G(); - g.setData("Giovanni"); + g.setData( "Giovanni" ); H h = new H(); - h.setData("Hellen"); + h.setData( "Hellen" ); - a.setG(g); - g.setA(a); + a.setG( g ); + g.setA( a ); - a.getHs().add(h); - h.setA(a); + a.getHs().add( h ); + h.setA( a ); - g.getHs().add(h); - h.getGs().add(g); + g.getHs().add( h ); + h.getGs().add( g ); } private void verifyModifications(long aId) { @@ -157,4 +334,4 @@ s.close(); } -} \ No newline at end of file +}